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.

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

Force exit to main menu when presenting scores from within playlists / multiplayer #29182

Merged
merged 1 commit into from
Jul 30, 2024

Conversation

bdach
Copy link
Collaborator

@bdach bdach commented Jul 29, 2024

When testing I failed to predict that in multiplayer there can be a different beatmap in the playlist queue. If this is the case, PresentScore() will exit out to Multiplayer, whose RoomSubScreen will update the selected item - and thus, the global beatmap - to the next item in queue, at which point trying to play games with "not touching the global beatmap bindable if we don't need to" fail to work, because the bindable must be touched for correct operation, yet it cannot (because OnlinePlayScreens disable it).

I'm not sure what the fix is here:

  • making replay player somehow independent of the global beatmap?
  • not exiting out to multiplayer, but instead doing the present from the results screen itself? if so, then how to ensure the screen stack can't overflow to infinity?

so I'm just reverting the broken part. The daily challenge part is left in because as is it should not cause issues.

…/ multiplayer

- Closes ppy#29152
- Partially reverts ppy#29097
- Reopens ppy#26666

When testing I failed to predict that in multiplayer there can be a
different beatmap in the playlist queue. If this is the case,
`PresentScore()` will exit out to `Multiplayer`, whose `RoomSubScreen`
will update the selected item - and thus, the global beatmap - to the
next item in queue, at which point trying to play games with "not
touching the global beatmap bindable if we don't need to" fail to work,
because the bindable *must* be touched for correct operation, yet it
cannot (because `OnlinePlayScreen`s disable it).

I'm not sure what the fix is here:

- making replay player somehow independent of the global beatmap?
- not exiting out to multiplayer, but instead doing the present from the
  results screen itself? if so, then how to ensure the screen stack
  can't overflow to infinity?

so I'm just reverting the broken part. The daily challenge part is left
in because as is it should not cause issues.
@bdach bdach added area:multiplayer type:reliability area:playlists next release Pull requests which are almost there. We'll aim to get them in the next release, but no guarantees! labels Jul 29, 2024
@peppy
Copy link
Member

peppy commented Jul 30, 2024

Would probably be good to have test coverage on the failure scenario if that's easy to add.

As for path forward I'm not sure. The flow you described sounds overly complicated and I'd need some more time to go through and figure how we can simplify it.

@bdach
Copy link
Collaborator Author

bdach commented Jul 30, 2024

Would probably be good to have test coverage on the failure scenario if that's easy to add.

It is not. The score presenting flow is dependent on OsuGame (not OsuGameBase), which means I'd need a full game test that somehow enters multiplayer and either sets a score on its own or somehow finds an existing room with existing scores somewhere.

@peppy peppy merged commit d488490 into ppy:master Jul 30, 2024
13 checks passed
@bdach bdach deleted the revert-present-score branch July 30, 2024 08:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:multiplayer area:playlists next release Pull requests which are almost there. We'll aim to get them in the next release, but no guarantees! size/XS type:reliability
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Game crashes when presenting a replay from multiplayer results screen
2 participants