Add command for recalculating mod multipliers#269
Conversation
|
I'll look into the test failures, but a thought that has just occurred to me is that this PR might be going too far. By which I mean we may want to skip recalculating mod multipliers on leaderboards with significance - I'm thinking multiplayer, playlists, and daily challenge. Which is to say, any multiplayer score would be excluded from having its multiplier recalculated (but also would need to become unranked in the process, which also has pp implications). Thoughts? |
|
@peppy could I ask that this change is looked at some point soon? With the recent creation of https://discord.com/channels/546120878908506119/1373971097434980463 and conversation happening there, I'm at a point where I'm not sure how exactly we're supposed to go ahead publicly if there's no framework to actually facilitate any changes that may arise from polling. Changing at least the classic multiplier is quite high priority in my opinion. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
bdach
left a comment
bdach
left a comment
There was a problem hiding this comment.
Just noting current known deficiencies / blockers after 2 years of not seeing this diff.
Will want to re-run with the new multiplier API at least locally and re-check that this is performing within expected parameters.
| score.beatmap = beatmapsById[score.beatmap_id]; | ||
| var scoreInfo = score.ToScoreInfo(); | ||
|
|
||
| if (scoreInfo.TotalScoreWithoutMods == 0 && scoreInfo.TotalScore != 0) | ||
| { | ||
| throw new InvalidOperationException($"Score with ID {score.id} has {scoreInfo.TotalScore} total score but {scoreInfo.TotalScoreWithoutMods} total score without mods. " | ||
| + $"This is likely to indicate that {nameof(scoreInfo.TotalScoreWithoutMods)} was not correctly backpopulated on all scores " | ||
| + "(or there is a process pushing new scores that was not updated to populate the field)."); | ||
| } |
There was a problem hiding this comment.
This will likely need further adjusting:
- Will die on missing beatmap in ways similar to what Fix
total_score_without_modspopulation failing if beatmap was deleted #371 was fixing - The hard throw on zero total score without mods but nonzero total score will likely need to be changed to a soft skip or something. @peppy not sure what sort of visibility you want on skips here (just log count of skipped rows?)
There was a problem hiding this comment.
To recap, we're currently populating the majority of scores, but there could be some which don't get the population due to deleted/missing beatmaps or other edge case issues.
I think skipping here with log output is fine for such scores.
| double multiplier = 1; | ||
|
|
||
| foreach (var mod in scoreInfo.Mods) | ||
| multiplier *= mod.ScoreMultiplier; |
There was a problem hiding this comment.
Depends on client changes (the new multipliers actually being present in client source), and will require upgrading in line with the new API
4 participants
total_score_without_modsin the json.