-
-
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
Calculate hit windows in performance calculator instead of databased difficulty attributes #31735
Conversation
…difficulty attributes
!diffcalc |
Can the great/ok attribute definitions be removed too? |
You should do the same for other rulesets too then |
Yes we can, but it's worth noting these other usages are not new. Note the following:
Following from https://discord.com/channels/188630481301012481/380598781432823815/1334524621625491507, I need to push a fix for the osu! changes to ensure that any mods which affect the base overall difficulty are applied to the beatmap difficulty. However, I'm unsure if this is going to result in a double application of mods in the case the beatmap provided already had them applied... I'm thinking if this is coming from the PP counter, or osu-tools, it's going to use a beatmap which already has the beatmap difficulty modified in context of the score. Maybe this could be done on the server-side so we can trust the beatmap difficulty object in the performance calculator and it will work in either case. In doing this, I noticed that we may be able to go an extra step and remove the |
I guess it depends on what your goals are here. If the goal is only to remove the newly added attribute, then sure, but I was intending to go the full way and remove every single one to reduce DB storage requirements. Good catch on OD, perhaps the AR attribute too? |
Yeah, I intend on getting rid of as many as possible. AR seems doable too. I'm still unsure if the application of difficulty from the score mods should be happening on server-side or in the performance calculator, though. It seems that some places are applying difficulty (i.e |
What does "application of difficulty" mean here? It's a bit hard to get a sense of what is happening here (and hard to really picture what ppy/osu-queue-score-statistics#321 is intending to achieve) so a summary/overview of what is using what data would be appreciated. |
Thought I replied before, but I disagree with this just to keep everything uniform. We never deal with the base beatmap containing post-modded values - only after |
I'm not sure if I didn't word my question well or not but I was more asking what the game actually passes rather the design that was preferred. I ran some scenarios with a debugger locally (results screen calculation, PP counter skin element during gameplay) and it looks like the performance calculator is passed the pre-modded values which solves my concerns. Note that I did have to add a Latest commits remove all difficulty attribute usage for hit windows, OD and AR. |
@peppy A summary of what's going on, so you're in the loop. Currently, each ruleset's performance calculations rely on hit windows, approach rate or overall difficulty with consideration to the mods played. These are stored as difficulty attributes in the database, and then used in performance calculations. However, we're able to calculate these in the performance calculator on-the-fly as we have all the data required (assuming the |
!diffcalc |
!diffcalc |
!diffcalc |
!diffcalc |
Difficulty calculation failed: https://github.com/ppy/osu/actions/runs/13175362737 |
!diffcalc |
!diffcalc |
Difficulty calculation failed: https://github.com/ppy/osu/actions/runs/13175367014 |
!diffcalc |
Thanks I finally get it. I was trying to figure out the cross-project dependency, and thought there was some data ferrying happening but it turns out that score-statistics just called a special method |
Difficulty calculation failed: https://github.com/ppy/osu/actions/runs/13196592967 |
Noticed this one failed to do the osu! calc above... !diffcalc |
Isn't it mania which failed? Which should be fine anyways, the only change to mania here was removing an unused property in the first place. osu sheet: #31735 (comment) |
Oh, you're right. Seems fine then. The error is quite concerning though:
|
…difficulty attributes (ppy#31735) * Calculate hit windows in performance calculator instead of databased difficulty attributes * Apply mods to beatmap difficulty in osu! performance calculator * Remove `GreatHitWindow` difficulty attribute for osu!mania * Remove use of approach rate and overall difficulty attributes for osu! * Remove use of hit window difficulty attributes in osu!taiko * Remove use of approach rate attribute in osu!catch * Remove unused attribute IDs * Code quality * Fix `computeDeviationUpperBound` being called before `greatHitWindow` is set
See #31595 (comment).
Depends on ppy/osu-queue-score-statistics#321 so that
score.BeatmapInfo.Difficulty
can be used in the calculator server-side.