From 465c95e952cceb7ff62b496861728b6d81e4b5b1 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sat, 16 Mar 2019 14:20:10 +0900 Subject: [PATCH 01/11] Refactor pause logic so GameplayClockContainer is in control --- .../Screens/Play/GameplayClockContainer.cs | 7 ++++- .../Screens/Play/PausableGameplayContainer.cs | 26 ++++++++---------- osu.Game/Screens/Play/Player.cs | 27 +++++++++++-------- 3 files changed, 33 insertions(+), 27 deletions(-) diff --git a/osu.Game/Screens/Play/GameplayClockContainer.cs b/osu.Game/Screens/Play/GameplayClockContainer.cs index 3c2cec1d94..1e4c9aba92 100644 --- a/osu.Game/Screens/Play/GameplayClockContainer.cs +++ b/osu.Game/Screens/Play/GameplayClockContainer.cs @@ -118,11 +118,16 @@ namespace osu.Game.Screens.Play // This accounts for the audio clock source potentially taking time to enter a completely stopped state adjustableClock.Seek(adjustableClock.CurrentTime); adjustableClock.Start(); + IsPaused.Value = false; } public void Seek(double time) => adjustableClock.Seek(time); - public void Stop() => adjustableClock.Stop(); + public void Stop() + { + adjustableClock.Stop(); + IsPaused.Value = true; + } public void ResetLocalAdjustments() { diff --git a/osu.Game/Screens/Play/PausableGameplayContainer.cs b/osu.Game/Screens/Play/PausableGameplayContainer.cs index 99f0083b55..6363b92a8f 100644 --- a/osu.Game/Screens/Play/PausableGameplayContainer.cs +++ b/osu.Game/Screens/Play/PausableGameplayContainer.cs @@ -41,8 +41,8 @@ namespace osu.Game.Screens.Play public Action OnRetry; public Action OnQuit; - public Action Stop; - public Action Start; + public Action RequestPause; + public Action RequestResume; /// /// Creates a new . @@ -70,15 +70,12 @@ namespace osu.Game.Screens.Play }; } - public void Pause(bool force = false) => Schedule(() => // Scheduled to ensure a stable position in execution order, no matter how it was called. + public void Pause() => Schedule(() => // Scheduled to ensure a stable position in execution order, no matter how it was called. { - if (!CanPause && !force) return; - - if (IsPaused.Value) return; + if (!CanPause) return; // stop the seekable clock (stops the audio eventually) - Stop?.Invoke(); - IsPaused.Value = true; + RequestPause?.Invoke(); pauseOverlay.Show(); @@ -89,14 +86,13 @@ namespace osu.Game.Screens.Play { if (!IsPaused.Value) return; - IsResuming = false; - lastPauseActionTime = Time.Current; - - IsPaused.Value = false; - - Start?.Invoke(); - pauseOverlay.Hide(); + + RequestResume?.Invoke(() => + { + IsResuming = false; + lastPauseActionTime = Time.Current; + }); } private OsuGameBase game; diff --git a/osu.Game/Screens/Play/Player.cs b/osu.Game/Screens/Play/Player.cs index b11c5e51c9..7637cfe869 100644 --- a/osu.Game/Screens/Play/Player.cs +++ b/osu.Game/Screens/Play/Player.cs @@ -111,10 +111,14 @@ namespace osu.Game.Screens.Play Retries = RestartCount, OnRetry = restart, OnQuit = performUserRequestedExit, - Start = gameplayClockContainer.Start, - Stop = gameplayClockContainer.Stop, + RequestResume = completion => + { + gameplayClockContainer.Start(); + completion(); + }, + RequestPause = gameplayClockContainer.Stop, IsPaused = { BindTarget = gameplayClockContainer.IsPaused }, - CheckCanPause = () => AllowPause && ValidForResume && !HasFailed && !RulesetContainer.HasReplayLoaded.Value, + CheckCanPause = () => CanPause, Children = new[] { StoryboardContainer = CreateStoryboardContainer(), @@ -337,6 +341,9 @@ namespace osu.Game.Screens.Play base.OnSuspending(next); } + public bool CanPause => AllowPause && ValidForResume && !HasFailed && !RulesetContainer.HasReplayLoaded.Value + && (PausableGameplayContainer?.IsPaused.Value == false || PausableGameplayContainer?.IsResuming == true); + public override bool OnExiting(IScreen next) { if (onCompletionEvent != null) @@ -346,18 +353,16 @@ namespace osu.Game.Screens.Play return true; } - if ((!AllowPause || HasFailed || !ValidForResume || PausableGameplayContainer?.IsPaused.Value != false || RulesetContainer?.HasReplayLoaded.Value != false) && (!PausableGameplayContainer?.IsResuming ?? true)) + if (LoadedBeatmapSuccessfully && CanPause) { - gameplayClockContainer.ResetLocalAdjustments(); - - fadeOut(); - return base.OnExiting(next); + PausableGameplayContainer?.Pause(); + return true; } - if (LoadedBeatmapSuccessfully) - PausableGameplayContainer?.Pause(); + gameplayClockContainer.ResetLocalAdjustments(); - return true; + fadeOut(); + return base.OnExiting(next); } private void fadeOut(bool instant = false) From bcaff9f7b47efdc3c0087609daef32e48110e1ab Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 18 Mar 2019 00:46:15 +0900 Subject: [PATCH 02/11] Add basic pause tests --- osu.Game.Tests/Visual/TestCasePause.cs | 47 +++++++++++++++++++ .../Screens/Play/GameplayClockContainer.cs | 6 +-- osu.Game/Screens/Play/Player.cs | 26 +++++----- 3 files changed, 63 insertions(+), 16 deletions(-) create mode 100644 osu.Game.Tests/Visual/TestCasePause.cs diff --git a/osu.Game.Tests/Visual/TestCasePause.cs b/osu.Game.Tests/Visual/TestCasePause.cs new file mode 100644 index 0000000000..6966eb3de9 --- /dev/null +++ b/osu.Game.Tests/Visual/TestCasePause.cs @@ -0,0 +1,47 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System; +using osu.Game.Rulesets; +using osu.Game.Rulesets.Osu; +using osu.Game.Rulesets.Scoring; +using osu.Game.Screens.Play; + +namespace osu.Game.Tests.Visual +{ + public class TestCasePause : TestCasePlayer + { + public TestCasePause() + : base(new OsuRuleset()) + { + } + + protected override Player CreatePlayer(Ruleset ruleset) => new PausePlayer(); + + protected override void AddCheckSteps(Func player) + { + PausePlayer pausable() => (PausePlayer)player(); + + base.AddCheckSteps(player); + //AddUntilStep(() => pausable().ScoreProcessor.TotalScore.Value > 0, "score above zero"); + + AddStep("pause", () => pausable().PausableGameplayContainer.Pause()); + AddAssert("clock stopped", () => !pausable().GameplayClockContainer.GameplayClock.IsRunning); + + AddStep("resume", () => pausable().PausableGameplayContainer.Resume()); + AddUntilStep(() => pausable().GameplayClockContainer.GameplayClock.IsRunning, "clock started"); + + AddStep("pause too soon", () => pausable().PausableGameplayContainer.Pause()); + AddAssert("clock not stopped", () => pausable().GameplayClockContainer.GameplayClock.IsRunning); + } + + private class PausePlayer : Player + { + public new PausableGameplayContainer PausableGameplayContainer => base.PausableGameplayContainer; + + public new GameplayClockContainer GameplayClockContainer => base.GameplayClockContainer; + + public new ScoreProcessor ScoreProcessor => base.ScoreProcessor; + } + } +} diff --git a/osu.Game/Screens/Play/GameplayClockContainer.cs b/osu.Game/Screens/Play/GameplayClockContainer.cs index 1e4c9aba92..deac5e02bf 100644 --- a/osu.Game/Screens/Play/GameplayClockContainer.cs +++ b/osu.Game/Screens/Play/GameplayClockContainer.cs @@ -18,7 +18,7 @@ using osu.Game.Rulesets.Mods; namespace osu.Game.Screens.Play { /// - /// Encapsulates gameplay timing logic and provides a for children. + /// Encapsulates gameplay timing logic and provides a for children. /// public class GameplayClockContainer : Container { @@ -48,7 +48,7 @@ namespace osu.Game.Screens.Play /// The final clock which is exposed to underlying components. /// [Cached] - private readonly GameplayClock gameplayClock; + public readonly GameplayClock GameplayClock; private Bindable userAudioOffset; @@ -78,7 +78,7 @@ namespace osu.Game.Screens.Play offsetClock = new FramedOffsetClock(platformOffsetClock); // the clock to be exposed via DI to children. - gameplayClock = new GameplayClock(offsetClock); + GameplayClock = new GameplayClock(offsetClock); } [BackgroundDependencyLoader] diff --git a/osu.Game/Screens/Play/Player.cs b/osu.Game/Screens/Play/Player.cs index 7637cfe869..3d60a44c85 100644 --- a/osu.Game/Screens/Play/Player.cs +++ b/osu.Game/Screens/Play/Player.cs @@ -82,7 +82,7 @@ namespace osu.Game.Screens.Play public bool LoadedBeatmapSuccessfully => RulesetContainer?.Objects.Any() == true; - private GameplayClockContainer gameplayClockContainer; + protected GameplayClockContainer GameplayClockContainer { get; private set; } [BackgroundDependencyLoader] private void load(AudioManager audio, IAPIProvider api, OsuConfigManager config) @@ -102,9 +102,9 @@ namespace osu.Game.Screens.Play if (!ScoreProcessor.Mode.Disabled) config.BindWith(OsuSetting.ScoreDisplayMode, ScoreProcessor.Mode); - InternalChild = gameplayClockContainer = new GameplayClockContainer(working, AllowLeadIn, RulesetContainer.GameplayStartTime); + InternalChild = GameplayClockContainer = new GameplayClockContainer(working, AllowLeadIn, RulesetContainer.GameplayStartTime); - gameplayClockContainer.Children = new Drawable[] + GameplayClockContainer.Children = new Drawable[] { PausableGameplayContainer = new PausableGameplayContainer { @@ -113,11 +113,11 @@ namespace osu.Game.Screens.Play OnQuit = performUserRequestedExit, RequestResume = completion => { - gameplayClockContainer.Start(); + GameplayClockContainer.Start(); completion(); }, - RequestPause = gameplayClockContainer.Stop, - IsPaused = { BindTarget = gameplayClockContainer.IsPaused }, + RequestPause = GameplayClockContainer.Stop, + IsPaused = { BindTarget = GameplayClockContainer.IsPaused }, CheckCanPause = () => CanPause, Children = new[] { @@ -141,15 +141,15 @@ namespace osu.Game.Screens.Play HUDOverlay = new HUDOverlay(ScoreProcessor, RulesetContainer, working) { HoldToQuit = { Action = performUserRequestedExit }, - PlayerSettingsOverlay = { PlaybackSettings = { UserPlaybackRate = { BindTarget = gameplayClockContainer.UserPlaybackRate } } }, + PlayerSettingsOverlay = { PlaybackSettings = { UserPlaybackRate = { BindTarget = GameplayClockContainer.UserPlaybackRate } } }, KeyCounter = { Visible = { BindTarget = RulesetContainer.HasReplayLoaded } }, - RequestSeek = gameplayClockContainer.Seek, + RequestSeek = GameplayClockContainer.Seek, Anchor = Anchor.Centre, Origin = Anchor.Centre }, new SkipOverlay(RulesetContainer.GameplayStartTime) { - RequestSeek = gameplayClockContainer.Seek + RequestSeek = GameplayClockContainer.Seek }, } }, @@ -171,7 +171,7 @@ namespace osu.Game.Screens.Play }; // bind clock into components that require it - RulesetContainer.IsPaused.BindTo(gameplayClockContainer.IsPaused); + RulesetContainer.IsPaused.BindTo(GameplayClockContainer.IsPaused); if (ShowStoryboard.Value) initializeStoryboard(false); @@ -295,7 +295,7 @@ namespace osu.Game.Screens.Play if (Beatmap.Value.Mods.Value.OfType().Any(m => !m.AllowFail)) return false; - gameplayClockContainer.Stop(); + GameplayClockContainer.Stop(); HasFailed = true; failOverlay.Retries = RestartCount; @@ -329,7 +329,7 @@ namespace osu.Game.Screens.Play storyboardReplacesBackground.Value = Beatmap.Value.Storyboard.ReplacesBackground && Beatmap.Value.Storyboard.HasDrawable; - gameplayClockContainer.Restart(); + GameplayClockContainer.Restart(); PausableGameplayContainer.Alpha = 0; PausableGameplayContainer.FadeIn(750, Easing.OutQuint); @@ -359,7 +359,7 @@ namespace osu.Game.Screens.Play return true; } - gameplayClockContainer.ResetLocalAdjustments(); + GameplayClockContainer.ResetLocalAdjustments(); fadeOut(); return base.OnExiting(next); From f13003c53b7957cf76be0042a85f7d99512349cb Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 18 Mar 2019 11:12:47 +0900 Subject: [PATCH 03/11] Simplify and localise storyboard logic in Player.cs --- osu.Game/Screens/Play/Player.cs | 48 ++++++++++++++++++--------------- 1 file changed, 26 insertions(+), 22 deletions(-) diff --git a/osu.Game/Screens/Play/Player.cs b/osu.Game/Screens/Play/Player.cs index 3d60a44c85..0e8bedefb0 100644 --- a/osu.Game/Screens/Play/Player.cs +++ b/osu.Game/Screens/Play/Player.cs @@ -1,4 +1,4 @@ -// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. using System; @@ -70,9 +70,32 @@ namespace osu.Game.Screens.Play protected HUDOverlay HUDOverlay { get; private set; } private FailOverlay failOverlay; + #region Storyboard + private DrawableStoryboard storyboard; protected UserDimContainer StoryboardContainer { get; private set; } + private void initializeStoryboard(bool asyncLoad) + { + if (StoryboardContainer == null || storyboard != null) + return; + + if (!ShowStoryboard.Value) + return; + + var beatmap = Beatmap.Value; + + storyboard = beatmap.Storyboard.CreateDrawable(); + storyboard.Masking = true; + + if (asyncLoad) + LoadComponentAsync(storyboard, StoryboardContainer.Add); + else + StoryboardContainer.Add(storyboard); + } + + #endregion + protected virtual UserDimContainer CreateStoryboardContainer() => new UserDimContainer(true) { RelativeSizeAxes = Axes.Both, @@ -173,7 +196,7 @@ namespace osu.Game.Screens.Play // bind clock into components that require it RulesetContainer.IsPaused.BindTo(GameplayClockContainer.IsPaused); - if (ShowStoryboard.Value) + // load storyboard as part of player's load if we can initializeStoryboard(false); // Bind ScoreProcessor to ourselves @@ -317,10 +340,7 @@ namespace osu.Game.Screens.Play .Delay(250) .FadeIn(250); - ShowStoryboard.ValueChanged += enabled => - { - if (enabled.NewValue) initializeStoryboard(true); - }; + ShowStoryboard.ValueChanged += _ => initializeStoryboard(true); Background.EnableUserDim.Value = true; @@ -376,22 +396,6 @@ namespace osu.Game.Screens.Play protected override bool OnScroll(ScrollEvent e) => mouseWheelDisabled.Value && !PausableGameplayContainer.IsPaused.Value; - private void initializeStoryboard(bool asyncLoad) - { - if (StoryboardContainer == null || storyboard != null) - return; - - var beatmap = Beatmap.Value; - - storyboard = beatmap.Storyboard.CreateDrawable(); - storyboard.Masking = true; - - if (asyncLoad) - LoadComponentAsync(storyboard, StoryboardContainer.Add); - else - StoryboardContainer.Add(storyboard); - } - protected virtual Results CreateResults(ScoreInfo score) => new SoloResults(score); } } From 536b5e0dab579ec7e55ee09d08397f30cfcb13b9 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 18 Mar 2019 11:48:11 +0900 Subject: [PATCH 04/11] Remove PausableGameplayContainer --- .../Visual/TestCaseBackgroundScreenBeatmap.cs | 6 +- .../Visual/TestCaseGameplayMenuOverlay.cs | 6 +- osu.Game.Tests/Visual/TestCasePause.cs | 28 ++- .../Screens/Play/PausableGameplayContainer.cs | 133 ------------ osu.Game/Screens/Play/PauseOverlay.cs | 29 +++ osu.Game/Screens/Play/Player.cs | 190 ++++++++++++------ 6 files changed, 183 insertions(+), 209 deletions(-) delete mode 100644 osu.Game/Screens/Play/PausableGameplayContainer.cs create mode 100644 osu.Game/Screens/Play/PauseOverlay.cs diff --git a/osu.Game.Tests/Visual/TestCaseBackgroundScreenBeatmap.cs b/osu.Game.Tests/Visual/TestCaseBackgroundScreenBeatmap.cs index 5484824c5b..d62ae07f6a 100644 --- a/osu.Game.Tests/Visual/TestCaseBackgroundScreenBeatmap.cs +++ b/osu.Game.Tests/Visual/TestCaseBackgroundScreenBeatmap.cs @@ -188,10 +188,10 @@ namespace osu.Game.Tests.Visual public void PauseTest() { performFullSetup(true); - AddStep("Pause", () => player.CurrentPausableGameplayContainer.Pause()); + AddStep("Pause", () => player.Pause()); waitForDim(); AddAssert("Screen is dimmed", () => songSelect.IsBackgroundDimmed()); - AddStep("Unpause", () => player.CurrentPausableGameplayContainer.Resume()); + AddStep("Unpause", () => player.Resume()); waitForDim(); AddAssert("Screen is dimmed", () => songSelect.IsBackgroundDimmed()); } @@ -328,8 +328,6 @@ namespace osu.Game.Tests.Visual }; } - public PausableGameplayContainer CurrentPausableGameplayContainer => PausableGameplayContainer; - public UserDimContainer CurrentStoryboardContainer => StoryboardContainer; // Whether or not the player should be allowed to load. diff --git a/osu.Game.Tests/Visual/TestCaseGameplayMenuOverlay.cs b/osu.Game.Tests/Visual/TestCaseGameplayMenuOverlay.cs index 93a059d214..c5ad57fec9 100644 --- a/osu.Game.Tests/Visual/TestCaseGameplayMenuOverlay.cs +++ b/osu.Game.Tests/Visual/TestCaseGameplayMenuOverlay.cs @@ -17,15 +17,15 @@ namespace osu.Game.Tests.Visual [Description("player pause/fail screens")] public class TestCaseGameplayMenuOverlay : ManualInputManagerTestCase { - public override IReadOnlyList RequiredTypes => new[] { typeof(FailOverlay), typeof(PausableGameplayContainer) }; + public override IReadOnlyList RequiredTypes => new[] { typeof(FailOverlay), typeof(PauseOverlay) }; private FailOverlay failOverlay; - private PausableGameplayContainer.PauseOverlay pauseOverlay; + private PauseOverlay pauseOverlay; [BackgroundDependencyLoader] private void load() { - Add(pauseOverlay = new PausableGameplayContainer.PauseOverlay + Add(pauseOverlay = new PauseOverlay { OnResume = () => Logger.Log(@"Resume"), OnRetry = () => Logger.Log(@"Retry"), diff --git a/osu.Game.Tests/Visual/TestCasePause.cs b/osu.Game.Tests/Visual/TestCasePause.cs index 6966eb3de9..622a12da81 100644 --- a/osu.Game.Tests/Visual/TestCasePause.cs +++ b/osu.Game.Tests/Visual/TestCasePause.cs @@ -2,6 +2,7 @@ // See the LICENCE file in the repository root for full licence text. using System; +using osu.Framework.Graphics.Containers; using osu.Game.Rulesets; using osu.Game.Rulesets.Osu; using osu.Game.Rulesets.Scoring; @@ -16,6 +17,8 @@ namespace osu.Game.Tests.Visual { } + protected override bool AllowFail => true; + protected override Player CreatePlayer(Ruleset ruleset) => new PausePlayer(); protected override void AddCheckSteps(Func player) @@ -25,23 +28,36 @@ namespace osu.Game.Tests.Visual base.AddCheckSteps(player); //AddUntilStep(() => pausable().ScoreProcessor.TotalScore.Value > 0, "score above zero"); - AddStep("pause", () => pausable().PausableGameplayContainer.Pause()); + AddStep("pause", () => pausable().Pause()); AddAssert("clock stopped", () => !pausable().GameplayClockContainer.GameplayClock.IsRunning); + AddAssert("pause overlay shown", () => pausable().PauseOverlayVisible); - AddStep("resume", () => pausable().PausableGameplayContainer.Resume()); - AddUntilStep(() => pausable().GameplayClockContainer.GameplayClock.IsRunning, "clock started"); + AddStep("resume", () => pausable().Resume()); + AddAssert("pause overlay hidden", () => !pausable().PauseOverlayVisible); - AddStep("pause too soon", () => pausable().PausableGameplayContainer.Pause()); + AddStep("pause too soon", () => pausable().Pause()); AddAssert("clock not stopped", () => pausable().GameplayClockContainer.GameplayClock.IsRunning); + AddAssert("pause overlay hidden", () => !pausable().PauseOverlayVisible); + + AddUntilStep(() => pausable().HasFailed, "wait for fail"); + + AddAssert("fail overlay shown", () => pausable().FailOverlayVisible); + + AddStep("try to pause", () => pausable().Pause()); + + AddAssert("pause overlay hidden", () => !pausable().PauseOverlayVisible); + AddAssert("fail overlay still shown", () => pausable().FailOverlayVisible); } private class PausePlayer : Player { - public new PausableGameplayContainer PausableGameplayContainer => base.PausableGameplayContainer; - public new GameplayClockContainer GameplayClockContainer => base.GameplayClockContainer; public new ScoreProcessor ScoreProcessor => base.ScoreProcessor; + + public bool FailOverlayVisible => FailOverlay.State == Visibility.Visible; + + public bool PauseOverlayVisible => PauseOverlay.State == Visibility.Visible; } } } diff --git a/osu.Game/Screens/Play/PausableGameplayContainer.cs b/osu.Game/Screens/Play/PausableGameplayContainer.cs deleted file mode 100644 index 6363b92a8f..0000000000 --- a/osu.Game/Screens/Play/PausableGameplayContainer.cs +++ /dev/null @@ -1,133 +0,0 @@ -// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. -// See the LICENCE file in the repository root for full licence text. - -using System; -using System.Linq; -using osu.Framework.Allocation; -using osu.Framework.Bindables; -using osu.Framework.Graphics; -using osu.Framework.Graphics.Containers; -using osu.Game.Graphics; -using osuTK.Graphics; - -namespace osu.Game.Screens.Play -{ - /// - /// A container which handles pausing children, displaying an overlay blocking its children during paused state. - /// - public class PausableGameplayContainer : Container - { - public readonly BindableBool IsPaused = new BindableBool(); - - public Func CheckCanPause; - - private const double pause_cooldown = 1000; - private double lastPauseActionTime; - - private readonly PauseOverlay pauseOverlay; - - private readonly Container content; - - protected override Container Content => content; - - public int Retries - { - set => pauseOverlay.Retries = value; - } - - public bool CanPause => (CheckCanPause?.Invoke() ?? true) && Time.Current >= lastPauseActionTime + pause_cooldown; - public bool IsResuming { get; private set; } - - public Action OnRetry; - public Action OnQuit; - - public Action RequestPause; - public Action RequestResume; - - /// - /// Creates a new . - /// - public PausableGameplayContainer() - { - RelativeSizeAxes = Axes.Both; - - InternalChildren = new[] - { - content = new Container - { - RelativeSizeAxes = Axes.Both - }, - pauseOverlay = new PauseOverlay - { - OnResume = () => - { - IsResuming = true; - this.Delay(400).Schedule(Resume); - }, - OnRetry = () => OnRetry(), - OnQuit = () => OnQuit(), - } - }; - } - - public void Pause() => Schedule(() => // Scheduled to ensure a stable position in execution order, no matter how it was called. - { - if (!CanPause) return; - - // stop the seekable clock (stops the audio eventually) - RequestPause?.Invoke(); - - pauseOverlay.Show(); - - lastPauseActionTime = Time.Current; - }); - - public void Resume() - { - if (!IsPaused.Value) return; - - pauseOverlay.Hide(); - - RequestResume?.Invoke(() => - { - IsResuming = false; - lastPauseActionTime = Time.Current; - }); - } - - private OsuGameBase game; - - [BackgroundDependencyLoader] - private void load(OsuGameBase game) - { - this.game = game; - } - - protected override void Update() - { - // eagerly pause when we lose window focus (if we are locally playing). - if (!game.IsActive.Value && CanPause) - Pause(); - - base.Update(); - } - - public class PauseOverlay : GameplayMenuOverlay - { - public Action OnResume; - - public override string Header => "paused"; - public override string Description => "you're not going to do what i think you're going to do, are ya?"; - - protected override Action BackAction => () => InternalButtons.Children.First().Click(); - - [BackgroundDependencyLoader] - private void load(OsuColour colours) - { - AddButton("Continue", colours.Green, () => OnResume?.Invoke()); - AddButton("Retry", colours.YellowDark, () => OnRetry?.Invoke()); - AddButton("Quit", new Color4(170, 27, 39, 255), () => OnQuit?.Invoke()); - } - } - } -} diff --git a/osu.Game/Screens/Play/PauseOverlay.cs b/osu.Game/Screens/Play/PauseOverlay.cs new file mode 100644 index 0000000000..6cc6027a03 --- /dev/null +++ b/osu.Game/Screens/Play/PauseOverlay.cs @@ -0,0 +1,29 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System; +using System.Linq; +using osu.Framework.Allocation; +using osu.Game.Graphics; +using osuTK.Graphics; + +namespace osu.Game.Screens.Play +{ + public class PauseOverlay : GameplayMenuOverlay + { + public Action OnResume; + + public override string Header => "paused"; + public override string Description => "you're not going to do what i think you're going to do, are ya?"; + + protected override Action BackAction => () => InternalButtons.Children.First().Click(); + + [BackgroundDependencyLoader] + private void load(OsuColour colours) + { + AddButton("Continue", colours.Green, () => OnResume?.Invoke()); + AddButton("Retry", colours.YellowDark, () => OnRetry?.Invoke()); + AddButton("Quit", new Color4(170, 27, 39, 255), () => OnQuit?.Invoke()); + } + } +} diff --git a/osu.Game/Screens/Play/Player.cs b/osu.Game/Screens/Play/Player.cs index 0e8bedefb0..018ff900ee 100644 --- a/osu.Game/Screens/Play/Player.cs +++ b/osu.Game/Screens/Play/Player.cs @@ -1,4 +1,4 @@ -// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. using System; @@ -56,8 +56,6 @@ namespace osu.Game.Screens.Play [Resolved] private ScoreManager scoreManager { get; set; } - protected PausableGameplayContainer PausableGameplayContainer { get; private set; } - private RulesetInfo ruleset; private IAPIProvider api; @@ -68,7 +66,6 @@ namespace osu.Game.Screens.Play protected RulesetContainer RulesetContainer { get; private set; } protected HUDOverlay HUDOverlay { get; private set; } - private FailOverlay failOverlay; #region Storyboard @@ -127,57 +124,47 @@ namespace osu.Game.Screens.Play InternalChild = GameplayClockContainer = new GameplayClockContainer(working, AllowLeadIn, RulesetContainer.GameplayStartTime); - GameplayClockContainer.Children = new Drawable[] + GameplayClockContainer.Children = new[] { - PausableGameplayContainer = new PausableGameplayContainer + StoryboardContainer = CreateStoryboardContainer(), + new ScalingContainer(ScalingMode.Gameplay) { - Retries = RestartCount, - OnRetry = restart, - OnQuit = performUserRequestedExit, - RequestResume = completion => + Child = new LocalSkinOverrideContainer(working.Skin) { - GameplayClockContainer.Start(); - completion(); - }, - RequestPause = GameplayClockContainer.Stop, - IsPaused = { BindTarget = GameplayClockContainer.IsPaused }, - CheckCanPause = () => CanPause, - Children = new[] - { - StoryboardContainer = CreateStoryboardContainer(), - new ScalingContainer(ScalingMode.Gameplay) - { - Child = new LocalSkinOverrideContainer(working.Skin) - { - RelativeSizeAxes = Axes.Both, - Child = RulesetContainer - } - }, - new BreakOverlay(working.Beatmap.BeatmapInfo.LetterboxInBreaks, ScoreProcessor) - { - Anchor = Anchor.Centre, - Origin = Anchor.Centre, - Breaks = working.Beatmap.Breaks - }, - // display the cursor above some HUD elements. - RulesetContainer.Cursor?.CreateProxy() ?? new Container(), - HUDOverlay = new HUDOverlay(ScoreProcessor, RulesetContainer, working) - { - HoldToQuit = { Action = performUserRequestedExit }, - PlayerSettingsOverlay = { PlaybackSettings = { UserPlaybackRate = { BindTarget = GameplayClockContainer.UserPlaybackRate } } }, - KeyCounter = { Visible = { BindTarget = RulesetContainer.HasReplayLoaded } }, - RequestSeek = GameplayClockContainer.Seek, - Anchor = Anchor.Centre, - Origin = Anchor.Centre - }, - new SkipOverlay(RulesetContainer.GameplayStartTime) - { - RequestSeek = GameplayClockContainer.Seek - }, + RelativeSizeAxes = Axes.Both, + Child = RulesetContainer } }, - failOverlay = new FailOverlay + new BreakOverlay(working.Beatmap.BeatmapInfo.LetterboxInBreaks, ScoreProcessor) { + Anchor = Anchor.Centre, + Origin = Anchor.Centre, + Breaks = working.Beatmap.Breaks + }, + // display the cursor above some HUD elements. + RulesetContainer.Cursor?.CreateProxy() ?? new Container(), + HUDOverlay = new HUDOverlay(ScoreProcessor, RulesetContainer, working) + { + HoldToQuit = { Action = performUserRequestedExit }, + PlayerSettingsOverlay = { PlaybackSettings = { UserPlaybackRate = { BindTarget = GameplayClockContainer.UserPlaybackRate } } }, + KeyCounter = { Visible = { BindTarget = RulesetContainer.HasReplayLoaded } }, + RequestSeek = GameplayClockContainer.Seek, + Anchor = Anchor.Centre, + Origin = Anchor.Centre + }, + new SkipOverlay(RulesetContainer.GameplayStartTime) + { + RequestSeek = GameplayClockContainer.Seek + }, + FailOverlay = new FailOverlay + { + OnRetry = restart, + OnQuit = performUserRequestedExit, + }, + PauseOverlay = new PauseOverlay + { + OnResume = Resume, + Retries = RestartCount, OnRetry = restart, OnQuit = performUserRequestedExit, }, @@ -197,7 +184,7 @@ namespace osu.Game.Screens.Play RulesetContainer.IsPaused.BindTo(GameplayClockContainer.IsPaused); // load storyboard as part of player's load if we can - initializeStoryboard(false); + initializeStoryboard(false); // Bind ScoreProcessor to ourselves ScoreProcessor.AllJudged += onCompletion; @@ -313,6 +300,14 @@ namespace osu.Game.Screens.Play return score; } + protected override bool OnScroll(ScrollEvent e) => mouseWheelDisabled.Value && !GameplayClockContainer.IsPaused.Value; + + protected virtual Results CreateResults(ScoreInfo score) => new SoloResults(score); + + #region Fail Logic + + protected FailOverlay FailOverlay { get; private set; } + private bool onFail() { if (Beatmap.Value.Mods.Value.OfType().Any(m => !m.AllowFail)) @@ -321,11 +316,87 @@ namespace osu.Game.Screens.Play GameplayClockContainer.Stop(); HasFailed = true; - failOverlay.Retries = RestartCount; - failOverlay.Show(); + + // There is a chance that we could be in a paused state as the ruleset's internal clock (see FrameStabilityContainer) + // could process an extra frame after the GameplayClock is stopped. + // In such cases we want the fail state to precede a user triggered pause. + if (PauseOverlay.State == Visibility.Visible) + PauseOverlay.Hide(); + + FailOverlay.Retries = RestartCount; + FailOverlay.Show(); return true; } + #endregion + + #region Pause Logic + + public bool IsResuming { get; private set; } + + /// + /// The amount of gameplay time after which a second pause is allowed. + /// + private const double pause_cooldown = 1000; + + protected PauseOverlay PauseOverlay { get; private set; } + + private double? lastPauseActionTime; + + private bool canPause => + // must pass basic screen conditions (beatmap loaded, instance allows pause) + LoadedBeatmapSuccessfully && AllowPause && ValidForResume + // replays cannot be paused and exit immediately + && !RulesetContainer.HasReplayLoaded.Value + // cannot pause if we are already in a fail state + && !HasFailed + // cannot pause if already paused (and not in the process of resuming) + && (GameplayClockContainer.IsPaused.Value == false || IsResuming) + // cannot pause too soon after previous pause + && (!lastPauseActionTime.HasValue || GameplayClockContainer.GameplayClock.CurrentTime >= lastPauseActionTime + pause_cooldown); + + private bool canResume => + // cannot resume from a non-paused state + GameplayClockContainer.IsPaused.Value + // cannot resume if we are already in a fail state + && !HasFailed + // already resuming + && !IsResuming; + + protected override void Update() + { + base.Update(); + + // eagerly pause when we lose window focus (if we are locally playing). + if (!Game.IsActive.Value) + Pause(); + } + + public void Pause() + { + if (!canPause) return; + + GameplayClockContainer.Stop(); + PauseOverlay.Show(); + lastPauseActionTime = GameplayClockContainer.GameplayClock.CurrentTime; + } + + public void Resume() + { + if (!canResume) return; + + //todo: add resume request support to ruleset + IsResuming = true; + + GameplayClockContainer.Start(); + PauseOverlay.Hide(); + IsResuming = false; + } + + #endregion + + #region Screen Logic + public override void OnEntering(IScreen last) { base.OnEntering(last); @@ -350,9 +421,7 @@ namespace osu.Game.Screens.Play storyboardReplacesBackground.Value = Beatmap.Value.Storyboard.ReplacesBackground && Beatmap.Value.Storyboard.HasDrawable; GameplayClockContainer.Restart(); - - PausableGameplayContainer.Alpha = 0; - PausableGameplayContainer.FadeIn(750, Easing.OutQuint); + GameplayClockContainer.FadeInFromZero(750, Easing.OutQuint); } public override void OnSuspending(IScreen next) @@ -361,9 +430,6 @@ namespace osu.Game.Screens.Play base.OnSuspending(next); } - public bool CanPause => AllowPause && ValidForResume && !HasFailed && !RulesetContainer.HasReplayLoaded.Value - && (PausableGameplayContainer?.IsPaused.Value == false || PausableGameplayContainer?.IsResuming == true); - public override bool OnExiting(IScreen next) { if (onCompletionEvent != null) @@ -373,9 +439,9 @@ namespace osu.Game.Screens.Play return true; } - if (LoadedBeatmapSuccessfully && CanPause) + if (LoadedBeatmapSuccessfully && canPause) { - PausableGameplayContainer?.Pause(); + Pause(); return true; } @@ -394,8 +460,6 @@ namespace osu.Game.Screens.Play storyboardReplacesBackground.Value = false; } - protected override bool OnScroll(ScrollEvent e) => mouseWheelDisabled.Value && !PausableGameplayContainer.IsPaused.Value; - - protected virtual Results CreateResults(ScoreInfo score) => new SoloResults(score); + #endregion } } From 9433a977479792bc35b470ca8720e446df69c8cf Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 18 Mar 2019 14:40:53 +0900 Subject: [PATCH 05/11] Add resume requesting support and fix exit scenarios --- osu.Game/Rulesets/UI/RulesetContainer.cs | 7 ++++++ osu.Game/Screens/Play/Player.cs | 27 ++++++++++++++++++------ 2 files changed, 28 insertions(+), 6 deletions(-) diff --git a/osu.Game/Rulesets/UI/RulesetContainer.cs b/osu.Game/Rulesets/UI/RulesetContainer.cs index d8813631dc..c522118962 100644 --- a/osu.Game/Rulesets/UI/RulesetContainer.cs +++ b/osu.Game/Rulesets/UI/RulesetContainer.cs @@ -130,6 +130,13 @@ namespace osu.Game.Rulesets.UI /// The input manager. public abstract PassThroughInputManager CreateInputManager(); + /// + /// Invoked when the interactive user requests resuming from a paused state. + /// Allows potentially delaying the resume process until an interaction is performed. + /// + /// The action to run when resuming is to be completed. + public void RequestResume(Action continueResume) => continueResume(); + protected virtual ReplayInputHandler CreateReplayInputHandler(Replay replay) => null; protected FrameStabilityContainer FrameStabilityContainer; diff --git a/osu.Game/Screens/Play/Player.cs b/osu.Game/Screens/Play/Player.cs index 018ff900ee..b53ed8ae17 100644 --- a/osu.Game/Screens/Play/Player.cs +++ b/osu.Game/Screens/Play/Player.cs @@ -352,8 +352,10 @@ namespace osu.Game.Screens.Play && !HasFailed // cannot pause if already paused (and not in the process of resuming) && (GameplayClockContainer.IsPaused.Value == false || IsResuming) - // cannot pause too soon after previous pause - && (!lastPauseActionTime.HasValue || GameplayClockContainer.GameplayClock.CurrentTime >= lastPauseActionTime + pause_cooldown); + && (!pauseCooldownActive || IsResuming); + + private bool pauseCooldownActive => + lastPauseActionTime.HasValue && GameplayClockContainer.GameplayClock.CurrentTime < lastPauseActionTime + pause_cooldown; private bool canResume => // cannot resume from a non-paused state @@ -376,6 +378,7 @@ namespace osu.Game.Screens.Play { if (!canPause) return; + IsResuming = false; GameplayClockContainer.Stop(); PauseOverlay.Show(); lastPauseActionTime = GameplayClockContainer.GameplayClock.CurrentTime; @@ -385,12 +388,20 @@ namespace osu.Game.Screens.Play { if (!canResume) return; - //todo: add resume request support to ruleset IsResuming = true; - - GameplayClockContainer.Start(); PauseOverlay.Hide(); - IsResuming = false; + + // time-based conditions may allow instant resume. + if (GameplayClockContainer.GameplayClock.CurrentTime < Beatmap.Value.Beatmap.HitObjects.First().StartTime) + completeResume(); + else + RulesetContainer.RequestResume(completeResume); + + void completeResume() + { + GameplayClockContainer.Start(); + IsResuming = false; + } } #endregion @@ -445,6 +456,10 @@ namespace osu.Game.Screens.Play return true; } + if (pauseCooldownActive && !GameplayClockContainer.IsPaused.Value) + // still want to block if we are within the cooldown period and not already paused. + return true; + GameplayClockContainer.ResetLocalAdjustments(); fadeOut(); From 4f075f4740b8592a5d1c1426ca13cf1054da042b Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 18 Mar 2019 14:57:06 +0900 Subject: [PATCH 06/11] Add more comprehensive testing --- osu.Game.Tests/Visual/TestCasePause.cs | 13 ++++++++++++- osu.Game/Screens/Play/Player.cs | 8 ++++---- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/osu.Game.Tests/Visual/TestCasePause.cs b/osu.Game.Tests/Visual/TestCasePause.cs index 622a12da81..f658dbee16 100644 --- a/osu.Game.Tests/Visual/TestCasePause.cs +++ b/osu.Game.Tests/Visual/TestCasePause.cs @@ -3,6 +3,7 @@ using System; using osu.Framework.Graphics.Containers; +using osu.Framework.Screens; using osu.Game.Rulesets; using osu.Game.Rulesets.Osu; using osu.Game.Rulesets.Scoring; @@ -26,7 +27,6 @@ namespace osu.Game.Tests.Visual PausePlayer pausable() => (PausePlayer)player(); base.AddCheckSteps(player); - //AddUntilStep(() => pausable().ScoreProcessor.TotalScore.Value > 0, "score above zero"); AddStep("pause", () => pausable().Pause()); AddAssert("clock stopped", () => !pausable().GameplayClockContainer.GameplayClock.IsRunning); @@ -47,6 +47,17 @@ namespace osu.Game.Tests.Visual AddAssert("pause overlay hidden", () => !pausable().PauseOverlayVisible); AddAssert("fail overlay still shown", () => pausable().FailOverlayVisible); + + AddStep("restart", () => pausable().Restart()); + + AddUntilStep(() => + { + pausable().Pause(); + return pausable().PauseOverlayVisible; + }, "keep trying to pause"); + + AddStep("exit", () => pausable().Exit()); + AddUntilStep(() => !pausable().IsCurrentScreen(), "player exited"); } private class PausePlayer : Player diff --git a/osu.Game/Screens/Play/Player.cs b/osu.Game/Screens/Play/Player.cs index b53ed8ae17..162350e088 100644 --- a/osu.Game/Screens/Play/Player.cs +++ b/osu.Game/Screens/Play/Player.cs @@ -158,14 +158,14 @@ namespace osu.Game.Screens.Play }, FailOverlay = new FailOverlay { - OnRetry = restart, + OnRetry = Restart, OnQuit = performUserRequestedExit, }, PauseOverlay = new PauseOverlay { OnResume = Resume, Retries = RestartCount, - OnRetry = restart, + OnRetry = Restart, OnQuit = performUserRequestedExit, }, new HotkeyRetryOverlay @@ -175,7 +175,7 @@ namespace osu.Game.Screens.Play if (!this.IsCurrentScreen()) return; fadeOut(true); - restart(); + Restart(); }, } }; @@ -246,7 +246,7 @@ namespace osu.Game.Screens.Play this.Exit(); } - private void restart() + public void Restart() { if (!this.IsCurrentScreen()) return; From f56e8d9bfe26b9d7ca85111d6489f5d884985892 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 18 Mar 2019 19:44:21 +0900 Subject: [PATCH 07/11] Make tests better --- osu.Game.Tests/Visual/TestCasePause.cs | 95 +++++++++++++++----------- 1 file changed, 54 insertions(+), 41 deletions(-) diff --git a/osu.Game.Tests/Visual/TestCasePause.cs b/osu.Game.Tests/Visual/TestCasePause.cs index f658dbee16..f53177e86a 100644 --- a/osu.Game.Tests/Visual/TestCasePause.cs +++ b/osu.Game.Tests/Visual/TestCasePause.cs @@ -1,7 +1,7 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -using System; +using NUnit.Framework; using osu.Framework.Graphics.Containers; using osu.Framework.Screens; using osu.Game.Rulesets; @@ -11,56 +11,69 @@ using osu.Game.Screens.Play; namespace osu.Game.Tests.Visual { - public class TestCasePause : TestCasePlayer + public class TestCasePause : PlayerTestCase { + protected new PausePlayer Player => (PausePlayer)base.Player; + public TestCasePause() : base(new OsuRuleset()) { } + [Test] + public void TestPauseResume() + { + AddStep("pause", () => Player.Pause()); + AddAssert("clock stopped", () => !Player.GameplayClockContainer.GameplayClock.IsRunning); + AddAssert("pause overlay shown", () => Player.PauseOverlayVisible); + + AddStep("resume", () => Player.Resume()); + AddAssert("pause overlay hidden", () => !Player.PauseOverlayVisible); + } + + [Test] + public void TestPauseTooSoon() + { + AddStep("pause", () => Player.Pause()); + AddAssert("clock stopped", () => !Player.GameplayClockContainer.GameplayClock.IsRunning); + AddStep("resume", () => Player.Resume()); + AddAssert("clock started", () => Player.GameplayClockContainer.GameplayClock.IsRunning); + AddStep("pause too soon", () => Player.Pause()); + AddAssert("clock not stopped", () => Player.GameplayClockContainer.GameplayClock.IsRunning); + AddAssert("pause overlay hidden", () => !Player.PauseOverlayVisible); + } + + [Test] + public void TestPauseAfterFail() + { + AddUntilStep(() => Player.HasFailed, "wait for fail"); + + AddAssert("fail overlay shown", () => Player.FailOverlayVisible); + + AddStep("try to pause", () => Player.Pause()); + + AddAssert("pause overlay hidden", () => !Player.PauseOverlayVisible); + AddAssert("fail overlay still shown", () => Player.FailOverlayVisible); + } + + [Test] + public void TestExitFromPause() + { + AddUntilStep(() => + { + Player.Pause(); + return Player.PauseOverlayVisible; + }, "keep trying to pause"); + + AddStep("exit", () => Player.Exit()); + AddUntilStep(() => !Player.IsCurrentScreen(), "player exited"); + } + protected override bool AllowFail => true; protected override Player CreatePlayer(Ruleset ruleset) => new PausePlayer(); - protected override void AddCheckSteps(Func player) - { - PausePlayer pausable() => (PausePlayer)player(); - - base.AddCheckSteps(player); - - AddStep("pause", () => pausable().Pause()); - AddAssert("clock stopped", () => !pausable().GameplayClockContainer.GameplayClock.IsRunning); - AddAssert("pause overlay shown", () => pausable().PauseOverlayVisible); - - AddStep("resume", () => pausable().Resume()); - AddAssert("pause overlay hidden", () => !pausable().PauseOverlayVisible); - - AddStep("pause too soon", () => pausable().Pause()); - AddAssert("clock not stopped", () => pausable().GameplayClockContainer.GameplayClock.IsRunning); - AddAssert("pause overlay hidden", () => !pausable().PauseOverlayVisible); - - AddUntilStep(() => pausable().HasFailed, "wait for fail"); - - AddAssert("fail overlay shown", () => pausable().FailOverlayVisible); - - AddStep("try to pause", () => pausable().Pause()); - - AddAssert("pause overlay hidden", () => !pausable().PauseOverlayVisible); - AddAssert("fail overlay still shown", () => pausable().FailOverlayVisible); - - AddStep("restart", () => pausable().Restart()); - - AddUntilStep(() => - { - pausable().Pause(); - return pausable().PauseOverlayVisible; - }, "keep trying to pause"); - - AddStep("exit", () => pausable().Exit()); - AddUntilStep(() => !pausable().IsCurrentScreen(), "player exited"); - } - - private class PausePlayer : Player + protected class PausePlayer : Player { public new GameplayClockContainer GameplayClockContainer => base.GameplayClockContainer; From b17c5c0bb317e67dacfa35e74827f6c8591601a6 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 21 Mar 2019 16:11:44 +0900 Subject: [PATCH 08/11] Add more tests for exitability --- osu.Game.Tests/Visual/TestCasePause.cs | 31 +++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/osu.Game.Tests/Visual/TestCasePause.cs b/osu.Game.Tests/Visual/TestCasePause.cs index 834b5e8ef1..8fcf6164a4 100644 --- a/osu.Game.Tests/Visual/TestCasePause.cs +++ b/osu.Game.Tests/Visual/TestCasePause.cs @@ -43,17 +43,40 @@ namespace osu.Game.Tests.Visual AddAssert("pause overlay hidden", () => !Player.PauseOverlayVisible); } + [Test] + public void TestExitTooSoon() + { + AddStep("pause", () => Player.Pause()); + AddAssert("clock stopped", () => !Player.GameplayClockContainer.GameplayClock.IsRunning); + AddStep("resume", () => Player.Resume()); + AddAssert("clock started", () => Player.GameplayClockContainer.GameplayClock.IsRunning); + AddStep("pause too soon", () => Player.Exit()); + AddAssert("clock not stopped", () => Player.GameplayClockContainer.GameplayClock.IsRunning); + AddAssert("pause overlay hidden", () => !Player.PauseOverlayVisible); + AddAssert("not exited", () => Player.IsCurrentScreen()); + } + [Test] public void TestPauseAfterFail() { AddUntilStep("wait for fail", () => Player.HasFailed); - AddAssert("fail overlay shown", () => Player.FailOverlayVisible); AddStep("try to pause", () => Player.Pause()); AddAssert("pause overlay hidden", () => !Player.PauseOverlayVisible); AddAssert("fail overlay still shown", () => Player.FailOverlayVisible); + + confirmExit(); + } + + [Test] + public void TestExitFromGameplay() + { + AddStep("exit", () => Player.Exit()); + AddUntilStep("wait for pause", () => Player.PauseOverlayVisible); + + confirmExit(); } [Test] @@ -65,6 +88,12 @@ namespace osu.Game.Tests.Visual return Player.PauseOverlayVisible; }); + confirmExit(); + } + + private void confirmExit() + { + AddUntilStep("player not exited", () => Player.IsCurrentScreen()); AddStep("exit", () => Player.Exit()); AddUntilStep("player exited", () => !Player.IsCurrentScreen()); } From aa1dfdd6631572a21aa9c20c02a3e6a064c235a8 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 21 Mar 2019 16:33:34 +0900 Subject: [PATCH 09/11] Extract logic shared between tests --- osu.Game.Tests/Visual/TestCasePause.cs | 102 +++++++++++++++++-------- 1 file changed, 69 insertions(+), 33 deletions(-) diff --git a/osu.Game.Tests/Visual/TestCasePause.cs b/osu.Game.Tests/Visual/TestCasePause.cs index 8fcf6164a4..d5d2cebbab 100644 --- a/osu.Game.Tests/Visual/TestCasePause.cs +++ b/osu.Game.Tests/Visual/TestCasePause.cs @@ -23,36 +23,34 @@ namespace osu.Game.Tests.Visual [Test] public void TestPauseResume() { - AddStep("pause", () => Player.Pause()); - AddAssert("clock stopped", () => !Player.GameplayClockContainer.GameplayClock.IsRunning); - AddAssert("pause overlay shown", () => Player.PauseOverlayVisible); - - AddStep("resume", () => Player.Resume()); - AddAssert("pause overlay hidden", () => !Player.PauseOverlayVisible); + pauseAndConfirm(); + resumeAndConfirm(); } [Test] public void TestPauseTooSoon() { - AddStep("pause", () => Player.Pause()); - AddAssert("clock stopped", () => !Player.GameplayClockContainer.GameplayClock.IsRunning); - AddStep("resume", () => Player.Resume()); - AddAssert("clock started", () => Player.GameplayClockContainer.GameplayClock.IsRunning); - AddStep("pause too soon", () => Player.Pause()); - AddAssert("clock not stopped", () => Player.GameplayClockContainer.GameplayClock.IsRunning); - AddAssert("pause overlay hidden", () => !Player.PauseOverlayVisible); + pauseAndConfirm(); + resumeAndConfirm(); + + pause(); + + confirmClockRunning(true); + confirmPauseOverlayShown(false); } [Test] public void TestExitTooSoon() { - AddStep("pause", () => Player.Pause()); - AddAssert("clock stopped", () => !Player.GameplayClockContainer.GameplayClock.IsRunning); - AddStep("resume", () => Player.Resume()); - AddAssert("clock started", () => Player.GameplayClockContainer.GameplayClock.IsRunning); - AddStep("pause too soon", () => Player.Exit()); - AddAssert("clock not stopped", () => Player.GameplayClockContainer.GameplayClock.IsRunning); - AddAssert("pause overlay hidden", () => !Player.PauseOverlayVisible); + pauseAndConfirm(); + + resume(); + + AddStep("exit too soon", () => Player.Exit()); + + confirmClockRunning(true); + confirmPauseOverlayShown(false); + AddAssert("not exited", () => Player.IsCurrentScreen()); } @@ -62,42 +60,80 @@ namespace osu.Game.Tests.Visual AddUntilStep("wait for fail", () => Player.HasFailed); AddAssert("fail overlay shown", () => Player.FailOverlayVisible); - AddStep("try to pause", () => Player.Pause()); + confirmClockRunning(false); + + pause(); + + confirmClockRunning(false); + confirmPauseOverlayShown(false); - AddAssert("pause overlay hidden", () => !Player.PauseOverlayVisible); AddAssert("fail overlay still shown", () => Player.FailOverlayVisible); - confirmExit(); + exitAndConfirm(); } [Test] public void TestExitFromGameplay() { AddStep("exit", () => Player.Exit()); - AddUntilStep("wait for pause", () => Player.PauseOverlayVisible); - confirmExit(); + confirmPaused(); + + exitAndConfirm(); } [Test] public void TestExitFromPause() { - AddUntilStep("keep trying to pause", () => - { - Player.Pause(); - return Player.PauseOverlayVisible; - }); - - confirmExit(); + pauseAndConfirm(); + exitAndConfirm(); } - private void confirmExit() + private void pauseAndConfirm() + { + pause(); + confirmPaused(); + } + + private void resumeAndConfirm() + { + resume(); + confirmResumed(); + } + + private void exitAndConfirm() { AddUntilStep("player not exited", () => Player.IsCurrentScreen()); AddStep("exit", () => Player.Exit()); + confirmExited(); + } + + private void confirmPaused() + { + confirmClockRunning(false); + AddAssert("pause overlay shown", () => Player.PauseOverlayVisible); + } + + private void confirmResumed() + { + confirmClockRunning(true); + confirmPauseOverlayShown(false); + } + + private void confirmExited() + { AddUntilStep("player exited", () => !Player.IsCurrentScreen()); } + private void pause() => AddStep("pause", () => Player.Pause()); + private void resume() => AddStep("resume", () => Player.Resume()); + + private void confirmPauseOverlayShown(bool isShown) => + AddAssert("pause overlay " + (isShown ? "shown" : "hidden"), () => Player.PauseOverlayVisible == isShown); + + private void confirmClockRunning(bool isRunning) => + AddAssert("clock " + (isRunning ? "running" : "stopped"), () => Player.GameplayClockContainer.GameplayClock.IsRunning == isRunning); + protected override bool AllowFail => true; protected override Player CreatePlayer(Ruleset ruleset) => new PausePlayer(); From 5b8fd6822e671c6fceb08ec46f18b3c270cf9d24 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 22 Mar 2019 14:39:20 +0900 Subject: [PATCH 10/11] Move storyboard logic region down --- osu.Game/Screens/Play/Player.cs | 70 ++++++++++++++++----------------- 1 file changed, 35 insertions(+), 35 deletions(-) diff --git a/osu.Game/Screens/Play/Player.cs b/osu.Game/Screens/Play/Player.cs index 88467aa7f5..70899d42bc 100644 --- a/osu.Game/Screens/Play/Player.cs +++ b/osu.Game/Screens/Play/Player.cs @@ -67,41 +67,6 @@ namespace osu.Game.Screens.Play protected HUDOverlay HUDOverlay { get; private set; } - #region Storyboard - - private DrawableStoryboard storyboard; - protected UserDimContainer StoryboardContainer { get; private set; } - - private void initializeStoryboard(bool asyncLoad) - { - if (StoryboardContainer == null || storyboard != null) - return; - - if (!showStoryboard.Value) - return; - - var beatmap = Beatmap.Value; - - storyboard = beatmap.Storyboard.CreateDrawable(); - storyboard.Masking = true; - - if (asyncLoad) - LoadComponentAsync(storyboard, StoryboardContainer.Add); - else - StoryboardContainer.Add(storyboard); - } - - #endregion - - private Bindable showStoryboard; - - protected virtual UserDimContainer CreateStoryboardContainer() => new UserDimContainer(true) - { - RelativeSizeAxes = Axes.Both, - Alpha = 1, - EnableUserDim = { Value = true } - }; - public bool LoadedBeatmapSuccessfully => DrawableRuleset?.Objects.Any() == true; protected GameplayClockContainer GameplayClockContainer { get; private set; } @@ -307,6 +272,41 @@ namespace osu.Game.Screens.Play protected virtual Results CreateResults(ScoreInfo score) => new SoloResults(score); + #region Storyboard + + private DrawableStoryboard storyboard; + protected UserDimContainer StoryboardContainer { get; private set; } + + protected virtual UserDimContainer CreateStoryboardContainer() => new UserDimContainer(true) + { + RelativeSizeAxes = Axes.Both, + Alpha = 1, + EnableUserDim = { Value = true } + }; + + private Bindable showStoryboard; + + private void initializeStoryboard(bool asyncLoad) + { + if (StoryboardContainer == null || storyboard != null) + return; + + if (!showStoryboard.Value) + return; + + var beatmap = Beatmap.Value; + + storyboard = beatmap.Storyboard.CreateDrawable(); + storyboard.Masking = true; + + if (asyncLoad) + LoadComponentAsync(storyboard, StoryboardContainer.Add); + else + StoryboardContainer.Add(storyboard); + } + + #endregion + #region Fail Logic protected FailOverlay FailOverlay { get; private set; } From 9e6cdd7bd50d0975293e9a9613b0841fda9adea3 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 22 Mar 2019 14:42:51 +0900 Subject: [PATCH 11/11] Combine conditionals and clarify comment --- osu.Game/Screens/Play/Player.cs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/osu.Game/Screens/Play/Player.cs b/osu.Game/Screens/Play/Player.cs index 70899d42bc..7b1cdd21a6 100644 --- a/osu.Game/Screens/Play/Player.cs +++ b/osu.Game/Screens/Play/Player.cs @@ -353,9 +353,8 @@ namespace osu.Game.Screens.Play && !DrawableRuleset.HasReplayLoaded.Value // cannot pause if we are already in a fail state && !HasFailed - // cannot pause if already paused (and not in the process of resuming) - && (GameplayClockContainer.IsPaused.Value == false || IsResuming) - && (!pauseCooldownActive || IsResuming); + // cannot pause if already paused (or in a cooldown state) unless we are in a resuming state. + && (IsResuming || (GameplayClockContainer.IsPaused.Value == false && !pauseCooldownActive)); private bool pauseCooldownActive => lastPauseActionTime.HasValue && GameplayClockContainer.GameplayClock.CurrentTime < lastPauseActionTime + pause_cooldown; @@ -454,7 +453,7 @@ namespace osu.Game.Screens.Play return true; } - if (LoadedBeatmapSuccessfully && canPause) + if (canPause) { Pause(); return true;