Skip to content
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

Jump to bottom

Multiplayer playlist items can be edited in ways that break ruleset eligibility #30415

Open
3 of 4 tasks
bdach opened this issue Oct 24, 2024 · 3 comments
Open
3 of 4 tasks

Multiplayer playlist items can be edited in ways that break ruleset eligibility #30415

bdach opened this issue Oct 24, 2024 · 3 comments
Assignees
@bdach
Labels
area:multiplayer priority:0 Showstopper. Critical to the next release. type:online

Comments

@bdach
Copy link
Collaborator

bdach commented Oct 24, 2024 •
edited
Loading

Type

Game behaviour

Bug description

See https://discord.com/channels/188630481301012481/1097318920991559880/1299038814060417035 for the report.

I have not done any investigation or confirmed the bug, I'm just moving this here so it doesn't get lost. Needs to be investigated yesterday.

Screenshots or videos

No response

Version

current master in theory

Logs

not provided by original reporter

Tasks
Beta Give feedback
  1. Check ruleset conversion validity when issuing submission tokens osu-web#11589
    area:multiplayer area:scoring
  2. Fix add/edit playlist item operations not checking ruleset conversion eligibility osu-server-spectator#248
  3. Do not fall back to beatmap's original ruleset if conversion fails #30424
    area:gameplay size/XS type:reliability
  4. Fix multiplayer song select not correctly applying filter sometimes #30425
    area:multiplayer area:song-select next release size/M
@bdach bdach added priority:0 Showstopper. Critical to the next release. type:online area:multiplayer labels Oct 24, 2024
@bdach bdach changed the title Multiplayer playlist items can be edited in ways that break ruleset eligibility Multiplayer playlist items can reportedly be edited in ways that break ruleset eligibility Oct 24, 2024
@LittleLilyBun
Copy link

Hi, I'm the reporter from that discord link, My current game version is 2024.1009.1-lazer and heres the logs:
compressed-logs.zip
To explain exactly what I'm doing, I'm hitting ctrl+1/2/3/4 immediately (like, within a frame or two) after clicking the edit button.

@chromb
Copy link

chromb commented Oct 24, 2024 •
edited
Loading

Here's a video of me reproducing it, hopefully useful:
https://github.com/user-attachments/assets/846229de-0b93-4e28-9ca1-7489809a1480
This should (hopefully) confirm it.

Confirmation that the scores do submit: https://osu.ppy.sh/scores/3736839340
They don't seem to give any pp though.

@shinmorisawa

This comment was marked as duplicate.

Sign in to view
@bdach bdach self-assigned this Oct 25, 2024
@bdach bdach changed the title Multiplayer playlist items can reportedly be edited in ways that break ruleset eligibility Multiplayer playlist items can be edited in ways that break ruleset eligibility Oct 25, 2024
bdach added a commit to bdach/osu-web that referenced this issue Oct 25, 2024
@bdach
Prevent starting play in multiplayer if playlist item is in incorrect…
db8c0d4 
… state

First layer of protection against ppy/osu#30415.
bdach added a commit to bdach/osu-web that referenced this issue Oct 25, 2024
@bdach
Prevent starting play in multiplayer if playlist item is in incorrect…
666b344 
… state

First layer of protection against ppy/osu#30415.
@bdach bdach mentioned this issue Oct 25, 2024
Merged
bdach added a commit to bdach/osu-server-spectator that referenced this issue Oct 25, 2024
@bdach
Fix add/edit playlist item operations not checking conversion eligibi…
615ceb0 
…lity

Second layer of protection against
ppy/osu#30415.
@bdach bdach mentioned this issue Oct 25, 2024
Merged
bdach added a commit to bdach/osu that referenced this issue Oct 25, 2024
@bdach
Fix multiplayer song select not correctly applying filter sometimes
4b5f4ee 
Fixes the root client-side failure causing
ppy#30415.

Thread of breakage is as follows:

1. `SongSelect` loads the carousel.
   At this point, the ruleset is what the ambient ruleset would have
   been at the time of pushing song select, so most likely it will
   match the current ruleset.
   Notably, the carousel is loaded with `AllowSelection == false`.
2. `OnlinePlaySongSelect` sets the ruleset to the one taken from
   the relevant playlist item in `LoadComplete()`.
3. At any point between the previous and the next step, the user
   changes the ruleset manually.
