Skip to content

Commit 1eee6dc

Browse files
author
Bartłomiej Dach
authored
Attempt to improve safety of pushing matchmaking screens (ppy#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 ppy#37374 but my hopes are not high. Includes some extra cleanups I noticed along the way.
1 parent 9a28465 commit 1eee6dc

3 files changed

Lines changed: 23 additions & 5 deletions

File tree

osu.Game.Tests/Visual/Matchmaking/TestSceneMatchmakingQueueScreen.cs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,22 @@ public void TestBasic()
7171
AddStep("change state to in room", () => queueScreen!.SetState(ScreenQueue.MatchmakingScreenState.InRoom));
7272
}
7373

74+
[Test]
75+
public void TestDelayedRoomScreenPushDoesNotRunIfRoomIsLeftPrematurely()
76+
{
77+
AddStep("change state to in room then immediately leave room", () =>
78+
{
79+
queueScreen!.SetState(ScreenQueue.MatchmakingScreenState.InRoom);
80+
MultiplayerClient.LeaveRoom();
81+
});
82+
83+
// the queue screen waits 2 seconds between transitioning to `InRoom` state and actually pushing the relevant screen.
84+
// if the room goes to `null` in that time, things die very hard.
85+
// therefore the wait here is to check that things don't die very hard.
86+
// if they do the test will throw an exception and fail.
87+
AddWaitStep("wait a little bit", 10);
88+
}
89+
7490
private static double generateCount(double x, double mean, double stdDev, double amplitude)
7591
{
7692
return amplitude * Math.Exp(-Math.Pow(x - mean, 2) / (2 * Math.Pow(stdDev, 2))) + Random.Shared.Next(300);

osu.Game/Screens/OnlinePlay/Matchmaking/Queue/QueueController.cs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,6 @@ namespace osu.Game.Screens.OnlinePlay.Matchmaking.Queue
2727
///
2828
/// Includes support for deferring to background.
2929
/// </summary>
30-
/// <remarks>
31-
/// This is initialised and cached in the <see cref="ScreenQueue"/> but can be used throughout the system via DI.</remarks>
3230
public partial class QueueController : Component
3331
{
3432
public readonly Bindable<ScreenQueue.MatchmakingScreenState> CurrentState = new Bindable<ScreenQueue.MatchmakingScreenState>();

osu.Game/Screens/OnlinePlay/Matchmaking/Queue/ScreenQueue.cs

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ public partial class ScreenQueue : OsuScreen
9090
private SampleChannel? waitingLoopChannel;
9191
private ScheduledDelegate? startLoopPlaybackDelegate;
9292
private DrawableSample waitingLoop = null!;
93+
private ScheduledDelegate? pushScreenDelegate;
9394

9495
private int? userRating;
9596

@@ -390,14 +391,14 @@ private void onSelectedPoolChanged(ValueChangedEvent<MatchmakingPool?> e)
390391

391392
if (e.NewValue == null)
392393
{
393-
client.MatchmakingLeaveLobby();
394+
client.MatchmakingLeaveLobby().FireAndForget();
394395
return;
395396
}
396397

397398
client.MatchmakingJoinLobbyWithParams(new MatchmakingJoinLobbyRequest
398399
{
399400
PoolId = e.NewValue.Id
400-
});
401+
}).FireAndForget();
401402
}
402403

403404
public override void OnEntering(ScreenTransitionEvent e)
@@ -465,6 +466,9 @@ public void SetState(MatchmakingScreenState newState)
465466
startLoopPlaybackDelegate?.Cancel();
466467
stopWaitingLoopPlayback();
467468

469+
pushScreenDelegate?.Cancel();
470+
pushScreenDelegate = null;
471+
468472
switch (newState)
469473
{
470474
case MatchmakingScreenState.Idle:
@@ -599,7 +603,7 @@ public void SetState(MatchmakingScreenState newState)
599603

600604
using (BeginDelayedSequence(2000))
601605
{
602-
Schedule(() =>
606+
pushScreenDelegate = Schedule(() =>
603607
{
604608
switch (poolType)
605609
{

0 commit comments

Comments
 (0)