From d4ebff6ea1f381bad406c692ab15b097fbdefd2b Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Thu, 3 Feb 2022 23:18:22 +0900 Subject: [PATCH 1/3] Add failing test --- .../Multiplayer/TestSceneMultiplayer.cs | 46 +++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayer.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayer.cs index 3563869d8b..27bf1c209c 100644 --- a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayer.cs +++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayer.cs @@ -871,6 +871,52 @@ namespace osu.Game.Tests.Visual.Multiplayer AddUntilStep("queue is empty", () => this.ChildrenOfType().Single().Items.Count == 0); } + [Test] + public void TestGameplayStartsWhileInSpectatorScreen() + { + createRoom(() => new Room + { + Name = { Value = "Test Room" }, + Playlist = + { + new PlaylistItem + { + Beatmap = { Value = beatmaps.GetWorkingBeatmap(importedSet.Beatmaps.First(b => b.Ruleset.OnlineID == 0)).BeatmapInfo }, + Ruleset = { Value = new OsuRuleset().RulesetInfo }, + } + } + }); + + AddStep("join other user and make host", () => + { + client.AddUser(new APIUser { Id = 1234 }); + client.TransferHost(1234); + }); + + AddStep("set local user spectating", () => client.ChangeUserState(API.LocalUser.Value.OnlineID, MultiplayerUserState.Spectating)); + AddUntilStep("wait for spectating state", () => client.LocalUser?.State == MultiplayerUserState.Spectating); + + runGameplay(); + + AddStep("exit gameplay for other user", () => client.ChangeUserState(1234, MultiplayerUserState.Idle)); + AddUntilStep("wait for room to be idle", () => client.Room?.State == MultiplayerRoomState.Open); + + runGameplay(); + + void runGameplay() + { + AddStep("start match by other user", () => + { + client.ChangeUserState(1234, MultiplayerUserState.Ready); + client.StartMatch(); + }); + + AddUntilStep("wait for loading", () => client.Room?.State == MultiplayerRoomState.WaitingForLoad); + AddStep("set player loaded", () => client.ChangeUserState(1234, MultiplayerUserState.Loaded)); + AddUntilStep("wait for gameplay to start", () => client.Room?.State == MultiplayerRoomState.Playing); + } + } + private void enterGameplay() { pressReadyButton(); From b41655d5b905cd28425e2851f6900f24f7a82893 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Thu, 3 Feb 2022 23:22:08 +0900 Subject: [PATCH 2/3] Fix crash when gameplay starts while in multi-spectator screen --- osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayer.cs | 1 + .../OnlinePlay/Multiplayer/MultiplayerMatchSubScreen.cs | 7 +++++++ 2 files changed, 8 insertions(+) diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayer.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayer.cs index 27bf1c209c..8f6ba6375f 100644 --- a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayer.cs +++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayer.cs @@ -914,6 +914,7 @@ namespace osu.Game.Tests.Visual.Multiplayer AddUntilStep("wait for loading", () => client.Room?.State == MultiplayerRoomState.WaitingForLoad); AddStep("set player loaded", () => client.ChangeUserState(1234, MultiplayerUserState.Loaded)); AddUntilStep("wait for gameplay to start", () => client.Room?.State == MultiplayerRoomState.Playing); + AddUntilStep("wait for local user to enter spectator", () => multiplayerComponents.CurrentScreen is MultiSpectatorScreen); } } diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerMatchSubScreen.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerMatchSubScreen.cs index 4bd68f2034..020217eac6 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerMatchSubScreen.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerMatchSubScreen.cs @@ -457,6 +457,13 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer return; } + // The beatmap is queried asynchronously when the selected item changes. + // This is an issue with MultiSpectatorScreen which is effectively in an always "ready" state and receives LoadRequested() callbacks + // even when it is not truly ready (i.e. the beatmap hasn't been selected by the client yet). For the time being, a simple fix to this is to ignore the callback. + // Note that spectator will be entered automatically when the client is capable of doing so via beatmap availability callbacks (see: updateBeatmapAvailability()). + if (client.LocalUser?.State == MultiplayerUserState.Spectating && Beatmap.IsDefault) + return; + StartPlay(); readyClickOperation?.Dispose(); From 0473c6c52f9156954eb7c782a421a540cbc35e5d Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Fri, 4 Feb 2022 17:53:30 +0900 Subject: [PATCH 3/3] Also handle null SelectedItem for safety --- .../Screens/OnlinePlay/Multiplayer/MultiplayerMatchSubScreen.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerMatchSubScreen.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerMatchSubScreen.cs index 020217eac6..a397493bab 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerMatchSubScreen.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerMatchSubScreen.cs @@ -461,7 +461,7 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer // This is an issue with MultiSpectatorScreen which is effectively in an always "ready" state and receives LoadRequested() callbacks // even when it is not truly ready (i.e. the beatmap hasn't been selected by the client yet). For the time being, a simple fix to this is to ignore the callback. // Note that spectator will be entered automatically when the client is capable of doing so via beatmap availability callbacks (see: updateBeatmapAvailability()). - if (client.LocalUser?.State == MultiplayerUserState.Spectating && Beatmap.IsDefault) + if (client.LocalUser?.State == MultiplayerUserState.Spectating && (SelectedItem.Value == null || Beatmap.IsDefault)) return; StartPlay();