Fix CardView IdRef substitution / retire UI_NETPLAY_COMPAT

[MostCromulent]

Summary

Fixes a CardView IdRef substitution bug that #10644 partially fixed, and retires the UI_NETPLAY_COMPAT toggle. The host's tracker isn't a reliable proxy for what the client knows about — e.g. Jhoira's random choice copies are in the host tracker as a side effect of buildAbilities but never broadcast — so IdRef substitution now gates on per-client DeltaSyncManager consumer registration instead.

Background

UI_NETPLAY_COMPAT toggled two outer-codec serialization paths: the default ("OFF") path used CObjectInputStream/OutputStream with thin class descriptors (sender writes the class name only, receiver looks up its own); the opt-in ("ON") path used standard ObjectStreamClass descriptors via TrackableSerializer's ReplacingOutputStream/ResolvingInputStream. It shipped in April 2020 as a fallback safety valve and has persisted for six years.

Scope error in #10644

Tracing how the two paths actually differed surfaced an inadvertent scope error in #10644 ("Serialize CardViews missing from host tracker").

Pre-#10644, both paths used a blanket "always IdRef" rule for CardView refs in non-event mode: tracked cards resolved fine, ephemerals didn't (the bug). #10644 replaced it with a tracker-aware rule — IdRef if the tracker has the id, fall through to inline otherwise — and added encoder-side setTracker wiring.

Both changes were meant universally, but the new logic was inadvertently consumed by the ON path's ReplacingOutputStream alone. CObjectOutputStream.replaceObject on the (default) OFF path kept hardcoding TrackableSerializer.replace(obj, null, false), so the encoder's tracker landed in a field the stream never read. With null tracker plus the new fall-through, every CardView on the default path serialized inline — ephemerals safely, but tracked CardViews lost the IdRef compression they previously had. This is the maintenance cost of carrying two parallel codec paths: a universal fix quietly covered only one, and the gap went unnoticed because the orphaned path was the default.

Investigation: ephemerals broke when we threaded the tracker

Threading gameView.getTracker() through CObjectOutputStream so the surviving codec applied the same substitution as the ON path produced an NPE in ListChooser and three Could not resolve IdRef warnings under manual test. The repro: Jhoira of the Ghitu Avatar's "copy three random sorceries, pick one" ability. Three CardViews the server had in its tracker were not in the client's.

Diagnostic logging on Tracker.putObj traced each Jhoira choice copy's registration back through:

TrackableTypes$TrackableObjectType.updateObjLookup
  <- TrackableProperty.updateObjLookup
  <- TrackableObject.set
  <- SpellAbilityView.updateHostCard

Card.fromPaperCard constructs the Card and immediately runs buildAbilities, which creates SpellAbilityView instances. Each SpellAbilityView.updateHostCard does set(HostCard, cardView), and the property-set cascade registers the CardView in the tracker's CardViewType lookup as a side effect.

So the server's tracker contains everything the server happens to know about, including transient CardViews created mid-resolution. The client's tracker contains what was broadcast via setGameView or applyDelta(newObjects). These are not the same set.

Solution: gate on per-client consumer registration

The host's tracker isn't a safe indicator of what the client knows — what is?

DeltaSyncManager already maintains a per-client consumerId. When it walks the GameView graph in walkAndRegister or registers a new/replacement object in collectDeltas, it calls obj.registerConsumer(consumerId) — the same registration that drives inclusion in the delta packet's newObjects map. So consumer presence is a faithful predicate for "the client has been told the properties of this CardView."

TrackableSerializer.replace now accepts a consumerId. When ≥ 0 (server-side encoders) the CardView gate uses trackable.hasConsumer(consumerId); when < 0 (client-side encoders) it falls back to tracker presence — sound there because the client's tracker is what it knows.

Wiring:

• TrackableObject exposes hasConsumer(int).
• DeltaSyncManager exposes getConsumerId().
• CObjectOutputStream and CompatibleObjectEncoder plumb the id through.
• RemoteClient.setCodecTracker(Tracker, int) wires both onto the pipeline (reapplied on swapChannel).
• RemoteClientGuiGame passes the id at first setGameView.

For Jhoira's choices the copies are never reachable from the GameView graph, so walkAndRegister never visits them, the consumer is never registered, hasConsumer returns false, and replace falls through to inline. Cards in real zones still IdRef. PlayerView IdRef stays unconditional — PlayerViews are stable from openView.

Why the UI_NETPLAY_COMPAT toggle was redundant

With the IdRef bug resolved, the rest of the PR retires UI_NETPLAY_COMPAT itself. The toggle is safe to remove because:

• It has defaulted off on every platform since it shipped, so production traffic and integration testing only ever exercise the OFF path. The ON path is formally maintained in lockstep when both branches get touched, but divergences from the default behavior can go unnoticed — Serialize CardViews missing from host tracker #10644's scope error is one example.
• Load-bearing reference compression — the bulk of netplay traffic — runs at a separate inner layer: DeltaSyncManager.toNetworkValue converts CardView/PlayerView refs in property maps to Integer IDs, and TrackableSerializer.wrapEvents wraps bundled events with IdRef/EventCardRef markers. Both layers run regardless of the pref, and both outer paths carry the same opaque bytes for those packets.
• A pref-mismatch between server and client could corrupt the stream on the first class descriptor — one side writes a one-byte type tag where the other expects a full ObjectStreamClass block. Removing the toggle removes the ability to fall into that state.

Other cleanup bundled

• Inline the surviving outer-codec path; CompatibleObjectEncoder/Decoder no longer branch on GuiBase.hasPropertyConfig.
• Run ProtocolMethod.checkArgs unconditionally; it only logs, mirrors the always-on checkReturnValue, and replaces a generic Method.invoke "argument type mismatch" with a structured log line.
• Codec documentation: class-level Javadocs on the four codec classes; named log thresholds; magic Netty constructor numbers commented; var5 → result, bout/oout → byteOut/objectOut; dead frameStart removed.
• Drop FPref.UI_NETPLAY_COMPAT, GuiBase.propertyConfig/hasPropertyConfig/enablePropertyConfig, startup wiring in FControl and Forge.getApp, the desktop and mobile settings UI, the mobile launcher's propertyConfig argument, and lblExperimentalNetworkCompatibility/nlExperimentalNetworkCompatibility across nine language files.

---

🤖 Generated with Claude Code

[MostCromulent]

TLDR: Netplay is currently working and ephemerals are resolving, but that's because all CardView protocol args are inadvertently being serialized inline rather than using IdRef substitution - the server tracker method attempted in #10644 doesn't work properly but this proposes a different solution that seems to.

While it's annoying this was missed, I note it was fairly difficult for the AI to find - took it a few goes to work out where #10644 had gone wrong while investigating UI_NETPLAY_COMPAT even with max effort. The two different codec paths seem to throw it pretty badly because they branch in non-obvious ways.

[tool4ever]

Sigh, my love/hate relationship with these complex PR doesn't get a break 😜
But still good your consolidation initiative uncovered the regression this quickly 👍

tbh understandable AI missed this first, the history how this was implemented is pretty messy as far as I can tell:

• initially not even an option it was meant to provide a more performant encoder
• then later it got inversed so now enabling it was the original implementation for potentially still supporting older Android SDK (no longer relevant for Forge at this point)
• the weird names for the extra methods controlled by the preference even trace back to log4j 1

I think this has the side effect that if we need to test with useDeltaSync disabled in the future all IdRef replacements are skipped too - which I'm not unhappy about as bundling these behaviours is pretty much part of the same concept anyway...

[tool4ever]

I'll probably merge this soon after I finish testing, so can you check for a follow-up if there's still a point to keep these classes instead of simply reusing the CObject* ones?
and as bonus the Android workaround for records would then no longer be needed, right?