Merge pull request #25689 from peppy/taiko-multiplier-fix

Fix osu!taiko slider velocity being written incorrectly to `.osu` file on export

osu.Game.Rulesets.Taiko.Tests/Editor/TestSceneTaikoEditorSaving.cs

@@ -1,10 +1,16 @@
 // 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 System.Globalization;
+using System.IO;
+using System.Linq;
 using NUnit.Framework;
+using osu.Framework.Extensions;
 using osu.Framework.Utils;
-using osu.Game.Rulesets.Taiko.Beatmaps;
+using osu.Game.Beatmaps;
 using osu.Game.Tests.Visual;
+using SharpCompress.Archives.Zip;
 
 namespace osu.Game.Rulesets.Taiko.Tests.Editor
 {
@@ -12,6 +18,44 @@ namespace osu.Game.Rulesets.Taiko.Tests.Editor
     {
         protected override Ruleset CreateRuleset() => new TaikoRuleset();
 
+        [TestCase(null)]
+        [TestCase(1f)]
+        [TestCase(2f)]
+        [TestCase(2.4f)]
+        public void TestTaikoSliderMultiplierInExport(float? multiplier)
+        {
+            if (multiplier.HasValue)
+                AddStep("Set slider multiplier", () => EditorBeatmap.Difficulty.SliderMultiplier = multiplier.Value);
+
+            SaveEditor();
+            AddStep("export beatmap", () => Game.BeatmapManager.Export(EditorBeatmap.BeatmapInfo.BeatmapSet!).WaitSafely());
+
+            AddAssert("check slider multiplier correct in file", () =>
+            {
+                string export = LocalStorage.GetFiles("exports").First();
+
+                using (var stream = LocalStorage.GetStream(export))
+                using (var zip = ZipArchive.Open(stream))
+                {
+                    using (var osuStream = zip.Entries.First().OpenEntryStream())
+                    using (var reader = new StreamReader(osuStream))
+                    {
+                        string? line;
+
+                        while ((line = reader.ReadLine()) != null)
+                        {
+                            if (line.StartsWith("SliderMultiplier", StringComparison.Ordinal))
+                            {
+                                return float.Parse(line.Split(':', StringSplitOptions.TrimEntries).Last(), provider: CultureInfo.InvariantCulture);
+                            }
+                        }
+                    }
+                }
+
+                return 0;
+            }, () => Is.EqualTo(multiplier ?? new BeatmapDifficulty().SliderMultiplier).Within(Precision.FLOAT_EPSILON));
+        }
+
         [Test]
         public void TestTaikoSliderMultiplier()
         {
@@ -27,11 +71,7 @@ namespace osu.Game.Rulesets.Taiko.Tests.Editor
 
             bool assertTaikoSliderMulitplier()
             {
-                // we can only assert value correctness on TaikoMultiplierAppliedDifficulty, because that is the final difficulty converted taiko beatmaps use.
+                return Precision.AlmostEquals(EditorBeatmap.Difficulty.SliderMultiplier, 2);
-                // therefore, ensure that we have that difficulty type by calling .CopyFrom(), which is a no-op if the type is already correct.
-                var taikoDifficulty = new TaikoBeatmapConverter.TaikoMultiplierAppliedDifficulty();
-                taikoDifficulty.CopyFrom(EditorBeatmap.Difficulty);
-                return Precision.AlmostEquals(taikoDifficulty.SliderMultiplier, 2);
             }
         }
     }

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

@@ -10,16 +10,25 @@ using System.Collections.Generic;
 using System.Linq;
 using osu.Framework.Utils;
 using System.Threading;
-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
 {
     internal class TaikoBeatmapConverter : BeatmapConverter<TaikoHitObject>
     {
+        /// <summary>
+        /// A speed multiplier applied globally to osu!taiko.
+        /// </summary>
+        /// <remarks>
+        /// osu! is generally slower than taiko, so a factor was historically added to increase speed for converts.
+        /// This must be used everywhere slider length or beat length is used in taiko.
+        ///
+        /// Of note, this has never been exposed to the end user, and is considered a hidden internal multiplier.
+        /// </remarks>
+        public const float VELOCITY_MULTIPLIER = 1.4f;
+
         /// <summary>
         /// Because swells are easier in taiko than spinners are in osu!,
         /// legacy taiko multiplies a factor when converting the number of required hits.
@@ -43,12 +52,6 @@ namespace osu.Game.Rulesets.Taiko.Beatmaps
 
         protected override Beatmap<TaikoHitObject> ConvertBeatmap(IBeatmap original, CancellationToken cancellationToken)
         {
-            if (!(original.Difficulty is TaikoMultiplierAppliedDifficulty))
-            {
-                // Rewrite the beatmap info to add the slider velocity multiplier
-                original.Difficulty = new TaikoMultiplierAppliedDifficulty(original.Difficulty);
-            }
-
             Beatmap<TaikoHitObject> converted = base.ConvertBeatmap(original, cancellationToken);
 
             if (original.BeatmapInfo.Ruleset.OnlineID == 0)
@@ -180,7 +183,7 @@ namespace osu.Game.Rulesets.Taiko.Beatmaps
             double distance = pathData.Path.ExpectedDistance.Value ?? 0;
 
             // Do not combine the following two lines!
-            distance *= LegacyBeatmapEncoder.LEGACY_TAIKO_VELOCITY_MULTIPLIER;
+            distance *= VELOCITY_MULTIPLIER;
             distance *= spans;
 
             TimingControlPoint timingPoint = beatmap.ControlPointInfo.TimingPointAt(obj.StartTime);
@@ -192,7 +195,7 @@ namespace osu.Game.Rulesets.Taiko.Beatmaps
             else
                 beatLength = timingPoint.BeatLength;
 
-            double sliderScoringPointDistance = osu_base_scoring_distance * beatmap.Difficulty.SliderMultiplier / beatmap.Difficulty.SliderTickRate;
+            double sliderScoringPointDistance = osu_base_scoring_distance * (beatmap.Difficulty.SliderMultiplier * VELOCITY_MULTIPLIER) / beatmap.Difficulty.SliderTickRate;
 
             // The velocity and duration of the taiko hit object - calculated as the velocity of a drum roll.
             double taikoVelocity = sliderScoringPointDistance * beatmap.Difficulty.SliderTickRate;
@@ -218,41 +221,5 @@ namespace osu.Game.Rulesets.Taiko.Beatmaps
         }
 
         protected override Beatmap<TaikoHitObject> CreateBeatmap() => new TaikoBeatmap();
-
-        // Important to note that this is subclassing a realm object.
-        // Realm doesn't allow this, but for now this can work since we aren't (in theory?) persisting this to the database.
-        // It is only used during beatmap conversion and processing.
-        internal class TaikoMultiplierAppliedDifficulty : BeatmapDifficulty
-        {
-            public TaikoMultiplierAppliedDifficulty(IBeatmapDifficultyInfo difficulty)
-            {
-                CopyFrom(difficulty);
-            }
-
-            [UsedImplicitly]
-            public TaikoMultiplierAppliedDifficulty()
-            {
-            }
-
-            #region Overrides of BeatmapDifficulty
-
-            public override BeatmapDifficulty Clone() => new TaikoMultiplierAppliedDifficulty(this);
-
-            public override void CopyTo(BeatmapDifficulty other)
-            {
-                base.CopyTo(other);
-                if (!(other is TaikoMultiplierAppliedDifficulty))
-                    other.SliderMultiplier /= LegacyBeatmapEncoder.LEGACY_TAIKO_VELOCITY_MULTIPLIER;
-            }
-
-            public override void CopyFrom(IBeatmapDifficultyInfo other)
-            {
-                base.CopyFrom(other);
-                if (!(other is TaikoMultiplierAppliedDifficulty))
-                    SliderMultiplier *= LegacyBeatmapEncoder.LEGACY_TAIKO_VELOCITY_MULTIPLIER;
-            }
-
-            #endregion
-        }
     }
 }

osu.Game.Rulesets.Taiko/Objects/DrumRoll.cs

@@ -1,14 +1,14 @@
 // 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 osu.Game.Rulesets.Objects.Types;
 using System.Threading;
 using osu.Game.Beatmaps;
 using osu.Game.Beatmaps.ControlPoints;
-using osu.Game.Beatmaps.Formats;
 using osu.Game.Rulesets.Judgements;
 using osu.Game.Rulesets.Objects;
+using osu.Game.Rulesets.Objects.Types;
 using osu.Game.Rulesets.Scoring;
+using osu.Game.Rulesets.Taiko.Beatmaps;
 using osuTK;
 
 namespace osu.Game.Rulesets.Taiko.Objects
@@ -51,7 +51,7 @@ namespace osu.Game.Rulesets.Taiko.Objects
             TimingControlPoint timingPoint = controlPointInfo.TimingPointAt(StartTime);
             EffectControlPoint effectPoint = controlPointInfo.EffectPointAt(StartTime);
 
-            double scoringDistance = base_distance * difficulty.SliderMultiplier * effectPoint.ScrollSpeed;
+            double scoringDistance = base_distance * (difficulty.SliderMultiplier * TaikoBeatmapConverter.VELOCITY_MULTIPLIER) * effectPoint.ScrollSpeed;
             Velocity = scoringDistance / timingPoint.BeatLength;
 
             TickRate = difficulty.SliderTickRate == 3 ? 3 : 4;
@@ -116,7 +116,7 @@ namespace osu.Game.Rulesets.Taiko.Objects
         double IHasDistance.Distance => Duration * Velocity;
 
         SliderPath IHasPath.Path
-            => new SliderPath(PathType.LINEAR, new[] { Vector2.Zero, new Vector2(1) }, ((IHasDistance)this).Distance / LegacyBeatmapEncoder.LEGACY_TAIKO_VELOCITY_MULTIPLIER);
+            => new SliderPath(PathType.LINEAR, new[] { Vector2.Zero, new Vector2(1) }, ((IHasDistance)this).Distance / TaikoBeatmapConverter.VELOCITY_MULTIPLIER);
 
         #endregion
     }

