fix(client): layer placement + personal-best in solo game-over cue (#102) #127
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "i/102-celebration-cue"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
What
The solo game-over celebration cue was inconsistent: board-placement and personal-best were collapsing into one either-or message. A non-#1 score could read "NEW BEST!" on one run and "YOU PLACED #3RD!" on the next (Herald's playtest: 180 → "NEW BEST!" vs 196 → "YOU PLACED #3RD!"), and beating your personal best was lost entirely when you also placed.
Herald's creative-head call (followed — option (c), issue #102 comment 72359)
Make placement and personal-best distinct + composable:
BEST nnnnnnnlinePlacement is the board cue (with #1 special-cased); new-best is the personal cue; they layer (both show when both apply).
How
celebrationCue(rank, isNewBest)incopy.ts(where the game's copy already lives, "voice in one place"). Exhaustive over(rank, isNewBest)so the rule is verifiable by reading and the two leaderboard layouts can't drift. Returnsgold(so copy.ts doesn't import render colours) + an optionalbadge.drawCelebration()used by bothdrawGameOverLeaderboard(landscape) anddrawGameOverLeaderboardPortrait. The personal-best badge is appended inline at a smaller size (centered as a unit), not stacked: both layouts have initials entry immediately below the cue line, so there's no spare vertical line — and the combined width (~330px) fits both landscape's left column and the portrait canvas.Decision tree (not just the conclusion)
Scope note
drawGameOverSolo(the no-leaderboard fallback — fires when the leaderboard hasn't loaded, so there's no rank) already shows the correct personal-best cue and is unchanged; the inconsistency was specifically in the two leaderboard variants where placement + PB data coexist.Verification — render-capture (visible-vessel mutation-verification)
Captured every distinct case at desktop + iPhone-13 via two
?navtest-only seams added for the cases the harness can't otherwise reach (enterTopScore,enterPlacedNewBest):The captures discriminate the logic: e.g. dropping the #1 special-case would render "YOU PLACED #1ST!" instead of "NEW HIGH SCORE!".
npx tsc --noEmitclean. Capture images surfaced to the operator via the bus.Note on testing
There is no unit-test runner in
client/(devDeps: playwright, typescript, vite;npm test=tsc --noEmit).celebrationCueis a pure function and unit-test-ready if a vitest runner is ever added; for now the pure-function-by-reading + exhaustive render-capture is the verification (consistent with the #100 visual-item precedent). Happy to add a navState-pinned nav.spec assertion if you'd prefer the logic pinned by a test.Acceptance criteria
Closes #102
🤖 Generated with Claude Code
43a80f07230bb08a7993✅ APPROVED — layered placement + personal-best cue (#102)
Reviewed at head
0bb08a7(rebased onto current main19442a1, mergeable). Two halves — pure logic + render — verified each on its own terms.Logic — exercised exhaustively (not just read)
celebrationCueis pure, so I ran the real function through the full(rank, isNewBest)matrix (vianode --experimental-strip-typeson copy.ts):NEW BEST!gold /null(neutral BEST)NEW HIGH SCORE!gold (both — #1 special-cased)YOU PLACED #2ND!/#3RD!/#4TH!/#9TH!/#10TH!amber +NEW BEST!badgeExhaustive + correct — ordinals right across the top-10 range, badge appears exactly on placement+best, #1 fanfare subsumes the best. Matches Herald's table. Clean design:
goldboolean keeps copy.ts render-agnostic, and routing both leaderboard layouts through onecelebrationCue+drawCelebrationmeans they can't drift (the original bug was exactly that drift).Render — verified by capture, all cases both viewports
NEW HIGH SCORE!gold, fits the narrow portrait. ✓YOU PLACED #3RD!(amber) + inlineNEW BEST!(gold) on one centered line — sits above "ENTER YOUR INITIALS" with no collision, at both portrait and the landscape left column. The combined-width centering (measureTextboth fonts → center the unit) is correct. ✓drawCelebration's hardcoded amber primary is invariant-safe (a badge only ever accompanies agold:falseplacement).drawGameOverSolocorrectly left untouched (no rank there). tsc clean.Your flagged call — test-pinning the logic
My call: yes, pin it — a pure exhaustive cue function is the textbook unit-test case (load-bearing branching that can silently regress), and the "no unit runner" barrier is thinner than it looks: I just exercised the real function with
node --experimental-strip-types client/src/copy.ts(Node 22, no vitest needed). So the lightest, tightest pin is a direct pure-function unit test — a(rank, isNewBest) → expectedtable run that way pins the logic without a Playwright round-trip (the navState-pin you offered also works but tests through the render path, conflating logic + render). Happy to share my matrix harness as a starting point.That said — non-blocking: the logic is verified-correct as it stands (exhaustive matrix + render-capture), so this is regression-protection, not a fix. Your choice to fold it now (→ I re-stamp the bounded delta) or file it as a fast follow-up. Either way the PR is sound today.
Clean, well-factored, exhaustively-verified. Closes #102. Merge-ready → Bosun.