Merge pull request #12376 from ekrctb/refactor-framed-replay-input-hander

Rewrite framed replay input handler for robustness

osu.Game.Tests/NonVisual/FramedReplayInputHandlerTest.cs

@@ -20,27 +20,14 @@ namespace osu.Game.Tests.NonVisual
         {
             handler = new TestInputHandler(replay = new Replay
             {
-                Frames = new List<ReplayFrame>
+                HasReceivedAllFrames = false
-                {
-                    new TestReplayFrame(0),
-                    new TestReplayFrame(1000),
-                    new TestReplayFrame(2000),
-                    new TestReplayFrame(3000, true),
-                    new TestReplayFrame(4000, true),
-                    new TestReplayFrame(5000, true),
-                    new TestReplayFrame(7000, true),
-                    new TestReplayFrame(8000),
-                }
             });
         }
 
         [Test]
         public void TestNormalPlayback()
         {
-            Assert.IsNull(handler.CurrentFrame);
+            setReplayFrames();
-
-            confirmCurrentFrame(null);
-            confirmNextFrame(0);
 
             setTime(0, 0);
             confirmCurrentFrame(0);
@@ -107,6 +94,8 @@ namespace osu.Game.Tests.NonVisual
         [Test]
         public void TestIntroTime()
         {
+            setReplayFrames();
+
             setTime(-1000, -1000);
             confirmCurrentFrame(null);
             confirmNextFrame(0);
@@ -123,6 +112,8 @@ namespace osu.Game.Tests.NonVisual
         [Test]
         public void TestBasicRewind()
         {
+            setReplayFrames();
+
             setTime(2800, 0);
             setTime(2800, 1000);
             setTime(2800, 2000);
@@ -133,34 +124,35 @@ namespace osu.Game.Tests.NonVisual
             // pivot without crossing a frame boundary
             setTime(2700, 2700);
             confirmCurrentFrame(2);
-            confirmNextFrame(1);
+            confirmNextFrame(3);
 
-            // cross current frame boundary; should not yet update frame
+            // cross current frame boundary
-            setTime(1980, 1980);
+            setTime(1980, 2000);
             confirmCurrentFrame(2);
-            confirmNextFrame(1);
+            confirmNextFrame(3);
 
             setTime(1200, 1200);
-            confirmCurrentFrame(2);
+            confirmCurrentFrame(1);
-            confirmNextFrame(1);
+            confirmNextFrame(2);
 
             // ensure each frame plays out until start
             setTime(-500, 1000);
             confirmCurrentFrame(1);
-            confirmNextFrame(0);
+            confirmNextFrame(2);
 
             setTime(-500, 0);
             confirmCurrentFrame(0);
-            confirmNextFrame(null);
+            confirmNextFrame(1);
 
             setTime(-500, -500);
-            confirmCurrentFrame(0);
+            confirmCurrentFrame(null);
-            confirmNextFrame(null);
+            confirmNextFrame(0);
         }
 
         [Test]
         public void TestRewindInsideImportantSection()
         {
+            setReplayFrames();
             fastForwardToPoint(3000);
 
             setTime(4000, 4000);
@@ -168,12 +160,12 @@ namespace osu.Game.Tests.NonVisual
             confirmNextFrame(5);
 
             setTime(3500, null);
-            confirmCurrentFrame(4);
+            confirmCurrentFrame(3);
-            confirmNextFrame(3);
+            confirmNextFrame(4);
 
             setTime(3000, 3000);
             confirmCurrentFrame(3);
-            confirmNextFrame(2);
+            confirmNextFrame(4);
 
             setTime(3500, null);
             confirmCurrentFrame(3);
@@ -187,46 +179,127 @@ namespace osu.Game.Tests.NonVisual
             confirmCurrentFrame(4);
             confirmNextFrame(5);
 
-            setTime(4000, null);
+            setTime(4000, 4000);
             confirmCurrentFrame(4);
             confirmNextFrame(5);
 
             setTime(3500, null);
-            confirmCurrentFrame(4);
+            confirmCurrentFrame(3);
-            confirmNextFrame(3);
+            confirmNextFrame(4);
 
             setTime(3000, 3000);
             confirmCurrentFrame(3);
-            confirmNextFrame(2);
+            confirmNextFrame(4);
         }
 
         [Test]
         public void TestRewindOutOfImportantSection()
         {
+            setReplayFrames();
             fastForwardToPoint(3500);
 
             confirmCurrentFrame(3);
             confirmNextFrame(4);
 
             setTime(3200, null);
-            // next frame doesn't change even though direction reversed, because of important section.
             confirmCurrentFrame(3);
             confirmNextFrame(4);
 
-            setTime(3000, null);
+            setTime(3000, 3000);
             confirmCurrentFrame(3);
             confirmNextFrame(4);
 
             setTime(2800, 2800);
-            confirmCurrentFrame(3);
+            confirmCurrentFrame(2);
-            confirmNextFrame(2);
+            confirmNextFrame(3);
+        }
+
+        [Test]
+        public void TestReplayStreaming()
+        {
+            // no frames are arrived yet
+            setTime(0, null);
+            setTime(1000, null);
+            Assert.IsTrue(handler.WaitingForFrame, "Should be waiting for the first frame");
+
+            replay.Frames.Add(new TestReplayFrame(0));
+            replay.Frames.Add(new TestReplayFrame(1000));
+
+            // should always play from beginning
+            setTime(1000, 0);
+            confirmCurrentFrame(0);
+            Assert.IsFalse(handler.WaitingForFrame, "Should not be waiting yet");
+            setTime(1000, 1000);
+            confirmCurrentFrame(1);
+            confirmNextFrame(null);
+            Assert.IsTrue(handler.WaitingForFrame, "Should be waiting");
+
+            // cannot seek beyond the last frame
+            setTime(1500, null);
+            confirmCurrentFrame(1);
+
+            setTime(-100, 0);
+            confirmCurrentFrame(0);
+
+            // can seek to the point before the first frame, however
+            setTime(-100, -100);
+            confirmCurrentFrame(null);
+            confirmNextFrame(0);
+
+            fastForwardToPoint(1000);
+            setTime(3000, null);
+            replay.Frames.Add(new TestReplayFrame(2000));
+            confirmCurrentFrame(1);
+            setTime(1000, 1000);
+            setTime(3000, 2000);
+        }
+
+        [Test]
+        public void TestMultipleFramesSameTime()
+        {
+            replay.Frames.Add(new TestReplayFrame(0));
+            replay.Frames.Add(new TestReplayFrame(0));
+            replay.Frames.Add(new TestReplayFrame(1000));
+            replay.Frames.Add(new TestReplayFrame(1000));
+            replay.Frames.Add(new TestReplayFrame(2000));
+
+            // forward direction is prioritized when multiple frames have the same time.
+            setTime(0, 0);
+            setTime(0, 0);
+
+            setTime(2000, 1000);
+            setTime(2000, 1000);
+
+            setTime(1000, 1000);
+            setTime(1000, 1000);
+            setTime(-100, 1000);
+            setTime(-100, 0);
+            setTime(-100, 0);
+            setTime(-100, -100);
+        }
+
+        private void setReplayFrames()
+        {
+            replay.Frames = new List<ReplayFrame>
+            {
+                new TestReplayFrame(0),
+                new TestReplayFrame(1000),
+                new TestReplayFrame(2000),
+                new TestReplayFrame(3000, true),
+                new TestReplayFrame(4000, true),
+                new TestReplayFrame(5000, true),
+                new TestReplayFrame(7000, true),
+                new TestReplayFrame(8000),
+            };
+            replay.HasReceivedAllFrames = true;
         }
 
         private void fastForwardToPoint(double destination)
         {
             for (int i = 0; i < 1000; i++)
             {
-                if (handler.SetFrameFromTime(destination) == null)
+                var time = handler.SetFrameFromTime(destination);
+                if (time == null || time == destination)
                     return;
             }
 
@@ -235,33 +308,17 @@ namespace osu.Game.Tests.NonVisual
 
         private void setTime(double set, double? expect)
         {
-            Assert.AreEqual(expect, handler.SetFrameFromTime(set));
+            Assert.AreEqual(expect, handler.SetFrameFromTime(set), "Unexpected return value");
         }
 
         private void confirmCurrentFrame(int? frame)
         {
-            if (frame.HasValue)
+            Assert.AreEqual(frame is int x ? replay.Frames[x].Time : (double?)null, handler.CurrentFrame?.Time, "Unexpected current frame");
-            {
-                Assert.IsNotNull(handler.CurrentFrame);
-                Assert.AreEqual(replay.Frames[frame.Value].Time, handler.CurrentFrame.Time);
-            }
-            else
-            {
-                Assert.IsNull(handler.CurrentFrame);
-            }
         }
 
         private void confirmNextFrame(int? frame)
         {
-            if (frame.HasValue)
+            Assert.AreEqual(frame is int x ? replay.Frames[x].Time : (double?)null, handler.NextFrame?.Time, "Unexpected next frame");
-            {
-                Assert.IsNotNull(handler.NextFrame);
-                Assert.AreEqual(replay.Frames[frame.Value].Time, handler.NextFrame.Time);
-            }
-            else
-            {
-                Assert.IsNull(handler.NextFrame);
-            }
         }
 
         private class TestReplayFrame : ReplayFrame

osu.Game.Tests/NonVisual/StreamingFramedReplayInputHandlerTest.cs

@@ -1,296 +0,0 @@
-// 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.Collections.Generic;
-using NUnit.Framework;
-using osu.Game.Replays;
-using osu.Game.Rulesets.Replays;
-
-namespace osu.Game.Tests.NonVisual
-{
-    [TestFixture]
-    public class StreamingFramedReplayInputHandlerTest
-    {
-        private Replay replay;
-        private TestInputHandler handler;
-
-        [SetUp]
-        public void SetUp()
-        {
-            handler = new TestInputHandler(replay = new Replay
-            {
-                HasReceivedAllFrames = false,
-                Frames = new List<ReplayFrame>
-                {
-                    new TestReplayFrame(0),
-                    new TestReplayFrame(1000),
-                    new TestReplayFrame(2000),
-                    new TestReplayFrame(3000, true),
-                    new TestReplayFrame(4000, true),
-                    new TestReplayFrame(5000, true),
-                    new TestReplayFrame(7000, true),
-                    new TestReplayFrame(8000),
-                }
-            });
-        }
-
-        [Test]
-        public void TestNormalPlayback()
-        {
-            Assert.IsNull(handler.CurrentFrame);
-
-            confirmCurrentFrame(null);
-            confirmNextFrame(0);
-
-            setTime(0, 0);
-            confirmCurrentFrame(0);
-            confirmNextFrame(1);
-
-            // if we hit the first frame perfectly, time should progress to it.
-            setTime(1000, 1000);
-            confirmCurrentFrame(1);
-            confirmNextFrame(2);
-
-            // in between non-important frames should progress based on input.
-            setTime(1200, 1200);
-            confirmCurrentFrame(1);
-
-            setTime(1400, 1400);
-            confirmCurrentFrame(1);
-
-            // progressing beyond the next frame should force time to that frame once.
-            setTime(2200, 2000);
-            confirmCurrentFrame(2);
-
-            // second attempt should progress to input time
-            setTime(2200, 2200);
-            confirmCurrentFrame(2);
-
-            // entering important section
-            setTime(3000, 3000);
-            confirmCurrentFrame(3);
-
-            // cannot progress within
-            setTime(3500, null);
-            confirmCurrentFrame(3);
-
-            setTime(4000, 4000);
-            confirmCurrentFrame(4);
-
-            // still cannot progress
-            setTime(4500, null);
-            confirmCurrentFrame(4);
-
-            setTime(5200, 5000);
-            confirmCurrentFrame(5);
-
-            // important section AllowedImportantTimeSpan allowance
-            setTime(5200, 5200);
-            confirmCurrentFrame(5);
-
-            setTime(7200, 7000);
-            confirmCurrentFrame(6);
-
-            setTime(7200, null);
-            confirmCurrentFrame(6);
-
-            // exited important section
-            setTime(8200, 8000);
-            confirmCurrentFrame(7);
-            confirmNextFrame(null);
-
-            setTime(8200, null);
-            confirmCurrentFrame(7);
-            confirmNextFrame(null);
-
-            setTime(8400, null);
-            confirmCurrentFrame(7);
-            confirmNextFrame(null);
-        }
-
-        [Test]
-        public void TestIntroTime()
-        {
-            setTime(-1000, -1000);
-            confirmCurrentFrame(null);
-            confirmNextFrame(0);
-
-            setTime(-500, -500);
-            confirmCurrentFrame(null);
-            confirmNextFrame(0);
-
-            setTime(0, 0);
-            confirmCurrentFrame(0);
-            confirmNextFrame(1);
-        }
-
-        [Test]
-        public void TestBasicRewind()
-        {
-            setTime(2800, 0);
-            setTime(2800, 1000);
-            setTime(2800, 2000);
-            setTime(2800, 2800);
-            confirmCurrentFrame(2);
-            confirmNextFrame(3);
-
-            // pivot without crossing a frame boundary
-            setTime(2700, 2700);
-            confirmCurrentFrame(2);
-            confirmNextFrame(1);
-
-            // cross current frame boundary; should not yet update frame
-            setTime(1980, 1980);
-            confirmCurrentFrame(2);
-            confirmNextFrame(1);
-
-            setTime(1200, 1200);
-            confirmCurrentFrame(2);
-            confirmNextFrame(1);
-
-            // ensure each frame plays out until start
-            setTime(-500, 1000);
-            confirmCurrentFrame(1);
-            confirmNextFrame(0);
-
-            setTime(-500, 0);
-            confirmCurrentFrame(0);
-            confirmNextFrame(null);
-
-            setTime(-500, -500);
-            confirmCurrentFrame(0);
-            confirmNextFrame(null);
-        }
-
-        [Test]
-        public void TestRewindInsideImportantSection()
-        {
-            fastForwardToPoint(3000);
-
-            setTime(4000, 4000);
-            confirmCurrentFrame(4);
-            confirmNextFrame(5);
-
-            setTime(3500, null);
-            confirmCurrentFrame(4);
-            confirmNextFrame(3);
-
-            setTime(3000, 3000);
-            confirmCurrentFrame(3);
-            confirmNextFrame(2);
-
-            setTime(3500, null);
-            confirmCurrentFrame(3);
-            confirmNextFrame(4);
-
-            setTime(4000, 4000);
-            confirmCurrentFrame(4);
-            confirmNextFrame(5);
-
-            setTime(4500, null);
-            confirmCurrentFrame(4);
-            confirmNextFrame(5);
-
-            setTime(4000, null);
-            confirmCurrentFrame(4);
-            confirmNextFrame(5);
-
-            setTime(3500, null);
-            confirmCurrentFrame(4);
-            confirmNextFrame(3);
-
-            setTime(3000, 3000);
-            confirmCurrentFrame(3);
-            confirmNextFrame(2);
-        }
-
-        [Test]
-        public void TestRewindOutOfImportantSection()
-        {
-            fastForwardToPoint(3500);
-
-            confirmCurrentFrame(3);
-            confirmNextFrame(4);
-
-            setTime(3200, null);
-            // next frame doesn't change even though direction reversed, because of important section.
-            confirmCurrentFrame(3);
-            confirmNextFrame(4);
-
-            setTime(3000, null);
-            confirmCurrentFrame(3);
-            confirmNextFrame(4);
-
-            setTime(2800, 2800);
-            confirmCurrentFrame(3);
-            confirmNextFrame(2);
-        }
-
-        private void fastForwardToPoint(double destination)
-        {
-            for (int i = 0; i < 1000; i++)
-            {
-                if (handler.SetFrameFromTime(destination) == null)
-                    return;
-            }
-
-            throw new TimeoutException("Seek was never fulfilled");
-        }
-
-        private void setTime(double set, double? expect)
-        {
-            Assert.AreEqual(expect, handler.SetFrameFromTime(set));
-        }
-
-        private void confirmCurrentFrame(int? frame)
-        {
-            if (frame.HasValue)
-            {
-                Assert.IsNotNull(handler.CurrentFrame);
-                Assert.AreEqual(replay.Frames[frame.Value].Time, handler.CurrentFrame.Time);
-            }
-            else
-            {
-                Assert.IsNull(handler.CurrentFrame);
-            }
-        }
-
-        private void confirmNextFrame(int? frame)
-        {
-            if (frame.HasValue)
-            {
-                Assert.IsNotNull(handler.NextFrame);
-                Assert.AreEqual(replay.Frames[frame.Value].Time, handler.NextFrame.Time);
-            }
-            else
-            {
-                Assert.IsNull(handler.NextFrame);
-            }
-        }
-
-        private class TestReplayFrame : ReplayFrame
-        {
-            public readonly bool IsImportant;
-
-            public TestReplayFrame(double time, bool isImportant = false)
-                : base(time)
-            {
-                IsImportant = isImportant;
-            }
-        }
-
-        private class TestInputHandler : FramedReplayInputHandler<TestReplayFrame>
-        {
-            public TestInputHandler(Replay replay)
-                : base(replay)
-            {
-                FrameAccuratePlayback = true;
-            }
-
-            protected override double AllowedImportantTimeSpan => 1000;
-
-            protected override bool IsImportant(TestReplayFrame frame) => frame.IsImportant;
-        }
-    }
-}

osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs

@@ -83,7 +83,7 @@ namespace osu.Game.Tests.Visual.Gameplay
             waitForPlayer();
             AddAssert("ensure frames arrived", () => replayHandler.HasFrames);
 
-            AddUntilStep("wait for frame starvation", () => replayHandler.NextFrame == null);
+            AddUntilStep("wait for frame starvation", () => replayHandler.WaitingForFrame);
             checkPaused(true);
 
             double? pausedTime = null;
@@ -92,7 +92,7 @@ namespace osu.Game.Tests.Visual.Gameplay
 
             sendFrames();
 
-            AddUntilStep("wait for frame starvation", () => replayHandler.NextFrame == null);
+            AddUntilStep("wait for frame starvation", () => replayHandler.WaitingForFrame);
             checkPaused(true);
 
             AddAssert("time advanced", () => currentFrameStableTime > pausedTime);

osu.Game.Tests/Visual/Gameplay/TestSceneSpectatorPlayback.cs

@@ -204,27 +204,27 @@ namespace osu.Game.Tests.Visual.Gameplay
                 return;
             }
 
-            if (replayHandler.NextFrame != null)
+            if (!replayHandler.HasFrames)
-            {
+                return;
-                var lastFrame = replay.Frames.LastOrDefault();
 
-                // this isn't perfect as we basically can't be aware of the rate-of-send here (the streamer is not sending data when not being moved).
+            var lastFrame = replay.Frames.LastOrDefault();
-                // in gameplay playback, the case where NextFrame is null would pause gameplay and handle this correctly; it's strictly a test limitation / best effort implementation.
-                if (lastFrame != null)
-                    latency = Math.Max(latency, Time.Current - lastFrame.Time);
 
-                latencyDisplay.Text = $"latency: {latency:N1}";
+            // this isn't perfect as we basically can't be aware of the rate-of-send here (the streamer is not sending data when not being moved).
+            // in gameplay playback, the case where NextFrame is null would pause gameplay and handle this correctly; it's strictly a test limitation / best effort implementation.
+            if (lastFrame != null)
+                latency = Math.Max(latency, Time.Current - lastFrame.Time);
 
-                double proposedTime = Time.Current - latency + Time.Elapsed;
+            latencyDisplay.Text = $"latency: {latency:N1}";
 
-                // this will either advance by one or zero frames.
+            double proposedTime = Time.Current - latency + Time.Elapsed;
-                double? time = replayHandler.SetFrameFromTime(proposedTime);
 
-                if (time == null)
+            // this will either advance by one or zero frames.
-                    return;
+            double? time = replayHandler.SetFrameFromTime(proposedTime);
 
-                manualClock.CurrentTime = time.Value;
+            if (time == null)
-            }
+                return;
+
+            manualClock.CurrentTime = time.Value;
         }
 
         [TearDownSteps]

osu.Game/Input/Handlers/ReplayInputHandler.cs

@@ -32,8 +32,6 @@ namespace osu.Game.Input.Handlers
 
         public override bool Initialize(GameHost host) => true;
 
-        public override bool IsActive => true;
-
         public class ReplayState<T> : IInput
             where T : struct
         {

osu.Game/Rulesets/Replays/FramedReplayInputHandler.cs

@@ -3,7 +3,6 @@
 
 using System;
 using System.Collections.Generic;
-using System.Diagnostics;
 using JetBrains.Annotations;
 using osu.Game.Input.Handlers;
 using osu.Game.Replays;
@@ -17,80 +16,92 @@ namespace osu.Game.Rulesets.Replays
     public abstract class FramedReplayInputHandler<TFrame> : ReplayInputHandler
         where TFrame : ReplayFrame
     {
-        private readonly Replay replay;
+        /// <summary>
+        /// Whether we have at least one replay frame.
+        /// </summary>
+        public bool HasFrames => Frames.Count != 0;
 
-        protected List<ReplayFrame> Frames => replay.Frames;
+        /// <summary>
+        /// Whether we are waiting for new frames to be received.
+        /// </summary>
+        public bool WaitingForFrame => !replay.HasReceivedAllFrames && currentFrameIndex == Frames.Count - 1;
 
+        /// <summary>
+        /// The current frame of the replay.
+        /// The current time is always between the start and the end time of the current frame.
+        /// </summary>
+        /// <remarks>Returns null if the current time is strictly before the first frame.</remarks>
+        /// <exception cref="InvalidOperationException">The replay is empty.</exception>
         public TFrame CurrentFrame
         {
             get
             {
-                if (!HasFrames || !currentFrameIndex.HasValue)
+                if (!HasFrames)
-                    return null;
+                    throw new InvalidOperationException($"Attempted to get {nameof(CurrentFrame)} of an empty replay");
 
-                return (TFrame)Frames[currentFrameIndex.Value];
+                return currentFrameIndex == -1 ? null : (TFrame)Frames[currentFrameIndex];
             }
         }
 
+        /// <summary>
+        /// The next frame of the replay.
+        /// The start time is always greater or equal to the start time of <see cref="CurrentFrame"/> regardless of the seeking direction.
+        /// </summary>
+        /// <remarks>Returns null if the current frame is the last frame.</remarks>
+        /// <exception cref="InvalidOperationException">The replay is empty.</exception>
         public TFrame NextFrame
         {
             get
             {
                 if (!HasFrames)
-                    return null;
+                    throw new InvalidOperationException($"Attempted to get {nameof(NextFrame)} of an empty replay");
 
-                if (!currentFrameIndex.HasValue)
+                return currentFrameIndex == Frames.Count - 1 ? null : (TFrame)Frames[currentFrameIndex + 1];
-                    return currentDirection > 0 ? (TFrame)Frames[0] : null;
-
-                int nextFrame = clampedNextFrameIndex;
-
-                if (nextFrame == currentFrameIndex.Value)
-                    return null;
-
-                return (TFrame)Frames[clampedNextFrameIndex];
             }
         }
 
-        private int? currentFrameIndex;
-
-        private int clampedNextFrameIndex =>
-            currentFrameIndex.HasValue ? Math.Clamp(currentFrameIndex.Value + currentDirection, 0, Frames.Count - 1) : 0;
-
-        protected FramedReplayInputHandler(Replay replay)
-        {
-            this.replay = replay;
-        }
-
-        private const double sixty_frame_time = 1000.0 / 60;
-
-        protected virtual double AllowedImportantTimeSpan => sixty_frame_time * 1.2;
-
-        protected double? CurrentTime { get; private set; }
-
-        private int currentDirection = 1;
-
         /// <summary>
         /// When set, we will ensure frames executed by nested drawables are frame-accurate to replay data.
         /// Disabling this can make replay playback smoother (useful for autoplay, currently).
         /// </summary>
         public bool FrameAccuratePlayback;
 
-        public bool HasFrames => Frames.Count > 0;
+        // This input handler should be enabled only if there is at least one replay frame.
+        public override bool IsActive => HasFrames;
+
+        // Can make it non-null but that is a breaking change.
+        protected double? CurrentTime { get; private set; }
+
+        protected virtual double AllowedImportantTimeSpan => sixty_frame_time * 1.2;
+
+        protected List<ReplayFrame> Frames => replay.Frames;
+
+        private readonly Replay replay;
+
+        private int currentFrameIndex;
+
+        private const double sixty_frame_time = 1000.0 / 60;
+
+        protected FramedReplayInputHandler(Replay replay)
+        {
+            // TODO: This replay frame ordering should be enforced on the Replay type.
+            // Currently, the ordering can be broken if the frames are added after this construction.
+            replay.Frames.Sort((x, y) => x.Time.CompareTo(y.Time));
+
+            this.replay = replay;
+            currentFrameIndex = -1;
+            CurrentTime = double.NegativeInfinity;
+        }
 
         private bool inImportantSection
         {
             get
             {
-                if (!HasFrames || !FrameAccuratePlayback)
+                if (!HasFrames || !FrameAccuratePlayback || CurrentFrame == null)
                     return false;
 
-                var frame = currentDirection > 0 ? CurrentFrame : NextFrame;
+                return IsImportant(CurrentFrame) && // a button is in a pressed state
-
+                       Math.Abs(CurrentTime - NextFrame.Time ?? 0) <= AllowedImportantTimeSpan; // the next frame is within an allowable time span
-                if (frame == null)
-                    return false;
-
-                return IsImportant(frame) && // a button is in a pressed state
-                       Math.Abs(CurrentTime - NextFrame?.Time ?? 0) <= AllowedImportantTimeSpan; // the next frame is within an allowable time span
             }
         }
 
@@ -105,71 +116,52 @@ namespace osu.Game.Rulesets.Replays
         /// <returns>The usable time value. If null, we should not advance time as we do not have enough data.</returns>
         public override double? SetFrameFromTime(double time)
         {
-            updateDirection(time);
-
-            Debug.Assert(currentDirection != 0);
-
             if (!HasFrames)
             {
-                // in the case all frames are received, allow time to progress regardless.
+                // In the case all frames are received, allow time to progress regardless.
                 if (replay.HasReceivedAllFrames)
                     return CurrentTime = time;
 
                 return null;
             }
 
-            TFrame next = NextFrame;
+            double frameStart = getFrameTime(currentFrameIndex);
+            double frameEnd = getFrameTime(currentFrameIndex + 1);
 
-            // if we have a next frame, check if it is before or at the current time in playback, and advance time to it if so.
+            // If the proposed time is after the current frame end time, we progress forwards to precisely the new frame's time (regardless of incoming time).
-            if (next != null)
+            if (frameEnd <= time)
             {
-                int compare = time.CompareTo(next.Time);
+                time = frameEnd;
-
+                currentFrameIndex++;
-                if (compare == 0 || compare == currentDirection)
-                {
-                    currentFrameIndex = clampedNextFrameIndex;
-                    return CurrentTime = CurrentFrame.Time;
-                }
             }
+            // If the proposed time is before the current frame start time, and we are at the frame boundary, we progress backwards.
+            else if (time < frameStart && CurrentTime == frameStart)
+                currentFrameIndex--;
 
-            // at this point, the frame index can't be advanced.
+            frameStart = getFrameTime(currentFrameIndex);
-            // even so, we may be able to propose the clock progresses forward due to being at an extent of the replay,
+            frameEnd = getFrameTime(currentFrameIndex + 1);
-            // or moving towards the next valid frame (ie. interpolating in a non-important section).
 
-            // the exception is if currently in an important section, which is respected above all.
+            // Pause until more frames are arrived.
-            if (inImportantSection)
+            if (WaitingForFrame && frameStart < time)
             {
-                Debug.Assert(next != null || !replay.HasReceivedAllFrames);
+                CurrentTime = frameStart;
                 return null;
             }
 
-            // if a next frame does exist, allow interpolation.
+            CurrentTime = Math.Clamp(time, frameStart, frameEnd);
-            if (next != null)
-                return CurrentTime = time;
 
-            // if all frames have been received, allow playing beyond extents.
+            // In an important section, a mid-frame time cannot be used and a null is returned instead.
-            if (replay.HasReceivedAllFrames)
+            return inImportantSection && frameStart < time && time < frameEnd ? null : CurrentTime;
-                return CurrentTime = time;
-
-            // if not all frames are received but we are before the first frame, allow playing.
-            if (time < Frames[0].Time)
-                return CurrentTime = time;
-
-            // in the case we have no next frames and haven't received enough frame data, block.
-            return null;
         }
 
-        private void updateDirection(double time)
+        private double getFrameTime(int index)
         {
-            if (!CurrentTime.HasValue)
+            if (index < 0)
-            {
+                return double.NegativeInfinity;
-                currentDirection = 1;
+            if (index >= Frames.Count)
-            }
+                return double.PositiveInfinity;
-            else
+
-            {
+            return Frames[index].Time;
-                currentDirection = time.CompareTo(CurrentTime);
-                if (currentDirection == 0) currentDirection = 1;
-            }
         }
     }
 }

osu.Game/Rulesets/Scoring/JudgementProcessor.cs

@@ -28,6 +28,8 @@ namespace osu.Game.Rulesets.Scoring
         /// </summary>
         public int JudgedHits { get; private set; }
 
+        private JudgementResult lastAppliedResult;
+
         private readonly BindableBool hasCompleted = new BindableBool();
 
         /// <summary>
@@ -53,12 +55,11 @@ namespace osu.Game.Rulesets.Scoring
         public void ApplyResult(JudgementResult result)
         {
             JudgedHits++;
+            lastAppliedResult = result;
 
             ApplyResultInternal(result);
 
             NewJudgement?.Invoke(result);
-
-            updateHasCompleted();
         }
 
         /// <summary>
@@ -69,8 +70,6 @@ namespace osu.Game.Rulesets.Scoring
         {
             JudgedHits--;
 
-            updateHasCompleted();
-
             RevertResultInternal(result);
         }
 
@@ -134,6 +133,10 @@ namespace osu.Game.Rulesets.Scoring
             }
         }
 
