Remove PausableGameplayContainer

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.

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<Type> RequiredTypes => new[] { typeof(FailOverlay), typeof(PausableGameplayContainer) };
+        public override IReadOnlyList<Type> 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"),

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> 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());
+            AddStep("resume", () => pausable().Resume());
-            AddUntilStep(() => pausable().GameplayClockContainer.GameplayClock.IsRunning, "clock started");
+            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;
         }
     }
 }

osu.Game/Screens/Play/PausableGameplayContainer.cs

@@ -1,133 +0,0 @@
-// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. 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
-{
-    /// <summary>
-    /// A container which handles pausing children, displaying an overlay blocking its children during paused state.
-    /// </summary>
-    public class PausableGameplayContainer : Container
-    {
-        public readonly BindableBool IsPaused = new BindableBool();
-
-        public Func<bool> CheckCanPause;
-
-        private const double pause_cooldown = 1000;
-        private double lastPauseActionTime;
-
-        private readonly PauseOverlay pauseOverlay;
-
-        private readonly Container content;
-
-        protected override Container<Drawable> 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<Action> RequestResume;
-
-        /// <summary>
-        /// Creates a new <see cref="PausableGameplayContainer"/>.
-        /// </summary>
-        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());
-            }
-        }
-    }
-}

osu.Game/Screens/Play/PauseOverlay.cs

@@ -0,0 +1,29 @@
+// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. 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());
+        }
+    }
+}

osu.Game/Screens/Play/Player.cs

@@ -1,4 +1,4 @@
-// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
+// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. 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,
+                    Child = new LocalSkinOverrideContainer(working.Skin)
-                    OnRetry = restart,
-                    OnQuit = performUserRequestedExit,
-                    RequestResume = completion =>
                     {
-                        GameplayClockContainer.Start();
+                        RelativeSizeAxes = Axes.Both,
-                        completion();
+                        Child = RulesetContainer
-                    },
-                    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
-                        },
                     }
                 },
-                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<IApplicableFailOverride>().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; }
+
+        /// <summary>
+        /// The amount of gameplay time after which a second pause is allowed.
+        /// </summary>
+        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();
-
+            GameplayClockContainer.FadeInFromZero(750, Easing.OutQuint);
-            PausableGameplayContainer.Alpha = 0;
-            PausableGameplayContainer.FadeIn(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;
+        #endregion
-
-        protected virtual Results CreateResults(ScoreInfo score) => new SoloResults(score);
     }
 }