From aceedbc50a99e1c1ea90d2a4deba95ba2da4af19 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Wed, 24 Dec 2025 14:39:55 +0900 Subject: [PATCH 1/4] Adjust to invoke LoadRequested on users instead On its own, this commit is not intended to change anything except adjust the tests slightly so that a future failing test can be added. --- .../MatchmakingMatchControllerTests.cs | 6 ++++- .../Multiplayer/MatchSpectatingTests.cs | 9 +++---- .../Multiplayer/MultiplayerFlowTests.cs | 5 ++-- .../MultiplayerMatchStartCountdownTest.cs | 11 +++++---- .../Multiplayer/MultiplayerTest.cs | 24 +++++++++++++++---- .../Multiplayer/UserStateManagementTests.cs | 3 ++- .../Hubs/Multiplayer/MultiplayerHubContext.cs | 3 +-- 7 files changed, 42 insertions(+), 19 deletions(-) diff --git a/osu.Server.Spectator.Tests/Matchmaking/MatchmakingMatchControllerTests.cs b/osu.Server.Spectator.Tests/Matchmaking/MatchmakingMatchControllerTests.cs index f9aad59b..f13951d1 100644 --- a/osu.Server.Spectator.Tests/Matchmaking/MatchmakingMatchControllerTests.cs +++ b/osu.Server.Spectator.Tests/Matchmaking/MatchmakingMatchControllerTests.cs @@ -130,7 +130,8 @@ public async Task NormalRoomFlow() playlistItemId = room.Item!.CurrentPlaylistItem.ID; // Check that a request to load gameplay was started. - Receiver.Verify(u => u.LoadRequested(), Times.Once); + UserReceiver.Verify(u => u.LoadRequested(), Times.Once); + User2Receiver.Verify(u => u.LoadRequested(), Times.Once); // Start gameplay for both users. SetUserContext(ContextUser); @@ -166,6 +167,9 @@ public async Task NormalRoomFlow() } Receiver.Invocations.Clear(); + UserReceiver.Invocations.Clear(); + User2Receiver.Invocations.Clear(); + await gotoNextStage(); } diff --git a/osu.Server.Spectator.Tests/Multiplayer/MatchSpectatingTests.cs b/osu.Server.Spectator.Tests/Multiplayer/MatchSpectatingTests.cs index b657973d..97472c7c 100644 --- a/osu.Server.Spectator.Tests/Multiplayer/MatchSpectatingTests.cs +++ b/osu.Server.Spectator.Tests/Multiplayer/MatchSpectatingTests.cs @@ -39,7 +39,7 @@ public async Task SpectatingUserStateDoesNotChange() SetUserContext(ContextUser); await Hub.StartMatch(); - Receiver.Verify(c => c.LoadRequested(), Times.Once); + UserReceiver.Verify(c => c.LoadRequested(), Times.Once); Clients.Verify(clients => clients.Client(ContextUser2.Object.ConnectionId).UserStateChanged(USER_ID_2, MultiplayerUserState.WaitingForLoad), Times.Never); await Hub.ChangeState(MultiplayerUserState.Loaded); @@ -64,7 +64,7 @@ public async Task SpectatingHostCanStartMatch() SetUserContext(ContextUser); await Hub.StartMatch(); - Receiver.Verify(c => c.LoadRequested(), Times.Once); + UserReceiver.Verify(c => c.LoadRequested(), Times.Once); } [Fact] @@ -73,7 +73,7 @@ public async Task SpectatingUserReceivesLoadRequestedAfterGameplayStarted() await Hub.JoinRoom(ROOM_ID); await MarkCurrentUserReadyAndAvailable(); await Hub.StartMatch(); - Receiver.Verify(c => c.LoadRequested(), Times.Once); + UserReceiver.Verify(c => c.LoadRequested(), Times.Once); SetUserContext(ContextUser2); await Hub.JoinRoom(ROOM_ID); @@ -81,7 +81,8 @@ public async Task SpectatingUserReceivesLoadRequestedAfterGameplayStarted() Caller.Verify(c => c.LoadRequested(), Times.Once); // Ensure no other clients received LoadRequested(). - Receiver.Verify(c => c.LoadRequested(), Times.Once); + UserReceiver.Verify(c => c.LoadRequested(), Times.Once); + User2Receiver.Verify(c => c.LoadRequested(), Times.Never); } } } diff --git a/osu.Server.Spectator.Tests/Multiplayer/MultiplayerFlowTests.cs b/osu.Server.Spectator.Tests/Multiplayer/MultiplayerFlowTests.cs index 4cb45b93..a3242a54 100644 --- a/osu.Server.Spectator.Tests/Multiplayer/MultiplayerFlowTests.cs +++ b/osu.Server.Spectator.Tests/Multiplayer/MultiplayerFlowTests.cs @@ -41,7 +41,7 @@ public async Task SingleUserMatchFlow() await Hub.StartMatch(); // server requests the all users start loading. - Receiver.Verify(r => r.LoadRequested(), Times.Once); + UserReceiver.Verify(r => r.LoadRequested(), Times.Once); Receiver.Verify(r => r.UserStateChanged(USER_ID, MultiplayerUserState.WaitingForLoad), Times.Once); using (var room = await Rooms.GetForUse(ROOM_ID)) @@ -136,7 +136,8 @@ public async Task MultiUserMatchFlow() await Hub.StartMatch(); // server requests the all users start loading. - Receiver.Verify(r => r.LoadRequested(), Times.Once); + UserReceiver.Verify(r => r.LoadRequested(), Times.Once); + User2Receiver.Verify(r => r.LoadRequested(), Times.Once); using (var room = await Rooms.GetForUse(ROOM_ID)) { diff --git a/osu.Server.Spectator.Tests/Multiplayer/MultiplayerMatchStartCountdownTest.cs b/osu.Server.Spectator.Tests/Multiplayer/MultiplayerMatchStartCountdownTest.cs index 4ac03632..67b7e697 100644 --- a/osu.Server.Spectator.Tests/Multiplayer/MultiplayerMatchStartCountdownTest.cs +++ b/osu.Server.Spectator.Tests/Multiplayer/MultiplayerMatchStartCountdownTest.cs @@ -62,7 +62,7 @@ public async Task GameplayStartsWhenCountdownEnds() Debug.Assert(room != null); Assert.Null(room.FindCountdownOfType()); - Receiver.Verify(r => r.LoadRequested(), Times.Once); + UserReceiver.Verify(r => r.LoadRequested(), Times.Once); } } @@ -96,7 +96,7 @@ public async Task GameplayStartsWhenCountdownFinished() Debug.Assert(room != null); Assert.Null(room.FindCountdownOfType()); - Receiver.Verify(r => r.LoadRequested(), Times.Once); + UserReceiver.Verify(r => r.LoadRequested(), Times.Once); } } @@ -185,7 +185,7 @@ public async Task NewCountdownOverridesExisting() Assert.Null(room.FindCountdownOfType()); Receiver.Verify(r => r.MatchEvent(It.Is(e => e.ID == secondCountdown.ID)), Times.Once); - Receiver.Verify(r => r.LoadRequested(), Times.Once); + UserReceiver.Verify(r => r.LoadRequested(), Times.Once); } } @@ -257,7 +257,7 @@ public async Task AutoStartCountdownStartsWhenHostReadies() Assert.NotNull(usage.Item!.FindCountdownOfType()); await skipToEndOfCountdown(); - Receiver.Verify(r => r.LoadRequested(), Times.Once); + UserReceiver.Verify(r => r.LoadRequested(), Times.Once); } [Fact(Timeout = test_timeout)] @@ -274,7 +274,8 @@ public async Task AutoStartCountdownStartsWhenGuestReadies() Assert.NotNull(usage.Item!.FindCountdownOfType()); await skipToEndOfCountdown(); - Receiver.Verify(r => r.LoadRequested(), Times.Once); + UserReceiver.Verify(r => r.LoadRequested(), Times.Once); + User2Receiver.Verify(r => r.LoadRequested(), Times.Once); } [Fact(Timeout = test_timeout)] diff --git a/osu.Server.Spectator.Tests/Multiplayer/MultiplayerTest.cs b/osu.Server.Spectator.Tests/Multiplayer/MultiplayerTest.cs index fae41593..ea724de6 100644 --- a/osu.Server.Spectator.Tests/Multiplayer/MultiplayerTest.cs +++ b/osu.Server.Spectator.Tests/Multiplayer/MultiplayerTest.cs @@ -101,10 +101,10 @@ protected MultiplayerTest() Groups = new Mock(); Receiver = new Mock { CallBase = true }; - Receiver.Setup(c => c.Clients).Returns(getClientsForGroup(MultiplayerHub.GetGroupId(ROOM_ID))); + Receiver.Setup(c => c.Clients).Returns(clientsByGroup(MultiplayerHub.GetGroupId(ROOM_ID))); Receiver2 = new Mock { CallBase = true }; - Receiver2.Setup(c => c.Clients).Returns(getClientsForGroup(MultiplayerHub.GetGroupId(ROOM_ID_2))); + Receiver2.Setup(c => c.Clients).Returns(clientsByGroup(MultiplayerHub.GetGroupId(ROOM_ID_2))); Caller = new Mock(); @@ -112,6 +112,8 @@ protected MultiplayerTest() hubContext.Setup(ctx => ctx.Groups).Returns(Groups.Object); hubContext.Setup(ctx => ctx.Clients.Client(It.IsAny())).Returns(connectionId => (ISingleClientProxy)Clients.Object.Client(connectionId)); hubContext.Setup(ctx => ctx.Clients.Group(It.IsAny())).Returns(groupName => (ISingleClientProxy)Clients.Object.Group(groupName)); + hubContext.Setup(ctx => ctx.Clients.User(It.IsAny())).Returns(userId => (ISingleClientProxy)Clients.Object.User(userId)); + hubContext.Setup(ctx => ctx.Clients.Users(It.IsAny>())).Returns>(userIds => (ISingleClientProxy)Clients.Object.Users(userIds)); hubContext.Setup(ctx => ctx.Clients.All).Returns((ISingleClientProxy)Clients.Object.All); Groups.Setup(g => g.AddToGroupAsync(It.IsAny(), It.IsAny(), It.IsAny())) @@ -133,10 +135,18 @@ protected MultiplayerTest() Clients.Setup(clients => clients.Group(It.IsAny())).Returns(groupName => { var groupReceiver = new Mock { CallBase = true }; - groupReceiver.Setup(c => c.Clients).Returns(getClientsForGroup(groupName)); + groupReceiver.Setup(c => c.Clients).Returns(clientsByGroup(groupName)); return groupReceiver.Object; }); + // Generic user receiver + Clients.Setup(clients => clients.Users(It.IsAny>())).Returns>(userIds => + { + var userReceiver = new Mock { CallBase = true }; + userReceiver.Setup(c => c.Clients).Returns(clientsByUserId(userIds)); + return userReceiver.Object; + }); + // Room-specific group receivers Clients.Setup(clients => clients.Group(MultiplayerHub.GetGroupId(ROOM_ID))).Returns(Receiver.Object); Clients.Setup(clients => clients.Group(MultiplayerHub.GetGroupId(ROOM_ID_2))).Returns(Receiver2.Object); @@ -189,7 +199,7 @@ protected MultiplayerTest() SetUserContext(ContextUser); - IEnumerable getClientsForGroup(string groupName) + IEnumerable clientsByGroup(string groupName) { if (!groupMapping.TryGetValue(groupName, out var connectionIds)) yield break; @@ -197,6 +207,12 @@ IEnumerable getClientsForGroup(string groupName) foreach (var id in connectionIds) yield return clientMapping[int.Parse(id)]; } + + IEnumerable clientsByUserId(IEnumerable userIds) + { + foreach (var id in userIds) + yield return clientMapping[int.Parse(id)]; + } } protected void CreateUser(int userId, out Mock context, out Mock client) diff --git a/osu.Server.Spectator.Tests/Multiplayer/UserStateManagementTests.cs b/osu.Server.Spectator.Tests/Multiplayer/UserStateManagementTests.cs index bbb2ce1d..fb331666 100644 --- a/osu.Server.Spectator.Tests/Multiplayer/UserStateManagementTests.cs +++ b/osu.Server.Spectator.Tests/Multiplayer/UserStateManagementTests.cs @@ -440,7 +440,8 @@ public async Task IdleUsersDoGetLoadRequest() // host requests the start of the match. await Hub.StartMatch(); - Receiver.Verify(r => r.LoadRequested(), Times.Once); + UserReceiver.Verify(r => r.LoadRequested(), Times.Once); + User2Receiver.Verify(r => r.LoadRequested(), Times.Once); using (var room = await Rooms.GetForUse(ROOM_ID)) { diff --git a/osu.Server.Spectator/Hubs/Multiplayer/MultiplayerHubContext.cs b/osu.Server.Spectator/Hubs/Multiplayer/MultiplayerHubContext.cs index 68106a33..bbcab5e8 100644 --- a/osu.Server.Spectator/Hubs/Multiplayer/MultiplayerHubContext.cs +++ b/osu.Server.Spectator/Hubs/Multiplayer/MultiplayerHubContext.cs @@ -300,10 +300,9 @@ public async Task StartMatch(ServerMultiplayerRoom room) foreach (var u in readyUsers) await ChangeAndBroadcastUserState(room, u, MultiplayerUserState.WaitingForLoad); - await ChangeRoomState(room, MultiplayerRoomState.WaitingForLoad); - await context.Clients.Group(MultiplayerHub.GetGroupId(room.RoomID)).SendAsync(nameof(IMultiplayerClient.LoadRequested)); + await context.Clients.Users(room.Users.Select(u => u.UserID.ToString())).SendAsync(nameof(IMultiplayerClient.LoadRequested)); await room.StartCountdown(new ForceGameplayStartCountdown { TimeRemaining = gameplay_load_timeout }, StartOrStopGameplay); From 002cfaf34a58f7ecba07309f2447bf7a3f90ab0b Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Wed, 24 Dec 2025 14:42:32 +0900 Subject: [PATCH 2/4] Add failing test --- .../Multiplayer/UserStateManagementTests.cs | 43 ++++++++++++++++++- 1 file changed, 42 insertions(+), 1 deletion(-) diff --git a/osu.Server.Spectator.Tests/Multiplayer/UserStateManagementTests.cs b/osu.Server.Spectator.Tests/Multiplayer/UserStateManagementTests.cs index fb331666..cca7e772 100644 --- a/osu.Server.Spectator.Tests/Multiplayer/UserStateManagementTests.cs +++ b/osu.Server.Spectator.Tests/Multiplayer/UserStateManagementTests.cs @@ -409,7 +409,7 @@ public async Task OnlyFinishedUsersTransitionToResults() } [Fact] - public async Task IdleUsersDoGetLoadRequest() + public async Task IdleUsersWithBeatmapReceiveLoadRequest() { await Hub.JoinRoom(ROOM_ID); @@ -450,6 +450,47 @@ public async Task IdleUsersDoGetLoadRequest() } } + [Fact] + public async Task IdleUsersWithoutBeatmapDoNotReceiveLoadRequest() + { + await Hub.JoinRoom(ROOM_ID); + + SetUserContext(ContextUser2); + await Hub.JoinRoom(ROOM_ID); + + SetUserContext(ContextUser); + + using (var room = await Rooms.GetForUse(ROOM_ID)) + { + Assert.NotNull(room.Item); + Assert.All(room.Item.Users, u => Assert.Equal(MultiplayerUserState.Idle, u.State)); + } + + // one user enters a ready state. + await MarkCurrentUserReadyAndAvailable(); + + using (var room = await Rooms.GetForUse(ROOM_ID)) + { + Assert.NotNull(room.Item); + Assert.Single(room.Item.Users, u => u.State == MultiplayerUserState.Idle); + Assert.Single(room.Item.Users, u => u.State == MultiplayerUserState.Ready); + + Assert.Equal(MultiplayerRoomState.Open, room.Item.State); + } + + // host requests the start of the match. + await Hub.StartMatch(); + + UserReceiver.Verify(r => r.LoadRequested(), Times.Once); + User2Receiver.Verify(r => r.LoadRequested(), Times.Never); + + using (var room = await Rooms.GetForUse(ROOM_ID)) + { + Assert.NotNull(room.Item); + Assert.True(room.Item.Users.Single(u => u.State == MultiplayerUserState.WaitingForLoad).UserID == USER_ID); + } + } + [Fact] public async Task UserCanNotSwitchToIdleDuringGameplay() { From ae391ccae13d06039aa0dc5aa463aa506ad443cb Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Wed, 24 Dec 2025 14:43:46 +0900 Subject: [PATCH 3/4] Fix users without beatmap getting LoadRequested() --- .../Hubs/Multiplayer/MultiplayerHubContext.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/osu.Server.Spectator/Hubs/Multiplayer/MultiplayerHubContext.cs b/osu.Server.Spectator/Hubs/Multiplayer/MultiplayerHubContext.cs index bbcab5e8..95734483 100644 --- a/osu.Server.Spectator/Hubs/Multiplayer/MultiplayerHubContext.cs +++ b/osu.Server.Spectator/Hubs/Multiplayer/MultiplayerHubContext.cs @@ -302,7 +302,11 @@ public async Task StartMatch(ServerMultiplayerRoom room) await ChangeAndBroadcastUserState(room, u, MultiplayerUserState.WaitingForLoad); await ChangeRoomState(room, MultiplayerRoomState.WaitingForLoad); - await context.Clients.Users(room.Users.Select(u => u.UserID.ToString())).SendAsync(nameof(IMultiplayerClient.LoadRequested)); + foreach (var u in room.Users) + { + if (u.State == MultiplayerUserState.WaitingForLoad || u.State == MultiplayerUserState.Spectating) + await context.Clients.User(u.UserID.ToString()).SendAsync(nameof(IMultiplayerClient.LoadRequested)); + } await room.StartCountdown(new ForceGameplayStartCountdown { TimeRemaining = gameplay_load_timeout }, StartOrStopGameplay); From a2a71274029a76beeb94761742afdebfb5edc001 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Wed, 24 Dec 2025 21:12:16 +0900 Subject: [PATCH 4/4] Fix more tests --- .../Multiplayer/MultiplayerFlowTests.cs | 3 +++ .../MultiplayerMatchStartCountdownTest.cs | 13 +++++++------ .../Multiplayer/RoomSettingsTests.cs | 8 +++++--- 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/osu.Server.Spectator.Tests/Multiplayer/MultiplayerFlowTests.cs b/osu.Server.Spectator.Tests/Multiplayer/MultiplayerFlowTests.cs index a3242a54..155e41ab 100644 --- a/osu.Server.Spectator.Tests/Multiplayer/MultiplayerFlowTests.cs +++ b/osu.Server.Spectator.Tests/Multiplayer/MultiplayerFlowTests.cs @@ -5,6 +5,7 @@ using Microsoft.AspNetCore.SignalR; using Moq; using osu.Game.Online.Multiplayer; +using osu.Game.Online.Rooms; using osu.Server.Spectator.Database.Models; using Xunit; @@ -232,6 +233,8 @@ public async Task SecondUserDoesReceiveLoadRequestWhenMatchRestartedAndNotReady( await Hub.AbortGameplay(); // Restart gameplay with just host being ready. + SetUserContext(ContextUser2); + await Hub.ChangeBeatmapAvailability(BeatmapAvailability.LocallyAvailable()); SetUserContext(ContextUser); await MarkCurrentUserReadyAndAvailable(); await Hub.StartMatch(); diff --git a/osu.Server.Spectator.Tests/Multiplayer/MultiplayerMatchStartCountdownTest.cs b/osu.Server.Spectator.Tests/Multiplayer/MultiplayerMatchStartCountdownTest.cs index 67b7e697..5d4359c1 100644 --- a/osu.Server.Spectator.Tests/Multiplayer/MultiplayerMatchStartCountdownTest.cs +++ b/osu.Server.Spectator.Tests/Multiplayer/MultiplayerMatchStartCountdownTest.cs @@ -9,6 +9,7 @@ using Moq; using osu.Game.Online.Multiplayer; using osu.Game.Online.Multiplayer.Countdown; +using osu.Game.Online.Rooms; using Xunit; namespace osu.Server.Spectator.Tests.Multiplayer @@ -37,7 +38,7 @@ public async Task CanStartCountdownIfNotReady() public async Task GameplayStartsWhenCountdownEnds() { await Hub.JoinRoom(ROOM_ID); - await Hub.ChangeState(MultiplayerUserState.Ready); + await MarkCurrentUserReadyAndAvailable(); await Hub.SendMatchRequest(new StartMatchCountdownRequest { Duration = TimeSpan.FromSeconds(3) }); @@ -70,7 +71,7 @@ public async Task GameplayStartsWhenCountdownEnds() public async Task GameplayStartsWhenCountdownFinished() { await Hub.JoinRoom(ROOM_ID); - await Hub.ChangeState(MultiplayerUserState.Ready); + await MarkCurrentUserReadyAndAvailable(); await Hub.SendMatchRequest(new StartMatchCountdownRequest { Duration = TimeSpan.FromMinutes(1) }); @@ -130,7 +131,7 @@ public async Task GameplayDoesNotStartWhenCountdownCancelled() public async Task NewCountdownOverridesExisting() { await Hub.JoinRoom(ROOM_ID); - await Hub.ChangeState(MultiplayerUserState.Ready); + await MarkCurrentUserReadyAndAvailable(); // Start first countdown. @@ -250,8 +251,7 @@ public async Task AutoStartCountdownStartsWhenHostReadies() { await Hub.JoinRoom(ROOM_ID); await Hub.ChangeSettings(new MultiplayerRoomSettings { AutoStartDuration = TimeSpan.FromMinutes(1) }); - - await Hub.ChangeState(MultiplayerUserState.Ready); + await MarkCurrentUserReadyAndAvailable(); using (var usage = await Hub.GetRoom(ROOM_ID)) Assert.NotNull(usage.Item!.FindCountdownOfType()); @@ -265,10 +265,11 @@ public async Task AutoStartCountdownStartsWhenGuestReadies() { await Hub.JoinRoom(ROOM_ID); await Hub.ChangeSettings(new MultiplayerRoomSettings { AutoStartDuration = TimeSpan.FromMinutes(1) }); + await Hub.ChangeBeatmapAvailability(BeatmapAvailability.LocallyAvailable()); SetUserContext(ContextUser2); await Hub.JoinRoom(ROOM_ID); - await Hub.ChangeState(MultiplayerUserState.Ready); + await MarkCurrentUserReadyAndAvailable(); using (var usage = await Hub.GetRoom(ROOM_ID)) Assert.NotNull(usage.Item!.FindCountdownOfType()); diff --git a/osu.Server.Spectator.Tests/Multiplayer/RoomSettingsTests.cs b/osu.Server.Spectator.Tests/Multiplayer/RoomSettingsTests.cs index 0d3d0fc4..e7fca52b 100644 --- a/osu.Server.Spectator.Tests/Multiplayer/RoomSettingsTests.cs +++ b/osu.Server.Spectator.Tests/Multiplayer/RoomSettingsTests.cs @@ -44,11 +44,11 @@ public async Task ChangingSettingsMarksReadyUsersAsIdle() }; await Hub.JoinRoom(ROOM_ID); - await Hub.ChangeState(MultiplayerUserState.Ready); + await MarkCurrentUserReadyAndAvailable(); SetUserContext(ContextUser2); await Hub.JoinRoom(ROOM_ID); - await Hub.ChangeState(MultiplayerUserState.Ready); + await MarkCurrentUserReadyAndAvailable(); using (var roomUsage = await Hub.GetRoom(ROOM_ID)) { @@ -74,8 +74,10 @@ public async Task ChangingSettingsMarksReadyUsersAsIdle() } // Check that both users start gameplay - the second user also starts despite being in an idle state. + SetUserContext(ContextUser2); + await Hub.ChangeBeatmapAvailability(BeatmapAvailability.LocallyAvailable()); SetUserContext(ContextUser); - await Hub.ChangeState(MultiplayerUserState.Ready); + await MarkCurrentUserReadyAndAvailable(); await Hub.StartMatch(); UserReceiver.Verify(r => r.LoadRequested(), Times.Once);