diff --git a/src/lib/sim/BuildingSystem.cpp b/src/lib/sim/BuildingSystem.cpp index 09348c3..294fc59 100644 --- a/src/lib/sim/BuildingSystem.cpp +++ b/src/lib/sim/BuildingSystem.cpp @@ -284,6 +284,21 @@ void BuildingSystem::setShipLayout(FactoryState& state, BuildingId id, const Shi { if (building.id == id) { + // No-op if the layout is unchanged, so re-applying the layout a shipyard + // already has does not cancel its production cycle or wipe its buffers + // (REQ-MAT-INPUT-BUFFER, REQ-BLD-SHIPYARD). Confirming the layout dialog + // without editing anything, and a blueprint configuration transfer onto an + // already-matching shipyard (REQ-UI-BLUEPRINT-TRANSFER), both land here. + // An unset layout counts as an empty one: the two are equivalent for + // buffers, production, and the spawned ship (see the spawn path below), + // so an empty layout arriving at an unconfigured shipyard changes nothing. + const bool unchanged = building.shipLayout.has_value() + ? *building.shipLayout == layout + : layout.placedModules.empty(); + if (unchanged) + { + return; + } if (building.production.has_value()) { building.production = std::nullopt; diff --git a/src/lib/sim/ShipLayout.h b/src/lib/sim/ShipLayout.h index f616f5a..4d9159d 100644 --- a/src/lib/sim/ShipLayout.h +++ b/src/lib/sim/ShipLayout.h @@ -15,8 +15,23 @@ struct PlacedModule Rotation rotation; }; +inline bool operator==(const PlacedModule& a, const PlacedModule& b) +{ + return a.moduleId == b.moduleId && a.position == b.position && a.rotation == b.rotation; +} + // The complete module configuration for a shipyard's current ship (REQ-MOD-CONFIG). struct ShipLayoutConfig { std::vector placedModules; }; + +// Deliberately order-sensitive: two layouts holding the same modules in a different +// vector order compare unequal. This only decides whether applying a layout is a no-op +// (REQ-MAT-INPUT-BUFFER), so the conservative answer is the safe one -- reporting a +// change that is not one costs a buffer reset, reporting no change when there is one +// would leave the shipyard stale. +inline bool operator==(const ShipLayoutConfig& a, const ShipLayoutConfig& b) +{ + return a.placedModules == b.placedModules; +} diff --git a/src/test/BuildingConfigTest.cpp b/src/test/BuildingConfigTest.cpp index 0b6c8a1..9aae77d 100644 --- a/src/test/BuildingConfigTest.cpp +++ b/src/test/BuildingConfigTest.cpp @@ -103,14 +103,25 @@ TEST_CASE("readBuildingConfig returns a shipyard's schematic and layout", const BuildingId id = placeOperational(sim, cfg, BuildingType::Shipyard, QPoint(0, 0)); SimulationTestAccess::buildings(sim).setRecipe(SimulationTestAccess::state(sim), id, schematic->id); - SimulationTestAccess::buildings(sim).setShipLayout(SimulationTestAccess::state(sim), id, ShipLayoutConfig{}); + + // A real layout, not an empty one: applying an empty layout to a shipyard that has + // none is a no-op (REQ-MAT-INPUT-BUFFER), so it would leave nothing to read back. + ShipLayoutConfig layout; + PlacedModule placed; + placed.moduleId = "armor_plate"; + placed.position = QPoint(0, 0); + placed.rotation = Rotation::East; + layout.placedModules.push_back(placed); + SimulationTestAccess::buildings(sim).setShipLayout(SimulationTestAccess::state(sim), id, layout); const std::optional config = readBuildingConfig(sim, id); REQUIRE(config.has_value()); CHECK(config->type == BuildingType::Shipyard); REQUIRE(config->recipeId.has_value()); CHECK(*config->recipeId == schematic->id); - CHECK(config->shipLayout.has_value()); + REQUIRE(config->shipLayout.has_value()); + REQUIRE(config->shipLayout->placedModules.size() == 1); + CHECK(config->shipLayout->placedModules[0].moduleId == "armor_plate"); } TEST_CASE("readBuildingConfig reads a queued construction site", "[blueprint]") diff --git a/src/test/ShipModuleTest.cpp b/src/test/ShipModuleTest.cpp index b24dc35..ee0caae 100644 --- a/src/test/ShipModuleTest.cpp +++ b/src/test/ShipModuleTest.cpp @@ -262,6 +262,123 @@ TEST_CASE("Shipyard: setShipLayout cancels in-progress production", CHECK_FALSE(b2->production.has_value()); } +// Applying a configuration a building already has is a no-op (REQ-MAT-INPUT-BUFFER). +// setRecipe has always worked this way; setShipLayout did not, and wiped the shipyard +// on every re-apply. That matters now that a blueprint configuration transfer +// (REQ-UI-BLUEPRINT-TRANSFER) can be clicked repeatedly onto matching shipyards. + +TEST_CASE("Shipyard: re-applying the same layout keeps production and buffers", + "[modules][shipyard]") +{ + Simulation sim(loadTestConfig(), 42); + const ShipDef* def = findSchematic(sim.getConfig(), "interceptor"); + REQUIRE(def != nullptr); + const BuildingDef* yardDef = findShipyardDef(sim.getConfig()); + REQUIRE(yardDef != nullptr); + + const BuildingId yardId = placeShipyard(sim, *yardDef); + SimulationTestAccess::buildings(sim).setRecipe(SimulationTestAccess::state(sim), yardId, "interceptor"); + + ShipLayoutConfig layout; + PlacedModule pm; + pm.moduleId = "armor_plate"; + pm.position = QPoint(0, 0); + pm.rotation = Rotation::East; + layout.placedModules.push_back(pm); + + SimulationTestAccess::buildings(sim).setShipLayout(SimulationTestAccess::state(sim), yardId, layout); + fillMaterials(sim, yardId, *def, layout); + sim.tick(); + + const Building* before = findBuilding(sim.getFactoryState(), yardId); + REQUIRE(before != nullptr); + REQUIRE(before->production.has_value()); + const Tick completesAt = before->production->completesAt; + + // A separately built but equal layout: equality is by value, not by identity. + ShipLayoutConfig sameLayout; + sameLayout.placedModules.push_back(pm); + SimulationTestAccess::buildings(sim).setShipLayout(SimulationTestAccess::state(sim), yardId, sameLayout); + + const Building* after = findBuilding(sim.getFactoryState(), yardId); + REQUIRE(after != nullptr); + REQUIRE(after->production.has_value()); + CHECK(after->production->completesAt == completesAt); + CHECK(after->inputBuffer.caps.at(ItemType{"iron_ingot"}) == 10); +} + +TEST_CASE("Shipyard: an empty layout on an unconfigured shipyard changes nothing", + "[modules][shipyard]") +{ + // An unset layout and an empty one are the same state everywhere that matters, so + // clearing an already-unconfigured shipyard must not cancel its cycle. This is the + // case a transfer from a layout-less source shipyard produces. + Simulation sim(loadTestConfig(), 42); + const ShipDef* def = findSchematic(sim.getConfig(), "interceptor"); + REQUIRE(def != nullptr); + const BuildingDef* yardDef = findShipyardDef(sim.getConfig()); + REQUIRE(yardDef != nullptr); + + const BuildingId yardId = placeShipyard(sim, *yardDef); + SimulationTestAccess::buildings(sim).setRecipe(SimulationTestAccess::state(sim), yardId, "interceptor"); + + ShipLayoutConfig emptyLayout; + fillMaterials(sim, yardId, *def, emptyLayout); + sim.tick(); + + const Building* before = findBuilding(sim.getFactoryState(), yardId); + REQUIRE(before != nullptr); + REQUIRE(before->production.has_value()); + REQUIRE_FALSE(before->shipLayout.has_value()); + + SimulationTestAccess::buildings(sim).setShipLayout(SimulationTestAccess::state(sim), yardId, emptyLayout); + + const Building* after = findBuilding(sim.getFactoryState(), yardId); + REQUIRE(after != nullptr); + CHECK(after->production.has_value()); + CHECK_FALSE(after->shipLayout.has_value()); +} + +TEST_CASE("Shipyard: the same modules in a different order count as a change", + "[modules][shipyard]") +{ + // Layout equality is deliberately order-sensitive (see ShipLayout.h): the safe + // answer when in doubt is "changed", which costs a buffer reset rather than + // leaving a shipyard building the wrong ship. + Simulation sim(loadTestConfig(), 42); + const ShipDef* def = findSchematic(sim.getConfig(), "interceptor"); + REQUIRE(def != nullptr); + const BuildingDef* yardDef = findShipyardDef(sim.getConfig()); + REQUIRE(yardDef != nullptr); + + const BuildingId yardId = placeShipyard(sim, *yardDef); + SimulationTestAccess::buildings(sim).setRecipe(SimulationTestAccess::state(sim), yardId, "interceptor"); + + PlacedModule armor; + armor.moduleId = "armor_plate"; + armor.position = QPoint(0, 0); + armor.rotation = Rotation::East; + PlacedModule sensor; + sensor.moduleId = "sensor_booster"; + sensor.position = QPoint(1, 0); + sensor.rotation = Rotation::East; + + ShipLayoutConfig layout; + layout.placedModules.push_back(armor); + layout.placedModules.push_back(sensor); + SimulationTestAccess::buildings(sim).setShipLayout(SimulationTestAccess::state(sim), yardId, layout); + fillMaterials(sim, yardId, *def, layout); + sim.tick(); + REQUIRE(findBuilding(sim.getFactoryState(), yardId)->production.has_value()); + + ShipLayoutConfig reordered; + reordered.placedModules.push_back(sensor); + reordered.placedModules.push_back(armor); + SimulationTestAccess::buildings(sim).setShipLayout(SimulationTestAccess::state(sim), yardId, reordered); + + CHECK_FALSE(findBuilding(sim.getFactoryState(), yardId)->production.has_value()); +} + TEST_CASE("Shipyard: builds a bare hull when no layout is configured", "[modules][shipyard]") {