Add support for "freestyle" in multiplayer #11760

smoogipoo · 2024-12-24T10:51:36Z

This is an MVP. @ppy/team-web please take this over.

I've added a new column freestyle, which, when set, indicates that players in the room are able to individually select their own choice of beatmap and ruleset.

I've added some TODO comments in areas that need validation. Importantly:

Scores must be stored only with beatmaps from the same set as multiplayer_playlist_item.beatmap_id. (See here).
Ruleset ID must be valid for the beatmap (either matching osu_beatmap.playmode or 0-3 for when osu_beatmap.playmode == 0). (See here).
When "freestyle" mode is enabled, required_mods/allowed_mods should be kept empty for now.

Feature request: The score controller (rooms/{roomId}/playlist/{playlistItemId}/scores/{scoreId}) should return a beatmap model for each score rather than just a beatmap_id.

bdach · 2024-12-26T11:55:04Z

From a schema design standpoint, as far as I can see, beatmap_id is still mandatory on multiplayer_playlist_items regardless of whether beatmapset_id is present, and the current interpretation of it would be that it's always present even on "freestyle" playlist items? This feels backwards to me. I'd accept either:

exclusive-or (either beatmap_id present, or beatmapset_id present, the other being null),
making beatmap_id optional rather than beatmapset_id, which is to say if beatmap_id is null on an item then that is "freestyle", and if it's not, then that means forcing a particular beatmap of a particular set (this carries with it some degree of redundancy as the set and the beatmap must match, which is potentially a data integrity rake, but it's probably fine).

