Solution precision fixes

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<SolutionComponent> solutionEnt = default;
+
+        await server.WaitPost(() =>
+        {
+            var system = server.System<SolutionContainerSystem>();
+            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();
+    }
+}

Content.IntegrationTests/Tests/Damageable/DamageableTest.cs

@@ -170,19 +170,19 @@ await server.WaitAssertion(() =>
 
                 // 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 @@ await server.WaitAssertion(() =>
 
                 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)));
                 });

Content.Shared/Chemistry/Components/Solution.cs

@@ -92,6 +92,12 @@ public bool CanAddSolution(Solution solution)
         /// </summary>
         [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 @@ public void UpdateHeatCapacity(IPrototypeManager? protoMan)
                 _heatCapacity += (float) quantity *
                                     protoMan.Index<ReagentPrototype>(reagent.Prototype).SpecificHeat;
             }
+
+            _heatCapacityUpdateCounter = 0;
         }
 
         public float GetHeatCapacity(IPrototypeManager? protoMan)
@@ -112,6 +120,15 @@ public float GetHeatCapacity(IPrototypeManager? protoMan)
             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 @@ public Solution(Solution solution)
             Temperature = solution.Temperature;
             _heatCapacity = solution._heatCapacity;
             _heatCapacityDirty = solution._heatCapacityDirty;
+            _heatCapacityUpdateCounter = solution._heatCapacityUpdateCounter;
             ValidateSolution();
         }
 
@@ -193,7 +211,7 @@ public void ValidateSolution()
                 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 @@ public void AddReagent(ReagentId id, FixedPoint2 quantity, bool dirtyHeatCap = t
         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 @@ public void ScaleSolution(int scale)
 
             _heatCapacity *= scale;
             Volume *= scale;
+            CheckRecalculateHeatCapacity();
 
             for (int i = 0; i < Contents.Count; i++)
             {
@@ -752,6 +773,7 @@ public void AddSolution(Solution otherSolution, IPrototypeManager? protoMan)
             }
 
             _heatCapacity += otherSolution._heatCapacity;
+            CheckRecalculateHeatCapacity();
             if (closeTemps)
                 _heatCapacityDirty |= otherSolution._heatCapacityDirty;
             else

Content.Shared/FixedPoint/FixedPoint2.cs

@@ -20,6 +20,9 @@ public struct FixedPoint2 : ISelfSerialize, IComparable<FixedPoint2>, IEquatable
         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 @@ public static FixedPoint2 New(int value)
 
         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);
         }
 
         /// <summary>
@@ -60,17 +73,12 @@ public static FixedPoint2 NewCeiling(float value)
 
         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 @@ private static float FloatFromString(string value)
 
         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 @@ private static float FloatFromString(string value)
 
         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 @@ public readonly double Double()
 
         public readonly int Int()
         {
-            return (int) ShiftDown();
+            return Value / ShiftConstant;
         }
 
         // Implicit operators ftw
@@ -266,7 +274,7 @@ public void Deserialize(string value)
             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)}";