replay: route player input through a command chokepoint (Phase 1)

Reshapes every UI-driven sim mutation to flow through one path so it can be
recorded and replayed later, with behaviour unchanged.

- Command model (lib): Command base + derived types (PlaceBuilding, Demolish,
  RotateInPlace, SetRecipe, SetShipLayout, Set[Site]SplitterFilters,
  ClearBeltTiles, ApplySchematicChoice, Reset), each with a playerId for the
  future-multiplayer shape. PlaceBuilding is atomic (carries optional
  recipe/layout/filters) because deferred commands never return the new
  BuildingId to the caller.
- CommandManager (lib): FIFO queue holding a Simulation&; enqueue + drain.
- Simulation::apply(const Command&): the single chokepoint, dispatching by
  kind to the existing mutators. Mutators stay public (enforced by convention,
  not compile-time, so the test suite keeps driving the sim directly).
- Timing: GameWorldView owns the CommandManager and drains it once per frame in
  onFrame, before the tick batch (runs at 0x too, so build-while-paused is
  preserved). A drained Reset triggers the view reset.
- UI fan-in: GameWorldView enqueues its own input directly; MainWindow and
  SelectedBuildingPanel emit CommandRequestedEvent, which GameWorldView
  subscribes to and enqueues. No UI site mutates the sim directly anymore.
- CommandTest: asserts apply(...) yields byte-identical state to the direct
  mutator path, and that CommandManager drains FIFO through apply.

Full suite green (338 cases / 3353 assertions); determinism double-run still
passes. Design doc updated with the atomic-PlaceBuilding and convention-
enforcement decisions.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DUsFgd2Ga6pmLz8giS8WUn

docs/replay_design.md

@@ -70,6 +70,16 @@ Commands use a **base class + derived classes** (mirroring the existing `Event`
 idiom, so it is native to this codebase). They are routed through a dedicated command path,
 **not** through `EventManager` (see next section).
 
+> **Implementation refinement (Phase 1).** `PlaceBuilding` is **atomic**: it carries the
+> optional recipe / ship-layout / splitter-filters to configure the new building in the same
+> command. This is forced by the deferred-drain timing — commands apply at a later tick
+> boundary, so the caller never sees the new `BuildingId` and therefore cannot issue a
+> follow-up `SetRecipe`/`SetShipLayout` against it. The standalone `SetRecipe`,
+> `SetShipLayout`, and the two `SetSplitterFilters` commands remain for the dialog-driven
+> edits on *existing* buildings (which reference a known id). `Reset` carries the (move-only)
+> `GameConfig` via `shared_ptr` and is moved into the sim on apply; a null config means "keep
+> current config".
+
 ## Command system: reuse the *pattern*, not the EventManager singleton
 
 We reuse the **pattern** of the existing event system (a polymorphic base + small derived
@@ -103,10 +113,20 @@ types), but the sim-mutating command path is a **dedicated, ordered queue**, not
 chokepoint. Any path that mutates the sim directly would not be recorded and would silently
 desync the replay.
 
-This is enforced **structurally, not by discipline**: the `Simulation` mutators
-(`tryPlaceBuilding`, `demolish`, `setRecipe`, …) are made private/non-public so the only way to
-reach them is `apply(command)`. A stray direct call then fails to compile rather than compiling
-and silently desyncing.
+This was originally intended to be enforced **structurally** (make the `Simulation` mutators
+non-public so the only way to reach them is `apply(command)`). 
+
+> **Implementation decision (Phase 1).** The structural-enforcement plan was **dropped in
+> favour of convention**, because the test suite legitimately drives the same mutators
+> directly (`sim.tryPlaceBuilding(...)` and its returned id, `buildings().setRecipe(...)`,
+> `applySchematicChoice`, `reset`, `placeImmediate`, …) and relies on their return values —
+> making them non-public would break ~30 test call sites, and `apply()` cannot hand a new
+> `BuildingId` back to a caller. So the mutators stay **public**; the rule "every UI mutation
+> goes through a command" is upheld by convention and a documented chokepoint comment on
+> `Simulation::apply`. A `[command]` Catch2 suite asserts `apply(...)` produces byte-identical
+> state to the direct mutator path, guarding the equivalence the replay relies on. Tests are
+> not gameplay (they never record), so direct mutator use there does not affect replay
+> correctness.
 
 Recording happens **at the apply chokepoint**, not at the UI gesture — so only commands that
 actually reached the sim are recorded, and they replay through the identical apply path.
