Add initial support for filtering by user tags in song select #34231
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The way that this works is that it plugs into the online request to retrieve the beatmap set that the client is already performing, and stores user tag data to the local realm database. This means that for now user tags will only populate for beatmaps that the user has displayed on song select which is obviously subpar. I plan to follow this change up by adding user tag state dumps to `online.db` and using that data for initial tag population to make the majority case (ranked beatmaps) work. Note that several decisions were made here that are potential discussion points: - `RealmPopulatingOnlineLookupSource` is set up such that it can be the middle man / redirection point for similar flows that we need and we are currently missing, such as storing guest difficulty information, or storing the user's current best score on a beatmap (handy for rank achieved sorting / filtering / etc.) - The user tags are stored in `BeatmapMetadata` which breaks the longstanding assumption that you can arbitrarily pull out a metadata instance from any of the beatmaps in a set and get essentially the same object back. I've attempted to constrain this some by not adding user tags to the `IBeatmapMetadataInfo` interface through which `BeatmapSetInfo` exposes metadata further, but I warn in advance that this is a temporary state of affairs and I will make it worse in the future when `BeatmapMetadata.Author` becomes `Authors` plural in order to support guest mapper display (and direct guest difficulty submission). - The syntax for searching via user tags is chosen to mostly match web - it's `tag=`, with support for all of the string matching modes song select already has (bare word for substring, `""` quotes for phrase isolated by whitespace, `""!` for exact full match).
…over So much passing of the `linkAction` to only then give up halfway through and reimplement it locally again down in the overflow popover. This materially matters now because mapper tags are searched as plain words and user tags are searched using the `tag=""!` syntax.
bdach
added
area:online functionality
peppy
reviewed
Jul 20, 2025
| /// 50 2025-07-11 Add UserTags to BeatmapMetadata. | ||
| /// </summary> | ||
| private const int schema_version = 49; | ||
| private const int schema_version = 50; |
Member
There was a problem hiding this comment.
Notably, this will cause version incompatibility when deployed to tachyon, so not sure we want to deploy immediately.
peppy
reviewed
Jul 20, 2025
| /// <item>the local user's best score on a given beatmap.</item> | ||
| /// </list> | ||
| /// </example> | ||
| public partial class RealmPopulatingOnlineLookupSource : Component |
Member
There was a problem hiding this comment.
I think my biggest issue with this PR is the naming of this class.
I think I'd prefer the "realm populating" part was a hidden implementation detail rather than loudly stated in the naming. But maybe you feel differently so I'd like to hear your thoughts.
I would see this eventually being instantiated at a game level and used elsewhere for any online lookups, for instance.
Collaborator
Author
There was a problem hiding this comment.
The reason I want this to say somewhere that it's populating realm is that if it doesn't then it's a rather non-obvious side effect. To the point that if I were to see a plainly-named online lookup class and see it write stuff to realm I would have a lot of questions.
Member
|
Going to get this in. Naming concerns can be revisited when usage of this expands. |
peppy
approved these changes
Jul 21, 2025
This was referenced Aug 27, 2025
This was referenced Sep 5, 2025
This was referenced Oct 3, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
RFC
Concludes #32568 (comment)
The way that this works is that it plugs into the online request to retrieve the beatmap set that the client is already performing, and stores user tag data to the local realm database.
This means that for now user tags will only populate for beatmaps that the user has displayed on song select which is obviously subpar. I plan to follow this change up by adding user tag state dumps to
online.dband using that data for initial tag population to make the majority case (ranked beatmaps) work.Note that several decisions were made here that are potential discussion points:
RealmPopulatingOnlineLookupSourceis set up such that it can be the middle man / redirection point for similar flows that we need and we are currently missing, such as storing guest difficulty information, or storing the user's current best score on a beatmap (handy for rank achieved sorting / filtering / etc.)The user tags are stored in
BeatmapMetadatawhich breaks the longstanding assumption that you can arbitrarily pull out a metadata instance from any of the beatmaps in a set and get essentially the same object back.I've attempted to constrain this some by not adding user tags to the
IBeatmapMetadataInfointerface through whichBeatmapSetInfoexposes metadata further, but I warn in advance that this is a temporary state of affairs and I will make it worse in the future whenBeatmapMetadata.AuthorbecomesAuthorsplural in order to support guest mapper display (and direct guest difficulty submission).Therefore this is the decision point as to whether we want that sort of data in
BeatmapMetadataor inBeatmapInfo.The syntax for searching via user tags is chosen to mostly match web - it's
tag=, with support for all of the string matching modes song select already has (bare word for substring,""quotes for phrase isolated by whitespace,""!for exact full match).Note that this contains a realm migration so if you've ever ran #34058 you'll need to nuke
client_50.realmto run this.