From d06600579577b746e703ec65a77f0a7cfbaa8490 Mon Sep 17 00:00:00 2001 From: Up <10714589+UpcraftLP@users.noreply.github.com> Date: Thu, 18 Jun 2026 02:52:49 +0200 Subject: [PATCH] fix bytebufs being treated as immutable (fixes #226) --- .../cca/api/v3/component/ComponentKey.java | 35 ++++++++++--------- .../api/v3/component/ComponentProvider.java | 3 +- .../cca/internal/base/CcaClientInternals.java | 12 +++++-- .../internal/base/ComponentUpdatePayload.java | 4 +-- .../cca/internal/base/MorePacketCodecs.java | 13 ------- .../mixin/block/common/MixinBlockEntity.java | 2 +- .../mixin/chunk/common/MixinWorldChunk.java | 2 +- .../v3/entity/C2SSelfMessagingComponent.java | 12 +++++-- .../entity/CardinalComponentsEntity.java | 10 ++++-- .../cca/mixin/entity/common/MixinEntity.java | 2 +- .../level/common/MixinLevelProperties.java | 2 +- .../scoreboard/MixinServerScoreboard.java | 2 +- .../cca/mixin/scoreboard/MixinTeam.java | 2 +- .../mixin/world/common/MixinServerWorld.java | 2 +- 14 files changed, 55 insertions(+), 48 deletions(-) diff --git a/cardinal-components-base/src/main/java/org/ladysnake/cca/api/v3/component/ComponentKey.java b/cardinal-components-base/src/main/java/org/ladysnake/cca/api/v3/component/ComponentKey.java index e01f855d..79fe192f 100644 --- a/cardinal-components-base/src/main/java/org/ladysnake/cca/api/v3/component/ComponentKey.java +++ b/cardinal-components-base/src/main/java/org/ladysnake/cca/api/v3/component/ComponentKey.java @@ -23,6 +23,7 @@ package org.ladysnake.cca.api.v3.component; import io.netty.buffer.Unpooled; +import net.fabricmc.fabric.api.networking.v1.PacketByteBufs; import net.fabricmc.fabric.api.networking.v1.ServerPlayNetworking; import net.fabricmc.loader.api.FabricLoader; import net.minecraft.network.PacketCallbacks; @@ -200,24 +201,26 @@ public void syncWith(ServerPlayerEntity player, ComponentProvider provider) { @ApiStatus.Experimental public void syncWith(ServerPlayerEntity player, ComponentProvider provider, ComponentPacketWriter writer, PlayerSyncPredicate predicate) { if (predicate.shouldSyncWith(player)) { - RegistryByteBuf buf = new RegistryByteBuf(Unpooled.buffer(), player.getEntityWorld().getRegistryManager()); - writer.writeSyncPacket(buf, player); - CustomPayload payload = provider.toComponentPacket(this, predicate.isRequiredOnClient(), buf); - - if (payload != null) { - if (ServerPlayNetworking.canSend(player, payload.getId())) { - ServerPlayNetworking.getSender(player).sendPacket(payload, PacketCallbacks.always(buf::release)); - } else { - if (predicate.isRequiredOnClient()) { - String specificMod = FabricLoader.getInstance().getModContainer(this.id.getNamespace()).map(c -> c.getMetadata().getName() + " and ").orElse(""); - player.networkHandler.disconnect(Text.literal( - "This server requires " + specificMod + "Cardinal Components API " + - "(unhandled packet: " + payload.getId().id() + ")" + - ComponentsInternals.getClientOptionalModAdvice())); + RegistryByteBuf buf = new RegistryByteBuf(PacketByteBufs.create(), player.getEntityWorld().getRegistryManager()); + try { + writer.writeSyncPacket(buf, player); + CustomPayload payload = provider.toComponentPacket(this, predicate.isRequiredOnClient(), buf); + if (payload != null) { + if (ServerPlayNetworking.canSend(player, payload.getId())) { + buf.retain(); // only release the buffer after the packet is sent, in case an implementation retains a reference to it + ServerPlayNetworking.getSender(player).sendPacket(payload, PacketCallbacks.always(buf::release)); + } else { + if (predicate.isRequiredOnClient()) { + String specificMod = FabricLoader.getInstance().getModContainer(this.id.getNamespace()).map(c -> c.getMetadata().getName() + " and ").orElse(""); + player.networkHandler.disconnect(Text.literal( + "This server requires " + specificMod + "Cardinal Components API " + + "(unhandled packet: " + payload.getId().id() + ")" + + ComponentsInternals.getClientOptionalModAdvice())); + } } - buf.release(); } - } else { + } + finally { buf.release(); } } diff --git a/cardinal-components-base/src/main/java/org/ladysnake/cca/api/v3/component/ComponentProvider.java b/cardinal-components-base/src/main/java/org/ladysnake/cca/api/v3/component/ComponentProvider.java index acc191b2..8f505f2c 100644 --- a/cardinal-components-base/src/main/java/org/ladysnake/cca/api/v3/component/ComponentProvider.java +++ b/cardinal-components-base/src/main/java/org/ladysnake/cca/api/v3/component/ComponentProvider.java @@ -29,7 +29,6 @@ import org.jetbrains.annotations.Nullable; import org.ladysnake.cca.api.v3.component.sync.AutoSyncedComponent; import org.ladysnake.cca.api.v3.component.sync.PlayerSyncPredicate; -import org.ladysnake.cca.internal.base.ComponentUpdatePayload; import java.util.List; @@ -67,7 +66,7 @@ default Iterable getRecipientsForComponentSync() { * @param key the key describing the component being synchronized * @param required {@code true} if attempting to sync a component key unknown to the client should disconnect it * @param data the component's raw sync data - * @return a {@link ComponentUpdatePayload} that has all the information required to perform the component sync + * @return a {@link CustomPayload} that has all the information required to perform the component sync * @since 6.0.0 */ @Nullable diff --git a/cardinal-components-base/src/main/java/org/ladysnake/cca/internal/base/CcaClientInternals.java b/cardinal-components-base/src/main/java/org/ladysnake/cca/internal/base/CcaClientInternals.java index 5115c5cd..40bf2563 100644 --- a/cardinal-components-base/src/main/java/org/ladysnake/cca/internal/base/CcaClientInternals.java +++ b/cardinal-components-base/src/main/java/org/ladysnake/cca/internal/base/CcaClientInternals.java @@ -22,8 +22,10 @@ */ package org.ladysnake.cca.internal.base; +import io.netty.buffer.Unpooled; import net.fabricmc.fabric.api.client.networking.v1.ClientPlayNetworking; import net.minecraft.network.DisconnectionInfo; +import net.minecraft.network.RegistryByteBuf; import net.minecraft.network.packet.CustomPayload; import net.minecraft.text.Text; import org.ladysnake.cca.api.v3.component.Component; @@ -38,15 +40,19 @@ public static > void registerComponentSync(C try { getter.apply(payload, ctx).ifPresent(c -> { if (c instanceof AutoSyncedComponent synced) { - synced.applySyncPacket(payload.buf()); + var buf = new RegistryByteBuf(Unpooled.wrappedBuffer(payload.rawPayload()), ctx.client().world.getRegistryManager()); + try { + synced.applySyncPacket(buf); + } + finally { + buf.release(); + } } }); } catch (UnknownComponentException e) { ctx.player().networkHandler.onDisconnected(new DisconnectionInfo(Text.literal( e.getMessage() + "\n(you are probably missing a mod installed on the server)" + ComponentsInternals.getClientOptionalModAdvice()) )); - } finally { - payload.buf().release(); } }); } diff --git a/cardinal-components-base/src/main/java/org/ladysnake/cca/internal/base/ComponentUpdatePayload.java b/cardinal-components-base/src/main/java/org/ladysnake/cca/internal/base/ComponentUpdatePayload.java index ae2a7aa7..43996b3b 100644 --- a/cardinal-components-base/src/main/java/org/ladysnake/cca/internal/base/ComponentUpdatePayload.java +++ b/cardinal-components-base/src/main/java/org/ladysnake/cca/internal/base/ComponentUpdatePayload.java @@ -38,7 +38,7 @@ public record ComponentUpdatePayload( T targetData, boolean required, Identifier componentKeyId, - RegistryByteBuf buf + byte[] rawPayload ) implements CustomPayload { public static CustomPayload.Id> id(String path) { return new CustomPayload.Id<>(Identifier.of("cardinal-components", path)); @@ -54,7 +54,7 @@ public static PacketCodec> codec( targetDataCodec, ComponentUpdatePayload::targetData, PacketCodecs.BOOLEAN, ComponentUpdatePayload::required, Identifier.PACKET_CODEC, ComponentUpdatePayload::componentKeyId, - MorePacketCodecs.REG_BYTE_BUF, ComponentUpdatePayload::buf, + PacketCodecs.BYTE_ARRAY, ComponentUpdatePayload::rawPayload, ComponentUpdatePayload::new ); } diff --git a/cardinal-components-base/src/main/java/org/ladysnake/cca/internal/base/MorePacketCodecs.java b/cardinal-components-base/src/main/java/org/ladysnake/cca/internal/base/MorePacketCodecs.java index dfb2dcc7..516cbcf3 100644 --- a/cardinal-components-base/src/main/java/org/ladysnake/cca/internal/base/MorePacketCodecs.java +++ b/cardinal-components-base/src/main/java/org/ladysnake/cca/internal/base/MorePacketCodecs.java @@ -30,17 +30,4 @@ public final class MorePacketCodecs { public static final PacketCodec EMPTY = PacketCodec.unit(Unit.INSTANCE); - - public static final PacketCodec REG_BYTE_BUF = PacketCodec.ofStatic( - (buf, value) -> { - buf.writeVarInt(value.readableBytes()); - buf.writeBytes(value); - }, - (buf) -> { - int readableBytes = buf.readVarInt(); - ByteBuf copy = Unpooled.buffer(readableBytes, readableBytes); - buf.readBytes(copy, readableBytes); - return new RegistryByteBuf(copy, buf.getRegistryManager()); - } - ); } diff --git a/cardinal-components-block/src/main/java/org/ladysnake/cca/mixin/block/common/MixinBlockEntity.java b/cardinal-components-block/src/main/java/org/ladysnake/cca/mixin/block/common/MixinBlockEntity.java index ee92614d..f2d5766d 100644 --- a/cardinal-components-block/src/main/java/org/ladysnake/cca/mixin/block/common/MixinBlockEntity.java +++ b/cardinal-components-block/src/main/java/org/ladysnake/cca/mixin/block/common/MixinBlockEntity.java @@ -111,7 +111,7 @@ public Iterable getRecipientsForComponentSync() { new BlockEntityAddress(this.getType(), this.getPos(), world.getRegistryKey()), required, key.getId(), - data + data.array() ); } } diff --git a/cardinal-components-chunk/src/main/java/org/ladysnake/cca/mixin/chunk/common/MixinWorldChunk.java b/cardinal-components-chunk/src/main/java/org/ladysnake/cca/mixin/chunk/common/MixinWorldChunk.java index 0ed461b5..5e596923 100644 --- a/cardinal-components-chunk/src/main/java/org/ladysnake/cca/mixin/chunk/common/MixinWorldChunk.java +++ b/cardinal-components-chunk/src/main/java/org/ladysnake/cca/mixin/chunk/common/MixinWorldChunk.java @@ -74,7 +74,7 @@ public Iterable getRecipientsForComponentSync() { this.getPos(), required, key.getId(), - data + data.array() ); } diff --git a/cardinal-components-entity/src/main/java/org/ladysnake/cca/api/v3/entity/C2SSelfMessagingComponent.java b/cardinal-components-entity/src/main/java/org/ladysnake/cca/api/v3/entity/C2SSelfMessagingComponent.java index 403d0062..5d31eae9 100644 --- a/cardinal-components-entity/src/main/java/org/ladysnake/cca/api/v3/entity/C2SSelfMessagingComponent.java +++ b/cardinal-components-entity/src/main/java/org/ladysnake/cca/api/v3/entity/C2SSelfMessagingComponent.java @@ -28,7 +28,6 @@ import net.fabricmc.fabric.api.networking.v1.PacketByteBufs; import net.fabricmc.fabric.api.networking.v1.PacketSender; import net.minecraft.client.MinecraftClient; -import net.minecraft.network.PacketCallbacks; import net.minecraft.network.RegistryByteBuf; import org.jetbrains.annotations.ApiStatus; import org.ladysnake.cca.api.v3.component.Component; @@ -82,7 +81,14 @@ default void sendC2SMessage(C2SComponentPacketWriter writer) { static void sendC2SMessage(ComponentKey key, C2SComponentPacketWriter writer) { PacketSender sender = ClientPlayNetworking.getSender(); // checks that the player is in game RegistryByteBuf buf = new RegistryByteBuf(PacketByteBufs.create(), Objects.requireNonNull(MinecraftClient.getInstance().getNetworkHandler()).getRegistryManager()); - writer.writeC2SPacket(buf); - sender.sendPacket(new ComponentUpdatePayload<>(CardinalComponentsEntity.C2S_SELF_PACKET_ID, Unit.INSTANCE, true, key.getId(), buf), PacketCallbacks.always(buf::release)); + byte[] bytes; + try { + writer.writeC2SPacket(buf); + bytes = buf.array(); + } + finally { + buf.release(); + } + sender.sendPacket(new ComponentUpdatePayload<>(CardinalComponentsEntity.C2S_SELF_PACKET_ID, Unit.INSTANCE, true, key.getId(), bytes)); } } diff --git a/cardinal-components-entity/src/main/java/org/ladysnake/cca/internal/entity/CardinalComponentsEntity.java b/cardinal-components-entity/src/main/java/org/ladysnake/cca/internal/entity/CardinalComponentsEntity.java index 9ee5201c..c0b3790a 100644 --- a/cardinal-components-entity/src/main/java/org/ladysnake/cca/internal/entity/CardinalComponentsEntity.java +++ b/cardinal-components-entity/src/main/java/org/ladysnake/cca/internal/entity/CardinalComponentsEntity.java @@ -23,6 +23,9 @@ package org.ladysnake.cca.internal.entity; import com.mojang.datafixers.util.Unit; +import io.netty.buffer.ByteBuf; +import io.netty.buffer.Unpooled; +import io.netty.buffer.UnpooledDirectByteBuf; import net.fabricmc.fabric.api.entity.event.v1.ServerLivingEntityEvents; import net.fabricmc.fabric.api.entity.event.v1.ServerPlayerEvents; import net.fabricmc.fabric.api.event.lifecycle.v1.ServerEntityEvents; @@ -33,6 +36,8 @@ import net.minecraft.entity.LivingEntity; import net.minecraft.entity.conversion.EntityConversionContext; import net.minecraft.entity.conversion.EntityConversionType; +import net.minecraft.network.PacketByteBuf; +import net.minecraft.network.RegistryByteBuf; import net.minecraft.network.codec.PacketCodecs; import net.minecraft.network.packet.CustomPayload; import net.minecraft.network.packet.s2c.common.CustomPayloadS2CPacket; @@ -82,11 +87,12 @@ public static void init() { PlayerSyncCallback.EVENT.register(player -> syncEntityComponents(player, player)); TrackingStartCallback.EVENT.register(CardinalComponentsEntity::syncEntityComponents); ServerPlayNetworking.registerGlobalReceiver(CardinalComponentsEntity.C2S_SELF_PACKET_ID, (payload, ctx) -> { + var buf = new RegistryByteBuf(Unpooled.wrappedBuffer(payload.rawPayload()), ctx.player().getRegistryManager()); try { Optional> componentKey = payload.componentKey(); if (componentKey.isPresent()) { if (componentKey.get().getNullable(ctx.player()) instanceof C2SSelfMessagingComponent synced) { - synced.handleC2SMessage(payload.buf()); + synced.handleC2SMessage(buf); } else if (payload.required() && (unknownC2SPlayerComponents.add(payload.componentKeyId()) || FabricLoader.getInstance().isDevelopmentEnvironment())) { // In prod, only log the first time an unknown component update is received ComponentsInternals.LOGGER.warn("[Cardinal Components API] Received an update for component {} from player {}, but this component is not registered for player entities", payload.componentKeyId(), ctx.player()); @@ -95,7 +101,7 @@ public static void init() { ComponentsInternals.LOGGER.warn("[Cardinal Components API] Received an update for unknown component {} from player {}", payload.componentKeyId(), ctx.player()); } } finally { - payload.buf().release(); + buf.release(); } }); } diff --git a/cardinal-components-entity/src/main/java/org/ladysnake/cca/mixin/entity/common/MixinEntity.java b/cardinal-components-entity/src/main/java/org/ladysnake/cca/mixin/entity/common/MixinEntity.java index 34436c4a..7ba60850 100644 --- a/cardinal-components-entity/src/main/java/org/ladysnake/cca/mixin/entity/common/MixinEntity.java +++ b/cardinal-components-entity/src/main/java/org/ladysnake/cca/mixin/entity/common/MixinEntity.java @@ -100,7 +100,7 @@ public ComponentUpdatePayload toComponentPack this.getId(), required, key.getId(), - data + data.array() ); } } diff --git a/cardinal-components-level/src/main/java/org/ladysnake/cca/mixin/level/common/MixinLevelProperties.java b/cardinal-components-level/src/main/java/org/ladysnake/cca/mixin/level/common/MixinLevelProperties.java index 2ca71c63..5a3af822 100644 --- a/cardinal-components-level/src/main/java/org/ladysnake/cca/mixin/level/common/MixinLevelProperties.java +++ b/cardinal-components-level/src/main/java/org/ladysnake/cca/mixin/level/common/MixinLevelProperties.java @@ -95,7 +95,7 @@ public ComponentUpdatePayload toComponentPack Unit.INSTANCE, required, key.getId(), - data + data.array() ); } diff --git a/cardinal-components-scoreboard/src/main/java/org/ladysnake/cca/mixin/scoreboard/MixinServerScoreboard.java b/cardinal-components-scoreboard/src/main/java/org/ladysnake/cca/mixin/scoreboard/MixinServerScoreboard.java index 0217eba0..de8988f9 100644 --- a/cardinal-components-scoreboard/src/main/java/org/ladysnake/cca/mixin/scoreboard/MixinServerScoreboard.java +++ b/cardinal-components-scoreboard/src/main/java/org/ladysnake/cca/mixin/scoreboard/MixinServerScoreboard.java @@ -96,7 +96,7 @@ public ComponentUpdatePayload toComponentPack Unit.INSTANCE, required, key.getId(), - data + data.array() ); } diff --git a/cardinal-components-scoreboard/src/main/java/org/ladysnake/cca/mixin/scoreboard/MixinTeam.java b/cardinal-components-scoreboard/src/main/java/org/ladysnake/cca/mixin/scoreboard/MixinTeam.java index ab3c11b6..f634a6ca 100644 --- a/cardinal-components-scoreboard/src/main/java/org/ladysnake/cca/mixin/scoreboard/MixinTeam.java +++ b/cardinal-components-scoreboard/src/main/java/org/ladysnake/cca/mixin/scoreboard/MixinTeam.java @@ -92,7 +92,7 @@ public ComponentUpdatePayload toComponentPack this.getName(), required, key.getId(), - data + data.array() ); } } diff --git a/cardinal-components-world/src/main/java/org/ladysnake/cca/mixin/world/common/MixinServerWorld.java b/cardinal-components-world/src/main/java/org/ladysnake/cca/mixin/world/common/MixinServerWorld.java index 19403a89..33c8ac6f 100644 --- a/cardinal-components-world/src/main/java/org/ladysnake/cca/mixin/world/common/MixinServerWorld.java +++ b/cardinal-components-world/src/main/java/org/ladysnake/cca/mixin/world/common/MixinServerWorld.java @@ -75,7 +75,7 @@ public ComponentUpdatePayload toComponentPack Unit.INSTANCE, required, key.getId(), - data + data.array() ); } }