Merge pull request #25566 from peppy/show-spectator-fail-2

Fix spectator not immediately showing when a spectated user fails

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

@@ -179,7 +179,6 @@ public void TestSubmissionOnFail()
             addFakeHit();
 
             AddUntilStep("wait for fail", () => Player.GameplayState.HasFailed);
-            AddStep("exit", () => Player.Exit());
 
             AddUntilStep("wait for submission", () => Player.SubmittedScore != null);
             AddAssert("ensure failing submission", () => Player.SubmittedScore.ScoreInfo.Passed == false);

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

@@ -81,7 +81,7 @@ public void TestSeekToGameplayStartFramesArriveAfterPlayerLoad()
 
             start();
 
-            waitForPlayer();
+            waitForPlayerCurrent();
 
             sendFrames(startTime: gameplay_start);
 
@@ -115,7 +115,7 @@ public void TestSeekToGameplayStartFramesArriveAsPlayerLoaded()
                 return true;
             });
 
-            waitForPlayer();
+            waitForPlayerCurrent();
 
             AddUntilStep("state is playing", () => spectatorClient.WatchedUserStates[streamingUser.Id].State == SpectatedUserState.Playing);
             AddAssert("time is greater than seek target", () => currentFrameStableTime, () => Is.GreaterThan(gameplay_start));
@@ -129,7 +129,7 @@ public void TestFrameStarvationAndResume()
             AddAssert("screen hasn't changed", () => Stack.CurrentScreen is SoloSpectatorScreen);
 
             start();
-            waitForPlayer();
+            waitForPlayerCurrent();
 
             sendFrames();
             AddAssert("ensure frames arrived", () => replayHandler.HasFrames);
@@ -155,7 +155,7 @@ public void TestPlayStartsWithNoFrames()
             loadSpectatingScreen();
 
             start();
-            waitForPlayer();
+            waitForPlayerCurrent();
             checkPaused(true);
 
             // send enough frames to ensure play won't be paused
@@ -171,7 +171,7 @@ public void TestSpectatingDuringGameplay()
             sendFrames(300);
 
             loadSpectatingScreen();
-            waitForPlayer();
+            waitForPlayerCurrent();
 
             sendFrames(300);
 
@@ -186,15 +186,15 @@ public void TestHostRetriesWhileWatching()
             start();
             sendFrames();
 
-            waitForPlayer();
+            waitForPlayerCurrent();
 
             Player lastPlayer = null;
             AddStep("store first player", () => lastPlayer = player);
 
             start();
             sendFrames();
 
-            waitForPlayer();
+            waitForPlayerCurrent();
             AddAssert("player is different", () => lastPlayer != player);
         }
 
@@ -205,7 +205,7 @@ public void TestHostFails()
 
             start();
 
-            waitForPlayer();
+            waitForPlayerCurrent();
             checkPaused(true);
             sendFrames();
 
@@ -223,7 +223,7 @@ public void TestStopWatchingDuringPlay()
 
             start();
             sendFrames();
-            waitForPlayer();
+            waitForPlayerCurrent();
 
             AddStep("stop spectating", () => (Stack.CurrentScreen as Player)?.Exit());
             AddUntilStep("spectating stopped", () => spectatorScreen.GetChildScreen() == null);
@@ -236,14 +236,14 @@ public void TestStopWatchingThenHostRetries()
 
             start();
             sendFrames();
-            waitForPlayer();
+            waitForPlayerCurrent();
 
             AddStep("stop spectating", () => (Stack.CurrentScreen as Player)?.Exit());
             AddUntilStep("spectating stopped", () => spectatorScreen.GetChildScreen() == null);
 
             // host starts playing a new session
             start();
-            waitForPlayer();
+            waitForPlayerCurrent();
         }
 
         [Test]
@@ -298,7 +298,7 @@ public void TestPlayingState()
 
             start();
             sendFrames();
-            waitForPlayer();
+            waitForPlayerCurrent();
             AddUntilStep("state is playing", () => spectatorClient.WatchedUserStates[streamingUser.Id].State == SpectatedUserState.Playing);
         }
 
@@ -309,14 +309,14 @@ public void TestPassedState()
 
             start();
             sendFrames();
-            waitForPlayer();
+            waitForPlayerCurrent();
 
             AddStep("send passed", () => spectatorClient.SendEndPlay(streamingUser.Id, SpectatedUserState.Passed));
             AddUntilStep("state is passed", () => spectatorClient.WatchedUserStates[streamingUser.Id].State == SpectatedUserState.Passed);
 
             start();
             sendFrames();
-            waitForPlayer();
+            waitForPlayerCurrent();
             AddUntilStep("state is playing", () => spectatorClient.WatchedUserStates[streamingUser.Id].State == SpectatedUserState.Playing);
         }
 
@@ -327,7 +327,7 @@ public void TestQuitState()
 
             start();
             sendFrames();
