From 489e172a7660c07030eb38799f9e23681cecf813 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 22 Aug 2022 19:43:18 +0900 Subject: [PATCH] Simplify track start/stop/paused tracking --- .../Screens/Play/GameplayClockContainer.cs | 79 ++++++++++--------- .../Play/MasterGameplayClockContainer.cs | 39 +++++---- 2 files changed, 64 insertions(+), 54 deletions(-) diff --git a/osu.Game/Screens/Play/GameplayClockContainer.cs b/osu.Game/Screens/Play/GameplayClockContainer.cs index a00ae27781..897d2cbdcd 100644 --- a/osu.Game/Screens/Play/GameplayClockContainer.cs +++ b/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); } /// /// Starts gameplay and marks un-paused state. /// - 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`). - // 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); + ensureSourceClockSet(); - // The case which cause 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), - // this means that the first frame ever exposed to children may have a non-zero current time. - // - // If the child component is not aware of the parent ElapsedFrameTime (which is the case for FrameStabilityContainer) - // they will take on the new CurrentTime with a zero elapsed time. This can in turn cause components to behave incorrectly - // 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(GameplayClock.Start); - } + // 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. + // Because we generally update our own current time quicker than children can query it (via Start/Seek/Update), + // this means that the first frame ever exposed to children may have a non-zero current time. + // + // If the child component is not aware of the parent ElapsedFrameTime (which is the case for FrameStabilityContainer) + // they will take on the new CurrentTime with a zero elapsed time. This can in turn cause components to behave incorrectly + // 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(); + }); } /// @@ -125,7 +127,17 @@ namespace osu.Game.Screens.Play /// /// Stops gameplay and marks paused state. /// - 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(); /// /// Resets this and the source to an initial state ready for gameplay. @@ -134,8 +146,9 @@ namespace osu.Game.Screens.Play /// Whether to start the clock immediately, if not already started. public void Reset(double? time = null, bool startClock = false) { - // Manually stop the source in order to not affect the IsPaused state. - GameplayClock.Stop(); + bool wasPaused = isPaused.Value; + + 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); } - /// - /// Invoked when the value of is changed to start or stop the clock. - /// - /// Whether the clock should now be paused. - protected virtual void OnIsPausedChanged(ValueChangedEvent isPaused) - { - if (isPaused.NewValue) - GameplayClock.Stop(); - else - GameplayClock.Start(); - } - #region IAdjustableClock bool IAdjustableClock.Seek(double position) diff --git a/osu.Game/Screens/Play/MasterGameplayClockContainer.cs b/osu.Game/Screens/Play/MasterGameplayClockContainer.cs index c3c92eb0fe..168a5658f5 100644 --- a/osu.Game/Screens/Play/MasterGameplayClockContainer.cs +++ b/osu.Game/Screens/Play/MasterGameplayClockContainer.cs @@ -84,29 +84,23 @@ namespace osu.Game.Screens.Play return time; } - protected override void OnIsPausedChanged(ValueChangedEvent 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 == isPaused.NewValue) - base.OnIsPausedChanged(isPaused); - }); - } - else - this.TransformBindableTo(GameplayClock.ExternalPauseFrequencyAdjust, 1, 200, Easing.In); + if (IsPaused.Value) + base.StopGameplayClock(); + }); } else { - if (isPaused.NewValue) - base.OnIsPausedChanged(isPaused); + base.StopGameplayClock(); // 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(); + } } ///