Merge pull request #8800 from iiSaLMaN/allow-cancelling-completion

Fix results screen pushed after rewinding in-between push delay

osu.Game.Rulesets.Mania.Tests/TestSceneHoldNoteInput.cs

@@ -27,7 +27,6 @@ namespace osu.Game.Rulesets.Mania.Tests
         private const double time_after_tail = 5250;
 
         private List<JudgementResult> judgementResults;
-        private bool allJudgedFired;
 
         /// <summary>
         ///     -----[           ]-----
@@ -283,20 +282,15 @@ namespace osu.Game.Rulesets.Mania.Tests
                     {
                         if (currentPlayer == p) judgementResults.Add(result);
                     };
-                    p.ScoreProcessor.AllJudged += () =>
-                    {
-                        if (currentPlayer == p) allJudgedFired = true;
-                    };
                 };
 
                 LoadScreen(currentPlayer = p);
-                allJudgedFired = false;
                 judgementResults = new List<JudgementResult>();
             });
 
             AddUntilStep("Beatmap at 0", () => Beatmap.Value.Track.CurrentTime == 0);
             AddUntilStep("Wait until player is loaded", () => currentPlayer.IsCurrentScreen());
-            AddUntilStep("Wait for all judged", () => allJudgedFired);
+            AddUntilStep("Wait for completion", () => currentPlayer.ScoreProcessor.HasCompleted.Value);
         }
 
         private class ScoreAccessibleReplayPlayer : ReplayPlayer

osu.Game.Rulesets.Osu.Tests/TestSceneOutOfOrderHits.cs

@@ -354,7 +354,6 @@ namespace osu.Game.Rulesets.Osu.Tests
 
         private ScoreAccessibleReplayPlayer currentPlayer;
         private List<JudgementResult> judgementResults;
-        private bool allJudgedFired;
 
         private void performTest(List<OsuHitObject> hitObjects, List<ReplayFrame> frames)
         {
@@ -380,20 +379,15 @@ namespace osu.Game.Rulesets.Osu.Tests
                     {
                         if (currentPlayer == p) judgementResults.Add(result);
                     };
-                    p.ScoreProcessor.AllJudged += () =>
-                    {
-                        if (currentPlayer == p) allJudgedFired = true;
-                    };
                 };
 
                 LoadScreen(currentPlayer = p);
-                allJudgedFired = false;
                 judgementResults = new List<JudgementResult>();
             });
 
             AddUntilStep("Beatmap at 0", () => Beatmap.Value.Track.CurrentTime == 0);
             AddUntilStep("Wait until player is loaded", () => currentPlayer.IsCurrentScreen());
-            AddUntilStep("Wait for all judged", () => allJudgedFired);
+            AddUntilStep("Wait for completion", () => currentPlayer.ScoreProcessor.HasCompleted.Value);
         }
 
         private class TestHitCircle : HitCircle

osu.Game.Rulesets.Osu.Tests/TestSceneSliderInput.cs

@@ -47,7 +47,6 @@ namespace osu.Game.Rulesets.Osu.Tests
         private const double time_slider_end = 4000;
 
         private List<JudgementResult> judgementResults;
-        private bool allJudgedFired;
 
         /// <summary>
         /// Scenario:
@@ -375,20 +374,15 @@ namespace osu.Game.Rulesets.Osu.Tests
                     {
                         if (currentPlayer == p) judgementResults.Add(result);
                     };
-                    p.ScoreProcessor.AllJudged += () =>
-                    {
-                        if (currentPlayer == p) allJudgedFired = true;
-                    };
                 };
 
                 LoadScreen(currentPlayer = p);
-                allJudgedFired = false;
                 judgementResults = new List<JudgementResult>();
             });
 
             AddUntilStep("Beatmap at 0", () => Beatmap.Value.Track.CurrentTime == 0);
             AddUntilStep("Wait until player is loaded", () => currentPlayer.IsCurrentScreen());
-            AddUntilStep("Wait for all judged", () => allJudgedFired);
+            AddUntilStep("Wait for completion", () => currentPlayer.ScoreProcessor.HasCompleted.Value);
         }
 
         private class ScoreAccessibleReplayPlayer : ReplayPlayer

osu.Game.Rulesets.Taiko.Tests/TestSceneSwellJudgements.cs

@@ -19,7 +19,7 @@ namespace osu.Game.Rulesets.Taiko.Tests
         [Test]
         public void TestZeroTickTimeOffsets()
         {
-            AddUntilStep("gameplay finished", () => Player.ScoreProcessor.HasCompleted);
+            AddUntilStep("gameplay finished", () => Player.ScoreProcessor.HasCompleted.Value);
             AddAssert("all tick offsets are 0", () => Player.Results.Where(r => r.HitObject is SwellTick).All(r => r.TimeOffset == 0));
         }
 

osu.Game.Tests/Visual/Gameplay/TestSceneCompletionCancellation.cs

