diff --git a/paper-server/src/main/java/org/bukkit/craftbukkit/CraftRegistry.java b/paper-server/src/main/java/org/bukkit/craftbukkit/CraftRegistry.java index b15f26bff2..98067400f0 100644 --- a/paper-server/src/main/java/org/bukkit/craftbukkit/CraftRegistry.java +++ b/paper-server/src/main/java/org/bukkit/craftbukkit/CraftRegistry.java @@ -46,32 +46,35 @@ public class CraftRegistry implements Registry { public static Registry createRegistry(Class bukkitClass, IRegistryCustom registryHolder) { if (bukkitClass == GameEvent.class) { - return new CraftRegistry<>(registryHolder.registryOrThrow(Registries.GAME_EVENT), CraftGameEvent::new); + return new CraftRegistry<>(GameEvent.class, registryHolder.registryOrThrow(Registries.GAME_EVENT), CraftGameEvent::new); } if (bukkitClass == MusicInstrument.class) { - return new CraftRegistry<>(registryHolder.registryOrThrow(Registries.INSTRUMENT), CraftMusicInstrument::new); + return new CraftRegistry<>(MusicInstrument.class, registryHolder.registryOrThrow(Registries.INSTRUMENT), CraftMusicInstrument::new); } if (bukkitClass == Structure.class) { - return new CraftRegistry<>(registryHolder.registryOrThrow(Registries.STRUCTURE), CraftStructure::new); + return new CraftRegistry<>(Structure.class, registryHolder.registryOrThrow(Registries.STRUCTURE), CraftStructure::new); } if (bukkitClass == StructureType.class) { - return new CraftRegistry<>(BuiltInRegistries.STRUCTURE_TYPE, CraftStructureType::new); + return new CraftRegistry<>(StructureType.class, BuiltInRegistries.STRUCTURE_TYPE, CraftStructureType::new); } if (bukkitClass == TrimMaterial.class) { - return new CraftRegistry<>(registryHolder.registryOrThrow(Registries.TRIM_MATERIAL), CraftTrimMaterial::new); + return new CraftRegistry<>(TrimMaterial.class, registryHolder.registryOrThrow(Registries.TRIM_MATERIAL), CraftTrimMaterial::new); } if (bukkitClass == TrimPattern.class) { - return new CraftRegistry<>(registryHolder.registryOrThrow(Registries.TRIM_PATTERN), CraftTrimPattern::new); + return new CraftRegistry<>(TrimPattern.class, registryHolder.registryOrThrow(Registries.TRIM_PATTERN), CraftTrimPattern::new); } return null; } + private final Class bukkitClass; private final Map cache = new HashMap<>(); private final IRegistry minecraftRegistry; private final BiFunction minecraftToBukkit; + private boolean init; - public CraftRegistry(IRegistry minecraftRegistry, BiFunction minecraftToBukkit) { + public CraftRegistry(Class bukkitClass, IRegistry minecraftRegistry, BiFunction minecraftToBukkit) { + this.bukkitClass = bukkitClass; this.minecraftRegistry = minecraftRegistry; this.minecraftToBukkit = minecraftToBukkit; } @@ -83,6 +86,27 @@ public class CraftRegistry implements Registry { return cached; } + // Make sure that the bukkit class is loaded before creating an instance. + // This ensures that only one instance with a given key is created. + // + // Without this code (when bukkit class is not loaded): + // Registry#get -> #createBukkit -> (load class -> create default) -> put in cache + // Result: Registry#get != . for possible one registry item + // + // With this code (when bukkit class is not loaded): + // Registry#get -> (load class -> create default) -> Registry#get -> get from cache + // Result: Registry#get == . + if (!init) { + init = true; + try { + Class.forName(bukkitClass.getName()); + } catch (ClassNotFoundException e) { + throw new RuntimeException("Could not load registry class " + bukkitClass, e); + } + + return get(namespacedKey); + } + B bukkit = createBukkit(namespacedKey, minecraftRegistry.getOptional(CraftNamespacedKey.toMinecraft(namespacedKey)).orElse(null)); if (bukkit == null) { return null; diff --git a/paper-server/src/test/java/org/bukkit/RegistryLoadOrderTest.java b/paper-server/src/test/java/org/bukkit/RegistryLoadOrderTest.java new file mode 100644 index 0000000000..5ae8ad5ce3 --- /dev/null +++ b/paper-server/src/test/java/org/bukkit/RegistryLoadOrderTest.java @@ -0,0 +1,145 @@ +package org.bukkit; + +import static org.junit.jupiter.api.Assertions.*; +import com.mojang.serialization.Lifecycle; +import java.util.function.BiFunction; +import java.util.function.Supplier; +import java.util.stream.Stream; +import net.minecraft.core.IRegistry; +import net.minecraft.core.RegistryMaterials; +import net.minecraft.resources.MinecraftKey; +import net.minecraft.resources.ResourceKey; +import org.bukkit.craftbukkit.CraftRegistry; +import org.bukkit.support.AbstractTestingBase; +import org.jetbrains.annotations.NotNull; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +public class RegistryLoadOrderTest extends AbstractTestingBase { + + private static boolean initInterface = false; + private static boolean initAbstract = false; + private static Registry registry; + + public static Stream data() { + return Stream.of( + Arguments.of( + (Supplier) () -> initInterface, + BukkitInterfaceTestType.class, + (BiFunction) CraftBukkitInterfaceTestType::new, + (Supplier) () -> BukkitInterfaceTestType.TEST_ONE, + (Supplier) () -> BukkitInterfaceTestType.TEST_TWO + ), + Arguments.of( + (Supplier) () -> initAbstract, + BukkitAbstractTestType.class, + (BiFunction) CraftBukkitAbstractTestType::new, + (Supplier) () -> BukkitAbstractTestType.TEST_ONE, + (Supplier) () -> BukkitAbstractTestType.TEST_TWO + ) + ); + } + + @ParameterizedTest + @MethodSource("data") + public void testRegistryLoadOrder(Supplier init, Class keyedClass, BiFunction minecraftToBukkit, Supplier first, Supplier second) { + testClassNotLoaded(init.get()); + + ResourceKey> resourceKey = ResourceKey.createRegistryKey(MinecraftKey.tryBuild("bukkit", "test-registry")); + RegistryMaterials minecraftRegistry = new RegistryMaterials<>(resourceKey, Lifecycle.experimental()); + + minecraftRegistry.register(ResourceKey.create(resourceKey, MinecraftKey.tryBuild("bukkit", "test-one")), new MinecraftTestType(), Lifecycle.experimental()); + minecraftRegistry.register(ResourceKey.create(resourceKey, MinecraftKey.tryBuild("bukkit", "test-two")), new MinecraftTestType(), Lifecycle.experimental()); + minecraftRegistry.freeze(); + + registry = new CraftRegistry<>(keyedClass, minecraftRegistry, minecraftToBukkit); + testClassNotLoaded(init.get()); + + Object testOne = registry.get(new NamespacedKey("bukkit", "test-one")); + Object otherTestOne = registry.get(new NamespacedKey("bukkit", "test-one")); + Object testTwo = registry.get(new NamespacedKey("bukkit", "test-two")); + + assertNotNull(testOne); + assertNotNull(otherTestOne); + assertNotNull(testTwo); + assertNotNull(first.get()); + assertNotNull(second.get()); + + assertSame(testOne, otherTestOne); + assertSame(testOne, first.get()); + assertSame(otherTestOne, first.get()); + assertSame(testTwo, second.get()); + + assertTrue(init.get()); + } + + private void testClassNotLoaded(boolean init) { + assertFalse(init, """ + TestType class is already loaded, this test however tests the behavior when the class is not loaded. + This should normally not happen with how classes should be loaded. + Something has changed how classes are loaded and a more manual deeper look is required. + """); + } + + public interface BukkitInterfaceTestType extends Keyed { + BukkitInterfaceTestType TEST_ONE = get("test-one"); + BukkitInterfaceTestType TEST_TWO = get("test-two"); + + private static BukkitInterfaceTestType get(String key) { + initInterface = true; + if (registry == null) { + return null; + } + + return (BukkitInterfaceTestType) registry.get(new NamespacedKey("bukkit", key)); + } + } + + public static class CraftBukkitInterfaceTestType implements BukkitInterfaceTestType { + + private final NamespacedKey key; + + public CraftBukkitInterfaceTestType(NamespacedKey key, MinecraftTestType minecraftTestType) { + this.key = key; + } + + @NotNull + @Override + public NamespacedKey getKey() { + return key; + } + } + + public abstract static class BukkitAbstractTestType implements Keyed { + public static final BukkitAbstractTestType TEST_ONE = get("test-one"); + public static final BukkitAbstractTestType TEST_TWO = get("test-two"); + + private static BukkitAbstractTestType get(String key) { + initAbstract = true; + if (registry == null) { + return null; + } + + return (BukkitAbstractTestType) registry.get(new NamespacedKey("bukkit", key)); + } + } + + public static class CraftBukkitAbstractTestType extends BukkitAbstractTestType { + + private final NamespacedKey key; + + public CraftBukkitAbstractTestType(NamespacedKey key, MinecraftTestType minecraftTestType) { + this.key = key; + } + + @NotNull + @Override + public NamespacedKey getKey() { + return key; + } + } + + public static class MinecraftTestType { + } +}