-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
feat: Add an accuracy filter to the beatmaps carousel search bar #35314
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
base: master
Are you sure you want to change the base?
Conversation
Having '<' include maps with no scores feels like weird UX imo, especially since there's already an explicit played=1/0 filter for played/unplayed maps. |
|
||
public Func<List<BeatmapCollection>> AllCollections { get; set; } = () => []; | ||
public Func<FilterCriteria, Dictionary<Guid, ScoreRank>> BeatmapInfoGuidToTopRankMapping { get; set; } = _ => new Dictionary<Guid, ScoreRank>(); | ||
public Func<FilterCriteria, Dictionary<Guid, double>> BeatmapInfoGuidToTopRankAccuracyMapping { get; set; } = _ => new Dictionary<Guid, double>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am incredibly reluctant to even approach this PR for two reasons:
- This PR will be immediately and inevitably followed by requests to filter by total score, pp, count of GREATs/GOODs, FCs, etc.
- This approach of spamming dictionaries for every user-score-related filter does not feel like it will scale.
So this PR is opening a whole swath of scope creep that I do not want to spend time burrowed in while other, possibly more important, stuff still needs doing.
A starting point that could maybe persuade me to consider this change is to limit the number of beatmap-info-to-score-data dictionaries to one. The value in this case would be an object containing the best rank, accuracy, pp, etc. achieved on the beatmap (...like ScoreInfo
even? depends on how this filter should work - see other review comment below)
Note that this may still result in prohibitively bad performance in the long run, but at least code-quality wise it's a start.
|
||
var allLocalScores = r.GetAllLocalScoresForUser(criteria.LocalUserId) | ||
.Filter($@"{nameof(ScoreInfo.Ruleset)}.{nameof(RulesetInfo.ShortName)} == $0", criteria.Ruleset?.ShortName) | ||
.OrderByDescending(s => s.TotalScore) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh and also this is a particular sticking point. This makes this filter based on "accuracy achieved on user's top score as determined by total score" not "user's best accuracy ever on this map". You remarked on that in the OP, but my question would be - is that what users want? I dunno. Arguable? Maybe they want to be able to do both?
Like the local leaderboards support sorting in the second interpretation of the phrase here, so I feel like this will be confusing to understand for users.
This PR adds an accuracy filter on the beatmaps carousel search bar.
This filter allows players to filter the beatmaps based on the accuracy they've achieved during their best score.
This filter recognizes the keywords
acc
andaccuracy
, and the value can be described in several formats :acc>=90.97
corresponds to90.97%
accuracyaccuracy<=95.84%
corresponds to95.84%
accuracyacc>0.90
corresponds to90%
accuracyBeatmaps that have not yet been played are considered to have an accuracy of 0%, so a player searching for
acc<x
oracc<=x
will still see unplayed maps.The best scores achieved by the offline ("Guest") player are taken into account when filtering.
Example
With a filter of
acc>96%
, we obtain the only beatmap where the accuracy of the best score is greater than “96%”:With a filter
acc>91
, we only get the two beatmaps where the user has an accuracy greater than 91%:Same goes for
accuracy>0.9
, since it's only the value format that changes :With a "less than" filter, such as
accuracy<91
, we only get beatmaps with a maximum accuracy of less than 91%, as well as beatmaps that have not yet been played :