-            waitForPlayer();
+            waitForPlayerCurrent();
 
             AddStep("send quit", () => spectatorClient.SendEndPlay(streamingUser.Id));
             AddUntilStep("state is quit", () => spectatorClient.WatchedUserStates[streamingUser.Id].State == SpectatedUserState.Quit);
@@ -336,25 +336,49 @@ public void TestQuitState()
 
             start();
             sendFrames();
-            waitForPlayer();
+            waitForPlayerCurrent();
             AddUntilStep("state is playing", () => spectatorClient.WatchedUserStates[streamingUser.Id].State == SpectatedUserState.Playing);
         }
 
         [Test]
-        public void TestFailedState()
+        public void TestFailedStateDuringPlay()
         {
             loadSpectatingScreen();
 
             start();
             sendFrames();
-            waitForPlayer();
+
+            waitForPlayerCurrent();
 
             AddStep("send failed", () => spectatorClient.SendEndPlay(streamingUser.Id, SpectatedUserState.Failed));
             AddUntilStep("state is failed", () => spectatorClient.WatchedUserStates[streamingUser.Id].State == SpectatedUserState.Failed);
 
+            AddUntilStep("wait for player to fail", () => player.GameplayState.HasFailed);
+
             start();
             sendFrames();
-            waitForPlayer();
+            waitForPlayerCurrent();
+            AddUntilStep("state is playing", () => spectatorClient.WatchedUserStates[streamingUser.Id].State == SpectatedUserState.Playing);
+        }
+
+        [Test]
+        public void TestFailedStateDuringLoading()
+        {
+            loadSpectatingScreen();
+
+            start();
+            sendFrames();
+
+            waitForPlayerLoader();
+
+            AddStep("send failed", () => spectatorClient.SendEndPlay(streamingUser.Id, SpectatedUserState.Failed));
+            AddUntilStep("state is failed", () => spectatorClient.WatchedUserStates[streamingUser.Id].State == SpectatedUserState.Failed);
+
+            AddAssert("wait for player exit", () => Stack.CurrentScreen is SoloSpectatorScreen);
+
+            start();
+            sendFrames();
+            waitForPlayerCurrent();
             AddUntilStep("state is playing", () => spectatorClient.WatchedUserStates[streamingUser.Id].State == SpectatedUserState.Playing);
         }
 
@@ -366,7 +390,9 @@ public void TestFailedState()
         private double currentFrameStableTime
             => player.ChildrenOfType<FrameStabilityContainer>().First().CurrentTime;
 
-        private void waitForPlayer() => AddUntilStep("wait for player", () => this.ChildrenOfType<Player>().SingleOrDefault()?.IsLoaded == true);
+        private void waitForPlayerLoader() => AddUntilStep("wait for loading", () => this.ChildrenOfType<SpectatorPlayerLoader>().SingleOrDefault()?.IsLoaded == true);
+
+        private void waitForPlayerCurrent() => AddUntilStep("wait for player current", () => this.ChildrenOfType<Player>().SingleOrDefault()?.IsCurrentScreen() == true);
 
         private void start(int? beatmapId = null) => AddStep("start play", () => spectatorClient.SendStartPlay(streamingUser.Id, beatmapId ?? importedBeatmapId));
 

osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/MultiSpectatorScreen.cs

@@ -244,6 +244,11 @@ protected override void StartGameplay(int userId, SpectatorGameplayState spectat
             playerArea.LoadScore(spectatorGameplayState.Score);
         });
 
