Object builder API updates (#2798)

* Add feature support to FabricEntityTypeBuilder

* Use IdentityHashMap for EntityType-keyed maps

* Improve tests

* Update fabric-object-builder-api-v1/src/main/java/net/fabricmc/fabric/api/object/builder/v1/entity/FabricEntityTypeBuilder.java

Co-authored-by: Juuz <6596629+Juuxel@users.noreply.github.com>

* Fix naming of static-final field

* Address some reviews

* Rebuild

* Split gametest

* Update fabric-object-builder-api-v1/src/main/java/net/fabricmc/fabric/api/object/builder/v1/entity/MinecartComparatorLogicRegistry.java

Co-authored-by: Juuz <6596629+Juuxel@users.noreply.github.com>

fabric-object-builder-api-v1/src/main/java/net/fabricmc/fabric/api/object/builder/v1/block/FabricBlockSettings.java

@@ -48,6 +48,10 @@ public class FabricBlockSettings extends AbstractBlock.Settings {
 		super(material, color);
 	}
 
+	protected FabricBlockSettings(Material material, Function<BlockState, MapColor> mapColorProvider) {
+		super(material, mapColorProvider);
+	}
+
 	protected FabricBlockSettings(AbstractBlock.Settings settings) {
 		super(((AbstractBlockSettingsAccessor) settings).getMaterial(), ((AbstractBlockSettingsAccessor) settings).getMapColorProvider());
 		// Mostly Copied from vanilla's copy method
@@ -97,6 +101,10 @@ public class FabricBlockSettings extends AbstractBlock.Settings {
 		return new FabricBlockSettings(material, color.getMapColor());
 	}
 
+	public static FabricBlockSettings of(Material material, Function<BlockState, MapColor> mapColor) {
+		return new FabricBlockSettings(material, mapColor);
+	}
+
 	public static FabricBlockSettings copyOf(AbstractBlock block) {
 		return new FabricBlockSettings(((AbstractBlockAccessor) block).getSettings());
 	}

fabric-object-builder-api-v1/src/main/java/net/fabricmc/fabric/api/object/builder/v1/entity/FabricEntityTypeBuilder.java

@@ -31,7 +31,9 @@ import net.minecraft.entity.SpawnGroup;
 import net.minecraft.entity.SpawnRestriction;
 import net.minecraft.entity.attribute.DefaultAttributeContainer;
 import net.minecraft.entity.mob.MobEntity;
+import net.minecraft.resource.featuretoggle.FeatureFlag;
 import net.minecraft.resource.featuretoggle.FeatureFlags;
+import net.minecraft.resource.featuretoggle.FeatureSet;
 import net.minecraft.world.Heightmap;
 import net.minecraft.world.World;
 
@@ -56,6 +58,8 @@ public class FabricEntityTypeBuilder<T extends Entity> {
 	private EntityDimensions dimensions = EntityDimensions.changing(-1.0f, -1.0f);
 	private ImmutableSet<Block> specificSpawnBlocks = ImmutableSet.of();
 
+	private FeatureSet requiredFeatures = FeatureFlags.VANILLA_FEATURES;
+
 	protected FabricEntityTypeBuilder(SpawnGroup spawnGroup, EntityType.EntityFactory<T> factory) {
 		this.spawnGroup = spawnGroup;
 		this.factory = factory;
@@ -251,20 +255,26 @@ public class FabricEntityTypeBuilder<T extends Entity> {
 		return this;
 	}
 
+	/**
+	 * Sets the features this entity requires. If a feature is not enabled,
+	 * the entity cannot be spawned, and existing ones will despawn immediately.
+	 * @param requiredFeatures the features
+	 * @return this builder for chaining
+	 */
+	public FabricEntityTypeBuilder<T> requires(FeatureFlag... requiredFeatures) {
+		this.requiredFeatures = FeatureFlags.FEATURE_MANAGER.featureSetOf(requiredFeatures);
+		return this;
+	}
+
 	/**
 	 * Creates the entity type.
 	 *
 	 * @return a new {@link EntityType}
 	 */
 	public EntityType<T> build() {
-		if (this.saveable) {
-			// SNIP! Modded datafixers are not supported anyway.
-			// TODO: Flesh out once modded datafixers exist.
-		}
+		// Modded DFU is a dream, currently not possible without screwing it up.
 
-		EntityType<T> type = new FabricEntityType<>(this.factory, this.spawnGroup, this.saveable, this.summonable, this.fireImmune, this.spawnableFarFromPlayer, this.specificSpawnBlocks, dimensions, trackRange, trackedUpdateRate, forceTrackedVelocityUpdates, FeatureFlags.DEFAULT_ENABLED_FEATURES);
-
-		return type;
+		return new FabricEntityType<>(this.factory, this.spawnGroup, this.saveable, this.summonable, this.fireImmune, this.spawnableFarFromPlayer, this.specificSpawnBlocks, dimensions, trackRange, trackedUpdateRate, forceTrackedVelocityUpdates, this.requiredFeatures);
 	}
 
 	/**

fabric-object-builder-api-v1/src/main/java/net/fabricmc/fabric/api/object/builder/v1/entity/MinecartComparatorLogicRegistry.java

@@ -16,23 +16,24 @@
 
 package net.fabricmc.fabric.api.object.builder.v1.entity;
 
-import java.util.HashMap;
+import java.util.IdentityHashMap;
 import java.util.Map;
+import java.util.Objects;
 
 import org.jetbrains.annotations.Nullable;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import net.minecraft.registry.Registries;
 import net.minecraft.entity.EntityType;
 import net.minecraft.entity.vehicle.AbstractMinecartEntity;
+import net.minecraft.registry.Registries;
 
 /**
  * A registry for {@linkplain MinecartComparatorLogic custom minecart comparator logic}.
  */
 public final class MinecartComparatorLogicRegistry {
 	private static final Logger LOGGER = LoggerFactory.getLogger(MinecartComparatorLogicRegistry.class);
-	private static final Map<EntityType<?>, MinecartComparatorLogic<?>> logics = new HashMap<>();
+	private static final Map<EntityType<?>, MinecartComparatorLogic<?>> LOGICS = new IdentityHashMap<>();
 
 	private MinecartComparatorLogicRegistry() {
 	}
@@ -46,7 +47,7 @@ public final class MinecartComparatorLogicRegistry {
 	@Nullable
 	@SuppressWarnings("unchecked")
 	public static MinecartComparatorLogic<AbstractMinecartEntity> getCustomComparatorLogic(EntityType<?> type) {
-		return (MinecartComparatorLogic<AbstractMinecartEntity>) logics.get(type);
+		return (MinecartComparatorLogic<AbstractMinecartEntity>) LOGICS.get(type);
 	}
 
 	/**
@@ -59,7 +60,10 @@ public final class MinecartComparatorLogicRegistry {
 	 * @param logic the logic to register
 	 */
 	public static <T extends AbstractMinecartEntity> void register(EntityType<T> type, MinecartComparatorLogic<? super T> logic) {
-		if (logics.put(type, logic) != null) {
+		Objects.requireNonNull(type, "Entity type cannot be null");
+		Objects.requireNonNull(logic, "Logic cannot be null");
+
+		if (LOGICS.put(type, logic) != null) {
 			LOGGER.warn("Overriding existing minecart comparator logic for entity type {}", Registries.ENTITY_TYPE.getId(type));
 		}
 	}

fabric-object-builder-api-v1/src/main/java/net/fabricmc/fabric/api/object/builder/v1/trade/TradeOfferHelper.java

@@ -59,7 +59,7 @@ public final class TradeOfferHelper {
 	/**
 	 * @deprecated This never did anything useful.
 	 */
-	@Deprecated
+	@Deprecated(forRemoval = true)
 	public static void refreshOffers() {
 		TradeOfferInternals.printRefreshOffersWarning();
 	}

fabric-object-builder-api-v1/src/main/java/net/fabricmc/fabric/mixin/object/builder/DefaultAttributeRegistryMixin.java

@@ -16,7 +16,7 @@
 
 package net.fabricmc.fabric.mixin.object.builder;
 
-import java.util.HashMap;
+import java.util.IdentityHashMap;
 import java.util.Map;
 
 import org.spongepowered.asm.mixin.Final;
@@ -41,6 +41,6 @@ public abstract class DefaultAttributeRegistryMixin {
 
 	@Inject(method = "<clinit>*", at = @At("TAIL"))
 	private static void injectAttributes(CallbackInfo ci) {
-		DEFAULT_ATTRIBUTE_REGISTRY = new HashMap<>(DEFAULT_ATTRIBUTE_REGISTRY);
+		DEFAULT_ATTRIBUTE_REGISTRY = new IdentityHashMap<>(DEFAULT_ATTRIBUTE_REGISTRY);
 	}
 }

fabric-object-builder-api-v1/src/testmod/java/net/fabricmc/fabric/test/object/builder/BlockEntityTypeBuilderTest.java

@@ -27,14 +27,14 @@ import net.minecraft.block.entity.BlockEntityType;
 import net.minecraft.entity.player.PlayerEntity;
 import net.minecraft.item.BlockItem;
 import net.minecraft.item.Item;
+import net.minecraft.registry.Registries;
+import net.minecraft.registry.Registry;
 import net.minecraft.text.Text;
 import net.minecraft.util.ActionResult;
 import net.minecraft.util.Hand;
 import net.minecraft.util.Identifier;
 import net.minecraft.util.hit.BlockHitResult;
 import net.minecraft.util.math.BlockPos;
-import net.minecraft.registry.Registries;
-import net.minecraft.registry.Registry;
 import net.minecraft.world.World;
 
 import net.fabricmc.api.ModInitializer;
@@ -42,16 +42,16 @@ import net.fabricmc.fabric.api.object.builder.v1.block.entity.FabricBlockEntityT
 
 public class BlockEntityTypeBuilderTest implements ModInitializer {
 	private static final Identifier INITIAL_BETRAYAL_BLOCK_ID = ObjectBuilderTestConstants.id("initial_betrayal_block");
-	private static final Block INITIAL_BETRAYAL_BLOCK = new BetrayalBlock(MapColor.BLUE);
+	static final Block INITIAL_BETRAYAL_BLOCK = new BetrayalBlock(MapColor.BLUE);
 
 	private static final Identifier ADDED_BETRAYAL_BLOCK_ID = ObjectBuilderTestConstants.id("added_betrayal_block");
-	private static final Block ADDED_BETRAYAL_BLOCK = new BetrayalBlock(MapColor.GREEN);
+	static final Block ADDED_BETRAYAL_BLOCK = new BetrayalBlock(MapColor.GREEN);
 
 	private static final Identifier FIRST_MULTI_BETRAYAL_BLOCK_ID = ObjectBuilderTestConstants.id("first_multi_betrayal_block");
-	private static final Block FIRST_MULTI_BETRAYAL_BLOCK = new BetrayalBlock(MapColor.RED);
+	static final Block FIRST_MULTI_BETRAYAL_BLOCK = new BetrayalBlock(MapColor.RED);
 
 	private static final Identifier SECOND_MULTI_BETRAYAL_BLOCK_ID = ObjectBuilderTestConstants.id("second_multi_betrayal_block");
-	private static final Block SECOND_MULTI_BETRAYAL_BLOCK = new BetrayalBlock(MapColor.YELLOW);
+	static final Block SECOND_MULTI_BETRAYAL_BLOCK = new BetrayalBlock(MapColor.YELLOW);
 
 	private static final Identifier BLOCK_ENTITY_TYPE_ID = ObjectBuilderTestConstants.id("betrayal_block");
 	public static final BlockEntityType<?> BLOCK_ENTITY_TYPE = FabricBlockEntityTypeBuilder.create(BetrayalBlockEntity::new, INITIAL_BETRAYAL_BLOCK)

fabric-object-builder-api-v1/src/testmod/java/net/fabricmc/fabric/test/object/builder/ObjectBuilderGameTest.java

@@ -0,0 +1,42 @@
+/*
+ * Copyright (c) 2016, 2017, 2018, 2019 FabricMC
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package net.fabricmc.fabric.test.object.builder;
+
+import java.util.List;
+
+import net.minecraft.block.Block;
+import net.minecraft.test.GameTest;
+import net.minecraft.test.TestContext;
+import net.minecraft.util.math.BlockPos;
+
+import net.fabricmc.fabric.api.gametest.v1.FabricGameTest;
+
+public class ObjectBuilderGameTest {
+	@GameTest(templateName = FabricGameTest.EMPTY_STRUCTURE)
+	public void testBlockUse(TestContext context) {
+		List<Block> blocks = List.of(BlockEntityTypeBuilderTest.INITIAL_BETRAYAL_BLOCK, BlockEntityTypeBuilderTest.ADDED_BETRAYAL_BLOCK, BlockEntityTypeBuilderTest.FIRST_MULTI_BETRAYAL_BLOCK, BlockEntityTypeBuilderTest.SECOND_MULTI_BETRAYAL_BLOCK);
+		BlockPos.Mutable pos = BlockPos.ORIGIN.mutableCopy();
+
+		for (Block block : blocks) {
+			context.setBlockState(pos, block);
+			context.useBlock(pos);
+			pos.up();
+		}
+
+		context.complete();
+	}
+}

fabric-object-builder-api-v1/src/testmod/java/net/fabricmc/fabric/test/object/builder/VillagerTypeTest1.java

@@ -49,12 +49,6 @@ public class VillagerTypeTest1 implements ModInitializer {
 		});
 
 		CommandRegistrationCallback.EVENT.register((dispatcher, registryAccess, environment) -> {
-			dispatcher.register(literal("fabric_refreshtrades").executes(context -> {
-				TradeOfferHelper.refreshOffers();
-				context.getSource().sendFeedback(Text.literal("Refreshed trades"), false);
-				return 1;
-			}));
-
 			dispatcher.register(literal("fabric_applywandering_trades")
 					.then(argument("entity", entity()).executes(context -> {
 						final Entity entity = getEntity(context, "entity");

fabric-object-builder-api-v1/src/testmod/resources/fabric.mod.json

@@ -26,6 +26,9 @@
       "net.fabricmc.fabric.test.object.builder.CriterionRegistryTest::init",
       "net.fabricmc.fabric.test.object.builder.VillagerTypeTest1",
       "net.fabricmc.fabric.test.object.builder.VillagerTypeTest2"
+    ],
+    "fabric-gametest": [
+      "net.fabricmc.fabric.test.object.builder.ObjectBuilderGameTest"
     ]
   }
 }