Fix(Humanoid): Prevent skin color verification failures due to precision loss (#42836)

port over uberrations fixes.

Content.Shared/Humanoid/SkinColorationPrototype.cs

@@ -1,3 +1,4 @@
+using System;
 using System.Numerics;
 using JetBrains.Annotations;
 using Robust.Shared.Prototypes;
@@ -22,7 +23,7 @@ public sealed partial class SkinColorationPrototype : IPrototype
 }
 
 /// <summary>
-/// The type of input taken by a <see cref="SkinColorationStrategy" />
+/// The type of input taken by a <see cref="ISkinColorationStrategy" />
 /// </summary>
 [Serializable, NetSerializable]
 public enum SkinColorationStrategyInput
@@ -151,10 +152,12 @@ public sealed partial class HumanTonedSkinColoration : ISkinColorationStrategy
 
         if (rangeOffset <= 0)
         {
+            // First 20 values adjust hue.
             hue += Math.Abs(rangeOffset);
         }
         else
         {
+            // Remaining 80 values adjust saturation and value.
             sat += rangeOffset;
             val -= rangeOffset;
         }
@@ -182,14 +185,15 @@ public sealed partial class HumanTonedSkinColoration : ISkinColorationStrategy
 }
 
 /// <summary>
-/// Unary coloration strategy that clamps the color within the HSV colorspace
+/// Coloration strategy that clamps the color within the HSV colorspace.
 /// </summary>
 [DataDefinition]
 [Serializable, NetSerializable]
 public sealed partial class ClampedHsvColoration : ISkinColorationStrategy
 {
     /// <summary>
-    /// The (min, max) of the hue channel.
+    /// Defines the valid (min, max) range for the hue channel (0.0 to 1.0).
+    /// If min > max, the range wraps around 1.0 (e.g., for reds).
     /// </summary>
     [DataField]
     public (float, float)? Hue;
@@ -212,13 +216,13 @@ public sealed partial class ClampedHsvColoration : ISkinColorationStrategy
     {
         var hsv = Color.ToHsv(color);
 
-        if (Hue is (var minHue, var maxHue) && (hsv.X < minHue || hsv.X > maxHue))
+        if (Hue is (var minHue, var maxHue) && !SkinColorationUtils.IsHueInRange(hsv.X, minHue, maxHue))
             return false;
 
-        if (Saturation is (var minSaturation, var maxSaturation) && (hsv.Y < minSaturation || hsv.Y > maxSaturation))
+        if (Saturation is (var minSat, var maxSat) && (hsv.Y < minSat - SkinColorationUtils.Epsilon || hsv.Y > maxSat + SkinColorationUtils.Epsilon))
             return false;
 
-        if (Value is (var minValue, var maxValue) && (hsv.Z < minValue || hsv.Z > maxValue))
+        if (Value is (var minVal, var maxVal) && (hsv.Z < minVal - SkinColorationUtils.Epsilon || hsv.Z > maxVal + SkinColorationUtils.Epsilon))
             return false;
 
         return true;
@@ -229,27 +233,26 @@ public sealed partial class ClampedHsvColoration : ISkinColorationStrategy
         var hsv = Color.ToHsv(color);
 
         if (Hue is (var minHue, var maxHue))
-            hsv.X = Math.Clamp(hsv.X, minHue, maxHue);
-
-        if (Saturation is (var minSaturation, var maxSaturation))
-            hsv.Y = Math.Clamp(hsv.Y, minSaturation, maxSaturation);
-
-        if (Value is (var minValue, var maxValue))
-            hsv.Z = Math.Clamp(hsv.Z, minValue, maxValue);
+            hsv.X = SkinColorationUtils.ClampHue(hsv.X, minHue, maxHue);
+        if (Saturation is (var minSat, var maxSat))
+            hsv.Y = Math.Clamp(hsv.Y, minSat, maxSat);
+        if (Value is (var minVal, var maxVal))
+            hsv.Z = Math.Clamp(hsv.Z, minVal, maxVal);
 
         return Color.FromHsv(hsv);
     }
 }
 
 /// <summary>
