Merge pull request #26384 from peppy/fix-multiplayer-fail-online-submit

Fix multiplayer scores being submitted as pass even if failed

osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayer.cs

@@ -29,6 +29,7 @@
 using osu.Game.Rulesets.Mods;
 using osu.Game.Rulesets.Osu;
 using osu.Game.Rulesets.Osu.Mods;
+using osu.Game.Scoring;
 using osu.Game.Screens.OnlinePlay.Components;
 using osu.Game.Screens.OnlinePlay.Lounge;
 using osu.Game.Screens.OnlinePlay.Lounge.Components;
@@ -690,6 +691,13 @@ public void TestGameplayFlow()
             }
 
             AddUntilStep("wait for results", () => multiplayerComponents.CurrentScreen is ResultsScreen);
+
+            AddAssert("check is fail", () =>
+            {
+                var scoreInfo = ((ResultsScreen)multiplayerComponents.CurrentScreen).Score;
+
+                return !scoreInfo.Passed && scoreInfo.Rank == ScoreRank.F;
+            });
         }
 
         [Test]

osu.Game/Rulesets/Mods/ModFlashlight.cs

@@ -56,9 +56,6 @@ public abstract partial class ModFlashlight<T> : ModFlashlight, IApplicableToDra
         public void ApplyToScoreProcessor(ScoreProcessor scoreProcessor)
         {
             Combo.BindTo(scoreProcessor.Combo);
-
-            // Default value of ScoreProcessor's Rank in Flashlight Mod should be SS+
-            scoreProcessor.Rank.Value = ScoreRank.XH;
         }
 
         public ScoreRank AdjustRank(ScoreRank rank, double accuracy)

osu.Game/Rulesets/Mods/ModHidden.cs

@@ -17,8 +17,6 @@ public abstract class ModHidden : ModWithVisibilityAdjustment, IApplicableToScor
 
         public void ApplyToScoreProcessor(ScoreProcessor scoreProcessor)
         {
-            // Default value of ScoreProcessor's Rank in Hidden Mod should be SS+
-            scoreProcessor.Rank.Value = ScoreRank.XH;
         }
 
         public ScoreRank AdjustRank(ScoreRank rank, double accuracy)

osu.Game/Rulesets/Scoring/ScoreProcessor.cs

@@ -86,7 +86,9 @@ public partial class ScoreProcessor : JudgementProcessor
         /// <summary>
         /// The current rank.
         /// </summary>
-        public readonly Bindable<ScoreRank> Rank = new Bindable<ScoreRank>(ScoreRank.X);
+        public IBindable<ScoreRank> Rank => rank;
+
+        private readonly Bindable<ScoreRank> rank = new Bindable<ScoreRank>(ScoreRank.X);
 
         /// <summary>
         /// The highest combo achieved by this score.
@@ -186,9 +188,13 @@ public ScoreProcessor(Ruleset ruleset)
             Combo.ValueChanged += combo => HighestCombo.Value = Math.Max(HighestCombo.Value, combo.NewValue);
             Accuracy.ValueChanged += accuracy =>
             {
-                Rank.Value = RankFromAccuracy(accuracy.NewValue);
+                // Once failed, we shouldn't update the rank anymore.
+                if (rank.Value == ScoreRank.F)
+                    return;
+
+                rank.Value = RankFromAccuracy(accuracy.NewValue);
                 foreach (var mod in Mods.Value.OfType<IApplicableToScoreProcessor>())
-                    Rank.Value = mod.AdjustRank(Rank.Value, accuracy.NewValue);
+                    rank.Value = mod.AdjustRank(Rank.Value, accuracy.NewValue);
             };
 
             Mods.ValueChanged += mods =>
@@ -411,8 +417,7 @@ protected override void Reset(bool storeResults)
             TotalScore.Value = 0;
             Accuracy.Value = 1;
             Combo.Value = 0;
-            Rank.Disabled = false;
-            Rank.Value = ScoreRank.X;
+            rank.Value = ScoreRank.X;
             HighestCombo.Value = 0;
         }
 
@@ -448,7 +453,7 @@ public void FailScore(ScoreInfo score)
                 return;
 
             score.Passed = false;
-            Rank.Value = ScoreRank.F;
+            rank.Value = ScoreRank.F;
 
             PopulateScore(score);
         }

osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerPlayer.cs

@@ -26,9 +26,6 @@ public partial class MultiplayerPlayer : RoomSubmittingPlayer
     {
         protected override bool PauseOnFocusLost => false;
 
-        // Disallow fails in multiplayer for now.
-        protected override bool CheckModsAllowFailure() => false;
-
         protected override UserActivity InitialActivity => new UserActivity.InMultiplayerGame(Beatmap.Value.BeatmapInfo, Ruleset.Value);
 
         [Resolved]
@@ -55,6 +52,7 @@ public MultiplayerPlayer(Room room, PlaylistItem playlistItem, MultiplayerRoomUs
             {
                 AllowPause = false,
                 AllowRestart = false,
+                AllowFailAnimation = false,
                 AllowSkipping = room.AutoSkip.Value,
                 AutomaticallySkipIntro = room.AutoSkip.Value,
                 AlwaysShowLeaderboard = true,

osu.Game/Screens/Play/BreakOverlay.cs

@@ -4,12 +4,14 @@
 #nullable disable
 
 using System.Collections.Generic;
+using osu.Framework.Bindables;
 using osu.Framework.Graphics;
 using osu.Framework.Graphics.Containers;
 using osu.Framework.Graphics.Shapes;
 using osu.Framework.Graphics.UserInterface;
 using osu.Game.Beatmaps.Timing;
 using osu.Game.Rulesets.Scoring;
+using osu.Game.Scoring;
 using osu.Game.Screens.Play.Break;
 
 namespace osu.Game.Screens.Play
@@ -113,7 +115,7 @@ protected override void LoadComplete()
             if (scoreProcessor != null)
             {
                 info.AccuracyDisplay.Current.BindTo(scoreProcessor.Accuracy);
-                info.GradeDisplay.Current.BindTo(scoreProcessor.Rank);
+                ((IBindable<ScoreRank>)info.GradeDisplay.Current).BindTo(scoreProcessor.Rank);
             }
         }
 

osu.Game/Screens/Play/GameplayState.cs

@@ -46,7 +46,7 @@ public class GameplayState
         public bool HasPassed { get; set; }
 
         /// <summary>
-        /// Whether the user failed during gameplay.
+        /// Whether the user failed during gameplay. This is only set when the gameplay session has completed due to the fail.
         /// </summary>
         public bool HasFailed { get; set; }
 

osu.Game/Screens/Play/Player.cs

@@ -735,7 +735,7 @@ private void checkScoreCompleted()
             }
 
             // Only show the completion screen if the player hasn't failed
-            if (HealthProcessor.HasFailed)
+            if (GameplayState.HasFailed)
                 return;
 
             GameplayState.HasPassed = true;
@@ -801,8 +801,6 @@ private void progressToResults(bool withDelay)
                     // This player instance may already be in the process of exiting.
                     return;
 
-                Debug.Assert(ScoreProcessor.Rank.Value != ScoreRank.F);
-
                 this.Push(CreateResults(prepareScoreForDisplayTask.GetResultSafely()));
             }, Time.Current + delay, 50);
 
@@ -924,37 +922,44 @@ private bool onFail()
             if (!CheckModsAllowFailure())
                 return false;
 
-            Debug.Assert(!GameplayState.HasFailed);
-            Debug.Assert(!GameplayState.HasPassed);
-            Debug.Assert(!GameplayState.HasQuit);
-
-            GameplayState.HasFailed = true;
+            if (Configuration.AllowFailAnimation)
+            {
+                Debug.Assert(!GameplayState.HasFailed);
+                Debug.Assert(!GameplayState.HasPassed);
+                Debug.Assert(!GameplayState.HasQuit);
 
-            updateGameplayState();
+                GameplayState.HasFailed = true;
 
-            // 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.Value == Visibility.Visible)
-                PauseOverlay.Hide();
+                updateGameplayState();
 
-            failAnimationContainer.Start();
+                // 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.Value == Visibility.Visible)
+                    PauseOverlay.Hide();
+
+                failAnimationContainer.Start();
+
+                // 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();
 
-            // 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(() =>
+                    if (GameplayState.Mods.OfType<IApplicableFailOverride>().Any(m => m.RestartOnFail))
+                        Restart(true);
+                });
+            }
+            else
             {
                 ScoreProcessor.FailScore(Score.ScoreInfo);
-                OnFail();
-
-                if (GameplayState.Mods.OfType<IApplicableFailOverride>().Any(m => m.RestartOnFail))
-                    Restart(true);
-            });
+            }
 
             return true;
         }

osu.Game/Screens/Play/PlayerConfiguration.cs

@@ -15,6 +15,12 @@ public class PlayerConfiguration
         /// </summary>
         public bool ShowResults { get; set; } = true;
 
+        /// <summary>
+        /// Whether the fail animation / screen should be triggered on failing.
+        /// If false, the score will still be marked as failed but gameplay will continue.
+        /// </summary>
+        public bool AllowFailAnimation { get; set; } = true;
+
         /// <summary>
         /// Whether the player should be allowed to trigger a restart.
         /// </summary>