Skip to content
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

Convert legacy total score to standardised when importing high scores #135

Merged
merged 5 commits into from
Aug 1, 2023

Conversation

smoogipoo
Copy link
Contributor

@smoogipoo smoogipoo commented Jun 28, 2023

Prereqs:

One questionable/important aspect is that we don't store difficulty attributes for all mod combinations. For example, for +HDHR, we only store the difficulty attributes for +HR. Because of this, there's an extra step here that re-multiplies the LegacyComboScore with the correct mod multiplier.

This could result in an off-by-one score for each hitobject because legacy scoring truncates at every judgement, that adds up over time. For example, chocomint's score on FREEDOM DiVE [FOUR DIMENSIONS] converts to 1116538 when imported via this process, but 1116540 when imported directly from replay into lazer.

I've taken the liberty to treat this small discrepancy as acceptable.


Required database attributes:

INSERT INTO osu_difficulty_attribs (attrib_id, name) VALUES (23, 'Legacy Accuracy Score'), (25, 'Legacy Combo Score'), (27, 'Legacy Bonus Score Ratio');

We must re-run the difficulty calculator across all ranked beatmaps on all rulesets prior to any new import, with the osu-side changes applied.

Comment on lines -511 to -520
case 0:
case 1:
case 2:
// In these rulesets, every mod combination has the same max combo. So use the attribute with mods=0.
break;

case 3:
// In mania, only keymods affect the number of objects and thus the max combo.
difficultyMods = highScore.enabled_mods & (int)LegacyModsHelper.KEY_MODS;
break;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't really know why this switch existed. The max combo appears to be identical for non-keymod mania difficulties too.

https://github.com/ppy/osu/blob/1d4380cfd0f7b91df58cc1342998bec0d6654420/osu.Game.Rulesets.Mania/Difficulty/ManiaDifficultyCalculator.cs#L60-L66

I believe this may have been legacy code...

@bdach
Copy link
Collaborator

bdach commented Jul 19, 2023

Just for reference, as far as I can tell, this will also fix an issue reported on discord today about realtime imports of stable scores not having mod multipliers applied. Current master does not include mods when computing total score from reference, while this pull will do, because game-side StandardisedScoreMigrationTools do.

@smoogipoo
Copy link
Contributor Author

@peppy I want to test SR/PP changes ppy/osu#22613 and ppy/osu#20963 and I need this PR + ppy/osu-difficulty-calculator#221 merged otherwise it gets messy dealing with multiple working trees.

@@ -58,7 +57,6 @@ public async Task<int> OnExecuteAsync(CancellationToken cancellationToken)
foreach (var score in scores)
{
bool requiresUpdate = ensureMaximumStatistics(score);
requiresUpdate |= ensureCorrectTotalScore(score);
Copy link
Member

Choose a reason for hiding this comment

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

Does this command even make sense anymore?

@peppy
Copy link
Member

peppy commented Aug 1, 2023

Haven't tested running this, but I think we can move forward for now.

@peppy peppy merged commit 208ee81 into ppy:master Aug 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants