From f9c4d9fa923a33c5ef7f4303879b5f9d355424a6 Mon Sep 17 00:00:00 2001 From: Jesse Date: Thu, 20 Aug 2026 23:26:39 -0400 Subject: [PATCH] =?UTF-8?q?v0.4.9d=20=E2=80=94=20three=20playtest=20bugs:?= =?UTF-8?q?=20a=20train=20that=20looked=20gone,=20an=20unload=20that=20ign?= =?UTF-8?q?ored=20the=20pick,=20a=20hang=20that=20wasn't=20one?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Patched directly onto 0.4.9a rather than the in-progress 0.5.0 line. A switching train vanished from the board the moment it left the Office. `selectedTrain` in `officeSvg` (board-svg.ts), which gates whether a card draws a train's crew badge, was only ever assigned inside the `cell.adTracks !== null` branch — true for the Office card alone. A train standing anywhere else, which is everywhere it stands while actually being switched, drew no badge at all. Game state was never affected; confirmed against the reported save with the engine directly. Fix hoists the assignment out of the guard so it runs for any card with a train on it. Unloading always took the westmost car, whichever one was picked. `laborer.beginUnload` carried the player's chosen `carIndex`, and `check()` validated that specific car, but the `unloadBegan` event it produced carried only the car's type — the reducer that performs the swap re-derived the target with `industryTrack.cars.findIndex(c => c.loaded)`, which always answers the first loaded car in track order regardless of what was requested. Fix adds `carIndex` to the event and uses it directly. A legal decision could render with zero buttons, which looked exactly like a hang. The "where does this Extra start" decision is titled by `trainCardTitle`, beginning "Making up Extra X22…" — the same prefix `renderActions()` stripped from the action list on the assumption it only ever belonged to the separate yard-chip car-placement panel. With no tray yet being filled, that panel is null, so nothing rendered the Extra's decision either: a legal, correctly-computed option with no button anywhere on the page. Replaying the reported save found no engine deadlock at any step — the engine always had a move. Fix matches the exact title of the one group the yard-chip panel covers instead of a title-prefix regex. New tests for all three: a train parked on an ordinary facility card must draw its crew badge; three differently-typed loaded cars, unloading index 2, must leave indices 0 and 1 alone; the existing softlock regression test now mirrors the real render filter instead of only the raw menu, plus a deterministic test for the exact reported scenario. 590 tests, 0 failures. --- CHANGELOG.md | 75 ++++++++++++++++++++++++++++++++++++++++++++ package.json | 2 +- src/engine/apply.ts | 20 ++++++++++-- src/engine/events.ts | 6 +++- src/sim/board-svg.ts | 12 +++++-- src/web/main.ts | 12 ++++++- test/apply.test.ts | 55 ++++++++++++++++++++++++++++++++ test/replay.test.ts | 2 +- test/web.test.ts | 66 +++++++++++++++++++++++++++++++++++++- 9 files changed, 241 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fd948ce..d90042e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,81 @@ page as `v0.1.0 · · `, so what is deployed can always be identifie --- +## 0.4.9d — 2026-08-21 + +Three bugs from the same playtest session, patched directly onto 0.4.9a rather than the in-progress +0.5.0 line: a display bug that made a switching train look like it had vanished, an engine bug that +silently ignored which car the player chose to unload, and a UI bug that could render a legal +decision with zero buttons — indistinguishable from a hang. + +### A switching train vanished from the board the moment it left the Office + +**REPORTED:** Train 3 stood at the Office; dropping the loaded boxcar off the back at the Freight +House made the train "vanish off the face of the earth," regardless of which way it moved +afterward. The game state was never wrong — replaying the reported save +(`docs/station-master-seed116956197-day2.json`) through the engine directly showed `tray3` fully +intact, in the right place, with the right consist, for the rest of the game. Nothing was ever lost +from `state.trays`. + +The bug was in `officeSvg` (`board-svg.ts`): `selectedTrain` — the value that decides whether a +card draws a train's crew badge (its label, engine arrow, and cars) — was assigned only inside the +`cell.adTracks !== null` branch, which is true for the Office card and nothing else. A train +standing on any other card, which is every card it stands on while actually being switched, got no +badge at all. It disappeared on the very first move off the Office, not only at the Freight House — +that was simply the first place a player was likely to pause and look. Traced to the Roster Pass +(37b1e5b), which rewrote `CellView.train` into `CellView.trains[]` to show every train sharing the +Office's A/D tracks and, in doing so, nested the general consist-drawing logic inside that +Office-only branch instead of leaving it to run for any card with a train on it. + +**Fixed** by hoisting the `selectedTrain` assignment out of the `adTracks !== null` guard so it +runs for every card; the A/D roster-chip loop itself stays inside the guard, since only the Office +has more than one A/D track to draw chips for. New test: a train parked on an ordinary facility +card, `adTracks: null`, now has to draw its crew badge — it did not before this fix. + +### Unloading always took the westmost car, whichever one was picked + +**REPORTED:** three loaded cars stood at a Freight House; asking to unload the east (right-hand) +car unloaded the west one instead, every time, no matter which car was actually chosen. + +`laborer.beginUnload` already carried the player's choice as `carIndex`, and `check()` validated +that specific car — but the event it produced, `unloadBegan`, carried only the car's `carType`, not +which one it was. The reducer that actually performs the swap re-derived the target itself with +`industryTrack.cars.findIndex(c => c.loaded)`, which always answers the first loaded car in track +order regardless of what was requested — so the player's choice was thrown away between `check` and +the reducer, and the westmost loaded car came off every time. + +**Fixed** by adding `carIndex` to the `unloadBegan` event and using it directly in the reducer +(`src/engine/apply.ts`, `case 'unloadBegan'`), instead of re-deriving a car by scanning for +`loaded`. New test in `apply.test.ts`: three loaded cars of three different types, asking to unload +index 2 (east) now leaves indices 0 and 1 alone and empties exactly the one requested — it did not +before this fix. + +### A legal decision could render with zero buttons — the game looked hung + +**REPORTED:** working the Freight House mid-cycle (unloading three cars, then reloading them), the +game "seemed to have hung up. Cannot advance." No trains were on the Division or the Limits. + +Replaying the reported save turned up no engine deadlock at any point — every step had a legal +action — but the exact final state (an Extra train due to start, no tray yet being made up) exposed +a UI bug: `newTrain.startExtra`'s action group is titled by `trainCardTitle`, which begins "Making +up Extra X22…" — the same prefix `renderActions()` (`main.ts`) used to strip from the action list on +the assumption it always belonged to the separate yard-chip car-placement panel. With no tray being +filled yet, that panel (`menu.makeUp`) is `null`, so nothing else rendered the Extra's "choose where +it starts" decision either — a legal, present, correctly-computed option with no button anywhere on +the page. Clicking nothing did nothing, which is indistinguishable from a hang. + +**Fixed** by matching the exact title of the one group `menu.makeUp` actually covers +(`g.title !== menu.makeUp?.title`) instead of a title-prefix regex, so any other group that happens +to start "Making up …" — however it got that title — stays on screen. The existing softlock +regression test (`never strands the player with a legal move and no way to make it`) now mirrors the +real render filter instead of only checking the raw menu, so this class of bug fails that test +directly; a new deterministic test constructs the exact reported scenario (a pending Extra, no +active make-up) and asserts its group survives the filter. + +590 tests, 0 failures. + +--- + ## 0.4.9a — 2026-08-20 Four small tweaks to the splash page (`index.html`, `splash.ts`), no engine changes. diff --git a/package.json b/package.json index 352444d..3eb5f87 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "station-master", - "version": "0.4.9a", + "version": "0.4.9d", "private": true, "type": "module", "description": "Station Master — a railroad operations game", diff --git a/src/engine/apply.ts b/src/engine/apply.ts index 7d80935..07b28a5 100644 --- a/src/engine/apply.ts +++ b/src/engine/apply.ts @@ -1663,7 +1663,13 @@ function execute(s: GameState, player: PlayerIndex, i: Intent): GameEvent[] { case 'laborer.beginUnload': { const f = facilityAt(s, player, i.at)!; return [ - { type: 'unloadBegan', player, at: i.at, carType: f.industryTrack.cars[i.carIndex]!.type }, + { + type: 'unloadBegan', + player, + at: i.at, + carType: f.industryTrack.cars[i.carIndex]!.type, + carIndex: i.carIndex, + }, ]; } @@ -2206,7 +2212,17 @@ export function reduce(s: GameState, e: GameEvent): void { case 'unloadBegan': { const f = facilityAt(s, e.player, e.at)!; - const ci = f.industryTrack.cars.findIndex((c) => c.loaded); + /** + * THE CAR THE PLAYER PICKED, not merely "a loaded one". + * + * REPORTED: with several loaded cars spotted on one industry track, unloading always took the + * WESTMOST car regardless of which one was chosen. This used to re-derive the target with + * `industryTrack.cars.findIndex(c => c.loaded)`, which returns the same answer — the first + * loaded car in track order — no matter which `carIndex` the intent actually named; `check` + * had already validated that specific car, and the choice was thrown away between there and + * here. `e.carIndex` is exactly what `laborer.beginUnload`'s reducer read off the intent. + */ + const ci = e.carIndex; if (ci >= 0) { /** * §9.3 — the replacement empty comes out of the Division Yard. Conjuring it here is what diff --git a/src/engine/events.ts b/src/engine/events.ts index 11db0d1..d45d6a0 100644 --- a/src/engine/events.ts +++ b/src/engine/events.ts @@ -167,7 +167,11 @@ export type GameEvent = | { type: 'loadAdvanced'; player: PlayerIndex; at: GridCoord; fromBox: number; toBox: number } | { type: 'unloadCompleted'; player: PlayerIndex; at: GridCoord; carType: CarType } | { type: 'loadCompleted'; player: PlayerIndex; at: GridCoord; carType: CarType } - | { type: 'unloadBegan'; player: PlayerIndex; at: GridCoord; carType: CarType } + /** + * `carIndex` carries the SPECIFIC car the player picked off the industry track — see the comment + * on `laborer.beginUnload` in `apply.ts` for why the reducer must not re-derive it. + */ + | { type: 'unloadBegan'; player: PlayerIndex; at: GridCoord; carType: CarType; carIndex: number } // -- consequences | { type: 'revenueChanged'; player: PlayerIndex; delta: number; total: number; reason: string } | { type: 'phaseEnded'; player: PlayerIndex; phase: string }; diff --git a/src/sim/board-svg.ts b/src/sim/board-svg.ts index bbedb12..5ae6d86 100644 --- a/src/sim/board-svg.ts +++ b/src/sim/board-svg.ts @@ -568,13 +568,21 @@ export function officeSvg( * Selected train's chip is lit the same amber the crew strip and action buttons already use, * and it is the one whose consist is drawn at the rail below. Every occupied chip carries * `data-crew`, so the play page can wire a click to switch which train that is. + * + * COMPUTED FOR EVERY CARD, NOT JUST THE OFFICE. This used to sit inside the `adTracks !== null` + * branch below, so it was only ever assigned on the Office card — a train standing anywhere else + * in the district (which is everywhere it stands while actually being switched) got no chip at + * all: no label, no engine arrow, and no cars drawn, though the game state never lost it. REPORTED: + * dropping a car at the Freight House made "the train vanish off the face of the earth" — it had + * simply left the one square this code drew a crew on. The Office is the only square that can hold + * more than one train, so `cell.trains[0]` is exactly right everywhere else too. */ - let selectedTrain: CellView['trains'][number] | null = null; + const selectedTrain: CellView['trains'][number] | null = + cell.trains.find((t) => t.trayId === selectedTrayId) ?? cell.trains[0] ?? null; if (cell.adTracks !== null) { const adTracks = cell.adTracks; const gaps = Math.max(0, adTracks - 1); const chipW = Math.min(52, (W - 12 - 4 * gaps) / adTracks); - selectedTrain = cell.trains.find((t) => t.trayId === selectedTrayId) ?? cell.trains[0] ?? null; for (let i = 0; i < adTracks; i++) { const train = cell.trains[i] ?? null; const cx = 6 + i * (chipW + 4); diff --git a/src/web/main.ts b/src/web/main.ts index 451dc21..2ac1542 100644 --- a/src/web/main.ts +++ b/src/web/main.ts @@ -832,9 +832,19 @@ function renderActions( * Everything about a card in hand now lives on the card, and everything about making up a train * lives on the yard chip. What remains is the rest of the turn: the Local Operations choice, * drawing, switching moves, the Freight Agent, and finishing. + * + * EXCLUDE BY TITLE, NOT BY PREFIX. This used to be `!/^Making up /.test(g.title)`, on the + * assumption only the yard-chip panel's own group is ever titled that way. It is not: "choose + * where Extra X22 starts" is titled by `trainCardTitle`, which also begins "Making up Extra + * X22…", so the regex swallowed it too — and with no tray yet being filled, `menu.makeUp` is + * null, so nothing rendered it anywhere else. REPORTED as the game hanging with the Freight + * House mid-cycle: an Extra came due, the action panel had a legal decision and zero buttons, + * and nothing short of restarting looked like it would ever move again. Matching the exact + * title of the ONE group `menu.makeUp` actually covers leaves every other "Making up …" group, + * however it is titled, on screen where a player can act on it. */ html += menu.direct - .filter((g) => !/^(Play|Discard) a card from my hand$/.test(g.title) && !/^Making up /.test(g.title)) + .filter((g) => !/^(Play|Discard) a card from my hand$/.test(g.title) && g.title !== menu.makeUp?.title) .map( (g) => `

