From ecf64dfc5796eb3526f84fcf763512fa6c57f1e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 27 Dec 2024 12:38:15 +0100 Subject: [PATCH 1/2] Add failing test case --- .../Beatmaps/SliderEventGenerationTest.cs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/osu.Game.Tests/Beatmaps/SliderEventGenerationTest.cs b/osu.Game.Tests/Beatmaps/SliderEventGenerationTest.cs index c7cf3fe956..ee2733ad91 100644 --- a/osu.Game.Tests/Beatmaps/SliderEventGenerationTest.cs +++ b/osu.Game.Tests/Beatmaps/SliderEventGenerationTest.cs @@ -112,5 +112,20 @@ namespace osu.Game.Tests.Beatmaps } }); } + + [Test] + public void TestRepeatsGeneratedEvenForZeroLengthSlider() + { + var events = SliderEventGenerator.Generate(start_time, span_duration, 1, span_duration / 2, 0, 2).ToArray(); + + Assert.That(events[0].Type, Is.EqualTo(SliderEventType.Head)); + Assert.That(events[0].Time, Is.EqualTo(start_time)); + + Assert.That(events[1].Type, Is.EqualTo(SliderEventType.Repeat)); + Assert.That(events[1].Time, Is.EqualTo(span_duration)); + + Assert.That(events[3].Type, Is.EqualTo(SliderEventType.Tail)); + Assert.That(events[3].Time, Is.EqualTo(span_duration * 2)); + } } } From e7225399a282c4f7194dd5ef9453ee3f52dd25ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 27 Dec 2024 12:25:51 +0100 Subject: [PATCH 2/2] Fix slider event generator incorrectly not generating repeats when tick distance is zero RFC. This closes https://github.com/ppy/osu/issues/31186. To explain why: The issue occurs on https://osu.ppy.sh/beatmapsets/594828#osu/1258033, specifically on the slider at time 128604. The failure site is https://github.com/ppy/osu/blob/fa0d2f4af22fb9319e2a8773bf635368d86360be/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/SliderCircleOverlay.cs#L65-L66 wherein `LastRepeat` is `null`, even though the slider's `RepeatCount` is 1 and thus `SpanCount` is 2. In this case, `SliderEventGenerator` is given a non-zero `tickDistance` but a zero `length`. The former is clamped to the latter: https://github.com/ppy/osu/blob/fa0d2f4af22fb9319e2a8773bf635368d86360be/osu.Game/Rulesets/Objects/SliderEventGenerator.cs#L34 Because of this, a whole block of code pertaining to tick generation gets turned off, because of zero tick spacing - however, that block also includes within it *repeat* generation, for seemingly very little reason whatsoever: https://github.com/ppy/osu/blob/fa0d2f4af22fb9319e2a8773bf635368d86360be/osu.Game/Rulesets/Objects/SliderEventGenerator.cs#L47-L77 While a zero tick distance would indeed cause `generateTicks()` to loop forever, it should have absolutely no effect on repeats. While this *is* ultimately an aspire-tier bug caused by people pushing things to limits, I do believe that in this case a fix is warranted because of how hard the current behaviour violates invariants. I do not like the possibility of having a slider with multiple spans and no repeats. --- .../Rulesets/Objects/SliderEventGenerator.cs | 30 +++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/osu.Game/Rulesets/Objects/SliderEventGenerator.cs b/osu.Game/Rulesets/Objects/SliderEventGenerator.cs index 9b8375f208..f5146d1675 100644 --- a/osu.Game/Rulesets/Objects/SliderEventGenerator.cs +++ b/osu.Game/Rulesets/Objects/SliderEventGenerator.cs @@ -44,13 +44,13 @@ namespace osu.Game.Rulesets.Objects PathProgress = 0, }; - if (tickDistance != 0) + for (int span = 0; span < spanCount; span++) { - for (int span = 0; span < spanCount; span++) - { - double spanStartTime = startTime + span * spanDuration; - bool reversed = span % 2 == 1; + double spanStartTime = startTime + span * spanDuration; + bool reversed = span % 2 == 1; + if (tickDistance != 0) + { var ticks = generateTicks(span, spanStartTime, spanDuration, reversed, length, tickDistance, minDistanceFromEnd, cancellationToken); if (reversed) @@ -61,18 +61,18 @@ namespace osu.Game.Rulesets.Objects foreach (var e in ticks) yield return e; + } - if (span < spanCount - 1) + if (span < spanCount - 1) + { + yield return new SliderEventDescriptor { - yield return new SliderEventDescriptor - { - Type = SliderEventType.Repeat, - SpanIndex = span, - SpanStartTime = startTime + span * spanDuration, - Time = spanStartTime + spanDuration, - PathProgress = (span + 1) % 2, - }; - } + Type = SliderEventType.Repeat, + SpanIndex = span, + SpanStartTime = startTime + span * spanDuration, + Time = spanStartTime + spanDuration, + PathProgress = (span + 1) % 2, + }; } }