Decompose snap aim evaluator into several methods - #38201
Open
tsunyoku wants to merge 127 commits into
Open
Conversation
* Add basic scorev2 support * Revert unnecessary score statistic changes * Fix CI * Revert changes * Disable score-based misscount for scoreV2
* Simplify star rating calculations * Refactor
Co-authored-by: StanR <8269193+stanriders@users.noreply.github.com>
Co-authored-by: StanR <8269193+stanriders@users.noreply.github.com>
Replaces a constant that assumes DecayWeight == 0.9 Co-authored-by: James Wilson <tsunyoku@gmail.com> Co-authored-by: StanR <8269193+stanriders@users.noreply.github.com>
* Extend `Skill` to include `ObjectDifficulties` * Remove generic * Change `ObjectDifficulties` to be modifiable by children skills * Fix tests
…of the hitwindow (ppy#36148)
* Implement new portion formula * Change the formula * Use base value of 5 sliders * Make the threshold harsher --------- Co-authored-by: StanR <8269193+stanriders@users.noreply.github.com>
…ulty values (ppy#36209) Fun correctness bug. ScoreV1 multiplier uses the **nomod** peppy stars (i.e the beatmap's "base" HP, OD and CS) in all cases, which was generally the intention with this code but it passes the wrong beatmap resulting in it using modded values for this. Cross referenced with stable and this now results in the expected multipliers. Results in some very minor (<3pp from my testing) changes for HR and EZ scores with combo breaks.
It's a common mistake by newer pp dev contributors to increase the wiggle multiplier not knowing its at its maximum, so I added a comment to try to avoid more people doing it. --------- Co-authored-by: StanR <8269193+stanriders@users.noreply.github.com>
…e to the evaluator level (ppy#36417) This PR moves the influence of d/t^2 from the skill (through strain) directly into the evaluator level as a bonus applied at the end. This makes it more clear what d/t^2 is, and the fact that it applies to *all* bonuses in the evaluator. As well, the peak strain value of a note is now equal to the evaluator value. This comes with a couple benefits: 1. The BPM weight is now subject to balance, and can be flattened easily for a more "d/t" like system (previously this required hacky solutions) 2. StrainDecayBase becomes a much more useful variable now that it does not affect the difficulty of the note itself. When adjusting this in live, your star rating would double upon changing from 0.15 in aim to 0.3, and now it is intuitive what it does (makes strain take longer to accumulate). This means that future balancing efforts can use evaluators to dynamically adjust strainDecayBase (potentially letting large spikes provide more strain for the same difficulty if they're wide angle, for example). 3. In the object inspector, you get the actual maximum difficulty value of a note as seen in the ObjectDifficulties list. This makes it easier to tell what notes are deemed as harder by the system. For context, previously, a note of difficulty 1 at 200bpm would cap out at 6 as a result of strain, and a note of the same difficulty but at 300bpm would cap out at 8. The actual implementation is really really simple. I'm willing to move this to flashlight if wanted (I don't think it's necessary), or even abstract this away so that (1 - decay) doesn't look like a balancing constant. Side note: this is equivalent to live, except for notes with a deltaTime of less than 25ms (since I am using AdjustedDeltaTime in the aim evaluator for the bonus to avoid divisions by zero).
…y#34696) The purpose of this change is to remove the arbitrary note based length bonus for speed and replace it with a more concrete per note difficulty aware calculation. As a result of the summation needing to be per-note, chunking and in consequence peak strain reduction have both been removed from the skill. ~~The various strain counting functions also heavily relied on chunking and the way summation worked, so they have also been changed to match values held before the change as much as possible (given the different summation some changes in values are bound to happen).~~ Above was a bug. Huis page: [https://pp.huismetbenen.nl/rankings/players/length-bonus](https://pp.huismetbenen.nl/rankings/players/length-bonus) --------- Co-authored-by: James Wilson <tsunyoku@gmail.com>
…rs (ppy#35555) ## This pr resolves the issue with the angle calculation. The old system calculated the angle between three objects. If the last object was a long slider that the player needed to follow, the system calculated the angle using only the slider’s end point and the previous object, which caused long sliders to have incorrect angle values. --------- Co-authored-by: StanR <hi@stanr.info> Co-authored-by: James Wilson <tsunyoku@gmail.com>
Currently the slider bonus works on the assumption that the travel velocity of the previous slider is a part of the current object's difficulty because it is part of the movement from prev to curr. However, this is contradicted by the fact that `currVelocity` is a combination of prev->curr + curr slider velocity and is actually breaking the assumption that slider difficulty is contained in the slider itself that we take when calculating difficult slider strains. This makes it so that the slider bonus difficulty is contained in the slider itself instead of buffing the next object, which makes both the calculation overall more consistent and the slider factor calculation actually work as expected. Aim multiplier got slightly lowered because this change makes most of the sliders gain a little bit --------- Co-authored-by: James Wilson <tsunyoku@gmail.com>
…! ruleset (ppy#33196) This PR aims to replace the current bonuses used to award high approach rates and scores made using the Hidden mod with a Reading skill that takes into account each note's reading difficulty separately, Important to note is the fact that as reading difficulty is now a skill the bonuses are now additive instead of multiplicative, meaning there are vast changes in deltas on the high and low end of scores. Due to the nature of adding a new skill new difficulty and performance attributes need to be added. Huis page: [https://pp.huismetbenen.nl/rankings/admin/kwotaq-reading](url) --------- Co-authored-by: apollo-dw <83023433+apollo-dw@users.noreply.github.com> Co-authored-by: js1086 <js1086@student.le.ac.uk> Co-authored-by: tsunyoku <tsunyoku@gmail.com> Co-authored-by: StanR <hi@stanr.info>
This doesn't solve _flow_ aim in any way, but makes it so that `Speed` doesn't have distance scaling (read as "aim") anymore which fixes some issues related to that like the length bonus behaving incorrectly, and in general `Speed` being an aim+tap skill instead of just a tapping skill --------- Co-authored-by: James Wilson <tsunyoku@gmail.com>
https://pp.huismetbenen.nl/rankings/players/tap-statacc --------- Co-authored-by: James Wilson <tsunyoku@gmail.com>
Right now they're summed normally what opens a problem that FL rewards too much pp when combined with reading map, since you're memorizing FL anyway. Co-authored-by: James Wilson <tsunyoku@gmail.com>
Part of this PR - ppy#27303 Current aim calculation have a flaw of sliderless aim still accounting for sliders. This happens because of usage of `LazyJumpDistance` as a main distance metric. This PR is fixing this by adding `JumpDistance` as true sliderless metric, using it instead of `LazyJumpDistance`. This can introduce very rare cases where sliderless aim is worth more than normal aim (because of velocity change bonus). The effect of this is minimal on most of the maps. It can be seen the best on this map - https://osu.ppy.sh/beatmapsets/594751#osu/1257904 Before:  After:  --------- Co-authored-by: James Wilson <tsunyoku@gmail.com> Co-authored-by: StanR <8269193+stanriders@users.noreply.github.com>
It existed for one map and now that that map is fine there's no need to do it anymore.
… higher (ppy#36487) There was a recent PR (ppy#36464) that was aimed on accounting for the fact that getting your map partially memorized with FL makes reading easier, so those bonuses shouldn't reward full pp to each other. But in cases where FL pp is significantly lower than reading it have lead to cases where FL adds practically 0 additional pp. This PR is adjusting a formula to account for cases like this. FL reward on this map - https://osu.ppy.sh/beatmapsets/1487999#osu/3259719 with EZHDHT(FL): Full bonus: 408pp -> 486pp (+78pp) Current: 408pp -> 424pp (+16pp) This PR: 408pp -> 478pp (+70pp) Co-authored-by: StanR <8269193+stanriders@users.noreply.github.com>
This should buff raw speed plays like Ivaxa Violation, at the same time undoing part of the buff on the lower end scores like Save Me NM Moved part of the multiplier out of the Pow to be more intuitive (it multiplies the 20 by Pow(difficulty/4), so it's more clear that it would be equal to 1 on difficulty = 4) The scaling itself was adjusted to be more similar to live (so buffs/nerfs on 98% acc remains +- the same through the difficulty curve)
This should be a pretty simple fix for doubles being systemically broken in aim. Doubles get essentially zero bonus from the aim eval itself - wide & acute bonuses are zero due to the lack of distance, and velocity is close to zero _again_ because of the lack of distance. However, the delta times of these notes mean that `highBpmBonus` is very kind and will buff the strain quite significantly. This change means that the 2nd note of a double should get next to no aim strain, which feels like correct behaviour. From testing, streams, stacks etc. are essentially unchanged by this due to the fact its using radius rather than diameter. https://pp.huismetbenen.nl/rankings/players/doubles-strain
It removes unnecessary function `DurationSpentInvisible` function that just rescaled preempt. Now it's just using preempt directly. I've made multiplier to be very close to the current one, so pp deltas should be minimal.
…SINGLE_SPACING_THRESHOLD` (ppy#36573) Slightly reducing aim/speedaim doubledipping, but mostly just done for consistency and ease of understanding of existing relation between both
I'd push these straight into the branch but I don't have perms /shrug
peppy
reviewed
Jul 1, 2026
| return DiffUtils.Pow(baseNerf + (1 - baseNerf) * vectorRepetition * maximum_vector_influence * stackFactor, 2); | ||
| } | ||
|
|
||
| private static double highBpmBonus(double ms) => 1 / (1 - DiffUtils.Pow(0.03, DiffUtils.Pow(ms / 1000, 0.65))); |
Member
There was a problem hiding this comment.
We now have five of these. Move to a DiffUtils helper method?
Member
Author
There was a problem hiding this comment.
Each one of these has different constants, I think it may cause more confusion to try and consolidate
Member
|
Probably need to run a diffcalc spreadsheets run here to guarantee no actual changes occur. |
stanriders
reviewed
Jul 20, 2026
|
|
||
| private static double calcAngleWideness(double angle) => DiffUtils.Smoothstep(angle, double.DegreesToRadians(40), double.DegreesToRadians(140)); | ||
|
|
||
| public static double CalcAngleAcuteness(double angle) => DiffUtils.Smoothstep(angle, double.DegreesToRadians(140), double.DegreesToRadians(40)); |
Member
There was a problem hiding this comment.
I feel like we can move this out to ODHO at this point as well
Member
There was a problem hiding this comment.
CalcAngleAcuteness, CalcAngleWideness
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
14 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I think this is a lot easier to read. The levels of nesting we were reaching, and coupling totally independent bonuses together makes this super overwhelming.