osu.Game.Rulesets.Taiko/UI/DrawableTaikoRuleset.cs

@@ -16,6 +16,7 @@ using osu.Game.Replays;
 using osu.Game.Rulesets.Mods;
 using osu.Game.Rulesets.Objects;
 using osu.Game.Rulesets.Objects.Drawables;
+using osu.Game.Rulesets.Taiko.Beatmaps;
 using osu.Game.Rulesets.Taiko.Objects;
 using osu.Game.Rulesets.Taiko.Replays;
 using osu.Game.Rulesets.Timing;
@@ -77,7 +78,11 @@ namespace osu.Game.Rulesets.Taiko.UI
             // We clamp the ratio to the maximum aspect ratio to keep scroll speed consistent on widths lower than the default.
             float ratio = Math.Max(DrawSize.X / 768f, TaikoPlayfieldAdjustmentContainer.MAXIMUM_ASPECT);
 
-            return (Playfield.HitObjectContainer.DrawWidth / ratio) * scroll_rate;
+            // Stable internally increased the slider velocity of objects by a factor of `VELOCITY_MULTIPLIER`.
+            // To simulate this, we shrink the time range by that factor here.
+            // This, when combined with the rest of the scrolling ruleset machinery (see `MultiplierControlPoint` et al.),
+            // has the effect of increasing each multiplier control point's multiplier by `VELOCITY_MULTIPLIER`, ensuring parity with stable.
+            return (Playfield.HitObjectContainer.DrawWidth / ratio) * scroll_rate / TaikoBeatmapConverter.VELOCITY_MULTIPLIER;
         }
 
         protected override void UpdateAfterChildren()