@@ -337,28 +357,30 @@ The whole feature rests on a deterministic sim, so prove that before building on
 - **Exit criteria:** the double-run test passes. If it fails, fix the nondeterminism here before
   proceeding.
 
-### Phase 1 — Command model + enforced chokepoint (no recording yet)
+### Phase 1 — Command model + chokepoint (no recording yet) — DONE
 
 Reshape mutations to flow through one path; behaviour unchanged.
 
-- Define `Command` base + derived types (`PlaceBuilding`, `Demolish`, `RotateInPlace`,
-  `SetRecipe`, `SetShipLayout`, `SetSplitterFilters`, `ClearBeltTiles`, `ApplySchematicChoice`,
-  `Reset`) in `lib`. Each carries a source/player id (always 0 now) for the future-multiplayer
-  shape.
-- Add `CommandManager` (FIFO queue, drain) in `lib`, holding a `Simulation&`.
-- Add `Simulation::apply(const Command&)` dispatching to the existing mutators; then **make
-  those mutators non-public** so `apply` is the only entry — compile-enforces the completeness
-  invariant.
-- Wire the drain: in `GameWorldView::onFrame`, call `CommandManager::drain()` **once per frame,
-  before** the tick batch; tag each command with the current completed-tick count.
-- Refactor every UI mutation site to **emit a single `CommandRequestedEvent`** (carrying a
-  `shared_ptr<Command>`) via the existing `EventManager`; one dispatcher subscribes and enqueues
-  onto `CommandManager`.
-- **Files:** new `lib` command + `CommandManager`; `Simulation.h/.cpp`; call sites in
-  `GameWorldView.cpp`, `MainWindow.cpp`, `SelectedBuildingPanel.cpp`; new dispatcher in
-  `ui`/`app`.
-- **Exit criteria:** game plays identically (including build-while-paused), determinism test
-  still passes, no widget can call a sim mutator directly (won't compile).
+- Defined `Command` base + derived types (`PlaceBuilding`, `Demolish`, `RotateInPlace`,
+  `SetRecipe`, `SetShipLayout`, `SetSiteSplitterFilters`, `SetSplitterFilters`,
+  `ClearBeltTiles`, `ApplySchematicChoice`, `Reset`) in `lib`, each with a `playerId` (always 0
+  now). `PlaceBuilding` is atomic (carries optional config — see the refinement note above).
+- Added `CommandManager` (FIFO queue, `enqueue`/`drain`) in `lib`, holding a `Simulation&`.
+- Added `Simulation::apply(const Command&)` dispatching by `CommandKind` to the existing
+  mutators — the single documented chokepoint. (Mutators stay public; enforced by convention,
+  see the decision note above.)
+- Wired the drain: `GameWorldView::onFrame` calls `CommandManager::drain()` once per frame,
+  before the tick batch (runs even at 0× → build-while-paused preserved). A drained `Reset`
+  triggers the view reset.
+- Refactored every UI mutation site: `GameWorldView` owns the `CommandManager` and enqueues
+  directly; `MainWindow` and `SelectedBuildingPanel` emit `CommandRequestedEvent` (carrying a
+  `shared_ptr<const Command>`) which `GameWorldView` subscribes to and enqueues.
+- **Files:** new `lib/sim/Command.h`, `CommandManager.{h,cpp}`; `CommandRequestedEvent.h`;
+  `Simulation.{h,cpp}` (`apply`); `GameWorldView.{h,cpp}`, `MainWindow.cpp`,
+  `SelectedBuildingPanel.cpp`; new `CommandTest.cpp`.
+- **Exit criteria:** game plays identically (including build-while-paused); determinism test
+  still passes; `[command]` equivalence tests pass; no UI call site mutates the sim directly
+  (verified by grep — convention, not compile-enforced).
 
 ### Phase 2 — Recording