Extend IdRef serialization from game events to protocol method args#10343
Open
MostCromulent wants to merge 8 commits intoCard-Forge:masterfrom
Open
Extend IdRef serialization from game events to protocol method args#10343MostCromulent wants to merge 8 commits intoCard-Forge:masterfrom
MostCromulent wants to merge 8 commits intoCard-Forge:masterfrom
Conversation
RafaelHGOliveira
added a commit
to RafaelHGOliveira/forge
that referenced
this pull request
Apr 10, 2026
…l method args Replaces full CardView/PlayerView object graphs in protocol method args with lightweight IdRef(typeTag, id) markers resolved from the local Tracker. Consolidates TrackableObject serialization into TrackableSerializer; removes GameEventProxy (absorbed into TrackableSerializer). Also fixes pre-existing PlayerControllerHuman call to updateHasAvailableActions using removed int signature; updated to Predicate<SpellAbility> form.
Protocol methods were serializing the entire CardView/PlayerView object graph for each argument. Add IdRef replacement at the Netty encoder/decoder level: the server encoder swaps CardView/PlayerView with lightweight IdRef(typeTag, id) markers, and the client decoder resolves them from the Tracker. setGameView and openView are excluded (they carry the full state). Extract shared ID-mapping primitives (IdRef, type tags, typeTagFor, trackableTypeFor) from GameEventProxy into TrackableRef so both the encoder/decoder path and GameEventProxy can reuse them. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Extends encoder replacement (previously server→client only) to the client→server path. The client encoder now replaces CardView/PlayerView with lightweight IdRef markers in protocol method args like selectCard, and the server decoder resolves them from the tracker. Testing revealed that after a zone change (e.g. playing a land from hand), the server tracker holds a stale CardView with the old zone, causing Game.findByView to search the wrong zone and return null. Added a fallback global search in findByView when the zone-specific search misses, rather than modifying tracker update behavior which has wide blast radius. Also removes GameEventProxy — raw GameEvent objects travel inside DeltaPacket, with replacement handled by the encoder/decoder. Confirmed by batch testing (0 dropped events). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…coder Events embedded in DeltaPacket are excluded from encoder IdRef replacement (to protect state data), so their TrackableObject references arrive as raw deserialized instances with null trackers. When event handlers call TrackableTypes.lookup(), the null tracker causes NPE. Added a fallback in TrackableSerializer.resolve() that detects raw TrackableObjects with no tracker and resolves them to the canonical tracker instance, matching what GameEventProxy.unwrapAll previously did. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Events bundled in DeltaPacket were excluded from encoder-level IdRef replacement to avoid a decoder timing issue: the Netty decoder resolves IdRefs during deserialization, before applyDelta creates new objects in the client tracker. This caused events to travel as raw TrackableObjects, losing StaleCardRef detection and inflating wire size. Restore the wrap/unwrap pattern from the removed GameEventProxy, now in TrackableSerializer: events are serialized to byte arrays with IdRef replacement at DeltaPacket construction time (server), and deserialized with resolution after state application (client), when the tracker is fully populated. This gives events proper IdRef/StaleCardRef handling without the decoder timing problem. - Remove applyDelta exclusion from shouldReplaceTrackables - Remove raw-TrackableObject fallback from TrackableSerializer.resolve - Add wrapEvents/unwrapEvents to TrackableSerializer - Wrap events in both delta+events and events-only paths Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…verload Address PR #17 review feedback from tool4ever: - Delete single-arg TrackableSerializer.replace(Object) — verified byte-equivalent to replace(obj, null) (the tracker null-check is the only difference, and skipping it falls through to the same IdRef return). Update CObjectOutputStream and TrackableSerializer's own ReplacingOutputStream to use the unified two-arg form. - Delete CompatibleObjectEncoder.ReplacingObjectOutputStream and CompatibleObjectDecoder.ResolvingObjectInputStream — exact duplicates of TrackableSerializer.ReplacingOutputStream/ResolvingInputStream modulo class qualifier. Make those inner classes package-private and reuse them directly from the encoder/decoder.
d751aba to
d4771ab
Compare
Contributor
|
Planning to merge in around two weeks if nothing else comes up |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
@tool4ever re-targeting this at master now that delta is merged.
Previous PR discussion should be preserved here: MostCromulent#17
Summary
Protocol methods serialize CardView/PlayerView references as full object graphs — every
selectCard,selectPlayer, and similar call sends the entire trackable object tree. This replaces that with lightweight IdRef(typeTag, id) markers at the Netty encoder/decoder level, and consolidates all TrackableObject serialization logic into a singleTrackableSerializerclass.wrapEvents/unwrapEventsGame.java— when a zone-specific search misses (e.g. IdRef resolved from a tracker that wasn't updated after a zone change), falls back to global ID searchsetGameViewandopenVieware excluded from replacement since they carry the full state that populates the client's Tracker.Design decisions
Fallback search vs tracker update (findByView): When the server tracker holds a stale CardView with an old zone (because
updateObjLookupskips objects whose ID is already registered),findByViewsearches the wrong zone. Rather than modifying the tracker's!hasObjguard (shared by delta sync and view updates, high blast radius), a fallback global search is added — it only triggers on a miss, matching the existing null-zone fallback path.Event wrap/unwrap timing: Events in DeltaPacket can't use encoder-level replacement because the decoder runs before
applyDeltacreates new objects in the tracker. Instead, events are serialized to byte arrays with IdRef replacement at packet construction time (server-side), and deserialized with resolution after state application (client-side), when the tracker is fully populated.Server vs client encoder modes: The server encoder uses a full Tracker (set by
RemoteClientGuiGame.setGameView) for verified replacement with StaleCardRef detection. The client encoder does simple IdRef replacement without a tracker — using one would create StaleCardRef markers that the server resolves as detached CardViews, breaking game object identity.🤖 Generated with Claude Code