Similarly, ruleset_id is still nullable on the playlist item. This has further implications in client because it potentially means that "freestyle" rooms will only be accessible through the client via the "primary" ruleset of the playlist item (so you'll need to search for osu! lobbies to find a freestyle room).

Thoughts? I know this entire series was prefaced as "bare minimum" / "make-work", but my feeling is that at present this change on its own is going a bit too far in that regard.

peppy · 2024-12-26T15:30:23Z

exclusive-or (either beatmap_id present, or beatmapset_id present, the other being null),

i'd prefer this one as it's the most clear as to what's happening.

smoogipoo · 2024-12-26T15:50:14Z

One of the things I was going to do is exactly that, but as you might expect, the effects of doing so will run very deep. I'll leave it to the osu-web team to understand the full scope of what that will break, but be aware that it already guides what's shown in the lounge screen, the initial selection within the room, and also the playlist items in the queue which are incompatible with plain sets without an attached difficulty.

The flow as it currently is "just works", but it will have to change dramatically if users are to be required to set an appropriate difficulty for themselves.

Finally, I would have done that if song select allowed me to select sets. Doing that is a whole other can of worms, but the current state is such that song select must always display sets and difficulties, not one or the other.

peppy · 2025-01-04T17:52:24Z

Finally, I would have done that if song select allowed me to select sets. Doing that is a whole other can of worms, but the current state is such that song select must always display sets and difficulties, not one or the other.

As a compromise couldn't/aren't you just allowing a difficulty selection and pulling the set ID out? I'm having a bit of a hard time understanding how song select display limitations would affect server-side implementation.

smoogipoo · 2025-01-06T01:29:09Z

It looks very silly to select a difficulty when the difficulty isn't used. The current form is already what I'd call a compromise, but it gives some purpose to the difficulty that hides what is otherwise possibly an imperfect UX.

If you want a nullable beatmap_id then there's many more UI/UX considerations that are involved. For example:

Do you (as a guest in the room) want to select the difficulty every time?
Do we now integrate the difficulty recommender? I'm a high level player so I can play any difficulty I want, this would feel very bad for me, I just want to play what the host is picking and the fact that a room is "freestyle" should not complicate things for me.

I think having a default difficulty is good.

As I mentioned before, nothing really supports "sets". It's all based on whether you have the difficulty and of displaying the difficulty. This affects even internal components unseen to the user like OnlinePlayBeatmapAvailabilityTracker, which is based on whether the difficulty is available.

Changing all of this is a monumental task, and gIven the above, I strongly feel having it as it is is a better direction.

peppy · 2025-01-06T03:08:19Z

Hmm I see.. I can agree that having a default difficulty is probably going to make things easier to get this live (ie. it doesn't add another step where each user in the room needs to have a difficulty assigned, either by client or server) but it's also generally not how arcade games handle this (where difficulty selection is always explicit). But probably okay.

If we go with this direction, I think there needs to be a way to mark that "freestyle" is turned on for an item, and that mark shouldn't be whether beatmapset_id is null or not. A bool flag on multiplayer_playlist_items could suffice.

@bdach interested in your thoughts too

bdach · 2025-01-06T09:06:28Z

It looks very silly to select a difficulty when the difficulty isn't used. The current form is already what I'd call a compromise, but it gives some purpose to the difficulty that hides what is otherwise possibly an imperfect UX.

As far as this concern goes, I don't think it's such a huge deal even if it looks imperfect.

Do you (as a guest in the room) want to select the difficulty every time?

Do we now integrate the difficulty recommender? I'm a high level player so I can play any difficulty I want, this would feel very bad for me, I just want to play what the host is picking and the fact that a room is "freestyle" should not complicate things for me.

I admittedly hadn't considered that part of it. I guess it'd need to be either what you have here which is an explicit choice by the playlist creator, or using the recommender as you say to just pick something. To that end I'm now a bit more convinced by what this PR is doing.

I think having a default difficulty is good.

One concern I have is that the fact that the difficulty is default should probably somehow be conveyed to the user. I'm not sure how it would be done without annoying "tutorial" style popups, but the user should know what is happening when they pick a particular difficulty to avoid situations wherein someone thinks "hmm why should the difficulty matter" and then ends up picking a 1* slog nobody wants to play off of that logic alone.

There is also stuff client-side like this, wherein the choice of the default difficulty effectively constrains the "freestyle" because some difficulties cease to be selectable due to discrepancy in gameplay duration. It's probably an edge case all things considered, but to me it is a very non-obvious interaction - when watching the video you posted in the OP I was completely baffled as to why other users couldn't actually switch to another difficulty (and it is - as far as I can tell - exactly due to this duration filtering).

If we go with this direction, I think there needs to be a way to mark that "freestyle" is turned on for an item, and that mark shouldn't be whether beatmapset_id is null or not. A bool flag on multiplayer_playlist_items could suffice.

I tend to agree with this. If the beatmap id being present is intended, then the beatmapset id is half-redundant and exposes us to data integrity issues (think beatmapset id not pointing at the set that beatmap with that beatmap id belongs to). I'd prefer a boolean flag as well if we're going with explicit default selection.

smoogipoo · 2025-01-07T03:15:56Z

difficulties cease to be selectable ... watching the video you posted in the OP I was completely baffled as to why other users couldn't actually switch to another difficulty

I agree it's not the most intuitive interaction. Several options were brought up IRL:

Disable it host-side.
My retort to this is that it should always be the host's choice of what to play. If they couldn't play a beatmap they wanted to, they would simply turn off freestyle and guests wouldn't be able to choose other diffs that may exist and fall within the allowable criteria.
Show the items as disabled.
I'd really like to do this but the carousel prevents me from doing anything other than hiding/showing the items. This could be considered a work item in peppy's song select changes - making it easy to craft/design your own carousel.

Something brought up is that beatmaps these days apparently don't require all diffs to be of similar lengths, but outside of marathons/compilations I don't recall this being a thing and I hope it doesn't become a thing. Would like @peppy to confirm.

Maybe I've missed other options...

then the beatmapset id is half-redundant and exposes us to data integrity issues

Sure, I agree that this makes sense, and I'm not sure why I didn't go with that from the start. Will see how it goes and report back.

smoogipoo · 2025-01-08T10:18:06Z

I've applied modifications to use a freestyle flag instead of beatmapset_id to all relevant systems.

Still awaiting @ppy/team-web to handle the TODO items in this PR's changeset, which I'm not immediately sure of how to change.

nanaya · 2025-01-08T12:35:03Z

I'd start with fixing/refactoring the score token validation, moving ruleset and beatmap checksum validation to the score token model itself instead of in two places (their respective multiplayer and solo controllers)...

Ruleset id validation is done in ScoreToken.

Easier to understand.

nanaya · 2025-01-15T12:47:16Z

I think the base todo is done.

Feature request: The score controller (rooms/{roomId}/playlist/{playlistItemId}/scores/{scoreId}) should return a beatmap model for each score rather than just a beatmap_id.

This is currently a bit difficult due to how scores_around is populated 😬 (also it looks like potentially returning a lot of redundant data?)

also consider that the whole data can be obtained from single beatmapset endpoint request...

smoogipoo · 2025-01-16T04:38:58Z

Yeah, ignore that feature request. The client results screen flow isn't really structured to support that right now but makes sense to do in a separate request as you suggest.

smoogipoo · 2025-01-16T06:06:40Z

app/Models/Multiplayer/PlaylistItem.php

@@ -171,6 +171,10 @@ private function assertValidRuleset()

    private function assertValidMods()
    {
+        if ($this->freestyle && (count($this->allowed_mods) === 0 || count($this->required_mods) === 0)) {


Shouldn't these be !== 0?

yes indeed 🤨

smoogipoo · 2025-01-16T06:33:15Z

The client currently doesn't post a beatmap_id when creating the score - only beatmap_hash. While I can add it, it's not consistent and generally not to be trustable for online play, so I'd tend towards not adding it.

That said, after adding it (and applying a fix for the comment above), I created a room and upon trying to start a play received:

[network] 2025-01-16 06:24:51 [verbose]: Request to http://localhost:8080/api/v2/rooms/91/playlist/143/scores failed with System.Net.WebException: NotFound.
[network] 2025-01-16 06:24:51 [verbose]: Response was: {
[network] 2025-01-16 06:24:51 [verbose]: "message": "No query results for model [App\\Models\\Multiplayer\\PlaylistItem] 143",
[network] 2025-01-16 06:24:51 [verbose]: "exception": "Symfony\\Component\\HttpKernel\\Exception\\NotFoundHttpException",
[network] 2025-01-16 06:24:51 [verbose]: "file": "/app/vendor/laravel/framework/src/Illuminate/Foundation/Exceptions/Handler.php",
[network] 2025-01-16 06:24:51 [verbose]: "line": 487,
[network] 2025-01-16 06:24:51 [verbose]: "trace": [
[network] 2025-01-16 06:24:51 [verbose]: {
[network] 2025-01-16 06:24:51 [verbose]: "file": "/app/vendor/laravel/framework/src/Illuminate/Foundation/Exceptions/Handler.php",
[network] 2025-01-16 06:24:51 [verbose]: "line": 463,
[network] 2025-01-16 06:24:51 [verbose]: "function": "prepareException",
[network] 2025-01-16 06:24:51 [verbose]: "class": "Illuminate\\Foundation\\Exceptions\\Handler",
[network] 2025-01-16 06:24:51 [verbose]: "type": "->"
[network] 2025-01-16 06:24:51 [verbose]: },
[network] 2025-01-16 06:24:51 [verbose]: {
[network] 2025-01-16 06:24:51 [verbose]: "file": "/app/app/Exceptions/Handler.php",
[network] 2025-01-16 06:24:51 [verbose]: "line": 187,
[network] 2025-01-16 06:24:51 [verbose]: "function": "render",
[network] 2025-01-16 06:24:51 [verbose]: "class": "Illuminate\\Foundation\\Exceptions\\Handler",
[network] 2025-01-16 06:24:51 [verbose]: "type": "->"
[network] 2025-01-16 06:24:51 [verbose]: },
[network] 2025-01-16 06:24:51 [verbose]: {
[network] 2025-01-16 06:24:51 [verbose]: "file": "/app/vendor/laravel/framework/src/Illuminate/Routing/Pipeline.php",
[network] 2025-01-16 06:24:51 [verbose]: "line": 51,
[network] 2025-01-16 06:24:51 [verbose]: "function": "render",
[network] 2025-01-16 06:24:51 [verbose]: "class": "App\\Exceptions\\Handler",
[network] 2025-01-16 06:24:51 [verbose]: 
[network] 2025-01-16 06:24:51 [verbose]: (Response was trimmed)
[network] 2025-01-16 06:24:51 [verbose]: Failing request osu.Game.Online.Rooms.CreateRoomScoreRequest (System.Net.WebException: NotFound)

This is on a playlist item with freestyle = true but no user style - beatmap_id and beatmap_hash match the playlist item. It only happens with freestyle enabled, and the error is quite nondescript.

nanaya · 2025-01-16T06:38:07Z

the beatmap_id is in line with solo play token requirement though. I can change it for multiplayer if you think it's not needed here.

smoogipoo · 2025-01-16T06:41:36Z

Ah, I missed that. In that case we can keep it, though I'm not sure about the above error...

Do I need to change the request in any way? I noticed for solo scores it's part of the request URL: beatmaps/{beatmapInfo.OnlineID}/solo/scores, but in multiplayer I'm adding it as part of the form-data.

smoogipoo · 2025-01-16T06:47:52Z

Hmm, I'm not sure about the above error, different (better) one this time:

{
  "message" : "Specified beatmap_id is not allowed",
  "exception" : "App\\Exceptions\\InvariantException",
  "file" : "/app/app/Models/Multiplayer/Room.php",
  "line" : 767,
  "trace" : [ {
    "file" : "/app/app/Models/Multiplayer/Room.php",
    "line" : 677,
    "function" : "assertValidStartPlay",
    "class" : "App\\Models\\Multiplayer\\Room",
    "type" : "->"
  }, {
    "file" : "/app/app/Http/Controllers/Multiplayer/Rooms/Playlist/ScoresController.php",
    "line" : 185,
    "function" : "startPlay",
    "class" : "App\\Models\\Multiplayer\\Room",
    "type" : "->"
  }, {

nanaya · 2025-01-16T06:49:18Z

Ah, I missed that. In that case we can keep it, though I'm not sure about the above error...

Do I need to change the request in any way? I noticed for solo scores it's part of the request URL: beatmaps/{beatmapInfo.OnlineID}/solo/scores, but in multiplayer I'm adding it as part of the form-data.

no it's just different like that...

Hmm, I'm not sure about the above error, different (better) one this time:
...

oh yeah that was my bad 👀 pushed a fix

smoogipoo

All looks to work correctly now 👍

notbakaneko

migration can use a redate

notbakaneko · 2025-01-16T11:01:29Z

app/Models/Multiplayer/PlaylistItem.php

@@ -21,6 +21,7 @@
 * @property int $owner_id
 * @property int|null $playlist_order
 * @property json|null $required_mods
+ * @property bool $freestyle


needs to be added into $casts as well.

(unrelated to this, why is the mods cast of this object but DailyChallengeQueueItem array 🤔 )

smoogipoo added 2 commits December 10, 2024 15:36

Add beatmapset_id to multiplayer playlist items

5bacdc6

Allow multiplayer scores with arbitrary beatmap/ruleset combinations

68c2605

smoogipoo mentioned this pull request Dec 24, 2024

Add support for "freestyle" in multiplayer ppy/osu#31260

Open

2 tasks

bdach added the area:multiplayer label Dec 26, 2024

smoogipoo added 3 commits January 8, 2025 15:37

Modify to boolean 'freestyle' parameter instead

fa57845

Require strict types + add license header

4699f50

Fix inspection

2ecb1f0

nanaya added 5 commits January 15, 2025 21:20

Merge remote-tracking branch 'origin/master' into HEAD

487a3fc

Readd beatmap_id validation

87db8f4

Ruleset id validation is done in ScoreToken.

Disallow mod on freestyle

0d393a8

Lint fix

adecc03

No smarts allowed

ac1b77b

Easier to understand.

nanaya requested a review from notbakaneko January 15, 2025 12:43

smoogipoo commented Jan 16, 2025

View reviewed changes

Fix incorrect check and short circuit if passed

2b05d09

Wrong id

6f053b3

smoogipoo commented Jan 16, 2025

View reviewed changes

Merge branch 'master' into multiplayer-free-style

5753857

notbakaneko requested changes Jan 16, 2025

View reviewed changes

nanaya added 2 commits January 16, 2025 20:52

Correctly cast the field

72baade

Redate migration

6e84fed

notbakaneko approved these changes Jan 16, 2025

View reviewed changes

notbakaneko merged commit 8c2ced6 into ppy:master Jan 16, 2025
3 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for "freestyle" in multiplayer #11760

Add support for "freestyle" in multiplayer #11760

smoogipoo commented Dec 24, 2024 •

edited

Loading

bdach commented Dec 26, 2024 •

edited

Loading

peppy commented Dec 26, 2024

smoogipoo commented Dec 26, 2024 •

edited

Loading

peppy commented Jan 4, 2025

smoogipoo commented Jan 6, 2025

peppy commented Jan 6, 2025

bdach commented Jan 6, 2025

smoogipoo commented Jan 7, 2025 •

edited

Loading

smoogipoo commented Jan 8, 2025 •

edited

Loading

nanaya commented Jan 8, 2025

nanaya commented Jan 15, 2025

smoogipoo commented Jan 16, 2025

smoogipoo Jan 16, 2025

nanaya Jan 16, 2025

smoogipoo commented Jan 16, 2025

nanaya commented Jan 16, 2025 •

edited

Loading

smoogipoo commented Jan 16, 2025

smoogipoo commented Jan 16, 2025

nanaya commented Jan 16, 2025

smoogipoo left a comment

notbakaneko left a comment

notbakaneko Jan 16, 2025

Add support for "freestyle" in multiplayer #11760

Add support for "freestyle" in multiplayer #11760

Conversation

smoogipoo commented Dec 24, 2024 • edited Loading

bdach commented Dec 26, 2024 • edited Loading

peppy commented Dec 26, 2024

smoogipoo commented Dec 26, 2024 • edited Loading

peppy commented Jan 4, 2025

smoogipoo commented Jan 6, 2025

peppy commented Jan 6, 2025

bdach commented Jan 6, 2025

smoogipoo commented Jan 7, 2025 • edited Loading

smoogipoo commented Jan 8, 2025 • edited Loading

nanaya commented Jan 8, 2025

nanaya commented Jan 15, 2025

smoogipoo commented Jan 16, 2025

smoogipoo Jan 16, 2025

Choose a reason for hiding this comment

nanaya Jan 16, 2025

Choose a reason for hiding this comment

smoogipoo commented Jan 16, 2025

nanaya commented Jan 16, 2025 • edited Loading

smoogipoo commented Jan 16, 2025

smoogipoo commented Jan 16, 2025

nanaya commented Jan 16, 2025

smoogipoo left a comment

Choose a reason for hiding this comment

notbakaneko left a comment

Choose a reason for hiding this comment

notbakaneko Jan 16, 2025

Choose a reason for hiding this comment

smoogipoo commented Dec 24, 2024 •

edited

Loading

bdach commented Dec 26, 2024 •

edited

Loading

smoogipoo commented Dec 26, 2024 •

edited

Loading

smoogipoo commented Jan 7, 2025 •

edited

Loading

smoogipoo commented Jan 8, 2025 •

edited

Loading

nanaya commented Jan 16, 2025 •

edited

Loading