Fix sliders not always being the correct length

[peppy]

This is another similar case where stable floating point precision comes into play due to use of hitObjectManager.Beatmap.BpmMultiplierAt (see https://github.com/peppy/osu-stable-reference/blob/1531237b63392e82c003c712faa028406073aa8f/osu!/GameplayElements/HitObjects/Osu/SliderOsu.cs#L680)

Closes #24708.

• Depends on Remove LegacyDifficultyControlPoint / LegacyBpmMultiplier #24738

[bdach]

Looking into the point raised on discord about stable mappings outputting slider ticks non-chronologically, that tracks with stable code: https://github.com/peppy/osu-stable-reference/blob/1531237b63392e82c003c712faa028406073aa8f/osu!/GameplayElements/HitObjects/Osu/SliderOsu.cs#L980-L984

The last sliderScoreTimingPoint is pulled back by 36ms, but the list of all of them is not re-sorted afterwards. Therefore I think this is safely ignorable and we can just reorder the mappings in the json output.

I haven't checked if the lack of re-sort has any adverse effect on stable, but in my view, if it did, it'd be a pretty horrible stable bug anyways, and should be fixed in lazer by all costs, so I kinda don't care about that (...unless proven otherwise I guess)

[bdach]

I've pushed changes to fix the test failures and to adapt this to the changes in #24738. Note that I'm not saying 5c6cd87 is correct in the "matches stable" sense, but it should be correct in the "rearranged terms" sense. @peppy would appreciate a double check whenever able.

I'll do a diffcalc on this and see what comes out (because something inevitably will), and I'll double check ground truth with stable after that.

[peppy]

I think the updated code looks correct, but the diffcalc will be the true test of that.

[bdach]

Diffcalc sheet came out... empty? https://docs.google.com/spreadsheets/d/1Bw4uf8Ixwr6T0Y96kdXcqEr_slx7fC5UgExry7aEN_g/edit#gid=400707347

I find that a little hard to believe, but not sure how I could have screwed that up...

[bdach]

On checking in game, even on the map / slider directly affected, SR does not change, tick or no tick.

Well I'm not really sure what to do with this change now... The way I see it, either we just merge it and see what happens, or I guess I try to somehow rig stable to extract conversion mappings for all ranked maps / all maps period and try to cross-test that against this branch? I'm not sure how else to validate this...

@peppy @smoogipoo thoughts, ideas?

[peppy]

I think it's fine to go ahead with merge. As far as I know, ticks do not factor into diffcalc so I would assume this doesn't result in a change.

[bdach]

Guess we do it live and see if this goes wrong...

[smoogipoo]

As far as I know, ticks do not factor into diffcalc

They are factored in:

osu/osu.Game.Rulesets.Osu/Difficulty/Preprocessing/OsuDifficultyHitObject.cs

Lines 212 to 269 in 0985bb0

	private void computeSliderCursorPosition(Slider slider)
	{
	if (slider.LazyEndPosition != null)
	return;
	slider.LazyTravelTime = slider.NestedHitObjects[^1].StartTime - slider.StartTime;
	double endTimeMin = slider.LazyTravelTime / slider.SpanDuration;
	if (endTimeMin % 2 >= 1)
	endTimeMin = 1 - endTimeMin % 1;
	else
	endTimeMin %= 1;
	slider.LazyEndPosition = slider.StackedPosition + slider.Path.PositionAt(endTimeMin); // temporary lazy end position until a real result can be derived.
	var currCursorPosition = slider.StackedPosition;
	double scalingFactor = NORMALISED_RADIUS / slider.Radius; // lazySliderDistance is coded to be sensitive to scaling, this makes the maths easier with the thresholds being used.
	for (int i = 1; i < slider.NestedHitObjects.Count; i++)
	{
	var currMovementObj = (OsuHitObject)slider.NestedHitObjects[i];
	Vector2 currMovement = Vector2.Subtract(currMovementObj.StackedPosition, currCursorPosition);
	double currMovementLength = scalingFactor * currMovement.Length;
	// Amount of movement required so that the cursor position needs to be updated.
	double requiredMovement = assumed_slider_radius;
	if (i == slider.NestedHitObjects.Count - 1)
	{
	// The end of a slider has special aim rules due to the relaxed time constraint on position.
	// There is both a lazy end position as well as the actual end slider position. We assume the player takes the simpler movement.
	// For sliders that are circular, the lazy end position may actually be farther away than the sliders true end.
	// This code is designed to prevent buffing situations where lazy end is actually a less efficient movement.
	Vector2 lazyMovement = Vector2.Subtract((Vector2)slider.LazyEndPosition, currCursorPosition);
	if (lazyMovement.Length < currMovement.Length)
	currMovement = lazyMovement;
	currMovementLength = scalingFactor * currMovement.Length;
	}
	else if (currMovementObj is SliderRepeat)
	{
	// For a slider repeat, assume a tighter movement threshold to better assess repeat sliders.
	requiredMovement = NORMALISED_RADIUS;
	}
	if (currMovementLength > requiredMovement)
	{
	// this finds the positional delta from the required radius and the current position, and updates the currCursorPosition accordingly, as well as rewarding distance.
	currCursorPosition = Vector2.Add(currCursorPosition, Vector2.Multiply(currMovement, (float)((currMovementLength - requiredMovement) / currMovementLength)));
	currMovementLength *= (currMovementLength - requiredMovement) / currMovementLength;
	slider.LazyTravelDistance += (float)currMovementLength;
	}
	if (i == slider.NestedHitObjects.Count - 1)
	slider.LazyEndPosition = currCursorPosition;
	}
	}

But it's likely that the difference is too small to matter. I agree with this change regardless.

somehow rig stable to extract conversion mappings for all ranked maps / all maps period and try to cross-test that against this branch?

I think this would be interesting to do but I wouldn't hold it as a blocker for this PR.