From 4995902bc016d06f2b91e58cc99a50c6131d6ea6 Mon Sep 17 00:00:00 2001 From: LexManos Date: Mon, 5 Aug 2024 13:30:14 -0700 Subject: [PATCH] Fix network sync of custom datapack registries. Add gametest to verify that it functions. --- .../core/RegistrySynchronization.java.patch | 16 ++++ .../resources/RegistryDataLoader.java.patch | 9 ++ .../registries/DataPackRegistriesHooks.java | 69 ++++++++------- .../registry/test_entry.json | 3 + .../NetworkDatapackRegistryTest.java | 87 +++++++++++++++++++ .../net/minecraftforge/test/BaseTestMod.java | 5 ++ 6 files changed, 159 insertions(+), 30 deletions(-) create mode 100644 patches/minecraft/net/minecraft/core/RegistrySynchronization.java.patch create mode 100644 src/test/generated/network_data_registry/data/network_data_registry/network_data_registry/registry/test_entry.json create mode 100644 src/test/java/net/minecraftforge/debug/registries/NetworkDatapackRegistryTest.java diff --git a/patches/minecraft/net/minecraft/core/RegistrySynchronization.java.patch b/patches/minecraft/net/minecraft/core/RegistrySynchronization.java.patch new file mode 100644 index 0000000000..e8c7cf1bfd --- /dev/null +++ b/patches/minecraft/net/minecraft/core/RegistrySynchronization.java.patch @@ -0,0 +1,16 @@ +--- a/net/minecraft/core/RegistrySynchronization.java ++++ b/net/minecraft/core/RegistrySynchronization.java +@@ -19,10 +_,11 @@ + import net.minecraft.server.packs.repository.KnownPack; + + public class RegistrySynchronization { +- public static final Set>> NETWORKABLE_REGISTRIES = RegistryDataLoader.SYNCHRONIZED_REGISTRIES ++ public static final Set>> NETWORKABLE_REGISTRIES = net.minecraftforge.registries.DataPackRegistriesHooks.grabNetworkableRegistries(() -> RegistryDataLoader.SYNCHRONIZED_REGISTRIES + .stream() + .map(RegistryDataLoader.RegistryData::key) +- .collect(Collectors.toUnmodifiableSet()); ++ .collect(Collectors.toUnmodifiableSet()) ++ ); + + public static void packRegistries( + DynamicOps p_330752_, diff --git a/patches/minecraft/net/minecraft/resources/RegistryDataLoader.java.patch b/patches/minecraft/net/minecraft/resources/RegistryDataLoader.java.patch index 04aa60c2cf..36419345c4 100644 --- a/patches/minecraft/net/minecraft/resources/RegistryDataLoader.java.patch +++ b/patches/minecraft/net/minecraft/resources/RegistryDataLoader.java.patch @@ -9,6 +9,15 @@ new RegistryDataLoader.RegistryData<>(Registries.DIMENSION_TYPE, DimensionType.DIRECT_CODEC), new RegistryDataLoader.RegistryData<>(Registries.BIOME, Biome.DIRECT_CODEC), new RegistryDataLoader.RegistryData<>(Registries.CHAT_TYPE, ChatType.DIRECT_CODEC), +@@ -100,7 +_,7 @@ + public static final List> DIMENSION_REGISTRIES = List.of( + new RegistryDataLoader.RegistryData<>(Registries.LEVEL_STEM, LevelStem.CODEC) + ); +- public static final List> SYNCHRONIZED_REGISTRIES = List.of( ++ public static final List> SYNCHRONIZED_REGISTRIES = net.minecraftforge.registries.DataPackRegistriesHooks.grabSynchronizedRegistries( + new RegistryDataLoader.RegistryData<>(Registries.BIOME, Biome.NETWORK_CODEC), + new RegistryDataLoader.RegistryData<>(Registries.CHAT_TYPE, ChatType.DIRECT_CODEC), + new RegistryDataLoader.RegistryData<>(Registries.TRIM_PATTERN, TrimPattern.DIRECT_CODEC), @@ -114,6 +_,10 @@ new RegistryDataLoader.RegistryData<>(Registries.JUKEBOX_SONG, JukeboxSong.DIRECT_CODEC) ); diff --git a/src/main/java/net/minecraftforge/registries/DataPackRegistriesHooks.java b/src/main/java/net/minecraftforge/registries/DataPackRegistriesHooks.java index c87f28427a..e2355d8f79 100644 --- a/src/main/java/net/minecraftforge/registries/DataPackRegistriesHooks.java +++ b/src/main/java/net/minecraftforge/registries/DataPackRegistriesHooks.java @@ -5,33 +5,40 @@ package net.minecraftforge.registries; -import com.google.common.collect.ImmutableMap; import net.minecraft.core.Registry; import net.minecraft.core.RegistrySynchronization; import net.minecraft.resources.RegistryDataLoader; import net.minecraft.resources.ResourceKey; import org.jetbrains.annotations.ApiStatus; -import java.util.*; -import java.util.stream.Stream; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.Set; +import java.util.function.Supplier; @ApiStatus.Internal public final class DataPackRegistriesHooks { private DataPackRegistriesHooks() {} // utility class - private static final Map>, RegistryDataLoader.RegistryData> NETWORKABLE_REGISTRIES = new LinkedHashMap<>(); + // Mutable views of vanilla registries + private static final List> SYNCHRONIZED_REGISTRIES = new ArrayList<>(); private static final List> WORLDGEN_REGISTRIES = new ArrayList<>(); - private static final Set>> SYNCED_CUSTOM_REGISTRIES = new HashSet<>(); - private static final Set>> SYNCED_CUSTOM_REGISTRIES_VIEW = Collections.unmodifiableSet(SYNCED_CUSTOM_REGISTRIES); + private static final Set>> NETWORKABLE_REGISTRIES = new HashSet<>(); + + // Out custom registries, things we need to make sure the client has + private static final Set>> SYNCHRONIZED_REGISTRIES_CUSTOM = new HashSet<>(); + private static final Set>> SYNCHRONIZED_REGISTRIESD_CUSTOM_VIEW = Collections.unmodifiableSet(SYNCHRONIZED_REGISTRIES_CUSTOM); /* Internal forge hook for retaining mutable access to RegistryAccess's codec registry when it bootstraps. */ - public static Map>, RegistryDataLoader.RegistryData> grabNetworkableRegistries(ImmutableMap.Builder>, RegistryDataLoader.RegistryData> builder) { - if (!StackWalker.getInstance(StackWalker.Option.RETAIN_CLASS_REFERENCE).getCallerClass().equals(RegistrySynchronization.class)) - throw new IllegalCallerException("Attempted to call DataPackRegistriesHooks#grabNetworkableRegistries!"); - NETWORKABLE_REGISTRIES.forEach(builder::put); - NETWORKABLE_REGISTRIES.clear(); - NETWORKABLE_REGISTRIES.putAll(builder.build()); - return Collections.unmodifiableMap(NETWORKABLE_REGISTRIES); + public static List> grabSynchronizedRegistries(RegistryDataLoader.RegistryData... vanilla) { + if (!StackWalker.getInstance(StackWalker.Option.RETAIN_CLASS_REFERENCE).getCallerClass().equals(RegistryDataLoader.class)) + throw new IllegalCallerException("Attempted to call DataPackRegistriesHooks#grabSynchronizedRegistries!"); + for (var reg : vanilla) + SYNCHRONIZED_REGISTRIES.add(reg); + return Collections.unmodifiableList(SYNCHRONIZED_REGISTRIES); } /* Internal forge method, registers a datapack registry codec and folder. */ @@ -39,25 +46,12 @@ public final class DataPackRegistriesHooks { RegistryDataLoader.RegistryData loaderData = data.loaderData(); WORLDGEN_REGISTRIES.add(loaderData); if (data.networkCodec() != null) { - SYNCED_CUSTOM_REGISTRIES.add(loaderData.key()); - NETWORKABLE_REGISTRIES.put(loaderData.key(), new RegistryDataLoader.RegistryData<>(loaderData.key(), data.networkCodec(), false)); + NETWORKABLE_REGISTRIES.add(loaderData.key()); + SYNCHRONIZED_REGISTRIES_CUSTOM.add(loaderData.key()); + SYNCHRONIZED_REGISTRIES.add(new RegistryDataLoader.RegistryData<>(loaderData.key(), data.networkCodec(), false)); } } - /** - * {@return An unmodifiable view of the list of datapack registries}. - * These registries are loaded from per-world datapacks on server startup. - */ - @Deprecated // Use RegistryDataLoader#WORLDGEN_REGISTRIES instead - public static List> getDataPackRegistries() { - return Collections.unmodifiableList(WORLDGEN_REGISTRIES); - } - - @Deprecated // Use RegistryDataLoader#WORLDGEN_REGISTRIES instead - public static Stream> getDataPackRegistriesWithDimensions() { - return WORLDGEN_REGISTRIES.stream(); - } - /** * Captures a mutable view of {@link RegistryDataLoader#WORLDGEN_REGISTRIES} so that we can register modded entries during the registry event. * All uses should use that field instead of this class. @@ -71,12 +65,27 @@ public final class DataPackRegistriesHooks { return Collections.unmodifiableList(WORLDGEN_REGISTRIES); } + /** + * Captures a mutable view of {@link RegistrySynchronization#NETWORKABLE_REGISTRIES} so that we can register modded entries during the registry event. + * All users should use that field instead of this class. + * + * @param vanilla Supplier for the default vanilla values + * @return An unmodifiable set of networksable registry names + */ + public static Set>> grabNetworkableRegistries(Supplier>>> vanilla) { + if (!StackWalker.getInstance(StackWalker.Option.RETAIN_CLASS_REFERENCE).getCallerClass().equals(RegistrySynchronization.class)) + throw new IllegalCallerException("Attempted to call DataPackRegistriesHooks#grabNetworkableRegistries!"); + + NETWORKABLE_REGISTRIES.addAll(vanilla.get()); + return Collections.unmodifiableSet(NETWORKABLE_REGISTRIES); + } + /** * {@return An unmodifiable view of the set of synced non-vanilla datapack registry IDs} * Clients must have each of a server's synced datapack registries to be able to connect to that server; * vanilla clients therefore cannot connect if this list is non-empty on the server. */ public static Set>> getSyncedCustomRegistries() { - return SYNCED_CUSTOM_REGISTRIES_VIEW; + return SYNCHRONIZED_REGISTRIESD_CUSTOM_VIEW; } } diff --git a/src/test/generated/network_data_registry/data/network_data_registry/network_data_registry/registry/test_entry.json b/src/test/generated/network_data_registry/data/network_data_registry/network_data_registry/registry/test_entry.json new file mode 100644 index 0000000000..006e7e0c19 --- /dev/null +++ b/src/test/generated/network_data_registry/data/network_data_registry/network_data_registry/registry/test_entry.json @@ -0,0 +1,3 @@ +{ + "value": "network_data_registry:test_value" +} \ No newline at end of file diff --git a/src/test/java/net/minecraftforge/debug/registries/NetworkDatapackRegistryTest.java b/src/test/java/net/minecraftforge/debug/registries/NetworkDatapackRegistryTest.java new file mode 100644 index 0000000000..930d6137a9 --- /dev/null +++ b/src/test/java/net/minecraftforge/debug/registries/NetworkDatapackRegistryTest.java @@ -0,0 +1,87 @@ +/* + * Copyright (c) Forge Development LLC and contributors + * SPDX-License-Identifier: LGPL-2.1-only + */ + +package net.minecraftforge.debug.registries; + +import java.util.Set; + +import org.slf4j.Logger; + +import com.mojang.logging.LogUtils; +import com.mojang.serialization.Codec; +import com.mojang.serialization.codecs.RecordCodecBuilder; + +import net.minecraft.client.Minecraft; +import net.minecraft.core.Registry; +import net.minecraft.core.RegistrySetBuilder; +import net.minecraft.gametest.framework.GameTest; +import net.minecraft.gametest.framework.GameTestHelper; +import net.minecraft.resources.ResourceKey; +import net.minecraft.resources.ResourceLocation; +import net.minecraftforge.common.data.DatapackBuiltinEntriesProvider; +import net.minecraftforge.data.event.GatherDataEvent; +import net.minecraftforge.eventbus.api.SubscribeEvent; +import net.minecraftforge.fml.common.Mod; +import net.minecraftforge.fml.loading.FMLLoader; +import net.minecraftforge.gametest.GameTestHolder; +import net.minecraftforge.registries.DataPackRegistryEvent; +import net.minecraftforge.test.BaseTestMod; + +@GameTestHolder("forge.network_data_registry") +@Mod(NetworkDatapackRegistryTest.MODID) +public class NetworkDatapackRegistryTest extends BaseTestMod { + private static final Logger LOGGER = LogUtils.getLogger(); + public static final String MODID = "network_data_registry"; + private static final ResourceKey> REGISTRY_KEY = ResourceKey.createRegistryKey(rl(MODID, "registry")); + private static final ResourceKey TEST_ENTRY = ResourceKey.create(REGISTRY_KEY, rl(MODID, "test_entry")); + private static final ResourceLocation TEST_VALUE = rl(MODID, "test_value"); + + @SubscribeEvent + public void onNewDatapackRegistry(DataPackRegistryEvent.NewRegistry event) { + event.dataPackRegistry(REGISTRY_KEY, DataObject.DIRECT_CODEC, DataObject.DIRECT_CODEC); + } + + @SubscribeEvent + public void onDataGen(GatherDataEvent event) { + var gen = event.getGenerator(); + var packOutput = gen.getPackOutput(); + gen.addProvider(event.includeServer(), new DatapackBuiltinEntriesProvider(packOutput, event.getLookupProvider(), + new RegistrySetBuilder() + .add(REGISTRY_KEY, + ctx -> { + ctx.register(TEST_ENTRY, new DataObject(TEST_VALUE)); + } + ), + Set.of(MODID)) + ); + } + + @GameTest(template = "forge:empty3x3x3") + public static void client_has_registry(GameTestHelper helper) { + if (FMLLoader.getDist().isDedicatedServer()) + LOGGER.info("client_has_registry test skipped as we're on dedicated server"); + else + client_has_registry_client_code(helper); + helper.succeed(); + } + + // Separate function to not resolve classes until it's called, as it references client only classes. + private static void client_has_registry_client_code(GameTestHelper helper) { + var mc = Minecraft.getInstance(); + var level = mc.level; + var reg = level.registryAccess().registry(REGISTRY_KEY); + helper.assertTrue(reg.isPresent(), "Failed to find " + REGISTRY_KEY.location()); + var entry = reg.get().get(TEST_ENTRY); + if (entry == null) + helper.fail("Failed to find " + TEST_ENTRY); + helper.assertValueEqual(entry.value, TEST_VALUE, "Loaded entry does not contain expected value"); + } + + public static record DataObject(ResourceLocation value) { + public static final Codec DIRECT_CODEC = RecordCodecBuilder.create(instance -> instance.group( + ResourceLocation.CODEC.fieldOf("value").forGetter(DataObject::value) + ).apply(instance, DataObject::new)); + } +} diff --git a/src/test/java/net/minecraftforge/test/BaseTestMod.java b/src/test/java/net/minecraftforge/test/BaseTestMod.java index 7d97d6b7be..e25b02c440 100644 --- a/src/test/java/net/minecraftforge/test/BaseTestMod.java +++ b/src/test/java/net/minecraftforge/test/BaseTestMod.java @@ -12,6 +12,7 @@ import java.util.List; import java.util.function.Function; import net.minecraft.world.item.ItemStack; import net.minecraft.core.HolderLookup; +import net.minecraft.resources.ResourceLocation; import net.minecraft.world.item.CreativeModeTab.TabVisibility; import net.minecraftforge.event.BuildCreativeModeTabContentsEvent; import net.minecraftforge.eventbus.api.SubscribeEvent; @@ -41,6 +42,10 @@ public abstract class BaseTestMod { } } + protected static ResourceLocation rl(String namespace, String path) { + return ResourceLocation.fromNamespaceAndPath(namespace, path); + } + protected void testItem(Function supplier) { this.testItems.add(supplier); }