Make BuiltInRegistries safe for Worldgen Registration during Mod Initialization (#1052)

* Adds a synchronisation for entries in BuiltInRegistries to the built-in DynamicRegistryManager, to a void class-loading DynamicRegistryManager during Mod initialization from messing up the Worldgen registrations of subsequently loaded mods.

* Changed to use updated Yarn mappings.

fabric-registry-sync-v0/src/main/java/net/fabricmc/fabric/impl/registry/sync/DynamicRegistrySync.java

@@ -0,0 +1,66 @@
+/*
+ * 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.impl.registry.sync;
+
+import com.mojang.serialization.Lifecycle;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+
+import net.minecraft.util.registry.BuiltinRegistries;
+import net.minecraft.util.registry.DynamicRegistryManager;
+import net.minecraft.util.registry.MutableRegistry;
+import net.minecraft.util.registry.Registry;
+import net.minecraft.util.registry.RegistryKey;
+
+import net.fabricmc.fabric.api.event.registry.RegistryEntryAddedCallback;
+import net.fabricmc.fabric.mixin.registry.sync.AccessorRegistry;
+
+/**
+ * Handles synchronising changes to the built-in registries into the dynamic registry manager's template manager,
+ * in case it gets classloaded early.
+ */
+public class DynamicRegistrySync {
+	private static final Logger LOGGER = LogManager.getLogger();
+
+	/**
+	 * Sets up a synchronisation that will propagate added entries to the given dynamic registry manager, which
+	 * should be the <em>built-in</em> manager. It is never destroyed. We don't ever have to unregister
+	 * the registry events.
+	 */
+	public static void setupSync(DynamicRegistryManager.Impl template) {
+		LOGGER.debug("Setting up synchronisation of new BuiltinRegistries entries to the built-in DynamicRegistryManager");
+		BuiltinRegistries.REGISTRIES.stream().forEach(source -> setupSync(source, template));
+	}
+
+	/**
+	 * Sets up an event registration for the source registy that will ensure all entries added from now on
+	 * are also added to the template for dynamic registry managers.
+	 */
+	private static <T> void setupSync(Registry<T> source, DynamicRegistryManager.Impl template) {
+		@SuppressWarnings("unchecked") AccessorRegistry<T> sourceAccessor = (AccessorRegistry<T>) source;
+		RegistryKey<? extends Registry<T>> sourceKey = source.getKey();
+		MutableRegistry<T> target = template.get(sourceKey);
+
+		RegistryEntryAddedCallback.event(source).register((rawId, id, object) -> {
+			LOGGER.trace("Synchronizing {} from built-in registry {} into built-in dynamic registry manager template.",
+					id, source.getKey());
+			Lifecycle lifecycle = sourceAccessor.callGetEntryLifecycle(object);
+			RegistryKey<T> entryKey = RegistryKey.of(sourceKey, id);
+			target.set(rawId, entryKey, object, lifecycle);
+		});
+	}
+}

fabric-registry-sync-v0/src/main/java/net/fabricmc/fabric/mixin/registry/sync/AccessorRegistry.java

@@ -16,8 +16,10 @@
 
 package net.fabricmc.fabric.mixin.registry.sync;
 
+import com.mojang.serialization.Lifecycle;
 import org.spongepowered.asm.mixin.Mixin;
 import org.spongepowered.asm.mixin.gen.Accessor;
+import org.spongepowered.asm.mixin.gen.Invoker;
 
 import net.minecraft.util.registry.MutableRegistry;
 import net.minecraft.util.registry.Registry;
@@ -32,4 +34,7 @@ public interface AccessorRegistry<T> {
 
 	@Accessor()
 	RegistryKey<Registry<T>> getRegistryKey();
+
+	@Invoker
+	Lifecycle callGetEntryLifecycle(T object);
 }

fabric-registry-sync-v0/src/main/java/net/fabricmc/fabric/mixin/registry/sync/MixinDynamicRegistryManager.java

@@ -0,0 +1,45 @@
+/*
+ * 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.mixin.registry.sync;
+
+import org.spongepowered.asm.mixin.Mixin;
+import org.spongepowered.asm.mixin.Shadow;
+import org.spongepowered.asm.mixin.injection.At;
+import org.spongepowered.asm.mixin.injection.Inject;
+import org.spongepowered.asm.mixin.injection.callback.CallbackInfo;
+
+import net.minecraft.util.registry.DynamicRegistryManager;
+
+import net.fabricmc.fabric.impl.registry.sync.DynamicRegistrySync;
+
+@Mixin(DynamicRegistryManager.class)
+public class MixinDynamicRegistryManager {
+	// This is the "template" for all subsequent built-in dynamic registry managers,
+	// but it still contains the same objects as BuiltinRegistries, while the subsequent
+	// managers built from this template will contain copies.
+	@Shadow
+	private static DynamicRegistryManager.Impl BUILTIN;
+
+	/**
+	 * Ensures that any registrations made into {@link net.minecraft.util.registry.BuiltinRegistries} after
+	 * {@link DynamicRegistryManager} has been class-loaded are still propagated.
+	 */
+	@Inject(method = "<clinit>", at = @At(value = "TAIL"))
+	private static void setupBuiltInSync(CallbackInfo ci) {
+		DynamicRegistrySync.setupSync(BUILTIN);
+	}
+}

