Add ability to change multiplayer queue modes #15640
Conversation
|
Renames have been applied. |
|
Tests no look good. |
|
I've skimmed this again (without testing) and I'm all good with the structure, so on that part I see no reason not to go forward. I may test later, but even if I find issues, I'm likely going to just open issues about it or fix myself post-merge because this diff is too large to hold up over bugs. As for the test failures, I have a few commits that seem to fix them on this branch (first, second, third - reasonings for each change are in the extended commit message). They seem fairly innocuous but I'm not 100% about them, so I didn't push them in outright; can do so if they are considered acceptable. |
|
Actually, on comparing this pull and the server-side pull, I believe there may be a discrepancy between the test multiplayer client implementation and the real one as to how playlist items are treated when the queue mode changes, so that may need to be addressed before this can be merged as it is potentially a pretty fundamental change in UX. |
|
Tests should be resolved now.
Indeed, fixed via 6420971. In general, the reason this was done is to not rely on the schedule having run for the server-side playlist to be up-to-date. I believe your third change (f7829cc) may prove problematic if you add a In my changes, I've instead stored a "server-side" playlist. 10dc08a I think it's better to compare via IDs in these cases, as I've done in 53dbbd6. |
|
I've reviewed this about as much as I can given its size. Let's get this merged and push forward. I'll do a (nuget-only) release post-merge to get the server side up-to-date and deployed on the dev server. |
Preview: https://drive.google.com/file/d/1ZnrHxGMBNJu9vFtb1-A0fR2To2a0cdGY/view?usp=sharing
Three new modes arrive in multiplayer:
Prereqs (osu and osu-server-spectator are inter-dependent):
I apologise for the large PR - it's an entirely breaking change so I have to go the full way through.
Multiplayer is restructured so that:
MultiplayerRoomSettingsno longer contains the current map/ruleset/mods. These are now expected to be taken from thePlaylistItem.MultiplayerRoomSettingsstill provides the playlist item id.AddPlaylistItem()server method.Both of the above are breaking changes.
The
MultiplayerPlaylistItemclass has been added in order to communicate playlist items with the server. I've left in aUserIdproperty, which is to be used for the upcoming fair-play mode, though initial usages will be server-side only.BeatmapSelectionControlno longer exists, as it's too complicated to make it work for all usages. Instead, the playlist has been inlined:MultiplayerMatchSubScreen, containing the full playlist.MultiplayerMatchSettingsOverlay, containing only a single item, and reproducing the previousBeatmapSelectionControlfunctionality. This means that the match has to be created with a single playlist item for the time being.The playlist has been inverted in
MultiplayerMatchSubScreenfor the time being. It's easier to handle this way without working around scroll, as the playlist length expands out of view.Still to be done:
MultiplayerMatchSubScreenplaylist into two sections, maybe something like Apple Music (history sorted newest-first, and up-next sorted oldest-first).As for testing - everything except fair-play mode is testable via visual tests (
TestSceneMultiplayer) to get a feel for things.