Disallow classic mod from giving PP (for now) #180
Merged
Conversation
This file contains 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
bdach
reviewed
Dec 13, 2023
| @@ -191,6 +191,10 @@ public static bool AllModsValidForPerformance(Mod[] mods) | |||
|
|
|||
| continue; | |||
|
|
|||
| case ModClassic: | |||
| // Classic mod is only allowed if it's attached to legacy scores (null build ID). | |||
| return score.BuildID == null; | |||
There was a problem hiding this comment.
Isn't examining that LegacyScoreID is not null the standard / canonical way to check if a SoloScoreInfo is legacy or not? I worry about introducing multiple heuristics for the same thing.
There was a problem hiding this comment.
There was a problem hiding this comment.
Just for context, I'm referring to
as places that use the other method. If there is some material difference I'm not aware of then maybe that's fine.
There was a problem hiding this comment.
Will wait on a new osu! nuget package before fixing this.
smoogipoo
force-pushed
the
disallow-classic-mod-ranking
branch
from
ce908c2 to
798c825
Compare
With the introduced logic rejecting scores with a null build ID from being processed, one of the tests began failing due to having a null build ID. Fixing this while preserving that added logic (which seems prudent) is however annoying as it requires a build to exist in the database with `allow_performance` set to 1.
|
I have attempted to fix test failures here but I am not sure everyone will agree as to what I did, so please scrutinise b9b51ac especially at your discretion. Rest seems fine to me. |
bdach
approved these changes
Dec 13, 2023
|
Looks fine to me |
Having it as a commented check makes me nervous about every other case which is using the `IsLegacyScore` flag, so let's just commit to this.
peppy
approved these changes
Dec 14, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Since we're not sure yet on the implementation of the mechanics of this mod (e.g. ppy/osu#11769), it's easiest to just not allow it to be ranked for now. Scores coming from stable are still allowed to give PP.