Fix encoder/game thread race; add ID fallback in event refs

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);
     }
 }

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);

forge-gui/src/main/java/forge/gamemodes/net/TrackableSerializer.java

@@ -33,38 +33,14 @@ public final class TrackableSerializer {
     static final byte TYPE_CARD_VIEW = 0;
     static final byte TYPE_PLAYER_VIEW = 1;
 
-    /**
-     * Lightweight serializable marker that replaces a TrackableObject reference.
-     */
-    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;
     }
 
-    /**
-     * 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 {
-        private static final long serialVersionUID = 1L;
-        final int id;
-        final String imageKey;
-        final String name;
-
-        StaleCardRef(int id, String imageKey, String name) {
-            this.id = id;
-            this.imageKey = imageKey;
-            this.name = name;
-        }
+    /** CardView ref inside a wrapped event. {@code preserveSnapshot=true} forces fallback even if the tracker has the id. */
+    record EventCardRef(int id, String name, String imageKey, boolean preserveSnapshot) implements Serializable {
+        @Serial private static final long serialVersionUID = 1L;
     }
 
     static byte typeTagFor(TrackableObject obj) {
@@ -83,59 +59,66 @@ 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).
+     * {@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) {
+    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 || tag == TYPE_PLAYER_VIEW) {
+                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());
             }
+            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;
     }
 
     /**
-     * Resolves {@link IdRef} and {@link StaleCardRef} markers back to
+     * Resolves {@link IdRef} and {@link EventCardRef} 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.
-            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 (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());
+                detached.getCurrentState().set(TrackableProperty.Name, ref.name());
             }
-            if (ref.imageKey != null) {
-                detached.getCurrentState().set(TrackableProperty.ImageKey, ref.imageKey);
+            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;
             }
@@ -152,7 +135,7 @@ 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();
@@ -163,7 +146,7 @@ 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
+     * 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.
@@ -183,7 +166,7 @@ public static List<Object> wrapEvents(List<GameEvent> events, Tracker tracker) {
         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()));
@@ -219,16 +202,18 @@ public static List<GameEvent> unwrapEvents(List<Object> 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);
         }
     }
 

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);
     }
 
     /**

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;
@@ -57,11 +57,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();
@@ -79,15 +99,33 @@ 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
     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();