From 3915b8e414a00e085e6c59a2da6c7df76f32fcc1 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Thu, 26 May 2022 20:01:33 +0900 Subject: [PATCH 1/2] Fix multiplayer race condition when starting gameplay --- .../Screens/OnlinePlay/Multiplayer/MultiplayerPlayer.cs | 6 ++++++ .../OnlinePlay/Multiplayer/MultiplayerPlayerLoader.cs | 9 +++++++-- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerPlayer.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerPlayer.cs index 5dab845999..d9125dc0a0 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerPlayer.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerPlayer.cs @@ -149,6 +149,12 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer protected override void StartGameplay() { + // We can enter this screen one of two ways: + // 1. Via the automatic natural progression of PlayerLoader into Player. + // We'll arrive here in a Loaded state, and we need to let the server know that we're ready to start. + // 2. Via the server forcefully starting gameplay because players have been hanging out in PlayerLoader for too long. + // We'll arrive here in a Playing state, and we should neither show the loading spinner nor tell the server that we're ready to start (gameplay has already started). + if (client.LocalUser?.State == MultiplayerUserState.Loaded) { // block base call, but let the server know we are ready to start. diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerPlayerLoader.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerPlayerLoader.cs index 7f01bd64ab..e9bf5339a9 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerPlayerLoader.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerPlayerLoader.cs @@ -26,8 +26,13 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer } protected override bool ReadyForGameplay => - base.ReadyForGameplay - // The server is forcefully starting gameplay. + ( + // The user is ready to enter gameplay. + base.ReadyForGameplay + // And the server has received the message that we're loaded. + && multiplayerClient.LocalUser?.State == MultiplayerUserState.Loaded + ) + // Or the server is forcefully starting gameplay. || multiplayerClient.LocalUser?.State == MultiplayerUserState.Playing; protected override void OnPlayerLoaded() From 0224947de0b886468dad7940c4dd6cd94b242e13 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Thu, 26 May 2022 20:09:28 +0900 Subject: [PATCH 2/2] Add comment about how starting gameplay works --- osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerPlayer.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerPlayer.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerPlayer.cs index d9125dc0a0..3bf8a09cb9 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerPlayer.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerPlayer.cs @@ -154,10 +154,11 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer // We'll arrive here in a Loaded state, and we need to let the server know that we're ready to start. // 2. Via the server forcefully starting gameplay because players have been hanging out in PlayerLoader for too long. // We'll arrive here in a Playing state, and we should neither show the loading spinner nor tell the server that we're ready to start (gameplay has already started). + // + // The base call is blocked here because in both cases gameplay is started only when the server says so via onGameplayStarted(). if (client.LocalUser?.State == MultiplayerUserState.Loaded) { - // block base call, but let the server know we are ready to start. loadingDisplay.Show(); client.ChangeState(MultiplayerUserState.ReadyForGameplay); }