Skip to content

Commit

Permalink
Fix MultiplayerMatchSubScreen erroneously pushing exit dialog on AP…
Browse files Browse the repository at this point in the history
…I failure

If `IAPIProvider.State` changes from `Online` at any point when being on
an `OnlinePlayScreen`, it will be forcefully exited from. However,
`MultiplayerMatchSubScreen` had local logic that suppressed the exit in
order to show a confirmation dialog.

The problem is, that in the suppression logic,
`MultiplayerMatchSubScreen` was checking
`MultiplayerClient.IsConnected`, which is a SignalR flag, and was not
checking `IAPIAccess.State`, which is maintained separately. Due to
differing timeouts and failure thresholds, it is not impossible to have
`MultiplayerClient.IsConnected == true` but `IAPIAccess.State !=
APIState.Online`.

In such a case, the match subscreen would wrongly consider itself to be
still online and due to that, push useless confirmation dialogs, while
being in the process of being forcefully exited. This then caused the
dialog to cause a crash, as it was calling `.Exit()` on the screen which
would already have been exited by that point, by the force-exit flow.
  • Loading branch information
bdach committed Jul 16, 2023
1 parent cd02a8a commit 7fbd47e
Showing 1 changed file with 7 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
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;
Expand Down Expand Up @@ -49,6 +50,9 @@ 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; }

Expand Down Expand Up @@ -251,10 +255,10 @@ 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 IsConnected status is also required.
if (client.Room == null || !client.IsConnected.Value)
// 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; exit immediately.
// room has not been created yet or we're offline; exit immediately.
return base.OnExiting(e);
}

Expand Down

0 comments on commit 7fbd47e

Please sign in to comment.