Merge pull request #19919 from khang06/nan-sv

Emulate osu!stable's NaN slider velocity behavior

osu.Game.Rulesets.Osu/Objects/Slider.cs

@@ -14,6 +14,7 @@
 using osu.Game.Audio;
 using osu.Game.Beatmaps;
 using osu.Game.Beatmaps.ControlPoints;
+using osu.Game.Beatmaps.Formats;
 using osu.Game.Rulesets.Judgements;
 using osu.Game.Rulesets.Osu.Judgements;
 using osu.Game.Rulesets.Scoring;
@@ -165,11 +166,15 @@ protected override void ApplyDefaultsToSelf(ControlPointInfo controlPointInfo, I
             base.ApplyDefaultsToSelf(controlPointInfo, difficulty);
 
             TimingControlPoint timingPoint = controlPointInfo.TimingPointAt(StartTime);
+#pragma warning disable 618
+            var legacyDifficultyPoint = DifficultyControlPoint as LegacyBeatmapDecoder.LegacyDifficultyControlPoint;
+#pragma warning restore 618
 
             double scoringDistance = BASE_SCORING_DISTANCE * difficulty.SliderMultiplier * DifficultyControlPoint.SliderVelocity;
+            bool generateTicks = legacyDifficultyPoint?.GenerateTicks ?? true;
 
             Velocity = scoringDistance / timingPoint.BeatLength;
-            TickDistance = scoringDistance / difficulty.SliderTickRate * TickDistanceMultiplier;
+            TickDistance = generateTicks ? (scoringDistance / difficulty.SliderTickRate * TickDistanceMultiplier) : double.PositiveInfinity;
         }
 
         protected override void CreateNestedHitObjects(CancellationToken cancellationToken)

osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapDecoderTest.cs

@@ -919,5 +919,30 @@ public void TestLegacyDuplicateInitialCatmullPointIsMerged()
                 Assert.That(controlPoints[1].Position, Is.Not.EqualTo(Vector2.Zero));
             }
         }
+
+        [Test]
+        public void TestNaNControlPoints()
+        {
+            var decoder = new LegacyBeatmapDecoder { ApplyOffsets = false };
+
+            using (var resStream = TestResources.OpenResource("nan-control-points.osu"))
+            using (var stream = new LineBufferedReader(resStream))
+            {
+                var controlPoints = (LegacyControlPointInfo)decoder.Decode(stream).ControlPointInfo;
+
+                Assert.That(controlPoints.TimingPoints.Count, Is.EqualTo(1));
+                Assert.That(controlPoints.DifficultyPoints.Count, Is.EqualTo(2));
+
+                Assert.That(controlPoints.TimingPointAt(1000).BeatLength, Is.EqualTo(500));
+
+                Assert.That(controlPoints.DifficultyPointAt(2000).SliderVelocity, Is.EqualTo(1));
+                Assert.That(controlPoints.DifficultyPointAt(3000).SliderVelocity, Is.EqualTo(1));
+
+#pragma warning disable 618
+                Assert.That(((LegacyBeatmapDecoder.LegacyDifficultyControlPoint)controlPoints.DifficultyPointAt(2000)).GenerateTicks, Is.False);
+                Assert.That(((LegacyBeatmapDecoder.LegacyDifficultyControlPoint)controlPoints.DifficultyPointAt(3000)).GenerateTicks, Is.True);
+#pragma warning restore 618
+            }
+        }
     }
 }

osu.Game.Tests/Beatmaps/Formats/ParsingTest.cs