-/// Unary coloration strategy that clamps the color within the HSL colorspace
+/// Coloration strategy that clamps the color within the HSL colorspace.
 /// </summary>
 [DataDefinition]
 [Serializable, NetSerializable]
 public sealed partial class ClampedHslColoration : ISkinColorationStrategy
 {
     /// <summary>
-    /// The (min, max) of the hue channel.
+    /// Defines the valid (min, max) range for the hue channel (0.0 to 1.0).
+    /// If min > max, the range wraps around 1.0 (e.g., for reds).
     /// </summary>
     [DataField]
     public (float, float)? Hue;
@@ -272,13 +275,13 @@ public sealed partial class ClampedHslColoration : ISkinColorationStrategy
     {
         var hsl = Color.ToHsl(color);
 
-        if (Hue is (var minHue, var maxHue) && (hsl.X < minHue || hsl.X > maxHue))
+        if (Hue is (var minHue, var maxHue) && !SkinColorationUtils.IsHueInRange(hsl.X, minHue, maxHue))
             return false;
 
-        if (Saturation is (var minSaturation, var maxSaturation) && (hsl.Y < minSaturation || hsl.Y > maxSaturation))
+        if (Saturation is (var minSat, var maxSat) && (hsl.Y < minSat - SkinColorationUtils.Epsilon || hsl.Y > maxSat + SkinColorationUtils.Epsilon))
             return false;
 
-        if (Lightness is (var minValue, var maxValue) && (hsl.Z < minValue || hsl.Z > maxValue))
+        if (Lightness is (var minLight, var maxLight) && (hsl.Z < minLight - SkinColorationUtils.Epsilon || hsl.Z > maxLight + SkinColorationUtils.Epsilon))
             return false;
 
         return true;
@@ -289,14 +292,64 @@ public sealed partial class ClampedHslColoration : ISkinColorationStrategy
         var hsl = Color.ToHsl(color);
 
         if (Hue is (var minHue, var maxHue))
-            hsl.X = Math.Clamp(hsl.X, minHue, maxHue);
-
-        if (Saturation is (var minSaturation, var maxSaturation))
-            hsl.Y = Math.Clamp(hsl.Y, minSaturation, maxSaturation);
-
-        if (Lightness is (var minValue, var maxValue))
-            hsl.Z = Math.Clamp(hsl.Z, minValue, maxValue);
+            hsl.X = SkinColorationUtils.ClampHue(hsl.X, minHue, maxHue);
+        if (Saturation is (var minSat, var maxSat))
+            hsl.Y = Math.Clamp(hsl.Y, minSat, maxSat);
+        if (Lightness is (var minLight, var maxLight))
+            hsl.Z = Math.Clamp(hsl.Z, minLight, maxLight);
 
         return Color.FromHsl(hsl);
     }
 }
