Merge pull request #14575 from peppy/add-legacy-convert-encode-stability-tests

Fix multiple conversions of a beatmap to taiko ruleset applying speed multiplier multiple times

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

@@ -18,12 +18,6 @@ namespace osu.Game.Rulesets.Taiko.Beatmaps
 {
     internal class TaikoBeatmapConverter : BeatmapConverter<TaikoHitObject>
     {
-        /// <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_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.
@@ -52,10 +46,12 @@ namespace osu.Game.Rulesets.Taiko.Beatmaps
 
         protected override Beatmap<TaikoHitObject> ConvertBeatmap(IBeatmap original, CancellationToken cancellationToken)
         {
-            // Rewrite the beatmap info to add the slider velocity multiplier
+            if (!(original.BeatmapInfo.BaseDifficulty is TaikoMutliplierAppliedDifficulty))
-            original.BeatmapInfo = original.BeatmapInfo.Clone();
+            {
-            original.BeatmapInfo.BaseDifficulty = original.BeatmapInfo.BaseDifficulty.Clone();
+                // Rewrite the beatmap info to add the slider velocity multiplier
-            original.BeatmapInfo.BaseDifficulty.SliderMultiplier *= LEGACY_VELOCITY_MULTIPLIER;
+                original.BeatmapInfo = original.BeatmapInfo.Clone();
+                original.BeatmapInfo.BaseDifficulty = new TaikoMutliplierAppliedDifficulty(original.BeatmapInfo.BaseDifficulty);
+            }
 
             Beatmap<TaikoHitObject> converted = base.ConvertBeatmap(original, cancellationToken);
 
@@ -155,7 +151,7 @@ namespace osu.Game.Rulesets.Taiko.Beatmaps
 
             // The true distance, accounting for any repeats. This ends up being the drum roll distance later
             int spans = (obj as IHasRepeats)?.SpanCount() ?? 1;
-            double distance = distanceData.Distance * spans * LEGACY_VELOCITY_MULTIPLIER;
+            double distance = distanceData.Distance * spans * LegacyBeatmapEncoder.LEGACY_TAIKO_VELOCITY_MULTIPLIER;
 
             TimingControlPoint timingPoint = beatmap.ControlPointInfo.TimingPointAt(obj.StartTime);
             DifficultyControlPoint difficultyPoint = beatmap.ControlPointInfo.DifficultyPointAt(obj.StartTime);
@@ -194,5 +190,14 @@ namespace osu.Game.Rulesets.Taiko.Beatmaps
         }
 
         protected override Beatmap<TaikoHitObject> CreateBeatmap() => new TaikoBeatmap();
+
+        private class TaikoMutliplierAppliedDifficulty : BeatmapDifficulty
+        {
+            public TaikoMutliplierAppliedDifficulty(BeatmapDifficulty difficulty)
+            {
+                difficulty.CopyTo(this);
+                SliderMultiplier *= LegacyBeatmapEncoder.LEGACY_TAIKO_VELOCITY_MULTIPLIER;
+            }
+        }
     }
 }

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

@@ -6,10 +6,10 @@ using System;
 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.Scoring;
-using osu.Game.Rulesets.Taiko.Beatmaps;
 using osu.Game.Rulesets.Taiko.Judgements;
 using osuTK;
 
@@ -120,7 +120,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 / TaikoBeatmapConverter.LEGACY_VELOCITY_MULTIPLIER);
+            => new SliderPath(PathType.Linear, new[] { Vector2.Zero, new Vector2(1) }, ((IHasDistance)this).Distance / LegacyBeatmapEncoder.LEGACY_TAIKO_VELOCITY_MULTIPLIER);
 
         #endregion
     }

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

@@ -49,6 +49,22 @@ namespace osu.Game.Tests.Beatmaps.Formats
             Assert.IsTrue(areComboColoursEqual(decodedAfterEncode.beatmapSkin.Configuration, decoded.beatmapSkin.Configuration));
         }
 
