From cc0fa2fec8d7260befc80a9af1322a7d08f8d3d5 Mon Sep 17 00:00:00 2001 From: modmuss Date: Wed, 27 Nov 2024 18:01:38 +0000 Subject: [PATCH] Add optional registries & refactor registry sync (#4233) * Refactor registry sync * Remove old file * Checkstyle * Fixes * Improve error message * Fix * Fix test --- .../ServerConfigurationNetworkAddon.java | 19 ++ .../sync/ClientRegistrySyncHandler.java | 240 ++++++++++++++ .../sync/FabricRegistryClientInit.java | 2 +- .../sync/client/MinecraftClientMixin.java | 18 +- .../api/event/registry/RegistryAttribute.java | 7 +- .../registry/sync/RegistryAttributeImpl.java | 12 + .../registry/sync/RegistrySyncManager.java | 219 ++++--------- .../registry/sync/RemappableRegistry.java | 8 +- .../packet/DirectRegistryPacketHandler.java | 59 +++- .../sync/packet/RegistryPacketHandler.java | 9 +- .../registry/sync/SimpleRegistryMixin.java | 40 +-- .../fabric-registry-sync-v0/lang/en_us.json | 7 +- .../sync/DirectRegistryPacketHandlerTest.java | 15 +- .../test/registry/sync/RegistryRemapTest.java | 310 ++++++++++++++++++ .../test/registry/sync/RegistrySyncTest.java | 41 --- .../src/testmod/resources/fabric.mod.json | 3 +- .../sync/client/RegistrySyncClientTest.java | 77 +++++ 17 files changed, 826 insertions(+), 260 deletions(-) create mode 100644 fabric-registry-sync-v0/src/client/java/net/fabricmc/fabric/impl/client/registry/sync/ClientRegistrySyncHandler.java create mode 100644 fabric-registry-sync-v0/src/test/java/net/fabricmc/fabric/test/registry/sync/RegistryRemapTest.java create mode 100644 fabric-registry-sync-v0/src/testmodClient/java/net/fabricmc/fabric/test/registry/sync/client/RegistrySyncClientTest.java diff --git a/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/impl/networking/server/ServerConfigurationNetworkAddon.java b/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/impl/networking/server/ServerConfigurationNetworkAddon.java index c70e7dbdc..e3179cba0 100644 --- a/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/impl/networking/server/ServerConfigurationNetworkAddon.java +++ b/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/impl/networking/server/ServerConfigurationNetworkAddon.java @@ -20,8 +20,11 @@ import java.util.Collections; import java.util.List; import java.util.Objects; +import org.jetbrains.annotations.Nullable; + import net.minecraft.network.NetworkPhase; import net.minecraft.network.PacketCallbacks; +import net.minecraft.network.packet.BrandCustomPayload; import net.minecraft.network.packet.CustomPayload; import net.minecraft.network.packet.Packet; import net.minecraft.network.packet.s2c.common.CommonPingS2CPacket; @@ -44,6 +47,8 @@ public final class ServerConfigurationNetworkAddon extends AbstractChanneledNetw private final MinecraftServer server; private final ServerConfigurationNetworking.Context context; private RegisterState registerState = RegisterState.NOT_SENT; + @Nullable + private String clientBrand = null; public ServerConfigurationNetworkAddon(ServerConfigurationNetworkHandler handler, MinecraftServer server) { super(ServerNetworkingImpl.CONFIGURATION, ((ServerCommonNetworkHandlerAccessor) handler).getConnection(), "ServerConfigurationNetworkAddon for " + handler.getDebugProfile().getName()); @@ -55,6 +60,16 @@ public final class ServerConfigurationNetworkAddon extends AbstractChanneledNetw this.registerPendingChannels((ChannelInfoHolder) this.connection, NetworkPhase.CONFIGURATION); } + @Override + public boolean handle(CustomPayload payload) { + if (payload instanceof BrandCustomPayload brandCustomPayload) { + clientBrand = brandCustomPayload.brand(); + return false; + } + + return super.handle(payload); + } + @Override protected void invokeInitEvent() { } @@ -169,6 +184,10 @@ public final class ServerConfigurationNetworkAddon extends AbstractChanneledNetw handler.send(packet, callback); } + public @Nullable String getClientBrand() { + return clientBrand; + } + private enum RegisterState { NOT_SENT, SENT, diff --git a/fabric-registry-sync-v0/src/client/java/net/fabricmc/fabric/impl/client/registry/sync/ClientRegistrySyncHandler.java b/fabric-registry-sync-v0/src/client/java/net/fabricmc/fabric/impl/client/registry/sync/ClientRegistrySyncHandler.java new file mode 100644 index 000000000..3a172cd2f --- /dev/null +++ b/fabric-registry-sync-v0/src/client/java/net/fabricmc/fabric/impl/client/registry/sync/ClientRegistrySyncHandler.java @@ -0,0 +1,240 @@ +/* + * 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.client.registry.sync; + +import java.util.ArrayList; +import java.util.EnumSet; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CompletionException; + +import it.unimi.dsi.fastutil.objects.Object2IntMap; +import org.jetbrains.annotations.VisibleForTesting; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import net.minecraft.registry.Registries; +import net.minecraft.registry.Registry; +import net.minecraft.screen.ScreenTexts; +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; +import net.fabricmc.fabric.impl.registry.sync.RegistrySyncManager; +import net.fabricmc.fabric.impl.registry.sync.RemapException; +import net.fabricmc.fabric.impl.registry.sync.RemappableRegistry; +import net.fabricmc.fabric.impl.registry.sync.packet.RegistryPacketHandler; + +public final class ClientRegistrySyncHandler { + private static final Logger LOGGER = LoggerFactory.getLogger(ClientRegistrySyncHandler.class); + + private ClientRegistrySyncHandler() { + } + + public static CompletableFuture receivePacket(ThreadExecutor executor, RegistryPacketHandler handler, T payload, boolean accept) { + handler.receivePayload(payload); + + if (!handler.isPacketFinished()) { + return CompletableFuture.completedFuture(false); + } + + if (RegistrySyncManager.DEBUG) { + String handlerName = handler.getClass().getSimpleName(); + LOGGER.info("{} total packet: {}", handlerName, handler.getTotalPacketReceived()); + LOGGER.info("{} raw size: {}", handlerName, handler.getRawBufSize()); + LOGGER.info("{} deflated size: {}", handlerName, handler.getDeflatedBufSize()); + } + + RegistryPacketHandler.SyncedPacketData data = handler.getSyncedPacketData(); + + if (!accept) { + return CompletableFuture.completedFuture(true); + } + + return executor.submit(() -> { + if (data == null) { + throw new CompletionException(new RemapException("Received null map in sync packet!")); + } + + try { + apply(data); + return true; + } catch (RemapException e) { + throw new CompletionException(e); + } + }); + } + + public static void apply(RegistryPacketHandler.SyncedPacketData data) throws RemapException { + // First check that all of the data provided is valid before making any changes + checkRemoteRemap(data); + + for (Map.Entry> entry : data.idMap().entrySet()) { + final Identifier registryId = entry.getKey(); + + Registry registry = Registries.REGISTRIES.get(registryId); + + // Registry was not found on the client, is it optional? + // If so we can just ignore it. + // Otherwise we throw an exception and disconnect. + if (registry == null) { + if (isRegistryOptional(registryId, data)) { + LOGGER.info("Received registry data for unknown optional registry: {}", registryId); + continue; + } + } + + if (registry instanceof RemappableRegistry remappableRegistry) { + remappableRegistry.remap(entry.getValue(), RemappableRegistry.RemapMode.REMOTE); + return; + } + + throw new RemapException("Registry " + registryId + " is not remappable"); + } + } + + @VisibleForTesting + public static void checkRemoteRemap(RegistryPacketHandler.SyncedPacketData data) throws RemapException { + Map> map = data.idMap(); + ArrayList missingRegistries = new ArrayList<>(); + Map> missingEntries = new HashMap<>(); + + for (Identifier registryId : map.keySet()) { + final Object2IntMap remoteRegistry = map.get(registryId); + Registry registry = Registries.REGISTRIES.get(registryId); + + if (registry == null) { + if (!isRegistryOptional(registryId, data)) { + // Registry was not found on the client, and is not optional. + missingRegistries.add(registryId); + } + + continue; + } + + for (Identifier remoteId : remoteRegistry.keySet()) { + if (!registry.containsId(remoteId)) { + // Found a registry entry from the server that is missing on the client + missingEntries.computeIfAbsent(registryId, i -> new ArrayList<>()).add(remoteId); + } + } + } + + if (missingRegistries.isEmpty() && missingEntries.isEmpty()) { + // All good :) + return; + } + + // Print out details to the log + if (!missingRegistries.isEmpty()) { + LOGGER.error("Received unknown remote registries from server"); + + for (Identifier registryId : missingRegistries) { + LOGGER.error("Received unknown remote registry ({}) from server", registryId); + } + } + + if (!missingEntries.isEmpty()) { + 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()); + } + } + } + + if (!missingRegistries.isEmpty()) { + throw new RemapException(missingRegistriesError(missingRegistries)); + } + + throw new RemapException(missingEntriesError(missingEntries)); + } + + private static Text missingRegistriesError(List missingRegistries) { + MutableText text = Text.empty(); + + final int count = missingRegistries.size(); + + if (count == 1) { + text = text.append(Text.translatable("fabric-registry-sync-v0.unknown-registry.title.singular")); + } else { + text = text.append(Text.translatable("fabric-registry-sync-v0.unknown-registry.title.plural", count)); + } + + text = text.append(Text.translatable("fabric-registry-sync-v0.unknown-registry.subtitle.1").formatted(Formatting.GREEN)); + text = text.append(Text.translatable("fabric-registry-sync-v0.unknown-registry.subtitle.2")); + + final int toDisplay = 4; + + for (int i = 0; i < Math.min(missingRegistries.size(), toDisplay); i++) { + text = text.append(Text.literal(missingRegistries.get(i).toString()).formatted(Formatting.YELLOW)); + text = text.append(ScreenTexts.LINE_BREAK); + } + + if (missingRegistries.size() > toDisplay) { + text = text.append(Text.translatable("fabric-registry-sync-v0.unknown-registry.footer", missingRegistries.size() - toDisplay)); + } + + return text; + } + + private static Text missingEntriesError(Map> missingEntries) { + MutableText text = Text.empty(); + + 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(ScreenTexts.LINE_BREAK); + } + + if (namespaces.size() > toDisplay) { + text = text.append(Text.translatable("fabric-registry-sync-v0.unknown-remote.footer", namespaces.size() - toDisplay)); + } + + return text; + } + + private static boolean isRegistryOptional(Identifier registryId, RegistryPacketHandler.SyncedPacketData data) { + EnumSet registryAttributes = data.attributes().get(registryId); + return registryAttributes.contains(RegistryAttribute.OPTIONAL); + } +} 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 6591b66d9..36b651203 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 @@ -40,7 +40,7 @@ public class FabricRegistryClientInit implements ClientModInitializer { private void registerSyncPacketReceiver(RegistryPacketHandler packetHandler) { ClientConfigurationNetworking.registerGlobalReceiver(packetHandler.getPacketId(), (payload, context) -> { - RegistrySyncManager.receivePacket(context.client(), packetHandler, payload, RegistrySyncManager.DEBUG || !context.client().isInSingleplayer()) + ClientRegistrySyncHandler.receivePacket(context.client(), packetHandler, payload, RegistrySyncManager.DEBUG || !context.client().isInSingleplayer()) .whenComplete((complete, throwable) -> { if (throwable != null) { LOGGER.error("Registry remapping failed!", throwable); diff --git a/fabric-registry-sync-v0/src/client/java/net/fabricmc/fabric/mixin/registry/sync/client/MinecraftClientMixin.java b/fabric-registry-sync-v0/src/client/java/net/fabricmc/fabric/mixin/registry/sync/client/MinecraftClientMixin.java index 582cd629c..2e70cb040 100644 --- a/fabric-registry-sync-v0/src/client/java/net/fabricmc/fabric/mixin/registry/sync/client/MinecraftClientMixin.java +++ b/fabric-registry-sync-v0/src/client/java/net/fabricmc/fabric/mixin/registry/sync/client/MinecraftClientMixin.java @@ -20,6 +20,7 @@ import org.slf4j.Logger; import org.spongepowered.asm.mixin.Final; import org.spongepowered.asm.mixin.Mixin; import org.spongepowered.asm.mixin.Shadow; +import org.spongepowered.asm.mixin.Unique; import org.spongepowered.asm.mixin.injection.At; import org.spongepowered.asm.mixin.injection.Inject; import org.spongepowered.asm.mixin.injection.callback.CallbackInfo; @@ -28,9 +29,11 @@ import net.minecraft.client.MinecraftClient; import net.minecraft.client.gui.screen.Screen; import net.minecraft.item.ItemGroups; import net.minecraft.registry.Registries; +import net.minecraft.registry.Registry; +import net.minecraft.util.Identifier; -import net.fabricmc.fabric.impl.registry.sync.RegistrySyncManager; import net.fabricmc.fabric.impl.registry.sync.RemapException; +import net.fabricmc.fabric.impl.registry.sync.RemappableRegistry; import net.fabricmc.fabric.impl.registry.sync.trackers.vanilla.BlockInitTracker; @Mixin(MinecraftClient.class) @@ -43,7 +46,7 @@ public class MinecraftClientMixin { @Inject(at = @At("RETURN"), method = "disconnect(Lnet/minecraft/client/gui/screen/Screen;Z)V") public void disconnectAfter(Screen disconnectionScreen, boolean bl, CallbackInfo ci) { try { - RegistrySyncManager.unmap(); + unmap(); } catch (RemapException e) { LOGGER.warn("Failed to unmap Fabric registries!", e); } @@ -57,4 +60,15 @@ public class MinecraftClientMixin { BlockInitTracker.postFreeze(); ItemGroups.collect(); } + + @Unique + private static void unmap() throws RemapException { + for (Identifier registryId : Registries.REGISTRIES.getIds()) { + Registry registry = Registries.REGISTRIES.get(registryId); + + if (registry instanceof RemappableRegistry) { + ((RemappableRegistry) registry).unmap(); + } + } + } } diff --git a/fabric-registry-sync-v0/src/main/java/net/fabricmc/fabric/api/event/registry/RegistryAttribute.java b/fabric-registry-sync-v0/src/main/java/net/fabricmc/fabric/api/event/registry/RegistryAttribute.java index 104e687e2..47f664710 100644 --- a/fabric-registry-sync-v0/src/main/java/net/fabricmc/fabric/api/event/registry/RegistryAttribute.java +++ b/fabric-registry-sync-v0/src/main/java/net/fabricmc/fabric/api/event/registry/RegistryAttribute.java @@ -25,5 +25,10 @@ public enum RegistryAttribute { /** * Registry has been modded. */ - MODDED + MODDED, + + /** + * Registry is optional, any connecting client will not be disconnected if the registry is not present. + */ + OPTIONAL } diff --git a/fabric-registry-sync-v0/src/main/java/net/fabricmc/fabric/impl/registry/sync/RegistryAttributeImpl.java b/fabric-registry-sync-v0/src/main/java/net/fabricmc/fabric/impl/registry/sync/RegistryAttributeImpl.java index 6ae1803d7..a419e3004 100644 --- a/fabric-registry-sync-v0/src/main/java/net/fabricmc/fabric/impl/registry/sync/RegistryAttributeImpl.java +++ b/fabric-registry-sync-v0/src/main/java/net/fabricmc/fabric/impl/registry/sync/RegistryAttributeImpl.java @@ -20,10 +20,13 @@ import java.util.EnumSet; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; +import org.jetbrains.annotations.VisibleForTesting; + import net.minecraft.registry.RegistryKey; import net.fabricmc.fabric.api.event.registry.RegistryAttribute; import net.fabricmc.fabric.api.event.registry.RegistryAttributeHolder; +import net.fabricmc.loader.api.FabricLoader; public final class RegistryAttributeImpl implements RegistryAttributeHolder { private static final Map, RegistryAttributeHolder> HOLDER_MAP = new ConcurrentHashMap<>(); @@ -43,6 +46,15 @@ public final class RegistryAttributeImpl implements RegistryAttributeHolder { return this; } + @VisibleForTesting + public void removeAttribute(RegistryAttribute attribute) { + if (!FabricLoader.getInstance().isDevelopmentEnvironment()) { + throw new AssertionError(); + } + + attributes.remove(attribute); + } + @Override public boolean hasAttribute(RegistryAttribute attribute) { return attributes.contains(attribute); 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 345cb71ce..4df4cdb53 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,31 +20,24 @@ 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.Objects; import java.util.Set; -import java.util.concurrent.CompletableFuture; -import java.util.concurrent.CompletionException; import java.util.function.Consumer; -import com.google.common.base.Joiner; -import com.google.common.collect.Sets; import it.unimi.dsi.fastutil.ints.IntOpenHashSet; import it.unimi.dsi.fastutil.ints.IntSet; import it.unimi.dsi.fastutil.objects.Object2IntLinkedOpenHashMap; import it.unimi.dsi.fastutil.objects.Object2IntMap; import org.jetbrains.annotations.Nullable; -import org.jetbrains.annotations.VisibleForTesting; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import net.minecraft.network.packet.Packet; import net.minecraft.registry.Registries; import net.minecraft.registry.Registry; -import net.minecraft.registry.RegistryKey; import net.minecraft.screen.ScreenTexts; import net.minecraft.server.MinecraftServer; import net.minecraft.server.network.ServerConfigurationNetworkHandler; @@ -53,13 +46,12 @@ 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; import net.fabricmc.fabric.api.event.registry.RegistryAttributeHolder; import net.fabricmc.fabric.api.networking.v1.ServerConfigurationNetworking; +import net.fabricmc.fabric.impl.networking.server.ServerNetworkingImpl; import net.fabricmc.fabric.impl.registry.sync.packet.DirectRegistryPacketHandler; -import net.fabricmc.fabric.impl.registry.sync.packet.RegistryPacketHandler; public final class RegistrySyncManager { public static final boolean DEBUG = Boolean.getBoolean("fabric.registry.debug"); @@ -80,11 +72,6 @@ public final class RegistrySyncManager { return; } - if (!ServerConfigurationNetworking.canSend(handler, DIRECT_PACKET_HANDLER.getPacketId())) { - // Don't send if the client cannot receive - return; - } - final Map> map = RegistrySyncManager.createAndPopulateRegistryMap(); if (map == null) { @@ -92,9 +79,62 @@ public final class RegistrySyncManager { return; } + if (!ServerConfigurationNetworking.canSend(handler, DIRECT_PACKET_HANDLER.getPacketId())) { + if (areAllRegistriesOptional(map)) { + // Allow the client to connect if all of the registries we want to sync are optional + return; + } + + // Disconnect incompatible clients + Text message = getIncompatibleClientText(ServerNetworkingImpl.getAddon(handler).getClientBrand(), map); + handler.disconnect(message); + return; + } + handler.addTask(new SyncConfigurationTask(handler, map)); } + private static Text getIncompatibleClientText(@Nullable String brand, Map> map) { + String brandText = switch (brand) { + case "fabric" -> "Fabric API"; + case null, default -> "Fabric Loader and Fabric API"; + }; + + final int toDisplay = 4; + + List namespaces = map.values().stream() + .map(Object2IntMap::keySet) + .flatMap(Set::stream) + .map(Identifier::getNamespace) + .filter(s -> !s.equals(Identifier.DEFAULT_NAMESPACE)) + .distinct() + .sorted() + .toList(); + + MutableText text = Text.literal("The following registry entry namespaces may be related:\n\n"); + + 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(ScreenTexts.LINE_BREAK); + } + + if (namespaces.size() > toDisplay) { + text = text.append(Text.literal("And %d more...".formatted(namespaces.size() - toDisplay))); + } + + return Text.literal("This server requires ").append(Text.literal(brandText).formatted(Formatting.GREEN)).append(" installed on your client!") + .append(ScreenTexts.LINE_BREAK).append(text) + .append(ScreenTexts.LINE_BREAK).append(ScreenTexts.LINE_BREAK).append(Text.literal("Contact the server's administrator for more information!").formatted(Formatting.GOLD)); + } + + private static boolean areAllRegistriesOptional(Map> map) { + return map.keySet().stream() + .map(Registries.REGISTRIES::get) + .filter(Objects::nonNull) + .map(RegistryAttributeHolder::get) + .allMatch(attributes -> attributes.hasAttribute(RegistryAttribute.OPTIONAL)); + } + public record SyncConfigurationTask( ServerConfigurationNetworkHandler handler, Map> map @@ -112,40 +152,6 @@ public final class RegistrySyncManager { } } - public static CompletableFuture receivePacket(ThreadExecutor executor, RegistryPacketHandler handler, T payload, boolean accept) { - handler.receivePayload(payload); - - if (!handler.isPacketFinished()) { - return CompletableFuture.completedFuture(false); - } - - if (DEBUG) { - String handlerName = handler.getClass().getSimpleName(); - LOGGER.info("{} total packet: {}", handlerName, handler.getTotalPacketReceived()); - LOGGER.info("{} raw size: {}", handlerName, handler.getRawBufSize()); - LOGGER.info("{} deflated size: {}", handlerName, handler.getDeflatedBufSize()); - } - - Map> map = handler.getSyncedRegistryMap(); - - if (!accept) { - return CompletableFuture.completedFuture(true); - } - - return executor.submit(() -> { - if (map == null) { - throw new CompletionException(new RemapException("Received null map in sync packet!")); - } - - try { - apply(map, RemappableRegistry.RemapMode.REMOTE); - return true; - } catch (RemapException e) { - throw new CompletionException(e); - } - }); - } - /** * Creates a {@link Map} used to sync the registry ids. * @@ -254,121 +260,6 @@ public final class RegistrySyncManager { return map; } - 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()) { - if (!containedRegistries.remove(registryId)) { - continue; - } - - Object2IntMap registryMap = map.get(registryId); - Registry registry = Registries.REGISTRIES.get(registryId); - - RegistryAttributeHolder attributeHolder = RegistryAttributeHolder.get(registry.getKey()); - - if (!attributeHolder.hasAttribute(RegistryAttribute.MODDED)) { - LOGGER.debug("Not applying registry data to vanilla registry {}", registryId.toString()); - continue; - } - - if (registry instanceof RemappableRegistry remappableRegistry) { - remappableRegistry.remap(registryId.toString(), registryMap, mode); - } else { - throw new RemapException("Registry " + registryId + " is not remappable"); - } - } - - if (!containedRegistries.isEmpty()) { - LOGGER.warn("[fabric-registry-sync] Could not find the following registries: " + Joiner.on(", ").join(containedRegistries)); - } - } - - @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.empty(); - - 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(ScreenTexts.LINE_BREAK); - } - - 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); - - if (registry instanceof RemappableRegistry) { - ((RemappableRegistry) registry).unmap(registryId.toString()); - } - } - } - public static void bootstrapRegistries() { postBootstrap = true; } diff --git a/fabric-registry-sync-v0/src/main/java/net/fabricmc/fabric/impl/registry/sync/RemappableRegistry.java b/fabric-registry-sync-v0/src/main/java/net/fabricmc/fabric/impl/registry/sync/RemappableRegistry.java index 8f3545e3e..44b04fc05 100644 --- a/fabric-registry-sync-v0/src/main/java/net/fabricmc/fabric/impl/registry/sync/RemappableRegistry.java +++ b/fabric-registry-sync-v0/src/main/java/net/fabricmc/fabric/impl/registry/sync/RemappableRegistry.java @@ -37,13 +37,9 @@ public interface RemappableRegistry { * client). */ REMOTE, - /** - * No differences in entry sets are allowed. - */ - EXACT } - void remap(String name, Object2IntMap remoteIndexedEntries, RemapMode mode) throws RemapException; + void remap(Object2IntMap remoteIndexedEntries, RemapMode mode) throws RemapException; - void unmap(String name) throws RemapException; + void unmap() throws RemapException; } diff --git a/fabric-registry-sync-v0/src/main/java/net/fabricmc/fabric/impl/registry/sync/packet/DirectRegistryPacketHandler.java b/fabric-registry-sync-v0/src/main/java/net/fabricmc/fabric/impl/registry/sync/packet/DirectRegistryPacketHandler.java index 9a4c4dfc6..602a5288f 100644 --- a/fabric-registry-sync-v0/src/main/java/net/fabricmc/fabric/impl/registry/sync/packet/DirectRegistryPacketHandler.java +++ b/fabric-registry-sync-v0/src/main/java/net/fabricmc/fabric/impl/registry/sync/packet/DirectRegistryPacketHandler.java @@ -17,7 +17,9 @@ package net.fabricmc.fabric.impl.registry.sync.packet; import java.util.ArrayList; +import java.util.Collections; import java.util.Comparator; +import java.util.EnumSet; import java.util.Iterator; import java.util.LinkedHashMap; import java.util.List; @@ -33,8 +35,12 @@ import org.jetbrains.annotations.Nullable; import net.minecraft.network.PacketByteBuf; import net.minecraft.network.codec.PacketCodec; import net.minecraft.network.packet.CustomPayload; +import net.minecraft.registry.Registries; +import net.minecraft.registry.Registry; import net.minecraft.util.Identifier; +import net.fabricmc.fabric.api.event.registry.RegistryAttribute; +import net.fabricmc.fabric.api.event.registry.RegistryAttributeHolder; import net.fabricmc.fabric.api.networking.v1.PacketByteBufs; /** @@ -65,6 +71,9 @@ public class DirectRegistryPacketHandler extends RegistryPacketHandler> syncedRegistryMap; + @Nullable + private Map> syncedRegistryAttributes; + private boolean isPacketFinished = false; private int totalPacketReceived = 0; @@ -89,6 +98,7 @@ public class DirectRegistryPacketHandler extends RegistryPacketHandler idMap = registryMap.get(regId); @@ -181,6 +191,7 @@ public class DirectRegistryPacketHandler extends RegistryPacketHandler(); + syncedRegistryAttributes = new LinkedHashMap<>(); int regNamespaceGroupAmount = combinedBuf.readVarInt(); for (int i = 0; i < regNamespaceGroupAmount; i++) { @@ -189,6 +200,7 @@ public class DirectRegistryPacketHandler extends RegistryPacketHandler attributes = decodeRegistryAttributes(combinedBuf.readByte()); Object2IntMap idMap = new Object2IntLinkedOpenHashMap<>(); int idNamespaceGroupAmount = combinedBuf.readVarInt(); @@ -214,7 +226,9 @@ public class DirectRegistryPacketHandler extends RegistryPacketHandler> getSyncedRegistryMap() { + public SyncedPacketData getSyncedPacketData() { Preconditions.checkState(isPacketFinished); - Map> map = syncedRegistryMap; + + if (syncedRegistryMap == null || syncedRegistryAttributes == null) { + return null; + } + + Map> map = Collections.unmodifiableMap(syncedRegistryMap); + Map> attributes = Collections.unmodifiableMap(syncedRegistryAttributes); + isPacketFinished = false; totalPacketReceived = 0; syncedRegistryMap = null; - return map; + syncedRegistryAttributes = null; + + return new SyncedPacketData(map, attributes); } private DirectRegistryPacketHandler.Payload createPayload(PacketByteBuf buf) { @@ -283,4 +306,32 @@ public class DirectRegistryPacketHandler extends RegistryPacketHandler registry = Registries.REGISTRIES.get(identifier); + + if (registry == null) { + return 0; + } + + RegistryAttributeHolder holder = RegistryAttributeHolder.get(registry); + byte encoded = 0; + + // Only send the optional marker. + if (holder.hasAttribute(RegistryAttribute.OPTIONAL)) { + encoded |= 0x1; + } + + return encoded; + } + + private static EnumSet decodeRegistryAttributes(byte encoded) { + EnumSet attributes = EnumSet.noneOf(RegistryAttribute.class); + + if ((encoded & 0x1) != 0) { + attributes.add(RegistryAttribute.OPTIONAL); + } + + return attributes; + } } diff --git a/fabric-registry-sync-v0/src/main/java/net/fabricmc/fabric/impl/registry/sync/packet/RegistryPacketHandler.java b/fabric-registry-sync-v0/src/main/java/net/fabricmc/fabric/impl/registry/sync/packet/RegistryPacketHandler.java index 7151465da..4a198793c 100644 --- a/fabric-registry-sync-v0/src/main/java/net/fabricmc/fabric/impl/registry/sync/packet/RegistryPacketHandler.java +++ b/fabric-registry-sync-v0/src/main/java/net/fabricmc/fabric/impl/registry/sync/packet/RegistryPacketHandler.java @@ -16,6 +16,7 @@ package net.fabricmc.fabric.impl.registry.sync.packet; +import java.util.EnumSet; import java.util.Map; import java.util.function.Consumer; import java.util.zip.Deflater; @@ -28,6 +29,7 @@ import net.minecraft.network.PacketByteBuf; import net.minecraft.network.packet.CustomPayload; import net.minecraft.util.Identifier; +import net.fabricmc.fabric.api.event.registry.RegistryAttribute; import net.fabricmc.fabric.api.networking.v1.PacketByteBufs; import net.fabricmc.fabric.impl.registry.sync.RegistrySyncManager; @@ -46,7 +48,7 @@ public abstract class RegistryPacketHandler> getSyncedRegistryMap(); + public abstract SyncedPacketData getSyncedPacketData(); protected final void computeBufSize(PacketByteBuf buf) { if (!RegistrySyncManager.DEBUG) { @@ -92,4 +94,9 @@ public abstract class RegistryPacketHandler> idMap, + Map> attributes + ) { } } diff --git a/fabric-registry-sync-v0/src/main/java/net/fabricmc/fabric/mixin/registry/sync/SimpleRegistryMixin.java b/fabric-registry-sync-v0/src/main/java/net/fabricmc/fabric/mixin/registry/sync/SimpleRegistryMixin.java index 0d59e39b8..7f678d5a8 100644 --- a/fabric-registry-sync-v0/src/main/java/net/fabricmc/fabric/mixin/registry/sync/SimpleRegistryMixin.java +++ b/fabric-registry-sync-v0/src/main/java/net/fabricmc/fabric/mixin/registry/sync/SimpleRegistryMixin.java @@ -73,9 +73,6 @@ public abstract class SimpleRegistryMixin implements MutableRegistry, Rema @Unique private static final Set VANILLA_NAMESPACES = Set.of("minecraft", "brigadier"); - @Unique - private static final Logger LOGGER = LoggerFactory.getLogger("FabricRegistrySync"); - @Shadow @Final private ObjectList> rawIdToEntry; @@ -123,7 +120,7 @@ public abstract class SimpleRegistryMixin implements MutableRegistry, Rema } @Inject(method = "(Lnet/minecraft/registry/RegistryKey;Lcom/mojang/serialization/Lifecycle;Z)V", at = @At("RETURN")) - private void init(RegistryKey key, Lifecycle lifecycle, boolean intrusive, CallbackInfo ci) { + private void init(RegistryKey key, Lifecycle lifecycle, boolean intrusive, CallbackInfo ci) { fabric_addObjectEvent = EventFactory.createArrayBacked(RegistryEntryAddedCallback.class, (callbacks) -> (rawId, id, object) -> { for (RegistryEntryAddedCallback callback : callbacks) { @@ -165,7 +162,7 @@ public abstract class SimpleRegistryMixin implements MutableRegistry, Rema } @Override - public void remap(String name, Object2IntMap remoteIndexedEntries, RemapMode mode) throws RemapException { + public void remap(Object2IntMap remoteIndexedEntries, RemapMode mode) throws RemapException { // Throw on invalid conditions. switch (mode) { case AUTHORITATIVE: @@ -184,34 +181,7 @@ public abstract class SimpleRegistryMixin implements MutableRegistry, Rema } if (strings != null) { - StringBuilder builder = new StringBuilder("Received ID map for " + name + " contains IDs unknown to the receiver!"); - - for (String s : strings) { - builder.append('\n').append(s); - } - - throw new RemapException(builder.toString()); - } - - break; - } - case EXACT: { - if (!idToEntry.keySet().equals(remoteIndexedEntries.keySet())) { - List strings = new ArrayList<>(); - - for (Identifier remoteId : remoteIndexedEntries.keySet()) { - if (!idToEntry.containsKey(remoteId)) { - strings.add(" - " + remoteId + " (missing on local)"); - } - } - - for (Identifier localId : getIds()) { - if (!remoteIndexedEntries.containsKey(localId)) { - strings.add(" - " + localId + " (missing on remote)"); - } - } - - StringBuilder builder = new StringBuilder("Local and remote ID sets for " + name + " do not match!"); + StringBuilder builder = new StringBuilder("Received ID map for " + getKey() + " contains IDs unknown to the receiver!"); for (String s : strings) { builder.append('\n').append(s); @@ -350,7 +320,7 @@ public abstract class SimpleRegistryMixin implements MutableRegistry, Rema } @Override - public void unmap(String name) throws RemapException { + public void unmap() throws RemapException { if (fabric_prevIndexedEntries != null) { List addedIds = new ArrayList<>(); @@ -372,7 +342,7 @@ public abstract class SimpleRegistryMixin implements MutableRegistry, Rema keyToEntry.put(entryKey, entry.getValue()); } - remap(name, fabric_prevIndexedEntries, RemapMode.AUTHORITATIVE); + remap(fabric_prevIndexedEntries, RemapMode.AUTHORITATIVE); for (Identifier id : addedIds) { fabric_getAddObjectEvent().invoker().onEntryAdded(entryToRawId.getInt(idToEntry.get(id)), id, get(id)); 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 index e374bbdb1..c71e2a916 100644 --- 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 @@ -3,5 +3,10 @@ "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.unknown-remote.footer" : "And %d more...", + "fabric-registry-sync-v0.unknown-registry.title.singular" : "Received a registry that is unknown to this client.\n", + "fabric-registry-sync-v0.unknown-registry.title.plural" : "Received %d registries that are unknown to this client.\n", + "fabric-registry-sync-v0.unknown-registry.subtitle.1" : "This is usually caused by a mismatched mod set between the client and server.", + "fabric-registry-sync-v0.unknown-registry.subtitle.2" : " See the client logs for more details.\nThe following registries are not present on the client:\n\n", + "fabric-registry-sync-v0.unknown-registry.footer" : "And %d more..." } diff --git a/fabric-registry-sync-v0/src/test/java/net/fabricmc/fabric/test/registry/sync/DirectRegistryPacketHandlerTest.java b/fabric-registry-sync-v0/src/test/java/net/fabricmc/fabric/test/registry/sync/DirectRegistryPacketHandlerTest.java index 8f6051b9e..acc9ba0b7 100644 --- a/fabric-registry-sync-v0/src/test/java/net/fabricmc/fabric/test/registry/sync/DirectRegistryPacketHandlerTest.java +++ b/fabric-registry-sync-v0/src/test/java/net/fabricmc/fabric/test/registry/sync/DirectRegistryPacketHandlerTest.java @@ -24,13 +24,22 @@ import java.util.Map; import it.unimi.dsi.fastutil.objects.Object2IntMap; import it.unimi.dsi.fastutil.objects.Object2IntOpenHashMap; +import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; +import net.minecraft.Bootstrap; +import net.minecraft.SharedConstants; import net.minecraft.util.Identifier; import net.fabricmc.fabric.impl.registry.sync.packet.DirectRegistryPacketHandler; public class DirectRegistryPacketHandlerTest { + @BeforeAll + static void beforeAll() { + SharedConstants.createGameVersion(); + Bootstrap.initialize(); + } + @Test void emptyRegistrySync() { DirectRegistryPacketHandler handler = new DirectRegistryPacketHandler(); @@ -48,7 +57,7 @@ public class DirectRegistryPacketHandlerTest { handler.receivePayload(payload); } - assertMatchesDeep(registry, handler.getSyncedRegistryMap()); + assertMatchesDeep(registry, handler.getSyncedPacketData().idMap()); } @Test @@ -69,7 +78,7 @@ public class DirectRegistryPacketHandlerTest { handler.receivePayload(payload); } - assertMatchesDeep(registry, handler.getSyncedRegistryMap()); + assertMatchesDeep(registry, handler.getSyncedPacketData().idMap()); } @Test @@ -93,7 +102,7 @@ public class DirectRegistryPacketHandlerTest { handler.receivePayload(payload); } - assertMatchesDeep(registry, handler.getSyncedRegistryMap()); + assertMatchesDeep(registry, handler.getSyncedPacketData().idMap()); } private static Object2IntMap createRegistry(int size) { diff --git a/fabric-registry-sync-v0/src/test/java/net/fabricmc/fabric/test/registry/sync/RegistryRemapTest.java b/fabric-registry-sync-v0/src/test/java/net/fabricmc/fabric/test/registry/sync/RegistryRemapTest.java new file mode 100644 index 000000000..0e5180d40 --- /dev/null +++ b/fabric-registry-sync-v0/src/test/java/net/fabricmc/fabric/test/registry/sync/RegistryRemapTest.java @@ -0,0 +1,310 @@ +/* + * 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.test.registry.sync; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.UUID; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CompletionException; +import java.util.concurrent.ExecutionException; + +import it.unimi.dsi.fastutil.objects.Object2IntMap; +import it.unimi.dsi.fastutil.objects.Object2IntOpenHashMap; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import net.minecraft.Bootstrap; +import net.minecraft.SharedConstants; +import net.minecraft.registry.Registry; +import net.minecraft.registry.RegistryKey; +import net.minecraft.registry.SimpleRegistry; +import net.minecraft.util.Identifier; +import net.minecraft.util.thread.ThreadExecutor; + +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.impl.client.registry.sync.ClientRegistrySyncHandler; +import net.fabricmc.fabric.impl.registry.sync.RegistryAttributeImpl; +import net.fabricmc.fabric.impl.registry.sync.RegistrySyncManager; +import net.fabricmc.fabric.impl.registry.sync.RemapException; +import net.fabricmc.fabric.impl.registry.sync.RemappableRegistry; +import net.fabricmc.fabric.impl.registry.sync.packet.DirectRegistryPacketHandler; + +public class RegistryRemapTest { + private RegistryKey> testRegistryKey; + private SimpleRegistry testRegistry; + + @BeforeAll + static void beforeAll() { + SharedConstants.createGameVersion(); + Bootstrap.initialize(); + } + + @BeforeEach + void beforeEach() { + testRegistryKey = RegistryKey.ofRegistry(id(UUID.randomUUID().toString())); + testRegistry = FabricRegistryBuilder.createSimple(testRegistryKey) + .attribute(RegistryAttribute.SYNCED) + .buildAndRegister(); + + Registry.register(testRegistry, id("zero"), "zero"); + Registry.register(testRegistry, id("one"), "one"); + Registry.register(testRegistry, id("two"), "two"); + } + + @AfterEach + void afterEach() throws RemapException { + // If a test fails, make sure we unmap the registry to avoid affecting other tests + RemappableRegistry remappableRegistry = (RemappableRegistry) testRegistry; + remappableRegistry.unmap(); + } + + @Test + void remapRegistry() throws RemapException { + RemappableRegistry remappableRegistry = (RemappableRegistry) testRegistry; + + assertEquals(0, testRegistry.getRawId("zero")); + assertEquals(1, testRegistry.getRawId("one")); + assertEquals(2, testRegistry.getRawId("two")); + + Map idMap = Map.of( + id("zero"), 2, + id("one"), 1, + id("two"), 0 + ); + remappableRegistry.remap(asFastMap(idMap), RemappableRegistry.RemapMode.AUTHORITATIVE); + + assertEquals(2, testRegistry.getRawId("zero")); + assertEquals(1, testRegistry.getRawId("one")); + assertEquals(0, testRegistry.getRawId("two")); + + remappableRegistry.unmap(); + + assertEquals(0, testRegistry.getRawId("zero")); + assertEquals(1, testRegistry.getRawId("one")); + assertEquals(2, testRegistry.getRawId("two")); + } + + @Test + void remapRegistryViaPacket() throws RemapException { + RemappableRegistry remappableRegistry = (RemappableRegistry) testRegistry; + + assertEquals(0, testRegistry.getRawId("zero")); + assertEquals(1, testRegistry.getRawId("one")); + assertEquals(2, testRegistry.getRawId("two")); + + Map idMap = Map.of( + id("two"), 0, + id("one"), 1, + id("zero"), 2 + ); + + var payloads = new ArrayList(); + + RegistrySyncManager.DIRECT_PACKET_HANDLER.sendPacket( + payloads::add, + Map.of(testRegistryKey.getValue(), asFastMap(idMap)) + ); + + List results = receivePayloads(payloads); + + // Expect 2 packets, 1 with the data (as it fits in one packet) and 1 empty packet to signal the end + assertEquals(2, results.size()); + assertFalse(results.getFirst()); + assertTrue(results.get(1)); + + assertEquals(2, testRegistry.getRawId("zero")); + assertEquals(1, testRegistry.getRawId("one")); + assertEquals(0, testRegistry.getRawId("two")); + + remappableRegistry.unmap(); + + assertEquals(0, testRegistry.getRawId("zero")); + assertEquals(1, testRegistry.getRawId("one")); + assertEquals(2, testRegistry.getRawId("two")); + } + + @Test + void unknownEntry() { + Map idMap = Map.of( + id("two"), 0, + id("one"), 1, + id("zero"), 2, + id("unknown"), 3 + ); + + var payloads = new ArrayList(); + + RegistrySyncManager.DIRECT_PACKET_HANDLER.sendPacket( + payloads::add, + Map.of(testRegistryKey.getValue(), asFastMap(idMap)) + ); + + RemapException remapException = assertThrows(RemapException.class, () -> receivePayloads(payloads)); + assertTrue(remapException.getMessage().contains("unknown-remote")); + } + + @Test + void unknownRegistry() { + Map idMap = Map.of( + id("two"), 0, + id("one"), 1, + id("zero"), 2 + ); + + var payloads = new ArrayList(); + + RegistrySyncManager.DIRECT_PACKET_HANDLER.sendPacket( + payloads::add, + Map.of(id("unknown"), asFastMap(idMap)) + ); + + RemapException remapException = assertThrows(RemapException.class, () -> receivePayloads(payloads)); + assertTrue(remapException.getMessage().contains("unknown-registry")); + } + + @Test + void unknownOptionalRegistry() throws RemapException { + Map idMap = Map.of( + id("two"), 0, + id("one"), 1, + id("zero"), 2 + ); + + RegistryAttributeImpl holder = (RegistryAttributeImpl) RegistryAttributeHolder.get(testRegistryKey); + holder.addAttribute(RegistryAttribute.OPTIONAL); + + var payloads = new ArrayList(); + + RegistrySyncManager.DIRECT_PACKET_HANDLER.sendPacket( + payloads::add, + Map.of(testRegistryKey.getValue(), asFastMap(idMap)) + ); + + // Packet should be handled without issue. + List results = receivePayloads(payloads); + assertEquals(2, results.size()); + assertFalse(results.getFirst()); + assertTrue(results.get(1)); + + holder.removeAttribute(RegistryAttribute.OPTIONAL); + } + + @Test + void missingRemoteEntries() throws RemapException { + RemappableRegistry remappableRegistry = (RemappableRegistry) testRegistry; + + assertEquals(0, testRegistry.getRawId("zero")); + assertEquals(1, testRegistry.getRawId("one")); + assertEquals(2, testRegistry.getRawId("two")); + + Map idMap = Map.of( + id("two"), 0, + id("zero"), 1 + ); + + var payloads = new ArrayList(); + + RegistrySyncManager.DIRECT_PACKET_HANDLER.sendPacket( + payloads::add, + Map.of(testRegistryKey.getValue(), asFastMap(idMap)) + ); + + receivePayloads(payloads); + + assertEquals(0, testRegistry.getRawId("two")); + assertEquals(1, testRegistry.getRawId("zero")); + // assigned an ID at the end of the registry + assertEquals(2, testRegistry.getRawId("one")); + + remappableRegistry.unmap(); + + assertEquals(0, testRegistry.getRawId("zero")); + assertEquals(1, testRegistry.getRawId("one")); + assertEquals(2, testRegistry.getRawId("two")); + } + + private static List receivePayloads(List payloads) throws RemapException { + var results = new ArrayList(); + + try { + for (DirectRegistryPacketHandler.Payload payload : payloads) { + CompletableFuture future = ClientRegistrySyncHandler.receivePacket( + ThisThreadExecutor.INSTANCE, + RegistrySyncManager.DIRECT_PACKET_HANDLER, + payload, + true + ); + results.add(future.get()); + } + } catch (CompletionException e) { + if (e.getCause() instanceof RemapException remapException) { + throw remapException; + } + + throw e; + } catch (ExecutionException | InterruptedException e) { + throw new RuntimeException(e); + } + + return results; + } + + private static Object2IntMap asFastMap(Map map) { + var fastMap = new Object2IntOpenHashMap(); + fastMap.putAll(map); + return fastMap; + } + + private static Identifier id(String path) { + return Identifier.of("registry_sync_test", path); + } + + // Run the task on the current thread instantly + private static class ThisThreadExecutor extends ThreadExecutor { + public static final ThisThreadExecutor INSTANCE = new ThisThreadExecutor(); + + private ThisThreadExecutor() { + super("Test thread executor"); + } + + @Override + protected boolean canExecute(Runnable task) { + return true; + } + + @Override + protected Thread getThread() { + return Thread.currentThread(); + } + + @Override + public Runnable createTask(Runnable runnable) { + return runnable; + } + } +} 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 054e49d39..5749d297c 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 @@ -18,13 +18,9 @@ package net.fabricmc.fabric.test.registry.sync; import java.util.ArrayList; import java.util.List; -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; @@ -37,19 +33,14 @@ 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; import net.fabricmc.fabric.api.event.registry.RegistryAttribute; import net.fabricmc.fabric.api.event.registry.RegistryAttributeHolder; -import net.fabricmc.fabric.impl.registry.sync.RegistrySyncManager; -import net.fabricmc.fabric.impl.registry.sync.RemapException; public class RegistrySyncTest implements ModInitializer { private static final Logger LOGGER = LogUtils.getLogger(); @@ -112,28 +103,6 @@ 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(); - }))); } public static void checkSyncedRegistry(RegistryKey> registry) { @@ -169,14 +138,4 @@ public class RegistrySyncTest implements ModInitializer { } } } - - private static Object2IntMap createFakeRegistryEntries() { - Object2IntMap map = new Object2IntOpenHashMap<>(); - - for (int i = 0; i < 12; i++) { - map.put(Identifier.of("mod_" + i, "entry"), 0); - } - - return map; - } } diff --git a/fabric-registry-sync-v0/src/testmod/resources/fabric.mod.json b/fabric-registry-sync-v0/src/testmod/resources/fabric.mod.json index 57745d062..486e0bba1 100644 --- a/fabric-registry-sync-v0/src/testmod/resources/fabric.mod.json +++ b/fabric-registry-sync-v0/src/testmod/resources/fabric.mod.json @@ -17,7 +17,8 @@ "net.fabricmc.fabric.test.registry.sync.RegistrySyncTest" ], "client": [ - "net.fabricmc.fabric.test.registry.sync.client.DynamicRegistryClientTest" + "net.fabricmc.fabric.test.registry.sync.client.DynamicRegistryClientTest", + "net.fabricmc.fabric.test.registry.sync.client.RegistrySyncClientTest" ] } } diff --git a/fabric-registry-sync-v0/src/testmodClient/java/net/fabricmc/fabric/test/registry/sync/client/RegistrySyncClientTest.java b/fabric-registry-sync-v0/src/testmodClient/java/net/fabricmc/fabric/test/registry/sync/client/RegistrySyncClientTest.java new file mode 100644 index 000000000..4e389ac47 --- /dev/null +++ b/fabric-registry-sync-v0/src/testmodClient/java/net/fabricmc/fabric/test/registry/sync/client/RegistrySyncClientTest.java @@ -0,0 +1,77 @@ +/* + * 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.test.registry.sync.client; + +import java.util.EnumSet; +import java.util.Map; +import java.util.Objects; + +import it.unimi.dsi.fastutil.objects.Object2IntMap; +import it.unimi.dsi.fastutil.objects.Object2IntOpenHashMap; + +import net.minecraft.registry.RegistryKeys; +import net.minecraft.server.command.CommandManager; +import net.minecraft.server.network.ServerPlayerEntity; +import net.minecraft.util.Identifier; + +import net.fabricmc.api.ClientModInitializer; +import net.fabricmc.fabric.api.command.v2.CommandRegistrationCallback; +import net.fabricmc.fabric.api.event.registry.RegistryAttribute; +import net.fabricmc.fabric.impl.client.registry.sync.ClientRegistrySyncHandler; +import net.fabricmc.fabric.impl.registry.sync.RemapException; +import net.fabricmc.fabric.impl.registry.sync.packet.RegistryPacketHandler; + +public class RegistrySyncClientTest implements ClientModInitializer { + @Override + public void onInitializeClient() { + 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() + ); + Map> attributes = Map.of( + RegistryKeys.BLOCK.getValue(), EnumSet.noneOf(RegistryAttribute.class), + RegistryKeys.ITEM.getValue(), EnumSet.noneOf(RegistryAttribute.class) + ); + + try { + ClientRegistrySyncHandler.checkRemoteRemap(new RegistryPacketHandler.SyncedPacketData(registryData, attributes)); + } 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 Object2IntMap createFakeRegistryEntries() { + Object2IntMap map = new Object2IntOpenHashMap<>(); + + for (int i = 0; i < 12; i++) { + map.put(Identifier.of("mod_" + i, "entry"), 0); + } + + return map; + } +}