Four things the game counted and never said, and two it said wrong
Stays in the unshipped v0.7.9. Prompted by Jesse asking the general question after two v0.7.9 fixes turned out to be the same shape: actingPlayer existed and the Frame threw it away, and collisionsToday / collisionsTotal rode the Frame for three releases with nothing drawing them. So what else is computed, serialised and sent to nobody? THE AUDIT, done rather than guessed. Every one of Frame's 59 top-level fields grepped for a read across the seven renderers, then the same for Tally's 26 members. 55 of 59 are read. Four are not. tally.unloadsBegun was visible rather than merely unused. §9.1 makes loading and unloading the same shape — begun, then carried through — and the results screen printed "Loads still in the pipeline" for one side and nothing for the other, reporting half of a symmetric mechanism. tally.cardsDiscarded was counted by the engine and listed beside "Cards drawn" and "Cards played" without it, though Gitea#9 made throwing a Timetabled train away a deliberate move — a player CHOICE the game counted and never reported. Both are reported now. viewerSeat and overHandLimit are deferred by Jesse. The second is the fullest version of the shape: engine computes it, view.ts puts it on the Frame, session.ts declares it on the Session interface AND implements it twice, and the only caller in the repo is its own test. Four layers of plumbing, no consumer. The decision when it comes is delete-or-document, not a patch. Fixing the two turned up a third thing: resultsHtml draws tallyHtml(report?.tally ?? f.tally), and report is f.official, so a finished game reports the tally frozen at the official ending rather than the live one. The first attempt at a test overrode f.tally alone, changed nothing on screen, and failed for a reason unrelated to the fix. A SHOUTED KEYWORD IS NOT A SENTENCE. `EXTRA X18 started…` attributed to a player rendered as `Player Solitaire eXTRA X18 started…`, and the same happened to TRAIN 1 MADE UP and COLLISION. `record` folds a narration's opening word into the middle of a sentence and did it with a flat charAt(0).toLowerCase(). It now folds only a sentence-cased word — ^[A-Z][a-z], a capital followed by a lower-case letter — which also leaves X22 Pee-Dee alone, where a naive uppercase test gets it wrong because '2'.toUpperCase() is '2'. It had been filed under Play Balance, where it has no business being, which is how it survived a session that had ruled balance work out of scope. A REPLAYED SAVE NOW NARRATES WHAT THE LIVE GAME NARRATED. fromSave's loop called record(game, result.events) with no actor, so every restored save, every undo (which rebuilds through fromSave) and the replay viewer stripped the "Player X" prefix off every attributed line. submit attributes and fromMultiplayerSave attributes; this was the one path of three that did not. One argument, with actor already computed on the line above. Why it survived: nothing ever compared a fromSave-built log against a LIVE-played one. The single log-comparing test compares undo's rebuilt log against another fromSave-built log — and undo itself rebuilds through fromSave — so the gap cancelled out on both sides. The suite was green with the bug in and green with it out. The new test plays a game, saves it, restores it and asserts the two logs are identical: the missing direction, not a new requirement. All three fixes were confirmed to go RED with the fix reverted before being called done. 884 tests pass, seventeen new. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YTaNBL1jVxNqgFdjHkHoo3
This commit is contained in:
co-authored by
Claude Opus 5
parent
d267f89a82
commit
e62ea54259
+40
-9
@@ -1122,6 +1122,24 @@ export function view(game: Game, seat: PlayerIndex = 0): Frame {
|
||||
return snapshot(game.state, [], null, null, null, false, seat);
|
||||
}
|
||||
|
||||
/**
|
||||
* Fold a narration's opening word into the middle of a sentence — "Chose to draw" after a name has
|
||||
* to read "Player Bob chose to draw".
|
||||
*
|
||||
* ONLY A SENTENCE-CASED WORD, which is the whole point. It used to be a flat
|
||||
* `text.charAt(0).toLowerCase()`, so every line opening with an all-caps keyword came out mangled:
|
||||
* `EXTRA X18 started…` rendered as `Player Solitaire eXTRA X18 started…`, and the same happened to
|
||||
* `TRAIN 1 MADE UP` and `COLLISION`. Those words are shouted deliberately.
|
||||
*
|
||||
* `^[A-Z][a-z]` is the test — a capital followed by a lower-case letter is an ordinary word that was
|
||||
* capitalised because it began a sentence, and nothing else is. It leaves all-caps keywords alone,
|
||||
* and it also leaves alone a word whose second character is a digit or a hyphen (`X22 Pee-Dee`),
|
||||
* which a naive "is it uppercase?" check would get wrong because `'2'.toUpperCase() === '2'`.
|
||||
*/
|
||||
function uncapitalise(text: string): string {
|
||||
return /^[A-Z][a-z]/.test(text) ? text.charAt(0).toLowerCase() + text.slice(1) : text;
|
||||
}
|
||||
|
||||
function record(game: Game, events: GameEvent[], actor: PlayerIndex | null = null): void {
|
||||
const who = actor === null ? null : (game.state.players[actor]?.name ?? null);
|
||||
for (const e of events) {
|
||||
@@ -1134,7 +1152,7 @@ function record(game: Game, events: GameEvent[], actor: PlayerIndex | null = nul
|
||||
// "Chose to DRAW a card" does not say WHO, which is unreadable the moment there is more than
|
||||
// one seat. Only events the player caused are attributed; the Division running itself is not.
|
||||
const mine = who !== null && 'player' in e;
|
||||
const text = mine ? `Player ${who} ${n.text.charAt(0).toLowerCase()}${n.text.slice(1)}` : n.text;
|
||||
const text = mine ? `Player ${who} ${uncapitalise(n.text)}` : n.text;
|
||||
game.log.push({ text, tone: mine ? 'act' : n.tone });
|
||||
|
||||
}
|
||||
@@ -1235,7 +1253,18 @@ export function fromSave(save: Save, config: GameConfig = SOLO_CONFIG): Game {
|
||||
const result = applyIntent(game.state, actor, intent);
|
||||
if (!result.ok) break;
|
||||
game.history.push(intent);
|
||||
record(game, result.events);
|
||||
/**
|
||||
* `actor` IS PASSED HERE, so a replayed game narrates exactly as the live one did.
|
||||
*
|
||||
* It was omitted, and the omission was invisible in solitaire for a reason worth keeping: the
|
||||
* only test that compares logs ("leaves nothing in the log describing a move that was taken
|
||||
* back") compares one `fromSave`-built log against ANOTHER, so the missing attribution cancelled
|
||||
* out on both sides. Live play attributes (`submit` passes `actor`) and so does multiplayer's
|
||||
* replay (`fromMultiplayerSave`) — this was the one path of the three that did not, which meant
|
||||
* a restored save, an undone game (undo rebuilds through here) and the replay viewer all
|
||||
* described the same moves in different words from the game that produced them.
|
||||
*/
|
||||
record(game, result.events, actor);
|
||||
drain(game);
|
||||
}
|
||||
return game;
|
||||
@@ -1249,16 +1278,18 @@ export function fromSave(save: Save, config: GameConfig = SOLO_CONFIG): Game {
|
||||
* this function only ever reconstructs from history that is already known to have been recorded
|
||||
* under the currently-running rules.
|
||||
*
|
||||
* UNLIKE `fromSave`'s loop, this passes `actor` to `record()` (matching `submit`'s own call,
|
||||
* LIKE `fromSave`'s loop, this passes `actor` to `record()` (matching `submit`'s own call,
|
||||
* `game.ts` above) — found while testing Phase 3's resume path: without it, every replayed line loses
|
||||
* its "Player X" attribution and reads as anonymous "Chose to..." narration, which `record`'s own
|
||||
* comment calls "unreadable the moment there is more than one seat" — exactly the multiplayer case a
|
||||
* resumed game hits every time. `fromSave` has the same gap (it predates multiplayer and nothing ever
|
||||
* compares its output against a LIVE-played log, so it has gone unnoticed — `undo`'s rebuilt game is
|
||||
* itself `fromSave`-built, so `test/web.test.ts`'s replay-fidelity test only ever compares one
|
||||
* unattributed replay against another). Flagged in `TODO.md` rather than fixed there in this pass —
|
||||
* out of scope for Phase 3 and used far more widely, so worth its own careful look rather than a
|
||||
* touch-in-passing.
|
||||
* resumed game hits every time.
|
||||
*
|
||||
* `fromSave` HAD THE SAME GAP AND NO LONGER DOES (fixed 2026-08-30). It predated multiplayer, and
|
||||
* nothing ever compared its output against a LIVE-played log: `undo`'s rebuilt game is itself
|
||||
* `fromSave`-built, so `test/web.test.ts`'s replay-fidelity test only ever compared one unattributed
|
||||
* replay against another and the gap cancelled out on both sides. The test that now pins it plays a
|
||||
* game live, restores it from its own save, and asserts the two logs are identical — which is the
|
||||
* comparison that had been missing rather than a new requirement.
|
||||
*/
|
||||
/**
|
||||
* Why the intent a replay stopped at is reported rather than swallowed.
|
||||
|
||||
Reference in New Issue
Block a user