Skip to content

Allow beatmap tagging if at least one score is eligible #36992

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.

Sign up for GitHub

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

Open
imvanni wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Allow beatmap tagging if at least one score is eligible #36992

Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
6a1beaa
Allow beatmap tagging if any score is eligible
imvanni Mar 15, 2026
fa91d23
Merge branch 'master' into tagging-use-available-score
bdach Mar 19, 2026
d3fcffe
Simplify prevent tagging reason check
imvanni Mar 19, 2026
73faf88
Add missing tests
imvanni Mar 19, 2026
554c333
Fix indentation
imvanni Mar 19, 2026
ccf8d44
Order test scores with Rank instead
imvanni Mar 19, 2026
f12881c
Fix else if statement
imvanni Mar 20, 2026
34ddcc7
Apply sorting corrections
imvanni Mar 23, 2026
1678f7a
Revert using a method to check tagging prevention reason
imvanni Mar 23, 2026
b05c2af
Add braces for else block
imvanni Mar 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 21 additions & 7 deletions osu.Game/Screens/Ranking/Statistics/StatisticsPanel.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
// See the LICENCE file in the repository root for full licence text.

using System;
using System.Collections.Generic;
using System.Linq;
using System.Threading;
Expand Down Expand Up @@ -245,13 +246,26 @@ protected virtual IEnumerable<StatisticItem> CreateStatisticItems(ScoreInfo newS

// We may want to iterate on the following conditions further in the future

var localUserScore = AchievedScore ?? realm.Run(r =>
r.GetAllLocalScoresForUser(api.LocalUser.Value.Id)
.Filter($@"{nameof(ScoreInfo.BeatmapInfo)}.{nameof(BeatmapInfo.ID)} == $0", newScore.BeatmapInfo.ID)
.AsEnumerable()
.OrderByDescending(score => score.Ruleset.MatchesOnlineID(newScore.BeatmapInfo.Ruleset))
.ThenByDescending(score => score.Rank)
.FirstOrDefault());
var localUserScore = AchievedScore;

if (localUserScore == null)
Copy link
Copy Markdown
Collaborator

@bdach bdach Mar 19, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure this makes sense anymore? If the point is to allow tagging if any local score matches, then there's little reason to still prefer the latest set score if the user has already proven themselves capable to tag.

{
var localUserScores = realm.Run(r =>
r.GetAllLocalScoresForUser(api.LocalUser.Value.Id)
.Filter($@"{nameof(ScoreInfo.BeatmapInfo)}.{nameof(BeatmapInfo.ID)} == $0", newScore.BeatmapInfo.ID)
.AsEnumerable()
.OrderByDescending(score => score.Ruleset.MatchesOnlineID(newScore.BeatmapInfo.Ruleset))
.ThenByDescending(score => score.Rank)
.ToArray());

localUserScore = Array.Find(localUserScores, score =>
{
return (score.Ruleset.OnlineID == newScore.BeatmapInfo!.Ruleset.OnlineID)
&& (score.Rank >= ScoreRank.C)
&& (!score.Mods.Any(m => (m.Type == ModType.Conversion)
|| m is not ModClassic));
Copy link
Copy Markdown
Collaborator

@bdach bdach Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not even sure this is correctly transcribed from code lower down. m.Type == ModTypeConversion || m is not ModClassic is not the same thing as (m.Type == ModType.Conversion) && !(m is ModClassic).

This should either be && in both places or the first should be correctly negated as per generalized De Morgan's laws.

In general there's kind of little reason why this needs to be duplicated like this. I imagine that you could split a method like getReasonForPreventingTagging(ScoreInfo) and then use it here instead in a getReasonForPreventingTagging(score) == null manner.

}) ?? localUserScores.FirstOrDefault();
}

if (localUserScore == null)
preventTaggingReason = "Play the beatmap to contribute to beatmap tags!";
Expand Down
Loading