osu.Game/Beatmaps/Formats/LegacyBeatmapEncoder.cs

@@ -23,12 +23,6 @@ namespace osu.Game.Beatmaps.Formats
     {
         public const int FIRST_LAZER_VERSION = 128;
 
-        /// <summary>
-        /// osu! is generally slower than taiko, so a factor is added to increase
-        /// speed. This must be used everywhere slider length or beat length is used.
-        /// </summary>
-        public const float LEGACY_TAIKO_VELOCITY_MULTIPLIER = 1.4f;
-
         private readonly IBeatmap beatmap;
 
         private readonly ISkin? skin;
@@ -149,11 +143,7 @@ namespace osu.Game.Beatmaps.Formats
             writer.WriteLine(FormattableString.Invariant($"OverallDifficulty: {beatmap.Difficulty.OverallDifficulty}"));
             writer.WriteLine(FormattableString.Invariant($"ApproachRate: {beatmap.Difficulty.ApproachRate}"));
 
-            // Taiko adjusts the slider multiplier (see: LEGACY_TAIKO_VELOCITY_MULTIPLIER)
+            writer.WriteLine(FormattableString.Invariant($"SliderMultiplier: {beatmap.Difficulty.SliderMultiplier}"));
-            writer.WriteLine(onlineRulesetID == 1
-                ? FormattableString.Invariant($"SliderMultiplier: {beatmap.Difficulty.SliderMultiplier / LEGACY_TAIKO_VELOCITY_MULTIPLIER}")
-                : FormattableString.Invariant($"SliderMultiplier: {beatmap.Difficulty.SliderMultiplier}"));
-
             writer.WriteLine(FormattableString.Invariant($"SliderTickRate: {beatmap.Difficulty.SliderTickRate}"));
         }