fix(client): resolve the local seat from the mode census (#7524) - #7525
fix(client): resolve the local seat from the mode census (#7524)#7525cuinhellcat wants to merge 1 commit into
Conversation
`usePlayerId()`/`getPlayerId()` are the single authority for "which seat is this client's own" — 50 production call sites across 36 files read one of them. Both resolved it from a hand-typed list of game modes that predates `"draft-match"`, so a pod-draft guest was answered seat 0 no matter what the pod had assigned it. Its own hand came back redacted, `GamePage`'s mulligan `pending.find` searched the host's seat, and once the host kept, both clients sat on "Opponent is deciding their opening hand…" with nobody able to answer for seat 1. `activePlayerId` was already correct (`setupDraftMatchAvatars`); only the resolver discarded it. Give `GAME_MODE_TRAITS` a third axis — `seat: "seat-zero" | "wire-assigned" | "no-seat"` — and read it through the new `seatSource()`. The census is a `Record<GameMode, …>`, so a mode added later cannot compile without declaring where its seat comes from; that is what the two lists could not do. Both entry points now share one `resolveLocalSeat`, with the spectator split passed in as a parameter so it stays a stated decision rather than a drift. `native-ai` is declared `"seat-zero"`, measured rather than assumed: `handleNativeEvent` (`GameProvider.tsx`) never calls `setActivePlayerId`, so reading the wire there would read a stale value from an earlier online game. Counter-probe: flipping only the `draft-match` census entry back to `"seat-zero"` drops both new rows — `expected +0 to be 1`. Does not cover: the pod-draft guest's seat still reaches the store via `setupDraftMatchAvatars`, which derives it from `matchPairing.type` rather than from the `assignedPlayerId` the host actually sent in `game_setup`. The two agree today; unifying them on the wire value is a separate change. Not evidenced by the new tests: they do not render `GamePage`, so they do not show the mulligan modal appearing for the guest. They pin the seat authority that modal reads. The solo-mode and spectator rows are green with and without the fix and are labelled in the file as nail-downs, not evidence. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
Included review availability: Your plan includes up to 2 reviews per rolling hour; 1 remains after this review. 📝 WalkthroughWalkthroughThe change adds seat-source metadata to game modes and uses it to resolve local player IDs. The getter and hook retain distinct spectator behavior. Tests cover pod-draft, multiplayer, solo, and spectator modes. ChangesSeat resolution
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to The change centralizes local-seat resolution and adds coverage for draft-match behavior; no actionable merge-blocking risk remains after normal checks and review. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Deferred by maintainer intake policy — not ignored. This current head ( A maintainer must explicitly take this PR or add a local frontend-review exception before it can receive substantive review. The defer label is a routing marker only, not a verdict on the change. |
matthewevans
added
the
defer-fe
2 participants
Fixes #7524.
A pod-draft guest was answered seat 0 by
usePlayerId()/getPlayerId()no matter which seat the pod had assigned it, because both resolved the answer from a hand-typed mode list written before"draft-match"existed. Its own hand rendered redacted,GamePage's mulliganpending.findsearched the host's seat, and once the host kept, both clients sat on "Opponent is deciding their opening hand…" with nobody able to answer for seat 1.The change
GAME_MODE_TRAITSgains a third axis:seatSource()reads it, andresolveLocalSeat()is the single resolver both entry points now share. The census is aRecord<GameMode, …>, so a mode added later cannot compile without declaring where its seat comes from — which is exactly what the two lists could not do, and why this bug existed.ai,local,native-aiseat-zeroonline,p2p-host,p2p-join,draft-matchwire-assignedspectateno-seatnative-aiis measured, not assumed:handleNativeEvent(GameProvider.tsx) never callssetActivePlayerId, so declaring itwire-assignedwould read a stale seat left by an earlier online game.The spectator split — 255 from
getPlayerId(), 0 fromusePlayerId()— is unchanged, but is now a named parameter rather than a difference between two copies.HudBadges.tsxdocuments its dependency on the hook's 0.Measurements
tsc -b --noEmit/eslint .vitest runCounter-probe: flipping only the
draft-matchcensus entry back to"seat-zero"drops both new rows —expected +0 to be 1.useConcedeHandler.test.tsxmocked the wholegameStoremodule, which left the new import undefined. It nowimportActuals the module and overrides only the store handle, so it concedes as the seat the real census resolves rather than as one the mock asserts.Not covered
The guest's seat still reaches the store via
setupDraftMatchAvatars, which derives it frommatchPairing.typerather than from theassignedPlayerIdthe host actually sent ingame_setup. The two agree today; unifying them on the wire value is a separate change.Not evidenced by the new tests
They do not render
GamePage, so they do not show the mulligan modal appearing for the guest — they pin the seat authority that modal reads. The solo-mode and spectator rows are green with and without the fix and are labelled in the file as nail-downs, not evidence.Summary by CodeRabbit
Bug Fixes
Tests