Improve missing local registry entries error message. (#3004)

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',
 ])

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(getText(e)));
-					client.execute(() -> handler.getConnection().disconnect(Text.literal("Registry remapping failed: " + e.getMessage())));
 				}));
 	}
+
+	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());
+	}
 }

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<Identifier, Object2IntMap<Identifier>> map, RemappableRegistry.RemapMode mode) throws RemapException {
+		if (mode == RemappableRegistry.RemapMode.REMOTE) {
+			checkRemoteRemap(map);
+		}
+
 		Set<Identifier> containedRegistries = Sets.newHashSet(map.keySet());
 
 		for (Identifier registryId : Registries.REGISTRIES.getIds()) {
@@ -296,7 +308,7 @@ public final class RegistrySyncManager {
 			}
 
 			Object2IntMap<Identifier> 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<Identifier, Object2IntMap<Identifier>> map) throws RemapException {
+		Map<Identifier, List<Identifier>> missingEntries = new HashMap<>();
+
+		for (Map.Entry<? extends RegistryKey<? extends Registry<?>>, ? extends Registry<?>> entry : Registries.REGISTRIES.getEntrySet()) {
+			final Registry<?> registry = entry.getValue();
+			final Identifier registryId = entry.getKey().getValue();
+			final Object2IntMap<Identifier> 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<Identifier, List<Identifier>> 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<String> 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);

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) {
+	public RemapException(Text text) {
-		super(message, t);
+		super(text.getString());
+		this.text = text;
+	}
+
+	@Nullable
+	public Text getText() {
+		return text;
 	}
 }

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..."
+}

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<Identifier, Object2IntMap<Identifier>> 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<Identifier> createFakeRegistryEntries() {
+		Object2IntMap<Identifier> map = new Object2IntOpenHashMap<>();
+
+		for (int i = 0; i < 12; i++) {
+			map.put(new Identifier("mod_" + i, "entry"), 0);
+		}
+
+		return map;
+	}
 }