@@ -0,0 +1,139 @@
+// 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 NUnit.Framework;
+using osu.Framework.Allocation;
+using osu.Framework.Audio;
+using osu.Framework.Audio.Track;
+using osu.Framework.Screens;
+using osu.Framework.Testing;
+using osu.Framework.Timing;
+using osu.Game.Beatmaps;
+using osu.Game.Rulesets;
+using osu.Game.Rulesets.Osu;
+using osu.Game.Rulesets.Osu.Objects;
+using osu.Game.Storyboards;
+using osuTK;
+
+namespace osu.Game.Tests.Visual.Gameplay
+{
+    public class TestSceneCompletionCancellation : PlayerTestScene
+    {
+        private Track track;
+
+        [Resolved]
+        private AudioManager audio { get; set; }
+
+        private int resultsDisplayWaitCount =>
+            (int)((Screens.Play.Player.RESULTS_DISPLAY_DELAY / TimePerAction) * 2);
+
+        protected override bool AllowFail => false;
+
+        public TestSceneCompletionCancellation()
+            : base(new OsuRuleset())
+        {
+        }
+
+        [SetUpSteps]
+        public override void SetUpSteps()
+        {
+            base.SetUpSteps();
+
+            // Ensure track has actually running before attempting to seek
+            AddUntilStep("wait for track to start running", () => track.IsRunning);
+        }
+
+        [Test]
+        public void TestCancelCompletionOnRewind()
+        {
+            complete();
+            cancel();
+
+            checkNoRanking();
+        }
+
+        [Test]
+        public void TestReCompleteAfterCancellation()
+        {
+            complete();
+            cancel();
+            complete();
+
+            AddUntilStep("attempted to push ranking", () => ((FakeRankingPushPlayer)Player).GotoRankingInvoked);
+        }
+
+        /// <summary>
+        /// Tests whether can still pause after cancelling completion by reverting <see cref="IScreen.ValidForResume"/> back to true.
+        /// </summary>
+        [Test]
+        public void TestCanPauseAfterCancellation()
+        {
+            complete();
+            cancel();
+
+            AddStep("pause", () => Player.Pause());
+            AddAssert("paused successfully", () => Player.GameplayClockContainer.IsPaused.Value);
+
+            checkNoRanking();
+        }
+
+        private void complete()
+        {
+            AddStep("seek to completion", () => track.Seek(5000));
+            AddUntilStep("completion set by processor", () => Player.ScoreProcessor.HasCompleted.Value);
+        }
+
+        private void cancel()
+        {
+            AddStep("rewind to cancel", () => track.Seek(4000));
+            AddUntilStep("completion cleared by processor", () => !Player.ScoreProcessor.HasCompleted.Value);
+        }
+
+        private void checkNoRanking()
+        {
+            // wait to ensure there was no attempt of pushing the results screen.
+            AddWaitStep("wait", resultsDisplayWaitCount);
+            AddAssert("no attempt to push ranking", () => !((FakeRankingPushPlayer)Player).GotoRankingInvoked);
+        }
+
+        protected override WorkingBeatmap CreateWorkingBeatmap(IBeatmap beatmap, Storyboard storyboard = null)
+        {
+            var working = new ClockBackedTestWorkingBeatmap(beatmap, storyboard, new FramedClock(new ManualClock { Rate = 1 }), audio);
+            track = working.Track;
+            return working;
+        }
+
+        protected override IBeatmap CreateBeatmap(RulesetInfo ruleset)
+        {
+            var beatmap = new Beatmap();
+
+            for (int i = 1; i <= 19; i++)
+            {
+                beatmap.HitObjects.Add(new HitCircle
+                {
+                    Position = new Vector2(256, 192),
+                    StartTime = i * 250,
+                });
+            }
+
+            return beatmap;
+        }
+
+        protected override TestPlayer CreatePlayer(Ruleset ruleset) => new FakeRankingPushPlayer();
+
+        public class FakeRankingPushPlayer : TestPlayer
+        {
+            public bool GotoRankingInvoked;
+
+            public FakeRankingPushPlayer()
+                : base(true, true)
+            {
+            }
+
+            protected override void GotoRanking()
+            {
+                GotoRankingInvoked = true;
+            }
+        }
+    }
+}

osu.Game/Rulesets/Scoring/JudgementProcessor.cs

@@ -2,6 +2,7 @@
 // See the LICENCE file in the repository root for full licence text.
 
 using System;
+using osu.Framework.Bindables;
 using osu.Framework.Extensions.TypeExtensions;
 using osu.Framework.Graphics;
 using osu.Game.Beatmaps;