+
+/// <summary>
+/// Contains shared utility methods for handling color manipulations in skin coloration strategies.
+/// </summary>
+internal static class SkinColorationUtils
+{
+    /// <summary>
+    /// An empirically determined epsilon to account for floating-point drift during RGB -> HSL/HSV -> RGB conversions.
+    /// Based on high-iteration testing (50M+ samples) which showed a max drift of ~4.9E-6 for HSL.
+    /// A value of 1E-5f provides a robust safety margin.
+    /// </summary>
+    public const float Epsilon = 1e-5f; // 0.00001
+
+    /// <summary>
+    /// Checks if a hue value is within a specified range, correctly handling ranges that wrap around 1.0 (e.g., reds).
+    /// </summary>
+    /// <param name="hue">The hue value to check (0.0 to 1.0).</param>
+    /// <param name="minHue">The minimum bound of the hue range.</param>
+    /// <param name="maxHue">The maximum bound of the hue range.</param>
+    /// <returns>True if the hue is within the range; otherwise, false.</returns>
+    public static bool IsHueInRange(float hue, float minHue, float maxHue)
+    {
+        if (minHue > maxHue) // Wraps around 1.0 (e.g., reds)
+            return hue >= minHue - Epsilon || hue <= maxHue + Epsilon;
+        return hue >= minHue - Epsilon && hue <= maxHue + Epsilon;
+    }
+
+    /// <summary>
+    /// Clamps a hue value to the closest boundary of a given range, correctly handling ranges that wrap around 1.0.
+    /// </summary>
+    /// <param name="hue">The hue value to clamp (0.0 to 1.0).</param>
+    /// <param name="minHue">The minimum bound of the hue range.</param>
+    /// <param name="maxHue">The maximum bound of the hue range.</param>
+    /// <returns>The clamped hue value, adjusted to the nearest boundary if it was outside the valid range.</returns>
+    public static float ClampHue(float hue, float minHue, float maxHue)
+    {
+        if (minHue > maxHue) // Wraps around 1.0
+        {
+            // If it's already in the valid range, do nothing.
+            if (hue >= minHue || hue <= maxHue)
+                return hue;
+
+            // It's in the "invalid" gap between maxHue and minHue. Find the closest boundary.
+            var mid = (maxHue + minHue) / 2f;
+            if (hue > mid)
+                return minHue;
+            return maxHue;
+        }
+
+        return Math.Clamp(hue, minHue, maxHue);
+    }
+}

Content.Tests/Shared/Preferences/Humanoid/SkinTonesTest.cs

@@ -1,28 +1,294 @@
+using System;
+using System.Numerics;
 using Content.Shared.Humanoid;
 using NUnit.Framework;
+using Robust.Shared.Maths;
+using Robust.Shared.Random;
 
 namespace Content.Tests.Shared.Preferences.Humanoid;
 
 [TestFixture]
