From 1eee6dc870224eee2e10f5ca6916754824609d67 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Tue, 21 Apr 2026 12:11:35 +0200 Subject: [PATCH] Attempt to improve safety of pushing matchmaking screens (#37452) Due to the push of the relevant screens being delayed it's possible that the room goes away between the scheduling of the push and the actual execution of the push. This maybe closes https://github.com/ppy/osu/issues/37374 but my hopes are not high. Includes some extra cleanups I noticed along the way. --- .../TestSceneMatchmakingQueueScreen.cs | 16 ++++++++++++++++ .../Matchmaking/Queue/QueueController.cs | 2 -- .../OnlinePlay/Matchmaking/Queue/ScreenQueue.cs | 10 +++++++--- 3 files changed, 23 insertions(+), 5 deletions(-) diff --git a/osu.Game.Tests/Visual/Matchmaking/TestSceneMatchmakingQueueScreen.cs b/osu.Game.Tests/Visual/Matchmaking/TestSceneMatchmakingQueueScreen.cs index 66a05f4f47..f8882cbff5 100644 --- a/osu.Game.Tests/Visual/Matchmaking/TestSceneMatchmakingQueueScreen.cs +++ b/osu.Game.Tests/Visual/Matchmaking/TestSceneMatchmakingQueueScreen.cs @@ -71,6 +71,22 @@ namespace osu.Game.Tests.Visual.Matchmaking AddStep("change state to in room", () => queueScreen!.SetState(ScreenQueue.MatchmakingScreenState.InRoom)); } + [Test] + public void TestDelayedRoomScreenPushDoesNotRunIfRoomIsLeftPrematurely() + { + AddStep("change state to in room then immediately leave room", () => + { + queueScreen!.SetState(ScreenQueue.MatchmakingScreenState.InRoom); + MultiplayerClient.LeaveRoom(); + }); + + // the queue screen waits 2 seconds between transitioning to `InRoom` state and actually pushing the relevant screen. + // if the room goes to `null` in that time, things die very hard. + // therefore the wait here is to check that things don't die very hard. + // if they do the test will throw an exception and fail. + AddWaitStep("wait a little bit", 10); + } + private static double generateCount(double x, double mean, double stdDev, double amplitude) { return amplitude * Math.Exp(-Math.Pow(x - mean, 2) / (2 * Math.Pow(stdDev, 2))) + Random.Shared.Next(300); diff --git a/osu.Game/Screens/OnlinePlay/Matchmaking/Queue/QueueController.cs b/osu.Game/Screens/OnlinePlay/Matchmaking/Queue/QueueController.cs index 02e944df9f..594f37fde7 100644 --- a/osu.Game/Screens/OnlinePlay/Matchmaking/Queue/QueueController.cs +++ b/osu.Game/Screens/OnlinePlay/Matchmaking/Queue/QueueController.cs @@ -27,8 +27,6 @@ namespace osu.Game.Screens.OnlinePlay.Matchmaking.Queue /// /// Includes support for deferring to background. /// - /// - /// This is initialised and cached in the but can be used throughout the system via DI. public partial class QueueController : Component { public readonly Bindable CurrentState = new Bindable(); diff --git a/osu.Game/Screens/OnlinePlay/Matchmaking/Queue/ScreenQueue.cs b/osu.Game/Screens/OnlinePlay/Matchmaking/Queue/ScreenQueue.cs index d08635a444..73eafecc80 100644 --- a/osu.Game/Screens/OnlinePlay/Matchmaking/Queue/ScreenQueue.cs +++ b/osu.Game/Screens/OnlinePlay/Matchmaking/Queue/ScreenQueue.cs @@ -90,6 +90,7 @@ namespace osu.Game.Screens.OnlinePlay.Matchmaking.Queue private SampleChannel? waitingLoopChannel; private ScheduledDelegate? startLoopPlaybackDelegate; private DrawableSample waitingLoop = null!; + private ScheduledDelegate? pushScreenDelegate; private int? userRating; @@ -390,14 +391,14 @@ namespace osu.Game.Screens.OnlinePlay.Matchmaking.Queue if (e.NewValue == null) { - client.MatchmakingLeaveLobby(); + client.MatchmakingLeaveLobby().FireAndForget(); return; } client.MatchmakingJoinLobbyWithParams(new MatchmakingJoinLobbyRequest { PoolId = e.NewValue.Id - }); + }).FireAndForget(); } public override void OnEntering(ScreenTransitionEvent e) @@ -465,6 +466,9 @@ namespace osu.Game.Screens.OnlinePlay.Matchmaking.Queue startLoopPlaybackDelegate?.Cancel(); stopWaitingLoopPlayback(); + pushScreenDelegate?.Cancel(); + pushScreenDelegate = null; + switch (newState) { case MatchmakingScreenState.Idle: @@ -599,7 +603,7 @@ namespace osu.Game.Screens.OnlinePlay.Matchmaking.Queue using (BeginDelayedSequence(2000)) { - Schedule(() => + pushScreenDelegate = Schedule(() => { switch (poolType) {