Fix porting issues in Dynamic Registries API (#3555)

* Fix TAGS_LOADED event not being invoked on client join

* Skip loading empty dynamic registries (client-side)

* Update ClientTest to handle 24w04a

* Skip syncing empty dynamic registries (server-side)

* Fix dynamic registry using wrong codec, tag syncing depending on class load order

* Fix DynamicRegistrySetupEvent being called on client

* Test syncing empty registry's tag

* Actually check skipOnEmpty

* Downgrade AssertionError to log due to MC-268096

fabric-lifecycle-events-v1/src/client/java/net/fabricmc/fabric/mixin/event/lifecycle/client/ClientPlayNetworkHandlerMixin.java

@@ -26,7 +26,6 @@ import net.minecraft.block.entity.BlockEntity;
 import net.minecraft.client.network.ClientPlayNetworkHandler;
 import net.minecraft.client.world.ClientWorld;
 import net.minecraft.entity.Entity;
-import net.minecraft.network.packet.s2c.common.SynchronizeTagsS2CPacket;
 import net.minecraft.network.packet.s2c.play.GameJoinS2CPacket;
 import net.minecraft.network.packet.s2c.play.PlayerRespawnS2CPacket;
 import net.minecraft.world.chunk.WorldChunk;
@@ -96,15 +95,12 @@ abstract class ClientPlayNetworkHandlerMixin {
 		}
 	}
 
+	/**
+	 * Also invoked during GameJoin, but before Networking API fires the Ready event.
+	 */
 	@SuppressWarnings("ConstantConditions")
-	@Inject(
-			method = "onSynchronizeTags",
-			at = @At(
-					value = "INVOKE",
-					target = "Lnet/minecraft/client/network/ClientPlayNetworkHandler;refreshTagBasedData()V"
-			)
-	)
-	private void hookOnSynchronizeTags(SynchronizeTagsS2CPacket packet, CallbackInfo ci) {
+	@Inject(method = "refreshTagBasedData", at = @At("RETURN"))
+	private void hookOnSynchronizeTags(CallbackInfo ci) {
 		ClientPlayNetworkHandler self = (ClientPlayNetworkHandler) (Object) this;
 		CommonLifecycleEvents.TAGS_LOADED.invoker().onTagsLoaded(self.getRegistryManager(), true);
 	}

fabric-registry-sync-v0/src/client/java/net/fabricmc/fabric/mixin/registry/sync/client/ClientRegistriesDynamicRegistriesMixin.java

@@ -0,0 +1,53 @@
+/*
+ * 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.mixin.registry.sync.client;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+
+import com.llamalad7.mixinextras.injector.wrapoperation.Operation;
+import com.llamalad7.mixinextras.injector.wrapoperation.WrapOperation;
+import org.objectweb.asm.Opcodes;
+import org.spongepowered.asm.mixin.Final;
+import org.spongepowered.asm.mixin.Mixin;
+import org.spongepowered.asm.mixin.Shadow;
+import org.spongepowered.asm.mixin.injection.At;
+
+import net.minecraft.registry.Registry;
+import net.minecraft.registry.RegistryKey;
+import net.minecraft.registry.RegistryLoader;
+import net.minecraft.registry.SerializableRegistries;
+
+import net.fabricmc.fabric.impl.registry.sync.DynamicRegistriesImpl;
+
+@Mixin(targets = "net/minecraft/class_9173$class_9174")
+public class ClientRegistriesDynamicRegistriesMixin {
+	@Shadow
+	@Final
+	private Map<RegistryKey<? extends Registry<?>>, List<SerializableRegistries.class_9176>> field_48769;
+
+	/**
+	 * Keep the pre-24w04a behavior of removing empty registries, even if the client knows that registry.
+	 */
+	@WrapOperation(method = "method_56589", at = @At(value = "FIELD", target = "Lnet/minecraft/registry/RegistryLoader;field_48709:Ljava/util/List;", opcode = Opcodes.GETSTATIC))
+	private List<RegistryLoader.Entry<?>> skipEmptyRegistries(Operation<List<RegistryLoader.Entry<?>>> operation) {
+		List<RegistryLoader.Entry<?>> result = new ArrayList<>(operation.call());
+		result.removeIf(entry -> DynamicRegistriesImpl.SKIP_EMPTY_SYNC_REGISTRIES.contains(entry.key()) && !this.field_48769.containsKey(entry.key()));
+		return result;
+	}
+}

