Remove rounding of slider velocity multiplier on juice streams

[bdach]

• Pending inspection of sheets (the full set - diffcalc / ppcalc / score).

Compare: #26616

This came up elsewhere, namely in #28277 (comment).

As it turns out, at least one beatmap among those whose scores had unexpected changes in total score, namely https://osu.ppy.sh/beatmapsets/971028#fruits/2062131, was using slider velocity multipliers that were not a multiple of 0.01. See juice stream at 04:25:568 and the control point at the same time instant. The specific value used was 0.225x:

265568,-444.444444444444,2,2,1,50,0,1

This meant that due to the rounding applied to SliderVelocityMultiplierBindable via Precision, the raw value was being incorrectly rounded, resulting in incorrect conversion.

The "direct" change that revealed this is most likely ppy/osu-framework#6249, by the virtue of shuffling the BindableNumber rounding code around and accidentally changing midpoint rounding semantics in the process. But it was not at fault here, as rounding was just as wrong before that change as after in this specific context.

Test is an abridged / cropped version of https://osu.ppy.sh/beatmapsets/971028#fruits/2062131 to demonstrate the specific failure case (unfortunately can't use the whole beatmap due to other conversion failures).

[bdach]

!diffcalc
RULESET=catch
GENERATORS=pp,sr,score

[github-actions]

Target: #28337
Spreadsheet: https://docs.google.com/spreadsheets/d/1W97RSYhchXf5LYEZ9rhABwnHBL1Wzt_0kDUBUK0ThJE/edit

[bdach]

I've checked the sheets in several ways and I have seen nothing that would make me doubt that this is a correct change to make, so I will be undrafting this, but there are several considerations so cliff notes version first:

• This is basically a full difficulty/pp change now. Difficulty attributes for catch maps will need recomputing, and pp for catch scores will need to be recalculated.
• This is also implicitly a scoring change because the effects of beatmap parsing also change the legacy conversion algorithm. While this won't affect many scores, my recommendation would be to bump the score version game-side and to run a full re-verify of imported catch scores server-side. I'm not bumping the score version yet pending review feedback.

@ppy/team-client While reviewing please focus on the above points and the implications thereof. Before this is merged I would like a conclusive opinion as to whether we're doing any of the aforementioned things (and if yes, make sure that total score version is bumped in client).

---

For the interested (is there anyone?), here's the breakdown:

• Every single beatmap I've checked from the sheet above had at least one control point with slider velocity multiplier specs more precise than 0.01x. For instance:
  • /b/1265925 has 4 difficulty control points that specify slider velocity multiplier with precision smaller than 0.01:
    • 51083,-136.054421768707,4,2,1,50,0,0 -> 0.735x multiplier
    • 162467,-800,4,2,1,80,0,0 -> 0.125x multiplier
    • 162775,-600,4,2,1,80,0,0 -> 0.1(6)x multiplier
    • 182006,-300,4,2,1,80,0,0 -> 0.(3)x multiplier
  • /b/1392244 has 28 difficulty control points that specify slider velocity multiplier with precision smaller than 0.01:
    • 72383,-37.5253549695742,4,2,1,60,0,1 -> 2.66(486)x multiplier
    • 76266,-40.5701754385967,4,2,1,60,0,1 -> 2.46(486)x multiplier
    • 78030,-44.1527446300719,4,2,1,60,0,1 -> 2.26(486)x multiplier
    • and so on
  • /b/1605148 has 238 difficulty control points that specify slider velocity multiplier with precision smaller than 0.01
  • /b/3942600 has 1 difficulty control point that specifies slider velocity multiplier with precision smaller than 0.01
  • /b/4044925 has 5 difficulty control points that specify slider velocity multiplier with precision smaller than 0.01
• Generally conversion mapping test results are more correct with this than on master (they still fail because of various other breakage such as the edgedash issue, but they have less assertion failures in that respect)
• Generally star rating as calculated by stable is closer to the star rating with this PR / as shown on the sheet above (this is notable, because this is a bug explicitly caused by beatmap parsing / implicit bindable rounding, and stable does not attempt to reimplement the lazer way of doing things)
• The score changes from Compute total score without mods during standardised score conversion #28277 (comment) are back here, but this is actually counterintuitively correct. The old sheet there was comparing broken master vs a branch that did not have the "directly offending" framework change merged, which means that it was showing increases because it was master that "broke" things. The new sheet here has the same increases which effectively demonstrates that this undoes the damage. The best way I can recommend of checking this is via osu-tools and a local checkout of the game at various commits.
• However, there are more score changes on the sheet in this PR than there were on that old sheet. That illustrates that even the old code was broken, because it was rounding still, and the "directly offending" framework change just revealed the rounding that caused the breakage by nudging midpoint rounding behaviour. And indeed, for instance, this score which is shown as changed by this pull but not that old sheet, is set on this map, which features difficulty control points that are specified with precision smaller than 0.01x.

[smoogipoo]

For me, one other important consideration is that osu! specifically doesn't have a precision set, and I'm sure that is purposefully so because it did at one point in time. Therefore, even optically, this looks like a positive change.

I think when SR/PP changes come we'll just reprocess all rulesets at this point anyway, just to get everything to a baseline that is off osu-performance.

Also pinging @peppy to read/take note of the above & click the green button.

[peppy]

my recommendation would be to bump the score version game-side and to run a full re-verify of imported catch scores server-side. I'm not bumping the score version yet pending review feedback.

Personally I wouldn't bother with this because it's unlikely to have affected more than a handful of scores (and whether the user notices this is even less likely). @smoogipoo thoughts on this one if you disagree?

[smoogipoo]

Agree - I don't think it needs to be bumped until it's noticed in practice.