${esc(g.title)}

` + diff --git a/test/apply.test.ts b/test/apply.test.ts index e4d1323..1588c59 100644 --- a/test/apply.test.ts +++ b/test/apply.test.ts @@ -685,6 +685,61 @@ describe('Load/Unload: the four-action freight pipeline (§9.3)', () => { assert.equal(check(s, 0, { type: 'laborer.startLoad', at: coord }), 'NO_EMPTY_CAR_SPOTTED'); }); + it('unloads the car the player actually picked, not always the westmost one', () => { + // REPORTED FROM PLAY: three loaded cars stood at a Freight House; asking to unload the EAST + // (index 2) car unloaded the WEST one instead, every time, regardless of which car was picked. + // The reducer for `unloadBegan` used to re-derive the target with + // `industryTrack.cars.findIndex(c => c.loaded)`, which always answers the first loaded car in + // track order no matter which `carIndex` the intent actually named. + const s = game(); + const coord = at(-1, 0); + addCard(s, coord, { + geometry: { kind: 'facility', facility: 'freightHouse' }, + baseOperationalRail: true, + standing: [], + standingWest: 0, + facility: { + kind: 'freight', + subtype: 'freightHouse', + allows: { outbound: true, inbound: true }, + outboundBox: [], + inboundBox: [], + capacity: { outbound: 1, inbound: 1 }, + menAtWork: [null, null, null], + industryTrack: { + cars: [ + { type: 'boxcar', loaded: true }, + { type: 'hopper', loaded: true }, + { type: 'tank', loaded: true }, + ], + }, + laborers: 3, + porters: 0, + usedThisStage: { laborers: 0, porters: 0 }, + }, + modifiers: [], + enhancements: [], + }); + s.clock.phase = 'loadUnload'; + // An empty replacement of every type standing on the track, so `check` never refuses for a + // reason unrelated to what this test is about. + s.yards.divisionYard.push( + { type: 'boxcar', loaded: false }, + { type: 'hopper', loaded: false }, + { type: 'tank', loaded: false }, + ); + + const r = applyIntent(s, 0, { type: 'laborer.beginUnload', at: coord, carIndex: 2 }); + assert.ok(r.ok, 'unloading the east (index 2) car should be legal'); + const f = areaOf(s, 0).grid.get(coordKey(coord))!.facility!; + assert.equal(f.industryTrack.cars[0]!.type, 'boxcar', 'the west car must be untouched'); + assert.equal(f.industryTrack.cars[0]!.loaded, true, 'the west car must still be loaded'); + assert.equal(f.industryTrack.cars[1]!.type, 'hopper', 'the middle car must be untouched'); + assert.equal(f.industryTrack.cars[1]!.loaded, true, 'the middle car must still be loaded'); + assert.equal(f.industryTrack.cars[2]!.type, 'tank', 'the east car — the one picked — must still be there'); + assert.equal(f.industryTrack.cars[2]!.loaded, false, 'the east car is the one that should be emptied'); + }); + it('takes four Laborer actions in total for one point', () => { // The asymmetry the whole economy is calibrated against (card-reference.md §2). const s = game(); diff --git a/test/replay.test.ts b/test/replay.test.ts index 9213aec..2b331bc 100644 --- a/test/replay.test.ts +++ b/test/replay.test.ts @@ -58,7 +58,7 @@ const SAMPLES: GameEvent[] = [ { type: 'loadAdvanced', player: 0, at: { row: 1, col: 0 }, fromBox: 0, toBox: 1 }, { type: 'unloadCompleted', player: 0, at: { row: 1, col: 0 }, carType: 'hopper' }, { type: 'loadCompleted', player: 0, at: { row: 1, col: 0 }, carType: 'hopper' }, - { type: 'unloadBegan', player: 0, at: { row: 1, col: 0 }, carType: 'hopper' }, + { type: 'unloadBegan', player: 0, at: { row: 1, col: 0 }, carType: 'hopper', carIndex: 0 }, { type: 'revenueChanged', player: 0, delta: 1, total: 3, reason: 'freightLoad' }, { type: 'phaseEnded', player: 0, phase: 'localOps' }, ]; diff --git a/test/web.test.ts b/test/web.test.ts index ff0ab17..0ac8fb4 100644 --- a/test/web.test.ts +++ b/test/web.test.ts @@ -1787,7 +1787,19 @@ describe('the static build', () => { // click-through happens to visit. const reachable = (menu: ReturnType): Set => { const out = new Set(); - for (const g of menu.direct) for (const a of g.actions) out.add(a.index); + /** + * THE SAME EXCLUSION `renderActions()` (main.ts) APPLIES, not the raw menu. + * + * `menu.direct` alone is what the ENGINE offers; `renderActions()` then drops any group whose + * title matches the one `menu.makeUp` already covers, so a group counted here but filtered + * there is exactly the gap that stranded a pending Extra with `menu.makeUp` null and its own + * "Making up Extra X22…" title caught by the OLD broader regex this once was. Mirroring the + * real filter is what makes this test the one that would have caught that. + */ + const onScreen = menu.direct.filter( + (g) => !/^(Play|Discard) a card from my hand$/.test(g.title) && g.title !== menu.makeUp?.title, + ); + for (const g of onScreen) for (const a of g.actions) out.add(a.index); for (const g of menu.placeable) for (const it of g.items) for (const sp of it.spots) out.add(sp.index); for (const h of menu.hand) { if (h.playNow !== null) out.add(h.playNow); @@ -1817,6 +1829,37 @@ describe('the static build', () => { } }); + it('offers "where does this Extra start" on screen, not just in the menu', () => { + // REPORTED: the game "hung" mid-Freight-House-work with nothing to click. `newTrain.startExtra` + // was legal and present in `menu.direct` — the engine was never stuck — but its group is titled + // by `trainCardTitle`, which begins "Making up Extra X22…" exactly like the yard-chip panel's + // OWN group. `renderActions()` used to drop every group titled that way, on the assumption only + // the yard-chip panel used the prefix; with no tray yet being filled (`menu.makeUp` is null, + // since the Extra has not chosen where it starts yet), that group was the only thing offered and + // it vanished from the page — a legal decision with zero buttons. + const game = newGame(430); + game.state.pendingExtras.push(22); // Extra X22 "Pee-Dee" — one caboose, per-diem. + game.state.clock.phase = 'newTrain'; + game.state.clock.currentActor = 0; + assert.equal(currentActor(game), 0, 'a decision should be waiting'); + + const menu = actionMenu(game); + assert.equal(menu.makeUp, null, 'no tray is being filled — the Extra has not started yet'); + const extraGroup = menu.direct.find((g) => g.actions.some((a) => menu.options[a.index]?.type === 'newTrain.startExtra')); + assert.ok(extraGroup, 'newTrain.startExtra should appear in some menu.direct group'); + assert.match(extraGroup!.title, /^Making up /, 'this is exactly the title shape that used to collide'); + + // The actual filter `renderActions()` applies (main.ts) — reproduced here for the reason given + // on `reachable()` above: there is no DOM harness in this suite to call the real renderer. + const onScreen = menu.direct.filter( + (g) => !/^(Play|Discard) a card from my hand$/.test(g.title) && g.title !== menu.makeUp?.title, + ); + assert.ok( + onScreen.includes(extraGroup!), + 'the Extra\'s "choose where it starts" group was filtered out along with the yard-chip panel', + ); + }); + it('renders every part of the menu it is given', () => { // THE CHECK THAT WOULD HAVE CAUGHT THE SOFTLOCK ABOVE. The option was in the menu all along — // `makeUp.pass` was computed correctly and simply never rendered, so the invariant on the menu @@ -2054,6 +2097,27 @@ describe('the static build', () => { } }); + it('draws the crew chip on a train standing away from the Office', () => { + // REPORTED: dropping a loaded boxcar off the back of Train 3 at the Freight House made "the + // train vanish off the face of the earth". The game state was never wrong — `selectedTrain` was + // only ever assigned inside the `cell.adTracks !== null` branch, so a train standing on any card + // OTHER than the Office (which is every card it stands on while actually being switched) got no + // badge at all: no label, no engine arrow, no cars. This is the Freight House from that report — + // an ordinary facility card, `adTracks: null`, one train parked on it mid-switch. + const cell = { + row: 1, col: 0, kind: 'facility', label: 'Freight House', running: false, + enhancements: [], enhancementsWhat: [], adTracks: null, + trains: [{ + trayId: 'tray3', label: 'T3', cars: ['empty boxcar'], engineAt: 0, facing: 'w' as const, + what: 'Train 3 "Express"', + }], + cars: [], standingWest: 0, facility: null, links: ['ew'], what: '', + }; + const svg = officeSvg([cell as never], 0); + assert.match(svg, /class="bs-crew"/, 'the crew badge is not drawn off the Office square'); + assert.match(svg, />T3 { // "Settled questions" in docs/plans/switching-paths.md — the split only ever predicts something // while a train stands on the card, and an earlier draft of the plan had this backwards. A stale