From 18aace177a7fdcd9a6756229326b102f669c7d53 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 19 Jul 2023 19:36:51 +0900 Subject: [PATCH 1/3] Fix deadlock when logging out while at the create match screen Closes https://github.com/ppy/osu/issues/24275. --- osu.Game/Screens/OnlinePlay/Match/RoomSubScreen.cs | 9 +++++++++ .../Multiplayer/MultiplayerMatchSubScreen.cs | 14 ++++---------- osu.Game/Screens/OnlinePlay/OnlinePlaySubScreen.cs | 6 ++---- 3 files changed, 15 insertions(+), 14 deletions(-) diff --git a/osu.Game/Screens/OnlinePlay/Match/RoomSubScreen.cs b/osu.Game/Screens/OnlinePlay/Match/RoomSubScreen.cs index 6b68024393..6e126a928a 100644 --- a/osu.Game/Screens/OnlinePlay/Match/RoomSubScreen.cs +++ b/osu.Game/Screens/OnlinePlay/Match/RoomSubScreen.cs @@ -23,6 +23,7 @@ using osu.Game.Audio; using osu.Game.Beatmaps; using osu.Game.Input.Bindings; +using osu.Game.Online.API; using osu.Game.Online.Rooms; using osu.Game.Overlays; using osu.Game.Overlays.Mods; @@ -76,6 +77,9 @@ public abstract partial class RoomSubScreen : OnlinePlaySubScreen, IPreviewTrack [Resolved] private RulesetStore rulesets { get; set; } + [Resolved] + private IAPIProvider api { get; set; } = null!; + [Resolved(canBeNull: true)] protected OnlinePlayScreen ParentScreen { get; private set; } @@ -284,6 +288,8 @@ protected override IReadOnlyDependencyContainer CreateChildDependencies(IReadOnl [Resolved(canBeNull: true)] private IDialogOverlay dialogOverlay { get; set; } + protected virtual bool IsConnected => api.State.Value == APIState.Online; + public override bool OnBackButton() { if (Room.RoomID.Value == null) @@ -356,6 +362,9 @@ private bool ensureExitConfirmed() if (ExitConfirmed) return true; + if (!IsConnected) + return true; + if (dialogOverlay == null || Room.RoomID.Value != null || Room.Playlist.Count == 0) return true; diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerMatchSubScreen.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerMatchSubScreen.cs index 01f04b44c9..f5746ca96c 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerMatchSubScreen.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerMatchSubScreen.cs @@ -17,7 +17,6 @@ using osu.Game.Configuration; using osu.Game.Graphics.Cursor; using osu.Game.Online; -using osu.Game.Online.API; using osu.Game.Online.Multiplayer; using osu.Game.Online.Rooms; using osu.Game.Overlays; @@ -50,9 +49,6 @@ public partial class MultiplayerMatchSubScreen : RoomSubScreen, IHandlePresentBe [Resolved] private MultiplayerClient client { get; set; } - [Resolved] - private IAPIProvider api { get; set; } - [Resolved(canBeNull: true)] private OsuGame game { get; set; } @@ -79,6 +75,8 @@ protected override void LoadComplete() handleRoomLost(); } + protected override bool IsConnected => base.IsConnected && client.IsConnected.Value; + protected override Drawable CreateMainContent() => new Container { RelativeSizeAxes = Axes.Both, @@ -254,13 +252,9 @@ protected override void UpdateMods() public override bool OnExiting(ScreenExitEvent e) { - // the room may not be left immediately after a disconnection due to async flow, - // so checking the MultiplayerClient / IAPIAccess statuses is also required. - if (client.Room == null || !client.IsConnected.Value || api.State.Value != APIState.Online) - { - // room has not been created yet or we're offline; exit immediately. + // room has not been created yet or we're offline; exit immediately. + if (client.Room == null || !IsConnected) return base.OnExiting(e); - } if (!exitConfirmed && dialogOverlay != null) { diff --git a/osu.Game/Screens/OnlinePlay/OnlinePlaySubScreen.cs b/osu.Game/Screens/OnlinePlay/OnlinePlaySubScreen.cs index c7b32131cf..b527bf98a2 100644 --- a/osu.Game/Screens/OnlinePlay/OnlinePlaySubScreen.cs +++ b/osu.Game/Screens/OnlinePlay/OnlinePlaySubScreen.cs @@ -1,8 +1,6 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -#nullable disable - using osu.Framework.Allocation; using osu.Framework.Graphics; using osu.Framework.Screens; @@ -15,8 +13,8 @@ public abstract partial class OnlinePlaySubScreen : OsuScreen, IOnlinePlaySubScr public virtual string ShortTitle => Title; - [Resolved(CanBeNull = true)] - protected IRoomManager RoomManager { get; private set; } + [Resolved] + protected IRoomManager? RoomManager { get; private set; } protected OnlinePlaySubScreen() { From e47722565ae2c53ee38f156a027aa197e54e3c7c Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 19 Jul 2023 19:39:10 +0900 Subject: [PATCH 2/3] Clarify guard condition in `RoomSubScreen` --- osu.Game/Screens/OnlinePlay/Match/RoomSubScreen.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/osu.Game/Screens/OnlinePlay/Match/RoomSubScreen.cs b/osu.Game/Screens/OnlinePlay/Match/RoomSubScreen.cs index 6e126a928a..75b673cf1b 100644 --- a/osu.Game/Screens/OnlinePlay/Match/RoomSubScreen.cs +++ b/osu.Game/Screens/OnlinePlay/Match/RoomSubScreen.cs @@ -365,7 +365,9 @@ private bool ensureExitConfirmed() if (!IsConnected) return true; - if (dialogOverlay == null || Room.RoomID.Value != null || Room.Playlist.Count == 0) + bool hasUnsavedChanges = Room.RoomID.Value == null && Room.Playlist.Count > 0; + + if (dialogOverlay == null || !hasUnsavedChanges) return true; // if the dialog is already displayed, block exiting until the user explicitly makes a decision. From 764029bde1eda3976982238bf9d1cba6ebe56fa9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 19 Jul 2023 19:23:08 +0200 Subject: [PATCH 3/3] Fix nullability inspection --- .../OnlinePlay/Multiplayer/MultiplayerLoungeSubScreen.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerLoungeSubScreen.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerLoungeSubScreen.cs index dd4f35cdd4..4478179726 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerLoungeSubScreen.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerLoungeSubScreen.cs @@ -41,7 +41,7 @@ public override void OnResuming(ScreenTransitionEvent e) // To work around this, temporarily remove the room and trigger an immediate listing poll. if (e.Last is MultiplayerMatchSubScreen match) { - RoomManager.RemoveRoom(match.Room); + RoomManager?.RemoveRoom(match.Room); ListingPollingComponent.PollImmediately(); } }