-
-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Add TappingFactor and use it to nerf HD bonus on speed pp #31478
base: pp-dev
Are you sure you want to change the base?
Conversation
hopefully I included the right files
} | ||
|
||
private readonly bool withTapping; |
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 naming is really confusing because the evaluator uses the same variable with the name withDistanceBonus
and these are polar opposite things. Please give the variables the same intention and double check it's working as intended (the below !withTapping
check concerns me)
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.
oh I probably just forgot to rename it when I inverted a bunch of other things
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.
fixed
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 is still wrong, speed has withoutFlow
while speed evaluator has withDistanceBonus
which are opposite behaviours - passing true
to speed will still cause the distance bonus to be present
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.
Okay, I think I was getting confused by all of the naming and inverses of logic. I suggest something like the below diff to make it less confusing. Note that my diff will include rhythm difficulty regardless of tappingOnly
- rhythm is a part of tapping so I would fairly assume that the flow factor would be more accurate if it's included. That much is open for debate, but changing that along with the renames made this read far less confusing to me.
diff --git a/osu.Game.Rulesets.Osu/Difficulty/Evaluators/SpeedEvaluator.cs b/osu.Game.Rulesets.Osu/Difficulty/Evaluators/SpeedEvaluator.cs
index e6a8a7104e..6c881d1a0f 100644
--- a/osu.Game.Rulesets.Osu/Difficulty/Evaluators/SpeedEvaluator.cs
+++ b/osu.Game.Rulesets.Osu/Difficulty/Evaluators/SpeedEvaluator.cs
@@ -28,7 +28,7 @@ public static class SpeedEvaluator
/// <item><description>and how easily they can be cheesed.</description></item>
/// </list>
/// </summary>
- public static double EvaluateDifficultyOf(DifficultyHitObject current, bool withDistanceBonus, IReadOnlyList<Mod> mods)
+ public static double EvaluateDifficultyOf(DifficultyHitObject current, bool tappingOnly, IReadOnlyList<Mod> mods)
{
if (current.BaseObject is Spinner)
return 0;
@@ -60,7 +60,7 @@ public static double EvaluateDifficultyOf(DifficultyHitObject current, bool with
// Max distance bonus is 1 * `distance_multiplier` at single_spacing_threshold
double distanceBonus = Math.Pow(distance / single_spacing_threshold, 3.95) * distance_multiplier;
- if (!withDistanceBonus || mods.OfType<OsuModAutopilot>().Any())
+ if (tappingOnly || mods.OfType<OsuModAutopilot>().Any())
distanceBonus = 0;
// Base difficulty with all bonuses
diff --git a/osu.Game.Rulesets.Osu/Difficulty/OsuDifficultyCalculator.cs b/osu.Game.Rulesets.Osu/Difficulty/OsuDifficultyCalculator.cs
index 14bba60d36..59e5cbfacc 100644
--- a/osu.Game.Rulesets.Osu/Difficulty/OsuDifficultyCalculator.cs
+++ b/osu.Game.Rulesets.Osu/Difficulty/OsuDifficultyCalculator.cs
@@ -39,7 +39,7 @@ protected override DifficultyAttributes CreateDifficultyAttributes(IBeatmap beat
double aimRating = Math.Sqrt(skills[0].DifficultyValue()) * difficulty_multiplier;
double aimRatingNoSliders = Math.Sqrt(skills[1].DifficultyValue()) * difficulty_multiplier;
double speedRating = Math.Sqrt(skills[2].DifficultyValue()) * difficulty_multiplier;
- double speedRatingNoFlow = Math.Sqrt(skills[3].DifficultyValue()) * difficulty_multiplier;
+ double speedRatingTappingOnly = Math.Sqrt(skills[3].DifficultyValue()) * difficulty_multiplier;
double speedNotes = ((Speed)skills[2]).RelevantNoteCount();
double difficultSliders = ((Aim)skills[0]).GetDifficultSliders();
double flashlightRating = 0.0;
@@ -48,7 +48,7 @@ protected override DifficultyAttributes CreateDifficultyAttributes(IBeatmap beat
flashlightRating = Math.Sqrt(skills[4].DifficultyValue()) * difficulty_multiplier;
double sliderFactor = aimRating > 0 ? aimRatingNoSliders / aimRating : 1;
- double flowFactor = speedRating > 0 ? Math.Pow(speedRatingNoFlow / speedRating, 3) : 1;
+ double flowFactor = speedRating > 0 ? Math.Pow(speedRatingTappingOnly / speedRating, 3) : 1;
double aimDifficultyStrainCount = ((OsuStrainSkill)skills[0]).CountTopWeightedStrains();
double speedDifficultyStrainCount = ((OsuStrainSkill)skills[2]).CountTopWeightedStrains();
diff --git a/osu.Game.Rulesets.Osu/Difficulty/Skills/Speed.cs b/osu.Game.Rulesets.Osu/Difficulty/Skills/Speed.cs
index ff8188b60e..d36e0532f2 100644
--- a/osu.Game.Rulesets.Osu/Difficulty/Skills/Speed.cs
+++ b/osu.Game.Rulesets.Osu/Difficulty/Skills/Speed.cs
@@ -23,13 +23,13 @@ public class Speed : OsuStrainSkill
protected override int ReducedSectionCount => 5;
- public Speed(Mod[] mods, bool withoutFlow)
+ public Speed(Mod[] mods, bool tappingOnly)
: base(mods)
{
- this.withoutFlow = withoutFlow;
+ this.tappingOnly = tappingOnly;
}
- private readonly bool withoutFlow;
+ private readonly bool tappingOnly;
private double strainDecay(double ms) => Math.Pow(strainDecayBase, ms / 1000);
@@ -38,9 +38,7 @@ public Speed(Mod[] mods, bool withoutFlow)
protected override double StrainValueAt(DifficultyHitObject current)
{
currentStrain *= strainDecay(((OsuDifficultyHitObject)current).StrainTime);
- currentStrain += SpeedEvaluator.EvaluateDifficultyOf(current, withoutFlow, Mods) * skillMultiplier;
-
- if (withoutFlow) return currentStrain;
+ currentStrain += SpeedEvaluator.EvaluateDifficultyOf(current, tappingOnly, Mods) * skillMultiplier;
currentRhythm = RhythmEvaluator.EvaluateDifficultyOf(current);
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 is still wrong, speed has withoutFlow while speed evaluator has withDistanceBonus which are opposite behaviours - passing true to speed will still cause the distance bonus to be present
didn't notice that when doing stuff in the other two files, oops.
- if (withoutFlow) return currentStrain;
If you remove this line, rhythm ends up not getting a bonus from HD. I'm still going back and forth on whether or not it should have a bonus, but I figure this should be mentioned anyways.
This reverts commit e5fe67c.
Makes HD only affect the flow aim portion of speed since HD does not make tapping harder. I'm personally not sure whether or not rhythm should be buffed by HD, since I can see arguments going either way, but it is currently omitted from the buff.