Merge pull request #24738 from peppy/no-legacy-difficulty-control-point

Remove `LegacyDifficultyControlPoint` / `LegacyBpmMultiplier`

osu.Game.Rulesets.Mania/Beatmaps/Patterns/Legacy/DistanceObjectPatternGenerator.cs

@@ -10,10 +10,11 @@ using System.Linq;
 using osu.Framework.Extensions.EnumExtensions;
 using osu.Game.Audio;
 using osu.Game.Beatmaps;
-using osu.Game.Rulesets.Objects;
-using osu.Game.Rulesets.Objects.Types;
-using osu.Game.Rulesets.Mania.Objects;
 using osu.Game.Beatmaps.ControlPoints;
+using osu.Game.Rulesets.Mania.Objects;
+using osu.Game.Rulesets.Objects;
+using osu.Game.Rulesets.Objects.Legacy;
+using osu.Game.Rulesets.Objects.Types;
 using osu.Game.Utils;
 
 namespace osu.Game.Rulesets.Mania.Beatmaps.Patterns.Legacy
@@ -50,10 +51,9 @@ namespace osu.Game.Rulesets.Mania.Beatmaps.Patterns.Legacy
             TimingControlPoint timingPoint = beatmap.ControlPointInfo.TimingPointAt(hitObject.StartTime);
 
             double beatLength;
-            if (hitObject.LegacyBpmMultiplier.HasValue)
+
-                beatLength = timingPoint.BeatLength * hitObject.LegacyBpmMultiplier.Value;
+            if (hitObject is IHasSliderVelocity hasSliderVelocity)
-            else if (hitObject is IHasSliderVelocity hasSliderVelocity)
+                beatLength = LegacyRulesetExtensions.GetPrecisionAdjustedBeatLength(hasSliderVelocity, timingPoint, ManiaRuleset.SHORT_NAME);
-                beatLength = timingPoint.BeatLength / hasSliderVelocity.SliderVelocityMultiplier;
             else
                 beatLength = timingPoint.BeatLength;
 

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

@@ -136,7 +136,6 @@ namespace osu.Game.Rulesets.Osu.Objects
 
         public BindableNumber<double> SliderVelocityMultiplierBindable { get; } = new BindableDouble(1)
         {
-            Precision = 0.01,
             MinValue = 0.1,
             MaxValue = 10
         };

osu.Game.Rulesets.Taiko/Beatmaps/TaikoBeatmapConverter.cs

@@ -14,6 +14,7 @@ using JetBrains.Annotations;
 using osu.Game.Audio;
 using osu.Game.Beatmaps.ControlPoints;
 using osu.Game.Beatmaps.Formats;
+using osu.Game.Rulesets.Objects.Legacy;
 
 namespace osu.Game.Rulesets.Taiko.Beatmaps
 {
@@ -186,10 +187,9 @@ namespace osu.Game.Rulesets.Taiko.Beatmaps
             TimingControlPoint timingPoint = beatmap.ControlPointInfo.TimingPointAt(obj.StartTime);
 
             double beatLength;
-            if (obj.LegacyBpmMultiplier.HasValue)
+
-                beatLength = timingPoint.BeatLength * obj.LegacyBpmMultiplier.Value;
+            if (obj is IHasSliderVelocity hasSliderVelocity)
-            else if (obj is IHasSliderVelocity hasSliderVelocity)
+                beatLength = LegacyRulesetExtensions.GetPrecisionAdjustedBeatLength(hasSliderVelocity, timingPoint, TaikoRuleset.SHORT_NAME);
-                beatLength = timingPoint.BeatLength / hasSliderVelocity.SliderVelocityMultiplier;
             else
                 beatLength = timingPoint.BeatLength;
 

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

@@ -1024,10 +1024,8 @@ namespace osu.Game.Tests.Beatmaps.Formats
                 Assert.That(controlPoints.DifficultyPointAt(2000).SliderVelocity, Is.EqualTo(1));
                 Assert.That(controlPoints.DifficultyPointAt(3000).SliderVelocity, Is.EqualTo(1));
 
-#pragma warning disable 618
+                Assert.That(controlPoints.DifficultyPointAt(2000).GenerateTicks, Is.False);
-                Assert.That(((LegacyBeatmapDecoder.LegacyDifficultyControlPoint)controlPoints.DifficultyPointAt(2000)).GenerateTicks, Is.False);
+                Assert.That(controlPoints.DifficultyPointAt(3000).GenerateTicks, Is.True);
-                Assert.That(((LegacyBeatmapDecoder.LegacyDifficultyControlPoint)controlPoints.DifficultyPointAt(3000)).GenerateTicks, Is.True);
-#pragma warning restore 618
             }
         }
     }

osu.Game/Beatmaps/ControlPoints/DifficultyControlPoint.cs

@@ -23,11 +23,16 @@ namespace osu.Game.Beatmaps.ControlPoints
         /// </summary>
         public readonly BindableDouble SliderVelocityBindable = new BindableDouble(1)
         {
-            Precision = 0.01,
             MinValue = 0.1,
             MaxValue = 10
         };
 
+        /// <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; set; } = true;
+
         public override Color4 GetRepresentingColour(OsuColour colours) => colours.Lime1;
 
         /// <summary>
@@ -41,11 +46,13 @@ namespace osu.Game.Beatmaps.ControlPoints
 
         public override bool IsRedundant(ControlPoint? existing)
             => existing is DifficultyControlPoint existingDifficulty
+               && GenerateTicks == existingDifficulty.GenerateTicks
                && SliderVelocity == existingDifficulty.SliderVelocity;
 
         public override void CopyFrom(ControlPoint other)
         {
             SliderVelocity = ((DifficultyControlPoint)other).SliderVelocity;
+            GenerateTicks = ((DifficultyControlPoint)other).GenerateTicks;
 
             base.CopyFrom(other);
         }
@@ -56,8 +63,10 @@ namespace osu.Game.Beatmaps.ControlPoints
 
         public bool Equals(DifficultyControlPoint? other)
             => base.Equals(other)
+               && GenerateTicks == other.GenerateTicks
                && SliderVelocity == other.SliderVelocity;
 
-        public override int GetHashCode() => HashCode.Combine(base.GetHashCode(), SliderVelocity);
+        // ReSharper disable once NonReadonlyMemberInGetHashCode
+        public override int GetHashCode() => HashCode.Combine(base.GetHashCode(), SliderVelocity, GenerateTicks);
     }
 }

osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs

@@ -1,8 +1,6 @@
 // Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
 // See the LICENCE file in the repository root for full licence text.
 
-#pragma warning disable 618
-
 using System;
 using System.Collections.Generic;
 using System.IO;