@@ -14,7 +14,12 @@ namespace osu.Game.Tests.Beatmaps.Formats
     public class ParsingTest
     {
         [Test]
-        public void TestNaNHandling() => allThrow<FormatException>("NaN");
+        public void TestNaNHandling()
+        {
+            allThrow<FormatException>("NaN");
+            Assert.That(Parsing.ParseFloat("NaN", allowNaN: true), Is.NaN);
+            Assert.That(Parsing.ParseDouble("NaN", allowNaN: true), Is.NaN);
+        }
 
         [Test]
         public void TestBadStringHandling() => allThrow<FormatException>("Random string 123");

osu.Game.Tests/Resources/nan-control-points.osu

@@ -0,0 +1,15 @@
+osu file format v14
+
+[TimingPoints]
+
+// NaN bpm (should be rejected)
+0,NaN,4,2,0,100,1,0
+
+// 120 bpm
+1000,500,4,2,0,100,1,0
+
+// NaN slider velocity
+2000,NaN,4,3,0,100,0,1
+
+// 1.0x slider velocity
+3000,-100,4,3,0,100,0,1

osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs

@@ -373,7 +373,11 @@ private void handleTimingPoint(string line)
             string[] split = line.Split(',');
 
             double time = getOffsetTime(Parsing.ParseDouble(split[0].Trim()));
-            double beatLength = Parsing.ParseDouble(split[1].Trim());
+
+            // beatLength is allowed to be NaN to handle an edge case in which some beatmaps use NaN slider velocity to disable slider tick generation (see LegacyDifficultyControlPoint).
+            double beatLength = Parsing.ParseDouble(split[1].Trim(), allowNaN: true);
+
+            // If beatLength is NaN, speedMultiplier should still be 1 because all comparisons against NaN are false.
             double speedMultiplier = beatLength < 0 ? 100.0 / -beatLength : 1;
 
             TimeSignature timeSignature = TimeSignature.SimpleQuadruple;
@@ -412,6 +416,9 @@ private void handleTimingPoint(string line)
 
             if (timingChange)
             {
+                if (double.IsNaN(beatLength))
+                    throw new InvalidDataException("Beat length cannot be NaN in a timing control point");
+
                 var controlPoint = CreateTimingControlPoint();
 
                 controlPoint.BeatLength = beatLength;

osu.Game/Beatmaps/Formats/LegacyDecoder.cs

@@ -168,23 +168,35 @@ public class LegacyDifficultyControlPoint : DifficultyControlPoint, IEquatable<L
             /// </summary>
             public double BpmMultiplier { get; private set; }
 
+            /// <summary>
+            /// Whether or not slider ticks should be generated at this control point.
+            /// This exists for backwards compatibility with maps that abuse NaN slider velocity behavior on osu!stable (e.g. /b/2628991).
+            /// </summary>
+            public bool GenerateTicks { get; private set; } = true;
+
             public LegacyDifficultyControlPoint(double beatLength)
                 : this()
             {
                 // Note: In stable, the division occurs on floats, but with compiler optimisations turned on actually seems to occur on doubles via some .NET black magic (possibly inlining?).
                 BpmMultiplier = beatLength < 0 ? Math.Clamp((float)-beatLength, 10, 10000) / 100.0 : 1;
+                GenerateTicks = !double.IsNaN(beatLength);
             }
 
             public LegacyDifficultyControlPoint()
             {
                 SliderVelocityBindable.Precision = double.Epsilon;
             }
 
+            public override bool IsRedundant(ControlPoint? existing)
+                => base.IsRedundant(existing)
+                   && GenerateTicks == ((existing as LegacyDifficultyControlPoint)?.GenerateTicks ?? true);
+
             public override void CopyFrom(ControlPoint other)
             {
                 base.CopyFrom(other);
 
                 BpmMultiplier = ((LegacyDifficultyControlPoint)other).BpmMultiplier;
+                GenerateTicks = ((LegacyDifficultyControlPoint)other).GenerateTicks;
             }
 
             public override bool Equals(ControlPoint? other)
@@ -193,10 +205,11 @@ public override bool Equals(ControlPoint? other)
 
             public bool Equals(LegacyDifficultyControlPoint? other)
                 => base.Equals(other)
-                   && BpmMultiplier == other.BpmMultiplier;
+                   && BpmMultiplier == other.BpmMultiplier
+                   && GenerateTicks == other.GenerateTicks;
 
-            // ReSharper disable once NonReadonlyMemberInGetHashCode
-            public override int GetHashCode() => HashCode.Combine(base.GetHashCode(), BpmMultiplier);
+            // ReSharper disable twice NonReadonlyMemberInGetHashCode
+            public override int GetHashCode() => HashCode.Combine(base.GetHashCode(), BpmMultiplier, GenerateTicks);
         }
 
         internal class LegacySampleControlPoint : SampleControlPoint, IEquatable<LegacySampleControlPoint>

osu.Game/Beatmaps/Formats/Parsing.cs

@@ -17,26 +17,26 @@ public static class Parsing
 
         public const double MAX_PARSE_VALUE = int.MaxValue;
 
-        public static float ParseFloat(string input, float parseLimit = (float)MAX_PARSE_VALUE)
+        public static float ParseFloat(string input, float parseLimit = (float)MAX_PARSE_VALUE, bool allowNaN = false)
         {
             float output = float.Parse(input, CultureInfo.InvariantCulture);
 
             if (output < -parseLimit) throw new OverflowException("Value is too low");
             if (output > parseLimit) throw new OverflowException("Value is too high");
 
-            if (float.IsNaN(output)) throw new FormatException("Not a number");
+            if (!allowNaN && float.IsNaN(output)) throw new FormatException("Not a number");
 
             return output;
         }
 
-        public static double ParseDouble(string input, double parseLimit = MAX_PARSE_VALUE)
+        public static double ParseDouble(string input, double parseLimit = MAX_PARSE_VALUE, bool allowNaN = false)
         {
             double output = double.Parse(input, CultureInfo.InvariantCulture);
 
             if (output < -parseLimit) throw new OverflowException("Value is too low");
             if (output > parseLimit) throw new OverflowException("Value is too high");
 
-            if (double.IsNaN(output)) throw new FormatException("Not a number");
+            if (!allowNaN && double.IsNaN(output)) throw new FormatException("Not a number");
 
             return output;
         }