+[TestOf(typeof(HumanTonedSkinColoration))]
+[TestOf(typeof(ClampedHslColoration))]
+[TestOf(typeof(ClampedHsvColoration))]
 public sealed class SkinTonesTest
 {
+    // These fields will track the maximum observed floating-point drift across all tests.
+    // This is for monitoring, even if tests pass due to a sufficiently large Epsilon in production code.
+    private static float _maxHslDrift;
+    private static float _maxHsvDrift;
+
+    [OneTimeTearDown]
+    public void OneTimeTearDown()
+    {
+        // After all tests in this fixture run, print the final results.
+        // This gives insight into the actual precision loss, even if VerifySkinColor passes.
+        TestContext.Out.WriteLine("\n--- FINAL DRIFT SUMMARY FOR ALL CLAMPING TESTS ---");
+        TestContext.Out.WriteLine($"Maximum observed HSL drift: {_maxHslDrift:E}"); // Scientific notation for precision
+        TestContext.Out.WriteLine($"Maximum observed HSV drift: {_maxHsvDrift:E}");
+        TestContext.Out.WriteLine("This indicates the actual max floating-point error observed. Production code's Epsilon should be >= this value.");
+        TestContext.Out.WriteLine("--------------------------------------------------");
+    }
+
+    /// <summary>
+    /// Checks that colors generated by HumanTonedSkinColoration.FromUnary pass verification.
+    /// </summary>
     [Test]
-    public void TestHumanSkinToneValidity()
+    public void TestHumanSkinTonesFromUnaryAreValid()
     {
         var strategy = new HumanTonedSkinColoration();
 
-        for (var i = 0; i <= 100; i++)
+        // Testing across a finer range to hit more edge cases
+        for (var i = 0; i <= 10000; i++)
         {
-            var color = strategy.FromUnary(i);
-            Assert.That(strategy.VerifySkinColor(color));
+            var unaryInput = i / 100f; // Test values like 0.0, 0.01, ..., 100.0
+            var color = strategy.FromUnary(unaryInput);
+            Assert.That(strategy.VerifySkinColor(color), $"Color {color} from unary value {unaryInput} failed verification.");
         }
     }
 
+    /// <summary>
+    /// Checks that converting a unary value to a color and back results in a similar unary value.
+    /// </summary>
     [Test]
-    public void TestDefaultSkinToneValid()
+    public void TestHumanTonedSkinColoration_RoundTrip()
     {
         var strategy = new HumanTonedSkinColoration();
+        // Test values across the full range, including transition points
+        for (var i = 0; i <= 10000; i++)
+        {
+            var originalUnary = i / 100f;
+            var color = strategy.FromUnary(originalUnary);
+            var resultUnary = strategy.ToUnary(color);
 
+            // A small tolerance is expected due to float precision and the nature of HSV conversions
+            // as well as the rounding logic in ToUnary.
+            Assert.That(resultUnary, Is.EqualTo(originalUnary).Within(1e-2f), // 1e-2f (0.01) is 1% of the unary range, which is reasonable for rounding and float error.
+                $"Round trip failed for unary {originalUnary}. Got {resultUnary} back.");
+        }
+    }
+
+    /// <summary>
+    /// Checks that the default human skin tone is considered valid.
+    /// </summary>
+    [Test]
+    public void TestDefaultHumanSkinToneValid()
+    {
+        var strategy = new HumanTonedSkinColoration();
         Assert.That(strategy.VerifySkinColor(strategy.ValidHumanSkinTone));
     }
+
+    /// <summary>
+    /// Checks that clamping random colors with a low-saturation, high-lightness HSL strategy produces valid colors.
+    /// This was the primary test case that originally revealed the precision bug.
+    /// </summary>
+    [Test]
+    public void TestTintedHuesValidHsl()
+    {
+        var random = new RobustRandom();
+        var strategy = new ClampedHslColoration()
+        {
+            Saturation = (0.0f, 0.1f),
+            Lightness = (0.85f, 1.0f),
+        };
+
+        for (var i = 0; i <= 10000; i++)
+        {
+            var color = new Color(random.NextFloat(), random.NextFloat(), random.NextFloat());
+            var skinColor = strategy.ClosestSkinColor(color);
+            LogDriftIfGreater(strategy, color, skinColor, TestContext.CurrentContext.Test.Name); // Monitor drift
+
+            Assert.That(strategy.VerifySkinColor(skinColor),
+                $"Color {skinColor} (from input {color}) failed verification in {TestContext.CurrentContext.Test.Name} on iteration {i}");
+        }
+    }
+
+    /// <summary>
+    /// Checks that clamping random colors with a low-saturation, high-value HSV strategy produces valid colors.
+    /// </summary>
+    [Test]
+    public void TestTintedHuesValidHsv()
+    {
+        var random = new RobustRandom();
+        var strategy = new ClampedHsvColoration()
+        {
+            Saturation = (0.0f, 0.1f),
+            Value = (0.85f, 1.0f),
+        };
+
+        for (var i = 0; i <= 10000; i++)
+        {
+            var color = new Color(random.NextFloat(), random.NextFloat(), random.NextFloat());
+            var skinColor = strategy.ClosestSkinColor(color);
+            LogDriftIfGreater(strategy, color, skinColor, TestContext.CurrentContext.Test.Name); // Monitor drift
+
+            Assert.That(strategy.VerifySkinColor(skinColor),
+                $"Color {skinColor} (from input {color}) failed verification in {TestContext.CurrentContext.Test.Name} on iteration {i}");
+        }
+    }
+
+    /// <summary>
+    /// Checks that clamping random colors with an HSL strategy that limits all three channels produces valid colors.
+    /// </summary>
+    [Test]
+    public void TestClampedHslWithAllChannels()
+    {
+        var random = new RobustRandom();
+        var strategy = new ClampedHslColoration()
+        {
+            Hue = (0.1f, 0.3f),
+            Saturation = (0.2f, 0.8f),
+            Lightness = (0.3f, 0.7f),
+        };
+
+        for (var i = 0; i <= 10000; i++)
+        {
+            var color = new Color(random.NextFloat(), random.NextFloat(), random.NextFloat());
+            var skinColor = strategy.ClosestSkinColor(color);
+            LogDriftIfGreater(strategy, color, skinColor, TestContext.CurrentContext.Test.Name); // Monitor drift
+
+            Assert.That(strategy.VerifySkinColor(skinColor),
+                $"Color {skinColor} (from input {color}) failed verification in {TestContext.CurrentContext.Test.Name} on iteration {i}");
+        }
+    }
+
+    /// <summary>
+    /// Checks that clamping random colors with an HSV strategy that limits all three channels produces valid colors.
+    /// </summary>
+    [Test]
+    public void TestClampedHsvWithAllChannels()
+    {
+        var random = new RobustRandom();
+        var strategy = new ClampedHsvColoration()
+        {
+            Hue = (0.1f, 0.3f),
+            Saturation = (0.2f, 0.8f),
+            Value = (0.3f, 0.7f),
+        };
+
+        for (var i = 0; i <= 10000; i++)
+        {
+            var color = new Color(random.NextFloat(), random.NextFloat(), random.NextFloat());
+            var skinColor = strategy.ClosestSkinColor(color);
+            LogDriftIfGreater(strategy, color, skinColor, TestContext.CurrentContext.Test.Name); // Monitor drift
+
+            Assert.That(strategy.VerifySkinColor(skinColor),
+                $"Color {skinColor} (from input {color}) failed verification in {TestContext.CurrentContext.Test.Name} on iteration {i}");
+        }
+    }
+
+    /// <summary>
+    /// Checks that clamping works correctly for HSL strategies where the hue range wraps around the 0-1 boundary.
+    /// </summary>
+    [Test]
+    public void TestClampedHslWithCircularHue()
+    {
+        var random = new RobustRandom();
+        var strategy = new ClampedHslColoration()
+        {
+            Hue = (0.9f, 0.1f), // A range that wraps around 1.0 (e.g., reds)
+            Saturation = (0.5f, 1.0f),
+            Lightness = (0.5f, 1.0f),
+        };
+
+        for (var i = 0; i <= 10000; i++)
+        {
+            var color = new Color(random.NextFloat(), random.NextFloat(), random.NextFloat());
+            var skinColor = strategy.ClosestSkinColor(color);
+            LogDriftIfGreater(strategy, color, skinColor, TestContext.CurrentContext.Test.Name); // Monitor drift
+
+            Assert.That(strategy.VerifySkinColor(skinColor),
+                $"Color {skinColor} (from input {color}) with circular hue failed verification in {TestContext.CurrentContext.Test.Name} on iteration {i}");
+        }
+    }
+
+    /// <summary>
+    /// Checks that a color that is already valid is not modified.
+    /// </summary>
+    [Test]
+    public void TestClosestSkinColorReturnsValidColor()
+    {
+        var strategy = new ClampedHslColoration()
+        {
+            Saturation = (0.0f, 1.0f),
+            Lightness = (0.0f, 1.0f),
+        };
+
+        var validColor = Color.FromHsl(new Vector4(0.5f, 0.5f, 0.5f, 1.0f));
+        var result = strategy.ClosestSkinColor(validColor);
+
+        Assert.That(strategy.VerifySkinColor(result), Is.True);
+    }
+
+    /// <summary>
+    /// Checks that a color outside the valid range is correctly clamped to a valid color.
+    /// </summary>
+    [Test]
+    public void TestClosestSkinColorClampsInvalidColor()
+    {
+        var strategy = new ClampedHslColoration()
+        {
+            Saturation = (0.0f, 0.1f),
+            Lightness = (0.85f, 1.0f),
+        };
+
+        // This color has high saturation and low lightness, should be clamped
+        var invalidColor = Color.FromHsl(new Vector4(0.5f, 0.9f, 0.2f, 1.0f));
+        var result = strategy.ClosestSkinColor(invalidColor);
+
+        Assert.That(strategy.VerifySkinColor(result), Is.True);
+        Assert.That(result, Is.Not.EqualTo(invalidColor));
+    }
+
+    /// <summary>
+    /// Helper method to calculate and log the maximum floating-point drift observed during clamping.
+    /// This is for monitoring the behavior of the clamping, not for causing test failures directly.
+    /// </summary>
+    private void LogDriftIfGreater(ISkinColorationStrategy strategy, Color original, Color clamped, string testName)
+    {
+        if (strategy is ClampedHslColoration hslStrategy)
+        {
+            var hsl = Color.ToHsl(clamped);
+            var (minSat, maxSat) = hslStrategy.Saturation ?? (0f, 1f);
+            var (minLight, maxLight) = hslStrategy.Lightness ?? (0f, 1f);
+
+            // Re-calculate the drift from the original bounds *without* applying Epsilon
+            // This shows the pure floating-point error relative to the intended boundaries.
+            var satDrift = Math.Max(minSat - hsl.Y, hsl.Y - maxSat);
+            var lightDrift = Math.Max(minLight - hsl.Z, hsl.Z - maxLight);
+            var currentDrift = Math.Max(satDrift, lightDrift);
+
+            if (currentDrift > _maxHslDrift)
+            {
+                TestContext.Out.WriteLine($"--- NEW MAX HSL DRIFT DETECTED in {testName} ---");
+                TestContext.Out.WriteLine($"Max HSL Drift: {currentDrift:E} (previously {_maxHslDrift:E})");
+                TestContext.Out.WriteLine($"Original RGB: {original}");
+                TestContext.Out.WriteLine($"Clamped RGB: {clamped}");
+                TestContext.Out.WriteLine($"Result HSL: H={hsl.X:F8}, S={hsl.Y:F8}, L={hsl.Z:F8}");
+                TestContext.Out.WriteLine($"Bounds: S=({minSat:F8}, {maxSat:F8}), L=({minLight:F8}, {maxLight:F8})");
+                _maxHslDrift = currentDrift;
+            }
+        }
+        else if (strategy is ClampedHsvColoration hsvStrategy)
+        {
+            var hsv = Color.ToHsv(clamped);
+            var (minSat, maxSat) = hsvStrategy.Saturation ?? (0f, 1f);
+            var (minValue, maxValue) = hsvStrategy.Value ?? (0f, 1f);
+
+            var satDrift = Math.Max(minSat - hsv.Y, hsv.Y - maxSat);
+            var valueDrift = Math.Max(minValue - hsv.Z, hsv.Z - maxValue);
+            var currentDrift = Math.Max(satDrift, valueDrift);
+
+            if (currentDrift > _maxHsvDrift)
+            {
+                TestContext.Out.WriteLine($"--- NEW MAX HSV DRIFT DETECTED in {testName} ---");
+                TestContext.Out.WriteLine($"Max HSV Drift: {currentDrift:E} (previously {_maxHsvDrift:E})");
+                TestContext.Out.WriteLine($"Original RGB: {original}");
+                TestContext.Out.WriteLine($"Clamped RGB: {clamped}");
+                TestContext.Out.WriteLine($"Result HSV: H={hsv.X:F8}, S={hsv.Y:F8}, V={hsv.Z:F8}");
+                TestContext.Out.WriteLine($"Bounds: S=({minSat:F8}, {maxSat:F8}), V=({minValue:F8}, {maxValue:F8})");
+                _maxHsvDrift = currentDrift;
+            }
+        }
+    }
 }