Skip to content

fix(client): count opponents correctly when the local seat is eliminated - #5187

Open
jeffrey701 wants to merge 2 commits into
phase-rs:mainfrom
jeffrey701:fix/resumable-opponent-count
Open

fix(client): count opponents correctly when the local seat is eliminated#5187
jeffrey701 wants to merge 2 commits into
phase-rs:mainfrom
jeffrey701:fix/resumable-opponent-count

Conversation

@jeffrey701

Copy link
Copy Markdown
Contributor

Problem

useResumables (client/src/hooks/useResumables.ts) builds a resumable-match summary and computes:

const liveCount = state.players.filter((p) => !p.is_eliminated).length;
// …
opponentCount: Math.max(0, liveCount - 1),

liveCount already excludes eliminated players, and the - 1 removes the local human (seat 0). But that assumes seat 0 is always alive. In a still-resumable multi-seat / FFA match where seat 0 has been eliminated, seat 0 isn't part of liveCount, so subtracting 1 undercounts the opponents by one. Example — players = [{id:0,eliminated}, {id:1,live}, {id:2,live}] → reports 1 opponent, should be 2.

Fix

Extract the counting into a pure, unit-tested countLiveOpponents helper (client/src/hooks/liveOpponents.ts) that only subtracts the local seat when it is itself alive, and call it from the hook. (Extracting the logic is what makes it testable without standing up the whole async hook — matching how the repo keeps other pure logic, e.g. gridBandMath, in dedicated tested modules.)

Tests

Adds liveOpponents.test.ts: seat 0 alive → live-others count; seat 0 eliminated → not decremented (the old formula returned 1, the fix returns 2 — discriminating); eliminated opponents excluded; degenerate all-eliminated / lone-survivor cases → 0.

Self-contained: only useResumables.ts, a new pure helper, and its test; no engine/Rust/protocol changes.

Model: claude-opus-4-8

@jeffrey701
jeffrey701 requested a review from matthewevans as a code owner July 6, 2026 05:30
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

The resumable-match summary computed opponentCount as Math.max(0, liveCount - 1), where liveCount already excludes eliminated players. The -1 assumes the local human (seat 0) is always alive; if seat 0 is eliminated but the match is still resumable (multi-seat local/FFA), seat 0 isn't in liveCount, so subtracting 1 undercounts opponents. Extract the pure counting into countLiveOpponents (unit-tested) and only subtract the local seat when it is alive.
@jeffrey701
jeffrey701 force-pushed the fix/resumable-opponent-count branch from 456c7c5 to 1af899c Compare July 6, 2026 07:53
@matthewevans matthewevans added the defer-fe Frontend/client/UI PR deferred to Matt's direct review label Jul 6, 2026
Resolve the maintainer-caused persisted public-state refactor while preserving the contributor’s local-seat elimination calculation.\n\nCo-authored-by: Jeff <jeffreyconradtucker@gmail.com>
@matthewevans matthewevans removed their assignment Jul 18, 2026
@matthewevans

Copy link
Copy Markdown
Member

Deferred by maintainer intake policy — not ignored.

This current head (e318893388f2099a16363b5d41fccbd9e9d88a25) was triaged as a frontend-only change (client/src/hooks/__tests__/liveOpponents.test.ts, client/src/hooks/liveOpponents.ts, client/src/hooks/useResumables.ts) by jeffrey701. The local frontend-review allowlist does not include this author, so this route does not perform an implementation-diff review or approve the PR.

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.

2 similar comments
@matthewevans

Copy link
Copy Markdown
Member

Deferred by maintainer intake policy — not ignored.

This current head (e318893388f2099a16363b5d41fccbd9e9d88a25) was triaged as a frontend-only change (client/src/hooks/__tests__/liveOpponents.test.ts, client/src/hooks/liveOpponents.ts, client/src/hooks/useResumables.ts) by jeffrey701. The local frontend-review allowlist does not include this author, so this route does not perform an implementation-diff review or approve the PR.

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

Copy link
Copy Markdown
Member

Deferred by maintainer intake policy — not ignored.

This current head (e318893388f2099a16363b5d41fccbd9e9d88a25) was triaged as a frontend-only change (client/src/hooks/__tests__/liveOpponents.test.ts, client/src/hooks/liveOpponents.ts, client/src/hooks/useResumables.ts) by jeffrey701. The local frontend-review allowlist does not include this author, so this route does not perform an implementation-diff review or approve the PR.

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 matthewevans self-assigned this Aug 1, 2026
@matthewevans matthewevans added the bug Bug fix label Aug 1, 2026
@matthewevans matthewevans removed their assignment Aug 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Bug fix defer-fe Frontend/client/UI PR deferred to Matt's direct review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants