Simplify track start/stop/paused tracking

osu.Game/Screens/Play/GameplayClockContainer.cs

@@ -74,39 +74,41 @@ namespace osu.Game.Screens.Play
                 GameplayClock = new FramedBeatmapClock(sourceClock, applyOffsets) { IsCoupled = false },
                 Content
             };
-
-            IsPaused.BindValueChanged(OnIsPausedChanged);
         }
 
         /// <summary>
         /// Starts gameplay and marks un-paused state.
         /// </summary>
-        public virtual void Start()
+        public void Start()
         {
-            ensureSourceClockSet();
+            if (!isPaused.Value)
+                return;
 
             isPaused.Value = false;
 
-            // the clock may be stopped via internal means (ie. not via `IsPaused`).
+            ensureSourceClockSet();
-            // see Reset() calling `GameplayClock.Stop()` as one example.
-            if (!GameplayClock.IsRunning)
-            {
-                // 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);
 
-                // The case which cause this to be added is FrameStabilityContainer, which manages its own current and elapsed time.
+            // Seeking the decoupled clock to its current time ensures that its source clock will be seeked to the same time
-                // Because we generally update our own current time quicker than children can query it (via Start/Seek/Update),
+            // This accounts for the clock source potentially taking time to enter a completely stopped state
-                // this means that the first frame ever exposed to children may have a non-zero current time.
+            Seek(GameplayClock.CurrentTime);
-                //
+
-                // If the child component is not aware of the parent ElapsedFrameTime (which is the case for FrameStabilityContainer)
+            // The case which cause this to be added is FrameStabilityContainer, which manages its own current and elapsed time.
-                // they will take on the new CurrentTime with a zero elapsed time. This can in turn cause components to behave incorrectly
+            // Because we generally update our own current time quicker than children can query it (via Start/Seek/Update),
-                // if they are intending to trigger events at the precise StartTime (ie. DrawableStoryboardSample).
+            // this means that the first frame ever exposed to children may have a non-zero current time.
-                //
+            //
-                // By scheduling the start call, children are guaranteed to receive one frame at the original start time, allowing
+            // If the child component is not aware of the parent ElapsedFrameTime (which is the case for FrameStabilityContainer)
-                // then to progress with a correct locally calculated elapsed time.
+            // they will take on the new CurrentTime with a zero elapsed time. This can in turn cause components to behave incorrectly
-                SchedulerAfterChildren.Add(GameplayClock.Start);
+            // if they are intending to trigger events at the precise StartTime (ie. DrawableStoryboardSample).
-            }
+            //
+            // By scheduling the start call, children are guaranteed to receive one frame at the original start time, allowing
+            // then to progress with a correct locally calculated elapsed time.
+            SchedulerAfterChildren.Add(() =>
+            {
+                if (isPaused.Value)
+                    return;
+
+                StartGameplayClock();
+            });
         }
 
         /// <summary>
@@ -125,7 +127,17 @@ namespace osu.Game.Screens.Play
         /// <summary>
         /// Stops gameplay and marks paused state.
         /// </summary>
-        public void Stop() => isPaused.Value = true;
+        public void Stop()
+        {
+            if (isPaused.Value)
+                return;
+
+            isPaused.Value = true;
+            StopGameplayClock();
+        }
+
+        protected virtual void StartGameplayClock() => GameplayClock.Start();
+        protected virtual void StopGameplayClock() => GameplayClock.Stop();
 
         /// <summary>
         /// Resets this <see cref="GameplayClockContainer"/> and the source to an initial state ready for gameplay.
@@ -134,8 +146,9 @@ namespace osu.Game.Screens.Play
         /// <param name="startClock">Whether to start the clock immediately, if not already started.</param>
         public void Reset(double? time = null, bool startClock = false)
         {
-            // Manually stop the source in order to not affect the IsPaused state.
+            bool wasPaused = isPaused.Value;
-            GameplayClock.Stop();
+
+            Stop();
 
             ensureSourceClockSet();
 
@@ -144,7 +157,7 @@ namespace osu.Game.Screens.Play
 
             Seek(StartTime);
 
-            if (!IsPaused.Value || startClock)
+            if (!wasPaused || startClock)
                 Start();
         }
 
@@ -167,18 +180,6 @@ namespace osu.Game.Screens.Play
                 ChangeSource(SourceClock);
         }
 
-        /// <summary>
-        /// Invoked when the value of <see cref="IsPaused"/> is changed to start or stop the <see cref="GameplayClock"/> clock.
-        /// </summary>
-        /// <param name="isPaused">Whether the clock should now be paused.</param>
-        protected virtual void OnIsPausedChanged(ValueChangedEvent<bool> isPaused)
-        {
-            if (isPaused.NewValue)
-                GameplayClock.Stop();
-            else
-                GameplayClock.Start();
-        }
-
         #region IAdjustableClock
 
         bool IAdjustableClock.Seek(double position)

osu.Game/Screens/Play/MasterGameplayClockContainer.cs

@@ -84,29 +84,23 @@ namespace osu.Game.Screens.Play
             return time;
         }
 
-        protected override void OnIsPausedChanged(ValueChangedEvent<bool> isPaused)
+        protected override void StopGameplayClock()
         {
             if (IsLoaded)
             {
                 // During normal operation, the source is stopped after performing a frequency ramp.
-                if (isPaused.NewValue)
+                this.TransformBindableTo(GameplayClock.ExternalPauseFrequencyAdjust, 0, 200, Easing.Out).OnComplete(_ =>
                 {
-                    this.TransformBindableTo(GameplayClock.ExternalPauseFrequencyAdjust, 0, 200, Easing.Out).OnComplete(_ =>
+                    if (IsPaused.Value)
-                    {
+                        base.StopGameplayClock();
-                        if (IsPaused.Value == isPaused.NewValue)
+                });
-                            base.OnIsPausedChanged(isPaused);
-                    });
-                }
-                else
-                    this.TransformBindableTo(GameplayClock.ExternalPauseFrequencyAdjust, 1, 200, Easing.In);
             }
             else
             {
-                if (isPaused.NewValue)
+                base.StopGameplayClock();
-                    base.OnIsPausedChanged(isPaused);
 
                 // If not yet loaded, we still want to ensure relevant state is correct, as it is used for offset calculations.
-                GameplayClock.ExternalPauseFrequencyAdjust.Value = isPaused.NewValue ? 0 : 1;
+                GameplayClock.ExternalPauseFrequencyAdjust.Value = 0;
 
                 // We must also process underlying gameplay clocks to update rate-adjusted offsets with the new frequency adjustment.
                 // Without doing this, an initial seek may be performed with the wrong offset.
@@ -114,10 +108,25 @@ namespace osu.Game.Screens.Play
             }
         }
 
-        public override void Start()
+        protected override void StartGameplayClock()
         {
             addSourceClockAdjustments();
-            base.Start();
+
+            base.StartGameplayClock();
+
+            if (IsLoaded)
+            {
+                this.TransformBindableTo(GameplayClock.ExternalPauseFrequencyAdjust, 1, 200, Easing.In);
+            }
+            else
+            {
+                // If not yet loaded, we still want to ensure relevant state is correct, as it is used for offset calculations.
+                GameplayClock.ExternalPauseFrequencyAdjust.Value = 1;
+
+                // We must also process underlying gameplay clocks to update rate-adjusted offsets with the new frequency adjustment.
+                // Without doing this, an initial seek may be performed with the wrong offset.
+                GameplayClock.ProcessFrame();
+            }
         }
 
         /// <summary>