@@ -12,11 +13,6 @@ namespace osu.Game.Rulesets.Scoring
 {
     public abstract class JudgementProcessor : Component
     {
-        /// <summary>
-        /// Invoked when all <see cref="HitObject"/>s have been judged by this <see cref="JudgementProcessor"/>.
-        /// </summary>
-        public event Action AllJudged;
-
         /// <summary>
         /// Invoked when a new judgement has occurred. This occurs after the judgement has been processed by this <see cref="JudgementProcessor"/>.
         /// </summary>
@@ -32,10 +28,12 @@ namespace osu.Game.Rulesets.Scoring
         /// </summary>
         public int JudgedHits { get; private set; }
 
+        private readonly BindableBool hasCompleted = new BindableBool();
+
         /// <summary>
         /// Whether all <see cref="Judgement"/>s have been processed.
         /// </summary>
-        public bool HasCompleted => JudgedHits == MaxHits;
+        public IBindable<bool> HasCompleted => hasCompleted;
 
         /// <summary>
         /// Applies a <see cref="IBeatmap"/> to this <see cref="ScoreProcessor"/>.
@@ -60,8 +58,7 @@ namespace osu.Game.Rulesets.Scoring
 
             NewJudgement?.Invoke(result);
 
-            if (HasCompleted)
+            updateHasCompleted();
-                AllJudged?.Invoke();
         }
 
         /// <summary>
@@ -72,6 +69,8 @@ namespace osu.Game.Rulesets.Scoring
         {
             JudgedHits--;
 
+            updateHasCompleted();
+
             RevertResultInternal(result);
         }
 
@@ -134,5 +133,7 @@ namespace osu.Game.Rulesets.Scoring
                 ApplyResult(result);
             }
         }
+
+        private void updateHasCompleted() => hasCompleted.Value = JudgedHits == MaxHits;
     }
 }

osu.Game/Screens/Play/BreakTracker.cs

@@ -51,7 +51,7 @@ namespace osu.Game.Screens.Play
 
             isBreakTime.Value = getCurrentBreak()?.HasEffect == true
                                 || Clock.CurrentTime < gameplayStartTime
-                                || scoreProcessor?.HasCompleted == true;
+                                || scoreProcessor?.HasCompleted.Value == true;
         }
 
         private BreakPeriod getCurrentBreak()

osu.Game/Screens/Play/Player.cs

@@ -37,6 +37,11 @@ namespace osu.Game.Screens.Play
     [Cached]
     public class Player : ScreenWithBeatmapBackground
     {
+        /// <summary>
+        /// The delay upon completion of the beatmap before displaying the results screen.
+        /// </summary>
+        public const double RESULTS_DISPLAY_DELAY = 1000.0;
+
         public override bool AllowBackButton => false; // handled by HoldForMenuButton
 
         protected override UserActivity InitialActivity => new UserActivity.SoloGame(Beatmap.Value.BeatmapInfo, Ruleset.Value);
@@ -197,7 +202,7 @@ namespace osu.Game.Screens.Play
             };
 
             // Bind the judgement processors to ourselves
-            ScoreProcessor.AllJudged += onCompletion;
+            ScoreProcessor.HasCompleted.ValueChanged += updateCompletionState;
             HealthProcessor.Failed += onFail;
 
             foreach (var mod in Mods.Value.OfType<IApplicableToScoreProcessor>())
@@ -412,22 +417,33 @@ namespace osu.Game.Screens.Play
 
         private ScheduledDelegate completionProgressDelegate;
 
-        private void onCompletion()
+        private void updateCompletionState(ValueChangedEvent<bool> completionState)
         {
             // screen may be in the exiting transition phase.
             if (!this.IsCurrentScreen())
                 return;
 
+            if (!completionState.NewValue)
+            {
+                completionProgressDelegate?.Cancel();
+                completionProgressDelegate = null;
+                ValidForResume = true;
+                return;
+            }
+
+            if (completionProgressDelegate != null)
+                throw new InvalidOperationException($"{nameof(updateCompletionState)} was fired more than once");
+
             // Only show the completion screen if the player hasn't failed
-            if (HealthProcessor.HasFailed || completionProgressDelegate != null)
+            if (HealthProcessor.HasFailed)
                 return;
 
             ValidForResume = false;
 
             if (!showResults) return;
 
-            using (BeginDelayedSequence(1000))
+            using (BeginDelayedSequence(RESULTS_DISPLAY_DELAY))
-                scheduleGotoRanking();
+                completionProgressDelegate = Schedule(GotoRanking);
         }
 
         protected virtual ScoreInfo CreateScore()
@@ -679,12 +695,6 @@ namespace osu.Game.Screens.Play
             storyboardReplacesBackground.Value = false;
         }
 
-        private void scheduleGotoRanking()
-        {
-            completionProgressDelegate?.Cancel();
-            completionProgressDelegate = Schedule(GotoRanking);
-        }
-
         #endregion
     }
 }

osu.Game/Tests/Visual/ModPerfectTestScene.cs

@@ -46,7 +46,7 @@ namespace osu.Game.Tests.Visual
             public bool CheckFailed(bool failed)
             {
                 if (!failed)
-                    return ScoreProcessor.HasCompleted && !HealthProcessor.HasFailed;
+                    return ScoreProcessor.HasCompleted.Value && !HealthProcessor.HasFailed;
 
                 return HealthProcessor.HasFailed;
             }