Skip to content

Fix LoadRequested sent to all users in multiplayer rooms #410

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
smoogipoo wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Fix LoadRequested sent to all users in multiplayer rooms #410

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
aceedbc
Adjust to invoke LoadRequested on users instead
smoogipoo Dec 24, 2025
002cfaf
Add failing test
smoogipoo Dec 24, 2025
ae391cc
Fix users without beatmap getting LoadRequested()
smoogipoo Dec 24, 2025
a2a7127
Fix more tests
smoogipoo Dec 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion osu.Server.Spectator.Tests/Matchmaking/MatchmakingMatchControllerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -166,6 +167,9 @@ public async Task NormalRoomFlow()
}

Receiver.Invocations.Clear();
UserReceiver.Invocations.Clear();
User2Receiver.Invocations.Clear();

await gotoNextStage();
}

Expand Down
9 changes: 5 additions & 4 deletions osu.Server.Spectator.Tests/Multiplayer/MatchSpectatingTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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]
Expand All @@ -73,15 +73,16 @@ 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);
await Hub.ChangeState(MultiplayerUserState.Spectating);
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);
}
}
}
8 changes: 6 additions & 2 deletions osu.Server.Spectator.Tests/Multiplayer/MultiplayerFlowTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -41,7 +42,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))
Expand Down Expand Up @@ -136,7 +137,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))
{
Expand Down Expand Up @@ -231,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();
Expand Down
24 changes: 13 additions & 11 deletions osu.Server.Spectator.Tests/Multiplayer/MultiplayerMatchStartCountdownTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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) });

Expand All @@ -62,15 +63,15 @@ public async Task GameplayStartsWhenCountdownEnds()
Debug.Assert(room != null);

Assert.Null(room.FindCountdownOfType<MatchStartCountdown>());
Receiver.Verify(r => r.LoadRequested(), Times.Once);
UserReceiver.Verify(r => r.LoadRequested(), Times.Once);
}
}

[Fact(Timeout = test_timeout)]
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) });

Expand All @@ -96,7 +97,7 @@ public async Task GameplayStartsWhenCountdownFinished()
Debug.Assert(room != null);

Assert.Null(room.FindCountdownOfType<MatchStartCountdown>());
Receiver.Verify(r => r.LoadRequested(), Times.Once);
UserReceiver.Verify(r => r.LoadRequested(), Times.Once);
}
}

Expand Down Expand Up @@ -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.

Expand Down Expand Up @@ -185,7 +186,7 @@ public async Task NewCountdownOverridesExisting()

Assert.Null(room.FindCountdownOfType<MatchStartCountdown>());
Receiver.Verify(r => r.MatchEvent(It.Is<CountdownStoppedEvent>(e => e.ID == secondCountdown.ID)), Times.Once);
Receiver.Verify(r => r.LoadRequested(), Times.Once);
UserReceiver.Verify(r => r.LoadRequested(), Times.Once);
}
}

Expand Down Expand Up @@ -250,31 +251,32 @@ 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<MatchStartCountdown>());

await skipToEndOfCountdown();
Receiver.Verify(r => r.LoadRequested(), Times.Once);
UserReceiver.Verify(r => r.LoadRequested(), Times.Once);
}

[Fact(Timeout = test_timeout)]
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<MatchStartCountdown>());

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)]
Expand Down
24 changes: 20 additions & 4 deletions osu.Server.Spectator.Tests/Multiplayer/MultiplayerTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,17 +101,19 @@ protected MultiplayerTest()
Groups = new Mock<IGroupManager>();

Receiver = new Mock<DelegatingMultiplayerClient> { 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<DelegatingMultiplayerClient> { 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<IMultiplayerClient>();

var hubContext = new Mock<IHubContext<MultiplayerHub>>();
hubContext.Setup(ctx => ctx.Groups).Returns(Groups.Object);
hubContext.Setup(ctx => ctx.Clients.Client(It.IsAny<string>())).Returns<string>(connectionId => (ISingleClientProxy)Clients.Object.Client(connectionId));
hubContext.Setup(ctx => ctx.Clients.Group(It.IsAny<string>())).Returns<string>(groupName => (ISingleClientProxy)Clients.Object.Group(groupName));
hubContext.Setup(ctx => ctx.Clients.User(It.IsAny<string>())).Returns<string>(userId => (ISingleClientProxy)Clients.Object.User(userId));
hubContext.Setup(ctx => ctx.Clients.Users(It.IsAny<IReadOnlyList<string>>())).Returns<IReadOnlyList<string>>(userIds => (ISingleClientProxy)Clients.Object.Users(userIds));
hubContext.Setup(ctx => ctx.Clients.All).Returns((ISingleClientProxy)Clients.Object.All);

Groups.Setup(g => g.AddToGroupAsync(It.IsAny<string>(), It.IsAny<string>(), It.IsAny<CancellationToken>()))
Expand All @@ -133,10 +135,18 @@ protected MultiplayerTest()
Clients.Setup(clients => clients.Group(It.IsAny<string>())).Returns<string>(groupName =>
{
var groupReceiver = new Mock<DelegatingMultiplayerClient> { 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<IReadOnlyList<string>>())).Returns<IReadOnlyList<string>>(userIds =>
{
var userReceiver = new Mock<DelegatingMultiplayerClient> { 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);
Expand Down Expand Up @@ -189,14 +199,20 @@ protected MultiplayerTest()

SetUserContext(ContextUser);

IEnumerable<IMultiplayerClient> getClientsForGroup(string groupName)
IEnumerable<IMultiplayerClient> clientsByGroup(string groupName)
{
if (!groupMapping.TryGetValue(groupName, out var connectionIds))
yield break;

foreach (var id in connectionIds)
yield return clientMapping[int.Parse(id)];
}

IEnumerable<IMultiplayerClient> clientsByUserId(IEnumerable<string> userIds)
{
foreach (var id in userIds)
yield return clientMapping[int.Parse(id)];
}
}

protected void CreateUser(int userId, out Mock<HubCallerContext> context, out Mock<DelegatingMultiplayerClient> client)
Expand Down
8 changes: 5 additions & 3 deletions osu.Server.Spectator.Tests/Multiplayer/RoomSettingsTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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))
{
Expand All @@ -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);
Expand Down
46 changes: 44 additions & 2 deletions osu.Server.Spectator.Tests/Multiplayer/UserStateManagementTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -409,7 +409,7 @@ public async Task OnlyFinishedUsersTransitionToResults()
}

[Fact]
public async Task IdleUsersDoGetLoadRequest()
public async Task IdleUsersWithBeatmapReceiveLoadRequest()
{
await Hub.JoinRoom(ROOM_ID);

Expand Down Expand Up @@ -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))
{
Expand All @@ -449,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()
{
Expand Down
7 changes: 5 additions & 2 deletions osu.Server.Spectator/Hubs/Multiplayer/MultiplayerHubContext.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -300,10 +300,13 @@ 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));
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);

Expand Down
Loading