feat(client): turn replay viewer + targeted component tests - #2253
feat(client): turn replay viewer + targeted component tests#2253claytonlin1110 wants to merge 4 commits into
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a turn-replay feature for single-player modes, allowing players to scrub through historical turn checkpoints while blocking game actions. It adds the ReplayControls UI component, integrates it into the main game page, updates translation files, and implements state management in gameStore along with extensive unit tests. The review feedback identifies critical issues where live legal actions and spell costs are not preserved upon entering replay mode, which would permanently lock players out of the live game upon exiting. It also points out that the live turn number in the replay banner incorrectly reads from the active historical checkpoint instead of the saved live game state.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| replayMode: boolean; | ||
| /** Index into turnCheckpoints currently being previewed. Null when not in replay mode. */ | ||
| replayIndex: number | null; | ||
| /** Live engine state saved before entering replay; restored on exit. */ | ||
| liveGameState: GameState | null; |
There was a problem hiding this comment.
[CRITICAL] Add store fields to preserve live legal actions and spell costs during replay mode.
Why it matters: These fields are required to stash the live game's interactive state so it can be fully restored when exiting replay mode.
replayMode: boolean;
/** Index into turnCheckpoints currently being previewed. Null when not in replay mode. */
replayIndex: number | null;
/** Live engine state saved before entering replay; restored on exit. */
liveGameState: GameState | null;
liveLegalActions: GameAction[];
liveLegalActionsByObject: Record<string, GameAction[]>;
liveSpellCosts: Record<string, ManaCost>;| replayMode: false, | ||
| replayIndex: null, | ||
| liveGameState: null, |
There was a problem hiding this comment.
[CRITICAL] Initialize the live state preservation fields in the initial store state.
Why it matters: Ensures the new fields are properly initialized when the store is created.
| replayMode: false, | |
| replayIndex: null, | |
| liveGameState: null, | |
| replayMode: false, | |
| replayIndex: null, | |
| liveGameState: null, | |
| liveLegalActions: [], | |
| liveLegalActionsByObject: {}, | |
| liveSpellCosts: {}, |
| enterReplay: (index) => { | ||
| const { turnCheckpoints, gameState, replayMode } = get(); | ||
| if (replayMode || turnCheckpoints.length === 0 || !gameState) return; | ||
| const targetIndex = index ?? turnCheckpoints.length - 1; | ||
| const clamped = Math.max(0, Math.min(targetIndex, turnCheckpoints.length - 1)); | ||
| set({ | ||
| replayMode: true, | ||
| replayIndex: clamped, | ||
| liveGameState: gameState, | ||
| gameState: turnCheckpoints[clamped], | ||
| waitingFor: turnCheckpoints[clamped].waiting_for, | ||
| legalActions: [], | ||
| legalActionsByObject: {}, | ||
| }); | ||
| }, | ||
|
|
||
| exitReplay: () => { | ||
| const { replayMode, liveGameState } = get(); | ||
| if (!replayMode || !liveGameState) return; | ||
| set({ | ||
| replayMode: false, | ||
| replayIndex: null, | ||
| liveGameState: null, | ||
| gameState: liveGameState, | ||
| waitingFor: liveGameState.waiting_for, | ||
| }); | ||
| }, |
There was a problem hiding this comment.
[CRITICAL] Stash live legal actions and spell costs on replay entry, and restore them on exit.
Why it matters: Restoring only the gameState and waitingFor leaves the player with empty legal actions and incorrect spell costs, permanently locking them out of playing the live game after exiting replay mode.
enterReplay: (index) => {
const { turnCheckpoints, gameState, replayMode, legalActions, legalActionsByObject, spellCosts } = get();
if (replayMode || turnCheckpoints.length === 0 || !gameState) return;
const targetIndex = index ?? turnCheckpoints.length - 1;
const clamped = Math.max(0, Math.min(targetIndex, turnCheckpoints.length - 1));
set({
replayMode: true,
replayIndex: clamped,
liveGameState: gameState,
liveLegalActions: legalActions,
liveLegalActionsByObject: legalActionsByObject,
liveSpellCosts: spellCosts,
gameState: turnCheckpoints[clamped],
waitingFor: turnCheckpoints[clamped].waiting_for,
legalActions: [],
legalActionsByObject: {},
spellCosts: {},
});
},
exitReplay: () => {
const { replayMode, liveGameState, liveLegalActions, liveLegalActionsByObject, liveSpellCosts } = get();
if (!replayMode || !liveGameState) return;
set({
replayMode: false,
replayIndex: null,
liveGameState: null,
liveLegalActions: [],
liveLegalActionsByObject: {},
liveSpellCosts: {},
gameState: liveGameState,
waitingFor: liveGameState.waiting_for,
legalActions: liveLegalActions,
legalActionsByObject: liveLegalActionsByObject,
spellCosts: liveSpellCosts,
});
},| const turnCheckpoints = useGameStore((s) => s.turnCheckpoints); | ||
| const replayMode = useGameStore((s) => s.replayMode); | ||
| const replayIndex = useGameStore((s) => s.replayIndex); | ||
| const currentTurn = useGameStore((s) => s.gameState?.turn_number ?? null); |
There was a problem hiding this comment.
[HIGH] Incorrect live turn number in replay banner.
Why it matters: Reading turn_number from gameState during replay mode returns the historical checkpoint's turn, resulting in a misleading live turn display (e.g., showing (live: T1) instead of the actual live turn).
Suggested fix: Use liveGameState to retrieve the live turn number when replayMode is active.
| const currentTurn = useGameStore((s) => s.gameState?.turn_number ?? null); | |
| const currentTurn = useGameStore((s) => (s.replayMode ? s.liveGameState : s.gameState)?.turn_number ?? null); |
🤖 Architecture Review (automated)Verdict: Seam: PASS — Frontend-only; replay state lives in
Findings
|
|
@claytonlin1110 Hi, and thanks for the contribution! I really appreciate the effort here, and the test coverage is genuinely nice to see. That said, I'm going to pass on this one. New player-facing UI features like this are something I'd prefer to design and drive myself, since they carry a lot of UX and product decisions that I want to keep a tight grip on, and that's hard to delegate through a PR after the fact. Where contributions are most valuable right now is the engine and parser work. Those issues are well-scoped, easier to review against a clear correctness bar, and there's a huge amount still open, so that's where I'd love to point your energy. If you grab one of those, I'm very happy to work through it with you. Closing this for now, but thank you again, genuinely. |
Summary
turnCheckpointsvia a floating bottom-center control bar.While replaying, the board shows the historical state and all
game dispatches are blocked — fully read-only.
or WASM engine.
and three previously-untested components.
Test plan
pnpm vitest run— 59/59 green