diff --git a/src/game/design/hull.cpp b/src/game/design/hull.cpp index ac83033..9c18b73 100644 --- a/src/game/design/hull.cpp +++ b/src/game/design/hull.cpp @@ -23,7 +23,7 @@ HullClass classify_design(const data::Catalog& cat, const Design& d) { const data::Species race = d.effective_race(); std::vector found; HullClass h; - for (Slot slot : kUsedSlots) { + for (Slot slot : kAggregationOrder) { const SectionUse& use = d.slot(slot); if (use.empty()) continue; const data::ShipSectionDef* s = diff --git a/src/game/design/hull.h b/src/game/design/hull.h index b99e70d..3122282 100644 --- a/src/game/design/hull.h +++ b/src/game/design/hull.h @@ -59,14 +59,26 @@ struct HullClass { bool ok() const { return resolved_sections > 0; } }; -// Classify from already-resolved section definitions, in the design's slot -// order. Null entries are empty slots and are skipped. +// The order the original's stats pass visits the slots in, which is the design +// record's IN-MEMORY array order and NOT its wire order. The wire writes +// command, mission, engine; the array holds mission, command, engine. The +// distinction only shows on hull size, because that is an assignment: a design +// with an empty engine slot and both other slots filled would take its class +// from the command section here and from the mission section under wire order. +// Rule A3 makes that shape invalid (a mission section without sockets requires +// both partners empty, with sockets requires both filled), so no design in the +// corpus can tell them apart -- which is exactly why the order is spelled out +// rather than left to whichever loop was convenient. +constexpr std::array kAggregationOrder{Slot::Mission, Slot::Command, Slot::Engine}; + +// Classify from already-resolved section definitions, in `kAggregationOrder`. +// Null entries are empty slots and are skipped. HullClass classify_sections(const std::vector& sections); // Classify a design record against a catalog. Slots are visited in -// command / mission / engine order; an empty slot is skipped, and a slot that -// names a section the catalog does not hold is counted in -// `unresolved_sections` and otherwise ignored. +// `kAggregationOrder`; an empty slot is skipped, and a slot that names a +// section the catalog does not hold is counted in `unresolved_sections` and +// otherwise ignored. HullClass classify_design(const data::Catalog& cat, const Design& d); // Six counters: ships and defence platforms, each by hull size. diff --git a/src/game/design/stats.cpp b/src/game/design/stats.cpp index 8ad48d2..9d8e7b9 100644 --- a/src/game/design/stats.cpp +++ b/src/game/design/stats.cpp @@ -1,4 +1,5 @@ #include "game/design/stats.h" +#include "game/design/hull.h" namespace game::design { @@ -90,7 +91,7 @@ DesignStats derive_stats(const Ruleset& rules, const Design& d) { st.maintenance_cost += s->maintenance_cost.value_or(0); if (s->command_quota) st.command_quota = st.command_quota.value_or(0) + *s->command_quota; if (st.hull_class.empty() && !s->section_class_text.empty()) st.hull_class = data::fold(s->section_class_text); - st.hull_size = data::hull_size(s->section_class); + // The role-flag word is an OR, so slot order does not matter here. if (s->defence_platform.value_or(false)) st.defence_platform = true; for (std::string_view key : capacity_keys()) if (const data::Attr* a = s->raw.find(key)) set_capacity(st, key, a->value); @@ -102,6 +103,12 @@ DesignStats derive_stats(const Ruleset& rules, const Design& d) { } st.sections.push_back(std::move(sec)); } + // Hull size is an ASSIGNMENT in the record's in-memory slot order, so it is + // taken after the loop above rather than inside it -- that loop runs in wire + // order, which is not the same order. See design/hull.h. + for (Slot slot : kAggregationOrder) + for (const SectionStats& sec : st.sections) + if (sec.slot == slot && sec.section) st.hull_size = data::hull_size(sec.section->section_class); st.total_cost_estimate = st.section_cost_with_options + st.weapon_cost_per_bank; return st; } diff --git a/tests/game_design/test_hull.cpp b/tests/game_design/test_hull.cpp index 0e71b81..46595a4 100644 --- a/tests/game_design/test_hull.cpp +++ b/tests/game_design/test_hull.cpp @@ -83,23 +83,37 @@ TEST(one_flagged_section_makes_the_whole_design_a_platform) { CHECK_EQ(h.resolved_sections, 3); } -TEST(hull_size_takes_the_last_resolved_section) { - // Assignment in slot order, not first-wins and not max. Class-mixing is a - // rule A6 error, so this can only be shown on a design the validator - // rejects -- which is exactly why it is worth pinning. +TEST(hull_size_takes_the_last_resolved_section_in_memory_order) { + // Assignment, not first-wins and not max, and the order is the record's + // in-memory array order -- mission, command, engine -- not its wire order. + // Class-mixing is a rule A6 error and an engine-less two-slot design breaks + // rule A3, so this can only be shown on designs the validator rejects, + // which is exactly why it is worth pinning. + CHECK(kAggregationOrder[0] == Slot::Mission); + CHECK(kAggregationOrder[1] == Slot::Command); + CHECK(kAggregationOrder[2] == Slot::Engine); + Design d; d.race = Species::Human; d.command() = use("DECommand", {"bal_gauss"}); d.mission() = use("CRArmor", {}); HullClass h = classify_design(mini_catalog(), d); CHECK_EQ(h.resolved_sections, 2); - CHECK_EQ(h.hull_size, 1); // the cruiser mission section, visited last + CHECK_EQ(h.hull_size, 0); // the destroyer COMMAND section, visited last Design r; r.race = Species::Human; r.command() = use("CRCommand", {}); r.mission() = use("DEArmor", {"bal_gauss", "bal_gauss", "mis"}); - CHECK_EQ(classify_design(mini_catalog(), r).hull_size, 0); + CHECK_EQ(classify_design(mini_catalog(), r).hull_size, 1); // the cruiser command section + + // With the engine slot filled, the engine section wins whatever the others say. + Design e; + e.race = Species::Human; + e.command() = use("DECommand", {"bal_gauss"}); + e.mission() = use("DEArmor", {"bal_gauss", "bal_gauss", "mis"}); + e.engine() = use("CRFission", {}); + CHECK_EQ(classify_design(mini_catalog(), e).hull_size, 1); } TEST(an_unresolvable_design_is_reported_not_counted_as_a_destroyer) {