+        [TestCaseSource(nameof(allBeatmaps))]
+        public void TestEncodeDecodeStabilityDoubleConvert(string name)
+        {
+            var decoded = decodeFromLegacy(beatmaps_resource_store.GetStream(name), name);
+            var decodedAfterEncode = decodeFromLegacy(encodeToLegacy(decoded), name);
+
+            // run an extra convert. this is expected to be stable.
+            decodedAfterEncode.beatmap = convert(decodedAfterEncode.beatmap);
+
+            sort(decoded.beatmap);
+            sort(decodedAfterEncode.beatmap);
+
+            Assert.That(decodedAfterEncode.beatmap.Serialize(), Is.EqualTo(decoded.beatmap.Serialize()));
+            Assert.IsTrue(areComboColoursEqual(decodedAfterEncode.beatmapSkin.Configuration, decoded.beatmapSkin.Configuration));
+        }
+
         [Test]
         public void TestEncodeMultiSegmentSliderWithFloatingPointError()
         {

osu.Game/Beatmaps/BeatmapDifficulty.cs

@@ -32,7 +32,23 @@ namespace osu.Game.Beatmaps
         /// <summary>
         /// Returns a shallow-clone of this <see cref="BeatmapDifficulty"/>.
         /// </summary>
-        public BeatmapDifficulty Clone() => (BeatmapDifficulty)MemberwiseClone();
+        public BeatmapDifficulty Clone()
+        {
+            var diff = new BeatmapDifficulty();
+            CopyTo(diff);
+            return diff;
+        }
+
+        public void CopyTo(BeatmapDifficulty difficulty)
+        {
+            difficulty.ApproachRate = ApproachRate;
+            difficulty.DrainRate = DrainRate;
+            difficulty.CircleSize = CircleSize;
+            difficulty.OverallDifficulty = OverallDifficulty;
+
+            difficulty.SliderMultiplier = SliderMultiplier;
+            difficulty.SliderTickRate = SliderTickRate;
+        }
 
         /// <summary>
         /// Maps a difficulty value [0, 10] to a two-piece linear range of values.

osu.Game/Beatmaps/Formats/LegacyBeatmapEncoder.cs

@@ -24,6 +24,12 @@ namespace osu.Game.Beatmaps.Formats
     {
         public const int LATEST_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;
 
         [CanBeNull]
@@ -140,9 +146,9 @@ namespace osu.Game.Beatmaps.Formats
             writer.WriteLine(FormattableString.Invariant($"OverallDifficulty: {beatmap.BeatmapInfo.BaseDifficulty.OverallDifficulty}"));
             writer.WriteLine(FormattableString.Invariant($"ApproachRate: {beatmap.BeatmapInfo.BaseDifficulty.ApproachRate}"));
 
-            // Taiko adjusts the slider multiplier (see: TaikoBeatmapConverter.LEGACY_VELOCITY_MULTIPLIER)
+            // Taiko adjusts the slider multiplier (see: LEGACY_TAIKO_VELOCITY_MULTIPLIER)
             writer.WriteLine(beatmap.BeatmapInfo.RulesetID == 1
-                ? FormattableString.Invariant($"SliderMultiplier: {beatmap.BeatmapInfo.BaseDifficulty.SliderMultiplier / 1.4f}")
+                ? FormattableString.Invariant($"SliderMultiplier: {beatmap.BeatmapInfo.BaseDifficulty.SliderMultiplier / LEGACY_TAIKO_VELOCITY_MULTIPLIER}")
                 : FormattableString.Invariant($"SliderMultiplier: {beatmap.BeatmapInfo.BaseDifficulty.SliderMultiplier}"));
 
             writer.WriteLine(FormattableString.Invariant($"SliderTickRate: {beatmap.BeatmapInfo.BaseDifficulty.SliderTickRate}"));