Fix gameplay PP counter not matching results screen #27808
Conversation
|
Can you explain what the actual issue is? I'm looking at the diff and I don't get it. How does a "parenting" mapping fix anything? (What does the notion of a "parent" even mean here, too?) I see something about "difficulty hitobjects not corresponding 1:1 to real hitobjects", but I don't get what that has to do with the report. This came up before, by the way, but I don't believe the problem described therein would be fixed by this. |
Whoops. My bad. I went through two implementations here and the original one was much simpler (but also didn't work correctly). This is a very good point which I didn't account for. I've fixed that case also by adding all intermediate parenting objects. I've also added a very rough test for all of this. |
| // Implementations expect the progressive beatmap to only contain top-level objects from the original beatmap. | ||
| // At the same time, we also need to consider the possibility DHOs may not be generated for any given object, | ||
| // so we'll add all remaining objects up to the current point in time to the progressive beatmap. | ||
| for (int i = progressiveBeatmap.HitObjects.Count; i < Beatmap.HitObjects.Count; i++) |
There was a problem hiding this comment.
This is a little better but there is still a bit of a problem with this implementation (related to #17964 moreso than to the issue this was originally trying to resolve), although it's only obvious at the point of usage of the method.
Most difficulty calculators skip the first several objects (because they need multiple to work with). This means that for the first object (possibly more, depending on ruleset), the pp calculator will use the TimedDifficultyCalculator for the first difficulty object, which may actually be the second or third or fourth actual object.
I'm not sure how much this matters, I guess I'd be willing to let it slide for another day, but given you've already spent time to attempt to fix #17964 with the recent changes too I figure I should at least bring this up before proceeding further...
There was a problem hiding this comment.
Is there actually an issue with that? Logically I wouldn't think so because it would round to 0pp no matter what.
I'm not sure how to handle that if there is an issue. Perhaps it would be handled in the counter directly rather than clamping to 0.
There was a problem hiding this comment.
It's probably fine, but I'll give it another check.
Fixes #27798
The issue is that
CreateDifficultyAttributesgenerally expects the original beatmap to be present for, for example, computing the max combo. Peeking into the implementation ofCatchDifficultyCalculatormakes it evident as to why it expects nested hitobjects to not be present in the beatmap:osu/osu.Game.Rulesets.Catch/Difficulty/CatchDifficultyCalculator.cs
Line 47 in 3e8ddbd
This implementation works ensures that the progressive beatmap only contains hitobjects from the original beatmap.