Merge pull request #16237 from smoogipoo/fix-finished-play-exception-2

Improve reliability of exiting gameplay in multiplayer

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

@@ -15,6 +15,26 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer
         [Resolved]
         private MultiplayerClient client { get; set; }
 
+        protected override void LoadComplete()
+        {
+            base.LoadComplete();
+
+            client.RoomUpdated += onRoomUpdated;
+            onRoomUpdated();
+        }
+
+        private void onRoomUpdated()
+        {
+            if (client.Room == null)
+                return;
+
+            Debug.Assert(client.LocalUser != null);
+
+            // If the user exits gameplay before score submission completes, we'll transition to idle when results has been prepared.
+            if (client.LocalUser.State == MultiplayerUserState.Results && this.IsCurrentScreen())
+                transitionFromResults();
+        }
+
         public override void OnResuming(IScreen last)
         {
             base.OnResuming(last);
@@ -22,23 +42,27 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer
             if (client.Room == null)
                 return;
 
+            if (!(last is MultiplayerPlayerLoader playerLoader))
+                return;
+
+            // If gameplay wasn't finished, then we have a simple path back to the idle state by aborting gameplay.
+            if (!playerLoader.GameplayPassed)
+            {
+                client.AbortGameplay();
+                return;
+            }
+
+            // If gameplay was completed and the user went all the way to results, we'll transition to idle here.
+            // Otherwise, the transition will happen in onRoomUpdated().
+            transitionFromResults();
+        }
+
+        private void transitionFromResults()
+        {
             Debug.Assert(client.LocalUser != null);
 
-            switch (client.LocalUser.State)
+            if (client.LocalUser.State == MultiplayerUserState.Results)
-            {
+                client.ChangeState(MultiplayerUserState.Idle);
-                case MultiplayerUserState.Spectating:
-                    break;
-
-                case MultiplayerUserState.WaitingForLoad:
-                case MultiplayerUserState.Loaded:
-                case MultiplayerUserState.Playing:
-                    client.AbortGameplay();
-                    break;
-
-                default:
-                    client.ChangeState(MultiplayerUserState.Idle);
-                    break;
-            }
         }
 
         protected override string ScreenTitle => "Multiplayer";
@@ -46,5 +70,13 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer
         protected override RoomManager CreateRoomManager() => new MultiplayerRoomManager();
 
         protected override LoungeSubScreen CreateLounge() => new MultiplayerLoungeSubScreen();
+
+        protected override void Dispose(bool isDisposing)
+        {
+            base.Dispose(isDisposing);
+
+            if (client != null)
+                client.RoomUpdated -= onRoomUpdated;
+        }
     }
 }

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

@@ -30,7 +30,6 @@ using osu.Game.Screens.OnlinePlay.Multiplayer.Match;
 using osu.Game.Screens.OnlinePlay.Multiplayer.Match.Playlist;
 using osu.Game.Screens.OnlinePlay.Multiplayer.Participants;
 using osu.Game.Screens.OnlinePlay.Multiplayer.Spectate;
-using osu.Game.Screens.Play;
 using osu.Game.Screens.Play.HUD;
 using osu.Game.Users;
 using osuTK;
@@ -478,7 +477,7 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer
                     return new MultiSpectatorScreen(users.Take(PlayerGrid.MAX_PLAYERS).ToArray());
 
                 default:
-                    return new PlayerLoader(() => new MultiplayerPlayer(Room, SelectedItem.Value, users));
+                    return new MultiplayerPlayerLoader(() => new MultiplayerPlayer(Room, SelectedItem.Value, users));
             }
         }
 

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

@@ -0,0 +1,27 @@
+// 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 osu.Framework.Screens;
+using osu.Game.Screens.Play;
+
+namespace osu.Game.Screens.OnlinePlay.Multiplayer
+{
+    public class MultiplayerPlayerLoader : PlayerLoader
+    {
+        public bool GameplayPassed => player?.GameplayPassed == true;
+
+        private Player player;
+
+        public MultiplayerPlayerLoader(Func<Player> createPlayer)
+            : base(createPlayer)
+        {
+        }
+
+        public override void OnSuspending(IScreen next)
+        {
+            base.OnSuspending(next);
+            player = (Player)next;
+        }
+    }
+}

osu.Game/Screens/Play/Player.cs

@@ -66,6 +66,11 @@ namespace osu.Game.Screens.Play
         /// </summary>
         protected virtual bool PauseOnFocusLost => true;
 
+        /// <summary>
+        /// Whether gameplay has completed without the user having failed.
+        /// </summary>
+        public bool GameplayPassed { get; private set; }
+
         public Action RestartRequested;
 
         public bool HasFailed { get; private set; }
@@ -666,6 +671,7 @@ namespace osu.Game.Screens.Play
                 resultsDisplayDelegate?.Cancel();
                 resultsDisplayDelegate = null;
 
+                GameplayPassed = false;
                 ValidForResume = true;
                 skipOutroOverlay.Hide();
                 return;
@@ -675,6 +681,8 @@ namespace osu.Game.Screens.Play
             if (HealthProcessor.HasFailed)
                 return;
 
+            GameplayPassed = true;
+
             // Setting this early in the process means that even if something were to go wrong in the order of events following, there
             // is no chance that a user could return to the (already completed) Player instance from a child screen.
             ValidForResume = false;