ppy / osu-performance

Calculates user performance aggregates from scores
GNU Affero General Public License v3.0
241 stars 45 forks source link

osu!mania pp calc : scoreMultiplier ignoring NF mutliplier seems contradictory #123

Open Zyfarok opened 4 years ago

Zyfarok commented 4 years ago

Facts

The scoreMultiplier retrieved here considers all mods multipliers except NF (because NF is "masked out") : https://github.com/ppy/osu-performance/blob/master/src/performance/mania/ManiaScore.cpp#L70

This makes all NF scores still have a score value bellow 500k after removing all (other) mods multipliers, which makes the pp value be always 0 because of the "mashing counter" part.

Contradiction

This seems contradictory with the fact that a multiplier is applied to the final pp value of NF scores here : https://github.com/ppy/osu-performance/blob/master/src/performance/mania/ManiaScore.cpp#L51-L52

Why would you have a specific case for NF plays where you multiply it's pp by 0.9x if the base pp is always 0 ? Additionally, I don't think the function to retrieve the score multiplier was intentionally modified to ignore NF and make NF scores gives 0 pp since it would be much easier and clearer to just change this 0.9x multiplier into a 0 otherwise.

Code

MaskRelevantDifficultyMods is the function that makes the NF mod ignored and is used in Beatmap::DifficultyAttribute here : https://github.com/ppy/osu-performance/blob/master/src/performance/Beatmap.cpp#L22-L24

and defined here : https://github.com/ppy/osu-performance/blob/92b3eaf832f79eb3e0731c4ce75a8944a2e7b48f/include/pp/Common.h#L98-L100

Supposed source of error

I suppose the error comes from the fact that the MaskRelevantDifficultyMods function used in Beatmap::DifficultyAttribute was originally made to only extract mods that affect the difficulty attributes considered in osu!std which are Speed/Aim/OD/AR (which are not affected by NF) and the possibility to retrieve the ScoreMultiplier from Beatmap::DifficultyAttribute was added afterward specifically for mania pp calculation without noticing NF was ignored.

Proposed fix

Either : 1- add NF to the masked mods (but then you'd have to make the _difficulty map twice bigger, even for other gamemodes) OR 2- make different masks for different tasks depending on the value you retrieve and/or the different gamemodes (since this code is used in all gamemodes pp calculation) which can also help making the _difficulty map smaller.

Then, once that is fixed and that NF plays actually give pp again, we might want to re-adjust the NF pp multiplier to avoid abuse as I heard that NF once gave too much PP in some situations ? Apparently this was a misunderstanding, the currently defined 0.9 multiplier added to the mashing counter should be sufficient.

smoogipoo commented 4 years ago

This requires the osu!lazer mania diffcalc to be finished.