diff --git a/CHANGELOG.md b/CHANGELOG.md index 77ff1d5..4c4804f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -341,6 +341,70 @@ read `#houserules`/`#gametype` were rewritten to open the card rather than delet not verified** — there is no browser on this box, so nothing has confirmed the segmented control, the card, or the reversed panel actually look right on screen. That wants a play session. +### Four things the game counted and never said + +Prompted by Jesse asking the general question after two separate v0.7.9 fixes turned out to have 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 being computed, +serialised and sent to nobody?** + +Audited 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.** The four that are not: + +- **`tally.unloadsBegun`** — and this one 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. It reported half of a symmetric + mechanism. There is an "Unloads still in the pipeline" line now. +- **`tally.cardsDiscarded`** — counted by the engine, listed beside "Cards drawn" and "Cards played" + without it. Gitea#9 made throwing a Timetabled train away a legal, deliberate move, so a discard is + a player CHOICE the game counted and never reported. It reports it now. +- **`viewerSeat`** and **`overHandLimit`** — deferred by Jesse. The second is the fullest version of + the shape: the 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 with no consumer. Left alone for now; the decision when it comes is + delete-or-document, not a patch. + +**Fixing the two turned up a third thing worth knowing.** `resultsHtml` draws +`tallyHtml(report?.tally ?? f.tally)`, and `report` is `f.official`, which the engine writes the +moment any game ends — so a finished game reports the tally frozen at the official ending, not the +live one. The first attempt at a test overrode `f.tally` alone, changed nothing on screen, and failed +for a reason that had nothing to do with the fix. + +### A shouted keyword is not a sentence + +`EXTRA X18 started…`, attributed to a player, rendered as `Player Solitaire eXTRA X18 started…`. The +same happened to `TRAIN 1 MADE UP` and `COLLISION`, which are shouted deliberately. + +`record` folds a narration's opening word into the middle of a sentence — "Chose to draw" has to read +"Player Bob chose to draw" — 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 is an ordinary +word capitalised because it began a sentence and nothing else is. That also leaves `X22 Pee-Dee` +alone, which a naive "is the first letter uppercase?" test gets 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 explicitly ruled balance work out of scope. Found by reading the section it did not +belong to. + +### 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. Live play attributes — `submit` passes `actor` — and so does multiplayer's +replay; this was the one path of the three that did not, which meant the same moves were described in +different words from the game that produced them. The fix is one argument, with `actor` already +computed on the line above. + +**Why it survived two years of green tests, which is the part worth keeping.** 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 since `undo` itself rebuilds through +`fromSave`, the missing attribution cancelled out on both sides. The whole suite stayed green with the +bug in place, and stayed green after the fix too. + +The new test plays a game, saves it, restores it, and asserts the two logs are identical. That is the +**missing direction**, not a new requirement. All three of this batch's fixes were confirmed to fail +with the fix reverted before being called done — the discipline the v0.7.5→v0.7.8 sequence bought. + ### Three wording and layout fixes - **The collision entries** on all three screens now read "The game ends immediately and results in @@ -355,7 +419,7 @@ card, or the reversed panel actually look right on screen. That wants a play ses cost, not a danger, and red would outrank the actual rules above it. The buttons say **Continue Existing Saved Game** and **Deal New Game** rather than "Continue saved game" and "Deal". -881 tests pass, fourteen of them new; one existing test asserted the opposite of the collision ruling +884 tests pass, seventeen of them new; one existing test asserted the opposite of the collision ruling above and says so where it was reversed. --- diff --git a/src/web/game.ts b/src/web/game.ts index c7c656d..5cb6c50 100644 --- a/src/web/game.ts +++ b/src/web/game.ts @@ -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. diff --git a/src/web/panels.ts b/src/web/panels.ts index aeb6680..6afb839 100644 --- a/src/web/panels.ts +++ b/src/web/panels.ts @@ -352,7 +352,17 @@ function tallyHtml(t: Frame['tally']): string { }; push('Loads made up', t.loadsCompleted); push('Loads broken', t.unloadsCompleted); + /** + * BOTH HALVES OF THE MEN | AT | WORK PIPELINE, not just the loading one. + * + * §9.1 makes loading and unloading the same shape — begun, then carried through — and the Tally + * has counted both since it was written. The screen reported only the loading side, so a player + * with three unloads part-finished at the final whistle was told nothing about them while the + * equivalent loads were listed. Found 2026-08-30 auditing which Frame fields nothing reads: + * `unloadsBegun` was one of four, and the only one whose absence was visible on screen. + */ push('Loads still in the pipeline', t.loadsStarted - t.loadsCompleted); + push('Unloads still in the pipeline', t.unloadsBegun - t.unloadsCompleted); push('Passengers boarded', t.passengersBoarded); push('Passengers detrained', t.passengersDetrained); push('Cars coupled', t.carsCoupled); @@ -386,6 +396,10 @@ function tallyHtml(t: Frame['tally']): string { } push('Cards drawn', t.cardsDrawn); push('Cards played', t.cardsPlayed); + // Gitea#9 made throwing a Timetabled train away a legal and deliberate move, so a discard is a + // CHOICE the player made rather than an accident of the hand limit — and the engine has counted + // it all along while the screen listed only draws and plays beside it. + push('Cards discarded', t.cardsDiscarded); return `

