From 00fb091965bc3ea13e91fd84ceee235fd11cdd27 Mon Sep 17 00:00:00 2001 From: MostCromulent <201167372+MostCromulent@users.noreply.github.com> Date: Wed, 29 Apr 2026 19:36:14 +0930 Subject: [PATCH 1/3] Encode network messages on calling thread; add fallback in event refs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The encoder previously ran on the Netty I/O thread, reading TrackableObject state concurrently with the game thread's set() calls. EnumMap.writeObject would intermittently fail with AIOOBE, and because RemoteClient.write had no failure listener, the dropped setGameView left the client tracker in an inconsistent state. Move encoding onto the calling thread (which is also the only mutator) and hand the resulting ByteBuf to the channel for async write, preserving non-blocking sends. Inside wrapped events, replace IdRef/StaleCardRef with a unified EventCardRef/ EventPlayerRef carrying name and imageKey. resolve() prefers the tracker but falls back to a detached view when the id isn't there — handles CardViews referenced by events that aren't reachable from GameView, without relying on cross-message ordering. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../gamemodes/net/CObjectOutputStream.java | 2 +- .../net/CompatibleObjectEncoder.java | 31 ++-- .../gamemodes/net/TrackableSerializer.java | 145 +++++++++--------- .../gamemodes/net/client/FGameClient.java | 15 +- .../gamemodes/net/server/RemoteClient.java | 46 +++++- 5 files changed, 146 insertions(+), 93 deletions(-) diff --git a/forge-gui/src/main/java/forge/gamemodes/net/CObjectOutputStream.java b/forge-gui/src/main/java/forge/gamemodes/net/CObjectOutputStream.java index 1867ba3b9432..1d481d180382 100644 --- a/forge-gui/src/main/java/forge/gamemodes/net/CObjectOutputStream.java +++ b/forge-gui/src/main/java/forge/gamemodes/net/CObjectOutputStream.java @@ -24,6 +24,6 @@ protected void writeClassDescriptor(ObjectStreamClass desc) throws IOException { @Override protected Object replaceObject(Object obj) throws IOException { - return TrackableSerializer.replace(obj, null); + return TrackableSerializer.replace(obj, null, false); } } diff --git a/forge-gui/src/main/java/forge/gamemodes/net/CompatibleObjectEncoder.java b/forge-gui/src/main/java/forge/gamemodes/net/CompatibleObjectEncoder.java index 91a5fd5e9d3c..62653a949dfb 100644 --- a/forge-gui/src/main/java/forge/gamemodes/net/CompatibleObjectEncoder.java +++ b/forge-gui/src/main/java/forge/gamemodes/net/CompatibleObjectEncoder.java @@ -5,6 +5,7 @@ import forge.trackable.Tracker; import forge.util.IHasForgeLog; import io.netty.buffer.ByteBuf; +import io.netty.buffer.ByteBufAllocator; import io.netty.buffer.ByteBufOutputStream; import io.netty.channel.ChannelHandlerContext; import io.netty.handler.codec.MessageToByteEncoder; @@ -29,6 +30,23 @@ public void setTracker(Tracker tracker) { @Override protected void encode(ChannelHandlerContext ctx, Serializable msg, ByteBuf out) throws Exception { + encodeInto(msg, out, this.tracker, this.byteTracker); + } + + /** Caller passes the returned buffer to writeAndFlush, which takes ownership. */ + public ByteBuf encodeToBuf(Serializable msg, ByteBufAllocator alloc) throws Exception { + ByteBuf out = alloc.buffer(); + try { + encodeInto(msg, out, this.tracker, this.byteTracker); + } catch (Exception e) { + out.release(); + throw e; + } + return out; + } + + private static void encodeInto(Serializable msg, ByteBuf out, Tracker tracker, + NetworkByteTracker byteTracker) throws Exception { int startIdx = out.writerIndex(); ByteBufOutputStream bout = new ByteBufOutputStream(out); ObjectOutputStream oout = null; @@ -39,7 +57,7 @@ protected void encode(ChannelHandlerContext ctx, Serializable msg, ByteBuf out) bout.write(LENGTH_PLACEHOLDER); if (GuiBase.hasPropertyConfig()) { oout = replace - ? new TrackableSerializer.ReplacingOutputStream(new LZ4BlockOutputStream(bout), tracker) + ? new TrackableSerializer.ReplacingOutputStream(new LZ4BlockOutputStream(bout), tracker, false) : new ObjectOutputStream(new LZ4BlockOutputStream(bout)); } else { oout = new CObjectOutputStream(new LZ4BlockOutputStream(bout), replace); @@ -68,16 +86,7 @@ protected void encode(ChannelHandlerContext ctx, Serializable msg, ByteBuf out) } } - /** - * Determines whether TrackableObject references should be replaced with - * IdRef markers for this message. Excludes only: - * - setGameView/openView: carry full state to populate the client's Tracker - * - * applyDelta is NOT excluded: its property maps already use Integer IDs - * (via DeltaSyncManager.toNetworkValue), and bundled events are wrapped - * by TrackableSerializer.wrapEvents before entering the packet, so no - * raw TrackableObjects remain in the serialization graph. - */ + // setGameView and openView carry full state to populate the receiver's tracker. private static boolean shouldReplaceTrackables(Serializable msg) { if (msg instanceof GuiGameEvent event) { ProtocolMethod method = event.getMethod(); diff --git a/forge-gui/src/main/java/forge/gamemodes/net/TrackableSerializer.java b/forge-gui/src/main/java/forge/gamemodes/net/TrackableSerializer.java index 7208f8343b06..c0326e032b86 100644 --- a/forge-gui/src/main/java/forge/gamemodes/net/TrackableSerializer.java +++ b/forge-gui/src/main/java/forge/gamemodes/net/TrackableSerializer.java @@ -18,24 +18,14 @@ import java.util.ArrayList; import java.util.List; -/** - * Handles serialization of {@link TrackableObject} references across the network. - * Replaces CardView/PlayerView with lightweight {@link IdRef} markers during - * encoding and resolves them back from the Tracker during decoding. - * - *

