diff --git a/docs/requirements.md b/docs/requirements.md index 58f3c99..416e81d 100644 --- a/docs/requirements.md +++ b/docs/requirements.md @@ -325,7 +325,7 @@ Any ship, module, building, or assembler recipe id that appears in no unlock gro - REQ-MOD-UI-EMPTY-PULSE: While a module is selected for placement in the layout configuration dialog (REQ-MOD-UI-DIALOG), the empty buildable cells of the layout grid pulse smoothly around their normal fill shade, oscillating between a slightly darker and a slightly brighter shade at approximately 1 Hz (one full cycle per second), to draw the player's attention to where the module can be placed. All empty buildable cells pulse in phase. When no module is selected for placement (including remove mode), empty buildable cells render at their normal static shade. Non-buildable cells and cells occupied by a placed module do not pulse. -- REQ-MOD-UI-AUTO-DIALOG: When the player selects a schematic for a shipyard (operational building or construction site) through the schematic selection dialog (REQ-UI-SELECT-BUTTON), and the chosen schematic **differs** from the shipyard's current schematic, the layout configuration dialog (REQ-MOD-UI-DIALOG) opens automatically and immediately once the selection dialog closes — exactly as if the player had then clicked "Configure". Re-selecting the schematic already set does not reopen the dialog. This auto-open applies only to the manual schematic selection dialog; a schematic applied by blueprint placement (REQ-UI-BLUEPRINT-PLACE) does **not** auto-open the dialog. The player may still cancel the auto-opened dialog (REQ-MOD-UI-DIALOG), which leaves the newly selected schematic in place with its default empty layout; the "Configure" button (REQ-MOD-UI-PREVIEW) remains available to open the dialog again later. +- REQ-MOD-UI-AUTO-DIALOG: When the player selects a schematic for a shipyard (operational building or construction site) through the schematic selection dialog (REQ-UI-SELECT-BUTTON), and the chosen schematic **differs** from the shipyard's current schematic, the layout configuration dialog (REQ-MOD-UI-DIALOG) opens automatically and immediately once the selection dialog closes — exactly as if the player had then clicked "Configure". Re-selecting the schematic already set does not reopen the dialog. Neither does **clearing** the shipyard: the `(None)` option (REQ-UI-SELECT-OPTIONS) differs from whatever was set, but it names no schematic and leaves nothing to configure, so no dialog opens — the same condition under which the "Configure" button is disabled (REQ-MOD-UI-PREVIEW). This auto-open applies only to the manual schematic selection dialog; a schematic applied by blueprint placement (REQ-UI-BLUEPRINT-PLACE) does **not** auto-open the dialog. The player may still cancel the auto-opened dialog (REQ-MOD-UI-DIALOG), which leaves the newly selected schematic in place with its default empty layout; the "Configure" button (REQ-MOD-UI-PREVIEW) remains available to open the dialog again later. - REQ-MOD-UI-MODULE-TOOLTIP: Each module selection button in the layout configuration dialog (REQ-MOD-UI-DIALOG) shows a hover tooltip with the descriptive text defined for that module type in `modules.toml` (the optional per-module tooltip field). If a module type defines no tooltip text, its button shows no tooltip. The "Remove" button is not a module type and has no config-defined tooltip. diff --git a/src/lib/config/ShipsConfig.h b/src/lib/config/ShipsConfig.h index 28cae28..fe548ef 100644 --- a/src/lib/config/ShipsConfig.h +++ b/src/lib/config/ShipsConfig.h @@ -63,4 +63,19 @@ struct ShipsConfig } return nullptr; } + + // The definition to configure a layout against, or nullptr when there is no layout + // to configure: no schematic is selected at all (the empty id the "(None)" option + // sets, REQ-UI-SELECT-OPTIONS), the id names no ship, or the ship defines no layout + // grid. Every entry to the layout configuration dialog asks this before opening it + // and the panel asks it before offering the button that opens it, so the dialog + // cannot appear over a grid with no cells (REQ-MOD-UI-DIALOG, REQ-MOD-UI-PREVIEW, + // REQ-MOD-UI-AUTO-DIALOG). + const ShipDef* findLayoutShipDef(const std::string& id) const + { + if (id.empty()) { return nullptr; } + const ShipDef* def = findShipDef(id); + if (!def || def->layout.empty()) { return nullptr; } + return def; + } }; diff --git a/src/test/ShipModuleTest.cpp b/src/test/ShipModuleTest.cpp index ee0caae..8dcbe54 100644 --- a/src/test/ShipModuleTest.cpp +++ b/src/test/ShipModuleTest.cpp @@ -690,3 +690,31 @@ TEST_CASE("calculateShipStats: maneuvering_thrusters additive maneuvering_accele const float expected = (base_mpss + 10.0f) / tileSize; CHECK(stats.maneuveringAcceleration_tpss == Approx(expected)); } + +// The precondition every entry to the layout configuration dialog checks +// (REQ-MOD-UI-DIALOG, REQ-MOD-UI-AUTO-DIALOG, REQ-MOD-UI-PREVIEW). The empty id is the +// one that used to slip through: "(None)" clears a shipyard and differs from whatever +// was set, which is not the same as naming a schematic to configure. +TEST_CASE("ShipsConfig: a layout is configurable only for a ship that has one", + "[modules][config]") +{ + const GameConfig cfg = loadTestConfig(); + + const ShipDef* interceptor = cfg.ships.findLayoutShipDef("interceptor"); + REQUIRE(interceptor != nullptr); + CHECK(interceptor->id == "interceptor"); + CHECK_FALSE(interceptor->layout.empty()); + + // The "(None)" option's id, and an id naming no ship at all. + CHECK(cfg.ships.findLayoutShipDef("") == nullptr); + CHECK(cfg.ships.findLayoutShipDef("no_such_ship") == nullptr); + + // A ship that exists but defines no grid has nothing to place modules on either, + // where plain findShipDef still finds it. + ShipsConfig gridless; + ShipDef def; + def.id = "hull_only"; + gridless.ships.push_back(def); + CHECK(gridless.findShipDef("hull_only") != nullptr); + CHECK(gridless.findLayoutShipDef("hull_only") == nullptr); +} diff --git a/src/ui/MainWindow.cpp b/src/ui/MainWindow.cpp index 17c3974..3c6f855 100644 --- a/src/ui/MainWindow.cpp +++ b/src/ui/MainWindow.cpp @@ -390,6 +390,15 @@ void MainWindow::handleEvent(std::shared_ptr e } const std::string& schematicId = b ? b->recipeId : s->recipeId; + // Nothing to configure without a schematic and a grid to place modules on. The + // Configure button that publishes this is already disabled then (REQ-MOD-UI-PREVIEW), + // so this guards the event rather than the button: the dialog would otherwise open + // over a grid of no cells. + if (!m_sim->getConfig().ships.findLayoutShipDef(schematicId)) + { + return; + } + const std::optional& layoutOpt = b ? b->shipLayout : s->shipLayout; @@ -445,8 +454,12 @@ void MainWindow::handleEvent(std::shared_ptr(command)); // REQ-MOD-UI-AUTO-DIALOG: picking a new schematic for a shipyard opens the - // layout configuration dialog immediately. Only on an actual change. - if (type == BuildingType::Shipyard && *dialog.getChosenId() != oldSchematic) + // layout configuration dialog immediately. Only on an actual change, and only + // for an actual schematic: "(None)" clears the shipyard and carries the empty + // id (REQ-UI-SELECT-OPTIONS), which differs from whatever was set but is not a + // schematic to configure. + if (type == BuildingType::Shipyard && *dialog.getChosenId() != oldSchematic + && m_sim->getConfig().ships.findLayoutShipDef(*dialog.getChosenId())) { autoOpenLayout = true; chosenSchematic = *dialog.getChosenId(); diff --git a/src/ui/selection/ShipyardContent.cpp b/src/ui/selection/ShipyardContent.cpp index 289ea20..40920be 100644 --- a/src/ui/selection/ShipyardContent.cpp +++ b/src/ui/selection/ShipyardContent.cpp @@ -44,11 +44,10 @@ void ShipyardContent::refreshControls(const BuildingTarget& target) // The preview and Configure button are always shown for a shipyard and are only // enabled once a schematic is selected (REQ-MOD-UI-PREVIEW). The schematic arrives // by queued command, so this refresh is what picks it up rather than the click that - // chose it. - const ShipDef* shipDef = target.recipeId.empty() - ? nullptr - : getContext().config->ships.findShipDef(target.recipeId); - const bool hasSchematic = shipDef && !shipDef->layout.empty(); + // chose it. Same question the dialog's own entry points ask, asked once. + const ShipDef* shipDef = + getContext().config->ships.findLayoutShipDef(target.recipeId); + const bool hasSchematic = shipDef != nullptr; if (hasSchematic) { m_layoutPreview->setShipAndLayout(