From e8d4bc4497c3fe3f7f2606db471a48fce87393cb Mon Sep 17 00:00:00 2001 From: Khang Date: Mon, 22 Aug 2022 21:04:26 -0400 Subject: [PATCH 01/12] Allow NaN during beatmap parsing if desired --- osu.Game.Tests/Beatmaps/Formats/ParsingTest.cs | 7 ++++++- osu.Game/Beatmaps/Formats/Parsing.cs | 8 ++++---- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/osu.Game.Tests/Beatmaps/Formats/ParsingTest.cs b/osu.Game.Tests/Beatmaps/Formats/ParsingTest.cs index f3673b0e7fcb..339063633ada 100644 --- a/osu.Game.Tests/Beatmaps/Formats/ParsingTest.cs +++ b/osu.Game.Tests/Beatmaps/Formats/ParsingTest.cs @@ -14,7 +14,12 @@ namespace osu.Game.Tests.Beatmaps.Formats public class ParsingTest { [Test] - public void TestNaNHandling() => allThrow("NaN"); + public void TestNaNHandling() + { + allThrow("NaN"); + Assert.That(Parsing.ParseFloat("NaN", allowNaN: true), Is.NaN); + Assert.That(Parsing.ParseDouble("NaN", allowNaN: true), Is.NaN); + } [Test] public void TestBadStringHandling() => allThrow("Random string 123"); diff --git a/osu.Game/Beatmaps/Formats/Parsing.cs b/osu.Game/Beatmaps/Formats/Parsing.cs index ce04f2702015..9b0d20007766 100644 --- a/osu.Game/Beatmaps/Formats/Parsing.cs +++ b/osu.Game/Beatmaps/Formats/Parsing.cs @@ -17,26 +17,26 @@ public static class Parsing public const double MAX_PARSE_VALUE = int.MaxValue; - public static float ParseFloat(string input, float parseLimit = (float)MAX_PARSE_VALUE) + public static float ParseFloat(string input, float parseLimit = (float)MAX_PARSE_VALUE, bool allowNaN = false) { float output = float.Parse(input, CultureInfo.InvariantCulture); if (output < -parseLimit) throw new OverflowException("Value is too low"); if (output > parseLimit) throw new OverflowException("Value is too high"); - if (float.IsNaN(output)) throw new FormatException("Not a number"); + if (!allowNaN && float.IsNaN(output)) throw new FormatException("Not a number"); return output; } - public static double ParseDouble(string input, double parseLimit = MAX_PARSE_VALUE) + public static double ParseDouble(string input, double parseLimit = MAX_PARSE_VALUE, bool allowNaN = false) { double output = double.Parse(input, CultureInfo.InvariantCulture); if (output < -parseLimit) throw new OverflowException("Value is too low"); if (output > parseLimit) throw new OverflowException("Value is too high"); - if (double.IsNaN(output)) throw new FormatException("Not a number"); + if (!allowNaN && double.IsNaN(output)) throw new FormatException("Not a number"); return output; } From 9f08c474caf57d8a84a510653c72a7f01ccf2b8d Mon Sep 17 00:00:00 2001 From: Khang Date: Mon, 22 Aug 2022 21:44:25 -0400 Subject: [PATCH 02/12] Treat NaN slider velocity timing points as 1.0x but without slider ticks --- .../Objects/JuiceStream.cs | 5 ++++- osu.Game.Rulesets.Mania/Objects/HoldNote.cs | 2 +- .../Mods/OsuModStrictTracking.cs | 2 +- osu.Game.Rulesets.Osu/Objects/Slider.cs | 8 ++++++- osu.Game.Rulesets.Taiko/Objects/DrumRoll.cs | 2 +- .../Beatmaps/SliderEventGenerationTest.cs | 17 ++++++++++----- .../ControlPoints/DifficultyControlPoint.cs | 15 ++++++++++--- .../Beatmaps/Formats/LegacyBeatmapDecoder.cs | 8 ++++++- .../Rulesets/Objects/SliderEventGenerator.cs | 21 +++++++++++-------- 9 files changed, 57 insertions(+), 23 deletions(-) diff --git a/osu.Game.Rulesets.Catch/Objects/JuiceStream.cs b/osu.Game.Rulesets.Catch/Objects/JuiceStream.cs index 311e15116e7b..a9c7d938d23e 100644 --- a/osu.Game.Rulesets.Catch/Objects/JuiceStream.cs +++ b/osu.Game.Rulesets.Catch/Objects/JuiceStream.cs @@ -40,6 +40,9 @@ public class JuiceStream : CatchHitObject, IHasPathWithRepeats [JsonIgnore] public double TickDistance => tickDistanceFactor * DifficultyControlPoint.SliderVelocity; + [JsonIgnore] + public bool GenerateTicks => DifficultyControlPoint.GenerateTicks; + /// /// The length of one span of this . /// @@ -64,7 +67,7 @@ protected override void CreateNestedHitObjects(CancellationToken cancellationTok int nodeIndex = 0; SliderEventDescriptor? lastEvent = null; - foreach (var e in SliderEventGenerator.Generate(StartTime, SpanDuration, Velocity, TickDistance, Path.Distance, this.SpanCount(), LegacyLastTickOffset, cancellationToken)) + foreach (var e in SliderEventGenerator.Generate(StartTime, SpanDuration, Velocity, TickDistance, Path.Distance, this.SpanCount(), LegacyLastTickOffset, GenerateTicks, cancellationToken)) { // generate tiny droplets since the last point if (lastEvent != null) diff --git a/osu.Game.Rulesets.Mania/Objects/HoldNote.cs b/osu.Game.Rulesets.Mania/Objects/HoldNote.cs index 22fab15c1b09..4ec039c40572 100644 --- a/osu.Game.Rulesets.Mania/Objects/HoldNote.cs +++ b/osu.Game.Rulesets.Mania/Objects/HoldNote.cs @@ -117,7 +117,7 @@ protected override void CreateNestedHitObjects(CancellationToken cancellationTok private void createTicks(CancellationToken cancellationToken) { - if (tickSpacing == 0) + if (tickSpacing == 0 || !DifficultyControlPoint.GenerateTicks) return; for (double t = StartTime + tickSpacing; t <= EndTime - tickSpacing; t += tickSpacing) diff --git a/osu.Game.Rulesets.Osu/Mods/OsuModStrictTracking.cs b/osu.Game.Rulesets.Osu/Mods/OsuModStrictTracking.cs index 67b19124e1cb..eb59122686cf 100644 --- a/osu.Game.Rulesets.Osu/Mods/OsuModStrictTracking.cs +++ b/osu.Game.Rulesets.Osu/Mods/OsuModStrictTracking.cs @@ -102,7 +102,7 @@ public StrictTrackingSlider(Slider original) protected override void CreateNestedHitObjects(CancellationToken cancellationToken) { - var sliderEvents = SliderEventGenerator.Generate(StartTime, SpanDuration, Velocity, TickDistance, Path.Distance, this.SpanCount(), LegacyLastTickOffset, cancellationToken); + var sliderEvents = SliderEventGenerator.Generate(StartTime, SpanDuration, Velocity, TickDistance, Path.Distance, this.SpanCount(), LegacyLastTickOffset, GenerateTicks, cancellationToken); foreach (var e in sliderEvents) { diff --git a/osu.Game.Rulesets.Osu/Objects/Slider.cs b/osu.Game.Rulesets.Osu/Objects/Slider.cs index a5468ff613b8..31117c0836af 100644 --- a/osu.Game.Rulesets.Osu/Objects/Slider.cs +++ b/osu.Game.Rulesets.Osu/Objects/Slider.cs @@ -142,6 +142,11 @@ public int RepeatCount /// public double TickDistanceMultiplier = 1; + /// + /// Whether this should generate s. + /// + public bool GenerateTicks { get; private set; } + /// /// Whether this 's judgement is fully handled by its nested s. /// If false, this will be judged proportionally to the number of nested s hit. @@ -170,13 +175,14 @@ protected override void ApplyDefaultsToSelf(ControlPointInfo controlPointInfo, I Velocity = scoringDistance / timingPoint.BeatLength; TickDistance = scoringDistance / difficulty.SliderTickRate * TickDistanceMultiplier; + GenerateTicks = DifficultyControlPoint.GenerateTicks; } protected override void CreateNestedHitObjects(CancellationToken cancellationToken) { base.CreateNestedHitObjects(cancellationToken); - var sliderEvents = SliderEventGenerator.Generate(StartTime, SpanDuration, Velocity, TickDistance, Path.Distance, this.SpanCount(), LegacyLastTickOffset, cancellationToken); + var sliderEvents = SliderEventGenerator.Generate(StartTime, SpanDuration, Velocity, TickDistance, Path.Distance, this.SpanCount(), LegacyLastTickOffset, GenerateTicks, cancellationToken); foreach (var e in sliderEvents) { diff --git a/osu.Game.Rulesets.Taiko/Objects/DrumRoll.cs b/osu.Game.Rulesets.Taiko/Objects/DrumRoll.cs index e1619e285782..09e465c37bb0 100644 --- a/osu.Game.Rulesets.Taiko/Objects/DrumRoll.cs +++ b/osu.Game.Rulesets.Taiko/Objects/DrumRoll.cs @@ -85,7 +85,7 @@ protected override void CreateNestedHitObjects(CancellationToken cancellationTok private void createTicks(CancellationToken cancellationToken) { - if (tickSpacing == 0) + if (tickSpacing == 0 || !DifficultyControlPoint.GenerateTicks) return; bool first = true; diff --git a/osu.Game.Tests/Beatmaps/SliderEventGenerationTest.cs b/osu.Game.Tests/Beatmaps/SliderEventGenerationTest.cs index d30ab3dea1e9..077398f0156e 100644 --- a/osu.Game.Tests/Beatmaps/SliderEventGenerationTest.cs +++ b/osu.Game.Tests/Beatmaps/SliderEventGenerationTest.cs @@ -18,7 +18,7 @@ public class SliderEventGenerationTest [Test] public void TestSingleSpan() { - var events = SliderEventGenerator.Generate(start_time, span_duration, 1, span_duration / 2, span_duration, 1, null).ToArray(); + var events = SliderEventGenerator.Generate(start_time, span_duration, 1, span_duration / 2, span_duration, 1, null, true).ToArray(); Assert.That(events[0].Type, Is.EqualTo(SliderEventType.Head)); Assert.That(events[0].Time, Is.EqualTo(start_time)); @@ -33,7 +33,7 @@ public void TestSingleSpan() [Test] public void TestRepeat() { - var events = SliderEventGenerator.Generate(start_time, span_duration, 1, span_duration / 2, span_duration, 2, null).ToArray(); + var events = SliderEventGenerator.Generate(start_time, span_duration, 1, span_duration / 2, span_duration, 2, null, true).ToArray(); Assert.That(events[0].Type, Is.EqualTo(SliderEventType.Head)); Assert.That(events[0].Time, Is.EqualTo(start_time)); @@ -54,7 +54,7 @@ public void TestRepeat() [Test] public void TestNonEvenTicks() { - var events = SliderEventGenerator.Generate(start_time, span_duration, 1, 300, span_duration, 2, null).ToArray(); + var events = SliderEventGenerator.Generate(start_time, span_duration, 1, 300, span_duration, 2, null, true).ToArray(); Assert.That(events[0].Type, Is.EqualTo(SliderEventType.Head)); Assert.That(events[0].Time, Is.EqualTo(start_time)); @@ -87,7 +87,7 @@ public void TestNonEvenTicks() [Test] public void TestLegacyLastTickOffset() { - var events = SliderEventGenerator.Generate(start_time, span_duration, 1, span_duration / 2, span_duration, 1, 100).ToArray(); + var events = SliderEventGenerator.Generate(start_time, span_duration, 1, span_duration / 2, span_duration, 1, 100, true).ToArray(); Assert.That(events[2].Type, Is.EqualTo(SliderEventType.LegacyLastTick)); Assert.That(events[2].Time, Is.EqualTo(900)); @@ -99,7 +99,7 @@ public void TestMinimumTickDistance() const double velocity = 5; const double min_distance = velocity * 10; - var events = SliderEventGenerator.Generate(start_time, span_duration, velocity, velocity, span_duration, 2, 0).ToArray(); + var events = SliderEventGenerator.Generate(start_time, span_duration, velocity, velocity, span_duration, 2, 0, true).ToArray(); Assert.Multiple(() => { @@ -114,5 +114,12 @@ public void TestMinimumTickDistance() } }); } + + [Test] + public void TestNoTickGeneration() + { + var events = SliderEventGenerator.Generate(start_time, span_duration, 1, span_duration / 2, span_duration, 1, null, false).ToArray(); + Assert.That(events.Any(e => e.Type == SliderEventType.Tick), Is.False); + } } } diff --git a/osu.Game/Beatmaps/ControlPoints/DifficultyControlPoint.cs b/osu.Game/Beatmaps/ControlPoints/DifficultyControlPoint.cs index c199d1da5905..dfa35524698f 100644 --- a/osu.Game/Beatmaps/ControlPoints/DifficultyControlPoint.cs +++ b/osu.Game/Beatmaps/ControlPoints/DifficultyControlPoint.cs @@ -40,13 +40,21 @@ public double SliderVelocity set => SliderVelocityBindable.Value = value; } + /// + /// Whether or not slider ticks should be generated at this control point. + /// This exists for backwards compatibility with maps that abuse NaN slider velocity behavior on osu!stable (e.g. /b/2628991). + /// + public bool GenerateTicks { get; set; } = true; + public override bool IsRedundant(ControlPoint? existing) => existing is DifficultyControlPoint existingDifficulty - && SliderVelocity == existingDifficulty.SliderVelocity; + && SliderVelocity == existingDifficulty.SliderVelocity + && GenerateTicks == existingDifficulty.GenerateTicks; public override void CopyFrom(ControlPoint other) { SliderVelocity = ((DifficultyControlPoint)other).SliderVelocity; + GenerateTicks = ((DifficultyControlPoint)other).GenerateTicks; base.CopyFrom(other); } @@ -57,8 +65,9 @@ public override bool Equals(ControlPoint? other) public bool Equals(DifficultyControlPoint? other) => base.Equals(other) - && SliderVelocity == other.SliderVelocity; + && SliderVelocity == other.SliderVelocity + && GenerateTicks == other.GenerateTicks; - public override int GetHashCode() => HashCode.Combine(base.GetHashCode(), SliderVelocity); + public override int GetHashCode() => HashCode.Combine(base.GetHashCode(), SliderVelocity, GenerateTicks); } } diff --git a/osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs b/osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs index 89d3465ab67d..3d3661745ad8 100644 --- a/osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs +++ b/osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs @@ -373,7 +373,9 @@ private void handleTimingPoint(string line) string[] split = line.Split(','); double time = getOffsetTime(Parsing.ParseDouble(split[0].Trim())); - double beatLength = Parsing.ParseDouble(split[1].Trim()); + double beatLength = Parsing.ParseDouble(split[1].Trim(), allowNaN: true); + + // If beatLength is NaN, speedMultiplier should still be 1 because all comparisons against NaN are false. double speedMultiplier = beatLength < 0 ? 100.0 / -beatLength : 1; TimeSignature timeSignature = TimeSignature.SimpleQuadruple; @@ -412,6 +414,9 @@ private void handleTimingPoint(string line) if (timingChange) { + if (double.IsNaN(beatLength)) + throw new InvalidDataException("Beat length cannot be NaN in a timing control point"); + var controlPoint = CreateTimingControlPoint(); controlPoint.BeatLength = beatLength; @@ -425,6 +430,7 @@ private void handleTimingPoint(string line) #pragma warning restore 618 { SliderVelocity = speedMultiplier, + GenerateTicks = !double.IsNaN(beatLength), }, timingChange); var effectPoint = new EffectControlPoint diff --git a/osu.Game/Rulesets/Objects/SliderEventGenerator.cs b/osu.Game/Rulesets/Objects/SliderEventGenerator.cs index d32a7cb16d51..b77d15d56558 100644 --- a/osu.Game/Rulesets/Objects/SliderEventGenerator.cs +++ b/osu.Game/Rulesets/Objects/SliderEventGenerator.cs @@ -14,7 +14,7 @@ public static class SliderEventGenerator { // ReSharper disable once MethodOverloadWithOptionalParameter public static IEnumerable Generate(double startTime, double spanDuration, double velocity, double tickDistance, double totalDistance, int spanCount, - double? legacyLastTickOffset, CancellationToken cancellationToken = default) + double? legacyLastTickOffset, bool shouldGenerateTicks, CancellationToken cancellationToken = default) { // A very lenient maximum length of a slider for ticks to be generated. // This exists for edge cases such as /b/1573664 where the beatmap has been edited by the user, and should never be reached in normal usage. @@ -41,16 +41,19 @@ public static IEnumerable Generate(double startTime, doub double spanStartTime = startTime + span * spanDuration; bool reversed = span % 2 == 1; - var ticks = generateTicks(span, spanStartTime, spanDuration, reversed, length, tickDistance, minDistanceFromEnd, cancellationToken); - - if (reversed) + if (shouldGenerateTicks) { - // For repeat spans, ticks are returned in reverse-StartTime order, which is undesirable for some rulesets - ticks = ticks.Reverse(); - } + var ticks = generateTicks(span, spanStartTime, spanDuration, reversed, length, tickDistance, minDistanceFromEnd, cancellationToken); - foreach (var e in ticks) - yield return e; + if (reversed) + { + // For repeat spans, ticks are returned in reverse-StartTime order, which is undesirable for some rulesets + ticks = ticks.Reverse(); + } + + foreach (var e in ticks) + yield return e; + } if (span < spanCount - 1) { From 8f708c1dcf30e7300b8f0c71ab48989a234f5cda Mon Sep 17 00:00:00 2001 From: Khang Date: Mon, 22 Aug 2022 22:43:44 -0400 Subject: [PATCH 03/12] Turn GenerateTicks into a bindable to pass code quality tests --- .../ControlPoints/DifficultyControlPoint.cs | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/osu.Game/Beatmaps/ControlPoints/DifficultyControlPoint.cs b/osu.Game/Beatmaps/ControlPoints/DifficultyControlPoint.cs index dfa35524698f..ca49d316e0d2 100644 --- a/osu.Game/Beatmaps/ControlPoints/DifficultyControlPoint.cs +++ b/osu.Game/Beatmaps/ControlPoints/DifficultyControlPoint.cs @@ -16,6 +16,7 @@ public class DifficultyControlPoint : ControlPoint, IEquatable @@ -29,6 +30,12 @@ public class DifficultyControlPoint : ControlPoint, IEquatable + /// Whether or not slider ticks should be generated at this control point. + /// This exists for backwards compatibility with maps that abuse NaN slider velocity behavior on osu!stable (e.g. /b/2628991). + /// + public readonly BindableBool GenerateTicksBindable = new BindableBool(true); + public override Color4 GetRepresentingColour(OsuColour colours) => colours.Lime1; /// @@ -44,7 +51,11 @@ public double SliderVelocity /// Whether or not slider ticks should be generated at this control point. /// This exists for backwards compatibility with maps that abuse NaN slider velocity behavior on osu!stable (e.g. /b/2628991). /// - public bool GenerateTicks { get; set; } = true; + public bool GenerateTicks + { + get => GenerateTicksBindable.Value; + set => GenerateTicksBindable.Value = value; + } public override bool IsRedundant(ControlPoint? existing) => existing is DifficultyControlPoint existingDifficulty From a81672f3dc9e885651f78852675fadb76940ace2 Mon Sep 17 00:00:00 2001 From: Khang Date: Mon, 22 Aug 2022 23:31:24 -0400 Subject: [PATCH 04/12] Use an infinite tick distance instead of directly disabling tick generation for SliderEventGenerator --- .../Objects/JuiceStream.cs | 5 +---- .../Mods/OsuModStrictTracking.cs | 2 +- osu.Game.Rulesets.Osu/Objects/Slider.cs | 10 ++------- .../Beatmaps/SliderEventGenerationTest.cs | 17 +++++---------- .../Rulesets/Objects/SliderEventGenerator.cs | 21 ++++++++----------- 5 files changed, 18 insertions(+), 37 deletions(-) diff --git a/osu.Game.Rulesets.Catch/Objects/JuiceStream.cs b/osu.Game.Rulesets.Catch/Objects/JuiceStream.cs index a9c7d938d23e..311e15116e7b 100644 --- a/osu.Game.Rulesets.Catch/Objects/JuiceStream.cs +++ b/osu.Game.Rulesets.Catch/Objects/JuiceStream.cs @@ -40,9 +40,6 @@ public class JuiceStream : CatchHitObject, IHasPathWithRepeats [JsonIgnore] public double TickDistance => tickDistanceFactor * DifficultyControlPoint.SliderVelocity; - [JsonIgnore] - public bool GenerateTicks => DifficultyControlPoint.GenerateTicks; - /// /// The length of one span of this . /// @@ -67,7 +64,7 @@ protected override void CreateNestedHitObjects(CancellationToken cancellationTok int nodeIndex = 0; SliderEventDescriptor? lastEvent = null; - foreach (var e in SliderEventGenerator.Generate(StartTime, SpanDuration, Velocity, TickDistance, Path.Distance, this.SpanCount(), LegacyLastTickOffset, GenerateTicks, cancellationToken)) + foreach (var e in SliderEventGenerator.Generate(StartTime, SpanDuration, Velocity, TickDistance, Path.Distance, this.SpanCount(), LegacyLastTickOffset, cancellationToken)) { // generate tiny droplets since the last point if (lastEvent != null) diff --git a/osu.Game.Rulesets.Osu/Mods/OsuModStrictTracking.cs b/osu.Game.Rulesets.Osu/Mods/OsuModStrictTracking.cs index eb59122686cf..67b19124e1cb 100644 --- a/osu.Game.Rulesets.Osu/Mods/OsuModStrictTracking.cs +++ b/osu.Game.Rulesets.Osu/Mods/OsuModStrictTracking.cs @@ -102,7 +102,7 @@ public StrictTrackingSlider(Slider original) protected override void CreateNestedHitObjects(CancellationToken cancellationToken) { - var sliderEvents = SliderEventGenerator.Generate(StartTime, SpanDuration, Velocity, TickDistance, Path.Distance, this.SpanCount(), LegacyLastTickOffset, GenerateTicks, cancellationToken); + var sliderEvents = SliderEventGenerator.Generate(StartTime, SpanDuration, Velocity, TickDistance, Path.Distance, this.SpanCount(), LegacyLastTickOffset, cancellationToken); foreach (var e in sliderEvents) { diff --git a/osu.Game.Rulesets.Osu/Objects/Slider.cs b/osu.Game.Rulesets.Osu/Objects/Slider.cs index 31117c0836af..0689c49085d1 100644 --- a/osu.Game.Rulesets.Osu/Objects/Slider.cs +++ b/osu.Game.Rulesets.Osu/Objects/Slider.cs @@ -142,11 +142,6 @@ public int RepeatCount /// public double TickDistanceMultiplier = 1; - /// - /// Whether this should generate s. - /// - public bool GenerateTicks { get; private set; } - /// /// Whether this 's judgement is fully handled by its nested s. /// If false, this will be judged proportionally to the number of nested s hit. @@ -174,15 +169,14 @@ protected override void ApplyDefaultsToSelf(ControlPointInfo controlPointInfo, I double scoringDistance = BASE_SCORING_DISTANCE * difficulty.SliderMultiplier * DifficultyControlPoint.SliderVelocity; Velocity = scoringDistance / timingPoint.BeatLength; - TickDistance = scoringDistance / difficulty.SliderTickRate * TickDistanceMultiplier; - GenerateTicks = DifficultyControlPoint.GenerateTicks; + TickDistance = DifficultyControlPoint.GenerateTicks ? (scoringDistance / difficulty.SliderTickRate * TickDistanceMultiplier) : double.PositiveInfinity; } protected override void CreateNestedHitObjects(CancellationToken cancellationToken) { base.CreateNestedHitObjects(cancellationToken); - var sliderEvents = SliderEventGenerator.Generate(StartTime, SpanDuration, Velocity, TickDistance, Path.Distance, this.SpanCount(), LegacyLastTickOffset, GenerateTicks, cancellationToken); + var sliderEvents = SliderEventGenerator.Generate(StartTime, SpanDuration, Velocity, TickDistance, Path.Distance, this.SpanCount(), LegacyLastTickOffset, cancellationToken); foreach (var e in sliderEvents) { diff --git a/osu.Game.Tests/Beatmaps/SliderEventGenerationTest.cs b/osu.Game.Tests/Beatmaps/SliderEventGenerationTest.cs index 077398f0156e..d30ab3dea1e9 100644 --- a/osu.Game.Tests/Beatmaps/SliderEventGenerationTest.cs +++ b/osu.Game.Tests/Beatmaps/SliderEventGenerationTest.cs @@ -18,7 +18,7 @@ public class SliderEventGenerationTest [Test] public void TestSingleSpan() { - var events = SliderEventGenerator.Generate(start_time, span_duration, 1, span_duration / 2, span_duration, 1, null, true).ToArray(); + var events = SliderEventGenerator.Generate(start_time, span_duration, 1, span_duration / 2, span_duration, 1, null).ToArray(); Assert.That(events[0].Type, Is.EqualTo(SliderEventType.Head)); Assert.That(events[0].Time, Is.EqualTo(start_time)); @@ -33,7 +33,7 @@ public void TestSingleSpan() [Test] public void TestRepeat() { - var events = SliderEventGenerator.Generate(start_time, span_duration, 1, span_duration / 2, span_duration, 2, null, true).ToArray(); + var events = SliderEventGenerator.Generate(start_time, span_duration, 1, span_duration / 2, span_duration, 2, null).ToArray(); Assert.That(events[0].Type, Is.EqualTo(SliderEventType.Head)); Assert.That(events[0].Time, Is.EqualTo(start_time)); @@ -54,7 +54,7 @@ public void TestRepeat() [Test] public void TestNonEvenTicks() { - var events = SliderEventGenerator.Generate(start_time, span_duration, 1, 300, span_duration, 2, null, true).ToArray(); + var events = SliderEventGenerator.Generate(start_time, span_duration, 1, 300, span_duration, 2, null).ToArray(); Assert.That(events[0].Type, Is.EqualTo(SliderEventType.Head)); Assert.That(events[0].Time, Is.EqualTo(start_time)); @@ -87,7 +87,7 @@ public void TestNonEvenTicks() [Test] public void TestLegacyLastTickOffset() { - var events = SliderEventGenerator.Generate(start_time, span_duration, 1, span_duration / 2, span_duration, 1, 100, true).ToArray(); + var events = SliderEventGenerator.Generate(start_time, span_duration, 1, span_duration / 2, span_duration, 1, 100).ToArray(); Assert.That(events[2].Type, Is.EqualTo(SliderEventType.LegacyLastTick)); Assert.That(events[2].Time, Is.EqualTo(900)); @@ -99,7 +99,7 @@ public void TestMinimumTickDistance() const double velocity = 5; const double min_distance = velocity * 10; - var events = SliderEventGenerator.Generate(start_time, span_duration, velocity, velocity, span_duration, 2, 0, true).ToArray(); + var events = SliderEventGenerator.Generate(start_time, span_duration, velocity, velocity, span_duration, 2, 0).ToArray(); Assert.Multiple(() => { @@ -114,12 +114,5 @@ public void TestMinimumTickDistance() } }); } - - [Test] - public void TestNoTickGeneration() - { - var events = SliderEventGenerator.Generate(start_time, span_duration, 1, span_duration / 2, span_duration, 1, null, false).ToArray(); - Assert.That(events.Any(e => e.Type == SliderEventType.Tick), Is.False); - } } } diff --git a/osu.Game/Rulesets/Objects/SliderEventGenerator.cs b/osu.Game/Rulesets/Objects/SliderEventGenerator.cs index b77d15d56558..d32a7cb16d51 100644 --- a/osu.Game/Rulesets/Objects/SliderEventGenerator.cs +++ b/osu.Game/Rulesets/Objects/SliderEventGenerator.cs @@ -14,7 +14,7 @@ public static class SliderEventGenerator { // ReSharper disable once MethodOverloadWithOptionalParameter public static IEnumerable Generate(double startTime, double spanDuration, double velocity, double tickDistance, double totalDistance, int spanCount, - double? legacyLastTickOffset, bool shouldGenerateTicks, CancellationToken cancellationToken = default) + double? legacyLastTickOffset, CancellationToken cancellationToken = default) { // A very lenient maximum length of a slider for ticks to be generated. // This exists for edge cases such as /b/1573664 where the beatmap has been edited by the user, and should never be reached in normal usage. @@ -41,20 +41,17 @@ public static IEnumerable Generate(double startTime, doub double spanStartTime = startTime + span * spanDuration; bool reversed = span % 2 == 1; - if (shouldGenerateTicks) - { - var ticks = generateTicks(span, spanStartTime, spanDuration, reversed, length, tickDistance, minDistanceFromEnd, cancellationToken); - - if (reversed) - { - // For repeat spans, ticks are returned in reverse-StartTime order, which is undesirable for some rulesets - ticks = ticks.Reverse(); - } + var ticks = generateTicks(span, spanStartTime, spanDuration, reversed, length, tickDistance, minDistanceFromEnd, cancellationToken); - foreach (var e in ticks) - yield return e; + if (reversed) + { + // For repeat spans, ticks are returned in reverse-StartTime order, which is undesirable for some rulesets + ticks = ticks.Reverse(); } + foreach (var e in ticks) + yield return e; + if (span < spanCount - 1) { yield return new SliderEventDescriptor From fbe8de2757ca2a752163bd6a2d61a4d788b55301 Mon Sep 17 00:00:00 2001 From: Khang Date: Tue, 23 Aug 2022 00:57:25 -0400 Subject: [PATCH 05/12] Disable the GetHashCode warning instead of using bindables --- .../ControlPoints/DifficultyControlPoint.cs | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/osu.Game/Beatmaps/ControlPoints/DifficultyControlPoint.cs b/osu.Game/Beatmaps/ControlPoints/DifficultyControlPoint.cs index ca49d316e0d2..ddb2b1e95c16 100644 --- a/osu.Game/Beatmaps/ControlPoints/DifficultyControlPoint.cs +++ b/osu.Game/Beatmaps/ControlPoints/DifficultyControlPoint.cs @@ -16,7 +16,6 @@ public class DifficultyControlPoint : ControlPoint, IEquatable @@ -30,12 +29,6 @@ public class DifficultyControlPoint : ControlPoint, IEquatable - /// Whether or not slider ticks should be generated at this control point. - /// This exists for backwards compatibility with maps that abuse NaN slider velocity behavior on osu!stable (e.g. /b/2628991). - /// - public readonly BindableBool GenerateTicksBindable = new BindableBool(true); - public override Color4 GetRepresentingColour(OsuColour colours) => colours.Lime1; /// @@ -51,11 +44,7 @@ public double SliderVelocity /// Whether or not slider ticks should be generated at this control point. /// This exists for backwards compatibility with maps that abuse NaN slider velocity behavior on osu!stable (e.g. /b/2628991). /// - public bool GenerateTicks - { - get => GenerateTicksBindable.Value; - set => GenerateTicksBindable.Value = value; - } + public bool GenerateTicks { get; set; } = true; public override bool IsRedundant(ControlPoint? existing) => existing is DifficultyControlPoint existingDifficulty @@ -79,6 +68,7 @@ public bool Equals(DifficultyControlPoint? other) && SliderVelocity == other.SliderVelocity && GenerateTicks == other.GenerateTicks; + // ReSharper disable once NonReadonlyMemberInGetHashCode public override int GetHashCode() => HashCode.Combine(base.GetHashCode(), SliderVelocity, GenerateTicks); } } From c1ced85b5e4fd5afe86e064774fda164be0ce88c Mon Sep 17 00:00:00 2001 From: Khang Date: Tue, 23 Aug 2022 14:07:18 -0400 Subject: [PATCH 06/12] Move GenerateTicks to LegacyDifficultyControlPoint and remove support for NaN slider velocity support for other rulesets (at least for now) --- .../Objects/JuiceStream.cs | 6 +++++- osu.Game.Rulesets.Mania/Objects/HoldNote.cs | 2 +- osu.Game.Rulesets.Osu/Objects/Slider.cs | 10 +++++++++- osu.Game.Rulesets.Taiko/Objects/DrumRoll.cs | 2 +- .../ControlPoints/DifficultyControlPoint.cs | 16 +++------------ .../Beatmaps/Formats/LegacyBeatmapDecoder.cs | 1 - osu.Game/Beatmaps/Formats/LegacyDecoder.cs | 20 ++++++++++++++++--- 7 files changed, 36 insertions(+), 21 deletions(-) diff --git a/osu.Game.Rulesets.Catch/Objects/JuiceStream.cs b/osu.Game.Rulesets.Catch/Objects/JuiceStream.cs index 311e15116e7b..2a5564297c2b 100644 --- a/osu.Game.Rulesets.Catch/Objects/JuiceStream.cs +++ b/osu.Game.Rulesets.Catch/Objects/JuiceStream.cs @@ -11,6 +11,7 @@ using osu.Game.Audio; using osu.Game.Beatmaps; using osu.Game.Beatmaps.ControlPoints; +using osu.Game.Beatmaps.Formats; using osu.Game.Rulesets.Judgements; using osu.Game.Rulesets.Objects; using osu.Game.Rulesets.Objects.Types; @@ -50,9 +51,12 @@ protected override void ApplyDefaultsToSelf(ControlPointInfo controlPointInfo, I base.ApplyDefaultsToSelf(controlPointInfo, difficulty); TimingControlPoint timingPoint = controlPointInfo.TimingPointAt(StartTime); +#pragma warning disable 618 + bool generateTicks = (DifficultyControlPoint as LegacyBeatmapDecoder.LegacyDifficultyControlPoint)?.GenerateTicks ?? true; +#pragma warning restore 618 velocityFactor = base_scoring_distance * difficulty.SliderMultiplier / timingPoint.BeatLength; - tickDistanceFactor = base_scoring_distance * difficulty.SliderMultiplier / difficulty.SliderTickRate; + tickDistanceFactor = generateTicks ? (base_scoring_distance * difficulty.SliderMultiplier / difficulty.SliderTickRate) : double.PositiveInfinity; } protected override void CreateNestedHitObjects(CancellationToken cancellationToken) diff --git a/osu.Game.Rulesets.Mania/Objects/HoldNote.cs b/osu.Game.Rulesets.Mania/Objects/HoldNote.cs index 4ec039c40572..22fab15c1b09 100644 --- a/osu.Game.Rulesets.Mania/Objects/HoldNote.cs +++ b/osu.Game.Rulesets.Mania/Objects/HoldNote.cs @@ -117,7 +117,7 @@ protected override void CreateNestedHitObjects(CancellationToken cancellationTok private void createTicks(CancellationToken cancellationToken) { - if (tickSpacing == 0 || !DifficultyControlPoint.GenerateTicks) + if (tickSpacing == 0) return; for (double t = StartTime + tickSpacing; t <= EndTime - tickSpacing; t += tickSpacing) diff --git a/osu.Game.Rulesets.Osu/Objects/Slider.cs b/osu.Game.Rulesets.Osu/Objects/Slider.cs index 0689c49085d1..df1252c060a1 100644 --- a/osu.Game.Rulesets.Osu/Objects/Slider.cs +++ b/osu.Game.Rulesets.Osu/Objects/Slider.cs @@ -14,6 +14,8 @@ using osu.Game.Audio; using osu.Game.Beatmaps; using osu.Game.Beatmaps.ControlPoints; +using osu.Game.Beatmaps.Formats; +using osu.Game.Beatmaps.Legacy; using osu.Game.Rulesets.Judgements; using osu.Game.Rulesets.Osu.Judgements; using osu.Game.Rulesets.Scoring; @@ -166,10 +168,16 @@ protected override void ApplyDefaultsToSelf(ControlPointInfo controlPointInfo, I TimingControlPoint timingPoint = controlPointInfo.TimingPointAt(StartTime); + var legacyInfo = controlPointInfo as LegacyControlPointInfo; +#pragma warning disable 618 + var legacyDifficultyPoint = legacyInfo?.DifficultyPointAt(StartTime) as LegacyBeatmapDecoder.LegacyDifficultyControlPoint; +#pragma warning restore 618 + double scoringDistance = BASE_SCORING_DISTANCE * difficulty.SliderMultiplier * DifficultyControlPoint.SliderVelocity; + bool generateTicks = legacyDifficultyPoint?.GenerateTicks ?? true; Velocity = scoringDistance / timingPoint.BeatLength; - TickDistance = DifficultyControlPoint.GenerateTicks ? (scoringDistance / difficulty.SliderTickRate * TickDistanceMultiplier) : double.PositiveInfinity; + TickDistance = generateTicks ? (scoringDistance / difficulty.SliderTickRate * TickDistanceMultiplier) : double.PositiveInfinity; } protected override void CreateNestedHitObjects(CancellationToken cancellationToken) diff --git a/osu.Game.Rulesets.Taiko/Objects/DrumRoll.cs b/osu.Game.Rulesets.Taiko/Objects/DrumRoll.cs index 09e465c37bb0..e1619e285782 100644 --- a/osu.Game.Rulesets.Taiko/Objects/DrumRoll.cs +++ b/osu.Game.Rulesets.Taiko/Objects/DrumRoll.cs @@ -85,7 +85,7 @@ protected override void CreateNestedHitObjects(CancellationToken cancellationTok private void createTicks(CancellationToken cancellationToken) { - if (tickSpacing == 0 || !DifficultyControlPoint.GenerateTicks) + if (tickSpacing == 0) return; bool first = true; diff --git a/osu.Game/Beatmaps/ControlPoints/DifficultyControlPoint.cs b/osu.Game/Beatmaps/ControlPoints/DifficultyControlPoint.cs index ddb2b1e95c16..c199d1da5905 100644 --- a/osu.Game/Beatmaps/ControlPoints/DifficultyControlPoint.cs +++ b/osu.Game/Beatmaps/ControlPoints/DifficultyControlPoint.cs @@ -40,21 +40,13 @@ public double SliderVelocity set => SliderVelocityBindable.Value = value; } - /// - /// Whether or not slider ticks should be generated at this control point. - /// This exists for backwards compatibility with maps that abuse NaN slider velocity behavior on osu!stable (e.g. /b/2628991). - /// - public bool GenerateTicks { get; set; } = true; - public override bool IsRedundant(ControlPoint? existing) => existing is DifficultyControlPoint existingDifficulty - && SliderVelocity == existingDifficulty.SliderVelocity - && GenerateTicks == existingDifficulty.GenerateTicks; + && SliderVelocity == existingDifficulty.SliderVelocity; public override void CopyFrom(ControlPoint other) { SliderVelocity = ((DifficultyControlPoint)other).SliderVelocity; - GenerateTicks = ((DifficultyControlPoint)other).GenerateTicks; base.CopyFrom(other); } @@ -65,10 +57,8 @@ public override bool Equals(ControlPoint? other) public bool Equals(DifficultyControlPoint? other) => base.Equals(other) - && SliderVelocity == other.SliderVelocity - && GenerateTicks == other.GenerateTicks; + && SliderVelocity == other.SliderVelocity; - // ReSharper disable once NonReadonlyMemberInGetHashCode - public override int GetHashCode() => HashCode.Combine(base.GetHashCode(), SliderVelocity, GenerateTicks); + public override int GetHashCode() => HashCode.Combine(base.GetHashCode(), SliderVelocity); } } diff --git a/osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs b/osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs index 3d3661745ad8..f064d89e1969 100644 --- a/osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs +++ b/osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs @@ -430,7 +430,6 @@ private void handleTimingPoint(string line) #pragma warning restore 618 { SliderVelocity = speedMultiplier, - GenerateTicks = !double.IsNaN(beatLength), }, timingChange); var effectPoint = new EffectControlPoint diff --git a/osu.Game/Beatmaps/Formats/LegacyDecoder.cs b/osu.Game/Beatmaps/Formats/LegacyDecoder.cs index 52e760a06860..c3fd16e86f96 100644 --- a/osu.Game/Beatmaps/Formats/LegacyDecoder.cs +++ b/osu.Game/Beatmaps/Formats/LegacyDecoder.cs @@ -168,11 +168,18 @@ public class LegacyDifficultyControlPoint : DifficultyControlPoint, IEquatable public double BpmMultiplier { get; private set; } + /// + /// Whether or not slider ticks should be generated at this control point. + /// This exists for backwards compatibility with maps that abuse NaN slider velocity behavior on osu!stable (e.g. /b/2628991). + /// + public bool GenerateTicks { get; private set; } = true; + public LegacyDifficultyControlPoint(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?). BpmMultiplier = beatLength < 0 ? Math.Clamp((float)-beatLength, 10, 10000) / 100.0 : 1; + GenerateTicks = !double.IsNaN(beatLength); } public LegacyDifficultyControlPoint() @@ -180,11 +187,17 @@ public LegacyDifficultyControlPoint() SliderVelocityBindable.Precision = double.Epsilon; } + public override bool IsRedundant(ControlPoint? existing) + => existing is LegacyDifficultyControlPoint existingLegacyDifficulty + && base.IsRedundant(existing) + && GenerateTicks == existingLegacyDifficulty.GenerateTicks; + public override void CopyFrom(ControlPoint other) { base.CopyFrom(other); BpmMultiplier = ((LegacyDifficultyControlPoint)other).BpmMultiplier; + GenerateTicks = ((LegacyDifficultyControlPoint)other).GenerateTicks; } public override bool Equals(ControlPoint? other) @@ -193,10 +206,11 @@ public override bool Equals(ControlPoint? other) public bool Equals(LegacyDifficultyControlPoint? other) => base.Equals(other) - && BpmMultiplier == other.BpmMultiplier; + && BpmMultiplier == other.BpmMultiplier + && GenerateTicks == other.GenerateTicks; - // ReSharper disable once NonReadonlyMemberInGetHashCode - public override int GetHashCode() => HashCode.Combine(base.GetHashCode(), BpmMultiplier); + // ReSharper disable twice NonReadonlyMemberInGetHashCode + public override int GetHashCode() => HashCode.Combine(base.GetHashCode(), BpmMultiplier, GenerateTicks); } internal class LegacySampleControlPoint : SampleControlPoint, IEquatable From ada61e6fe7121960b2f531ff08bb29379ade63b1 Mon Sep 17 00:00:00 2001 From: Khang Date: Tue, 23 Aug 2022 14:10:26 -0400 Subject: [PATCH 07/12] Forgot to undo broken changes in JuiceStream --- osu.Game.Rulesets.Catch/Objects/JuiceStream.cs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/osu.Game.Rulesets.Catch/Objects/JuiceStream.cs b/osu.Game.Rulesets.Catch/Objects/JuiceStream.cs index 2a5564297c2b..311e15116e7b 100644 --- a/osu.Game.Rulesets.Catch/Objects/JuiceStream.cs +++ b/osu.Game.Rulesets.Catch/Objects/JuiceStream.cs @@ -11,7 +11,6 @@ using osu.Game.Audio; using osu.Game.Beatmaps; using osu.Game.Beatmaps.ControlPoints; -using osu.Game.Beatmaps.Formats; using osu.Game.Rulesets.Judgements; using osu.Game.Rulesets.Objects; using osu.Game.Rulesets.Objects.Types; @@ -51,12 +50,9 @@ protected override void ApplyDefaultsToSelf(ControlPointInfo controlPointInfo, I base.ApplyDefaultsToSelf(controlPointInfo, difficulty); TimingControlPoint timingPoint = controlPointInfo.TimingPointAt(StartTime); -#pragma warning disable 618 - bool generateTicks = (DifficultyControlPoint as LegacyBeatmapDecoder.LegacyDifficultyControlPoint)?.GenerateTicks ?? true; -#pragma warning restore 618 velocityFactor = base_scoring_distance * difficulty.SliderMultiplier / timingPoint.BeatLength; - tickDistanceFactor = generateTicks ? (base_scoring_distance * difficulty.SliderMultiplier / difficulty.SliderTickRate) : double.PositiveInfinity; + tickDistanceFactor = base_scoring_distance * difficulty.SliderMultiplier / difficulty.SliderTickRate; } protected override void CreateNestedHitObjects(CancellationToken cancellationToken) From ec9daec93bcea47e7983564db3fb75b1c531733a Mon Sep 17 00:00:00 2001 From: Khang Date: Tue, 23 Aug 2022 14:18:40 -0400 Subject: [PATCH 08/12] Remove unnecessary workaround --- osu.Game.Rulesets.Osu/Objects/Slider.cs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Objects/Slider.cs b/osu.Game.Rulesets.Osu/Objects/Slider.cs index df1252c060a1..a7495a280905 100644 --- a/osu.Game.Rulesets.Osu/Objects/Slider.cs +++ b/osu.Game.Rulesets.Osu/Objects/Slider.cs @@ -167,10 +167,8 @@ protected override void ApplyDefaultsToSelf(ControlPointInfo controlPointInfo, I base.ApplyDefaultsToSelf(controlPointInfo, difficulty); TimingControlPoint timingPoint = controlPointInfo.TimingPointAt(StartTime); - - var legacyInfo = controlPointInfo as LegacyControlPointInfo; #pragma warning disable 618 - var legacyDifficultyPoint = legacyInfo?.DifficultyPointAt(StartTime) as LegacyBeatmapDecoder.LegacyDifficultyControlPoint; + var legacyDifficultyPoint = DifficultyControlPoint as LegacyBeatmapDecoder.LegacyDifficultyControlPoint; #pragma warning restore 618 double scoringDistance = BASE_SCORING_DISTANCE * difficulty.SliderMultiplier * DifficultyControlPoint.SliderVelocity; From c9f364d6a086c7582040c9b66a94eca09e8ce25e Mon Sep 17 00:00:00 2001 From: Khang Date: Wed, 24 Aug 2022 02:10:19 -0400 Subject: [PATCH 09/12] Document why beatLength can be NaN --- osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs b/osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs index f064d89e1969..75500fbc4ea5 100644 --- a/osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs +++ b/osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs @@ -373,6 +373,8 @@ private void handleTimingPoint(string line) string[] split = line.Split(','); double time = getOffsetTime(Parsing.ParseDouble(split[0].Trim())); + + // beatLength is allowed to be NaN to handle an edge case in which some beatmaps use NaN slider velocity to disable slider tick generation (see LegacyDifficultyControlPoint). double beatLength = Parsing.ParseDouble(split[1].Trim(), allowNaN: true); // If beatLength is NaN, speedMultiplier should still be 1 because all comparisons against NaN are false. From c7d4c739aa067825e89a97f11a8d3ae3e4965602 Mon Sep 17 00:00:00 2001 From: Khang Date: Wed, 24 Aug 2022 02:53:55 -0400 Subject: [PATCH 10/12] Add a basic NaN control point test for LegacyBeatmapDecoder --- .../Formats/LegacyBeatmapDecoderTest.cs | 27 +++++++++++++++++++ .../Resources/nan-control-points.osu | 15 +++++++++++ 2 files changed, 42 insertions(+) create mode 100644 osu.Game.Tests/Resources/nan-control-points.osu diff --git a/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapDecoderTest.cs b/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapDecoderTest.cs index 9acd3a6cab8c..7bd32eb5bd54 100644 --- a/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapDecoderTest.cs +++ b/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapDecoderTest.cs @@ -919,5 +919,32 @@ public void TestLegacyDuplicateInitialCatmullPointIsMerged() Assert.That(controlPoints[1].Position, Is.Not.EqualTo(Vector2.Zero)); } } + + [Test] + public void TestNaNControlPoints() + { + var decoder = new LegacyBeatmapDecoder { ApplyOffsets = false }; + + using (var resStream = TestResources.OpenResource("nan-control-points.osu")) + using (var stream = new LineBufferedReader(resStream)) + { + var controlPoints = (LegacyControlPointInfo)decoder.Decode(stream).ControlPointInfo; + + Assert.That(controlPoints.TimingPoints.Count, Is.EqualTo(1)); + Assert.That(controlPoints.DifficultyPoints.Count, Is.EqualTo(3)); + + Assert.That(controlPoints.TimingPointAt(1000).BeatLength, Is.EqualTo(500)); + + Assert.That(controlPoints.DifficultyPointAt(1000).SliderVelocity, Is.EqualTo(1)); + Assert.That(controlPoints.DifficultyPointAt(2000).SliderVelocity, Is.EqualTo(1)); + Assert.That(controlPoints.DifficultyPointAt(3000).SliderVelocity, Is.EqualTo(1)); + +#pragma warning disable 618 + Assert.That(((LegacyBeatmapDecoder.LegacyDifficultyControlPoint)controlPoints.DifficultyPointAt(1000)).GenerateTicks, Is.True); + Assert.That(((LegacyBeatmapDecoder.LegacyDifficultyControlPoint)controlPoints.DifficultyPointAt(2000)).GenerateTicks, Is.False); + Assert.That(((LegacyBeatmapDecoder.LegacyDifficultyControlPoint)controlPoints.DifficultyPointAt(3000)).GenerateTicks, Is.True); +#pragma warning restore 618 + } + } } } diff --git a/osu.Game.Tests/Resources/nan-control-points.osu b/osu.Game.Tests/Resources/nan-control-points.osu new file mode 100644 index 000000000000..dcaa705116ce --- /dev/null +++ b/osu.Game.Tests/Resources/nan-control-points.osu @@ -0,0 +1,15 @@ +osu file format v14 + +[TimingPoints] + +// NaN bpm (should be rejected) +0,NaN,4,2,0,100,1,0 + +// 120 bpm +1000,500,4,2,0,100,1,0 + +// NaN slider velocity +2000,NaN,4,3,0,100,0,1 + +// 1.0x slider velocity +3000,-100,4,3,0,100,0,1 \ No newline at end of file From 9c6968e96dfb322cbcc08a17dcd5ed0dbaf6ea00 Mon Sep 17 00:00:00 2001 From: Khang Date: Wed, 24 Aug 2022 02:54:40 -0400 Subject: [PATCH 11/12] Remove unused import in Slider --- osu.Game.Rulesets.Osu/Objects/Slider.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/osu.Game.Rulesets.Osu/Objects/Slider.cs b/osu.Game.Rulesets.Osu/Objects/Slider.cs index a7495a280905..e3c1b1e16808 100644 --- a/osu.Game.Rulesets.Osu/Objects/Slider.cs +++ b/osu.Game.Rulesets.Osu/Objects/Slider.cs @@ -15,7 +15,6 @@ using osu.Game.Beatmaps; using osu.Game.Beatmaps.ControlPoints; using osu.Game.Beatmaps.Formats; -using osu.Game.Beatmaps.Legacy; using osu.Game.Rulesets.Judgements; using osu.Game.Rulesets.Osu.Judgements; using osu.Game.Rulesets.Scoring; From adea29c10604fb50a91d680e751f744c84cfe559 Mon Sep 17 00:00:00 2001 From: Khang Date: Wed, 24 Aug 2022 03:37:33 -0400 Subject: [PATCH 12/12] Fix test failures --- osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapDecoderTest.cs | 4 +--- osu.Game/Beatmaps/Formats/LegacyDecoder.cs | 5 ++--- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapDecoderTest.cs b/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapDecoderTest.cs index 7bd32eb5bd54..9fc1eb7650e3 100644 --- a/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapDecoderTest.cs +++ b/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapDecoderTest.cs @@ -931,16 +931,14 @@ public void TestNaNControlPoints() var controlPoints = (LegacyControlPointInfo)decoder.Decode(stream).ControlPointInfo; Assert.That(controlPoints.TimingPoints.Count, Is.EqualTo(1)); - Assert.That(controlPoints.DifficultyPoints.Count, Is.EqualTo(3)); + Assert.That(controlPoints.DifficultyPoints.Count, Is.EqualTo(2)); Assert.That(controlPoints.TimingPointAt(1000).BeatLength, Is.EqualTo(500)); - Assert.That(controlPoints.DifficultyPointAt(1000).SliderVelocity, Is.EqualTo(1)); Assert.That(controlPoints.DifficultyPointAt(2000).SliderVelocity, Is.EqualTo(1)); Assert.That(controlPoints.DifficultyPointAt(3000).SliderVelocity, Is.EqualTo(1)); #pragma warning disable 618 - Assert.That(((LegacyBeatmapDecoder.LegacyDifficultyControlPoint)controlPoints.DifficultyPointAt(1000)).GenerateTicks, Is.True); Assert.That(((LegacyBeatmapDecoder.LegacyDifficultyControlPoint)controlPoints.DifficultyPointAt(2000)).GenerateTicks, Is.False); Assert.That(((LegacyBeatmapDecoder.LegacyDifficultyControlPoint)controlPoints.DifficultyPointAt(3000)).GenerateTicks, Is.True); #pragma warning restore 618 diff --git a/osu.Game/Beatmaps/Formats/LegacyDecoder.cs b/osu.Game/Beatmaps/Formats/LegacyDecoder.cs index c3fd16e86f96..3d65ab8e0f3c 100644 --- a/osu.Game/Beatmaps/Formats/LegacyDecoder.cs +++ b/osu.Game/Beatmaps/Formats/LegacyDecoder.cs @@ -188,9 +188,8 @@ public LegacyDifficultyControlPoint() } public override bool IsRedundant(ControlPoint? existing) - => existing is LegacyDifficultyControlPoint existingLegacyDifficulty - && base.IsRedundant(existing) - && GenerateTicks == existingLegacyDifficulty.GenerateTicks; + => base.IsRedundant(existing) + && GenerateTicks == ((existing as LegacyDifficultyControlPoint)?.GenerateTicks ?? true); public override void CopyFrom(ControlPoint other) {