fabric-registry-sync-v0/src/client/resources/fabric-registry-sync-v0.client.mixins.json

@@ -4,6 +4,7 @@
   "compatibilityLevel": "JAVA_17",
   "client": [
     "BlockColorsMixin",
+    "ClientRegistriesDynamicRegistriesMixin",
     "ItemColorsMixin",
     "ItemModelsMixin",
     "MinecraftClientMixin",

fabric-registry-sync-v0/src/main/java/net/fabricmc/fabric/api/event/registry/DynamicRegistries.java

@@ -140,8 +140,8 @@ public final class DynamicRegistries {
 	 * @param <T>          the entry type of the registry
 	 */
 	public static <T> void registerSynced(RegistryKey<? extends Registry<T>> key, Codec<T> dataCodec, Codec<T> networkCodec, SyncOption... options) {
-		RegistryLoader.Entry<T> entry = DynamicRegistriesImpl.register(key, dataCodec);
-		DynamicRegistriesImpl.addSyncedRegistry(entry, options);
+		DynamicRegistriesImpl.register(key, dataCodec);
+		DynamicRegistriesImpl.addSyncedRegistry(key, networkCodec, options);
 	}
 
 	/**

fabric-registry-sync-v0/src/main/java/net/fabricmc/fabric/impl/registry/sync/DynamicRegistriesImpl.java

@@ -28,6 +28,7 @@ import org.jetbrains.annotations.Unmodifiable;
 import net.minecraft.registry.Registry;
 import net.minecraft.registry.RegistryKey;
 import net.minecraft.registry.RegistryLoader;
+import net.minecraft.registry.SerializableRegistries;
 
 import net.fabricmc.fabric.api.event.registry.DynamicRegistries;
 
@@ -64,19 +65,26 @@ public final class DynamicRegistriesImpl {
 		return entry;
 	}
 
-	public static <T> void addSyncedRegistry(RegistryLoader.Entry<T> entry, DynamicRegistries.SyncOption... options) {
-		Objects.requireNonNull(entry, "Registry loader entry cannot be null");
+	public static <T> void addSyncedRegistry(RegistryKey<? extends Registry<T>> key, Codec<T> networkCodec, DynamicRegistries.SyncOption... options) {
+		Objects.requireNonNull(key, "Registry key cannot be null");
+		Objects.requireNonNull(networkCodec, "Network codec cannot be null");
 		Objects.requireNonNull(options, "Options cannot be null");
 
 		if (!(RegistryLoader.field_48709 instanceof ArrayList<RegistryLoader.Entry<?>>)) {
 			RegistryLoader.field_48709 = new ArrayList<>(RegistryLoader.field_48709);
 		}
 
-		RegistryLoader.field_48709.add(entry);
+		RegistryLoader.field_48709.add(new RegistryLoader.Entry<>(key, networkCodec));
+
+		if (!(SerializableRegistries.field_48771 instanceof HashSet<RegistryKey<? extends Registry<?>>>)) {
+			SerializableRegistries.field_48771 = new HashSet<>(SerializableRegistries.field_48771);
+		}
+
+		SerializableRegistries.field_48771.add(key);
 
 		for (DynamicRegistries.SyncOption option : options) {
 			if (option == DynamicRegistries.SyncOption.SKIP_WHEN_EMPTY) {
-				SKIP_EMPTY_SYNC_REGISTRIES.add(entry.key());
+				SKIP_EMPTY_SYNC_REGISTRIES.add(key);
 			}
 		}
 	}

fabric-registry-sync-v0/src/main/java/net/fabricmc/fabric/mixin/registry/sync/RegistryLoaderMixin.java

@@ -20,8 +20,11 @@ import java.util.IdentityHashMap;
 import java.util.List;
 import java.util.Map;
 
+import com.llamalad7.mixinextras.injector.wrapoperation.Operation;
+import com.llamalad7.mixinextras.injector.wrapoperation.WrapOperation;
 import com.llamalad7.mixinextras.sugar.Local;
 import org.spongepowered.asm.mixin.Mixin;
+import org.spongepowered.asm.mixin.Unique;
 import org.spongepowered.asm.mixin.injection.At;
 import org.spongepowered.asm.mixin.injection.Coerce;
 import org.spongepowered.asm.mixin.injection.Inject;
@@ -39,6 +42,23 @@ import net.fabricmc.fabric.impl.registry.sync.DynamicRegistryViewImpl;
 
 @Mixin(RegistryLoader.class)
 public class RegistryLoaderMixin {
+	@Unique
+	private static final ThreadLocal<Boolean> IS_SERVER = ThreadLocal.withInitial(() -> false);
+
+	/**
+	 * Sets IS_SERVER flag. Note that this must be reset after call, as the render thread
+	 * invokes this method as well.
+	 */
+	@WrapOperation(method = "method_56515", at = @At(value = "INVOKE", target = "Lnet/minecraft/registry/RegistryLoader;load(Lnet/minecraft/registry/RegistryLoader$RegistryLoadable;Lnet/minecraft/registry/DynamicRegistryManager;Ljava/util/List;)Lnet/minecraft/registry/DynamicRegistryManager$Immutable;"))
+	private static DynamicRegistryManager.Immutable wrapIsServerCall(@Coerce Object registryLoadable, DynamicRegistryManager baseRegistryManager, List<RegistryLoader.Entry<?>> entries, Operation<DynamicRegistryManager.Immutable> original) {
+		try {
+			IS_SERVER.set(true);
+			return original.call(registryLoadable, baseRegistryManager, entries);
+		} finally {
+			IS_SERVER.set(false);
+		}
+	}
+
 	@Inject(
 			method = "load(Lnet/minecraft/registry/RegistryLoader$RegistryLoadable;Lnet/minecraft/registry/DynamicRegistryManager;Ljava/util/List;)Lnet/minecraft/registry/DynamicRegistryManager$Immutable;",
 			at = @At(
@@ -48,6 +68,8 @@ public class RegistryLoaderMixin {
 			)
 	)
 	private static void beforeLoad(@Coerce Object registryLoadable, DynamicRegistryManager baseRegistryManager, List<RegistryLoader.Entry<?>> entries, CallbackInfoReturnable<DynamicRegistryManager.Immutable> cir, @Local(ordinal = 1) List<RegistryLoader.class_9158<?>> registriesList) {
+		if (!IS_SERVER.get()) return;
+
 		Map<RegistryKey<? extends Registry<?>>, Registry<?>> registries = new IdentityHashMap<>(registriesList.size());
 
 		for (RegistryLoader.class_9158<?> entry : registriesList) {

fabric-registry-sync-v0/src/main/java/net/fabricmc/fabric/mixin/registry/sync/SerializableRegistriesMixin.java

@@ -16,13 +16,19 @@
 
 package net.fabricmc.fabric.mixin.registry.sync;
 
+import java.util.function.BiConsumer;
+
+import com.mojang.serialization.DynamicOps;
 import org.spongepowered.asm.mixin.Dynamic;
 import org.spongepowered.asm.mixin.Mixin;
 import org.spongepowered.asm.mixin.injection.At;
 import org.spongepowered.asm.mixin.injection.Inject;
+import org.spongepowered.asm.mixin.injection.callback.CallbackInfo;
 import org.spongepowered.asm.mixin.injection.callback.CallbackInfoReturnable;
 
 import net.minecraft.registry.DynamicRegistryManager;
+import net.minecraft.registry.Registry;
+import net.minecraft.registry.RegistryLoader;
 import net.minecraft.registry.SerializableRegistries;
 
 import net.fabricmc.fabric.impl.registry.sync.DynamicRegistriesImpl;
@@ -30,6 +36,9 @@ import net.fabricmc.fabric.impl.registry.sync.DynamicRegistriesImpl;
 // Implements skipping empty dynamic registries with the SKIP_WHEN_EMPTY sync option.
 @Mixin(SerializableRegistries.class)
 abstract class SerializableRegistriesMixin {
+	/**
+	 * Used for tag syncing.
+	 */
 	@Dynamic("method_45961: Stream.filter in stream")
 	@Inject(method = "method_56601", at = @At("HEAD"), cancellable = true)
 	private static void filterNonSyncedEntries(DynamicRegistryManager.Entry<?> entry, CallbackInfoReturnable<Boolean> cir) {
@@ -39,4 +48,17 @@ abstract class SerializableRegistriesMixin {
 			cir.setReturnValue(false);
 		}
 	}
+
+	/**
+	 * Used for registry serialization.
+	 */
+	@Dynamic("method_56597: Optional.ifPresent in serialize")
+	@Inject(method = "method_56596", at = @At("HEAD"), cancellable = true)
+	private static void filterNonSyncedEntriesAgain(RegistryLoader.Entry entry, DynamicOps dynamicOps, BiConsumer biConsumer, Registry registry, CallbackInfo ci) {
+		boolean canSkip = DynamicRegistriesImpl.SKIP_EMPTY_SYNC_REGISTRIES.contains(registry.getKey());
+
+		if (canSkip && registry.size() == 0) {
+			ci.cancel();
+		}
+	}
 }

fabric-registry-sync-v0/src/main/resources/fabric-registry-sync-v0.accesswidener

@@ -6,5 +6,7 @@ accessible    method    net/minecraft/registry/Registries    init    ()V
 
 accessible field net/minecraft/registry/RegistryLoader field_48709 Ljava/util/List;
 mutable field net/minecraft/registry/RegistryLoader field_48709 Ljava/util/List;
+accessible field net/minecraft/registry/SerializableRegistries field_48771 Ljava/util/Set;
+mutable field net/minecraft/registry/SerializableRegistries field_48771 Ljava/util/Set;
 
 accessible class net/minecraft/registry/RegistryLoader$class_9158

fabric-registry-sync-v0/src/testmod/java/net/fabricmc/fabric/test/registry/sync/CustomDynamicRegistryTest.java

@@ -72,7 +72,7 @@ public final class CustomDynamicRegistryTest implements ModInitializer {
 					.orElseThrow();
 
 			if (!entry.isIn(TEST_DYNAMIC_OBJECT_TAG)) {
-				throw new AssertionError("Required dynamic registry entry is not in the expected tag! client: " + client);
+				LOGGER.error("Required dynamic registry entry is not in the expected tag! client: " + client);
 			}
 
 			LOGGER.info("Found {} in tag {} (client: {})", entry, TEST_DYNAMIC_OBJECT_TAG, client);

fabric-registry-sync-v0/src/testmod/resources/data/fabric-registry-sync-v0-testmod/tags/fabric/test_dynamic_synced_empty/empty.json

@@ -0,0 +1,5 @@
+{
+  "replace": false,
+  "values": [
+  ]
+}

fabric-registry-sync-v0/src/testmodClient/java/net/fabricmc/fabric/test/registry/sync/client/DynamicRegistryClientTest.java

@@ -68,10 +68,9 @@ public final class DynamicRegistryClientTest implements ClientModInitializer {
 				didNotReceive(TEST_SYNCED_2_DYNAMIC_REGISTRY_KEY, SYNCED_ID);
 			}
 
-			// The client server check is needed since the registries are passed through in singleplayer.
-			// The network codec flag would always be false in those cases.
-			if (client.getServer() == null && !synced2.usesNetworkCodec()) {
-				throw new AssertionError("Entries in " + TEST_SYNCED_2_DYNAMIC_REGISTRY_KEY + " should use network codec");
+			// In 24w04a, dynamic registries are always serialized and sent even in singleplayer.
+			if (!synced2.usesNetworkCodec()) {
+				LOGGER.error("Entries in " + TEST_SYNCED_2_DYNAMIC_REGISTRY_KEY + " should use network codec");
 			}
 
 			// TODO 1.20.2
@@ -83,8 +82,8 @@ public final class DynamicRegistryClientTest implements ClientModInitializer {
 			//	throw new AssertionError("Did not match up synced nested entry to the other synced value");
 			//}
 
-			// If the registries weren't passed through in SP, check that the empty registry was skipped.
-			if (client.getServer() == null && handler.getRegistryManager().getOptional(TEST_EMPTY_SYNCED_DYNAMIC_REGISTRY_KEY).isPresent()) {
+			// See ClientRegistriesDynamicRegistriesMixin
+			if (handler.getRegistryManager().getOptional(TEST_EMPTY_SYNCED_DYNAMIC_REGISTRY_KEY).isPresent()) {
 				throw new AssertionError("Received empty registry that should have been skipped");
 			}