Merge pull request #24138 from tybug/negative-frame-handling

Handle replay frames with negative time delta appropriately

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

@@ -33,6 +33,7 @@ using osu.Game.Scoring;
 using osu.Game.Scoring.Legacy;
 using osu.Game.Tests.Resources;
 using osu.Game.Users;
+using osuTK;
 
 namespace osu.Game.Tests.Beatmaps.Formats
 {
@@ -126,17 +127,20 @@ namespace osu.Game.Tests.Beatmaps.Formats
         [TestCase(LegacyBeatmapDecoder.LATEST_VERSION, false)]
         public void TestLegacyBeatmapReplayOffsetsDecode(int beatmapVersion, bool offsetApplied)
         {
-            const double first_frame_time = 48;
+            const double first_frame_time = 31;
-            const double second_frame_time = 65;
+            const double second_frame_time = 48;
+            const double third_frame_time = 65;
 
             var decoder = new TestLegacyScoreDecoder(beatmapVersion);
 
             using (var resourceStream = TestResources.OpenResource("Replays/mania-replay.osr"))
             {
                 var score = decoder.Parse(resourceStream);
+                int offset = offsetApplied ? LegacyBeatmapDecoder.EARLY_VERSION_TIMING_OFFSET : 0;
 
-                Assert.That(score.Replay.Frames[0].Time, Is.EqualTo(first_frame_time + (offsetApplied ? LegacyBeatmapDecoder.EARLY_VERSION_TIMING_OFFSET : 0)));
+                Assert.That(score.Replay.Frames[0].Time, Is.EqualTo(first_frame_time + offset));
-                Assert.That(score.Replay.Frames[1].Time, Is.EqualTo(second_frame_time + (offsetApplied ? LegacyBeatmapDecoder.EARLY_VERSION_TIMING_OFFSET : 0)));
+                Assert.That(score.Replay.Frames[1].Time, Is.EqualTo(second_frame_time + offset));
+                Assert.That(score.Replay.Frames[2].Time, Is.EqualTo(third_frame_time + offset));
             }
         }
 
@@ -177,6 +181,94 @@ namespace osu.Game.Tests.Beatmaps.Formats
             Assert.That(decodedAfterEncode.Replay.Frames[1].Time, Is.EqualTo(second_frame_time));
         }
 
+        [Test]
+        public void TestNegativeFrameSkipped()
+        {
+            var ruleset = new OsuRuleset().RulesetInfo;
+            var scoreInfo = TestResources.CreateTestScoreInfo(ruleset);
+            var beatmap = new TestBeatmap(ruleset);
+
+            var score = new Score
+            {
+                ScoreInfo = scoreInfo,
+                Replay = new Replay
+                {
+                    Frames = new List<ReplayFrame>
+                    {
+                        new OsuReplayFrame(0, new Vector2()),
+                        new OsuReplayFrame(1000, OsuPlayfield.BASE_SIZE),
+                        new OsuReplayFrame(500, OsuPlayfield.BASE_SIZE / 2),
+                        new OsuReplayFrame(2000, OsuPlayfield.BASE_SIZE),
+                    }
+                }
+            };
+
+            var decodedAfterEncode = encodeThenDecode(LegacyScoreEncoder.LATEST_VERSION, score, beatmap);
+
+            Assert.That(decodedAfterEncode.Replay.Frames, Has.Count.EqualTo(3));
+            Assert.That(decodedAfterEncode.Replay.Frames[0].Time, Is.EqualTo(0));
+            Assert.That(decodedAfterEncode.Replay.Frames[1].Time, Is.EqualTo(1000));
+            Assert.That(decodedAfterEncode.Replay.Frames[2].Time, Is.EqualTo(2000));
+        }
+
+        [Test]
+        public void FirstTwoFramesSwappedIfInWrongOrder()
+        {
+            var ruleset = new OsuRuleset().RulesetInfo;
+            var scoreInfo = TestResources.CreateTestScoreInfo(ruleset);
+            var beatmap = new TestBeatmap(ruleset);
+
+            var score = new Score
+            {
+                ScoreInfo = scoreInfo,
+                Replay = new Replay
+                {
+                    Frames = new List<ReplayFrame>
+                    {
+                        new OsuReplayFrame(100, new Vector2()),
+                        new OsuReplayFrame(50, OsuPlayfield.BASE_SIZE / 2),
+                        new OsuReplayFrame(1000, OsuPlayfield.BASE_SIZE),
+                    }
+                }
+            };
+
+            var decodedAfterEncode = encodeThenDecode(LegacyScoreEncoder.LATEST_VERSION, score, beatmap);
+
+            Assert.That(decodedAfterEncode.Replay.Frames, Has.Count.EqualTo(3));
+            Assert.That(decodedAfterEncode.Replay.Frames[0].Time, Is.EqualTo(0));
+            Assert.That(decodedAfterEncode.Replay.Frames[1].Time, Is.EqualTo(100));
+            Assert.That(decodedAfterEncode.Replay.Frames[2].Time, Is.EqualTo(1000));
+        }
+
+        [Test]
+        public void FirstTwoFramesPulledTowardThirdIfTheyAreAfterIt()
+        {
+            var ruleset = new OsuRuleset().RulesetInfo;
+            var scoreInfo = TestResources.CreateTestScoreInfo(ruleset);
+            var beatmap = new TestBeatmap(ruleset);
+
+            var score = new Score
+            {
+                ScoreInfo = scoreInfo,
+                Replay = new Replay
+                {
+                    Frames = new List<ReplayFrame>
+                    {
+                        new OsuReplayFrame(0, new Vector2()),
+                        new OsuReplayFrame(500, OsuPlayfield.BASE_SIZE / 2),
+                        new OsuReplayFrame(-1500, OsuPlayfield.BASE_SIZE),
+                    }
+                }
+            };
+
+            var decodedAfterEncode = encodeThenDecode(LegacyScoreEncoder.LATEST_VERSION, score, beatmap);
+
+            Assert.That(decodedAfterEncode.Replay.Frames, Has.Count.EqualTo(3));
+            Assert.That(decodedAfterEncode.Replay.Frames[0].Time, Is.EqualTo(-1500));
+            Assert.That(decodedAfterEncode.Replay.Frames[1].Time, Is.EqualTo(-1500));
+            Assert.That(decodedAfterEncode.Replay.Frames[2].Time, Is.EqualTo(-1500));
+        }
+
         [Test]
         public void TestCultureInvariance()
         {

osu.Game/Scoring/Legacy/LegacyScoreDecoder.cs

@@ -1,4 +1,4 @@
-// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
+// 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 disable
@@ -21,6 +21,7 @@ using osu.Game.Rulesets;
 using osu.Game.Rulesets.Mods;
 using osu.Game.Rulesets.Replays;
 using osu.Game.Rulesets.Scoring;
+using osuTK;
 using SharpCompress.Compressors.LZMA;
 
 namespace osu.Game.Scoring.Legacy
@@ -262,7 +263,7 @@ namespace osu.Game.Scoring.Legacy
         private void readLegacyReplay(Replay replay, StreamReader reader)
         {
             float lastTime = beatmapOffset;
-            ReplayFrame currentFrame = null;
+            var legacyFrames = new List<LegacyReplayFrame>();
 
             string[] frames = reader.ReadToEnd().Split(',');
 
@@ -285,23 +286,44 @@ namespace osu.Game.Scoring.Legacy
 
                 lastTime += diff;
 
-                if (i < 2 && mouseX == 256 && mouseY == -500)
+                legacyFrames.Add(new LegacyReplayFrame(lastTime,
-                    // at the start of the replay, stable places two replay frames, at time 0 and SkipBoundary - 1, respectively.
-                    // both frames use a position of (256, -500).
-                    // ignore these frames as they serve no real purpose (and can even mislead ruleset-specific handlers - see mania)
-                    continue;
-
-                // Todo: At some point we probably want to rewind and play back the negative-time frames
-                // but for now we'll achieve equal playback to stable by skipping negative frames
-                if (diff < 0)
-                    continue;
-
-                currentFrame = convertFrame(new LegacyReplayFrame(lastTime,
                     mouseX,
                     mouseY,
-                    (ReplayButtonState)Parsing.ParseInt(split[3])), currentFrame);
+                    (ReplayButtonState)Parsing.ParseInt(split[3])));
+            }
 
-                replay.Frames.Add(currentFrame);
+            // https://github.com/peppy/osu-stable-reference/blob/e53980dd76857ee899f66ce519ba1597e7874f28/osu!/GameModes/Play/ReplayWatcher.cs#L62-L67
+            if (legacyFrames.Count >= 2 && legacyFrames[1].Time < legacyFrames[0].Time)
+            {
+                legacyFrames[1].Time = legacyFrames[0].Time;
+                legacyFrames[0].Time = 0;
+            }
+
+            // https://github.com/peppy/osu-stable-reference/blob/e53980dd76857ee899f66ce519ba1597e7874f28/osu!/GameModes/Play/ReplayWatcher.cs#L69-L71
+            if (legacyFrames.Count >= 3 && legacyFrames[0].Time > legacyFrames[2].Time)
+                legacyFrames[0].Time = legacyFrames[1].Time = legacyFrames[2].Time;
+
+            // at the start of the replay, stable places two replay frames, at time 0 and SkipBoundary - 1, respectively.
+            // both frames use a position of (256, -500).
+            // ignore these frames as they serve no real purpose (and can even mislead ruleset-specific handlers - see mania)
+            if (legacyFrames.Count >= 2 && legacyFrames[1].Position == new Vector2(256, -500))
+                legacyFrames.RemoveAt(1);
+
+            if (legacyFrames.Count >= 1 && legacyFrames[0].Position == new Vector2(256, -500))
+                legacyFrames.RemoveAt(0);
+
+            ReplayFrame currentFrame = null;
+
+            foreach (var legacyFrame in legacyFrames)
+            {
+                // never allow backwards time traversal in relation to the current frame.
+                // this handles frames with negative delta.
+                // this doesn't match stable 100% as stable will do something similar to adding an interpolated "intermediate frame"
+                // at the point wherein time flow changes from backwards to forwards, but it'll do for now.
+                if (currentFrame != null && legacyFrame.Time < currentFrame.Time)
+                    continue;
+
+                replay.Frames.Add(currentFrame = convertFrame(legacyFrame, currentFrame));
             }
         }