Merge pull request #20014 from peppy/fix-resume-skip-forward

Fix gameplay skipping forward during resume operation

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

@@ -66,7 +66,7 @@ namespace osu.Game.Tests.Visual.Gameplay
                 Player.OnUpdate += _ =>
                 {
                     double currentTime = Player.GameplayClockContainer.CurrentTime;
-                    alwaysGoingForward &= currentTime >= lastTime;
+                    alwaysGoingForward &= currentTime >= lastTime - 500;
                     lastTime = currentTime;
                 };
             });
@@ -77,7 +77,7 @@ namespace osu.Game.Tests.Visual.Gameplay
 
             resumeAndConfirm();
 
-            AddAssert("time didn't go backwards", () => alwaysGoingForward);
+            AddAssert("time didn't go too far backwards", () => alwaysGoingForward);
 
             AddStep("reset offset", () => LocalConfig.SetValue(OsuSetting.AudioOffset, 0.0));
         }
@@ -90,6 +90,9 @@ namespace osu.Game.Tests.Visual.Gameplay
             AddAssert("player not playing", () => !Player.LocalUserPlaying.Value);
 
             resumeAndConfirm();
+
+            AddAssert("Resumed without seeking forward", () => Player.LastResumeTime, () => Is.LessThanOrEqualTo(Player.LastPauseTime));
+
             AddUntilStep("player playing", () => Player.LocalUserPlaying.Value);
         }
 
@@ -378,7 +381,16 @@ namespace osu.Game.Tests.Visual.Gameplay
             AddAssert("pause overlay " + (isShown ? "shown" : "hidden"), () => Player.PauseOverlayVisible == isShown);
 
         private void confirmClockRunning(bool isRunning) =>
-            AddUntilStep("clock " + (isRunning ? "running" : "stopped"), () => Player.GameplayClockContainer.IsRunning == isRunning);
+            AddUntilStep("clock " + (isRunning ? "running" : "stopped"), () =>
+            {
+                bool completed = Player.GameplayClockContainer.IsRunning == isRunning;
+
+                if (completed)
+                {
+                }
+
+                return completed;
+            });
 
         protected override bool AllowFail => true;
 
@@ -386,6 +398,9 @@ namespace osu.Game.Tests.Visual.Gameplay
 
         protected class PausePlayer : TestPlayer
         {
+            public double LastPauseTime { get; private set; }
+            public double LastResumeTime { get; private set; }
+
             public bool FailOverlayVisible => FailOverlay.State.Value == Visibility.Visible;
 
             public bool PauseOverlayVisible => PauseOverlay.State.Value == Visibility.Visible;
@@ -399,6 +414,23 @@ namespace osu.Game.Tests.Visual.Gameplay
                 base.OnEntering(e);
                 GameplayClockContainer.Stop();
             }
+
+            private bool? isRunning;
+
+            protected override void UpdateAfterChildren()
+            {
+                base.UpdateAfterChildren();
+
+                if (GameplayClockContainer.IsRunning != isRunning)
+                {
+                    isRunning = GameplayClockContainer.IsRunning;
+
+                    if (isRunning.Value)
+                        LastResumeTime = GameplayClockContainer.CurrentTime;
+                    else
+                        LastPauseTime = GameplayClockContainer.CurrentTime;
+                }
+            }
         }
     }
 }

osu.Game/Screens/Play/GameplayClockContainer.cs

@@ -88,9 +88,7 @@ namespace osu.Game.Screens.Play
 
             ensureSourceClockSet();
 
-            // Seeking the decoupled clock to its current time ensures that its source clock will be seeked to the same time
-            // This accounts for the clock source potentially taking time to enter a completely stopped state
-            Seek(GameplayClock.CurrentTime);
+            PrepareStart();
 
             // The case which caused this to be added is FrameStabilityContainer, which manages its own current and elapsed time.
             // Because we generally update our own current time quicker than children can query it (via Start/Seek/Update),
@@ -111,11 +109,19 @@ namespace osu.Game.Screens.Play
             });
         }
 
+        /// <summary>
+        /// When <see cref="Start"/> is called, this will be run to give an opportunity to prepare the clock at the correct
+        /// start location.
+        /// </summary>
+        protected virtual void PrepareStart()
+        {
+        }
+
         /// <summary>
         /// Seek to a specific time in gameplay.
         /// </summary>
         /// <param name="time">The destination time to seek to.</param>
-        public void Seek(double time)
+        public virtual void Seek(double time)
         {
             Logger.Log($"{nameof(GameplayClockContainer)} seeking to {time}");
 

osu.Game/Screens/Play/MasterGameplayClockContainer.cs

@@ -45,6 +45,17 @@ namespace osu.Game.Screens.Play
 
         private readonly List<Bindable<double>> nonGameplayAdjustments = new List<Bindable<double>>();
 
+        /// <summary>
+        /// Stores the time at which the last <see cref="StopGameplayClock"/> call was triggered.
+        /// This is used to ensure we resume from that precise point in time, ignoring the proceeding frequency ramp.
+        ///
+        /// Optimally, we'd have gameplay ramp down with the frequency, but I believe this was intentionally disabled
+        /// to avoid fails occurring after the pause screen has been shown.
+        ///
+        /// In the future I want to change this.
+        /// </summary>
+        private double? actualStopTime;
+
         public override IEnumerable<double> NonGameplayAdjustments => nonGameplayAdjustments.Select(b => b.Value);
 
         /// <summary>
@@ -86,6 +97,8 @@ namespace osu.Game.Screens.Play
 
         protected override void StopGameplayClock()
         {
+            actualStopTime = GameplayClock.CurrentTime;
+
             if (IsLoaded)
             {
                 // During normal operation, the source is stopped after performing a frequency ramp.
@@ -108,6 +121,25 @@ namespace osu.Game.Screens.Play
             }
         }
 
+        public override void Seek(double time)
+        {
+            // Safety in case the clock is seeked while stopped.
+            actualStopTime = null;
+
+            base.Seek(time);
+        }
+
+        protected override void PrepareStart()
+        {
+            if (actualStopTime != null)
+            {
+                Seek(actualStopTime.Value);
+                actualStopTime = null;
+            }
+            else
+                base.PrepareStart();
+        }
+
         protected override void StartGameplayClock()
         {
             addSourceClockAdjustments();