fabric-registry-sync-v0/src/main/resources/fabric-registry-sync-v0.mixins.json

@@ -5,6 +5,7 @@
   "mixins": [
     "AccessorRegistry",
     "MixinBootstrap",
+    "MixinDynamicRegistryManager",
     "MixinIdList",
     "MixinIdRegistry",
     "MixinPlayerManager",

fabric-registry-sync-v0/src/testmod/java/net/fabricmc/fabric/test/registry/sync/RegistrySyncTest.java

@@ -24,13 +24,19 @@ import net.minecraft.block.Material;
 import net.minecraft.item.BlockItem;
 import net.minecraft.item.Item;
 import net.minecraft.util.Identifier;
+import net.minecraft.util.registry.BuiltinRegistries;
+import net.minecraft.util.registry.DynamicRegistryManager;
+import net.minecraft.util.registry.MutableRegistry;
 import net.minecraft.util.registry.Registry;
 import net.minecraft.util.registry.SimpleRegistry;
+import net.minecraft.world.gen.feature.ConfiguredFeature;
+import net.minecraft.world.gen.feature.DefaultFeatureConfig;
+import net.minecraft.world.gen.feature.Feature;
 
 import net.fabricmc.api.ModInitializer;
+import net.fabricmc.fabric.api.event.registry.FabricRegistryBuilder;
 import net.fabricmc.fabric.api.event.registry.RegistryAttribute;
 import net.fabricmc.fabric.api.event.registry.RegistryAttributeHolder;
-import net.fabricmc.fabric.api.event.registry.FabricRegistryBuilder;
 
 public class RegistrySyncTest implements ModInitializer {
 	/**
@@ -41,6 +47,8 @@ public class RegistrySyncTest implements ModInitializer {
 
 	@Override
 	public void onInitialize() {
+		testBuiltInRegistrySync();
+
 		if (REGISTER_BLOCKS) {
 			for (int i = 0; i < 5; i++) {
 				Block block = new Block(AbstractBlock.Settings.of(Material.STONE));
@@ -69,4 +77,53 @@ public class RegistrySyncTest implements ModInitializer {
 		Validate.isTrue(RegistryAttributeHolder.get(fabricRegistry).hasAttribute(RegistryAttribute.SYNCED));
 		Validate.isTrue(!RegistryAttributeHolder.get(fabricRegistry).hasAttribute(RegistryAttribute.PERSISTED));
 	}
+
+	/**
+	 * Tests that built-in registries are properly synchronized even after the dynamic reigstry managers have been
+	 * class-loaded.
+	 */
+	private void testBuiltInRegistrySync() {
+		System.out.println("Checking built-in registry sync...");
+
+		// Register a configured feature before force-loading the dynamic registry manager
+		ConfiguredFeature<DefaultFeatureConfig, ?> cf1 = Feature.BASALT_PILLAR.configure(DefaultFeatureConfig.INSTANCE);
+		Identifier f1Id = new Identifier("registry_sync", "f1");
+		Registry.register(BuiltinRegistries.CONFIGURED_FEATURE, f1Id, cf1);
+
+		// Force-Initialize the dynamic registry manager, doing this in a Mod initializer would cause
+		// further registrations into BuiltInRegistries to _NOT_ propagate into DynamicRegistryManager.BUILTIN
+		checkFeature(DynamicRegistryManager.create(), f1Id);
+
+		ConfiguredFeature<DefaultFeatureConfig, ?> cf2 = Feature.DESERT_WELL.configure(DefaultFeatureConfig.INSTANCE);
+		Identifier f2Id = new Identifier("registry_sync", "f2");
+		Registry.register(BuiltinRegistries.CONFIGURED_FEATURE, f2Id, cf2);
+
+		DynamicRegistryManager.Impl impl2 = DynamicRegistryManager.create();
+		checkFeature(impl2, f1Id);
+		checkFeature(impl2, f2Id);
+	}
+
+	private void checkFeature(DynamicRegistryManager manager, Identifier id) {
+		MutableRegistry<ConfiguredFeature<?, ?>> registry = manager.get(Registry.CONFIGURED_FEATURE_WORLDGEN);
+
+		ConfiguredFeature<?, ?> builtInEntry = BuiltinRegistries.CONFIGURED_FEATURE.get(id);
+
+		if (builtInEntry == null) {
+			throw new IllegalStateException("Expected built-in entry to exist for: " + id);
+		}
+
+		ConfiguredFeature<?, ?> entry = registry.get(id);
+
+		if (entry == null) {
+			throw new IllegalStateException("Expected dynamic registry to contain entry " + id);
+		}
+
+		if (builtInEntry == entry) {
+			throw new IllegalStateException("Expected that the built-in entry and dynamic entry don't have object identity because the dynamic entry is created by serializing the built-in entry to JSON and back.");
+		}
+
+		if (builtInEntry.feature != entry.feature) {
+			throw new IllegalStateException("Expected both entries to reference the same feature since it's only in Registry and is never copied");
+		}
+	}
 }