-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Flashlight difficulty rework #28278
base: master
Are you sure you want to change the base?
Flashlight difficulty rework #28278
Conversation
…-performance', 'fl-slider-lazy-position' and 'fl-combo-difficulty' into fl-main-2
!diffcalc |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a quick parse to start with.
private static double getComboScaleFor(int combo) | ||
{ | ||
// Taken from ModFlashlight. | ||
if (combo >= 200) | ||
return 125.0; | ||
if (combo >= 100) | ||
return 162.5; | ||
|
||
return 200.0; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Honestly, at this point you might as well stick to ground truth here and use the GetSize()
method directly from ModFlashlight
. It'd have the added bonus of adding calculation support for mod settings. It would mean passing the flashlight mod through to the evaluator, but I think this is practically expected for a mod evaluator.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This kind of thing goes over my head. There is also a lot more to consider than just the radius that it might be worth making another PR instead (unless someone tells me otherwise)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
confused what state this was left in, if there's a reason you can't use GetSize
directly could you explain? as apollo said, using the mod's method is going to be a requirement to support custom radius settings. if you need to do some math on the size for a final "combo scale" result that is fine but you should be calling GetSize
first regardless
@@ -105,6 +115,9 @@ public OsuDifficultyHitObject(HitObject hitObject, HitObject lastObject, HitObje | |||
} | |||
|
|||
setDistances(clockRate); | |||
|
|||
PreviousMaxCombo = index > 0 ? ((OsuDifficultyHitObject)Previous(0)).CurrentMaxCombo : getObjectCombo(lastObject); | |||
CurrentMaxCombo = PreviousMaxCombo + getObjectCombo(hitObject); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Previous()
call here is probably something to give a quick double check on from a profiling perspective, since that's going to be called for every object in the map when creating difficulty objects. I'll leave this for @tsunyoku though...
This could be optimised by moving this to the CreateDifficultyHitObjects
method loop if desired.
osu.Game.Rulesets.Osu/Difficulty/Evaluators/FlashlightEvaluator.cs
Outdated
Show resolved
Hide resolved
osu.Game.Rulesets.Osu/Difficulty/Evaluators/FlashlightEvaluator.cs
Outdated
Show resolved
Hide resolved
if (hitObject is Slider slider) | ||
{ | ||
if (slider.NestedHitObjects[1] is SliderRepeat) | ||
return slider.RepeatCount + 2; | ||
else | ||
return slider.NestedHitObjects.Count; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This entire method makes me feel nervous. What's going on with the first SliderRepeat
check? What if the slider has ticks as well as the repeats? Is there a reason we can't just use NestedHitObjects.Count
always?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code comes from this conversation in the pp dev discord (with an explanation of what this does). I didn't look to see if there was a better way to do this.
New version of #24124.
This is a combination of many small changes that should make flashlight performance technically more correct. Overall, flashlight scores will generally give more pp than before if they were FCs.
Spinner bug fix
The most notable nerf in this set of changes, caused by not adding StrainTime during a spinner.
Example maps most affected by this change:
New slider calculation
Before this rework, some of the long sliders on maps such as Notch Hell gave no extra pp despite definitely warranting it. This rework uses a better measure of slider velocity and uses it to scale the slider's difficulty. This is the most notable buff in this set of changes.
Example maps most affected by this change:
Flashlight radius consideration in difficulty
This adds 2 new attributes to
OsuDifficultyHitObject
:PreviousMaxCombo
andCurrentMaxCombo
. These are used in the flashlight evaluator to get the current flashlight radius to scale the new distance based nerfs.Flashlight radius consideration in performance
When a player misses, the flashlight radius will increase in size. This rework attempts to estimate how much of the score was spent in each combo radius by using the missing combo from an FC and the effectiveMissCount. Since this is a new nerf to the performance calculator, this will cause non-FCs to lose pp. In the future, removing the combo scaling factor should be considered.
A new length bonus formula to better reflect the flashlight radii was also added. The comparison can be seen here: https://www.desmos.com/calculator/4tt2ypchzq
Accuracy changes
FL and HD now have the same accuracy multiplier to prevent rare cases where FL SS scores were worth less than HD SS scores on low difficulties. Also includes a slight buff to HDFL scores.
Slider lazy end consideration
This rework uses the LazyEndPosition for jumps between non-consecutive objects to better reflect actual cursor positions. This doesn't really affect values all that much.