Merge pull request #17429 from peppy/fix-old-legacy-replays

Fix replays set on old beatmaps having incorrect timing

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

@@ -8,6 +8,7 @@ using System.Linq;
 using NUnit.Framework;
 using osu.Framework.Utils;
 using osu.Game.Beatmaps;
+using osu.Game.Beatmaps.Formats;
 using osu.Game.Replays;
 using osu.Game.Rulesets;
 using osu.Game.Rulesets.Catch;
@@ -64,6 +65,62 @@ namespace osu.Game.Tests.Beatmaps.Formats
             }
         }
 
+        [TestCase(3, true)]
+        [TestCase(6, false)]
+        [TestCase(LegacyBeatmapDecoder.LATEST_VERSION, false)]
+        public void TestLegacyBeatmapReplayOffsetsDecode(int beatmapVersion, bool offsetApplied)
+        {
+            const double first_frame_time = 48;
+            const double second_frame_time = 65;
+
+            var decoder = new TestLegacyScoreDecoder(beatmapVersion);
+
+            using (var resourceStream = TestResources.OpenResource("Replays/mania-replay.osr"))
+            {
+                var score = decoder.Parse(resourceStream);
+
+                Assert.That(score.Replay.Frames[0].Time, Is.EqualTo(first_frame_time + (offsetApplied ? LegacyBeatmapDecoder.EARLY_VERSION_TIMING_OFFSET : 0)));
+                Assert.That(score.Replay.Frames[1].Time, Is.EqualTo(second_frame_time + (offsetApplied ? LegacyBeatmapDecoder.EARLY_VERSION_TIMING_OFFSET : 0)));
+            }
+        }
+
+        [TestCase(3)]
+        [TestCase(6)]
+        [TestCase(LegacyBeatmapDecoder.LATEST_VERSION)]
+        public void TestLegacyBeatmapReplayOffsetsEncodeDecode(int beatmapVersion)
+        {
+            const double first_frame_time = 2000;
+            const double second_frame_time = 3000;
+
+            var ruleset = new OsuRuleset().RulesetInfo;
+            var scoreInfo = TestResources.CreateTestScoreInfo(ruleset);
+            var beatmap = new TestBeatmap(ruleset)
+            {
+                BeatmapInfo =
+                {
+                    BeatmapVersion = beatmapVersion
+                }
+            };
+
+            var score = new Score
+            {
+                ScoreInfo = scoreInfo,
+                Replay = new Replay
+                {
+                    Frames = new List<ReplayFrame>
+                    {
+                        new OsuReplayFrame(first_frame_time, OsuPlayfield.BASE_SIZE / 2, OsuAction.LeftButton),
+                        new OsuReplayFrame(second_frame_time, OsuPlayfield.BASE_SIZE / 2, OsuAction.LeftButton)
+                    }
+                }
+            };
+
+            var decodedAfterEncode = encodeThenDecode(beatmapVersion, score, beatmap);
+
+            Assert.That(decodedAfterEncode.Replay.Frames[0].Time, Is.EqualTo(first_frame_time));
+            Assert.That(decodedAfterEncode.Replay.Frames[1].Time, Is.EqualTo(second_frame_time));
+        }
+
         [Test]
         public void TestCultureInvariance()
         {
@@ -86,15 +143,7 @@ namespace osu.Game.Tests.Beatmaps.Formats
             // rather than the classic ASCII U+002D HYPHEN-MINUS.
             CultureInfo.CurrentCulture = new CultureInfo("se");
 
-            var encodeStream = new MemoryStream();
+            var decodedAfterEncode = encodeThenDecode(LegacyBeatmapDecoder.LATEST_VERSION, score, beatmap);
-
-            var encoder = new LegacyScoreEncoder(score, beatmap);
-            encoder.Encode(encodeStream);
-
-            var decodeStream = new MemoryStream(encodeStream.GetBuffer());
-
-            var decoder = new TestLegacyScoreDecoder();
-            var decodedAfterEncode = decoder.Parse(decodeStream);
 
             Assert.Multiple(() =>
             {
@@ -110,6 +159,20 @@ namespace osu.Game.Tests.Beatmaps.Formats
             });
         }
 
+        private static Score encodeThenDecode(int beatmapVersion, Score score, TestBeatmap beatmap)
+        {
+            var encodeStream = new MemoryStream();
+
+            var encoder = new LegacyScoreEncoder(score, beatmap);
+            encoder.Encode(encodeStream);
+
+            var decodeStream = new MemoryStream(encodeStream.GetBuffer());
+
+            var decoder = new TestLegacyScoreDecoder(beatmapVersion);
+            var decodedAfterEncode = decoder.Parse(decodeStream);
+            return decodedAfterEncode;
+        }
+
         [TearDown]
         public void TearDown()
         {
@@ -118,6 +181,8 @@ namespace osu.Game.Tests.Beatmaps.Formats
 
         private class TestLegacyScoreDecoder : LegacyScoreDecoder
         {
+            private readonly int beatmapVersion;
+
             private static readonly Dictionary<int, Ruleset> rulesets = new Ruleset[]
             {
                 new OsuRuleset(),
@@ -126,6 +191,11 @@ namespace osu.Game.Tests.Beatmaps.Formats
                 new ManiaRuleset()
             }.ToDictionary(ruleset => ((ILegacyRuleset)ruleset).LegacyID);
 
+            public TestLegacyScoreDecoder(int beatmapVersion = LegacyBeatmapDecoder.LATEST_VERSION)
+            {
+                this.beatmapVersion = beatmapVersion;
+            }
+
             protected override Ruleset GetRuleset(int rulesetId) => rulesets[rulesetId];
 
             protected override WorkingBeatmap GetBeatmap(string md5Hash) => new TestWorkingBeatmap(new Beatmap
@@ -134,7 +204,8 @@ namespace osu.Game.Tests.Beatmaps.Formats
                 {
                     MD5Hash = md5Hash,
                     Ruleset = new OsuRuleset().RulesetInfo,
-                    Difficulty = new BeatmapDifficulty()
+                    Difficulty = new BeatmapDifficulty(),
+                    BeatmapVersion = beatmapVersion,
                 }
             });
         }

osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs

@@ -19,6 +19,11 @@ namespace osu.Game.Beatmaps.Formats
 {
     public class LegacyBeatmapDecoder : LegacyDecoder<Beatmap>
     {
+        /// <summary>
+        /// An offset which needs to be applied to old beatmaps (v4 and lower) to correct timing changes that were applied at a game client level.
+        /// </summary>
+        public const int EARLY_VERSION_TIMING_OFFSET = 24;
+
         internal static RulesetStore RulesetStore;
 
         private Beatmap beatmap;
@@ -50,8 +55,7 @@ namespace osu.Game.Beatmaps.Formats
                 RulesetStore = new AssemblyRulesetStore();
             }
 
-            // BeatmapVersion 4 and lower had an incorrect offset (stable has this set as 24ms off)
+            offset = FormatVersion < 5 ? EARLY_VERSION_TIMING_OFFSET : 0;
-            offset = FormatVersion < 5 ? 24 : 0;
         }
 
         protected override Beatmap CreateTemplateObject()

osu.Game/Scoring/Legacy/LegacyScoreDecoder.cs

@@ -23,6 +23,8 @@ namespace osu.Game.Scoring.Legacy
         private IBeatmap currentBeatmap;
         private Ruleset currentRuleset;
 
+        private float beatmapOffset;
+
         public Score Parse(Stream stream)
         {
             var score = new Score
@@ -72,6 +74,9 @@ namespace osu.Game.Scoring.Legacy
                 currentBeatmap = workingBeatmap.GetPlayableBeatmap(currentRuleset.RulesetInfo, scoreInfo.Mods);
                 scoreInfo.BeatmapInfo = currentBeatmap.BeatmapInfo;
 
+                // As this is baked into hitobject timing (see `LegacyBeatmapDecoder`) we also need to apply this to replay frame timing.
+                beatmapOffset = currentBeatmap.BeatmapInfo.BeatmapVersion < 5 ? LegacyBeatmapDecoder.EARLY_VERSION_TIMING_OFFSET : 0;
+
                 /* score.HpGraphString = */
                 sr.ReadString();
 
@@ -229,7 +234,7 @@ namespace osu.Game.Scoring.Legacy
 
         private void readLegacyReplay(Replay replay, StreamReader reader)
         {
-            float lastTime = 0;
+            float lastTime = beatmapOffset;
             ReplayFrame currentFrame = null;
 
             string[] frames = reader.ReadToEnd().Split(',');

osu.Game/Scoring/Legacy/LegacyScoreEncoder.cs

@@ -1,12 +1,15 @@
 // 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.
 
+#nullable enable
+
 using System;
 using System.IO;
 using System.Linq;
 using System.Text;
 using osu.Framework.Extensions;
 using osu.Game.Beatmaps;
+using osu.Game.Beatmaps.Formats;
 using osu.Game.Extensions;
 using osu.Game.IO.Legacy;
 using osu.Game.Replays.Legacy;
@@ -14,8 +17,6 @@ using osu.Game.Rulesets.Replays;
 using osu.Game.Rulesets.Replays.Types;
 using SharpCompress.Compressors.LZMA;
 
-#nullable enable
-
 namespace osu.Game.Scoring.Legacy
 {
     public class LegacyScoreEncoder
@@ -111,6 +112,9 @@ namespace osu.Game.Scoring.Legacy
             {
                 StringBuilder replayData = new StringBuilder();
 
+                // As this is baked into hitobject timing (see `LegacyBeatmapDecoder`) we also need to apply this to replay frame timing.
+                double offset = beatmap?.BeatmapInfo.BeatmapVersion < 5 ? -LegacyBeatmapDecoder.EARLY_VERSION_TIMING_OFFSET : 0;
+
                 if (score.Replay != null)
                 {
                     int lastTime = 0;
@@ -120,7 +124,7 @@ namespace osu.Game.Scoring.Legacy
                         var legacyFrame = getLegacyFrame(f);
 
                         // Rounding because stable could only parse integral values
-                        int time = (int)Math.Round(legacyFrame.Time);
+                        int time = (int)Math.Round(legacyFrame.Time + offset);
                         replayData.Append(FormattableString.Invariant($"{time - lastTime}|{legacyFrame.MouseX ?? 0}|{legacyFrame.MouseY ?? 0}|{(int)legacyFrame.ButtonState},"));
                         lastTime = time;
                     }