From 727335dcadfb4ae2d4c25b5ca41e9f56a346ef15 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Fri, 24 Dec 2021 14:23:09 +0900 Subject: [PATCH 1/3] Improve reliability of exiting gameplay --- .../OnlinePlay/Multiplayer/Multiplayer.cs | 54 +++++++++++++------ .../Multiplayer/MultiplayerMatchSubScreen.cs | 3 +- .../Multiplayer/MultiplayerPlayerLoader.cs | 27 ++++++++++ osu.Game/Screens/Play/Player.cs | 7 +++ 4 files changed, 74 insertions(+), 17 deletions(-) create mode 100644 osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerPlayerLoader.cs diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/Multiplayer.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/Multiplayer.cs index e136627d43..2663f4f05f 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/Multiplayer.cs +++ b/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.GameplayCompleted) + { + 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) - { - case MultiplayerUserState.Spectating: - break; - - case MultiplayerUserState.WaitingForLoad: - case MultiplayerUserState.Loaded: - case MultiplayerUserState.Playing: - client.AbortGameplay(); - break; - - default: - client.ChangeState(MultiplayerUserState.Idle); - break; - } + if (client.LocalUser.State == MultiplayerUserState.Results) + client.ChangeState(MultiplayerUserState.Idle); } protected override string ScreenTitle => "Multiplayer"; diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerMatchSubScreen.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerMatchSubScreen.cs index af83543b16..c4dd200614 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerMatchSubScreen.cs +++ b/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)); } } diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerPlayerLoader.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerPlayerLoader.cs new file mode 100644 index 0000000000..21459ae5fb --- /dev/null +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerPlayerLoader.cs @@ -0,0 +1,27 @@ +// Copyright (c) ppy Pty Ltd . 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 GameplayCompleted => player?.GameplayCompleted == true; + + private Player player; + + public MultiplayerPlayerLoader(Func createPlayer) + : base(createPlayer) + { + } + + public override void OnSuspending(IScreen next) + { + base.OnSuspending(next); + player = (Player)next; + } + } +} diff --git a/osu.Game/Screens/Play/Player.cs b/osu.Game/Screens/Play/Player.cs index 745e1f9e7c..34db9d39b5 100644 --- a/osu.Game/Screens/Play/Player.cs +++ b/osu.Game/Screens/Play/Player.cs @@ -66,6 +66,11 @@ namespace osu.Game.Screens.Play /// protected virtual bool PauseOnFocusLost => true; + /// + /// Whether gameplay has completed without the user having failed. + /// + public bool GameplayCompleted { get; private set; } + public Action RestartRequested; public bool HasFailed { get; private set; } @@ -675,6 +680,8 @@ namespace osu.Game.Screens.Play if (HealthProcessor.HasFailed) return; + GameplayCompleted = 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; From c6854b37c896fd57f91e7d86f73524a093407be5 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Fri, 24 Dec 2021 21:57:29 +0900 Subject: [PATCH 2/3] Unbind event on disposal --- osu.Game/Screens/OnlinePlay/Multiplayer/Multiplayer.cs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/Multiplayer.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/Multiplayer.cs index 2663f4f05f..609880a725 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/Multiplayer.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/Multiplayer.cs @@ -70,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; + } } } From a43cc20ae21cd9c2e3ff76ef9e70a1fdc094c9ef Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Fri, 24 Dec 2021 21:58:20 +0900 Subject: [PATCH 3/3] Apply changes to GameplayCompleted from reviews --- osu.Game/Screens/OnlinePlay/Multiplayer/Multiplayer.cs | 2 +- .../OnlinePlay/Multiplayer/MultiplayerPlayerLoader.cs | 2 +- osu.Game/Screens/Play/Player.cs | 5 +++-- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/Multiplayer.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/Multiplayer.cs index 609880a725..28c9bef3f0 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/Multiplayer.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/Multiplayer.cs @@ -46,7 +46,7 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer return; // If gameplay wasn't finished, then we have a simple path back to the idle state by aborting gameplay. - if (!playerLoader.GameplayCompleted) + if (!playerLoader.GameplayPassed) { client.AbortGameplay(); return; diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerPlayerLoader.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerPlayerLoader.cs index 21459ae5fb..470ba59a76 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerPlayerLoader.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerPlayerLoader.cs @@ -9,7 +9,7 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer { public class MultiplayerPlayerLoader : PlayerLoader { - public bool GameplayCompleted => player?.GameplayCompleted == true; + public bool GameplayPassed => player?.GameplayPassed == true; private Player player; diff --git a/osu.Game/Screens/Play/Player.cs b/osu.Game/Screens/Play/Player.cs index 34db9d39b5..77a6b27114 100644 --- a/osu.Game/Screens/Play/Player.cs +++ b/osu.Game/Screens/Play/Player.cs @@ -69,7 +69,7 @@ namespace osu.Game.Screens.Play /// /// Whether gameplay has completed without the user having failed. /// - public bool GameplayCompleted { get; private set; } + public bool GameplayPassed { get; private set; } public Action RestartRequested; @@ -671,6 +671,7 @@ namespace osu.Game.Screens.Play resultsDisplayDelegate?.Cancel(); resultsDisplayDelegate = null; + GameplayPassed = false; ValidForResume = true; skipOutroOverlay.Hide(); return; @@ -680,7 +681,7 @@ namespace osu.Game.Screens.Play if (HealthProcessor.HasFailed) return; - GameplayCompleted = true; + 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.