Skip to content

Fix LoadRequested sent to all users in multiplayer rooms#410

Open
smoogipoo wants to merge 4 commits intoppy:masterfrom
smoogipoo:mp-fix-load-request-without-beatmap
Open

Fix LoadRequested sent to all users in multiplayer rooms#410
smoogipoo wants to merge 4 commits intoppy:masterfrom
smoogipoo:mp-fix-load-request-without-beatmap

Conversation

@smoogipoo
Copy link
Copy Markdown
Contributor

This is part 2 to fixing ppy/osu#36045, and mentioned as bullet point 1 in ppy/osu#36121:

The multiplayer server sends LoadRequested to all users in the room regardless of their beatmap availability. This is an issue on its own that will be fixed separately.

I'm not 100% sure on this change in principle, and there are 3 paths that we can go down here:

  1. This change.
  2. Re-introduce the server-side "gameplay" group rather than using .Users(). This seems like it'd be a more complex change.
  3. Continue sending LoadRequested to all users in the room and handle this locally in the client.

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.
@smoogipoo
Copy link
Copy Markdown
Contributor Author

The number of test changes here makes me feel a bit uneasy, as if option 3 from the OP is the better one to go down... Let's treat this PR as an RFC.

@bdach
Copy link
Copy Markdown
Collaborator

bdach commented Dec 29, 2025

The number of test changes here makes me feel a bit uneasy

Without real full-stack testing I'm kind of unbothered by anything in here? Unsure whether this is because of lack of bothering capacity or there's really not much to see here.

@smoogipoo
Copy link
Copy Markdown
Contributor Author

Maybe it's just me being a bit too critical.

@peppy
Copy link
Copy Markdown
Member

peppy commented May 5, 2026

@smoogipoo shall we give this another try? needs some conflict resolution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants