Allow lower slider velocities in beatmap specification

[peppy]

Lazer counterpart to ppy/osu-stable-issues#1072.

[bdach]

If this is to be allowed in stable too, then I'm not sure this is all. There's also this logic in the decoder:

osu/osu.Game/Beatmaps/Formats/LegacyDecoder.cs

Lines 181 to 188 in ad2582a

	public LegacyDifficultyControlPoint(int rulesetId, double beatLength)
	: this()
	{
	// Note: In stable, the division occurs on floats, but with compiler optimisations turned on actually seems to occur on doubles via some .NET black magic (possibly inlining?).
	if (rulesetId == 1 || rulesetId == 3)
	BpmMultiplier = beatLength < 0 ? Math.Clamp((float)-beatLength, 10, 10000) / 100.0 : 1;
	else
	BpmMultiplier = beatLength < 0 ? Math.Clamp((float)-beatLength, 10, 1000) / 100.0 : 1;

0.01x SVs were previously allowed in taiko and mania specifically. If this is to be allowed for the rest too, then these clamp ranges probably need to be unified to allow 10000 max regardless of ruleset.

Not that it'll have any actual effect, because the other rulesets aren't using BpmMultiplier for anything at all. But if we're cleaning house...

[peppy]

That particular code doesn't even show 0.01x being allowed, as far as I can tell. The ranges there are 0.1-100 for mania / taiko and 0.1-10 for osu!. So maybe both need to be adjusted if we want to standardise things? I'm not sure if there's a reason it's a different specification there, though.

[smoogipoo]

That particular code doesn't even show 0.01x being allowed

The ranges there are 0.1-100 for mania / taiko and 0.1-10 for osu!.

I'm confused what you mean by both of these. The purpose of this PR and the proposal is to allow 0.01x.

[peppy]

Okay so taking a step back:

• @bdach asked for an additional change to be made to BpmMultiplier, which seemingly is only really used for taiko convert purposes.
• He suggested increasing the max to 10000
• I responded saying that that seems wrong, and these values already do not match expectations. To make them match with master, the mania minimum should be adjusted. But I'm not sure if this was left non-matching for another reason. It will require further investigation to understand whether this is the case.

[bdach]

One point that may not be immediately clear from my initial comment and which may clear this confusion up in entirety: BpmMultiplier is the inverse of slider velocity.

[peppy]

I see. In that case it may be correct as it stands, and only require combining the two cases to allow the same clamping everywhere?

[bdach]

Is my thinking, yep. See also: #20658

[smoogipoo]

What I am saying is that these changes should correspond 1-1 to osu-stable changes. If there is a discrepancy - which there is - one of the two are wrong and it sounds like the testing methodology used is broken.

For reference, the change to stable was:

-               return OsuMathHelper.Clamp((float)-BeatLength, 10, 1000) / 100f;
+               return OsuMathHelper.Clamp((float)-BeatLength, 1, 1000) / 100f;

Edit: With, in osu-stable, taiko/mania being unaltered as:

                return OsuMathHelper.Clamp((float)-BeatLength, 10, 10000) / 100f;

[peppy]

Sure, I can match that here. The discrepancy is that stable uses BpmMultiplier for slider calculations while lazer doesn't.

[peppy]

Check on https://github.com/peppy/osu-stable/pull/2441, which would standardise things at stable's end. We'd need to verify existing ranked beatmaps to ensure nothing breaks, but it would be the sanest solution going forward if it works.

[smoogipoo]

I'm running diffcalc tests on this so I'll leave it open for now pending those results.

osu: https://docs.google.com/spreadsheets/d/1lm4uHyat7DlDn_ZPhmNtfl0iHLsxK68ZN3UQqC1L2vA/edit
taiko: https://docs.google.com/spreadsheets/d/1B3QrgO68g5L7NRnHqBxLkgpy8iidlz3qOB3hvgg5-KI/edit
catch: https://docs.google.com/spreadsheets/d/137JTgqb3srGIht3EyexXwCHT3kJIcI_7P_1PQwa6oCA/edit
mania: https://docs.google.com/spreadsheets/d/16qdZdjCPh8gp6Wd_8HaEeYHWOuDjqjIOBMhq_9blXhg/edit

[smoogipoo]

This appears to have significantly broken some beatmaps. Most egregiously is https://osu.ppy.sh/beatmapsets/423194#osu/914570 which now exhibits "2B" layering making it impossible in catch:

[peppy]

Awesome, kinda expected that so it's good to have confirmation.

I'll leave this open for now pending investigating how easy it would be to fix backwards compatibility.