fix bug where selecting the same layout for a shipyard discarded the current progress and buffers

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;

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<PlacedModule> 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;
+}

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<BuildingConfig> 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]")

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]")
 {