Provide a RegistryByteBuf for attachment syncing (#4223)

* Provide a RegistryByteBuf for attachment syncing

* None breaking?

* Slight improvement

* Test syncing an item stack

* Jdoc fix

* Jdoc fix

fabric-data-attachment-api-v1/src/main/java/net/fabricmc/fabric/api/attachment/v1/AttachmentRegistry.java

@@ -22,7 +22,7 @@ import java.util.function.Supplier;
 import com.mojang.serialization.Codec;
 import org.jetbrains.annotations.ApiStatus;
 
-import net.minecraft.network.PacketByteBuf;
+import net.minecraft.network.RegistryByteBuf;
 import net.minecraft.network.codec.PacketCodec;
 import net.minecraft.util.Identifier;
 
@@ -157,7 +157,7 @@ public final class AttachmentRegistry {
 		 * @param syncPredicate an {@link AttachmentSyncPredicate} determining with which clients to synchronize data
 		 * @return the builder
 		 */
-		AttachmentRegistry.Builder<A> syncWith(PacketCodec<PacketByteBuf, A> packetCodec, AttachmentSyncPredicate syncPredicate);
+		AttachmentRegistry.Builder<A> syncWith(PacketCodec<? super RegistryByteBuf, A> packetCodec, AttachmentSyncPredicate syncPredicate);
 
 		/**
 		 * Builds and registers the {@link AttachmentType}.

fabric-data-attachment-api-v1/src/main/java/net/fabricmc/fabric/impl/attachment/AttachmentRegistryImpl.java

@@ -29,7 +29,7 @@ import org.jetbrains.annotations.Nullable;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import net.minecraft.network.PacketByteBuf;
+import net.minecraft.network.RegistryByteBuf;
 import net.minecraft.network.codec.PacketCodec;
 import net.minecraft.util.Identifier;
 
@@ -80,7 +80,7 @@ public final class AttachmentRegistryImpl {
 		@Nullable
 		private Codec<A> persistenceCodec = null;
 		@Nullable
-		private PacketCodec<PacketByteBuf, A> packetCodec = null;
+		private PacketCodec<? super RegistryByteBuf, A> packetCodec = null;
 		@Nullable
 		private AttachmentSyncPredicate syncPredicate = null;
 		private boolean copyOnDeath = false;
@@ -107,7 +107,8 @@ public final class AttachmentRegistryImpl {
 			return this;
 		}
 
-		public AttachmentRegistry.Builder<A> syncWith(PacketCodec<PacketByteBuf, A> packetCodec, AttachmentSyncPredicate syncPredicate) {
+		@Deprecated
+		public AttachmentRegistry.Builder<A> syncWith(PacketCodec<? super RegistryByteBuf, A> packetCodec, AttachmentSyncPredicate syncPredicate) {
 			Objects.requireNonNull(packetCodec, "packet codec cannot be null");
 			Objects.requireNonNull(syncPredicate, "sync predicate cannot be null");
 

fabric-data-attachment-api-v1/src/main/java/net/fabricmc/fabric/impl/attachment/AttachmentTargetImpl.java

@@ -22,6 +22,7 @@ import java.util.function.Consumer;
 import org.jetbrains.annotations.Nullable;
 
 import net.minecraft.nbt.NbtCompound;
+import net.minecraft.registry.DynamicRegistryManager;
 import net.minecraft.registry.RegistryWrapper;
 import net.minecraft.server.network.ServerPlayerEntity;
 
@@ -93,4 +94,6 @@ public interface AttachmentTargetImpl extends AttachmentTarget {
 	default boolean fabric_shouldTryToSync() {
 		throw new UnsupportedOperationException("Implemented via mixin");
 	}
+
+	DynamicRegistryManager fabric_getDynamicRegistryManager();
 }

fabric-data-attachment-api-v1/src/main/java/net/fabricmc/fabric/impl/attachment/AttachmentTypeImpl.java

@@ -21,7 +21,7 @@ import java.util.function.Supplier;
 import com.mojang.serialization.Codec;
 import org.jetbrains.annotations.Nullable;
 
-import net.minecraft.network.PacketByteBuf;
+import net.minecraft.network.RegistryByteBuf;
 import net.minecraft.network.codec.PacketCodec;
 import net.minecraft.util.Identifier;
 
@@ -32,7 +32,7 @@ public record AttachmentTypeImpl<A>(
 		Identifier identifier,
 		@Nullable Supplier<A> initializer,
 		@Nullable Codec<A> persistenceCodec,
-		@Nullable PacketCodec<PacketByteBuf, A> packetCodec,
+		@Nullable PacketCodec<? super RegistryByteBuf, A> packetCodec,
 		@Nullable AttachmentSyncPredicate syncPredicate,
 		boolean copyOnDeath
 ) implements AttachmentType<A> {

fabric-data-attachment-api-v1/src/main/java/net/fabricmc/fabric/impl/attachment/sync/AttachmentChange.java

@@ -20,15 +20,16 @@ import java.util.ArrayList;
 import java.util.Comparator;
 import java.util.List;
 import java.util.Objects;
-import java.util.Optional;
 import java.util.Set;
 
 import io.netty.buffer.Unpooled;
 import org.jetbrains.annotations.Nullable;
 
 import net.minecraft.network.PacketByteBuf;
+import net.minecraft.network.RegistryByteBuf;
 import net.minecraft.network.codec.PacketCodec;
 import net.minecraft.network.codec.PacketCodecs;
+import net.minecraft.registry.DynamicRegistryManager;
 import net.minecraft.server.network.ServerPlayerEntity;
 import net.minecraft.util.Identifier;
 import net.minecraft.world.World;
@@ -57,12 +58,20 @@ public record AttachmentChange(AttachmentTargetInfo<?> targetInfo, AttachmentTyp
 	private static final int MAX_DATA_SIZE_IN_BYTES = CustomPayloadS2CPacketAccessor.getMaxPayloadSize() - MAX_PADDING_SIZE_IN_BYTES;
 
 	@SuppressWarnings("unchecked")
-	public static AttachmentChange create(AttachmentTargetInfo<?> targetInfo, AttachmentType<?> type, @Nullable Object value) {
-		PacketCodec<PacketByteBuf, Object> codec = (PacketCodec<PacketByteBuf, Object>) ((AttachmentTypeImpl<?>) type).packetCodec();
+	public static AttachmentChange create(AttachmentTargetInfo<?> targetInfo, AttachmentType<?> type, @Nullable Object value, DynamicRegistryManager dynamicRegistryManager) {
+		PacketCodec<? super RegistryByteBuf, Object> codec = (PacketCodec<? super RegistryByteBuf, Object>) ((AttachmentTypeImpl<?>) type).packetCodec();
 		Objects.requireNonNull(codec, "attachment packet codec cannot be null");
+		Objects.requireNonNull(dynamicRegistryManager, "dynamic registry manager cannot be null");
+
+		RegistryByteBuf buf = new RegistryByteBuf(PacketByteBufs.create(), dynamicRegistryManager);
+
+		if (value != null) {
+			buf.writeBoolean(true);
+			codec.encode(buf, value);
+		} else {
+			buf.writeBoolean(false);
+		}
 
-		PacketByteBuf buf = PacketByteBufs.create();
-		buf.writeOptional(Optional.ofNullable(value), codec);
 		byte[] encoded = buf.array();
 
 		if (encoded.length > MAX_DATA_SIZE_IN_BYTES) {
@@ -109,15 +118,21 @@ public record AttachmentChange(AttachmentTargetInfo<?> targetInfo, AttachmentTyp
 
 	@SuppressWarnings("unchecked")
 	@Nullable
-	public Object decodeValue() {
-		PacketCodec<PacketByteBuf, Object> codec = (PacketCodec<PacketByteBuf, Object>) ((AttachmentTypeImpl<?>) type).packetCodec();
+	public Object decodeValue(DynamicRegistryManager dynamicRegistryManager) {
+		PacketCodec<? super RegistryByteBuf, Object> codec = (PacketCodec<? super RegistryByteBuf, Object>) ((AttachmentTypeImpl<?>) type).packetCodec();
 		Objects.requireNonNull(codec, "codec was null");
+		Objects.requireNonNull(dynamicRegistryManager, "dynamic registry manager cannot be null");
 
-		PacketByteBuf buf = new PacketByteBuf(Unpooled.copiedBuffer(data));
-		return buf.readOptional(codec).orElse(null);
+		RegistryByteBuf buf = new RegistryByteBuf(Unpooled.copiedBuffer(data), dynamicRegistryManager);
+
+		if (!buf.readBoolean()) {
+			return null;
+		}
+
+		return codec.decode(buf);
 	}
 
 	public void apply(World world) {
-		targetInfo.getTarget(world).setAttached((AttachmentType<Object>) type, decodeValue());
+		targetInfo.getTarget(world).setAttached((AttachmentType<Object>) type, decodeValue(world.getRegistryManager()));
 	}
 }

fabric-data-attachment-api-v1/src/main/java/net/fabricmc/fabric/mixin/attachment/AttachmentTargetsMixin.java

@@ -61,7 +61,7 @@ abstract class AttachmentTargetsMixin implements AttachmentTargetImpl {
 		this.fabric_markChanged(type);
 
 		if (this.fabric_shouldTryToSync() && type.isSynced()) {
-			AttachmentChange change = AttachmentChange.create(fabric_getSyncTargetInfo(), type, value);
+			AttachmentChange change = AttachmentChange.create(fabric_getSyncTargetInfo(), type, value, fabric_getDynamicRegistryManager());
 			acknowledgeSyncedEntry(type, change);
 			this.fabric_syncChange(type, new AttachmentSyncPayloadS2C(List.of(change)));
 		}
@@ -118,7 +118,7 @@ abstract class AttachmentTargetsMixin implements AttachmentTargetImpl {
 
 	@Unique
 	private void acknowledgeSynced(AttachmentType<?> type, Object value) {
-		acknowledgeSyncedEntry(type, AttachmentChange.create(fabric_getSyncTargetInfo(), type, value));
+		acknowledgeSyncedEntry(type, AttachmentChange.create(fabric_getSyncTargetInfo(), type, value, fabric_getDynamicRegistryManager()));
 	}
 
 	@Unique

fabric-data-attachment-api-v1/src/main/java/net/fabricmc/fabric/mixin/attachment/BlockEntityMixin.java

@@ -27,6 +27,7 @@ import org.spongepowered.asm.mixin.injection.callback.CallbackInfoReturnable;
 
 import net.minecraft.block.entity.BlockEntity;
 import net.minecraft.nbt.NbtCompound;
+import net.minecraft.registry.DynamicRegistryManager;
 import net.minecraft.registry.RegistryWrapper;
 import net.minecraft.util.math.BlockPos;
 import net.minecraft.world.World;
@@ -95,4 +96,9 @@ abstract class BlockEntityMixin implements AttachmentTargetImpl {
 		// Persistent attachments are read at a time with no world
 		return !this.hasWorld() || !this.world.isClient();
 	}
+
+	@Override
+	public DynamicRegistryManager fabric_getDynamicRegistryManager() {
+		return this.world.getRegistryManager();
+	}
 }

fabric-data-attachment-api-v1/src/main/java/net/fabricmc/fabric/mixin/attachment/ChunkMixin.java

@@ -20,6 +20,7 @@ import org.spongepowered.asm.mixin.Final;
 import org.spongepowered.asm.mixin.Mixin;
 import org.spongepowered.asm.mixin.Shadow;
 
+import net.minecraft.registry.DynamicRegistryManager;
 import net.minecraft.util.math.ChunkPos;
 import net.minecraft.world.chunk.Chunk;
 import net.minecraft.world.chunk.ChunkStatus;
@@ -67,4 +68,10 @@ abstract class ChunkMixin implements AttachmentTargetImpl {
 		// ProtoChunk or EmptyChunk
 		return false;
 	}
+
+	@Override
+	public DynamicRegistryManager fabric_getDynamicRegistryManager() {
+		// Should never happen as this is only used for sync
+		throw new UnsupportedOperationException("Chunk does not have a DynamicRegistryManager.");
+	}
 }

fabric-data-attachment-api-v1/src/main/java/net/fabricmc/fabric/mixin/attachment/EntityMixin.java

@@ -25,6 +25,7 @@ import org.spongepowered.asm.mixin.injection.callback.CallbackInfoReturnable;
 
 import net.minecraft.entity.Entity;
 import net.minecraft.nbt.NbtCompound;
+import net.minecraft.registry.DynamicRegistryManager;
 import net.minecraft.server.network.ServerPlayerEntity;
 import net.minecraft.world.World;
 
@@ -89,4 +90,9 @@ abstract class EntityMixin implements AttachmentTargetImpl {
 	public boolean fabric_shouldTryToSync() {
 		return !this.getWorld().isClient();
 	}
+
+	@Override
+	public DynamicRegistryManager fabric_getDynamicRegistryManager() {
+		return this.getWorld().getRegistryManager();
+	}
 }

fabric-data-attachment-api-v1/src/main/java/net/fabricmc/fabric/mixin/attachment/ServerWorldMixin.java

@@ -89,4 +89,9 @@ abstract class ServerWorldMixin extends World implements AttachmentTargetImpl {
 	public AttachmentTargetInfo<?> fabric_getSyncTargetInfo() {
 		return AttachmentTargetInfo.WorldTarget.INSTANCE;
 	}
+
+	@Override
+	public DynamicRegistryManager fabric_getDynamicRegistryManager() {
+		return getRegistryManager();
+	}
 }

fabric-data-attachment-api-v1/src/main/java/net/fabricmc/fabric/mixin/attachment/WorldChunkMixin.java

@@ -27,6 +27,7 @@ import org.spongepowered.asm.mixin.injection.Inject;
 import org.spongepowered.asm.mixin.injection.callback.CallbackInfo;
 
 import net.minecraft.block.entity.BlockEntity;
+import net.minecraft.registry.DynamicRegistryManager;
 import net.minecraft.server.network.ServerPlayerEntity;
 import net.minecraft.server.world.ServerWorld;
 import net.minecraft.util.math.BlockPos;
@@ -83,4 +84,9 @@ abstract class WorldChunkMixin extends AttachmentTargetsMixin implements Attachm
 	public boolean fabric_shouldTryToSync() {
 		return !this.world.isClient();
 	}
+
+	@Override
+	public DynamicRegistryManager fabric_getDynamicRegistryManager() {
+		return world.getRegistryManager();
+	}
 }

fabric-data-attachment-api-v1/src/main/java/net/fabricmc/fabric/mixin/attachment/WorldMixin.java

@@ -19,6 +19,7 @@ package net.fabricmc.fabric.mixin.attachment;
 import org.spongepowered.asm.mixin.Mixin;
 import org.spongepowered.asm.mixin.Shadow;
 
+import net.minecraft.registry.DynamicRegistryManager;
 import net.minecraft.world.World;
 
 import net.fabricmc.fabric.impl.attachment.AttachmentTargetImpl;
@@ -28,8 +29,16 @@ abstract class WorldMixin implements AttachmentTargetImpl {
 	@Shadow
 	public abstract boolean isClient();
 
+	@Shadow
+	public abstract DynamicRegistryManager getRegistryManager();
+
 	@Override
 	public boolean fabric_shouldTryToSync() {
 		return !this.isClient();
 	}
+
+	@Override
+	public DynamicRegistryManager fabric_getDynamicRegistryManager() {
+		return getRegistryManager();
+	}
 }

fabric-data-attachment-api-v1/src/main/java/net/fabricmc/fabric/mixin/attachment/WrapperProtoChunkMixin.java

@@ -25,6 +25,7 @@ import org.spongepowered.asm.mixin.Mixin;
 import org.spongepowered.asm.mixin.Shadow;
 
 import net.minecraft.nbt.NbtCompound;
+import net.minecraft.registry.DynamicRegistryManager;
 import net.minecraft.registry.RegistryWrapper;
 import net.minecraft.server.network.ServerPlayerEntity;
 import net.minecraft.world.chunk.WorldChunk;
@@ -103,4 +104,9 @@ abstract class WrapperProtoChunkMixin extends AttachmentTargetsMixin {
 	public void fabric_markChanged(AttachmentType<?> type) {
 		((AttachmentTargetImpl) wrapped).fabric_markChanged(type);
 	}
+
+	@Override
+	public DynamicRegistryManager fabric_getDynamicRegistryManager() {
+		return ((AttachmentTargetImpl) wrapped).fabric_getDynamicRegistryManager();
+	}
 }

fabric-data-attachment-api-v1/src/testmod/java/net/fabricmc/fabric/test/attachment/AttachmentTestMod.java

@@ -34,12 +34,15 @@ import net.minecraft.block.entity.BlockEntity;
 import net.minecraft.command.argument.BlockPosArgumentType;
 import net.minecraft.command.argument.ColumnPosArgumentType;
 import net.minecraft.command.argument.EntityArgumentType;
+import net.minecraft.entity.EquipmentSlot;
+import net.minecraft.item.ItemStack;
 import net.minecraft.network.codec.PacketCodecs;
 import net.minecraft.registry.Registries;
 import net.minecraft.registry.Registry;
 import net.minecraft.registry.RegistryKey;
 import net.minecraft.registry.RegistryKeys;
 import net.minecraft.server.command.ServerCommandSource;
+import net.minecraft.server.network.ServerPlayerEntity;
 import net.minecraft.server.world.ServerWorld;
 import net.minecraft.text.Text;
 import net.minecraft.util.Identifier;
@@ -63,6 +66,7 @@ import net.fabricmc.fabric.api.biome.v1.BiomeModifications;
 import net.fabricmc.fabric.api.biome.v1.BiomeSelectors;
 import net.fabricmc.fabric.api.command.v2.CommandRegistrationCallback;
 import net.fabricmc.fabric.api.event.lifecycle.v1.ServerChunkEvents;
+import net.fabricmc.fabric.api.event.lifecycle.v1.ServerEntityEvents;
 import net.fabricmc.fabric.api.event.lifecycle.v1.ServerLifecycleEvents;
 
 public class AttachmentTestMod implements ModInitializer {
@@ -80,28 +84,35 @@ public class AttachmentTestMod implements ModInitializer {
 			builder -> builder
 					.initializer(() -> false)
 					.persistent(Codec.BOOL)
-					.syncWith(PacketCodecs.BOOLEAN.cast(), AttachmentSyncPredicate.all())
+					.syncWith(PacketCodecs.BOOLEAN, AttachmentSyncPredicate.all())
 	);
 	public static final AttachmentType<Boolean> SYNCED_WITH_TARGET = AttachmentRegistry.create(
 			Identifier.of(MOD_ID, "synced_target"),
 			builder -> builder
 					.initializer(() -> false)
 					.persistent(Codec.BOOL)
-					.syncWith(PacketCodecs.BOOLEAN.cast(), AttachmentSyncPredicate.targetOnly())
+					.syncWith(PacketCodecs.BOOLEAN, AttachmentSyncPredicate.targetOnly())
 	);
 	public static final AttachmentType<Boolean> SYNCED_EXCEPT_TARGET = AttachmentRegistry.create(
 			Identifier.of(MOD_ID, "synced_except_target"),
 			builder -> builder
 					.initializer(() -> false)
 					.persistent(Codec.BOOL)
-					.syncWith(PacketCodecs.BOOLEAN.cast(), AttachmentSyncPredicate.allButTarget())
+					.syncWith(PacketCodecs.BOOLEAN, AttachmentSyncPredicate.allButTarget())
 	);
 	public static final AttachmentType<Boolean> SYNCED_CREATIVE_ONLY = AttachmentRegistry.create(
 			Identifier.of(MOD_ID, "synced_custom"),
 			builder -> builder
 					.initializer(() -> false)
 					.persistent(Codec.BOOL)
-					.syncWith(PacketCodecs.BOOLEAN.cast(), (target, player) -> player.isCreative())
+					.syncWith(PacketCodecs.BOOLEAN, (target, player) -> player.isCreative())
+	);
+	public static final AttachmentType<ItemStack> SYNCED_ITEM = AttachmentRegistry.create(
+			Identifier.of(MOD_ID, "synced_item"),
+			builder -> builder
+					.initializer(() -> ItemStack.EMPTY)
+					.persistent(ItemStack.CODEC)
+					.syncWith(ItemStack.OPTIONAL_PACKET_CODEC, AttachmentSyncPredicate.all())
 	);
 	public static final SimpleCommandExceptionType TARGET_NOT_FOUND = new SimpleCommandExceptionType(Text.literal("Target not found"));
 
@@ -196,6 +207,12 @@ public class AttachmentTestMod implements ModInitializer {
 						.then(buildCommandForKind("others_only", "all but self", SYNCED_EXCEPT_TARGET))
 						.then(buildCommandForKind("creative_only", "creative players only", SYNCED_CREATIVE_ONLY))
 		));
+
+		ServerEntityEvents.EQUIPMENT_CHANGE.register((livingEntity, equipmentSlot, previousStack, currentStack) -> {
+			if (equipmentSlot == EquipmentSlot.HEAD && livingEntity instanceof ServerPlayerEntity player) {
+				player.setAttached(SYNCED_ITEM, currentStack);
+			}
+		});
 	}
 
 	private static LiteralArgumentBuilder<ServerCommandSource> buildCommandForKind(String id, String syncedWith, AttachmentType<Boolean> type) {

fabric-data-attachment-api-v1/src/testmodClient/java/net/fabricmc/fabric/test/attachment/client/AttachmentTestModClient.java

@@ -37,6 +37,7 @@ import net.minecraft.command.argument.EntityArgumentType;
 import net.minecraft.command.argument.UuidArgumentType;
 import net.minecraft.entity.Entity;
 import net.minecraft.entity.player.PlayerEntity;
+import net.minecraft.item.ItemStack;
 import net.minecraft.text.Text;
 import net.minecraft.util.Colors;
 import net.minecraft.util.math.BlockPos;
@@ -99,6 +100,11 @@ public class AttachmentTestModClient implements ClientModInitializer {
 				Text.literal("Synced-with-creative attachment: %s".formatted(attCustom))
 						.withColor(attCustom ? target instanceof PlayerEntity p && p.isCreative() ? Colors.GREEN : Colors.RED : Colors.WHITE)
 		);
+		ItemStack stack = target.getAttachedOrCreate(AttachmentTestMod.SYNCED_ITEM);
+		context.getSource().sendFeedback(
+				Text.literal("Synced-item attachment: %s".formatted(stack))
+						.withColor(attOther ? target != MinecraftClient.getInstance().player ? Colors.GREEN : Colors.RED : Colors.WHITE)
+		);
 	}
 
 	@Override