From 670e8ac6c505e824855cb44af4f37df718b60b74 Mon Sep 17 00:00:00 2001 From: modmuss50 Date: Sun, 23 Apr 2023 13:21:39 +0100 Subject: [PATCH] Improve missing local registry entries error message. (#3004) --- fabric-registry-sync-v0/build.gradle | 3 +- .../sync/FabricRegistryClientInit.java | 16 +++- .../registry/sync/RegistrySyncManager.java | 91 ++++++++++++++++++- .../impl/registry/sync/RemapException.java | 18 +++- .../fabric-registry-sync-v0/lang/en_us.json | 7 ++ .../test/registry/sync/RegistrySyncTest.java | 38 ++++++++ 6 files changed, 164 insertions(+), 9 deletions(-) create mode 100644 fabric-registry-sync-v0/src/main/resources/assets/fabric-registry-sync-v0/lang/en_us.json diff --git a/fabric-registry-sync-v0/build.gradle b/fabric-registry-sync-v0/build.gradle index 91d4af092..e653c9a3d 100644 --- a/fabric-registry-sync-v0/build.gradle +++ b/fabric-registry-sync-v0/build.gradle @@ -11,5 +11,6 @@ moduleDependencies(project, [ ]) testDependencies(project, [ - ':fabric-lifecycle-events-v1' + ':fabric-lifecycle-events-v1', + ':fabric-command-api-v2', ]) diff --git a/fabric-registry-sync-v0/src/client/java/net/fabricmc/fabric/impl/client/registry/sync/FabricRegistryClientInit.java b/fabric-registry-sync-v0/src/client/java/net/fabricmc/fabric/impl/client/registry/sync/FabricRegistryClientInit.java index b26b35ef6..1301618a6 100644 --- a/fabric-registry-sync-v0/src/client/java/net/fabricmc/fabric/impl/client/registry/sync/FabricRegistryClientInit.java +++ b/fabric-registry-sync-v0/src/client/java/net/fabricmc/fabric/impl/client/registry/sync/FabricRegistryClientInit.java @@ -24,6 +24,7 @@ import net.minecraft.text.Text; import net.fabricmc.api.ClientModInitializer; import net.fabricmc.fabric.api.client.networking.v1.ClientPlayNetworking; import net.fabricmc.fabric.impl.registry.sync.RegistrySyncManager; +import net.fabricmc.fabric.impl.registry.sync.RemapException; import net.fabricmc.fabric.impl.registry.sync.packet.RegistryPacketHandler; public class FabricRegistryClientInit implements ClientModInitializer { @@ -39,8 +40,19 @@ public class FabricRegistryClientInit implements ClientModInitializer { ClientPlayNetworking.registerGlobalReceiver(packetHandler.getPacketId(), (client, handler, buf, responseSender) -> RegistrySyncManager.receivePacket(client, packetHandler, buf, RegistrySyncManager.DEBUG || !client.isInSingleplayer(), (e) -> { LOGGER.error("Registry remapping failed!", e); - - client.execute(() -> handler.getConnection().disconnect(Text.literal("Registry remapping failed: " + e.getMessage()))); + client.execute(() -> handler.getConnection().disconnect(getText(e))); })); } + + private Text getText(Exception e) { + if (e instanceof RemapException remapException) { + final Text text = remapException.getText(); + + if (text != null) { + return text; + } + } + + return Text.literal("Registry remapping failed: " + e.getMessage()); + } } diff --git a/fabric-registry-sync-v0/src/main/java/net/fabricmc/fabric/impl/registry/sync/RegistrySyncManager.java b/fabric-registry-sync-v0/src/main/java/net/fabricmc/fabric/impl/registry/sync/RegistrySyncManager.java index ccfe9197c..b86e5783f 100644 --- a/fabric-registry-sync-v0/src/main/java/net/fabricmc/fabric/impl/registry/sync/RegistrySyncManager.java +++ b/fabric-registry-sync-v0/src/main/java/net/fabricmc/fabric/impl/registry/sync/RegistrySyncManager.java @@ -20,7 +20,10 @@ import java.io.File; import java.io.FileOutputStream; import java.io.IOException; import java.nio.charset.StandardCharsets; +import java.util.ArrayList; +import java.util.HashMap; import java.util.LinkedHashMap; +import java.util.List; import java.util.Map; import java.util.Set; import java.util.concurrent.ExecutionException; @@ -36,16 +39,21 @@ import it.unimi.dsi.fastutil.objects.Object2IntLinkedOpenHashMap; import it.unimi.dsi.fastutil.objects.Object2IntMap; import it.unimi.dsi.fastutil.objects.Object2IntOpenHashMap; import org.jetbrains.annotations.Nullable; +import org.jetbrains.annotations.VisibleForTesting; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import net.minecraft.nbt.NbtCompound; import net.minecraft.network.PacketByteBuf; -import net.minecraft.server.MinecraftServer; -import net.minecraft.server.network.ServerPlayerEntity; -import net.minecraft.util.Identifier; import net.minecraft.registry.Registries; import net.minecraft.registry.Registry; +import net.minecraft.registry.RegistryKey; +import net.minecraft.server.MinecraftServer; +import net.minecraft.server.network.ServerPlayerEntity; +import net.minecraft.text.MutableText; +import net.minecraft.text.Text; +import net.minecraft.util.Formatting; +import net.minecraft.util.Identifier; import net.minecraft.util.thread.ThreadExecutor; import net.fabricmc.fabric.api.event.registry.RegistryAttribute; @@ -288,6 +296,10 @@ public final class RegistrySyncManager { } public static void apply(Map> map, RemappableRegistry.RemapMode mode) throws RemapException { + if (mode == RemappableRegistry.RemapMode.REMOTE) { + checkRemoteRemap(map); + } + Set containedRegistries = Sets.newHashSet(map.keySet()); for (Identifier registryId : Registries.REGISTRIES.getIds()) { @@ -296,7 +308,7 @@ public final class RegistrySyncManager { } Object2IntMap registryMap = map.get(registryId); - Registry registry = Registries.REGISTRIES.get(registryId); + Registry registry = Registries.REGISTRIES.get(registryId); RegistryAttributeHolder attributeHolder = RegistryAttributeHolder.get(registry.getKey()); @@ -321,6 +333,77 @@ public final class RegistrySyncManager { } } + @VisibleForTesting + public static void checkRemoteRemap(Map> map) throws RemapException { + Map> missingEntries = new HashMap<>(); + + for (Map.Entry>, ? extends Registry> entry : Registries.REGISTRIES.getEntrySet()) { + final Registry registry = entry.getValue(); + final Identifier registryId = entry.getKey().getValue(); + final Object2IntMap remoteRegistry = map.get(registryId); + + if (remoteRegistry == null) { + // Registry sync does not contain data for this registry, will print a warning when applying. + continue; + } + + for (Identifier remoteId : remoteRegistry.keySet()) { + if (!registry.containsId(remoteId)) { + // Found a registry entry from the server that is + missingEntries.computeIfAbsent(registryId, i -> new ArrayList<>()).add(remoteId); + } + } + } + + if (missingEntries.isEmpty()) { + // All good :) + return; + } + + // Print out details to the log + LOGGER.error("Received unknown remote registry entries from server"); + + for (Map.Entry> entry : missingEntries.entrySet()) { + for (Identifier identifier : entry.getValue()) { + LOGGER.error("Registry entry ({}) is missing from local registry ({})", identifier, entry.getKey()); + } + } + + // Create a nice user friendly error message. + MutableText text = Text.literal(""); + + final int count = missingEntries.values().stream().mapToInt(List::size).sum(); + + if (count == 1) { + text = text.append(Text.translatable("fabric-registry-sync-v0.unknown-remote.title.singular")); + } else { + text = text.append(Text.translatable("fabric-registry-sync-v0.unknown-remote.title.plural", count)); + } + + text = text.append(Text.translatable("fabric-registry-sync-v0.unknown-remote.subtitle.1").formatted(Formatting.GREEN)); + text = text.append(Text.translatable("fabric-registry-sync-v0.unknown-remote.subtitle.2")); + + final int toDisplay = 4; + // Get the distinct missing namespaces + final List namespaces = missingEntries.values().stream() + .flatMap(List::stream) + .map(Identifier::getNamespace) + .distinct() + .sorted() + .toList(); + + for (int i = 0; i < Math.min(namespaces.size(), toDisplay); i++) { + text = text.append(Text.literal(namespaces.get(i)).formatted(Formatting.YELLOW)); + text = text.append("\n"); + } + + if (namespaces.size() > toDisplay) { + text = text.append(Text.translatable("fabric-registry-sync-v0.unknown-remote.footer", namespaces.size() - toDisplay)); + } + + throw new RemapException(text); + } + public static void unmap() throws RemapException { for (Identifier registryId : Registries.REGISTRIES.getIds()) { Registry registry = Registries.REGISTRIES.get(registryId); diff --git a/fabric-registry-sync-v0/src/main/java/net/fabricmc/fabric/impl/registry/sync/RemapException.java b/fabric-registry-sync-v0/src/main/java/net/fabricmc/fabric/impl/registry/sync/RemapException.java index 528d2964f..6eb9cee94 100644 --- a/fabric-registry-sync-v0/src/main/java/net/fabricmc/fabric/impl/registry/sync/RemapException.java +++ b/fabric-registry-sync-v0/src/main/java/net/fabricmc/fabric/impl/registry/sync/RemapException.java @@ -16,12 +16,26 @@ package net.fabricmc.fabric.impl.registry.sync; +import org.jetbrains.annotations.Nullable; + +import net.minecraft.text.Text; + public class RemapException extends Exception { + @Nullable + private final Text text; + public RemapException(String message) { super(message); + this.text = null; } - public RemapException(String message, Throwable t) { - super(message, t); + public RemapException(Text text) { + super(text.getString()); + this.text = text; + } + + @Nullable + public Text getText() { + return text; } } diff --git a/fabric-registry-sync-v0/src/main/resources/assets/fabric-registry-sync-v0/lang/en_us.json b/fabric-registry-sync-v0/src/main/resources/assets/fabric-registry-sync-v0/lang/en_us.json new file mode 100644 index 000000000..e374bbdb1 --- /dev/null +++ b/fabric-registry-sync-v0/src/main/resources/assets/fabric-registry-sync-v0/lang/en_us.json @@ -0,0 +1,7 @@ +{ + "fabric-registry-sync-v0.unknown-remote.title.singular" : "Received a registry entry that is unknown to this client.\n", + "fabric-registry-sync-v0.unknown-remote.title.plural" : "Received %d registry entries that are unknown to this client.\n", + "fabric-registry-sync-v0.unknown-remote.subtitle.1" : "This is usually caused by a mismatched mod set between the client and server.", + "fabric-registry-sync-v0.unknown-remote.subtitle.2" : " See the client logs for more details.\nThe following registry entry namespaces may be related:\n\n", + "fabric-registry-sync-v0.unknown-remote.footer" : "And %d more..." +} 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 ce61d08da..61aba45d6 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 @@ -17,10 +17,12 @@ package net.fabricmc.fabric.test.registry.sync; import java.util.Map; +import java.util.Objects; import java.util.concurrent.atomic.AtomicBoolean; import com.mojang.logging.LogUtils; import it.unimi.dsi.fastutil.objects.Object2IntMap; +import it.unimi.dsi.fastutil.objects.Object2IntOpenHashMap; import org.apache.commons.lang3.Validate; import org.slf4j.Logger; @@ -34,9 +36,12 @@ import net.minecraft.registry.Registry; import net.minecraft.registry.RegistryKey; import net.minecraft.registry.RegistryKeys; import net.minecraft.registry.SimpleRegistry; +import net.minecraft.server.command.CommandManager; +import net.minecraft.server.network.ServerPlayerEntity; import net.minecraft.util.Identifier; import net.fabricmc.api.ModInitializer; +import net.fabricmc.fabric.api.command.v2.CommandRegistrationCallback; import net.fabricmc.fabric.api.event.lifecycle.v1.ServerLifecycleEvents; import net.fabricmc.fabric.api.event.registry.DynamicRegistrySetupCallback; import net.fabricmc.fabric.api.event.registry.FabricRegistryBuilder; @@ -45,6 +50,7 @@ import net.fabricmc.fabric.api.event.registry.RegistryAttributeHolder; import net.fabricmc.fabric.api.networking.v1.PacketByteBufs; import net.fabricmc.fabric.api.networking.v1.ServerPlayConnectionEvents; import net.fabricmc.fabric.impl.registry.sync.RegistrySyncManager; +import net.fabricmc.fabric.impl.registry.sync.RemapException; import net.fabricmc.fabric.impl.registry.sync.packet.DirectRegistryPacketHandler; import net.fabricmc.fabric.impl.registry.sync.packet.NbtRegistryPacketHandler; import net.fabricmc.fabric.impl.registry.sync.packet.RegistryPacketHandler; @@ -128,6 +134,28 @@ public class RegistrySyncTest implements ModInitializer { // Vanilla status effects don't have an entry for the int id 0, test we can handle this. RegistryAttributeHolder.get(Registries.STATUS_EFFECT).addAttribute(RegistryAttribute.MODDED); + + CommandRegistrationCallback.EVENT.register((dispatcher, registryAccess, environment) -> + dispatcher.register(CommandManager.literal("remote_remap_error_test").executes(context -> { + Map> registryData = Map.of( + RegistryKeys.BLOCK.getValue(), createFakeRegistryEntries(), + RegistryKeys.ITEM.getValue(), createFakeRegistryEntries() + ); + + try { + RegistrySyncManager.checkRemoteRemap(registryData); + } catch (RemapException e) { + final ServerPlayerEntity player = context.getSource().getPlayer(); + + if (player != null) { + player.networkHandler.disconnect(Objects.requireNonNull(e.getText())); + } + + return 1; + } + + throw new IllegalStateException(); + }))); } private static void registerBlocks(String namespace, int amount, int startingId) { @@ -141,4 +169,14 @@ public class RegistrySyncTest implements ModInitializer { } } } + + private static Object2IntMap createFakeRegistryEntries() { + Object2IntMap map = new Object2IntOpenHashMap<>(); + + for (int i = 0; i < 12; i++) { + map.put(new Identifier("mod_" + i, "entry"), 0); + } + + return map; + } }