fix: resolve 3 critical ProductionSolver bugs
- Bug #1: CalculateExecution now filters product by targetResourceId instead of blindly taking .First() — fixes multi-product recipes - Bug #2: Replace visited HashSet with recipeOutput tracker — allows re-processing when demand increases (double targets on same resource) - Bug #3: Clamp effectiveSpeed to MinEffectiveSpeed (0.05) floor — prevents division by zero with heavy productivity module stacks Also: switch from Stack (DFS) to Queue (BFS) for more predictable resolution order; add 4 regression tests (24 total, all green)
This commit is contained in:
@@ -11,6 +11,7 @@ public sealed class ProductionSolver : ISolver
|
||||
{
|
||||
private readonly IRecipeRepository _repository;
|
||||
private const double DefaultMachineSpeed = 0.5; // base crafting speed for all machines
|
||||
private const double MinEffectiveSpeed = 0.05; // safety floor: machine cannot slow below 5% of base
|
||||
|
||||
public ProductionSolver(IRecipeRepository repository)
|
||||
{
|
||||
@@ -26,42 +27,57 @@ public sealed class ProductionSolver : ISolver
|
||||
{
|
||||
if (targets == null) throw new ArgumentNullException(nameof(targets));
|
||||
|
||||
var demand = new Dictionary<int, double>(); // resourceId -> needed amount/sec
|
||||
var demand = new Dictionary<int, double>(); // resourceId -> needed amount/sec
|
||||
var executions = new List<RecipeExecution>();
|
||||
var visited = new HashSet<int>(); // prevent cycles
|
||||
var recipeOutput = new Dictionary<int, double>(); // recipeId -> total output already planned (sec)
|
||||
|
||||
// Seed with user targets
|
||||
foreach (var target in targets)
|
||||
{
|
||||
demand[target.ResourceId] = target.AmountPerSecond;
|
||||
demand[target.ResourceId] = demand.GetValueOrDefault(target.ResourceId) + target.AmountPerSecond;
|
||||
}
|
||||
|
||||
// DFS: resolve each demand
|
||||
var stack = new Stack<int>(targets.Select(t => t.ResourceId));
|
||||
while (stack.Count > 0)
|
||||
{
|
||||
var resourceId = stack.Pop();
|
||||
// Collect all resource IDs that need resolving (including ingredients discovered during DFS)
|
||||
var unresolved = new Queue<int>(targets.Select(t => t.ResourceId));
|
||||
|
||||
if (!demand.TryGetValue(resourceId, out var needed) || needed <= 0)
|
||||
while (unresolved.Count > 0)
|
||||
{
|
||||
var resourceId = unresolved.Dequeue();
|
||||
|
||||
// Skip if demand was already fully satisfied or is zero
|
||||
if (!demand.TryGetValue(resourceId, out var needed) || needed <= 0.001)
|
||||
continue;
|
||||
|
||||
// Find the best recipe that produces this resource
|
||||
var recipe = FindBestRecipe(resourceId, modules);
|
||||
if (recipe == null)
|
||||
{
|
||||
// This is a raw resource — nothing to craft
|
||||
// Raw resource — nothing to craft
|
||||
continue;
|
||||
}
|
||||
|
||||
// Avoid re-processing the same recipe for the same resource
|
||||
var recipeKey = recipe.Id;
|
||||
if (visited.Contains(recipeKey))
|
||||
continue;
|
||||
visited.Add(recipeKey);
|
||||
// Calculate how much MORE this recipe needs to produce
|
||||
var alreadyPlanned = recipeOutput.GetValueOrDefault(recipe.Id);
|
||||
var remainingNeeded = needed - alreadyPlanned;
|
||||
|
||||
// Calculate recipe rate needed
|
||||
if (remainingNeeded <= 0.001)
|
||||
{
|
||||
// Already covered by a previous execution of this recipe
|
||||
continue;
|
||||
}
|
||||
|
||||
// Calculate execution for the remaining demand
|
||||
var (recipeRate, effectiveSpeed, effectiveProductivity, machineCount, machineId) =
|
||||
CalculateExecution(recipe, needed, modules);
|
||||
CalculateExecution(recipe, remainingNeeded, resourceId, modules);
|
||||
|
||||
// Update tracked output for this recipe
|
||||
var mainProduct = recipe.Products.FirstOrDefault(p => p.ResourceId == resourceId);
|
||||
if (mainProduct != null)
|
||||
{
|
||||
var outputPerCycle = mainProduct.Amount * (1 + effectiveProductivity);
|
||||
var newOutput = outputPerCycle * recipeRate * (int)Math.Ceiling(machineCount);
|
||||
recipeOutput[recipe.Id] = alreadyPlanned + newOutput;
|
||||
}
|
||||
|
||||
// Record execution
|
||||
executions.Add(new RecipeExecution(
|
||||
@@ -71,18 +87,15 @@ public sealed class ProductionSolver : ISolver
|
||||
// Add ingredient demands (DFS)
|
||||
foreach (var ingredient in recipe.Ingredients)
|
||||
{
|
||||
// How much of this ingredient does one cycle consume?
|
||||
// With productivity, input is NOT reduced — only output is increased
|
||||
var ingredientNeeded = ingredient.Amount * recipeRate;
|
||||
|
||||
var ingredientNeeded = ingredient.Amount * recipeRate * (int)Math.Ceiling(machineCount);
|
||||
demand[ingredient.ResourceId] = demand.GetValueOrDefault(ingredient.ResourceId) + ingredientNeeded;
|
||||
stack.Push(ingredient.ResourceId);
|
||||
unresolved.Enqueue(ingredient.ResourceId);
|
||||
}
|
||||
}
|
||||
|
||||
// Build resource flows
|
||||
var resourceFlows = BuildResourceFlows(executions, demand);
|
||||
var requiredInputs = BuildRequiredInputs(executions, demand, _repository.Recipes);
|
||||
var resourceFlows = BuildResourceFlows(executions);
|
||||
var requiredInputs = BuildRequiredInputs(demand, _repository.Recipes);
|
||||
|
||||
return new ProductionResult(
|
||||
executions.AsReadOnly(),
|
||||
@@ -114,29 +127,36 @@ public sealed class ProductionSolver : ISolver
|
||||
// Return the recipe with the best efficiency (highest output per cycle)
|
||||
return candidates.OrderByDescending(r =>
|
||||
{
|
||||
var product = r.Products.First(p => p.ResourceId == resourceId);
|
||||
return product.Amount / r.CraftTime;
|
||||
var product = r.Products.FirstOrDefault(p => p.ResourceId == resourceId);
|
||||
return product != null ? product.Amount / r.CraftTime : 0;
|
||||
}).First();
|
||||
}
|
||||
|
||||
private (double recipeRate, double effectiveSpeed, double effectiveProductivity, double machineCount, int machineId)
|
||||
CalculateExecution(Recipe recipe, double neededPerSec, IReadOnlyCollection<Module> modules)
|
||||
CalculateExecution(Recipe recipe, double neededPerSec, int targetResourceId, IReadOnlyCollection<Module> modules)
|
||||
{
|
||||
// Find the main product for our target resource
|
||||
var mainProduct = recipe.Products.First();
|
||||
// FIX #1: Find the product matching the target resource, not just the first one
|
||||
var mainProduct = recipe.Products.FirstOrDefault(p => p.ResourceId == targetResourceId);
|
||||
if (mainProduct == null)
|
||||
throw new InvalidOperationException(
|
||||
$"Recipe '{recipe.Name}' does not produce resource ID {targetResourceId}.");
|
||||
|
||||
// Calculate module bonuses
|
||||
var totalSpeedBonus = modules.Sum(m => m.SpeedBonus);
|
||||
var totalProductivityBonus = modules.Sum(m => m.ProductivityBonus);
|
||||
|
||||
// Effective speed = base speed * (1 + speed bonus)
|
||||
var effectiveSpeed = DefaultMachineSpeed * (1 + totalSpeedBonus);
|
||||
// FIX #3: Clamp to minimum to prevent division by zero
|
||||
var rawSpeed = DefaultMachineSpeed * (1 + totalSpeedBonus);
|
||||
var effectiveSpeed = Math.Max(rawSpeed, MinEffectiveSpeed);
|
||||
|
||||
// Effective productivity
|
||||
var effectiveProductivity = totalProductivityBonus;
|
||||
|
||||
// Output per cycle with productivity
|
||||
var outputPerCycle = mainProduct.Amount * (1 + effectiveProductivity);
|
||||
if (outputPerCycle <= 0)
|
||||
outputPerCycle = mainProduct.Amount; // fallback: productivity should not zero out output
|
||||
|
||||
// Recipe rate (cycles/sec) needed
|
||||
var recipeRate = neededPerSec / outputPerCycle;
|
||||
@@ -175,7 +195,7 @@ public sealed class ProductionSolver : ISolver
|
||||
return candidates.OrderByDescending(m => m.Value.CraftingSpeed).First().Value.Id;
|
||||
}
|
||||
|
||||
private Dictionary<int, double> BuildResourceFlows(List<RecipeExecution> executions, Dictionary<int, double> demand)
|
||||
private Dictionary<int, double> BuildResourceFlows(List<RecipeExecution> executions)
|
||||
{
|
||||
var flows = new Dictionary<int, double>();
|
||||
|
||||
@@ -202,15 +222,18 @@ public sealed class ProductionSolver : ISolver
|
||||
return flows;
|
||||
}
|
||||
|
||||
private Dictionary<int, double> BuildRequiredInputs(List<RecipeExecution> executions, Dictionary<int, double> demand, IReadOnlyDictionary<int, Recipe> recipes)
|
||||
private Dictionary<int, double> BuildRequiredInputs(Dictionary<int, double> demand, IReadOnlyDictionary<int, Recipe> recipes)
|
||||
{
|
||||
var inputs = new Dictionary<int, double>();
|
||||
|
||||
foreach (var (resourceId, amount) in demand)
|
||||
{
|
||||
if (amount <= 0.001)
|
||||
continue;
|
||||
|
||||
// If no recipe produces this, it's a raw input
|
||||
var hasProducer = recipes.Any(r => r.Value.Products.Any(p => p.ResourceId == resourceId));
|
||||
if (!hasProducer && amount > 0)
|
||||
if (!hasProducer)
|
||||
{
|
||||
inputs[resourceId] = amount;
|
||||
}
|
||||
|
||||
@@ -0,0 +1,189 @@
|
||||
using FactorioCalc.Domain;
|
||||
using FactorioCalc.Solver;
|
||||
using Xunit;
|
||||
|
||||
namespace FactorioCalc.Tests;
|
||||
|
||||
/// <summary>
|
||||
/// Regression tests for critical bugs fixed in ProductionSolver.
|
||||
/// </summary>
|
||||
public class SolverBugFixTests
|
||||
{
|
||||
// --- Bug #1: mainProduct should filter by target resourceId, not .First() ---
|
||||
|
||||
[Fact]
|
||||
public void Solve_MultiProductRecipe_UsesCorrectProduct()
|
||||
{
|
||||
// Simulate a recipe that produces multiple products (like oil refining)
|
||||
var resources = new Dictionary<int, Resource>
|
||||
{
|
||||
{ 1, new Resource(1, "Crude Oil") },
|
||||
{ 2, new Resource(2, "Light Oil") },
|
||||
{ 3, new Resource(3, "Heavy Oil") },
|
||||
};
|
||||
var machines = new Dictionary<int, Machine>
|
||||
{
|
||||
{ 1, new Machine(1, "Chemical Plant", 0.5, 6.0, 4, new[] { "advanced-crafting" }) },
|
||||
};
|
||||
|
||||
// One recipe produces BOTH Light Oil and Heavy Oil
|
||||
var recipes = new Dictionary<int, Recipe>
|
||||
{
|
||||
{
|
||||
1, new Recipe(1, "Oil Refining", "advanced-crafting", 4.0, "advanced-crafting",
|
||||
new[] { new Ingredient(1, 1) },
|
||||
new[] { new Product(3, 1), new Product(2, 1) }) // Heavy Oil first, Light Oil second
|
||||
},
|
||||
};
|
||||
|
||||
var repo = new TestRepository(recipes, resources, machines, new Dictionary<int, Module>());
|
||||
var solver = new ProductionSolver(repo);
|
||||
|
||||
// Target Light Oil (resourceId=2) — it's the SECOND product in the list
|
||||
var targets = new[] { new ProductionTarget(2, 10) };
|
||||
var result = solver.Solve(targets);
|
||||
|
||||
Assert.Single(result.Executions);
|
||||
// Should not throw — the solver correctly finds the matching product
|
||||
}
|
||||
|
||||
// --- Bug #2: DFS visited should not block recalculation for double targets ---
|
||||
|
||||
[Fact]
|
||||
public void Solve_DoubleTargetsForResource_AggregatesDemand()
|
||||
{
|
||||
var resources = new Dictionary<int, Resource>
|
||||
{
|
||||
{ 1, new Resource(1, "Iron Ore") },
|
||||
{ 7, new Resource(7, "Iron Plate") },
|
||||
{ 9, new Resource(9, "Steel Plate") },
|
||||
{ 5, new Resource(5, "Coal") },
|
||||
};
|
||||
var machines = new Dictionary<int, Machine>
|
||||
{
|
||||
{ 3, new Machine(3, "Smelter", 0.5, 3.0, 2, new[] { "smelting" }) },
|
||||
};
|
||||
var recipes = new Dictionary<int, Recipe>
|
||||
{
|
||||
{
|
||||
1, new Recipe(1, "Iron Plate", "smelting", 3.0, "smelting",
|
||||
new[] { new Ingredient(1, 1) },
|
||||
new[] { new Product(7, 1) })
|
||||
},
|
||||
{
|
||||
2, new Recipe(2, "Steel Plate", "smelting", 5.0, "smelting",
|
||||
new[] { new Ingredient(7, 2), new Ingredient(5, 1) },
|
||||
new[] { new Product(9, 1) })
|
||||
},
|
||||
};
|
||||
|
||||
var repo = new TestRepository(recipes, resources, machines, new Dictionary<int, Module>());
|
||||
var solver = new ProductionSolver(repo);
|
||||
|
||||
// Two targets that both need Iron Plate: Steel Plate (needs 2/sec × 2 iron plate) + direct 5/sec
|
||||
var targets = new[]
|
||||
{
|
||||
new ProductionTarget(9, 2), // Steel Plate → needs 4 Iron Plate/sec
|
||||
new ProductionTarget(7, 5), // Iron Plate direct → needs 5 more/sec
|
||||
};
|
||||
var result = solver.Solve(targets);
|
||||
|
||||
// Should have both Steel Plate and Iron Plate executions
|
||||
Assert.Equal(2, result.Executions.Count);
|
||||
|
||||
var ironPlateExec = result.Executions.First(e => e.RecipeId == 1);
|
||||
// Iron Plate should account for BOTH demands (4 from steel + 5 direct = 9 total)
|
||||
Assert.True(ironPlateExec.MachineCount >= 9,
|
||||
$"Expected at least 9 machines for Iron Plate (demand=9/sec), got {ironPlateExec.MachineCount}");
|
||||
}
|
||||
|
||||
// --- Bug #3: effectiveSpeed should not go to zero with heavy productivity modules ---
|
||||
|
||||
[Fact]
|
||||
public void Solve_HeavyProductivityModules_DoesNotDivideByZero()
|
||||
{
|
||||
var resources = new Dictionary<int, Resource>
|
||||
{
|
||||
{ 1, new Resource(1, "Iron Ore") },
|
||||
{ 7, new Resource(7, "Iron Plate") },
|
||||
};
|
||||
var machines = new Dictionary<int, Machine>
|
||||
{
|
||||
{ 3, new Machine(3, "Smelter", 0.5, 3.0, 2, new[] { "smelting" }) },
|
||||
};
|
||||
var recipes = new Dictionary<int, Recipe>
|
||||
{
|
||||
{
|
||||
1, new Recipe(1, "Iron Plate", "smelting", 3.0, "smelting",
|
||||
new[] { new Ingredient(1, 1) },
|
||||
new[] { new Product(7, 1) })
|
||||
},
|
||||
};
|
||||
|
||||
// Extreme productivity modules: -20% speed × 4 slots = -80% total speed
|
||||
var extremeModules = new[]
|
||||
{
|
||||
new Module(1, "Prod Mod 3", ModuleType.Productivity, -0.20, 0.30, -0.15),
|
||||
new Module(2, "Prod Mod 3", ModuleType.Productivity, -0.20, 0.30, -0.15),
|
||||
new Module(3, "Prod Mod 3", ModuleType.Productivity, -0.20, 0.30, -0.15),
|
||||
new Module(4, "Prod Mod 3", ModuleType.Productivity, -0.20, 0.30, -0.15),
|
||||
};
|
||||
|
||||
var moduleDict = new Dictionary<int, Module>();
|
||||
for (var i = 0; i < extremeModules.Length; i++)
|
||||
moduleDict[i + 1] = extremeModules[i];
|
||||
var repo = new TestRepository(recipes, resources, machines, moduleDict);
|
||||
var solver = new ProductionSolver(repo);
|
||||
|
||||
var targets = new[] { new ProductionTarget(7, 10) };
|
||||
|
||||
// Should NOT throw DivideByZeroException
|
||||
var result = solver.SolveWithModules(targets, extremeModules);
|
||||
|
||||
Assert.Single(result.Executions);
|
||||
var exec = result.Executions.First();
|
||||
// Speed should be clamped to minimum (0.05), not negative or zero
|
||||
Assert.True(exec.EffectiveSpeed >= 0.05, $"EffectiveSpeed {exec.EffectiveSpeed} below minimum");
|
||||
Assert.True(exec.MachineCount > 0 && exec.MachineCount < int.MaxValue,
|
||||
$"MachineCount {exec.MachineCount} is unreasonable");
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Solve_ProductivityModules_IncreasesOutput()
|
||||
{
|
||||
var resources = new Dictionary<int, Resource>
|
||||
{
|
||||
{ 1, new Resource(1, "Iron Ore") },
|
||||
{ 7, new Resource(7, "Iron Plate") },
|
||||
};
|
||||
var machines = new Dictionary<int, Machine>
|
||||
{
|
||||
{ 3, new Machine(3, "Smelter", 0.5, 3.0, 2, new[] { "smelting" }) },
|
||||
};
|
||||
var recipes = new Dictionary<int, Recipe>
|
||||
{
|
||||
{
|
||||
1, new Recipe(1, "Iron Plate", "smelting", 3.0, "smelting",
|
||||
new[] { new Ingredient(1, 1) },
|
||||
new[] { new Product(7, 1) })
|
||||
},
|
||||
};
|
||||
|
||||
var prodModule = new Module(1, "Prod Mod 1", ModuleType.Productivity, -0.10, 0.10, -0.05);
|
||||
|
||||
var repo = new TestRepository(recipes, resources, machines, new Dictionary<int, Module> { { 1, prodModule } });
|
||||
var solver = new ProductionSolver(repo);
|
||||
|
||||
var targets = new[] { new ProductionTarget(7, 10) };
|
||||
|
||||
var resultNoModules = solver.Solve(targets);
|
||||
var resultWithModules = solver.SolveWithModules(targets, new[] { prodModule });
|
||||
|
||||
var execNoModules = resultNoModules.Executions.First();
|
||||
var execWithModules = resultWithModules.Executions.First();
|
||||
|
||||
// With productivity, we need fewer machines because output per cycle is higher
|
||||
// (even though speed is lower, the +10% output compensates)
|
||||
Assert.True(execWithModules.EffectiveProductivity > 0, "Productivity bonus should be positive");
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user