make re-applying an unchanged ship layout a no-op

setRecipe has always returned early when the recipe is unchanged, so a
redundant selection does not reset buffers. setShipLayout had no such
guard: it cancelled the production cycle and wiped every buffer on any
call, including one that set the layout the shipyard already had. That
already contradicted REQ-BLD-SHIPYARD, which cancels a cycle only when the
player "confirms a layout change", and it becomes load-bearing for the
blueprint configuration transfer of REQ-UI-BLUEPRINT-TRANSFER, which is
meant to be clicked repeatedly onto matching shipyards.

REQ-MAT-INPUT-BUFFER now states the rule generally: setting a
configuration to the value it already holds changes nothing, whatever
path applies it.

An unset layout counts as an empty one. The two are equivalent for
buffers, production, and the spawned ship -- the spawn path already
converts nullopt to an empty layout deliberately -- so clearing an
already-unconfigured shipyard is a no-op too. That is the case a transfer
from a layout-less source produces. The only place the two differ is the
has_value() bit in StateChecksum, and record and replay take the same
branch, so determinism is unaffected.

Layout equality is order-sensitive by choice: reporting a change that is
not one costs a buffer reset, while missing a real change would leave a
shipyard building the wrong ship.

BuildingConfigTest's shipyard case used an empty layout as shorthand for
"a layout is set", which is now a no-op and left nothing to read back. It
configures a real module instead and checks it round-trips.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JcReq7hVk4KUPhTDKWAG7K
This commit is contained in:
2026-08-06 17:37:57 +02:00
parent 128bf81467
commit 250da8c9aa
4 changed files with 160 additions and 2 deletions

View File

@@ -284,6 +284,21 @@ void BuildingSystem::setShipLayout(FactoryState& state, BuildingId id, const Shi
{ {
if (building.id == id) 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()) if (building.production.has_value())
{ {
building.production = std::nullopt; building.production = std::nullopt;

View File

@@ -15,8 +15,23 @@ struct PlacedModule
Rotation rotation; 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). // The complete module configuration for a shipyard's current ship (REQ-MOD-CONFIG).
struct ShipLayoutConfig struct ShipLayoutConfig
{ {
std::vector<PlacedModule> placedModules; 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;
}

View File

@@ -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)); const BuildingId id = placeOperational(sim, cfg, BuildingType::Shipyard, QPoint(0, 0));
SimulationTestAccess::buildings(sim).setRecipe(SimulationTestAccess::state(sim), id, schematic->id); 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); const std::optional<BuildingConfig> config = readBuildingConfig(sim, id);
REQUIRE(config.has_value()); REQUIRE(config.has_value());
CHECK(config->type == BuildingType::Shipyard); CHECK(config->type == BuildingType::Shipyard);
REQUIRE(config->recipeId.has_value()); REQUIRE(config->recipeId.has_value());
CHECK(*config->recipeId == schematic->id); 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]") TEST_CASE("readBuildingConfig reads a queued construction site", "[blueprint]")

View File

@@ -262,6 +262,123 @@ TEST_CASE("Shipyard: setShipLayout cancels in-progress production",
CHECK_FALSE(b2->production.has_value()); 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", TEST_CASE("Shipyard: builds a bare hull when no layout is configured",
"[modules][shipyard]") "[modules][shipyard]")
{ {