The railroad

${factTable(rows)}`; } diff --git a/test/web.test.ts b/test/web.test.ts index 920d980..3c27b3c 100644 --- a/test/web.test.ts +++ b/test/web.test.ts @@ -1368,6 +1368,55 @@ describe('replays are saves', () => { assert.deepEqual(back.log, fromSave(toSave(back)).log, 'the rebuilt log is not the replayed log'); }); + it('replays a save into the same words the live game wrote', () => { + /** + * THE COMPARISON THAT WAS MISSING. `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 — describing the same + * moves in different words from the game that produced them. + * + * It went unnoticed because nothing compared a `fromSave`-built log against a LIVE-played one. + * The test above compares one rebuilt log against another rebuilt log, so the gap cancelled out + * on both sides and stayed green throughout. This plays a game, saves it, restores it, and + * asserts the two logs are identical — the direction that catches it. + */ + const live = newGame(202); + for (let i = 0; i < 30; i++) { + if (currentActor(live) === null) break; + const { options } = actionGroups(live); + if (options.length === 0 || !submit(live, options[0]!)) break; + } + const attributed = live.log.filter((l) => l.text.startsWith('Player ')); + assert.ok(attributed.length > 0, 'the live game attributed nothing, so this proves nothing'); + + assert.deepEqual( + fromSave(toSave(live)).log, + live.log, + 'a restored game does not narrate what the live game narrated', + ); + }); + + it('does not mangle a narration that opens with a shouted keyword', () => { + /** + * `record` folds a narration's first word into the middle of a sentence — "Chose to draw" has to + * read "Player Bob chose to draw". It used to do that with a flat `charAt(0).toLowerCase()`, so + * every line opening with an all-caps keyword came out as `Player Solitaire eXTRA X18 started…`. + * The same happened to `TRAIN 1 MADE UP` and `COLLISION`, which are shouted on purpose. + */ + const game = newGame(202); + for (let i = 0; i < 120; i++) { + if (currentActor(game) === null) break; + const { options } = actionGroups(game); + if (options.length === 0 || !submit(game, options[0]!)) break; + } + const mangled = game.log.filter((l) => /^Player .* [a-z][A-Z]{2}/.test(l.text)); + assert.deepEqual(mangled, [], 'a shouted keyword was lowercased into the middle of a word'); + + // And the ordinary case still folds, or the prefix would read "Player Bob Chose to draw". + const folded = game.log.filter((l) => /^Player \S+ [a-z]/.test(l.text)); + assert.ok(folded.length > 0, 'nothing was folded at all — the prefix is no longer a sentence'); + }); + it('stays small enough to email', () => { const game = newGame(202); for (let i = 0; i < 400; i++) { @@ -3624,6 +3673,50 @@ describe('the end-of-game results screen (Gitea#16)', () => { assert.ok(html.includes('Trains through the Division'), 'the tally section is missing'); }); + it('reports BOTH halves of the work pipeline, and cards discarded', () => { + /** + * FOUND 2026-08-30 auditing which `Frame` fields nothing reads. Two `Tally` members were + * counted by the engine and drawn by nothing: + * + * - `unloadsBegun`. §9.1 makes loading and unloading the same shape — begun, then carried + * through — and the screen printed "Loads still in the pipeline" for one side and nothing for + * the other, so it reported half of a symmetric mechanism. + * - `cardsDiscarded`. Gitea#9 made throwing a Timetabled train away a legal, deliberate move, + * so a discard is a CHOICE, listed beside draws and plays rather than left out of them. + * + * The tally is overridden rather than played into, because reaching a part-finished unload and + * a discard in the same seeded game is incidental to what is being checked here. + */ + const f = finished(); + /** + * THE OFFICIAL REPORT'S TALLY IS THE ONE DRAWN, not the Frame's — `resultsHtml` renders + * `tallyHtml(report?.tally ?? f.tally)` and `report` is `f.official`, which the engine writes + * the moment any game ends. Overriding only `f.tally` here changed nothing at all and the test + * failed for a reason that had nothing to do with the fix, which is worth pinning in passing: + * a finished game reports the numbers frozen at the official ending, not the live ones. + */ + const withTally = (over: Partial): Frame => ({ + ...f, + tally: { ...f.tally, ...over }, + official: f.official ? { ...f.official, tally: { ...f.official.tally, ...over } } : f.official, + }); + const html = resultsHtml(withTally({ unloadsBegun: 5, unloadsCompleted: 2, cardsDiscarded: 3 })); + assert.ok(html.includes('Unloads still in the pipeline'), 'the unloading pipeline is not reported'); + assert.ok(html.includes('Cards discarded'), 'a deliberate discard is counted and never reported'); + + // The in-flight count is begun-minus-completed, matching the loading line beside it. + assert.match( + html, + /Unloads still in the pipeline<\/[a-z]+><[^>]*>3 { const f = finished({ mode: 'competitive', minCombinedRevenue: 0 }, ['Ada', 'Bo']); // Ada won at the timetable; Bo overtakes during extended play. The frozen result stands.