stop the layout dialog opening for a shipyard with no schematic
Clearing a shipyard with "(None)" opened the layout configuration dialog on a
grid of no cells. The auto-open guard asked only whether the chosen id differs
from the current one, and the "(None)" option carries the empty id, which
differs from every schematic; ShipLayoutDialog then found no ship def and
derived a 0x0 grid. Predates this branch -- 698dd4d, 2026-07-13.
The question all three sites were answering by hand is now one:
findLayoutShipDef() returns the ship to configure a layout against, or nullptr
when no schematic is set, the id names no ship, or the ship defines no grid.
The auto-open path and the LayoutDialogRequestedEvent handler now ask it
before opening, and ShipyardContent asks it instead of spelling the same test
out for the preview and the Configure button.
The event handler was reachable only through a button ShipyardContent already
disables, so guarding it changes nothing today; it is guarded because the
dialog's precondition belongs to the dialog's entry, not to the widget that
happens to be the only caller.
REQ-MOD-UI-AUTO-DIALOG said "differs" and left clearing implicit, which is the
reading the code took. It now says clearing opens nothing.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Ne3mejABZoLWKLh8fgpM3x
This commit is contained in:
@@ -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-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.
|
- 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.
|
||||||
|
|
||||||
|
|||||||
@@ -63,4 +63,19 @@ struct ShipsConfig
|
|||||||
}
|
}
|
||||||
return nullptr;
|
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;
|
||||||
|
}
|
||||||
};
|
};
|
||||||
|
|||||||
@@ -690,3 +690,31 @@ TEST_CASE("calculateShipStats: maneuvering_thrusters additive maneuvering_accele
|
|||||||
const float expected = (base_mpss + 10.0f) / tileSize;
|
const float expected = (base_mpss + 10.0f) / tileSize;
|
||||||
CHECK(stats.maneuveringAcceleration_tpss == Approx(expected));
|
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);
|
||||||
|
}
|
||||||
|
|||||||
@@ -390,6 +390,15 @@ void MainWindow::handleEvent(std::shared_ptr<const LayoutDialogRequestedEvent> e
|
|||||||
}
|
}
|
||||||
|
|
||||||
const std::string& schematicId = b ? b->recipeId : s->recipeId;
|
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<ShipLayoutConfig>& layoutOpt =
|
const std::optional<ShipLayoutConfig>& layoutOpt =
|
||||||
b ? b->shipLayout : s->shipLayout;
|
b ? b->shipLayout : s->shipLayout;
|
||||||
|
|
||||||
@@ -445,8 +454,12 @@ void MainWindow::handleEvent(std::shared_ptr<const RecipeSelectionRequestedEvent
|
|||||||
std::make_shared<CommandRequestedEvent>(command));
|
std::make_shared<CommandRequestedEvent>(command));
|
||||||
|
|
||||||
// REQ-MOD-UI-AUTO-DIALOG: picking a new schematic for a shipyard opens the
|
// REQ-MOD-UI-AUTO-DIALOG: picking a new schematic for a shipyard opens the
|
||||||
// layout configuration dialog immediately. Only on an actual change.
|
// layout configuration dialog immediately. Only on an actual change, and only
|
||||||
if (type == BuildingType::Shipyard && *dialog.getChosenId() != oldSchematic)
|
// 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;
|
autoOpenLayout = true;
|
||||||
chosenSchematic = *dialog.getChosenId();
|
chosenSchematic = *dialog.getChosenId();
|
||||||
|
|||||||
@@ -44,11 +44,10 @@ void ShipyardContent::refreshControls(const BuildingTarget& target)
|
|||||||
// The preview and Configure button are always shown for a shipyard and are only
|
// 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
|
// 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
|
// by queued command, so this refresh is what picks it up rather than the click that
|
||||||
// chose it.
|
// chose it. Same question the dialog's own entry points ask, asked once.
|
||||||
const ShipDef* shipDef = target.recipeId.empty()
|
const ShipDef* shipDef =
|
||||||
? nullptr
|
getContext().config->ships.findLayoutShipDef(target.recipeId);
|
||||||
: getContext().config->ships.findShipDef(target.recipeId);
|
const bool hasSchematic = shipDef != nullptr;
|
||||||
const bool hasSchematic = shipDef && !shipDef->layout.empty();
|
|
||||||
if (hasSchematic)
|
if (hasSchematic)
|
||||||
{
|
{
|
||||||
m_layoutPreview->setShipAndLayout(
|
m_layoutPreview->setShipAndLayout(
|
||||||
|
|||||||
Reference in New Issue
Block a user