Fix #2108: FlammableBlockRegistry ignores tags unless /reload (#2326)

fabric-content-registries-v0/src/main/java/net/fabricmc/fabric/impl/content/registry/FlammableBlockRegistryImpl.java

@@ -16,78 +16,63 @@
 
 package net.fabricmc.fabric.impl.content.registry;
 
-import java.util.Collection;
-import java.util.Collections;
 import java.util.HashMap;
+import java.util.IdentityHashMap;
 import java.util.Map;
 
 import net.minecraft.block.Block;
-import net.minecraft.resource.ResourceManager;
-import net.minecraft.resource.ResourceType;
 import net.minecraft.tag.TagKey;
-import net.minecraft.util.Identifier;
 import net.minecraft.util.registry.Registry;
 import net.minecraft.util.registry.RegistryEntry;
 
+import net.fabricmc.fabric.api.event.lifecycle.v1.CommonLifecycleEvents;
 import net.fabricmc.fabric.api.registry.FlammableBlockRegistry;
-import net.fabricmc.fabric.api.resource.ResourceManagerHelper;
-import net.fabricmc.fabric.api.resource.ResourceReloadListenerKeys;
-import net.fabricmc.fabric.api.resource.SimpleSynchronousResourceReloadListener;
 
-public class FlammableBlockRegistryImpl implements FlammableBlockRegistry, SimpleSynchronousResourceReloadListener {
+public class FlammableBlockRegistryImpl implements FlammableBlockRegistry {
 	private static final FlammableBlockRegistry.Entry REMOVED = new FlammableBlockRegistry.Entry(0, 0);
 	private static final Map<Block, FlammableBlockRegistryImpl> REGISTRIES = new HashMap<>();
-	private static final Collection<Identifier> RELOAD_DEPS = Collections.singletonList(ResourceReloadListenerKeys.TAGS);
-	private static int idCounter = 0;
 
 	private final Map<Block, FlammableBlockRegistry.Entry> registeredEntriesBlock = new HashMap<>();
 	private final Map<TagKey<Block>, FlammableBlockRegistry.Entry> registeredEntriesTag = new HashMap<>();
-	private final Map<Block, FlammableBlockRegistry.Entry> computedEntries = new HashMap<>();
-	private final Identifier id;
+	private volatile Map<Block, FlammableBlockRegistry.Entry> computedEntries = null;
 	private final Block key;
-	private boolean tagsPresent = false;
 
 	private FlammableBlockRegistryImpl(Block key) {
-		ResourceManagerHelper.get(ResourceType.SERVER_DATA).registerReloadListener(this);
-		this.id = new Identifier("fabric:private/fire_registry_" + (++idCounter));
 		this.key = key;
+
+		// Reset computed values after tags change since they depends on tags.
+		CommonLifecycleEvents.TAGS_LOADED.register((registries, client) -> {
+			computedEntries = null;
+		});
 	}
 
-	// TODO: Asynchronous?
-	@Override
-	public void reload(ResourceManager var1) {
-		reload();
-		tagsPresent = true;
-	}
+	private Map<Block, FlammableBlockRegistry.Entry> getEntryMap() {
+		Map<Block, FlammableBlockRegistry.Entry> ret = computedEntries;
 
-	private void reload() {
-		computedEntries.clear();
+		if (ret == null) {
+			ret = new IdentityHashMap<>();
 
-		// tags take precedence before blocks
-		for (TagKey<Block> tag : registeredEntriesTag.keySet()) {
-			FlammableBlockRegistry.Entry entry = registeredEntriesTag.get(tag);
+			// tags take precedence over blocks
+			for (TagKey<Block> tag : registeredEntriesTag.keySet()) {
+				FlammableBlockRegistry.Entry entry = registeredEntriesTag.get(tag);
 
-			for (RegistryEntry<Block> block : Registry.BLOCK.iterateEntries(tag)) {
-				computedEntries.put(block.value(), entry);
+				for (RegistryEntry<Block> block : Registry.BLOCK.iterateEntries(tag)) {
+					ret.put(block.value(), entry);
+				}
 			}
+
+			ret.putAll(registeredEntriesBlock);
+
+			computedEntries = ret;
 		}
 
-		computedEntries.putAll(registeredEntriesBlock);
-
-		/* computedBurnChances.clear();
-		computedSpreadChances.clear();
-
-		for (Block block : computedEntries.keySet()) {
-			FlammableBlockRegistry.Entry entry = computedEntries.get(block);
-			computedBurnChances.put(block, entry.getBurnChance());
-			computedSpreadChances.put(block, entry.getSpreadChance());
-		} */
+		return ret;
 	}
 
 	// User-facing fire registry interface - queries vanilla fire block
 	@Override
 	public Entry get(Block block) {
-		Entry entry = computedEntries.get(block);
+		Entry entry = getEntryMap().get(block);
 
 		if (entry != null) {
 			return entry;
@@ -97,25 +82,21 @@ public class FlammableBlockRegistryImpl implements FlammableBlockRegistry, Simpl
 	}
 
 	public Entry getFabric(Block block) {
-		return computedEntries.get(block);
+		return getEntryMap().get(block);
 	}
 
 	@Override
 	public void add(Block block, Entry value) {
 		registeredEntriesBlock.put(block, value);
 
-		if (tagsPresent) {
-			reload();
-		}
+		computedEntries = null;
 	}
 
 	@Override
 	public void add(TagKey<Block> tag, Entry value) {
 		registeredEntriesTag.put(tag, value);
 
-		if (tagsPresent) {
-			reload();
-		}
+		computedEntries = null;
 	}
 
 	@Override
@@ -132,18 +113,14 @@ public class FlammableBlockRegistryImpl implements FlammableBlockRegistry, Simpl
 	public void clear(Block block) {
 		registeredEntriesBlock.remove(block);
 
-		if (tagsPresent) {
-			reload();
-		}
+		computedEntries = null;
 	}
 
 	@Override
 	public void clear(TagKey<Block> tag) {
 		registeredEntriesTag.remove(tag);
 
-		if (tagsPresent) {
-			reload();
-		}
+		computedEntries = null;
 	}
 
 	public static FlammableBlockRegistryImpl getInstance(Block block) {
@@ -153,14 +130,4 @@ public class FlammableBlockRegistryImpl implements FlammableBlockRegistry, Simpl
 
 		return REGISTRIES.computeIfAbsent(block, FlammableBlockRegistryImpl::new);
 	}
-
-	@Override
-	public Identifier getFabricId() {
-		return id;
-	}
-
-	@Override
-	public Collection<Identifier> getFabricDependencies() {
-		return RELOAD_DEPS;
-	}
 }

fabric-content-registries-v0/src/testmod/java/net/fabricmc/fabric/test/content/registry/ContentRegistryTest.java

@@ -22,10 +22,12 @@ import org.slf4j.LoggerFactory;
 import net.minecraft.block.Blocks;
 import net.minecraft.item.HoeItem;
 import net.minecraft.item.Items;
+import net.minecraft.tag.BlockTags;
 import net.minecraft.util.Identifier;
 import net.minecraft.village.VillagerProfession;
 
 import net.fabricmc.api.ModInitializer;
+import net.fabricmc.fabric.api.registry.FlammableBlockRegistry;
 import net.fabricmc.fabric.api.registry.FlattenableBlockRegistry;
 import net.fabricmc.fabric.api.registry.OxidizableBlocksRegistry;
 import net.fabricmc.fabric.api.registry.StrippableBlockRegistry;
@@ -47,7 +49,9 @@ public final class ContentRegistryTest implements ModInitializer {
 		//  - villagers can now collect, consume (at the same level of bread) and compost apples
 		//  - villagers can now collect and plant oak saplings
 		//  - assign a loot table to the nitwit villager type
+		//  - sand is now flammable
 
+		FlammableBlockRegistry.getDefaultInstance().add(BlockTags.SAND, 4, 4);
 		FlattenableBlockRegistry.register(Blocks.RED_WOOL, Blocks.YELLOW_WOOL.getDefaultState());
 		StrippableBlockRegistry.register(Blocks.QUARTZ_PILLAR, Blocks.HAY_BLOCK);
 

fabric-content-registries-v0/src/testmod/java/net/fabricmc/fabric/test/content/registry/FlammableTest.java

@@ -0,0 +1,39 @@
+/*
+ * 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.content.registry;
+
+import net.minecraft.block.Blocks;
+import net.minecraft.test.GameTest;
+import net.minecraft.test.GameTestException;
+import net.minecraft.test.TestContext;
+
+import net.fabricmc.fabric.api.gametest.v1.FabricGameTest;
+import net.fabricmc.fabric.api.registry.FlammableBlockRegistry;
+
+public class FlammableTest {
+	/**
+	 * Regression test for <a href="https://github.com/FabricMC/fabric/issues/2108">FlammableBlockRegistry ignoring tags on first load</a>.
+	 */
+	@GameTest(structureName = FabricGameTest.EMPTY_STRUCTURE)
+	public void testFlammableTag(TestContext context) {
+		if (FlammableBlockRegistry.getDefaultInstance().get(Blocks.SAND).getBurnChance() != 4) {
+			throw new GameTestException("Expected blocks in the sand tag to be flammable!");
+		}
+
+		context.complete();
+	}
+}

fabric-content-registries-v0/src/testmod/resources/fabric.mod.json

@@ -11,6 +11,9 @@
   "entrypoints": {
     "main": [
       "net.fabricmc.fabric.test.content.registry.ContentRegistryTest"
+    ],
+    "fabric-gametest": [
+      "net.fabricmc.fabric.test.content.registry.FlammableTest"
     ]
   }
 }