Used by the Netty encoder/decoder pipeline ({@link CompatibleObjectEncoder}, - * {@link CompatibleObjectDecoder}) and the mobile codec path - * ({@link CObjectOutputStream}, {@link CObjectInputStream}). - */ +/** Replaces CardView/PlayerView refs with compact wire markers; resolves on the receiving side. */ public final class TrackableSerializer { private static final TaggedLogger netLog = Logger.tag("NETWORK"); static final byte TYPE_CARD_VIEW = 0; static final byte TYPE_PLAYER_VIEW = 1; - /** - * Lightweight serializable marker that replaces a TrackableObject reference. - */ + /** Marker for tracker-stable refs (top-level protocol method args). */ static final class IdRef implements Serializable { private static final long serialVersionUID = 1L; final byte typeTag; @@ -47,23 +37,33 @@ static final class IdRef implements Serializable { } } - /** - * Marker for a stale CardView reference — the object holds a previous - * incarnation of a card (same ID, different Java object) that has since - * been replaced in the tracker by a zone-change copy. Carries the - * image key and name so the decoder can construct a detached CardView - * with the correct display data. - */ - static final class StaleCardRef implements Serializable { + /** CardView ref inside a wrapped event. {@code preserveSnapshot=true} forces fallback even if the tracker has the id. */ + static final class EventCardRef implements Serializable { private static final long serialVersionUID = 1L; final int id; - final String imageKey; final String name; + final String imageKey; + final boolean preserveSnapshot; - StaleCardRef(int id, String imageKey, String name) { + EventCardRef(int id, String name, String imageKey, boolean preserveSnapshot) { this.id = id; + this.name = name; this.imageKey = imageKey; + this.preserveSnapshot = preserveSnapshot; + } + } + + /** PlayerView ref inside a wrapped event. {@code preserveSnapshot=true} forces fallback even if the tracker has the id. */ + static final class EventPlayerRef implements Serializable { + private static final long serialVersionUID = 1L; + final int id; + final String name; + final boolean preserveSnapshot; + + EventPlayerRef(int id, String name, boolean preserveSnapshot) { + this.id = id; this.name = name; + this.preserveSnapshot = preserveSnapshot; } } @@ -81,45 +81,45 @@ static TrackableType trackableTypeFor(byte typeTag) { } } - /** - * Replaces TrackableObject references with {@link IdRef} markers, or - * {@link StaleCardRef} markers for CardViews whose tracker entry has - * been replaced by a zone-change copy. When {@code tracker} is null, - * stale detection is skipped (used by the client encoder, which has - * no game-state awareness). - */ - static Object replace(Object obj, Tracker tracker) { + static Object replace(Object obj, Tracker tracker, boolean eventMode) { if (obj instanceof TrackableObject trackable) { byte tag = typeTagFor(trackable); - if (tag >= 0) { - if (tracker != null) { - TrackableType type = trackableTypeFor(tag); - if (type != null) { - Object tracked = tracker.getObj(type, trackable.getId()); - if (tracked != trackable && tag == TYPE_CARD_VIEW && tracked != null) { - // Stale reference: previous incarnation of this card - CardView cv = (CardView) trackable; - String imgKey = cv.getCurrentState() != null - ? cv.getCurrentState().getImageKey(null) : null; - return new StaleCardRef(cv.getId(), imgKey, cv.getName()); - } + if (tag < 0) return obj; + + if (!eventMode) { + return new IdRef(tag, trackable.getId()); + } + + boolean preserveSnapshot = false; + if (tracker != null) { + TrackableType type = trackableTypeFor(tag); + if (type != null) { + Object tracked = tracker.getObj(type, trackable.getId()); + if (tracked != null && tracked != trackable) { + preserveSnapshot = true; } } - return new IdRef(tag, trackable.getId()); + } + if (tag == TYPE_CARD_VIEW) { + CardView cv = (CardView) trackable; + String imgKey = cv.getCurrentState() != null + ? cv.getCurrentState().getImageKey(null) : null; + return new EventCardRef(trackable.getId(), cv.getName(), imgKey, preserveSnapshot); + } + if (tag == TYPE_PLAYER_VIEW) { + PlayerView pv = (PlayerView) trackable; + return new EventPlayerRef(trackable.getId(), pv.getName(), preserveSnapshot); } } return obj; } - /** - * Resolves {@link IdRef} and {@link StaleCardRef} markers back to - * TrackableObjects from the given Tracker. - */ static Object resolve(Object obj, Tracker tracker) { - if (obj instanceof StaleCardRef ref) { - // Create a detached CardView with the correct image key. - // Not registered in the tracker — used only for display - // (game log thumbnail) so it won't affect live game state. + if (obj instanceof EventCardRef ref) { + if (!ref.preserveSnapshot) { + CardView fromTracker = tracker.getObj(TrackableTypes.CardViewType, ref.id); + if (fromTracker != null) return fromTracker; + } CardView detached = new CardView(ref.id, tracker); if (ref.name != null) { detached.set(TrackableProperty.Name, ref.name); @@ -130,6 +130,17 @@ static Object resolve(Object obj, Tracker tracker) { } return detached; } + if (obj instanceof EventPlayerRef ref) { + if (!ref.preserveSnapshot) { + PlayerView fromTracker = tracker.getObj(TrackableTypes.PlayerViewType, ref.id); + if (fromTracker != null) return fromTracker; + } + PlayerView detached = new PlayerView(ref.id, tracker); + if (ref.name != null) { + detached.set(TrackableProperty.Name, ref.name); + } + return detached; + } if (obj instanceof IdRef ref) { TrackableType type = trackableTypeFor(ref.typeTag); if (type != null) { @@ -143,16 +154,12 @@ static Object resolve(Object obj, Tracker tracker) { return obj; } - /** - * Measures serialized size matching the encoder wire format - * for applyDelta messages with IdRef replacement (when tracker not null). - * otherwise for setGameView messages. - */ + /** Approximate serialized size for telemetry. */ public static int measureSize(Object obj, Tracker tracker) { try { ByteArrayOutputStream baos = new ByteArrayOutputStream(); LZ4BlockOutputStream lz4Out = new LZ4BlockOutputStream(baos); - ObjectOutputStream oos = tracker == null ? new ObjectOutputStream(lz4Out) : new ReplacingOutputStream(lz4Out, tracker); + ObjectOutputStream oos = tracker == null ? new ObjectOutputStream(lz4Out) : new ReplacingOutputStream(lz4Out, tracker, false); oos.writeObject(obj); oos.close(); return baos.size(); @@ -161,29 +168,18 @@ public static int measureSize(Object obj, Tracker tracker) { } } - /** - * Serializable wrapper for a GameEvent whose TrackableObject references - * have been replaced with IdRef/StaleCardRef markers. Stored in - * DeltaPacket.events so events travel as compact byte arrays rather than - * full object graphs. Unwrapped after delta state is applied, when the - * client tracker is populated. - */ static final class WrappedEvent implements Serializable { private static final long serialVersionUID = 1L; final byte[] data; WrappedEvent(byte[] data) { this.data = data; } } - /** - * Wraps GameEvents by serializing each with IdRef replacement. - * Events that fail to serialize are dropped (logged). - */ public static List wrapEvents(List events, Tracker tracker) { List wrapped = new ArrayList<>(events.size()); for (GameEvent event : events) { try { ByteArrayOutputStream baos = new ByteArrayOutputStream(256); - try (ReplacingOutputStream out = new ReplacingOutputStream(baos, tracker)) { + try (ReplacingOutputStream out = new ReplacingOutputStream(baos, tracker, true)) { out.writeObject(event); } wrapped.add(new WrappedEvent(baos.toByteArray())); @@ -194,11 +190,6 @@ public static List wrapEvents(List events, Tracker tracker) { return wrapped; } - /** - * Unwraps events by deserializing with IdRef resolution from the tracker. - * Called after delta state is applied so new objects are resolvable. - * Events that fail to unwrap are dropped (logged). - */ public static List unwrapEvents(List items, Tracker tracker) { List events = new ArrayList<>(items.size()); for (Object item : items) { @@ -219,16 +210,18 @@ public static List unwrapEvents(List items, Tracker tracker) static class ReplacingOutputStream extends ObjectOutputStream { private final Tracker tracker; + private final boolean eventMode; - ReplacingOutputStream(OutputStream out, Tracker tracker) throws IOException { + ReplacingOutputStream(OutputStream out, Tracker tracker, boolean eventMode) throws IOException { super(out); this.tracker = tracker; + this.eventMode = eventMode; enableReplaceObject(true); } @Override protected Object replaceObject(Object obj) { - return replace(obj, tracker); + return replace(obj, tracker, eventMode); } } diff --git a/forge-gui/src/main/java/forge/gamemodes/net/client/FGameClient.java b/forge-gui/src/main/java/forge/gamemodes/net/client/FGameClient.java index 0f20a48669b5..28b91d5cdebb 100644 --- a/forge-gui/src/main/java/forge/gamemodes/net/client/FGameClient.java +++ b/forge-gui/src/main/java/forge/gamemodes/net/client/FGameClient.java @@ -11,6 +11,7 @@ import forge.gui.interfaces.IGuiGame; import forge.interfaces.ILobbyListener; import io.netty.bootstrap.Bootstrap; +import io.netty.buffer.ByteBuf; import io.netty.channel.*; import io.netty.channel.nio.NioEventLoopGroup; import io.netty.channel.socket.SocketChannel; @@ -105,7 +106,19 @@ public void send(final NetEvent event) { return; } netLog.info("Client sent {}", event); - channel.writeAndFlush(event); + final CompatibleObjectEncoder encoder = channel.pipeline().get(CompatibleObjectEncoder.class); + if (encoder == null) { + netLog.error("No encoder in client pipeline for {}", event); + return; + } + final ByteBuf encoded; + try { + encoded = encoder.encodeToBuf(event, channel.alloc()); + } catch (Exception e) { + netLog.error(e, "Client encode error for {}", event); + return; + } + channel.writeAndFlush(encoded); } /** diff --git a/forge-gui/src/main/java/forge/gamemodes/net/server/RemoteClient.java b/forge-gui/src/main/java/forge/gamemodes/net/server/RemoteClient.java index ff9676bb8ed2..aa3ce9874b9e 100644 --- a/forge-gui/src/main/java/forge/gamemodes/net/server/RemoteClient.java +++ b/forge-gui/src/main/java/forge/gamemodes/net/server/RemoteClient.java @@ -7,7 +7,7 @@ import forge.gamemodes.net.event.IdentifiableNetEvent; import forge.gamemodes.net.event.NetEvent; import forge.util.IHasForgeLog; - +import io.netty.buffer.ByteBuf; import io.netty.channel.Channel; import java.util.concurrent.atomic.AtomicInteger; @@ -54,11 +54,31 @@ public boolean hasValidSlot() { return index >= 0; } + /** Encodes synchronously on the caller's thread. Returns null on failure (logged). */ + private ByteBuf encodeOnCallingThread(final NetEvent event) { + final Channel ch = channel; + final CompatibleObjectEncoder encoder = ch.pipeline().get(CompatibleObjectEncoder.class); + if (encoder == null) { + netLog.error("No encoder in pipeline for {} (event: {})", username, event); + sendErrors.incrementAndGet(); + return null; + } + try { + return encoder.encodeToBuf(event, ch.alloc()); + } catch (Exception e) { + sendErrors.incrementAndGet(); + netLog.error(e, "Network encode error for {} (event: {})", username, event); + return null; + } + } + @Override public void send(final NetEvent event) { final Channel ch = channel; recordSendIfSaturated(ch); - ch.writeAndFlush(event).addListener(f -> { + final ByteBuf encoded = encodeOnCallingThread(event); + if (encoded == null) return; + ch.writeAndFlush(encoded).addListener(f -> { if (!f.isSuccess()) { sendErrors.incrementAndGet(); Throwable c = f.cause(); @@ -76,7 +96,20 @@ public void send(final NetEvent event) { public void write(final NetEvent event) { final Channel ch = channel; recordSendIfSaturated(ch); - ch.write(event); + final ByteBuf encoded = encodeOnCallingThread(event); + if (encoded == null) return; + ch.write(encoded).addListener(f -> { + if (!f.isSuccess()) { + sendErrors.incrementAndGet(); + Throwable c = f.cause(); + if (c != null) { + netLog.error(c, "Network write error for {} (event: {})", username, event); + } else { + netLog.error("Network write error for {} (event: {}, cause: {})", + username, event, f.isCancelled() ? "cancelled" : "no cause"); + } + } + }); } @Override @@ -84,7 +117,12 @@ public Object sendAndWait(final IdentifiableNetEvent event) { replies.initialize(event.getId()); final Channel ch = channel; recordSendIfSaturated(ch); - ch.writeAndFlush(event).addListener(f -> { + final ByteBuf encoded = encodeOnCallingThread(event); + if (encoded == null) { + replies.complete(event.getId(), null); + return replies.get(event.getId()); + } + ch.writeAndFlush(encoded).addListener(f -> { if (!f.isSuccess()) { sendErrors.incrementAndGet(); Throwable c = f.cause(); From 99976a386eb09508894ca38a8b2ecac769660a7d Mon Sep 17 00:00:00 2001 From: MostCromulent <201167372+MostCromulent@users.noreply.github.com> Date: Thu, 30 Apr 2026 05:45:30 +0930 Subject: [PATCH 2/3] Drop EventPlayerRef; restore JavaDoc; convert refs to records Address review on PR #10561: - Drop EventPlayerRef and route PlayerView through IdRef in event mode. PlayerView is stable in the tracker for the lifetime of a game (Player.getView() returns a stable instance, no zone-change copies), so the snapshot/fallback path was guarding a case that doesn't fire. Top-level protocol args have always trusted the tracker for PlayerView; events now do the same. Saves the embedded name on the wire. - Restore the JavaDoc on shouldReplaceTrackables that was over-trimmed, including the rationale for why applyDelta is not in the exclusion list. - Convert IdRef and EventCardRef to records. Same semantics, less boilerplate, free toString() for net log output. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../net/CompatibleObjectEncoder.java | 11 ++- .../gamemodes/net/TrackableSerializer.java | 91 +++++-------------- 2 files changed, 31 insertions(+), 71 deletions(-) diff --git a/forge-gui/src/main/java/forge/gamemodes/net/CompatibleObjectEncoder.java b/forge-gui/src/main/java/forge/gamemodes/net/CompatibleObjectEncoder.java index 62653a949dfb..9819e4432a75 100644 --- a/forge-gui/src/main/java/forge/gamemodes/net/CompatibleObjectEncoder.java +++ b/forge-gui/src/main/java/forge/gamemodes/net/CompatibleObjectEncoder.java @@ -86,7 +86,16 @@ private static void encodeInto(Serializable msg, ByteBuf out, Tracker tracker, } } - // setGameView and openView carry full state to populate the receiver's tracker. + /** + * Determines whether TrackableObject references should be replaced with + * IdRef markers for this message. Excludes only: + * - setGameView/openView: carry full state to populate the client's Tracker + * + * applyDelta is NOT excluded: its property maps already use Integer IDs + * (via DeltaSyncManager.toNetworkValue), and bundled events are wrapped + * by TrackableSerializer.wrapEvents before entering the packet, so no + * raw TrackableObjects remain in the serialization graph. + */ private static boolean shouldReplaceTrackables(Serializable msg) { if (msg instanceof GuiGameEvent event) { ProtocolMethod method = event.getMethod(); diff --git a/forge-gui/src/main/java/forge/gamemodes/net/TrackableSerializer.java b/forge-gui/src/main/java/forge/gamemodes/net/TrackableSerializer.java index c0326e032b86..0cf2c7b39a18 100644 --- a/forge-gui/src/main/java/forge/gamemodes/net/TrackableSerializer.java +++ b/forge-gui/src/main/java/forge/gamemodes/net/TrackableSerializer.java @@ -25,46 +25,14 @@ public final class TrackableSerializer { static final byte TYPE_CARD_VIEW = 0; static final byte TYPE_PLAYER_VIEW = 1; - /** Marker for tracker-stable refs (top-level protocol method args). */ - static final class IdRef implements Serializable { - private static final long serialVersionUID = 1L; - final byte typeTag; - final int id; - - IdRef(byte typeTag, int id) { - this.typeTag = typeTag; - this.id = id; - } + /** Marker for tracker-stable refs (top-level protocol method args, and PlayerView in events). */ + record IdRef(byte typeTag, int id) implements Serializable { + @Serial private static final long serialVersionUID = 1L; } /** CardView ref inside a wrapped event. {@code preserveSnapshot=true} forces fallback even if the tracker has the id. */ - static final class EventCardRef implements Serializable { - private static final long serialVersionUID = 1L; - final int id; - final String name; - final String imageKey; - final boolean preserveSnapshot; - - EventCardRef(int id, String name, String imageKey, boolean preserveSnapshot) { - this.id = id; - this.name = name; - this.imageKey = imageKey; - this.preserveSnapshot = preserveSnapshot; - } - } - - /** PlayerView ref inside a wrapped event. {@code preserveSnapshot=true} forces fallback even if the tracker has the id. */ - static final class EventPlayerRef implements Serializable { - private static final long serialVersionUID = 1L; - final int id; - final String name; - final boolean preserveSnapshot; - - EventPlayerRef(int id, String name, boolean preserveSnapshot) { - this.id = id; - this.name = name; - this.preserveSnapshot = preserveSnapshot; - } + record EventCardRef(int id, String name, String imageKey, boolean preserveSnapshot) implements Serializable { + @Serial private static final long serialVersionUID = 1L; } static byte typeTagFor(TrackableObject obj) { @@ -86,7 +54,7 @@ static Object replace(Object obj, Tracker tracker, boolean eventMode) { byte tag = typeTagFor(trackable); if (tag < 0) return obj; - if (!eventMode) { + if (!eventMode || tag == TYPE_PLAYER_VIEW) { return new IdRef(tag, trackable.getId()); } @@ -100,53 +68,36 @@ static Object replace(Object obj, Tracker tracker, boolean eventMode) { } } } - if (tag == TYPE_CARD_VIEW) { - CardView cv = (CardView) trackable; - String imgKey = cv.getCurrentState() != null - ? cv.getCurrentState().getImageKey(null) : null; - return new EventCardRef(trackable.getId(), cv.getName(), imgKey, preserveSnapshot); - } - if (tag == TYPE_PLAYER_VIEW) { - PlayerView pv = (PlayerView) trackable; - return new EventPlayerRef(trackable.getId(), pv.getName(), preserveSnapshot); - } + CardView cv = (CardView) trackable; + String imgKey = cv.getCurrentState() != null + ? cv.getCurrentState().getImageKey(null) : null; + return new EventCardRef(trackable.getId(), cv.getName(), imgKey, preserveSnapshot); } return obj; } static Object resolve(Object obj, Tracker tracker) { if (obj instanceof EventCardRef ref) { - if (!ref.preserveSnapshot) { - CardView fromTracker = tracker.getObj(TrackableTypes.CardViewType, ref.id); + if (!ref.preserveSnapshot()) { + CardView fromTracker = tracker.getObj(TrackableTypes.CardViewType, ref.id()); if (fromTracker != null) return fromTracker; } - CardView detached = new CardView(ref.id, tracker); - if (ref.name != null) { - detached.set(TrackableProperty.Name, ref.name); - detached.getCurrentState().set(TrackableProperty.Name, ref.name); - } - if (ref.imageKey != null) { - detached.getCurrentState().set(TrackableProperty.ImageKey, ref.imageKey); - } - return detached; - } - if (obj instanceof EventPlayerRef ref) { - if (!ref.preserveSnapshot) { - PlayerView fromTracker = tracker.getObj(TrackableTypes.PlayerViewType, ref.id); - if (fromTracker != null) return fromTracker; + CardView detached = new CardView(ref.id(), tracker); + if (ref.name() != null) { + detached.set(TrackableProperty.Name, ref.name()); + detached.getCurrentState().set(TrackableProperty.Name, ref.name()); } - PlayerView detached = new PlayerView(ref.id, tracker); - if (ref.name != null) { - detached.set(TrackableProperty.Name, ref.name); + if (ref.imageKey() != null) { + detached.getCurrentState().set(TrackableProperty.ImageKey, ref.imageKey()); } return detached; } if (obj instanceof IdRef ref) { - TrackableType type = trackableTypeFor(ref.typeTag); + TrackableType type = trackableTypeFor(ref.typeTag()); if (type != null) { - Object resolved = tracker.getObj(type, ref.id); + Object resolved = tracker.getObj(type, ref.id()); if (resolved == null) { - netLog.warn("Could not resolve IdRef(tag={}, id={}) from Tracker", ref.typeTag, ref.id); + netLog.warn("Could not resolve IdRef(tag={}, id={}) from Tracker", ref.typeTag(), ref.id()); } return resolved; } From 83336f92cade4ccf13371c91e76fc4a8f6700bf2 Mon Sep 17 00:00:00 2001 From: MostCromulent <201167372+MostCromulent@users.noreply.github.com> Date: Thu, 30 Apr 2026 06:28:08 +0930 Subject: [PATCH 3/3] Restore over-trimmed JavaDoc in TrackableSerializer Address review on PR #10561 (the JavaDoc-removal thread). Restores the seven JavaDoc blocks that were dropped in the original fix commit: class header, replace(), resolve(), measureSize(), WrappedEvent, wrapEvents(), unwrapEvents(). Four come back verbatim; three pick up StaleCardRef -> EventCardRef renames, and replace() adds a paragraph for the new eventMode/preserveSnapshot semantics. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../gamemodes/net/TrackableSerializer.java | 45 ++++++++++++++++++- 1 file changed, 43 insertions(+), 2 deletions(-) diff --git a/forge-gui/src/main/java/forge/gamemodes/net/TrackableSerializer.java b/forge-gui/src/main/java/forge/gamemodes/net/TrackableSerializer.java index 0cf2c7b39a18..7087500c9db9 100644 --- a/forge-gui/src/main/java/forge/gamemodes/net/TrackableSerializer.java +++ b/forge-gui/src/main/java/forge/gamemodes/net/TrackableSerializer.java @@ -18,7 +18,15 @@ import java.util.ArrayList; import java.util.List; -/** Replaces CardView/PlayerView refs with compact wire markers; resolves on the receiving side. */ +/** + * Handles serialization of {@link TrackableObject} references across the network. + * Replaces CardView/PlayerView with lightweight {@link IdRef} markers during + * encoding and resolves them back from the Tracker during decoding. + * + *

Used by the Netty encoder/decoder pipeline ({@link CompatibleObjectEncoder}, + * {@link CompatibleObjectDecoder}) and the mobile codec path + * ({@link CObjectOutputStream}, {@link CObjectInputStream}). + */ public final class TrackableSerializer { private static final TaggedLogger netLog = Logger.tag("NETWORK"); @@ -49,6 +57,15 @@ static TrackableType trackableTypeFor(byte typeTag) { } } + /** + * Replaces TrackableObject references with {@link IdRef} markers, or + * {@link EventCardRef} markers for CardViews inside wrapped events + * ({@code eventMode = true}). When the tracker holds a different object + * for the CardView's id (zone-change copy), {@code preserveSnapshot} is + * set so the receiver decodes a detached CardView from the carried name + * and image key. When {@code tracker} is null, the snapshot check is + * skipped (used by the client encoder, which has no game-state awareness). + */ static Object replace(Object obj, Tracker tracker, boolean eventMode) { if (obj instanceof TrackableObject trackable) { byte tag = typeTagFor(trackable); @@ -76,6 +93,10 @@ static Object replace(Object obj, Tracker tracker, boolean eventMode) { return obj; } + /** + * Resolves {@link IdRef} and {@link EventCardRef} markers back to + * TrackableObjects from the given Tracker. + */ static Object resolve(Object obj, Tracker tracker) { if (obj instanceof EventCardRef ref) { if (!ref.preserveSnapshot()) { @@ -105,7 +126,11 @@ static Object resolve(Object obj, Tracker tracker) { return obj; } - /** Approximate serialized size for telemetry. */ + /** + * Measures serialized size matching the encoder wire format + * for applyDelta messages with IdRef replacement (when tracker not null). + * otherwise for setGameView messages. + */ public static int measureSize(Object obj, Tracker tracker) { try { ByteArrayOutputStream baos = new ByteArrayOutputStream(); @@ -119,12 +144,23 @@ public static int measureSize(Object obj, Tracker tracker) { } } + /** + * Serializable wrapper for a GameEvent whose TrackableObject references + * have been replaced with IdRef/EventCardRef markers. Stored in + * DeltaPacket.events so events travel as compact byte arrays rather than + * full object graphs. Unwrapped after delta state is applied, when the + * client tracker is populated. + */ static final class WrappedEvent implements Serializable { private static final long serialVersionUID = 1L; final byte[] data; WrappedEvent(byte[] data) { this.data = data; } } + /** + * Wraps GameEvents by serializing each with IdRef replacement. + * Events that fail to serialize are dropped (logged). + */ public static List wrapEvents(List events, Tracker tracker) { List wrapped = new ArrayList<>(events.size()); for (GameEvent event : events) { @@ -141,6 +177,11 @@ public static List wrapEvents(List events, Tracker tracker) { return wrapped; } + /** + * Unwraps events by deserializing with IdRef resolution from the tracker. + * Called after delta state is applied so new objects are resolvable. + * Events that fail to unwrap are dropped (logged). + */ public static List unwrapEvents(List items, Tracker tracker) { List events = new ArrayList<>(items.size()); for (Object item : items) {