From 7490af87a10f24b2a5e4b43dbb34b5da5f34c430 Mon Sep 17 00:00:00 2001
From: shartte <shartte@users.noreply.github.com>
Date: Fri, 18 Sep 2020 19:10:47 +0200
Subject: [PATCH] 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.
---
.../registry/sync/DynamicRegistrySync.java | 66 +++++++++++++++++++
.../mixin/registry/sync/AccessorRegistry.java | 5 ++
.../sync/MixinDynamicRegistryManager.java | 45 +++++++++++++
.../fabric-registry-sync-v0.mixins.json | 1 +
.../test/registry/sync/RegistrySyncTest.java | 59 ++++++++++++++++-
5 files changed, 175 insertions(+), 1 deletion(-)
create mode 100644 fabric-registry-sync-v0/src/main/java/net/fabricmc/fabric/impl/registry/sync/DynamicRegistrySync.java
create mode 100644 fabric-registry-sync-v0/src/main/java/net/fabricmc/fabric/mixin/registry/sync/MixinDynamicRegistryManager.java
diff --git a/fabric-registry-sync-v0/src/main/java/net/fabricmc/fabric/impl/registry/sync/DynamicRegistrySync.java b/fabric-registry-sync-v0/src/main/java/net/fabricmc/fabric/impl/registry/sync/DynamicRegistrySync.java
new file mode 100644
index 000000000..b63a5bd12
--- /dev/null
+++ b/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);
+ });
+ }
+}
diff --git a/fabric-registry-sync-v0/src/main/java/net/fabricmc/fabric/mixin/registry/sync/AccessorRegistry.java b/fabric-registry-sync-v0/src/main/java/net/fabricmc/fabric/mixin/registry/sync/AccessorRegistry.java
index 2f4d72649..e5a73c141 100644
--- a/fabric-registry-sync-v0/src/main/java/net/fabricmc/fabric/mixin/registry/sync/AccessorRegistry.java
+++ b/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);
}
diff --git a/fabric-registry-sync-v0/src/main/java/net/fabricmc/fabric/mixin/registry/sync/MixinDynamicRegistryManager.java b/fabric-registry-sync-v0/src/main/java/net/fabricmc/fabric/mixin/registry/sync/MixinDynamicRegistryManager.java
new file mode 100644
index 000000000..0fcdec8dc
--- /dev/null
+++ b/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);
+ }
+}
diff --git a/fabric-registry-sync-v0/src/main/resources/fabric-registry-sync-v0.mixins.json b/fabric-registry-sync-v0/src/main/resources/fabric-registry-sync-v0.mixins.json
index f1955d64f..435ba89ee 100644
--- a/fabric-registry-sync-v0/src/main/resources/fabric-registry-sync-v0.mixins.json
+++ b/fabric-registry-sync-v0/src/main/resources/fabric-registry-sync-v0.mixins.json
@@ -5,6 +5,7 @@
"mixins": [
"AccessorRegistry",
"MixinBootstrap",
+ "MixinDynamicRegistryManager",
"MixinIdList",
"MixinIdRegistry",
"MixinPlayerManager",
diff --git a/fabric-registry-sync-v0/src/testmod/java/net/fabricmc/fabric/test/registry/sync/RegistrySyncTest.java b/fabric-registry-sync-v0/src/testmod/java/net/fabricmc/fabric/test/registry/sync/RegistrySyncTest.java
index 63c266d9e..6a687784c 100644
--- a/fabric-registry-sync-v0/src/testmod/java/net/fabricmc/fabric/test/registry/sync/RegistrySyncTest.java
+++ b/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");
+ }
+ }
}