From c2196cb34ef4fa36dc1a59f3e7c65b4543a1b008 Mon Sep 17 00:00:00 2001 From: Malte Langkabel Date: Tue, 30 Jun 2026 22:56:51 +0200 Subject: [PATCH] replay: compile-enforce the command chokepoint Privatize the Simulation player-action mutators (tryPlaceBuilding, demolish, applySchematicChoice) and the mutable subsystem accessors (now buildingsMutable/ beltsMutable; only the const buildings()/belts() stay public). Production's only handle to a subsystem is through Simulation, and every production buildings()/ belts() call is a const query, so this fully locks gameplay out of any command-bypassing mutation path -- such code now fails to compile instead of silently desyncing replays. Tests reach the private mutators through SimulationTestAccess, a friend struct under src/test (off the lib/ui/app include path), so they keep calling the real mutators and keep their return values -- no id-by-position recovery needed. The BuildingSystem subsystem mutators stay public (BuildingTest unit-tests a bare subsystem; unreachable from production anyway). Upgrades the Phase 1 "convention only" decision to structural enforcement. All targets build; 354 cases / 3418 assertions pass. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01DUsFgd2Ga6pmLz8giS8WUn --- docs/replay_design.md | 54 +++++++++++++++++++++----------- src/lib/sim/Simulation.cpp | 4 +-- src/lib/sim/Simulation.h | 41 +++++++++++++++--------- src/test/BlueprintTest.cpp | 33 +++++++++---------- src/test/CommandTest.cpp | 17 +++++----- src/test/DeterminismTest.cpp | 9 +++--- src/test/RecipeSchematicTest.cpp | 9 +++--- src/test/ShipModuleTest.cpp | 19 +++++------ src/test/ShipyardTest.cpp | 11 ++++--- src/test/SimulationTestAccess.h | 40 +++++++++++++++++++++++ src/test/WaveSystemTest.cpp | 3 +- 11 files changed, 159 insertions(+), 81 deletions(-) create mode 100644 src/test/SimulationTestAccess.h diff --git a/docs/replay_design.md b/docs/replay_design.md index 366a93d..beed998 100644 --- a/docs/replay_design.md +++ b/docs/replay_design.md @@ -113,20 +113,36 @@ 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 was originally intended to be enforced **structurally** (make the `Simulation` mutators -non-public so the only way to reach them is `apply(command)`). +This is enforced **structurally**: the `Simulation` player-action mutators are **private**, so +the only way production code can 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. +> **Implementation decision (Phase 1, revised post-Phase 4).** Structural enforcement was +> initially deferred in favour of convention, because the test suite legitimately drives the +> same mutators directly and relies on their return values (notably the `BuildingId` from +> placement, which `apply()` cannot hand back to a caller). It was later restored once a key +> observation made the change cheap: **the UI's only handle to a mutable subsystem is through +> `Simulation`** — no production code in `ui`/`app`/`balancing` holds a `BuildingSystem`/ +> `BeltSystem` directly, and every production `buildings()`/`belts()` call is a const query. +> So: +> +> - `Simulation::tryPlaceBuilding`, `demolish`, and `applySchematicChoice` are **private**. +> - The mutable subsystem accessors are private and renamed `buildingsMutable()` / +> `beltsMutable()`; only `const BuildingSystem& buildings() const` / `belts() const` are +> public (queries). UI query sites bind to the const overload unchanged. +> - `Simulation::apply` still mutates through the private members directly, so the chokepoint +> itself is unaffected. +> - Tests reach the private mutators through `SimulationTestAccess` (src/test, a `friend struct` +> of `Simulation`), so they keep calling the real mutators **and keep getting return values** +> — no id-by-position recovery needed. This header is not on the lib/ui/app include path, so +> only test translation units can use it. +> +> The `BuildingSystem` subsystem mutators (`place`, `setRecipe`, `placeImmediate`, +> `forEachBuilding`, …) stay **public**: `BuildingTest` unit-tests a bare `BuildingSystem` with +> no `Simulation`/command layer, and that surface is unreachable from production anyway (you +> cannot obtain a mutable subsystem without the private accessor). A `[command]` Catch2 suite +> still 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. @@ -366,9 +382,10 @@ Reshape mutations to flow through one path; behaviour unchanged. `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.) +- Added `Simulation::apply(const Command&)` dispatching by `CommandKind` to the underlying + mutators — the single chokepoint. The `Simulation` player-action mutators are **private** + (compile-time enforced; tests reach them via the `SimulationTestAccess` friend) — 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. @@ -379,8 +396,9 @@ Reshape mutations to flow through one path; behaviour unchanged. `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). + still passes; `[command]` equivalence tests pass; no production call site can mutate the sim + directly (compile-enforced: the `Simulation` mutators are private, tests excepted via + `SimulationTestAccess`). ### Phase 2 — Recording — DONE diff --git a/src/lib/sim/Simulation.cpp b/src/lib/sim/Simulation.cpp index e735fef..6455591 100644 --- a/src/lib/sim/Simulation.cpp +++ b/src/lib/sim/Simulation.cpp @@ -1182,7 +1182,7 @@ void Simulation::demolish(BuildingId id) m_buildingBlocksStock += m_buildingSystem->demolish(id); } -BuildingSystem& Simulation::buildings() +BuildingSystem& Simulation::buildingsMutable() { return *m_buildingSystem; } @@ -1192,7 +1192,7 @@ const BuildingSystem& Simulation::buildings() const return *m_buildingSystem; } -BeltSystem& Simulation::belts() +BeltSystem& Simulation::beltsMutable() { return m_beltSystem; } diff --git a/src/lib/sim/Simulation.h b/src/lib/sim/Simulation.h index f404655..d87036e 100644 --- a/src/lib/sim/Simulation.h +++ b/src/lib/sim/Simulation.h @@ -68,11 +68,6 @@ public: // Returns true if there are pending schematic choices waiting for player input. bool hasSchematicChoicesPending() const; - // Applies the player's chosen schematic from the pending choices. - // choiceIndex must be in [0, pendingChoices.size()). - // Clears the pending choices after application. - void applySchematicChoice(int choiceIndex); - Tick currentTick() const; // The seed this run was (re)initialized with; written to the replay header. unsigned int getSeed() const; @@ -108,16 +103,11 @@ public: // a superset of rngFingerprint(). unsigned long long computeStateChecksum() const; - // Checks affordability, deducts building blocks, and places the building. - // Returns the new entity id, or kInvalidBuildingId if blocks are insufficient. - BuildingId tryPlaceBuilding(BuildingType type, QPoint anchor, Rotation rotation); - - // Demolishes the building with the given id and refunds building blocks. - void demolish(BuildingId id); - - BuildingSystem& buildings(); + // Const subsystem accessors (queries only). The mutable counterparts are + // private and reachable only through Simulation::apply (the command + // chokepoint) or, in tests, SimulationTestAccess — so production code cannot + // mutate the factory outside the recorded command path (docs/replay_design.md). const BuildingSystem& buildings() const; - BeltSystem& belts(); const BeltSystem& belts() const; ShipSystem& ships(); const ShipSystem& ships() const; @@ -127,6 +117,29 @@ public: const EntityAdmin& admin() const; private: + // Grants tests access to the private player-action mutators below without + // opening them to production code (see src/test/SimulationTestAccess.h). + friend struct SimulationTestAccess; + + // -- Player-action mutators (command chokepoint only) -------------------- + // Reached during play exclusively via apply(); never called by UI/app code. + + // Checks affordability, deducts building blocks, and places the building. + // Returns the new entity id, or kInvalidBuildingId if blocks are insufficient. + BuildingId tryPlaceBuilding(BuildingType type, QPoint anchor, Rotation rotation); + + // Demolishes the building with the given id and refunds building blocks. + void demolish(BuildingId id); + + // Applies the player's chosen schematic from the pending choices. + // choiceIndex must be in [0, pendingChoices.size()). + // Clears the pending choices after application. + void applySchematicChoice(int choiceIndex); + + // Mutable subsystem accessors; same chokepoint rule as the mutators above. + BuildingSystem& buildingsMutable(); + BeltSystem& beltsMutable(); + void handleEvent(std::shared_ptr event) override; BuildingId allocateBuildingId(); // Strictly increasing; never returns kInvalidBuildingId. diff --git a/src/test/BlueprintTest.cpp b/src/test/BlueprintTest.cpp index 2f87754..9fdb467 100644 --- a/src/test/BlueprintTest.cpp +++ b/src/test/BlueprintTest.cpp @@ -16,6 +16,7 @@ #include "Rotation.h" #include "ShipLayout.h" #include "Simulation.h" +#include "SimulationTestAccess.h" #include "SurfaceMask.h" #include "Tick.h" @@ -524,9 +525,9 @@ TEST_CASE("Blueprint placement: buildings land at anchor + offset from cursor", const QPoint offsetA(-1, 0); const QPoint offsetB( 1, 0); - const BuildingId idA = sim.tryPlaceBuilding( + const BuildingId idA = SimulationTestAccess::place(sim, BuildingType::Belt, cursor + offsetA, Rotation::East); - const BuildingId idB = sim.tryPlaceBuilding( + const BuildingId idB = SimulationTestAccess::place(sim, BuildingType::Belt, cursor + offsetB, Rotation::East); REQUIRE(idA != kInvalidBuildingId); @@ -551,10 +552,10 @@ TEST_CASE("Blueprint placement: cost is deducted for each building in sequence", const int startBlocks = sim.buildingBlocksStock(); REQUIRE(startBlocks >= 2 * beltCost); // test config has enough starting blocks - sim.tryPlaceBuilding(BuildingType::Belt, QPoint(-6, 0), Rotation::East); + SimulationTestAccess::place(sim,BuildingType::Belt, QPoint(-6, 0), Rotation::East); REQUIRE(sim.buildingBlocksStock() == startBlocks - beltCost); - sim.tryPlaceBuilding(BuildingType::Belt, QPoint(-4, 0), Rotation::East); + SimulationTestAccess::place(sim,BuildingType::Belt, QPoint(-4, 0), Rotation::East); REQUIRE(sim.buildingBlocksStock() == startBlocks - 2 * beltCost); } @@ -576,12 +577,12 @@ TEST_CASE("Blueprint placement: insufficient blocks returns kInvalidBuildingId a int col = -2; while (sim.buildingBlocksStock() >= minerCost) { - sim.tryPlaceBuilding(BuildingType::Miner, QPoint(col, 0), Rotation::East); + SimulationTestAccess::place(sim,BuildingType::Miner, QPoint(col, 0), Rotation::East); col -= 2; } const int blocksBeforeAttempt = sim.buildingBlocksStock(); - const BuildingId id = sim.tryPlaceBuilding( + const BuildingId id = SimulationTestAccess::place(sim, BuildingType::Miner, QPoint(col - 2, 0), Rotation::East); // Placement must fail and leave the stock unchanged. @@ -598,7 +599,7 @@ TEST_CASE("Simulation: tryPlaceBuilding rejects terrain-invalid placement and ch // A miner is all-asteroid; placing it in space (x >= 0) violates the terrain // rule, so it must be rejected without consuming building blocks. const BuildingId id = - sim.tryPlaceBuilding(BuildingType::Miner, QPoint(0, 0), Rotation::East); + SimulationTestAccess::place(sim,BuildingType::Miner, QPoint(0, 0), Rotation::East); REQUIRE(id == kInvalidBuildingId); REQUIRE(sim.buildingBlocksStock() == startBlocks); @@ -621,7 +622,7 @@ TEST_CASE("Simulation: tryPlaceBuilding accepts a valid asteroid spot, occupies // Miner mask ["AA","A>"] East at (-3,0) → all-asteroid body at // (-3,0),(-2,0),(-3,1); a valid spot. const BuildingId id = - sim.tryPlaceBuilding(BuildingType::Miner, QPoint(-3, 0), Rotation::East); + SimulationTestAccess::place(sim,BuildingType::Miner, QPoint(-3, 0), Rotation::East); REQUIRE(id != kInvalidBuildingId); REQUIRE(sim.buildingBlocksStock() == startBlocks - minerCost); @@ -664,10 +665,10 @@ TEST_CASE("Blueprint placement: setRecipe on construction site stores recipe", " Simulation sim(loadConfig()); // Miner body cells: (0,0),(1,0),(0,1) — all at x < 0, valid for asteroid. - const BuildingId id = sim.tryPlaceBuilding(BuildingType::Miner, QPoint(-2, 0), Rotation::East); + const BuildingId id = SimulationTestAccess::place(sim,BuildingType::Miner, QPoint(-2, 0), Rotation::East); REQUIRE(id != kInvalidBuildingId); - sim.buildings().setRecipe(id, "mine_iron_ore"); + SimulationTestAccess::buildings(sim).setRecipe(id, "mine_iron_ore"); const ConstructionSite* site = sim.buildings().findSite(id); REQUIRE(site != nullptr); @@ -679,9 +680,9 @@ TEST_CASE("Blueprint placement: recipe transfers to building after construction { Simulation sim(loadConfig()); - const BuildingId id = sim.tryPlaceBuilding(BuildingType::Miner, QPoint(-2, 0), Rotation::East); + const BuildingId id = SimulationTestAccess::place(sim,BuildingType::Miner, QPoint(-2, 0), Rotation::East); REQUIRE(id != kInvalidBuildingId); - sim.buildings().setRecipe(id, "mine_copper_ore"); + SimulationTestAccess::buildings(sim).setRecipe(id, "mine_copper_ore"); // Miner construction_time_seconds = 10 → completesAt = secondsToTicks(10) = 300. // Run 301 ticks (0..300) to process the completion tick. @@ -759,7 +760,7 @@ TEST_CASE("Blueprint placement: setShipLayout on construction site stores layout // Shipyard surface_mask ["AAAS>","AAAS "] with Rotation::East: // A-tiles at (-3,0),(-2,0),(-1,0),(-3,1),(-2,1),(-1,1) — all x < 0, valid asteroid tiles. // S-tile at (0,0) and (0,1) — x >= 0, valid space tiles. - const BuildingId id = sim.tryPlaceBuilding(BuildingType::Shipyard, QPoint(-3, 0), Rotation::East); + const BuildingId id = SimulationTestAccess::place(sim,BuildingType::Shipyard, QPoint(-3, 0), Rotation::East); REQUIRE(id != kInvalidBuildingId); ShipLayoutConfig layout; @@ -769,7 +770,7 @@ TEST_CASE("Blueprint placement: setShipLayout on construction site stores layout pm.rotation = Rotation::East; layout.placedModules.push_back(pm); - sim.buildings().setShipLayout(id, layout); + SimulationTestAccess::buildings(sim).setShipLayout(id, layout); const ConstructionSite* site = sim.buildings().findSite(id); REQUIRE(site != nullptr); @@ -783,7 +784,7 @@ TEST_CASE("Blueprint placement: ship layout transfers to building after construc { Simulation sim(loadConfig()); - const BuildingId id = sim.tryPlaceBuilding(BuildingType::Shipyard, QPoint(-3, 0), Rotation::East); + const BuildingId id = SimulationTestAccess::place(sim,BuildingType::Shipyard, QPoint(-3, 0), Rotation::East); REQUIRE(id != kInvalidBuildingId); ShipLayoutConfig layout; @@ -793,7 +794,7 @@ TEST_CASE("Blueprint placement: ship layout transfers to building after construc pm.rotation = Rotation::North; layout.placedModules.push_back(pm); - sim.buildings().setShipLayout(id, layout); + SimulationTestAccess::buildings(sim).setShipLayout(id, layout); // Shipyard construction_time_seconds = 30 in the test config. double constructionTime = 0.0; diff --git a/src/test/CommandTest.cpp b/src/test/CommandTest.cpp index b767b83..8a43cd1 100644 --- a/src/test/CommandTest.cpp +++ b/src/test/CommandTest.cpp @@ -9,6 +9,7 @@ #include "GameConfig.h" #include "Rotation.h" #include "Simulation.h" +#include "SimulationTestAccess.h" namespace { @@ -33,7 +34,7 @@ TEST_CASE("apply(PlaceBuildingCommand) matches direct placement", "[command]") command.rotation = Rotation::East; viaCommand.apply(command); - viaDirect.tryPlaceBuilding(BuildingType::Miner, QPoint(-3, 0), Rotation::East); + SimulationTestAccess::place(viaDirect, BuildingType::Miner, QPoint(-3, 0), Rotation::East); REQUIRE(viaCommand.computeStateChecksum() == viaDirect.computeStateChecksum()); } @@ -51,8 +52,8 @@ TEST_CASE("apply(PlaceBuildingCommand) with recipe matches place-then-setRecipe" viaCommand.apply(command); const BuildingId id = - viaDirect.tryPlaceBuilding(BuildingType::Miner, QPoint(-2, 0), Rotation::East); - viaDirect.buildings().setRecipe(id, "mine_iron_ore"); + SimulationTestAccess::place(viaDirect, BuildingType::Miner, QPoint(-2, 0), Rotation::East); + SimulationTestAccess::buildings(viaDirect).setRecipe(id, "mine_iron_ore"); REQUIRE(viaCommand.computeStateChecksum() == viaDirect.computeStateChecksum()); } @@ -63,16 +64,16 @@ TEST_CASE("apply(DemolishCommand) matches direct demolish", "[command]") Simulation viaDirect(loadConfig(), 99); const BuildingId idA = - viaCommand.tryPlaceBuilding(BuildingType::Miner, QPoint(-3, 0), Rotation::East); + SimulationTestAccess::place(viaCommand, BuildingType::Miner, QPoint(-3, 0), Rotation::East); const BuildingId idB = - viaDirect.tryPlaceBuilding(BuildingType::Miner, QPoint(-3, 0), Rotation::East); + SimulationTestAccess::place(viaDirect, BuildingType::Miner, QPoint(-3, 0), Rotation::East); REQUIRE(idA == idB); DemolishCommand command; command.id = idA; viaCommand.apply(command); - viaDirect.demolish(idB); + SimulationTestAccess::demolish(viaDirect, idB); REQUIRE(viaCommand.computeStateChecksum() == viaDirect.computeStateChecksum()); } @@ -98,8 +99,8 @@ TEST_CASE("CommandManager drains queued commands in FIFO order through apply", " manager.drain(); REQUIRE_FALSE(manager.hasPending()); - viaDirect.tryPlaceBuilding(BuildingType::Miner, QPoint(-3, 0), Rotation::East); - viaDirect.tryPlaceBuilding(BuildingType::Belt, QPoint(-2, 0), Rotation::East); + SimulationTestAccess::place(viaDirect, BuildingType::Miner, QPoint(-3, 0), Rotation::East); + SimulationTestAccess::place(viaDirect, BuildingType::Belt, QPoint(-2, 0), Rotation::East); REQUIRE(viaManager.computeStateChecksum() == viaDirect.computeStateChecksum()); } diff --git a/src/test/DeterminismTest.cpp b/src/test/DeterminismTest.cpp index c7987fa..c766425 100644 --- a/src/test/DeterminismTest.cpp +++ b/src/test/DeterminismTest.cpp @@ -8,6 +8,7 @@ #include "GameConfig.h" #include "Rotation.h" #include "Simulation.h" +#include "SimulationTestAccess.h" #include "StateChecksum.h" #include "Tick.h" @@ -28,9 +29,9 @@ std::vector runScriptedSession(unsigned int seed) Simulation sim(loadConfig(), seed); // Tick 0: a miner feeding a short belt line on the asteroid. - sim.tryPlaceBuilding(BuildingType::Miner, QPoint(-3, 0), Rotation::East); - sim.tryPlaceBuilding(BuildingType::Belt, QPoint(-2, 0), Rotation::East); - sim.tryPlaceBuilding(BuildingType::Belt, QPoint(-1, 0), Rotation::East); + SimulationTestAccess::place(sim, BuildingType::Miner, QPoint(-3, 0), Rotation::East); + SimulationTestAccess::place(sim, BuildingType::Belt, QPoint(-2, 0), Rotation::East); + SimulationTestAccess::place(sim, BuildingType::Belt, QPoint(-1, 0), Rotation::East); std::vector checksums; checksums.reserve(kScriptTicks); @@ -40,7 +41,7 @@ std::vector runScriptedSession(unsigned int seed) if (t == 500) { // Demolish the second belt mid-run to exercise the removal paths. - sim.tryPlaceBuilding(BuildingType::Smelter, QPoint(-3, 3), Rotation::East); + SimulationTestAccess::place(sim, BuildingType::Smelter, QPoint(-3, 3), Rotation::East); } sim.tick(); diff --git a/src/test/RecipeSchematicTest.cpp b/src/test/RecipeSchematicTest.cpp index c48b338..53bf740 100644 --- a/src/test/RecipeSchematicTest.cpp +++ b/src/test/RecipeSchematicTest.cpp @@ -10,6 +10,7 @@ #include "RecipesConfig.h" #include "SchematicChoiceOption.h" #include "Simulation.h" +#include "SimulationTestAccess.h" #include "StationBodyComponent.h" static GameConfig loadConfig() @@ -38,7 +39,7 @@ static void killEnemyStationsAndApply(Simulation& sim) killEnemyStations(sim); if (sim.hasSchematicChoicesPending()) { - sim.applySchematicChoice(0); + SimulationTestAccess::applySchematicChoice(sim, 0); } } @@ -246,7 +247,7 @@ TEST_CASE("RecipeSchematic: recipe schematic can appear in pending choices", break; } } - sim.applySchematicChoice(0); + SimulationTestAccess::applySchematicChoice(sim, 0); } } CHECK(foundRecipeChoice); @@ -308,7 +309,7 @@ TEST_CASE("RecipeSchematic: newlyUnlockedItemNames is sorted, deduplicated, and } } - sim.applySchematicChoice(0); + SimulationTestAccess::applySchematicChoice(sim, 0); } } @@ -340,7 +341,7 @@ TEST_CASE("RecipeSchematic: newlyUnlockedItemNames matches recipes that actually const std::set unlockedBefore = unlockedTrackedRecipeIds(); const SchematicChoiceOption choice = sim.getPendingSchematicChoices()[0]; - sim.applySchematicChoice(0); + SimulationTestAccess::applySchematicChoice(sim, 0); std::set expectedNames; for (const RecipeDef& def : cfg.recipes.recipes) diff --git a/src/test/ShipModuleTest.cpp b/src/test/ShipModuleTest.cpp index 2e58efa..7c70246 100644 --- a/src/test/ShipModuleTest.cpp +++ b/src/test/ShipModuleTest.cpp @@ -17,6 +17,7 @@ #include "ShipStatsCalculator.h" #include "ShipSystem.h" #include "Simulation.h" +#include "SimulationTestAccess.h" #include "Tick.h" #include "WeaponComponent.h" @@ -62,7 +63,7 @@ static const BuildingDef* findShipyardDef(const GameConfig& cfg) static BuildingId placeShipyard(Simulation& sim, const BuildingDef& yardDef) { - return sim.buildings().placeImmediate( + return SimulationTestAccess::buildings(sim).placeImmediate( BuildingType::Shipyard, yardDef.surfaceMask, QPoint(0, 0), @@ -73,7 +74,7 @@ static void fillMaterials(Simulation& sim, BuildingId yardId, const ShipDef& def, const ShipLayoutConfig& layout) { - sim.buildings().forEachBuilding([&](Building& b) { + SimulationTestAccess::buildings(sim).forEachBuilding([&](Building& b) { if (b.id != yardId) { return; @@ -216,7 +217,7 @@ TEST_CASE("Shipyard: setShipLayout reinitializes buffers with module materials", REQUIRE(yardDef != nullptr); const BuildingId yardId = placeShipyard(sim, *yardDef); - sim.buildings().setRecipe(yardId, "interceptor"); + SimulationTestAccess::buildings(sim).setRecipe(yardId,"interceptor"); ShipLayoutConfig layout; PlacedModule pm; @@ -225,7 +226,7 @@ TEST_CASE("Shipyard: setShipLayout reinitializes buffers with module materials", pm.rotation = Rotation::East; layout.placedModules.push_back(pm); - sim.buildings().setShipLayout(yardId, layout); + SimulationTestAccess::buildings(sim).setShipLayout(yardId, layout); const Building* b = sim.buildings().findBuilding(yardId); REQUIRE(b != nullptr); @@ -245,7 +246,7 @@ TEST_CASE("Shipyard: setShipLayout cancels in-progress production", REQUIRE(yardDef != nullptr); const BuildingId yardId = placeShipyard(sim, *yardDef); - sim.buildings().setRecipe(yardId, "interceptor"); + SimulationTestAccess::buildings(sim).setRecipe(yardId,"interceptor"); // Fill materials and tick to start production. ShipLayoutConfig emptyLayout; @@ -264,7 +265,7 @@ TEST_CASE("Shipyard: setShipLayout cancels in-progress production", pm.rotation = Rotation::East; layout.placedModules.push_back(pm); - sim.buildings().setShipLayout(yardId, layout); + SimulationTestAccess::buildings(sim).setShipLayout(yardId, layout); const Building* b2 = sim.buildings().findBuilding(yardId); REQUIRE(b2 != nullptr); @@ -278,7 +279,7 @@ TEST_CASE("Shipyard: setRecipe clears ship layout", "[modules][shipyard]") REQUIRE(yardDef != nullptr); const BuildingId yardId = placeShipyard(sim, *yardDef); - sim.buildings().setRecipe(yardId, "interceptor"); + SimulationTestAccess::buildings(sim).setRecipe(yardId,"interceptor"); ShipLayoutConfig layout; PlacedModule pm; @@ -286,13 +287,13 @@ TEST_CASE("Shipyard: setRecipe clears ship layout", "[modules][shipyard]") pm.position = QPoint(0, 0); pm.rotation = Rotation::East; layout.placedModules.push_back(pm); - sim.buildings().setShipLayout(yardId, layout); + SimulationTestAccess::buildings(sim).setShipLayout(yardId, layout); const Building* b1 = sim.buildings().findBuilding(yardId); REQUIRE(b1 != nullptr); REQUIRE(b1->shipLayout.has_value()); - sim.buildings().setRecipe(yardId, "destroyer"); + SimulationTestAccess::buildings(sim).setRecipe(yardId,"destroyer"); const Building* b2 = sim.buildings().findBuilding(yardId); REQUIRE(b2 != nullptr); diff --git a/src/test/ShipyardTest.cpp b/src/test/ShipyardTest.cpp index d778852..932f0f0 100644 --- a/src/test/ShipyardTest.cpp +++ b/src/test/ShipyardTest.cpp @@ -12,6 +12,7 @@ #include "ShipIdentityComponent.h" #include "ShipSystem.h" #include "Simulation.h" +#include "SimulationTestAccess.h" #include "Tick.h" static GameConfig loadConfig() @@ -45,7 +46,7 @@ static const BuildingDef* findShipyardDef(const GameConfig& cfg) static BuildingId placeShipyard(Simulation& sim, const BuildingDef& yardDef) { - return sim.buildings().placeImmediate( + return SimulationTestAccess::buildings(sim).placeImmediate( BuildingType::Shipyard, yardDef.surfaceMask, QPoint(0, 0), @@ -62,7 +63,7 @@ static int countShips(Simulation& sim) static void fillMaterials(Simulation& sim, BuildingId yardId, const ShipDef& def) { - sim.buildings().forEachBuilding([&](Building& b) + SimulationTestAccess::buildings(sim).forEachBuilding([&](Building& b) { if (b.id != yardId) { @@ -94,7 +95,7 @@ TEST_CASE("Shipyard: spawns a player ship after production cycle completes", const BuildingId yardId = placeShipyard(sim, *yardDef); REQUIRE(yardId != kInvalidBuildingId); - sim.buildings().setRecipe(yardId, def->id); + SimulationTestAccess::buildings(sim).setRecipe(yardId, def->id); fillMaterials(sim, yardId, *def); // First tick: materials consumed, production cycle starts — no ship yet. @@ -153,7 +154,7 @@ TEST_CASE("Shipyard: does not spawn with insufficient materials", "[shipyard]") const int shipsBefore = countShips(sim); const BuildingId yardId = placeShipyard(sim, *yardDef); - sim.buildings().setRecipe(yardId, def->id); + SimulationTestAccess::buildings(sim).setRecipe(yardId, def->id); // Materials remain at zero (default after setRecipe); no cycle starts. const Tick cycleTicks = secondsToTicks(def->schematic.productionTimeSeconds); @@ -175,7 +176,7 @@ TEST_CASE("Shipyard: spawns a second ship after materials replenished", "[shipya REQUIRE(yardDef != nullptr); const BuildingId yardId = placeShipyard(sim, *yardDef); - sim.buildings().setRecipe(yardId, def->id); + SimulationTestAccess::buildings(sim).setRecipe(yardId, def->id); const Tick cycleTicks = secondsToTicks(def->schematic.productionTimeSeconds); diff --git a/src/test/SimulationTestAccess.h b/src/test/SimulationTestAccess.h new file mode 100644 index 0000000..5a31f27 --- /dev/null +++ b/src/test/SimulationTestAccess.h @@ -0,0 +1,40 @@ +#pragma once + +#include + +#include "BuildingId.h" +#include "BuildingType.h" +#include "Rotation.h" +#include "Simulation.h" + +class BeltSystem; +class BuildingSystem; + +// Test-only backdoor to Simulation's private player-action mutators and mutable +// subsystem accessors. Declared a friend of Simulation, so it can hand tests the +// same mutation surface that Simulation::apply uses internally — without exposing +// those mutators to production (UI/app) code, which must go through the command +// chokepoint (docs/replay_design.md). +// +// This header lives under src/test and is not on the lib/ui/app include path, so +// only test translation units can reach it. Non-player test setup that has no +// command equivalent (e.g. placeImmediate, forEachBuilding buffer injection) is +// reached via buildings(sim)/belts(sim). +struct SimulationTestAccess +{ + static BuildingSystem& buildings(Simulation& sim) { return sim.buildingsMutable(); } + static BeltSystem& belts(Simulation& sim) { return sim.beltsMutable(); } + + static BuildingId place(Simulation& sim, BuildingType type, QPoint anchor, + Rotation rotation) + { + return sim.tryPlaceBuilding(type, anchor, rotation); + } + + static void demolish(Simulation& sim, BuildingId id) { sim.demolish(id); } + + static void applySchematicChoice(Simulation& sim, int choiceIndex) + { + sim.applySchematicChoice(choiceIndex); + } +}; diff --git a/src/test/WaveSystemTest.cpp b/src/test/WaveSystemTest.cpp index afc1244..21e3e6e 100644 --- a/src/test/WaveSystemTest.cpp +++ b/src/test/WaveSystemTest.cpp @@ -21,6 +21,7 @@ #include "SchematicChoiceOption.h" #include "ShipsConfig.h" #include "Simulation.h" +#include "SimulationTestAccess.h" #include "Tick.h" #include "ThreatCostCalculator.h" #include "WaveSystem.h" @@ -359,7 +360,7 @@ TEST_CASE("WaveSystem: applySchematicChoice clears pending and applies", "[wave] sim.tick(); REQUIRE(sim.hasSchematicChoicesPending()); - sim.applySchematicChoice(0); + SimulationTestAccess::applySchematicChoice(sim, 0); REQUIRE_FALSE(sim.hasSchematicChoicesPending()); }