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

Remove estimations where score data is available for osu! difficulty calculations #27691

Merged
merged 41 commits into from
Oct 22, 2024
Merged
Changes from all commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
941c048
Make length bonus account for sliders, use proper misscount for classic
Finadoggie Mar 22, 2024
4db6f28
Use actual sliderends dropped instead of estimating
Finadoggie Mar 22, 2024
3dafdc0
Revert "Make length bonus account for sliders, use proper misscount f…
Finadoggie Mar 22, 2024
8408455
Use miss count for effective miss count
Finadoggie Mar 22, 2024
12afa8d
Merge pull request #1 from Finadoggie/miss-count-fix
Finadoggie Mar 22, 2024
eb30b4a
Merge branch 'estimation-removal' into dropped-tail-fix
Finadoggie Mar 22, 2024
c9e3c10
Merge pull request #2 from Finadoggie/dropped-tail-fix
Finadoggie Mar 22, 2024
b0d20e6
Update OsuPerformanceCalculator.cs
Finadoggie Mar 22, 2024
6fe478c
Add slider ticks and reverse arrows to effective misscount
Finadoggie Mar 22, 2024
4f5f0e5
Merge branch 'master' into estimation-removal
Finadoggie Mar 23, 2024
58bc184
Use sliderend data for all non-legacy scores
Finadoggie Mar 23, 2024
c24f99e
Merge branch 'estimation-removal' of https://github.com/Finadoggie/os…
Finadoggie Mar 23, 2024
2dd4903
Cap Buzz Slider Related Misses
Finadoggie Apr 11, 2024
dd17c89
removed large tick misses from effectivemisscount
Finadoggie Apr 12, 2024
ca24601
Add bool useSliderHead
Finadoggie Apr 19, 2024
77814ec
Fix getting slider head drops
Finadoggie Apr 19, 2024
759a826
Clamp estimatedSliderEndsDrop
Finadoggie Apr 19, 2024
4a7b813
Re-add bool useSliderHead
Finadoggie Apr 19, 2024
d1dcac0
fix code formatting
Finadoggie Apr 19, 2024
4fe55d4
Renamed useSliderHead to useClassicSlider (and refactored code accord…
Finadoggie Apr 20, 2024
1f55c14
merged givi's accuracy changes
Finadoggie May 24, 2024
6c9e906
Revert "merged givi's accuracy changes"
Finadoggie May 24, 2024
8dea601
Merge branch 'master' into estimation-removal
Finadoggie May 25, 2024
44c9425
Merge branch 'ppy:master' into estimation-removal
Finadoggie Sep 27, 2024
3d7f4ae
Merge branch 'ppy:master' into estimation-removal
Finadoggie Oct 12, 2024
b921424
Update to use variable usingClassicSliderAccuracy
Finadoggie Oct 12, 2024
3b517e0
Convert estimateSliderEndsDropped assignment into '?:' expression
Finadoggie Oct 12, 2024
3ac6a9f
Join variable assignments with declarations
Finadoggie Oct 12, 2024
29b1697
consolidated if statements for getting effectiveMissCount and countSl…
Finadoggie Oct 12, 2024
88af578
only assign countLargeTickMiss for slider accuracy scores
Finadoggie Oct 12, 2024
5192599
remove score debugging code I accidentally left in
Finadoggie Oct 12, 2024
6bcfed8
Revert "remove score debugging code I accidentally left in"
Finadoggie Oct 12, 2024
1337b7e
use LargeTickHit instead of LargeTickMiss
Finadoggie Oct 17, 2024
6d4cb60
Revert "use LargeTickHit instead of LargeTickMiss"
Finadoggie Oct 19, 2024
31e0853
add large tick misses back into effectivemisscount
Finadoggie Oct 21, 2024
e31e10d
merge effectivemisscount functions
Finadoggie Oct 21, 2024
3778246
Addressed code quality concerns
Finadoggie Oct 21, 2024
5907c2a
Only clamp estimated miss count with relevant statistics
Finadoggie Oct 21, 2024
98800fe
Fix variables being used before being assigned
Finadoggie Oct 21, 2024
bcb9970
Refactor and add comments
stanriders Oct 21, 2024
acf282d
Fix effectiveMissCount being calculated wrong
stanriders Oct 21, 2024
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
81 changes: 60 additions & 21 deletions osu.Game.Rulesets.Osu/Difficulty/OsuPerformanceCalculator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,19 @@ public class OsuPerformanceCalculator : PerformanceCalculator
private int countMeh;
private int countMiss;

/// <summary>
/// Missed slider ticks that includes missed reverse arrows. Will only be correct on non-classic scores
/// </summary>
private int countSliderTickMiss;
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like renaming this to countLargeTickMiss makes it alot clearer what it is because despite the comment reading it anywhere else in code could put up confusion... I don't see why it shouldn't be named 1:1 after the hit result it represents.

Copy link
Member

Choose a reason for hiding this comment

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

"LargeTick" does not represent anything on its own unless you're aware of what it actually contains

Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like it's still a more meaningful term since it matches the hit result and also the "Large Tick Misses" field I added to osu-tools GUI and soon to CLI.


/// <summary>
/// Amount of missed slider tails that don't break combo. Will only be correct on non-classic scores
/// </summary>
private int countSliderEndsDropped;
Copy link
Contributor

Choose a reason for hiding this comment

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

Since slider-related code uses the term "tail" (and the hit result does too), I'd suggest renaming this to countSliderTailMiss or countSliderTailsDropped. I'd prefer to consider "miss" the opposite of hit even for slider tails but calling them "dropped" is arguable


/// <summary>
/// Estimated total amount of combo breaks
/// </summary>
private double effectiveMissCount;

public OsuPerformanceCalculator()
Expand All @@ -44,7 +57,36 @@ protected override PerformanceAttributes CreatePerformanceAttributes(ScoreInfo s
countOk = score.Statistics.GetValueOrDefault(HitResult.Ok);
countMeh = score.Statistics.GetValueOrDefault(HitResult.Meh);
countMiss = score.Statistics.GetValueOrDefault(HitResult.Miss);
effectiveMissCount = calculateEffectiveMissCount(osuAttributes);
countSliderEndsDropped = osuAttributes.SliderCount - score.Statistics.GetValueOrDefault(HitResult.SliderTailHit);
countSliderTickMiss = score.Statistics.GetValueOrDefault(HitResult.LargeTickMiss);

if (osuAttributes.SliderCount > 0)
{
if (usingClassicSliderAccuracy)
{
// Consider that full combo is maximum combo minus dropped slider tails since they don't contribute to combo but also don't break it
// In classic scores we can't know the amount of dropped sliders so we estimate to 10% of all sliders on the map
double fullComboThreshold = attributes.MaxCombo - 0.1 * osuAttributes.SliderCount;

if (scoreMaxCombo < fullComboThreshold)
effectiveMissCount = fullComboThreshold / Math.Max(1.0, scoreMaxCombo);

// In classic scores there can't be more misses than a sum of all non-perfect judgements
effectiveMissCount = Math.Min(effectiveMissCount, totalImperfectHits);
}
else
{
double fullComboThreshold = attributes.MaxCombo - countSliderEndsDropped;
Comment on lines +78 to +79
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
{
double fullComboThreshold = attributes.MaxCombo - countSliderEndsDropped;
{
// Consider that full combo is maximum combo minus dropped slider tails since they don't contribute to combo but also don't break it
double fullComboThreshold = attributes.MaxCombo - countSliderEndsDropped;

A comment to stay consistent with the other if-branch would be nice

Copy link
Member

Choose a reason for hiding this comment

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

Repeating the same comment is redundant

Copy link
Contributor

Choose a reason for hiding this comment

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

fair just wanted to mention it incase it is desired


if (scoreMaxCombo < fullComboThreshold)
effectiveMissCount = fullComboThreshold / Math.Max(1.0, scoreMaxCombo);

// Combine regular misses with tick misses since tick misses break combo as well
effectiveMissCount = Math.Min(effectiveMissCount, countSliderTickMiss + countMiss);
}
}

effectiveMissCount = Math.Max(countMiss, effectiveMissCount);

double multiplier = PERFORMANCE_BASE_MULTIPLIER;

Expand Down Expand Up @@ -124,8 +166,22 @@ private double computeAimValue(ScoreInfo score, OsuDifficultyAttributes attribut

if (attributes.SliderCount > 0)
{
double estimateSliderEndsDropped = Math.Clamp(Math.Min(countOk + countMeh + countMiss, attributes.MaxCombo - scoreMaxCombo), 0, estimateDifficultSliders);
double sliderNerfFactor = (1 - attributes.SliderFactor) * Math.Pow(1 - estimateSliderEndsDropped / estimateDifficultSliders, 3) + attributes.SliderFactor;
double estimateImproperlyFollowedDifficultSliders;

if (usingClassicSliderAccuracy)
{
// When the score is considered classic (regardless if it was made on old client or not) we consider all missing combo to be dropped difficult sliders
int maximumPossibleDroppedSliders = totalImperfectHits;
estimateImproperlyFollowedDifficultSliders = Math.Clamp(Math.Min(maximumPossibleDroppedSliders, attributes.MaxCombo - scoreMaxCombo), 0, estimateDifficultSliders);
}
else
{
// We add tick misses here since they too mean that the player didn't follow the slider properly
// We however aren't adding misses here because missing slider heads has a harsh penalty by itself and doesn't mean that the rest of the slider wasn't followed properly
estimateImproperlyFollowedDifficultSliders = Math.Min(countSliderEndsDropped + countSliderTickMiss, estimateDifficultSliders);
}

double sliderNerfFactor = (1 - attributes.SliderFactor) * Math.Pow(1 - estimateImproperlyFollowedDifficultSliders / estimateDifficultSliders, 3) + attributes.SliderFactor;
Comment on lines +169 to +184
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
double estimateImproperlyFollowedDifficultSliders;
if (usingClassicSliderAccuracy)
{
// When the score is considered classic (regardless if it was made on old client or not) we consider all missing combo to be dropped difficult sliders
int maximumPossibleDroppedSliders = totalImperfectHits;
estimateImproperlyFollowedDifficultSliders = Math.Clamp(Math.Min(maximumPossibleDroppedSliders, attributes.MaxCombo - scoreMaxCombo), 0, estimateDifficultSliders);
}
else
{
// We add tick misses here since they too mean that the player didn't follow the slider properly
// We however aren't adding misses here because missing slider heads has a harsh penalty by itself and doesn't mean that the rest of the slider wasn't followed properly
estimateImproperlyFollowedDifficultSliders = Math.Min(countSliderEndsDropped + countSliderTickMiss, estimateDifficultSliders);
}
double sliderNerfFactor = (1 - attributes.SliderFactor) * Math.Pow(1 - estimateImproperlyFollowedDifficultSliders / estimateDifficultSliders, 3) + attributes.SliderFactor;
double improperlyFollowedDifficultSliders;
if (usingClassicSliderAccuracy)
{
// When the score is considered classic (regardless if it was made on old client or not) we consider all missing combo to be dropped difficult sliders
int maximumPossibleDroppedSliders = totalImperfectHits;
improperlyFollowedDifficultSliders = Math.Clamp(Math.Min(maximumPossibleDroppedSliders, attributes.MaxCombo - scoreMaxCombo), 0, estimateDifficultSliders);
}
else
{
// We add tick misses here since they too mean that the player didn't follow the slider properly
// We however aren't adding misses here because missing slider heads has a harsh penalty by itself and doesn't mean that the rest of the slider wasn't followed properly
improperlyFollowedDifficultSliders = Math.Min(countSliderEndsDropped + countSliderTickMiss, estimateDifficultSliders);
}
double sliderNerfFactor = (1 - attributes.SliderFactor) * Math.Pow(1 - improperlyFollowedDifficultSliders / estimateDifficultSliders, 3) + attributes.SliderFactor;

The variable name is quite long and since it's not always estimated so it doesn't necessarily have a valuable meaning besides making the code more straining to read.

Copy link
Member

@stanriders stanriders Oct 21, 2024

Choose a reason for hiding this comment

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

I prefer longer variable names that encapsulate what it actually is instead of hiding the fact that it can be estimated (and will be for billions of scores)

Copy link
Contributor

Choose a reason for hiding this comment

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

hmm idk, I feel like there isn't much difference between "estimated" when it can or cannot be vs. not saying it when it can or cannot be

aimValue *= sliderNerfFactor;
}

Expand Down Expand Up @@ -247,29 +303,12 @@ private double computeFlashlightValue(ScoreInfo score, OsuDifficultyAttributes a
return flashlightValue;
}

private double calculateEffectiveMissCount(OsuDifficultyAttributes attributes)
{
// Guess the number of misses + slider breaks from combo
double comboBasedMissCount = 0.0;

if (attributes.SliderCount > 0)
{
double fullComboThreshold = attributes.MaxCombo - 0.1 * attributes.SliderCount;
if (scoreMaxCombo < fullComboThreshold)
comboBasedMissCount = fullComboThreshold / Math.Max(1.0, scoreMaxCombo);
}

// Clamp miss count to maximum amount of possible breaks
comboBasedMissCount = Math.Min(comboBasedMissCount, countOk + countMeh + countMiss);

return Math.Max(countMiss, comboBasedMissCount);
}

// Miss penalty assumes that a player will miss on the hardest parts of a map,
// so we use the amount of relatively difficult sections to adjust miss penalty
// to make it more punishing on maps with lower amount of hard sections.
private double calculateMissPenalty(double missCount, double difficultStrainCount) => 0.96 / ((missCount / (4 * Math.Pow(Math.Log(difficultStrainCount), 0.94))) + 1);
private double getComboScalingFactor(OsuDifficultyAttributes attributes) => attributes.MaxCombo <= 0 ? 1.0 : Math.Min(Math.Pow(scoreMaxCombo, 0.8) / Math.Pow(attributes.MaxCombo, 0.8), 1.0);
private int totalHits => countGreat + countOk + countMeh + countMiss;
private int totalImperfectHits => countOk + countMeh + countMiss;
}
}
Loading