-        private void updateHasCompleted() => hasCompleted.Value = JudgedHits == MaxHits;
+        protected override void Update()
+        {
+            base.Update();
+            hasCompleted.Value = JudgedHits == MaxHits && (JudgedHits == 0 || lastAppliedResult.TimeAbsolute < Clock.CurrentTime);
+        }
     }
 }

osu.Game/Rulesets/UI/RulesetInputManager.cs

@@ -2,6 +2,7 @@
 // See the LICENCE file in the repository root for full licence text.
 
 using System;
+using System.Collections.Generic;
 using System.Linq;
 using osu.Framework.Allocation;
 using osu.Framework.Bindables;
@@ -10,6 +11,7 @@ using osu.Framework.Graphics.Containers;
 using osu.Framework.Input;
 using osu.Framework.Input.Bindings;
 using osu.Framework.Input.Events;
+using osu.Framework.Input.StateChanges;
 using osu.Framework.Input.StateChanges.Events;
 using osu.Framework.Input.States;
 using osu.Game.Configuration;
@@ -100,6 +102,17 @@ namespace osu.Game.Rulesets.UI
 
         #endregion
 
+        // to avoid allocation
+        private readonly List<IInput> emptyInputList = new List<IInput>();
+
+        protected override List<IInput> GetPendingInputs()
+        {
+            if (replayInputHandler?.IsActive == false)
+                return emptyInputList;
+
+            return base.GetPendingInputs();
+        }
+
         #region Setting application (disables etc.)
 
         private Bindable<bool> mouseDisabled;