From 75c049fa6766e7c4476df30dbab5d345ad517ada Mon Sep 17 00:00:00 2001 From: Malte Langkabel Date: Thu, 20 Aug 2026 10:48:57 +0200 Subject: [PATCH] hand BuildingSystem the belts per call, as its siblings get them The last system holding a piece of the world as a member. ConstructionSystem and ProductionSystem take the transport layer as a tick argument; BuildingSystem kept a BeltSystem& from construction, so the same object arrived two different ways depending on which system you were reading. Three methods need it -- deconstruct, cancelDeconstruction and rotateInPlace, all of which register or unregister a belt tile -- and they now take it after the state, in the argument order the other systems use. The constructor is down to the config alone. CombatSystemTest's fixture kept a BeltSystem only to pass it here, so that goes too. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Ne3mejABZoLWKLh8fgpM3x --- src/balancing/ArenaSimulation.cpp | 2 +- src/lib/sim/BuildingSystem.cpp | 24 ++++++++++++----------- src/lib/sim/BuildingSystem.h | 20 +++++++++---------- src/lib/sim/Simulation.cpp | 9 +++++---- src/test/BehaviorSystemTest.cpp | 2 +- src/test/BuildingTest.cpp | 32 +++++++++++++++---------------- src/test/CombatSystemTest.cpp | 5 +---- 7 files changed, 47 insertions(+), 47 deletions(-) diff --git a/src/balancing/ArenaSimulation.cpp b/src/balancing/ArenaSimulation.cpp index 2741cc3..e7c63fa 100644 --- a/src/balancing/ArenaSimulation.cpp +++ b/src/balancing/ArenaSimulation.cpp @@ -49,7 +49,7 @@ ArenaSimulation::ArenaSimulation(const GameConfig& gameConfig, // No ProductionSystem here: the arena stages ships directly and never runs a // factory, so nothing ticks material flow (ProductionSystem.h). - m_buildingSystem = std::make_unique(m_gameConfig, m_beltSystem); + m_buildingSystem = std::make_unique(m_gameConfig); m_shipSystem = std::make_unique(m_gameConfig, m_admin); // Arena fights are symmetric and aggressive: player-faction ships must not diff --git a/src/lib/sim/BuildingSystem.cpp b/src/lib/sim/BuildingSystem.cpp index 693e0f1..00c00b8 100644 --- a/src/lib/sim/BuildingSystem.cpp +++ b/src/lib/sim/BuildingSystem.cpp @@ -10,9 +10,8 @@ #include "SurfaceMask.h" -BuildingSystem::BuildingSystem(const GameConfig& config, BeltSystem& belts) +BuildingSystem::BuildingSystem(const GameConfig& config) : m_config(config) - , m_belts(belts) { } @@ -75,7 +74,8 @@ std::optional BuildingSystem::place(FactoryState& state, BuildingTyp // Deconstruct // --------------------------------------------------------------------------- -void BuildingSystem::deconstruct(FactoryState& state, BuildingId id, Tick currentTick) +void BuildingSystem::deconstruct(FactoryState& state, BeltSystem& belts, BuildingId id, + Tick currentTick) { // Construction site? Removed instantly with the full refund; never queued // for deconstruction (REQ-BLD-DECONSTRUCT). @@ -116,7 +116,7 @@ void BuildingSystem::deconstruct(FactoryState& state, BuildingId id, Tick curren if (building.type == BuildingType::Splitter) { if (const std::optional info = - m_belts.getSplitterInfo(building.anchor)) + belts.getSplitterInfo(building.anchor)) { entry.splitterFilterA = info->filterA; entry.splitterFilterB = info->filterB; @@ -124,7 +124,7 @@ void BuildingSystem::deconstruct(FactoryState& state, BuildingId id, Tick curren } if (isBeltSubsystemType(building.type)) { - m_belts.removeTile(building.anchor); + belts.removeTile(building.anchor); } const bool wasEmpty = state.deconstructionQueue.empty(); @@ -274,7 +274,8 @@ void BuildingSystem::setSiteSplitterFilters(FactoryState& state, BuildingId id, // Tick hooks // --------------------------------------------------------------------------- -void BuildingSystem::cancelDeconstruction(FactoryState& state, BuildingId id) +void BuildingSystem::cancelDeconstruction(FactoryState& state, BeltSystem& belts, + BuildingId id) { for (std::deque::iterator it = state.deconstructionQueue.begin(); it != state.deconstructionQueue.end(); @@ -288,7 +289,7 @@ void BuildingSystem::cancelDeconstruction(FactoryState& state, BuildingId id) if (Building* building = findBuilding(state, id)) { building->queuedForDeconstruction = false; - reregisterBeltTile(m_belts, m_config, *building, it->splitterFilterA, it->splitterFilterB); + reregisterBeltTile(belts, m_config, *building, it->splitterFilterA, it->splitterFilterB); } state.deconstructionQueue.erase(it); @@ -298,7 +299,8 @@ void BuildingSystem::cancelDeconstruction(FactoryState& state, BuildingId id) } } -void BuildingSystem::rotateInPlace(FactoryState& state, BuildingId id, Rotation newRotation) +void BuildingSystem::rotateInPlace(FactoryState& state, BeltSystem& belts, BuildingId id, + Rotation newRotation) { // Construction site path — just update rotation; no ports to recompute. for (ConstructionSite& site : state.constructionQueue) @@ -348,15 +350,15 @@ void BuildingSystem::rotateInPlace(FactoryState& state, BuildingId id, Rotation if (b.type == BuildingType::Splitter) { if (const std::optional info = - m_belts.getSplitterInfo(b.anchor)) + belts.getSplitterInfo(b.anchor)) { splitterFilterA = info->filterA; splitterFilterB = info->filterB; } } - m_belts.removeTile(b.anchor); - reregisterBeltTile(m_belts, m_config, b, splitterFilterA, splitterFilterB); + belts.removeTile(b.anchor); + reregisterBeltTile(belts, m_config, b, splitterFilterA, splitterFilterB); } return; diff --git a/src/lib/sim/BuildingSystem.h b/src/lib/sim/BuildingSystem.h index 7025397..cc6ff78 100644 --- a/src/lib/sim/BuildingSystem.h +++ b/src/lib/sim/BuildingSystem.h @@ -36,7 +36,7 @@ class BuildingSystem { public: - BuildingSystem(const GameConfig& config, BeltSystem& belts); + explicit BuildingSystem(const GameConfig& config); // -- Placement / deconstruct ------------------------------------------------ // Returns the new entity id, or nullopt if the placement falls outside the @@ -60,13 +60,14 @@ public: // queue (REQ-BLD-DECON-QUEUE) and stops operating at once; its (partial) refund is // credited later, on completion, by DeconstructionSystem. No-op for unknown ids and // for a building already queued. - void deconstruct(FactoryState& state, BuildingId id, Tick currentTick); + void deconstruct(FactoryState& state, BeltSystem& belts, BuildingId id, + Tick currentTick); // Take a building back out of the deconstruction queue before it is removed // (REQ-BLD-DECON-QUEUE). Clears its queued flag and resumes operation // (re-registering belt/tunnel/splitter tiles); discards deconstruction // progress and credits no refund. No-op if the id is not queued. - void cancelDeconstruction(FactoryState& state, BuildingId id); + void cancelDeconstruction(FactoryState& state, BeltSystem& belts, BuildingId id); // Set the recipe (or schematic id for shipyard) on a building or queued // construction site. Clears both buffers on an operational building. @@ -99,7 +100,8 @@ public: // Rotate an existing building or construction site to newRotation in place. // For belt-type operational buildings, re-registers with BeltSystem (items // currently on the tile are discarded by BeltSystem::removeTile). - void rotateInPlace(FactoryState& state, BuildingId id, Rotation newRotation); + void rotateInPlace(FactoryState& state, BeltSystem& belts, BuildingId id, + Rotation newRotation); // Register / unregister tile occupancy for ECS station entities. void registerTileOccupancy(FactoryState& state, const std::vector& cells, BuildingId ownerPlaceholder); @@ -116,11 +118,9 @@ public: void forEachBuilding(FactoryState& state, std::function fn); private: - // No world data here: the factory arrives per call (FactoryState.h). The config says - // what a building costs, occupies and can run; the belts are what a placed, rotated or - // demolished belt tile must be registered with and unregistered from. Nothing else -- - // no RNG and no callbacks, those having gone to ProductionSystem with the material - // flow that needed them. + // The config alone: what a building costs, occupies and can run. No world data -- + // the factory and the transport layer both arrive per call, as they do for + // ConstructionSystem and ProductionSystem (FactoryState.h) -- and no RNG or callbacks, + // those having gone to ProductionSystem with the material flow that needed them. const GameConfig& m_config; - BeltSystem& m_belts; }; diff --git a/src/lib/sim/Simulation.cpp b/src/lib/sim/Simulation.cpp index 90bd8f4..8b95951 100644 --- a/src/lib/sim/Simulation.cpp +++ b/src/lib/sim/Simulation.cpp @@ -108,7 +108,7 @@ void Simulation::reset(unsigned int seed) void Simulation::initializeSubsystems() { - m_buildingSystem = std::make_unique(m_config, m_beltSystem); + m_buildingSystem = std::make_unique(m_config); m_productionSystem = std::make_unique( m_config, [this](const std::string& id, QVector2D pos, @@ -174,7 +174,8 @@ void Simulation::apply(const Command& command) case CommandKind::RotateInPlace: { const RotateInPlaceCommand& c = static_cast(command); - m_buildingSystem->rotateInPlace(m_factoryState, *c.id, c.newRotation); + m_buildingSystem->rotateInPlace(m_factoryState, m_beltSystem, *c.id, + c.newRotation); break; } case CommandKind::SetRecipe: @@ -888,12 +889,12 @@ std::optional Simulation::tryPlaceBuilding(BuildingType type, QPoint void Simulation::deconstruct(BuildingId id) { - m_buildingSystem->deconstruct(m_factoryState, id, m_currentTick); + m_buildingSystem->deconstruct(m_factoryState, m_beltSystem, id, m_currentTick); } void Simulation::cancelDeconstruction(BuildingId id) { - m_buildingSystem->cancelDeconstruction(m_factoryState, id); + m_buildingSystem->cancelDeconstruction(m_factoryState, m_beltSystem, id); } BuildingSystem& Simulation::getBuildingsMutable() diff --git a/src/test/BehaviorSystemTest.cpp b/src/test/BehaviorSystemTest.cpp index fdfcc17..d66f2e0 100644 --- a/src/test/BehaviorSystemTest.cpp +++ b/src/test/BehaviorSystemTest.cpp @@ -77,7 +77,7 @@ struct Fixture : cfg(loadTestConfig()) , belts(cfg.world.beltSpeed_tps) , rng(42) - , buildings(cfg, belts) + , buildings(cfg) , construction(cfg) , ships(cfg, admin) , ai(cfg) diff --git a/src/test/BuildingTest.cpp b/src/test/BuildingTest.cpp index 1479b70..d64dbcd 100644 --- a/src/test/BuildingTest.cpp +++ b/src/test/BuildingTest.cpp @@ -118,7 +118,7 @@ struct PlacementFixture std::optional beltSpeed_tps = std::nullopt, std::function isItemUnlocked = nullptr) : belts(beltSpeed_tps.value_or(cfg.world.beltSpeed_tps)) - , bs(cfg, belts) + , bs(cfg) , production(cfg, [](const std::string&, QVector2D, const std::optional&) {}, isItemUnlocked ? std::move(isItemUnlocked) @@ -289,7 +289,7 @@ TEST_CASE("BuildingSystem: deconstructing a construction site removes it instant // Still queued for construction (not yet built): instant removal, full cost // refunded immediately, never entering the deconstruction queue (REQ-BLD-DECONSTRUCT). - f.bs.deconstruct(f.state, id, 0); + f.bs.deconstruct(f.state, f.belts, id, 0); REQUIRE(f.getRefundedBlocks() == 15); // Miner cost = 15 REQUIRE_FALSE(isTileOccupied(f.state, QPoint(0, 0))); @@ -362,7 +362,7 @@ TEST_CASE("BuildingSystem: deconstructing a built building queues it; refund cre // Deconstructing a built building returns nothing immediately and queues it, // stopping it operating while its tiles stay occupied (REQ-BLD-DECON-QUEUE). - f.bs.deconstruct(f.state, id, tick); + f.bs.deconstruct(f.state, f.belts, id, tick); REQUIRE(isQueuedForDeconstruction(f.state, id)); REQUIRE(isTileOccupied(f.state, QPoint(0, 0))); REQUIRE(f.getRefundedBlocks() == 0); @@ -386,8 +386,8 @@ TEST_CASE("BuildingSystem: deconstruction queue removes one building at a time", runUntilBuilt(f, b, tick); // Queue both in one tick; 'a' is at the front of the deconstruction queue. - f.bs.deconstruct(f.state, a, tick); - f.bs.deconstruct(f.state, b, tick); + f.bs.deconstruct(f.state, f.belts, a, tick); + f.bs.deconstruct(f.state, f.belts, b, tick); REQUIRE(isQueuedForDeconstruction(f.state, a)); REQUIRE(isQueuedForDeconstruction(f.state, b)); @@ -415,11 +415,11 @@ TEST_CASE("BuildingSystem: cancelling deconstruction resumes the building with n Tick tick = 0; runUntilBuilt(f, id, tick); - f.bs.deconstruct(f.state, id, tick); + f.bs.deconstruct(f.state, f.belts, id, tick); REQUIRE(isQueuedForDeconstruction(f.state, id)); // Un-queue before it drains: it operates again, no refund, tiles still occupied. - f.bs.cancelDeconstruction(f.state, id); + f.bs.cancelDeconstruction(f.state, f.belts, id); REQUIRE_FALSE(isQueuedForDeconstruction(f.state, id)); REQUIRE(findBuilding(f.state, id) != nullptr); REQUIRE(isTileOccupied(f.state, QPoint(0, 0))); @@ -443,12 +443,12 @@ TEST_CASE("BuildingSystem: queued belt stops transporting; cancel restores it", // Queuing a belt unregisters its tile from the belt subsystem, so it no longer // accepts or transports items, though the tile stays occupied (REQ-BLD-DECON-QUEUE). - f.bs.deconstruct(f.state, id, tick); + f.bs.deconstruct(f.state, f.belts, id, tick); REQUIRE_FALSE(f.belts.tryPutItem(QPoint(0, 0), makeItem("iron_ore"), Rotation::East)); REQUIRE(isTileOccupied(f.state, QPoint(0, 0))); // Un-queuing re-registers the belt tile so it transports again. - f.bs.cancelDeconstruction(f.state, id); + f.bs.cancelDeconstruction(f.state, f.belts, id); REQUIRE(f.belts.tryPutItem(QPoint(0, 0), makeItem("iron_ore"), Rotation::East)); } @@ -465,10 +465,10 @@ TEST_CASE("BuildingSystem: splitter filters survive a queue/un-queue round-trip" // Queue: the belt subsystem tile (and its filters) are unregistered, but the // filters are captured so an un-queue can restore them. - f.bs.deconstruct(f.state, id, tick); + f.bs.deconstruct(f.state, f.belts, id, tick); REQUIRE_FALSE(f.belts.getSplitterInfo(QPoint(0, 0)).has_value()); - f.bs.cancelDeconstruction(f.state, id); + f.bs.cancelDeconstruction(f.state, f.belts, id); const std::optional info = f.belts.getSplitterInfo(QPoint(0, 0)); REQUIRE(info.has_value()); REQUIRE(info->filterA.size() == 1); @@ -1782,7 +1782,7 @@ TEST_CASE("BuildingSystem: rotateInPlace updates the rotation field of a constru const BuildingId id = f.bs.place(f.state, BuildingType::Belt, QPoint(0, 0), Rotation::East, 0).value(); REQUIRE(findSite(f.state, id)->rotation == Rotation::East); - f.bs.rotateInPlace(f.state, id, Rotation::North); + f.bs.rotateInPlace(f.state, f.belts, id, Rotation::North); REQUIRE(findSite(f.state, id)->rotation == Rotation::North); } @@ -1796,7 +1796,7 @@ TEST_CASE("BuildingSystem: rotateInPlace preserves the construction progress of const Tick completesAt = findSite(f.state, id)->completesAt; REQUIRE(completesAt > 0); - f.bs.rotateInPlace(f.state, id, Rotation::South); + f.bs.rotateInPlace(f.state, f.belts, id, Rotation::South); REQUIRE(findSite(f.state, id)->completesAt == completesAt); } @@ -1815,7 +1815,7 @@ TEST_CASE("BuildingSystem: rotateInPlace updates rotation and output port direct const Building& before = *findBuilding(f.state, id); REQUIRE(before.outputPorts[0].direction == Rotation::East); - f.bs.rotateInPlace(f.state, id, Rotation::North); + f.bs.rotateInPlace(f.state, f.belts, id, Rotation::North); const Building& after = *findBuilding(f.state, id); REQUIRE(after.rotation == Rotation::North); @@ -1832,7 +1832,7 @@ TEST_CASE("BuildingSystem: rotateInPlace re-registers a belt tile with BeltSyste Tick tick = 0; runTicks(f.production, f.cfg, f.state, f.belts, static_cast(secondsToTicks(1.0)) + 1, tick); - f.bs.rotateInPlace(f.state, id, Rotation::North); + f.bs.rotateInPlace(f.state, f.belts, id, Rotation::North); // Belt tile must still be registered after rotation — items can be placed on it. REQUIRE(f.belts.tryPutItem(QPoint(0, 0), makeItem("iron_ore"))); @@ -1858,7 +1858,7 @@ TEST_CASE("BuildingSystem: rotateInPlace preserves the output filters of a split const std::vector filterB{ ItemType{"copper_ore"} }; f.belts.setSplitterFilters(tile, filterA, filterB); - f.bs.rotateInPlace(f.state, id, Rotation::North); + f.bs.rotateInPlace(f.state, f.belts, id, Rotation::North); // The tile is re-registered with BeltSystem carrying the filters it had before // the rotation — rotating must not reset a configured splitter to "accept all". diff --git a/src/test/CombatSystemTest.cpp b/src/test/CombatSystemTest.cpp index ea321db..68a358a 100644 --- a/src/test/CombatSystemTest.cpp +++ b/src/test/CombatSystemTest.cpp @@ -2,7 +2,6 @@ #include -#include "BeltSystem.h" #include "Building.h" #include "BuildingSystem.h" #include "FactoryState.h" @@ -55,7 +54,6 @@ struct CombatFixture FactoryState state = makeFactoryState(cfg); std::mt19937 rng; EntityAdmin admin; - BeltSystem belts; ShipSystem ships; BuildingSystem buildings; CombatSystem combat; @@ -63,9 +61,8 @@ struct CombatFixture explicit CombatFixture() : cfg(loadTestConfig()) , rng(42) - , belts(cfg.world.beltSpeed_tps) , ships(cfg, admin) - , buildings(cfg, belts) + , buildings(cfg) , combat(cfg) { }