From 5b86b15c71e0f5f11ee0e8c7b73a055ff4c93b19 Mon Sep 17 00:00:00 2001 From: mlangkabel Date: Wed, 8 Jul 2026 20:28:31 +0200 Subject: [PATCH] Fix ThreatCostCalculator: per-unit division, scrap fallback, fixpoint, staggered-recipe max --- bin/test/data/config/recipes.toml | 57 ++++++ docs/progression_design.md | 28 --- docs/requirements.md | 13 +- src/lib/sim/ThreatCostCalculator.cpp | 265 ++++++++++++++++++++------ src/test/ThreatCostCalculatorTest.cpp | 59 ++++++ tools/threat_report.py | 9 +- 6 files changed, 333 insertions(+), 98 deletions(-) diff --git a/bin/test/data/config/recipes.toml b/bin/test/data/config/recipes.toml index efd7016..883307e 100644 --- a/bin/test/data/config/recipes.toml +++ b/bin/test/data/config/recipes.toml @@ -92,3 +92,60 @@ duration_seconds = 3.0 item = "advanced_alloy" amount = 1 probability = 0.1 + +# ------------------------------------------------------------------- +# Extra recipes for ThreatCostCalculator unit tests (fixes 6-9) +# ------------------------------------------------------------------- + +# Fix 6: scrap-consuming smelter recipe for iron_ingot. Because iron_ingot +# already has a scrap-free smelter recipe above, this recipe must be excluded +# from iron_ingot's threat computation. +[[recipe]] +id = "scrap_iron" +building = "smelter" +inputs = [{item = "scrap", amount = 1}] +outputs = [{item = "iron_ingot", amount = 1}] +duration_seconds = 1.0 + +# Fix 7: a recipe that produces 2 items per cycle. Per-unit threat must +# divide by the output amount. +# dual_wire: (duration=3.0 + iron_ore(1.0)*1) / 2 = 4.0 / 2 = 2.0 per unit. +[[recipe]] +id = "dual_wire" +building = "assembler" +inputs = [{item = "iron_ore", amount = 1}] +outputs = [{item = "dual_wire", amount = 2}] +duration_seconds = 3.0 + +# Fix 8: an item downstream of a reprocessing-only item (advanced_alloy). +# advanced_alloy is resolved only by the reprocessing pass; downstream_product +# can only resolve in a non-reprocessing pass that runs AFTER the reprocessing +# pass, requiring proper fixpoint iteration. +# downstream_product: 2.0 + advanced_alloy(80.0)*1 = 82.0 +[[recipe]] +id = "downstream_product" +building = "assembler" +inputs = [{item = "advanced_alloy", amount = 1}] +outputs = [{item = "downstream_product", amount = 1}] +duration_seconds = 2.0 + +# Fix 9: two recipes producing the same staggered_item. The cheap recipe +# resolves before circuit_board is known; the expensive one requires +# circuit_board. The item must be committed only once BOTH are computable, +# so the result is max(cheap, expensive). +# staggered_item_cheap: 1.0 + iron_ore(1.0)*1 = 2.0 (resolves early) +# staggered_item_expensive: 1.0 + circuit_board(28.0)*1 = 29.0 (resolves later) +# expected: max = 29.0 +[[recipe]] +id = "staggered_item_cheap" +building = "assembler" +inputs = [{item = "iron_ore", amount = 1}] +outputs = [{item = "staggered_item", amount = 1}] +duration_seconds = 1.0 + +[[recipe]] +id = "staggered_item_expensive" +building = "assembler" +inputs = [{item = "circuit_board", amount = 1}] +outputs = [{item = "staggered_item", amount = 1}] +duration_seconds = 1.0 diff --git a/docs/progression_design.md b/docs/progression_design.md index 1c5853c..6e9fb17 100644 --- a/docs/progression_design.md +++ b/docs/progression_design.md @@ -373,31 +373,3 @@ in `requirements.md` and the git history). Still open: one matching deposit tile). Touches REQ-BLD-MINER ("every asteroid tile is equivalent" no longer holds), REQ-GW-ASTEROID-EXPAND / REQ-EXP-*, `world.toml`, and `visuals.toml`. -6. **Scrap-consuming recipes as threat fallback only.** Amend - REQ-THREAT-ITEM: recipes that take scrap as an input participate in - an item's threat computation only if no scrap-free recipe (miner, - smelter, or assembler) produces that item — mirroring the existing - rule for the reprocessing path. Otherwise the scrap→ingot smelter - recipe would inflate the basic materials' threat via the - max-across-recipes rule, poisoning every downstream value. -7. **Per-unit item threat.** Amend REQ-THREAT-ITEM and - `ThreatCostCalculator`: a recipe's threat is divided by its output - amount, so item threat is production-seconds *per unit*. Currently a - recipe producing 2 copper_wire per run assigns each wire the full - run's threat, double-pricing multi-output items and everything - downstream of them. -8. **Fixpoint resolution in ThreatCostCalculator.** Items downstream of - reprocessing-only items (e.g. capital parts built from the scrap-only - input) never resolve, because resolution stops after the reprocessing - pass instead of iterating; their consumers silently drop the missing - materials, so capital hull threat is currently underestimated (found - by `tools/threat_report.py`, which implements the correct fixpoint). -9. **Max rule across staggered recipes in ThreatCostCalculator.** An - item is committed at the first iteration where *any* of its recipes - resolves, taking the max only over the recipes resolvable at that - point. A shallow shortcut recipe (e.g. steel plate from raw ore) - resolves one iteration earlier than the base path and wins, silently - underpricing the item and everything downstream — violating the - "shortcuts are pure rewards" rule. Fix: commit an item's threat only - once every eligible recipe for it is computable (as - `tools/threat_report.py` does), with a fallback for recipe cycles. diff --git a/docs/requirements.md b/docs/requirements.md index eeda358..34d59cc 100644 --- a/docs/requirements.md +++ b/docs/requirements.md @@ -224,12 +224,13 @@ Modules in `modules.toml` define a `surface_mask` — a list of strings that des 2. The `production_time_seconds` of every module instance in the configured layout. 3. For every material required (the union of the ship's base materials and all module instance materials, with quantities summed per item type): the recursive production time of that material multiplied by the required quantity (see REQ-THREAT-ITEM). -- REQ-THREAT-ITEM: The threat value of an item type (in seconds) is determined by the recipe that produces it: - - **Miner recipe**: the recipe's `duration_seconds`. - - **Smelter recipe**: the recipe's `duration_seconds` plus the sum of each input's threat value multiplied by that input's required quantity. - - **Assembler recipe**: the recipe's `duration_seconds` plus the sum of each input's threat value multiplied by that input's required quantity. - - **Reprocessing-only item** (an item type that has no miner, smelter, or assembler recipe producing it, and is only obtainable via reprocessing): `(scrap_threat × scrap_per_cycle + duration_seconds) / probability`, where `scrap_threat` is the threat value of scrap (see REQ-THREAT-SCRAP), `scrap_per_cycle` is the number of scrap consumed per reprocessing cycle, `duration_seconds` is the reprocessing cycle time, and `probability` is the normalized weight of that item in the reprocessing output pool. - - **Multiple recipes**: if an item type can be produced by more than one non-reprocessing recipe (miner, smelter, or assembler), its threat value is the **maximum** across all such recipes. The reprocessing path is only used when no other recipe exists. +- REQ-THREAT-ITEM: The threat value of an item type (in production-seconds **per unit**) is determined by the recipe that produces it: + - **Miner recipe**: `duration_seconds / output_amount`, where `output_amount` is the number of units produced per cycle. + - **Smelter recipe**: `(duration_seconds + Σ (input_threat × input_amount)) / output_amount`, where the sum is over all inputs. + - **Assembler recipe**: `(duration_seconds + Σ (input_threat × input_amount)) / output_amount`, where the sum is over all inputs. + - **Reprocessing-only item** (an item type that has no miner, smelter, or assembler recipe producing it, and is only obtainable via reprocessing): `(scrap_threat × scrap_per_cycle + duration_seconds) / probability`, where `scrap_threat` is the threat value of scrap (see REQ-THREAT-SCRAP), `scrap_per_cycle` is the number of scrap consumed per reprocessing cycle, `duration_seconds` is the reprocessing cycle time, and `probability` is the normalized weight of that item in the reprocessing output pool. (Reprocessing output amounts are 1 in practice, so per-unit division is already implicit in the formula.) + - **Multiple recipes**: if an item type can be produced by more than one non-reprocessing recipe (miner, smelter, or assembler), its threat value is the **maximum** across **all** such eligible recipes, and the threat is committed only once every eligible recipe is computable (so a shallow shortcut recipe that resolves earlier than a deeper base recipe cannot lower the item's threat). The reprocessing path is only used when no other recipe exists. If recipe cycles prevent full resolution, the max over the currently computable subset is used as a fallback. + - **Scrap-consuming recipe fallback**: a non-reprocessing recipe that takes `scrap` as an input participates in an item's threat computation only if no scrap-free recipe (miner, smelter, or assembler) produces that item. This mirrors the reprocessing fallback rule and prevents the scrap-to-ingot smelter recipe from inflating basic material threats via the max rule. - REQ-THREAT-SCRAP: The threat value of scrap is the constant `1 / world.toml [world].scrap_per_threat`. This is the exact inverse of the scrap-drop conversion in REQ-RES-SCRAP-DROP, so a destroyed ship drops scrap worth precisely its own threat cost. Because scrap threat is now a fixed constant, it no longer depends on any ship's threat cost, removing the potential circularity with REQ-MOD-THREAT for ships built from reprocessing-only materials. - REQ-MOD-STAT-CALC: For each stat (on the ship hull or on a capability module instance), the final value is computed as: `final = base × total_multiplier + total_additive`, where: diff --git a/src/lib/sim/ThreatCostCalculator.cpp b/src/lib/sim/ThreatCostCalculator.cpp index 2ef7c04..30ecfc1 100644 --- a/src/lib/sim/ThreatCostCalculator.cpp +++ b/src/lib/sim/ThreatCostCalculator.cpp @@ -30,6 +30,7 @@ double computeMaterialThreat(const ThreatCostTable& table, return total; } +// Returns true if every input of the recipe has a resolved threat value. bool allInputsResolved(const RecipeDef& recipe, const std::map& resolved) { @@ -43,15 +44,18 @@ bool allInputsResolved(const RecipeDef& recipe, return true; } -double computeRecipeThreat(const RecipeDef& recipe, - const std::map& resolved) +// Computes the raw recipe threat (duration + sum of input threats × amounts), +// divided by the output amount to get the per-unit threat. +double computeRecipeThreatPerUnit(const RecipeDef& recipe, + int outputAmount, + const std::map& resolved) { double threat = recipe.durationSeconds; for (const RecipeIngredient& input : recipe.inputs) { threat += resolved.at(input.item) * input.amount; } - return threat; + return threat / static_cast(outputAmount); } } // namespace @@ -69,118 +73,259 @@ ThreatCostTable computeThreatCostTable(const GameConfig& config) ? 1.0 / config.world.scrapPerThreat : 0.0; - // Build lookup: output item → non-reprocessing recipes and reprocessing recipes. - std::map> nonReprocessingRecipes; + // ------------------------------------------------------------------------- + // Build per-item recipe lookup tables. + // ------------------------------------------------------------------------- + + // Items that have at least one non-reprocessing recipe that does NOT consume + // scrap — these items' scrap-consuming recipes are excluded from threat + // computation (REQ-THREAT-ITEM: scrap-consuming recipes are a fallback only). + std::set scrapFreeItems; + + // nonReprocessingRecipes: item → all eligible non-reprocessing (recipe, output) + // pairs. Scrap-consuming recipes are collected here temporarily; they are + // filtered out per item after we know which items have a scrap-free producer. + struct EligiblePair + { + const RecipeDef* recipe; + int outputAmount; + bool consumesScrap; + }; + std::map> nonReprocessingCandidates; + + // reprocessingRecipes: item → all reprocessing-recipe refs (probability + // values are raw from config; we normalize them per-recipe below). std::map> reprocessingRecipes; for (const RecipeDef& recipe : config.recipes.recipes) { if (recipe.building == BuildingType::ReprocessingPlant) { + // Compute the total weight across all outputs of this reprocessing recipe + // so we can normalize each output's probability. + double totalWeight = 0.0; + for (const RecipeOutput& out : recipe.outputs) + { + totalWeight += out.probability.value_or(1.0); + } + if (totalWeight <= 0.0) + { + continue; + } + for (const RecipeOutput& out : recipe.outputs) { RecipeRef ref; ref.recipe = &recipe; ref.outputItem = out.item; ref.outputAmount = out.amount; - ref.probability = out.probability.value_or(1.0); + ref.probability = out.probability.value_or(1.0) / totalWeight; reprocessingRecipes[out.item].push_back(ref); } } else { + // Check whether this non-reprocessing recipe consumes scrap. + bool consumesScrap = false; + for (const RecipeIngredient& input : recipe.inputs) + { + if (input.item == "scrap") + { + consumesScrap = true; + break; + } + } + for (const RecipeOutput& out : recipe.outputs) { - RecipeRef ref; - ref.recipe = &recipe; - ref.outputItem = out.item; - ref.outputAmount = out.amount; - ref.probability = 1.0; - nonReprocessingRecipes[out.item].push_back(ref); + if (!consumesScrap) + { + scrapFreeItems.insert(out.item); + } + + EligiblePair pair; + pair.recipe = &recipe; + pair.outputAmount = out.amount; + pair.consumesScrap = consumesScrap; + nonReprocessingCandidates[out.item].push_back(pair); } } } - // Collect all item names that need resolving. - std::set unresolved; - for (const std::pair>& entry : nonReprocessingRecipes) + // Filter nonReprocessingCandidates: for items that have at least one + // scrap-free producer, drop their scrap-consuming recipes. + // Build the final per-item list of (recipe, outputAmount) pairs eligible + // for the max-across-recipes rule (REQ-THREAT-ITEM). + std::map>> eligibleRecipes; + for (std::map>::const_iterator it = + nonReprocessingCandidates.begin(); + it != nonReprocessingCandidates.end(); + ++it) { - unresolved.insert(entry.first); - } - for (const std::pair>& entry : reprocessingRecipes) - { - unresolved.insert(entry.first); + const std::string& item = it->first; + const std::vector& candidates = it->second; + bool hasScrapFree = (scrapFreeItems.find(item) != scrapFreeItems.end()); + + for (const EligiblePair& candidate : candidates) + { + if (candidate.consumesScrap && hasScrapFree) + { + // Scrap-consuming recipe excluded: item has a scrap-free producer. + continue; + } + eligibleRecipes[item].emplace_back(candidate.recipe, candidate.outputAmount); + } } - // Iteratively resolve non-reprocessing items. - bool progress = true; - while (progress) + // ------------------------------------------------------------------------- + // Resolution: seed resolved map with scrap, then alternate the + // non-reprocessing pass and the reprocessing pass to a fixpoint. + // Fix (8): iterate until neither pass makes progress, rather than running + // the reprocessing pass once at the end. + // ------------------------------------------------------------------------- + + std::map& resolved = table.itemThreat; + resolved["scrap"] = table.scrapThreat; + + // Non-reprocessing resolution pass. + // Fix (9): commit an item only when EVERY eligible recipe for it is + // computable, not just the first one that resolves. This ensures a shallow + // shortcut recipe cannot undercut a deeper base recipe by resolving earlier. + auto runNonReprocessingPass = [&](bool requireAllRecipes) -> bool { - progress = false; - std::set newlyResolved; - for (const std::string& item : unresolved) + bool progress = false; + std::map newValues; + + for (std::map>>::const_iterator + it = eligibleRecipes.begin(); + it != eligibleRecipes.end(); + ++it) { - std::map>::const_iterator it = - nonReprocessingRecipes.find(item); - if (it == nonReprocessingRecipes.end()) + const std::string& item = it->first; + if (resolved.find(item) != resolved.end()) { continue; } + const std::vector>& pairs = it->second; + + bool allComputable = true; double maxThreat = -1.0; - for (const RecipeRef& ref : it->second) + for (const std::pair& pair : pairs) { - if (allInputsResolved(*ref.recipe, table.itemThreat)) + if (!allInputsResolved(*pair.first, resolved)) { - double threat = computeRecipeThreat(*ref.recipe, table.itemThreat); - if (threat > maxThreat) + allComputable = false; + if (requireAllRecipes) { - maxThreat = threat; + break; } + // In fallback mode: skip this recipe but continue gathering + // the computable subset. + continue; + } + double threat = computeRecipeThreatPerUnit(*pair.first, pair.second, resolved); + if (threat > maxThreat) + { + maxThreat = threat; } } + if (requireAllRecipes && !allComputable) + { + continue; + } if (maxThreat >= 0.0) { - table.itemThreat[item] = maxThreat; - newlyResolved.insert(item); - progress = true; + newValues[item] = maxThreat; } } - for (const std::string& item : newlyResolved) - { - unresolved.erase(item); - } - } - // Resolve reprocessing-only items. - for (const std::string& item : unresolved) + for (std::map::const_iterator it = newValues.begin(); + it != newValues.end(); + ++it) + { + resolved[it->first] = it->second; + progress = true; + } + return progress; + }; + + // Reprocessing pass: resolve items produced exclusively by reprocessing. + // Items that also have a non-reprocessing recipe are skipped here (they are + // covered by the non-reprocessing pass or do not need the reprocessing path). + auto runReprocessingPass = [&]() -> bool { - std::map>::const_iterator it = - reprocessingRecipes.find(item); - if (it == reprocessingRecipes.end()) + bool progress = false; + for (std::map>::const_iterator it = + reprocessingRecipes.begin(); + it != reprocessingRecipes.end(); + ++it) { - continue; - } - - for (const RecipeRef& ref : it->second) - { - int scrapPerCycle = 0; - for (const RecipeIngredient& input : ref.recipe->inputs) + const std::string& item = it->first; + if (resolved.find(item) != resolved.end()) { - scrapPerCycle += input.amount; + continue; + } + // Reprocessing defines an item's threat only when nothing else + // produces it (REQ-THREAT-ITEM). + if (scrapFreeItems.find(item) != scrapFreeItems.end()) + { + continue; + } + // Also skip items covered by eligible (non-reprocessing) recipes. + if (eligibleRecipes.find(item) != eligibleRecipes.end()) + { + continue; } - double threat = (table.scrapThreat * scrapPerCycle - + ref.recipe->durationSeconds) / ref.probability; - std::map::iterator existing = table.itemThreat.find(item); - if (existing == table.itemThreat.end() || threat > existing->second) + for (const RecipeRef& ref : it->second) { - table.itemThreat[item] = threat; + // Sum all scrap inputs for this reprocessing recipe. + int scrapPerCycle = 0; + for (const RecipeIngredient& input : ref.recipe->inputs) + { + scrapPerCycle += input.amount; + } + + double threat = (table.scrapThreat * scrapPerCycle + + ref.recipe->durationSeconds) / ref.probability; + + std::map::iterator existing = resolved.find(item); + if (existing == resolved.end() || threat > existing->second) + { + resolved[item] = threat; + progress = true; + } } } + return progress; + }; + + // Main fixpoint loop: alternate non-reprocessing and reprocessing passes + // until neither makes any progress (fix 8). + bool anyProgress = true; + while (anyProgress) + { + anyProgress = runNonReprocessingPass(true); + anyProgress = runReprocessingPass() || anyProgress; } + // Deadlock fallback: if any items remain unresolved due to recipe cycles, + // fall back to committing with the max over the currently computable subset + // of recipes (fix 9, deadlock guard — same approach as threat_report.py's + // require_all_recipes=False mode). + anyProgress = true; + while (anyProgress) + { + anyProgress = runNonReprocessingPass(false); + anyProgress = runReprocessingPass() || anyProgress; + } + + // Remove the sentinel scrap entry — scrapThreat is already stored on the + // table struct; having it in itemThreat would confuse callers iterating items. + resolved.erase("scrap"); + return table; } diff --git a/src/test/ThreatCostCalculatorTest.cpp b/src/test/ThreatCostCalculatorTest.cpp index 6fa2f13..0c9542e 100644 --- a/src/test/ThreatCostCalculatorTest.cpp +++ b/src/test/ThreatCostCalculatorTest.cpp @@ -107,3 +107,62 @@ TEST_CASE("ThreatCostCalculator: unknown ship returns zero", "[threat]") double threat = calculateShipThreatCost(table, cfg, "nonexistent_ship", {}); CHECK(threat == Approx(0.0)); } + +// Fix 6: scrap-consuming recipes are a fallback only. +// iron_ingot has a scrap-free smelter recipe, so the scrap_iron recipe must +// be excluded. iron_ingot threat must not be inflated by the scrap path. +TEST_CASE("ThreatCostCalculator: scrap-consuming recipe excluded when scrap-free recipe exists", "[threat]") +{ + const GameConfig cfg = loadConfig(); + const ThreatCostTable& table = cfg.threatCosts; + + // scrap_iron recipe: duration=1.0, 1 scrap (threat=1.0) -> 1 iron_ingot. + // That would give 1.0 + 1.0*1 = 2.0 per unit — but it must be excluded + // because the scrap-free iron_ingot smelter recipe (threat=4.0) exists. + // iron_ingot threat stays at 4.0. + CHECK(table.itemThreat.at("iron_ingot") == Approx(4.0)); + + // The pure reprocessing-only item (advanced_alloy) must still be resolved + // via the reprocessing path. + CHECK(table.itemThreat.count("advanced_alloy") == 1u); +} + +// Fix 7: per-unit item threat divides by output amount. +// dual_wire: assembler, 1 iron_ore -> 2 dual_wire, duration 3.0. +// Per-unit threat = (3.0 + iron_ore(1.0)*1) / 2 = 4.0 / 2 = 2.0. +TEST_CASE("ThreatCostCalculator: per-unit division by output amount", "[threat]") +{ + const GameConfig cfg = loadConfig(); + const ThreatCostTable& table = cfg.threatCosts; + + CHECK(table.itemThreat.at("dual_wire") == Approx(2.0)); +} + +// Fix 8: fixpoint resolution — items downstream of reprocessing-only items +// must be resolved after the reprocessing pass re-enables the non-reprocessing +// pass. +// downstream_product: assembler, 1 advanced_alloy -> 1, duration 2.0. +// advanced_alloy = 80.0 (reprocessing-only). +// downstream_product = 2.0 + 80.0*1 = 82.0. +TEST_CASE("ThreatCostCalculator: downstream-of-reprocessing item resolves via fixpoint", "[threat]") +{ + const GameConfig cfg = loadConfig(); + const ThreatCostTable& table = cfg.threatCosts; + + CHECK(table.itemThreat.at("advanced_alloy") == Approx(80.0)); + CHECK(table.itemThreat.at("downstream_product") == Approx(82.0)); +} + +// Fix 9: max rule across staggered recipes — item is committed only once +// every eligible recipe for it is computable. +// staggered_item has two recipes: +// cheap: 1 iron_ore (1.0) + 1.0 s = 2.0 (resolves early) +// expensive: 1 circuit_board (28.0) + 1.0 s = 29.0 (resolves later) +// expected threat = max(2.0, 29.0) = 29.0, not 2.0. +TEST_CASE("ThreatCostCalculator: staggered recipes committed only when all computable", "[threat]") +{ + const GameConfig cfg = loadConfig(); + const ThreatCostTable& table = cfg.threatCosts; + + CHECK(table.itemThreat.at("staggered_item") == Approx(29.0)); +} diff --git a/tools/threat_report.py b/tools/threat_report.py index 92a6614..f5da3aa 100644 --- a/tools/threat_report.py +++ b/tools/threat_report.py @@ -23,10 +23,11 @@ Reads recipes.toml, ships.toml, modules.toml, and world.toml and prints: 5. Belt feasibility — input demand in items/s per building vs. the single-belt cap (belt_speed_mps / tile_size_m, in items/s). -NOTE: the per-unit division (1) and the scrap fallback rule are the -agreed design semantics (see docs/progression_design.md action items); -src/lib/sim/ThreatCostCalculator.cpp does not implement them yet. Until -it does, this report is the design reference, not a mirror of the game. +NOTE: the semantics described above — per-unit division, scrap fallback +rule, fixpoint resolution including reprocessing, and the commit-all- +recipes max rule — are implemented in both this script and in +src/lib/sim/ThreatCostCalculator.cpp. The two should produce identical +values for any given config. Usage (from the repository root or anywhere else):