4. `SongSelect.carouselBeatmapsLoaded()` is ran, which calls
   `transferRulesetValue()`, which calls `FilterControl.FilterChanged`.
   But at this stage `Carousel.AllowSelection` is still false, so
   the filter is not executed, but `pendingFilterApplication` is set
   instead.
   Unfortunately, the pending filter never gets applied after that.
   The only place that checks that flag is `OnEntering()`, which at
   this point has already ran.

To fix, move the `pendingFilterApplication` check to `Update()`, which
seems like the most obvious and safe solution.
bdach added a commit to bdach/osu that referenced this issue Oct 25, 2024
@bdach
Fix multiplayer song select not correctly applying filter sometimes
66f608c 
Fixes the root client-side failure causing
ppy#30415.

Thread of breakage is as follows:

1. `SongSelect` loads the carousel.
   At this point, the ruleset is what the ambient ruleset would have
   been at the time of pushing song select, so most likely it will
   match the current ruleset.
   Notably, the carousel is loaded with `AllowSelection == false`.
2. `OnlinePlaySongSelect` sets the ruleset to the one taken from
   the relevant playlist item in `LoadComplete()`.
3. At any point between the previous and the next step, the user
   changes the ruleset manually.
4. `SongSelect.carouselBeatmapsLoaded()` is ran, which calls
   `transferRulesetValue()`, which calls `FilterControl.FilterChanged`.
   But at this stage `Carousel.AllowSelection` is still false, so
   the filter is not executed, but `pendingFilterApplication` is set
   instead.
   Unfortunately, the pending filter never gets applied after that.
   The only place that checks that flag is `OnEntering()`, which at
   this point has already ran.

To fix, move the `pendingFilterApplication` check to `Update()`, which
seems like the most obvious and safe solution.
bdach added a commit to bdach/osu that referenced this issue Oct 25, 2024
@bdach
Fix multiplayer song select not correctly applying filter sometimes
0cb48cc 
Fixes the root client-side failure causing
ppy#30415.

Thread of breakage is as follows:

1. `SongSelect` loads the carousel.
   At this point, the ruleset is what the ambient ruleset would have
   been at the time of pushing song select, so most likely it will
   match the current ruleset.
   Notably, the carousel is loaded with `AllowSelection == false`.
2. `OnlinePlaySongSelect` sets the ruleset to the one taken from
   the relevant playlist item in `LoadComplete()`.
3. At any point between the previous and the next step, the user
   changes the ruleset manually.
4. `SongSelect.carouselBeatmapsLoaded()` is ran, which calls
   `transferRulesetValue()`, which calls `FilterControl.FilterChanged`.
   But at this stage `Carousel.AllowSelection` is still false, so
   the filter is not executed, but `pendingFilterApplication` is set
   instead.
   Unfortunately, the pending filter never gets applied after that.
   The only place that checks that flag is `OnEntering()`, which at
   this point has already ran.

To fix, move the `pendingFilterApplication` check to `Update()`, which
seems like the most obvious and safe solution.
@bdach bdach mentioned this issue Oct 25, 2024
Merged
bdach added a commit to bdach/osu that referenced this issue Oct 25, 2024
@bdach
Fix multiplayer song select not correctly applying filter sometimes
b78e7d5 
Fixes the root client-side failure causing
ppy#30415.

Thread of breakage is as follows:

1. `SongSelect` loads the carousel.
   At this point, the ruleset is what the ambient ruleset would have
   been at the time of pushing song select, so most likely it will
   match the current ruleset.
   Notably, the carousel is loaded with `AllowSelection == false`.
2. `OnlinePlaySongSelect` sets the ruleset to the one taken from
   the relevant playlist item in `LoadComplete()`.
3. At any point between the previous and the next step, the user
   changes the ruleset manually.
4. `SongSelect.carouselBeatmapsLoaded()` is ran, which calls
   `transferRulesetValue()`, which calls `FilterControl.FilterChanged`.
   But at this stage `Carousel.AllowSelection` is still false, so
   the filter is not executed, but `pendingFilterApplication` is set
   instead.
   Unfortunately, the pending filter never gets applied after that.
   The only place that checks that flag is `OnEntering()`, which at
   this point has already ran.

To fix, move the `pendingFilterApplication` check to `Update()`, which
seems like the most obvious and safe solution.
@bdach bdach mentioned this issue Oct 25, 2024
Open
@ppy ppy deleted a comment Oct 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@bdach bdach

Labels
area:multiplayer priority:0 Showstopper. Critical to the next release. type:online
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@bdach @LittleLilyBun @chromb @shinmorisawa