diff --git a/Content.IntegrationTests/Tests/Chemistry/SolutionRoundingTest.cs b/Content.IntegrationTests/Tests/Chemistry/SolutionRoundingTest.cs new file mode 100644 index 0000000000..4d19a96d9e --- /dev/null +++ b/Content.IntegrationTests/Tests/Chemistry/SolutionRoundingTest.cs @@ -0,0 +1,122 @@ +using Content.Server.Chemistry.Containers.EntitySystems; +using Content.Shared.Chemistry.Components; +using Content.Shared.Chemistry.Reaction; +using Content.Shared.Chemistry.Reagent; +using Content.Shared.FixedPoint; +using Robust.Shared.GameObjects; + +namespace Content.IntegrationTests.Tests.Chemistry; + +[TestFixture] +[TestOf(typeof(ChemicalReactionSystem))] +public sealed class SolutionRoundingTest +{ + // This test tests two things: + // * A rounding error in reaction code while I was making chloral hydrate + // * An assert with solution heat capacity calculations that I found a repro for while testing the above. + + [TestPrototypes] + private const string Prototypes = @" +- type: entity + id: SolutionRoundingTestContainer + components: + - type: SolutionContainerManager + solutions: + beaker: + maxVol: 100 + +# This is the Chloral Hydrate recipe fyi. +- type: reagent + id: SolutionRoundingTestReagentA + name: reagent-name-nothing + desc: reagent-desc-nothing + physicalDesc: reagent-physical-desc-nothing + +- type: reagent + id: SolutionRoundingTestReagentB + name: reagent-name-nothing + desc: reagent-desc-nothing + physicalDesc: reagent-physical-desc-nothing + +- type: reagent + id: SolutionRoundingTestReagentC + name: reagent-name-nothing + desc: reagent-desc-nothing + physicalDesc: reagent-physical-desc-nothing + +- type: reagent + id: SolutionRoundingTestReagentD + name: reagent-name-nothing + desc: reagent-desc-nothing + physicalDesc: reagent-physical-desc-nothing + +- type: reaction + id: SolutionRoundingTestReaction + impact: Medium + reactants: + SolutionRoundingTestReagentA: + amount: 3 + SolutionRoundingTestReagentB: + amount: 1 + SolutionRoundingTestReagentC: + amount: 1 + products: + SolutionRoundingTestReagentD: 1 +"; + + [Test] + public async Task Test() + { + await using var pair = await PoolManager.GetServerClient(); + var server = pair.Server; + var testMap = await pair.CreateTestMap(); + + Solution solution = default; + Entity solutionEnt = default; + + await server.WaitPost(() => + { + var system = server.System(); + var beaker = server.EntMan.SpawnEntity("SolutionRoundingTestContainer", testMap.GridCoords); + + system.TryGetSolution(beaker, "beaker", out var newSolutionEnt, out var newSolution); + + solutionEnt = newSolutionEnt!.Value; + solution = newSolution!; + + system.TryAddSolution(solutionEnt, new Solution("SolutionRoundingTestReagentC", 50)); + system.TryAddSolution(solutionEnt, new Solution("SolutionRoundingTestReagentB", 30)); + + for (var i = 0; i < 9; i++) + { + system.TryAddSolution(solutionEnt, new Solution("SolutionRoundingTestReagentA", 10)); + } + }); + + await server.WaitAssertion(() => + { + Assert.Multiple(() => + { + Assert.That( + solution.ContainsReagent("SolutionRoundingTestReagentA", null), + Is.False, + "Solution should not contain reagent A"); + + Assert.That( + solution.ContainsReagent("SolutionRoundingTestReagentB", null), + Is.False, + "Solution should not contain reagent B"); + + Assert.That( + solution![new ReagentId("SolutionRoundingTestReagentC", null)].Quantity, + Is.EqualTo((FixedPoint2) 20)); + + Assert.That( + solution![new ReagentId("SolutionRoundingTestReagentD", null)].Quantity, + Is.EqualTo((FixedPoint2) 30)); + }); + }); + + await pair.CleanReturnAsync(); + } +} diff --git a/Content.IntegrationTests/Tests/Damageable/DamageableTest.cs b/Content.IntegrationTests/Tests/Damageable/DamageableTest.cs index 3ab90b07eb..16744d83dc 100644 --- a/Content.IntegrationTests/Tests/Damageable/DamageableTest.cs +++ b/Content.IntegrationTests/Tests/Damageable/DamageableTest.cs @@ -170,19 +170,19 @@ namespace Content.IntegrationTests.Tests.Damageable // Check that damage works properly if it is NOT perfectly divisible among group members types = group3.DamageTypes; - damageToDeal = FixedPoint2.New(types.Count * 5 - 1); - damage = new DamageSpecifier(group3, damageToDeal); + + Assert.That(types, Has.Count.EqualTo(3)); + + damage = new DamageSpecifier(group3, 14); sDamageableSystem.TryChangeDamage(uid, damage, true); Assert.Multiple(() => { - Assert.That(sDamageableComponent.TotalDamage, Is.EqualTo(damageToDeal)); - Assert.That(sDamageableComponent.DamagePerGroup[group3.ID], Is.EqualTo(damageToDeal)); - Assert.That(sDamageableComponent.Damage.DamageDict[type3a.ID], Is.EqualTo(damageToDeal / types.Count)); - Assert.That(sDamageableComponent.Damage.DamageDict[type3b.ID], Is.EqualTo(damageToDeal / types.Count)); - - // last one will get 0.01 less, since its not perfectly divisble by 3 - Assert.That(sDamageableComponent.Damage.DamageDict[type3c.ID], Is.EqualTo(damageToDeal / types.Count - 0.01)); + Assert.That(sDamageableComponent.TotalDamage, Is.EqualTo(FixedPoint2.New(14))); + Assert.That(sDamageableComponent.DamagePerGroup[group3.ID], Is.EqualTo(FixedPoint2.New(14))); + Assert.That(sDamageableComponent.Damage.DamageDict[type3a.ID], Is.EqualTo(FixedPoint2.New(4.66f))); + Assert.That(sDamageableComponent.Damage.DamageDict[type3b.ID], Is.EqualTo(FixedPoint2.New(4.67f))); + Assert.That(sDamageableComponent.Damage.DamageDict[type3c.ID], Is.EqualTo(FixedPoint2.New(4.67f))); }); // Heal @@ -224,7 +224,7 @@ namespace Content.IntegrationTests.Tests.Damageable Assert.Multiple(() => { - Assert.That(sDamageableComponent.Damage.DamageDict[type3a.ID], Is.EqualTo(FixedPoint2.New(1.33))); + Assert.That(sDamageableComponent.Damage.DamageDict[type3a.ID], Is.EqualTo(FixedPoint2.New(1.34))); Assert.That(sDamageableComponent.Damage.DamageDict[type3b.ID], Is.EqualTo(FixedPoint2.New(3.33))); Assert.That(sDamageableComponent.Damage.DamageDict[type3c.ID], Is.EqualTo(FixedPoint2.New(0))); }); diff --git a/Content.Shared/Chemistry/Components/Solution.cs b/Content.Shared/Chemistry/Components/Solution.cs index f8e4821319..1c24c860dd 100644 --- a/Content.Shared/Chemistry/Components/Solution.cs +++ b/Content.Shared/Chemistry/Components/Solution.cs @@ -92,6 +92,12 @@ namespace Content.Shared.Chemistry.Components /// [ViewVariables] private bool _heatCapacityDirty = true; + [ViewVariables(VVAccess.ReadWrite)] + private int _heatCapacityUpdateCounter; + + // This value is arbitrary btw. + private const int HeatCapacityUpdateInterval = 15; + public void UpdateHeatCapacity(IPrototypeManager? protoMan) { IoCManager.Resolve(ref protoMan); @@ -103,6 +109,8 @@ namespace Content.Shared.Chemistry.Components _heatCapacity += (float) quantity * protoMan.Index(reagent.Prototype).SpecificHeat; } + + _heatCapacityUpdateCounter = 0; } public float GetHeatCapacity(IPrototypeManager? protoMan) @@ -112,6 +120,15 @@ namespace Content.Shared.Chemistry.Components return _heatCapacity; } + public void CheckRecalculateHeatCapacity() + { + // For performance, we have a few ways for heat capacity to get modified without a full recalculation. + // To avoid these drifting too much due to float error, we mark it as dirty after N such operations, + // so it will be recalculated. + if (++_heatCapacityUpdateCounter >= HeatCapacityUpdateInterval) + _heatCapacityDirty = true; + } + public float GetThermalEnergy(IPrototypeManager? protoMan) { return GetHeatCapacity(protoMan) * Temperature; @@ -165,6 +182,7 @@ namespace Content.Shared.Chemistry.Components Temperature = solution.Temperature; _heatCapacity = solution._heatCapacity; _heatCapacityDirty = solution._heatCapacityDirty; + _heatCapacityUpdateCounter = solution._heatCapacityUpdateCounter; ValidateSolution(); } @@ -193,7 +211,7 @@ namespace Content.Shared.Chemistry.Components var cur = _heatCapacity; _heatCapacityDirty = true; UpdateHeatCapacity(null); - DebugTools.Assert(MathHelper.CloseTo(_heatCapacity, cur)); + DebugTools.Assert(MathHelper.CloseTo(_heatCapacity, cur, tolerance: 0.01)); } #endif } @@ -380,7 +398,9 @@ namespace Content.Shared.Chemistry.Components public void AddReagent(ReagentPrototype proto, ReagentId reagentId, FixedPoint2 quantity) { AddReagent(reagentId, quantity, false); + _heatCapacity += quantity.Float() * proto.SpecificHeat; + CheckRecalculateHeatCapacity(); } public void AddReagent(ReagentQuantity reagentQuantity) @@ -419,6 +439,7 @@ namespace Content.Shared.Chemistry.Components _heatCapacity *= scale; Volume *= scale; + CheckRecalculateHeatCapacity(); for (int i = 0; i < Contents.Count; i++) { @@ -752,6 +773,7 @@ namespace Content.Shared.Chemistry.Components } _heatCapacity += otherSolution._heatCapacity; + CheckRecalculateHeatCapacity(); if (closeTemps) _heatCapacityDirty |= otherSolution._heatCapacityDirty; else diff --git a/Content.Shared/FixedPoint/FixedPoint2.cs b/Content.Shared/FixedPoint/FixedPoint2.cs index eb0e75e7ab..33a9d25bc2 100644 --- a/Content.Shared/FixedPoint/FixedPoint2.cs +++ b/Content.Shared/FixedPoint/FixedPoint2.cs @@ -20,6 +20,9 @@ namespace Content.Shared.FixedPoint public static FixedPoint2 Epsilon { get; } = new(1); public static FixedPoint2 Zero { get; } = new(0); + // This value isn't picked by any proper testing, don't @ me. + private const float FloatEpsilon = 0.00001f; + #if DEBUG static FixedPoint2() { @@ -47,7 +50,17 @@ namespace Content.Shared.FixedPoint public static FixedPoint2 New(float value) { - return new((int) MathF.Round(value * ShiftConstant, MidpointRounding.AwayFromZero)); + return new((int) ApplyFloatEpsilon(value * ShiftConstant)); + } + + private static float ApplyFloatEpsilon(float value) + { + return value + FloatEpsilon * Math.Sign(value); + } + + private static double ApplyFloatEpsilon(double value) + { + return value + FloatEpsilon * Math.Sign(value); } /// @@ -60,17 +73,12 @@ namespace Content.Shared.FixedPoint public static FixedPoint2 New(double value) { - return new((int) Math.Round(value * ShiftConstant, MidpointRounding.AwayFromZero)); + return new((int) ApplyFloatEpsilon(value * ShiftConstant)); } public static FixedPoint2 New(string value) { - return New(FloatFromString(value)); - } - - private static float FloatFromString(string value) - { - return float.Parse(value, CultureInfo.InvariantCulture); + return New(Parse.Float(value)); } public static FixedPoint2 operator +(FixedPoint2 a) => a; @@ -85,17 +93,17 @@ namespace Content.Shared.FixedPoint public static FixedPoint2 operator *(FixedPoint2 a, FixedPoint2 b) { - return new((int) MathF.Round(b.Value * a.Value / (float) ShiftConstant, MidpointRounding.AwayFromZero)); + return new(b.Value * a.Value / ShiftConstant); } public static FixedPoint2 operator *(FixedPoint2 a, float b) { - return new((int) MathF.Round(a.Value * b, MidpointRounding.AwayFromZero)); + return new((int) ApplyFloatEpsilon(a.Value * b)); } public static FixedPoint2 operator *(FixedPoint2 a, double b) { - return new((int) Math.Round(a.Value * b, MidpointRounding.AwayFromZero)); + return new((int) ApplyFloatEpsilon(a.Value * b)); } public static FixedPoint2 operator *(FixedPoint2 a, int b) @@ -105,12 +113,12 @@ namespace Content.Shared.FixedPoint public static FixedPoint2 operator /(FixedPoint2 a, FixedPoint2 b) { - return new((int) MathF.Round((ShiftConstant * a.Value) / (float) b.Value, MidpointRounding.AwayFromZero)); + return new((int) (ShiftConstant * (long) a.Value / b.Value)); } public static FixedPoint2 operator /(FixedPoint2 a, float b) { - return new((int) MathF.Round(a.Value / b, MidpointRounding.AwayFromZero)); + return new((int) ApplyFloatEpsilon(a.Value / b)); } public static bool operator <=(FixedPoint2 a, int b) @@ -185,7 +193,7 @@ namespace Content.Shared.FixedPoint public readonly int Int() { - return (int) ShiftDown(); + return Value / ShiftConstant; } // Implicit operators ftw @@ -266,7 +274,7 @@ namespace Content.Shared.FixedPoint if (value == "MaxValue") Value = int.MaxValue; else - this = New(FloatFromString(value)); + this = New(Parse.Float(value)); } public override readonly string ToString() => $"{ShiftDown().ToString(CultureInfo.InvariantCulture)}"; diff --git a/Content.Tests/Shared/Chemistry/FixedPoint2_Tests.cs b/Content.Tests/Shared/Chemistry/FixedPoint2_Tests.cs index 5ad157c747..8f9de2c2f8 100644 --- a/Content.Tests/Shared/Chemistry/FixedPoint2_Tests.cs +++ b/Content.Tests/Shared/Chemistry/FixedPoint2_Tests.cs @@ -4,7 +4,7 @@ using NUnit.Framework; namespace Content.Tests.Shared.Chemistry { - [TestFixture, TestOf(typeof(FixedPoint2))] + [TestFixture, TestOf(typeof(FixedPoint2)), Parallelizable] public sealed class FixedPoint2_Tests { [Test] @@ -18,8 +18,11 @@ namespace Content.Tests.Shared.Chemistry } [Test] - [TestCase(1.001f, "1")] - [TestCase(0.999f, "1")] + [TestCase(0.999f, "0.99")] + [TestCase(1.005f, "1")] + [TestCase(1.015f, "1.01")] + [TestCase(1.05f, "1.05")] + [TestCase(-1.05f, "-1.05")] public void FixedPoint2FloatTests(float value, string expected) { var result = FixedPoint2.New(value); @@ -27,8 +30,10 @@ namespace Content.Tests.Shared.Chemistry } [Test] - [TestCase(1.001d, "1")] - [TestCase(0.999d, "1")] + [TestCase(0.999, "0.99")] + [TestCase(1.005, "1")] + [TestCase(1.015, "1.01")] + [TestCase(1.05, "1.05")] public void FixedPoint2DoubleTests(double value, string expected) { var result = FixedPoint2.New(value); @@ -36,8 +41,10 @@ namespace Content.Tests.Shared.Chemistry } [Test] - [TestCase("1.005", "1.01")] - [TestCase("0.999", "1")] + [TestCase("0.999", "0.99")] + [TestCase("1.005", "1")] + [TestCase("1.015", "1.01")] + [TestCase("1.05", "1.05")] public void FixedPoint2StringTests(string value, string expected) { var result = FixedPoint2.New(value); @@ -45,11 +52,9 @@ namespace Content.Tests.Shared.Chemistry } [Test] - [TestCase(1.001f, 1.001f, "2")] - [TestCase(1.001f, 1.004f, "2")] - [TestCase(1f, 1.005f, "2.01")] - [TestCase(1f, 2.005f, "3.01")] - public void CalculusPlus(float aFloat, float bFloat, string expected) + [TestCase(1, 1, "2")] + [TestCase(1.05f, 1, "2.05")] + public void ArithmeticAddition(float aFloat, float bFloat, string expected) { var a = FixedPoint2.New(aFloat); var b = FixedPoint2.New(bFloat); @@ -60,10 +65,9 @@ namespace Content.Tests.Shared.Chemistry } [Test] - [TestCase(1.001f, 1.001f, "0")] - [TestCase(1.001f, 1.004f, "0")] - [TestCase(1f, 2.005f, "-1.01")] - public void CalculusMinus(float aFloat, float bFloat, string expected) + [TestCase(1, 1, "0")] + [TestCase(1f, 2.5f, "-1.5")] + public void ArithmeticSubtraction(float aFloat, float bFloat, string expected) { var a = FixedPoint2.New(aFloat); var b = FixedPoint2.New(bFloat); @@ -77,7 +81,8 @@ namespace Content.Tests.Shared.Chemistry [TestCase(1.001f, 3f, "0.33")] [TestCase(0.999f, 3f, "0.33")] [TestCase(2.1f, 3f, "0.7")] - public void CalculusDivision(float aFloat, float bFloat, string expected) + [TestCase(0.03f, 2f, "0.01")] + public void ArithmeticDivision(float aFloat, float bFloat, string expected) { var a = FixedPoint2.New(aFloat); var b = FixedPoint2.New(bFloat); @@ -88,9 +93,24 @@ namespace Content.Tests.Shared.Chemistry } [Test] - [TestCase(1.001f, 0.999f, "1")] - [TestCase(0.999f, 3f, "3")] - public void CalculusMultiplication(float aFloat, float bFloat, string expected) + [TestCase(1.001f, 3f, "0.33")] + [TestCase(0.999f, 3f, "0.33")] + [TestCase(2.1f, 3f, "0.7")] + [TestCase(0.03f, 2f, "0.01")] + [TestCase(1f, 1 / 1.05f, "1.05")] + public void ArithmeticDivisionFloat(float aFloat, float b, string expected) + { + var a = FixedPoint2.New(aFloat); + + var result = a / b; + + Assert.That($"{result}", Is.EqualTo(expected)); + } + + [Test] + [TestCase(1, 1, "1")] + [TestCase(1, 3f, "3")] + public void ArithmeticMultiplication(float aFloat, float bFloat, string expected) { var a = FixedPoint2.New(aFloat); var b = FixedPoint2.New(bFloat); @@ -100,6 +120,17 @@ namespace Content.Tests.Shared.Chemistry Assert.That($"{result}", Is.EqualTo(expected)); } + [Test] + [TestCase(1, 1, "1")] + [TestCase(1, 1.05f, "1.05")] + public void ArithmeticMultiplicationFloat(float aFloat, float b, string expected) + { + var a = FixedPoint2.New(aFloat); + var result = a * b; + + Assert.That($"{result}", Is.EqualTo(expected)); + } + [Test] [TestCase(0.995f, 100)] [TestCase(1.005f, 101)] diff --git a/Content.Tests/Shared/DamageTest.cs b/Content.Tests/Shared/DamageTest.cs index b06b8368a9..11b810bf36 100644 --- a/Content.Tests/Shared/DamageTest.cs +++ b/Content.Tests/Shared/DamageTest.cs @@ -121,9 +121,9 @@ namespace Content.Tests.Shared damageSpec = new(_prototypeManager.Index("Brute"), 4); Assert.That(damageSpec.DamageDict.TryGetValue("Blunt", out damage)); Assert.That(damage, Is.EqualTo(FixedPoint2.New(1.33))); - Assert.That(damageSpec.DamageDict.TryGetValue("Piercing", out damage)); - Assert.That(damage, Is.EqualTo(FixedPoint2.New(1.33))); Assert.That(damageSpec.DamageDict.TryGetValue("Slash", out damage)); + Assert.That(damage, Is.EqualTo(FixedPoint2.New(1.33))); + Assert.That(damageSpec.DamageDict.TryGetValue("Piercing", out damage)); Assert.That(damage, Is.EqualTo(FixedPoint2.New(1.34))); // doesn't divide evenly, so the 0.01 goes to the last one damageSpec = new(_prototypeManager.Index("Piercing"), 4); @@ -161,7 +161,7 @@ namespace Content.Tests.Shared Assert.That(damageSpec.DamageDict["Blunt"], Is.EqualTo(FixedPoint2.New(30))); Assert.That(damageSpec.DamageDict["Piercing"], Is.EqualTo(FixedPoint2.New(-40))); // resistances don't apply to healing Assert.That(!damageSpec.DamageDict.ContainsKey("Slash")); // Reduction reduced to 0, and removed from specifier - Assert.That(damageSpec.DamageDict["Radiation"], Is.EqualTo(FixedPoint2.New(65.63))); + Assert.That(damageSpec.DamageDict["Radiation"], Is.EqualTo(FixedPoint2.New(65.62))); } // Default damage Yaml