@@ -103,12 +101,8 @@ namespace osu.Game.Beatmaps.Formats
         {
             DifficultyControlPoint difficultyControlPoint = (beatmap.ControlPointInfo as LegacyControlPointInfo)?.DifficultyPointAt(hitObject.StartTime) ?? DifficultyControlPoint.DEFAULT;
 
-            if (difficultyControlPoint is LegacyDifficultyControlPoint legacyDifficultyControlPoint)
+            if (hitObject is IHasGenerateTicks hasGenerateTicks)
-            {
+                hasGenerateTicks.GenerateTicks = difficultyControlPoint.GenerateTicks;
-                hitObject.LegacyBpmMultiplier = legacyDifficultyControlPoint.BpmMultiplier;
-                if (hitObject is IHasGenerateTicks hasGenerateTicks)
-                    hasGenerateTicks.GenerateTicks = legacyDifficultyControlPoint.GenerateTicks;
-            }
 
             if (hitObject is IHasSliderVelocity hasSliderVelocity)
                 hasSliderVelocity.SliderVelocityMultiplier = difficultyControlPoint.SliderVelocity;
@@ -497,8 +491,9 @@ namespace osu.Game.Beatmaps.Formats
 
             int onlineRulesetID = beatmap.BeatmapInfo.Ruleset.OnlineID;
 
-            addControlPoint(time, new LegacyDifficultyControlPoint(onlineRulesetID, beatLength)
+            addControlPoint(time, new DifficultyControlPoint
             {
+                GenerateTicks = !double.IsNaN(beatLength),
                 SliderVelocity = speedMultiplier,
             }, timingChange);
 

osu.Game/Beatmaps/Formats/LegacyDecoder.cs

@@ -163,63 +163,6 @@ namespace osu.Game.Beatmaps.Formats
             Mania,
         }
 
-        [Obsolete("Do not use unless you're a legacy ruleset and 100% sure.")]
-        public class LegacyDifficultyControlPoint : DifficultyControlPoint, IEquatable<LegacyDifficultyControlPoint>
-        {
-            /// <summary>
-            /// Legacy BPM multiplier that introduces floating-point errors for rulesets that depend on it.
-            /// DO NOT USE THIS UNLESS 100% SURE.
-            /// </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(int rulesetId, 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?).
-                if (rulesetId == 1 || rulesetId == 3)
-                    BpmMultiplier = beatLength < 0 ? Math.Clamp((float)-beatLength, 10, 10000) / 100.0 : 1;
-                else
-                    BpmMultiplier = beatLength < 0 ? Math.Clamp((float)-beatLength, 10, 1000) / 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)
-                => other is LegacyDifficultyControlPoint otherLegacyDifficultyControlPoint
-                   && Equals(otherLegacyDifficultyControlPoint);
-
-            public bool Equals(LegacyDifficultyControlPoint? other)
-                => base.Equals(other)
-                   && BpmMultiplier == other.BpmMultiplier
-                   && GenerateTicks == other.GenerateTicks;
-
-            // ReSharper disable twice NonReadonlyMemberInGetHashCode
-            public override int GetHashCode() => HashCode.Combine(base.GetHashCode(), BpmMultiplier, GenerateTicks);
-        }
-
         internal class LegacySampleControlPoint : SampleControlPoint, IEquatable<LegacySampleControlPoint>
         {
             public int CustomSampleBank;

osu.Game/Rulesets/Objects/HitObject.cs

@@ -76,12 +76,6 @@ namespace osu.Game.Rulesets.Objects
         /// </summary>
         public virtual IList<HitSampleInfo> AuxiliarySamples => ImmutableList<HitSampleInfo>.Empty;
 
-        /// <summary>
-        /// Legacy BPM multiplier that introduces floating-point errors for rulesets that depend on it.
-        /// DO NOT USE THIS UNLESS 100% SURE.
-        /// </summary>
-        public double? LegacyBpmMultiplier { get; set; }
-
         /// <summary>
         /// Whether this <see cref="HitObject"/> is in Kiai time.
         /// </summary>

osu.Game/Rulesets/Objects/Legacy/LegacyRulesetExtensions.cs

@@ -0,0 +1,42 @@
+// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
+// See the LICENCE file in the repository root for full licence text.
+
+using System;
+using osu.Game.Beatmaps.ControlPoints;
+using osu.Game.Rulesets.Objects.Types;
+
+namespace osu.Game.Rulesets.Objects.Legacy
+{
+    public static class LegacyRulesetExtensions
+    {
+        /// <summary>
+        /// Introduces floating-point errors to post-multiplied beat length for legacy rulesets that depend on it.
+        /// You should definitely not use this unless you know exactly what you're doing.
+        /// </summary>
+        public static double GetPrecisionAdjustedBeatLength(IHasSliderVelocity hasSliderVelocity, TimingControlPoint timingControlPoint, string rulesetShortName)
+        {
+            double sliderVelocityAsBeatLength = -100 / hasSliderVelocity.SliderVelocityMultiplier;
+
+            // 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?).
+            double bpmMultiplier;
+
+            switch (rulesetShortName)
+            {
+                case "taiko":
+                case "mania":
+                    bpmMultiplier = sliderVelocityAsBeatLength < 0 ? Math.Clamp((float)-sliderVelocityAsBeatLength, 10, 10000) / 100.0 : 1;
+                    break;
+
+                case "osu":
+                case "fruits":
+                    bpmMultiplier = sliderVelocityAsBeatLength < 0 ? Math.Clamp((float)-sliderVelocityAsBeatLength, 10, 1000) / 100.0 : 1;
+                    break;
+
+                default:
+                    throw new ArgumentException("Must be a legacy ruleset", nameof(rulesetShortName));
+            }
+
+            return timingControlPoint.BeatLength * bpmMultiplier;
+        }
+    }
+}