+        protected override void FailGameplay(int userId)
+        {
+            // We probably want to visualise this in the future.
+        }
+
         protected override void QuitGameplay(int userId) => Schedule(() =>
         {
             RemoveUser(userId);

osu.Game/Screens/Play/FailOverlay.cs

@@ -26,11 +26,22 @@ public partial class FailOverlay : GameplayMenuOverlay
 
         public override LocalisableString Header => GameplayMenuOverlayStrings.FailedHeader;
 
+        private readonly bool showButtons;
+
+        public FailOverlay(bool showButtons = true)
+        {
+            this.showButtons = showButtons;
+        }
+
         [BackgroundDependencyLoader]
         private void load(OsuColour colours)
         {
-            AddButton(GameplayMenuOverlayStrings.Retry, colours.YellowDark, () => OnRetry?.Invoke());
-            AddButton(GameplayMenuOverlayStrings.Quit, new Color4(170, 27, 39, 255), () => OnQuit?.Invoke());
+            if (showButtons)
+            {
+                AddButton(GameplayMenuOverlayStrings.Retry, colours.YellowDark, () => OnRetry?.Invoke());
+                AddButton(GameplayMenuOverlayStrings.Quit, new Color4(170, 27, 39, 255), () => OnQuit?.Invoke());
+            }
+
             // from #10339 maybe this is a better visual effect
             Add(new Container
             {

osu.Game/Screens/Play/Player.cs

@@ -267,7 +267,7 @@ private void load(OsuConfigManager config, OsuGameBase game, CancellationToken c
                         createGameplayComponents(Beatmap.Value)
                     }
                 },
-                FailOverlay = new FailOverlay
+                FailOverlay = new FailOverlay(Configuration.AllowUserInteraction)
                 {
                     SaveReplay = async () => await prepareAndImportScoreAsync(true).ConfigureAwait(false),
                     OnRetry = () => Restart(),
@@ -894,6 +894,13 @@ private void updateLeaderboardExpandedState() =>
 
         #region Fail Logic
 
+        /// <summary>
+        /// Invoked when gameplay has permanently failed.
+        /// </summary>
+        protected virtual void OnFail()
+        {
+        }
+
         protected FailOverlay FailOverlay { get; private set; }
 
         private FailAnimationContainer failAnimationContainer;
@@ -923,8 +930,21 @@ private bool onFail()
 
             failAnimationContainer.Start();
 
-            if (GameplayState.Mods.OfType<IApplicableFailOverride>().Any(m => m.RestartOnFail))
-                Restart(true);
+            // Failures can be triggered either by a judgement, or by a mod.
+            //
+            // For the case of a judgement, due to ordering considerations, ScoreProcessor will not have received
+            // the final judgement which triggered the failure yet (see DrawableRuleset.NewResult handling above).
+            //
+            // A schedule here ensures that any lingering judgements from the current frame are applied before we
+            // finalise the score as "failed".
+            Schedule(() =>
+            {
+                ScoreProcessor.FailScore(Score.ScoreInfo);
+                OnFail();
+
+                if (GameplayState.Mods.OfType<IApplicableFailOverride>().Any(m => m.RestartOnFail))
+                    Restart(true);
+            });
 
             return true;
         }
@@ -934,11 +954,6 @@ private bool onFail()
         /// </summary>
         private void onFailComplete()
         {
-            // fail completion is a good point to mark a score as failed,
-            // since the last judgement that caused the fail only applies to score processor after onFail.
-            // todo: this should probably be handled better.
-            ScoreProcessor.FailScore(Score.ScoreInfo);
-
             GameplayClockContainer.Stop();
 
             FailOverlay.Retries = RestartCount;

osu.Game/Screens/Play/SoloSpectatorPlayer.cs

@@ -17,8 +17,8 @@ public partial class SoloSpectatorPlayer : SpectatorPlayer
 
         protected override UserActivity InitialActivity => new UserActivity.SpectatingUser(Score.ScoreInfo);
 
-        public SoloSpectatorPlayer(Score score, PlayerConfiguration configuration = null)
-            : base(score, configuration)
+        public SoloSpectatorPlayer(Score score)
+            : base(score, new PlayerConfiguration { AllowUserInteraction = false })
         {
             this.score = score;
         }

osu.Game/Screens/Play/SoloSpectatorScreen.cs

@@ -178,21 +178,34 @@ protected override void StartGameplay(int userId, SpectatorGameplayState spectat
             scheduleStart(spectatorGameplayState);
         });
 
+        protected override void FailGameplay(int userId)
+        {
+            if (this.GetChildScreen() is SpectatorPlayerLoader loader)
+            {
+                if (loader.GetChildScreen() is SpectatorPlayer player)
+                {
+                    player.AllowFail();
+                    resetStartState();
+                }
+                else
+                    QuitGameplay(userId);
+            }
+        }
+
         protected override void QuitGameplay(int userId)
         {
             // Importantly, don't schedule this call, as a child screen may be present (and will cause the schedule to not be run as expected).
             this.MakeCurrent();
-
-            Schedule(() =>
-            {
-                scheduledStart?.Cancel();
-                immediateSpectatorGameplayState = null;
-                watchButton.Enabled.Value = false;
-
-                clearDisplay();
-            });
+            resetStartState();
         }
 
+        private void resetStartState() => Schedule(() =>
+        {
+            scheduledStart?.Cancel();
+            immediateSpectatorGameplayState = null;
+            clearDisplay();
+        });
+
         private void clearDisplay()
         {
             watchButton.Enabled.Value = false;

osu.Game/Screens/Play/SpectatorPlayer.cs

@@ -25,7 +25,15 @@ public abstract partial class SpectatorPlayer : Player
 
         private readonly Score score;
 
-        protected override bool CheckModsAllowFailure() => false; // todo: better support starting mid-way through beatmap
+        protected override bool CheckModsAllowFailure()
+        {
+            if (!allowFail)
+                return false;
+
+            return base.CheckModsAllowFailure();
+        }
+
+        private bool allowFail;
 
         protected SpectatorPlayer(Score score, PlayerConfiguration configuration = null)
             : base(configuration)
@@ -60,6 +68,12 @@ protected override void LoadComplete()
             }, true);
         }
 
+        /// <summary>
+        /// Should be called when it is apparent that the player being spectated has failed.
+        /// This will subsequently stop blocking the fail screen from displaying (usually done out of safety).
+        /// </summary>
+        public void AllowFail() => allowFail = true;
+
         protected override void StartGameplay()
         {
             base.StartGameplay();