From 799c74cfe533da28509699dd2feea84179a2b8f8 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 18 Jan 2024 02:40:34 +0900 Subject: [PATCH 1/3] Simplify gameplay pause sequence --- .../Visual/Gameplay/TestScenePause.cs | 5 +- .../Play/MasterGameplayClockContainer.cs | 66 ------------------- 2 files changed, 1 insertion(+), 70 deletions(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestScenePause.cs b/osu.Game.Tests/Visual/Gameplay/TestScenePause.cs index ec3b3e0822..d55af2777f 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestScenePause.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestScenePause.cs @@ -93,15 +93,12 @@ namespace osu.Game.Tests.Visual.Gameplay double currentTime = masterClock.CurrentTime; - bool goingForward = currentTime >= (masterClock.LastStopTime ?? lastStopTime); + bool goingForward = currentTime >= lastStopTime; alwaysGoingForward &= goingForward; if (!goingForward) Logger.Log($"Went too far backwards (last stop: {lastStopTime:N1} current: {currentTime:N1})"); - - if (masterClock.LastStopTime != null) - lastStopTime = masterClock.LastStopTime.Value; }; }); diff --git a/osu.Game/Screens/Play/MasterGameplayClockContainer.cs b/osu.Game/Screens/Play/MasterGameplayClockContainer.cs index 12f541167f..10451963a1 100644 --- a/osu.Game/Screens/Play/MasterGameplayClockContainer.cs +++ b/osu.Game/Screens/Play/MasterGameplayClockContainer.cs @@ -7,7 +7,6 @@ using osu.Framework.Allocation; using osu.Framework.Audio; using osu.Framework.Audio.Track; using osu.Framework.Bindables; -using osu.Framework.Graphics; using osu.Framework.Logging; using osu.Framework.Timing; using osu.Game.Beatmaps; @@ -60,17 +59,6 @@ namespace osu.Game.Screens.Play private readonly double skipTargetTime; - /// - /// Stores the time at which the last 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. - /// - internal double? LastStopTime; - [Resolved] private MusicController musicController { get; set; } = null!; @@ -113,71 +101,17 @@ namespace osu.Game.Screens.Play return time; } - protected override void StopGameplayClock() - { - LastStopTime = GameplayClock.CurrentTime; - - if (IsLoaded) - { - // During normal operation, the source is stopped after performing a frequency ramp. - this.TransformBindableTo(GameplayClock.ExternalPauseFrequencyAdjust, 0, 200, Easing.Out).OnComplete(_ => - { - if (IsPaused.Value) - base.StopGameplayClock(); - }); - } - else - { - 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 = 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. - GameplayClock.ProcessFrame(); - } - } - public override void Seek(double time) { - // Safety in case the clock is seeked while stopped. - LastStopTime = null; elapsedValidationTime = null; base.Seek(time); } - protected override void PrepareStart() - { - if (LastStopTime != null) - { - Seek(LastStopTime.Value); - LastStopTime = null; - } - else - base.PrepareStart(); - } - protected override void StartGameplayClock() { addAdjustmentsToTrack(); - 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(); - } } /// From 8ab8c90e338f00c784e05fc406540b872cefa5a0 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 18 Jan 2024 14:21:02 +0900 Subject: [PATCH 2/3] Remove "pause rate adjust" flow --- osu.Game/Beatmaps/FramedBeatmapClock.cs | 11 +++-------- .../Screens/Play/MasterGameplayClockContainer.cs | 2 -- osu.Game/Screens/Play/OffsetCorrectionClock.cs | 14 +++----------- 3 files changed, 6 insertions(+), 21 deletions(-) diff --git a/osu.Game/Beatmaps/FramedBeatmapClock.cs b/osu.Game/Beatmaps/FramedBeatmapClock.cs index 587e6bbeed..d0ffbdd459 100644 --- a/osu.Game/Beatmaps/FramedBeatmapClock.cs +++ b/osu.Game/Beatmaps/FramedBeatmapClock.cs @@ -27,11 +27,6 @@ namespace osu.Game.Beatmaps { private readonly bool applyOffsets; - /// - /// The total frequency adjustment from pause transforms. Should eventually be handled in a better way. - /// - public readonly BindableDouble ExternalPauseFrequencyAdjust = new BindableDouble(1); - private readonly OffsetCorrectionClock? userGlobalOffsetClock; private readonly OffsetCorrectionClock? platformOffsetClock; private readonly OffsetCorrectionClock? userBeatmapOffsetClock; @@ -69,13 +64,13 @@ namespace osu.Game.Beatmaps { // Audio timings in general with newer BASS versions don't match stable. // This only seems to be required on windows. We need to eventually figure out why, with a bit of luck. - platformOffsetClock = new OffsetCorrectionClock(interpolatedTrack, ExternalPauseFrequencyAdjust) { Offset = RuntimeInfo.OS == RuntimeInfo.Platform.Windows ? 15 : 0 }; + platformOffsetClock = new OffsetCorrectionClock(interpolatedTrack) { Offset = RuntimeInfo.OS == RuntimeInfo.Platform.Windows ? 15 : 0 }; // User global offset (set in settings) should also be applied. - userGlobalOffsetClock = new OffsetCorrectionClock(platformOffsetClock, ExternalPauseFrequencyAdjust); + userGlobalOffsetClock = new OffsetCorrectionClock(platformOffsetClock); // User per-beatmap offset will be applied to this final clock. - finalClockSource = userBeatmapOffsetClock = new OffsetCorrectionClock(userGlobalOffsetClock, ExternalPauseFrequencyAdjust); + finalClockSource = userBeatmapOffsetClock = new OffsetCorrectionClock(userGlobalOffsetClock); } else { diff --git a/osu.Game/Screens/Play/MasterGameplayClockContainer.cs b/osu.Game/Screens/Play/MasterGameplayClockContainer.cs index 10451963a1..93bdcb1cab 100644 --- a/osu.Game/Screens/Play/MasterGameplayClockContainer.cs +++ b/osu.Game/Screens/Play/MasterGameplayClockContainer.cs @@ -207,7 +207,6 @@ namespace osu.Game.Screens.Play musicController.ResetTrackAdjustments(); track.BindAdjustments(AdjustmentsFromMods); - track.AddAdjustment(AdjustableProperty.Frequency, GameplayClock.ExternalPauseFrequencyAdjust); track.AddAdjustment(AdjustableProperty.Frequency, UserPlaybackRate); speedAdjustmentsApplied = true; @@ -219,7 +218,6 @@ namespace osu.Game.Screens.Play return; track.UnbindAdjustments(AdjustmentsFromMods); - track.RemoveAdjustment(AdjustableProperty.Frequency, GameplayClock.ExternalPauseFrequencyAdjust); track.RemoveAdjustment(AdjustableProperty.Frequency, UserPlaybackRate); speedAdjustmentsApplied = false; diff --git a/osu.Game/Screens/Play/OffsetCorrectionClock.cs b/osu.Game/Screens/Play/OffsetCorrectionClock.cs index 207980f45c..e83ed7e464 100644 --- a/osu.Game/Screens/Play/OffsetCorrectionClock.cs +++ b/osu.Game/Screens/Play/OffsetCorrectionClock.cs @@ -1,15 +1,12 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -using osu.Framework.Bindables; using osu.Framework.Timing; namespace osu.Game.Screens.Play { public class OffsetCorrectionClock : FramedOffsetClock { - private readonly BindableDouble pauseRateAdjust; - private double offset; public new double Offset @@ -28,10 +25,9 @@ namespace osu.Game.Screens.Play public double RateAdjustedOffset => base.Offset; - public OffsetCorrectionClock(IClock source, BindableDouble pauseRateAdjust) + public OffsetCorrectionClock(IClock source) : base(source) { - this.pauseRateAdjust = pauseRateAdjust; } public override void ProcessFrame() @@ -42,12 +38,8 @@ namespace osu.Game.Screens.Play private void updateOffset() { - // changing this during the pause transform effect will cause a potentially large offset to be suddenly applied as we approach zero rate. - if (pauseRateAdjust.Value == 1) - { - // we always want to apply the same real-time offset, so it should be adjusted by the difference in playback rate (from realtime) to achieve this. - base.Offset = Offset * Rate; - } + // we always want to apply the same real-time offset, so it should be adjusted by the difference in playback rate (from realtime) to achieve this. + base.Offset = Offset * Rate; } } } From c4e9bcd140bbb59a094c282fc7b21ac04e8ec5d2 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 18 Jan 2024 20:06:53 +0900 Subject: [PATCH 3/3] Remove test guarantee of audio time not advancing --- osu.Game.Tests/Visual/Gameplay/TestScenePause.cs | 2 +- osu.Game/Screens/Play/GameplayClockContainer.cs | 10 ---------- 2 files changed, 1 insertion(+), 11 deletions(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestScenePause.cs b/osu.Game.Tests/Visual/Gameplay/TestScenePause.cs index d55af2777f..73aa3be73d 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestScenePause.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestScenePause.cs @@ -122,7 +122,7 @@ namespace osu.Game.Tests.Visual.Gameplay resumeAndConfirm(); - AddAssert("Resumed without seeking forward", () => Player.LastResumeTime, () => Is.LessThanOrEqualTo(Player.LastPauseTime)); + AddAssert("continued playing forward", () => Player.LastResumeTime, () => Is.GreaterThanOrEqualTo(Player.LastPauseTime)); AddUntilStep("player playing", () => Player.LocalUserPlaying.Value); } diff --git a/osu.Game/Screens/Play/GameplayClockContainer.cs b/osu.Game/Screens/Play/GameplayClockContainer.cs index 4def1d36bb..c039d1e535 100644 --- a/osu.Game/Screens/Play/GameplayClockContainer.cs +++ b/osu.Game/Screens/Play/GameplayClockContainer.cs @@ -78,8 +78,6 @@ namespace osu.Game.Screens.Play isPaused.Value = false; - 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), // this means that the first frame ever exposed to children may have a non-zero current time. @@ -99,14 +97,6 @@ namespace osu.Game.Screens.Play }); } - /// - /// When is called, this will be run to give an opportunity to prepare the clock at the correct - /// start location. - /// - protected virtual void PrepareStart() - { - } - /// /// Seek to a specific time in gameplay. ///