v0.8.5 — housekeeping from the audit, and the playtest line retired
The third release from the audit; nothing a player sees changes. CHANGELOG has the detail. The 0.4.9 playtest line is no longer maintained (Jesse, 2026-09-29): the deploy rule that existed for it is gone and #85 is moot. The table test (#39 #35 #42a #40) is closed — every line of the checklist was met at a table. #46 is done and cannot regrow: the 36 unused declarations are removed and `noUnusedLocals`/`noUnusedParameters` are on; two of them were dead bot functions from rejected candidates the round said it had deleted. The documents no longer teach `trainCapSlack` (a knob that throws), point at `as-built.md` (deleted in 0.8.2), model `officeType` (the engine says `tier`) or describe `collisionOccurred` (never emitted); the README's account of bot flags now matches the bot's. Five playtest saves committed in `docs/` against the repository's own rule are in the ignored `playtests/`. What the audit found and did not fix is written down as TODO #112-#117, each with its reason. #112 is `docs/plans/structure.md`, the proposal for `http.ts`, `main.ts` and `check`. #117 — `/api/save` hands a seat the seed mid-game — waits on a conversation. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FrCWubm9GAftYCm2hWdKwK
This commit is contained in:
co-authored by
Claude Fable 5.1
parent
e47cd3d400
commit
04ca74c365
@@ -0,0 +1,113 @@
|
||||
# Structure — a proposal (2026-09-29)
|
||||
|
||||
What the 2026-09-29 audit found about the SHAPE of the code, as distinct from its faults, and what
|
||||
to do about it. Nothing here changes behaviour; each item is a seam that would let the next fault be
|
||||
found by a test instead of by a reviewer reading three thousand lines. Ordered by payoff.
|
||||
|
||||
The faults the audit fixed in 0.8.3 and 0.8.4 were nearly all in the three largest functions or in
|
||||
the one file no test stood up. That is the argument: the structure is where the bugs were.
|
||||
|
||||
## 1. `http.ts` — a route table, and one place for authentication
|
||||
|
||||
**Today.** `startServer` is one 600-line async closure of `if (pathname === … && method === …)`
|
||||
chains. The token → session → lobby-or-game resolution is copy-pasted eight times; the admin-secret
|
||||
check, the join-secret check and the host check are each inline where they are needed. The catch at
|
||||
the bottom is the only error path, and until 0.8.4 it was itself a crash.
|
||||
|
||||
**Proposed.** A table of routes, each `{ method, path, auth, handler }`, with three auth wrappers:
|
||||
|
||||
```
|
||||
withToken(handler) // resolves { ps, lobby?, game? } from the token, 404s if none
|
||||
withHost(handler) // withToken, then refuses anyone but lobby.hostToken
|
||||
withAdmin(handler) // the x-admin-secret gate; the whole prefix is absent when unset
|
||||
```
|
||||
|
||||
Handlers become ten to thirty lines each and take a typed context. The one dispatcher owns
|
||||
`readJson`, `sendJson`, the `HttpError` catch and the `headersSent` guard — one place, so 0.8.4's
|
||||
crash fix cannot be forgotten by the next route. The per-game `inTurn` queue becomes a property of
|
||||
the game context rather than something a handler has to remember to call.
|
||||
|
||||
**Cost.** A day. Every route is exercised by `test/server/http.test.ts` now, so the move is
|
||||
mechanical and verifiable. Do it before adding the next route (the seatless display stream, #20).
|
||||
|
||||
## 2. `main.ts` — five extractions, and a `Selection` value
|
||||
|
||||
**Today.** 3200 lines, ~21 module-level `let`s (`session`, `selected`, `mode`, `pendingAt`,
|
||||
`selectedCrew`, `peekPlayer`, `zoom`, `soundOn`, `gameCode`, `remoteToken`, …), and `start()` runs at
|
||||
import. `render()` is ~470 lines; `renderActions` ~360. `test/web.test.ts` has to stub the DOM before
|
||||
importing, can never build two pages, and cannot call `render()` with a `Frame` of its choosing —
|
||||
which is why every screen fault in the audit was found by reading, not by a test.
|
||||
|
||||
**Proposed, in the order they pay:**
|
||||
|
||||
1. **`web/remote-store.ts`** — `RemoteRecord`, `readStore`/`writeStore`, `loadRemote`/`saveRemote`/
|
||||
`forgetRemote`/`knownRemote`, and `lobbyHandlers`. Pure functions over `localStorage`; testable
|
||||
with a Map. Today they are untestable except through the page.
|
||||
2. **`web/url-options.ts`** — `RULE_PARAMS`/`VICTORY_PARAMS`/`OPTIONAL_PARAMS`, `gameOptionsFromUrl`,
|
||||
`solitaireDefaults`, `rulesToUrl`. A pure round trip; the audit found the `location.search = ''`
|
||||
no-op assumption (#114) by reading this code, and a test on the round trip would have found it.
|
||||
3. **`web/setup-screen.ts`** — `wireGameTypeBlock`, `commitNewGame`, `runSolitaireSetup`.
|
||||
4. **A `Selection` object** replacing the five `let`s (`selected`, `mode`, `pendingAt`,
|
||||
`selectedCrew`, `peekPlayer`) that every click handler mutates. One value, one `reset()`, passed
|
||||
to `render` rather than read from module scope — which is what lets a test call `render(frame,
|
||||
selection)` and assert on the HTML.
|
||||
5. **`web/game-shell.ts`** — session routing: `beginRemote`, `abandonRemote`, `claimSeat`, `start`,
|
||||
`applyCapabilities`, the handoff. This is where the animation loop outliving the session (#114)
|
||||
lives, and it is easier to see once it is not surrounded by rendering.
|
||||
|
||||
Keep `render()` in `main.ts` but split the board-and-hand wiring (~240 lines of `addEventListener`)
|
||||
into `wireBoard(selection)` so what is DRAWN and what is CLICKABLE are separate functions.
|
||||
|
||||
**Cost.** Two to three days across the five, each its own commit; the first two are an afternoon
|
||||
each and carry no risk.
|
||||
|
||||
## 3. The engine — `check` per phase, and one car category
|
||||
|
||||
**Today.** `check` (~600 lines) and `reduce` (~700) are single switches mixing every phase;
|
||||
`moveTrain` nests four position kinds by three decision kinds. Car category (coach / caboose /
|
||||
freight) is computed in three places with three spellings. The seat guard 0.8.3 added to the four
|
||||
switching intents is the same four lines four times.
|
||||
|
||||
**Proposed.**
|
||||
|
||||
- Split `check` by phase — `checkLocalOps`, `checkNewTrain`, `checkLoadUnload`, `checkMainline` —
|
||||
each a switch over its own intents, dispatched by `inPhase`. The shared guards (`ownTray`, the
|
||||
option-chosen test, the Moves test) become the first lines of `checkLocalOps` rather than four
|
||||
repeats. `reduce` splits the same way.
|
||||
- One exported `carCategory(type)` in `content.ts`, used by `acceptsCar`, `newTrainPhase`,
|
||||
`consistNeeds` and `isFreight`/`carriesLoad`.
|
||||
- `maneuver.flyingSwitch` is a weaker copy of `switch.dropCars` (no `switchingRefusal`, no
|
||||
`engineAt` clamp, no `standingWest` handling). It should CALL the dropCars path with a flag,
|
||||
or be deleted until the card is dealt (it is at 0 copies). Deciding is #115's job; the structure
|
||||
point is that there must be one cut-dropping reducer.
|
||||
|
||||
**Cost.** The `check`/`reduce` split is a day and is pure motion — every test in `apply.test.ts`
|
||||
and `advance.test.ts` runs unchanged. `carCategory` is an hour.
|
||||
|
||||
## 4. One `Push` type
|
||||
|
||||
`web/session.ts` mirrors `server/session.ts`'s `Push` by hand so the browser bundle never imports
|
||||
from `src/server/`. Every envelope change (0.8.4 added `lastSeq`) is two edits. Move the wire types
|
||||
— `Push`, `LobbyPush`, `LobbyPreview` — into `src/sim/wire.ts`, which both sides already import
|
||||
from, and delete the mirror.
|
||||
|
||||
**Cost.** An hour.
|
||||
|
||||
## 5. Tests that would then exist
|
||||
|
||||
Each extraction above names the test it makes possible:
|
||||
|
||||
| after | test |
|
||||
| --- | --- |
|
||||
| route table | one test per route for the 404/403/400 answers, from a table |
|
||||
| `remote-store.ts` | the `RemoteRecord` round trip, the "seat tied to this browser" notice (#33) |
|
||||
| `url-options.ts` | `rulesToUrl(gameOptionsFromUrl(x)) === x`, and the `search = ''` case |
|
||||
| `Selection` | `render(frame, selection)` snapshot tests for each screen state |
|
||||
| `check` per phase | the seat guard once, as a property over every switching intent |
|
||||
|
||||
## What not to do
|
||||
|
||||
- Do not refactor `render()` and fix a screen bug in the same commit. The audit's value was that
|
||||
each finding could be verified against unchanged code.
|
||||
- Do not introduce a framework or a runtime dependency to do any of this. The zero-dependency rule
|
||||
is what makes the package a 63 MB `.s9pk` and the site a static upload.
|
||||
Reference in New Issue
Block a user