From 72fb34f82cf1ae65ad7a96ed98a40a12aae7b26b Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Tue, 21 Apr 2020 14:19:05 +0900 Subject: [PATCH] Fix overriding control points incorrectly --- .../Formats/LegacyBeatmapDecoderTest.cs | 5 ++++ .../Beatmaps/Formats/LegacyBeatmapDecoder.cs | 30 +++++++++---------- 2 files changed, 20 insertions(+), 15 deletions(-) diff --git a/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapDecoderTest.cs b/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapDecoderTest.cs index 33f484a9aa..acb30a6277 100644 --- a/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapDecoderTest.cs +++ b/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapDecoderTest.cs @@ -241,6 +241,11 @@ public void TestDecodeOverlappingTimingPoints() { var controlPoints = decoder.Decode(stream).ControlPointInfo; + Assert.That(controlPoints.TimingPoints.Count, Is.EqualTo(4)); + Assert.That(controlPoints.DifficultyPoints.Count, Is.EqualTo(3)); + Assert.That(controlPoints.EffectPoints.Count, Is.EqualTo(3)); + Assert.That(controlPoints.SamplePoints.Count, Is.EqualTo(3)); + Assert.That(controlPoints.DifficultyPointAt(500).SpeedMultiplier, Is.EqualTo(1.5).Within(0.1)); Assert.That(controlPoints.DifficultyPointAt(1500).SpeedMultiplier, Is.EqualTo(1.5).Within(0.1)); Assert.That(controlPoints.DifficultyPointAt(2500).SpeedMultiplier, Is.EqualTo(0.75).Within(0.1)); diff --git a/osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs b/osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs index 33bb9774df..388abf4648 100644 --- a/osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs +++ b/osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs @@ -386,17 +386,10 @@ private void handleTimingPoint(string line) SampleVolume = sampleVolume, CustomSampleBank = customSampleBank, }, timingChange); - - // To handle the scenario where a non-timing line shares the same time value as a subsequent timing line but - // appears earlier in the file, we buffer non-timing control points and rewrite them *after* control points from the timing line - // with the same time value (allowing them to overwrite as necessary). - // - // The expected outcome is that we prefer the non-timing line's adjustments over the timing line's adjustments when time is equal. - if (timingChange) - flushPendingPoints(); } private readonly List pendingControlPoints = new List(); + private readonly HashSet pendingControlPointTypes = new HashSet(); private double pendingControlPointsTime; private void addControlPoint(double time, ControlPoint point, bool timingChange) @@ -405,21 +398,28 @@ private void addControlPoint(double time, ControlPoint point, bool timingChange) flushPendingPoints(); if (timingChange) - { - beatmap.ControlPointInfo.Add(time, point); - return; - } + pendingControlPoints.Insert(0, point); + else + pendingControlPoints.Add(point); - pendingControlPoints.Add(point); pendingControlPointsTime = time; } private void flushPendingPoints() { - foreach (var p in pendingControlPoints) - beatmap.ControlPointInfo.Add(pendingControlPointsTime, p); + // Changes from non-timing-points are added to the end of the list (see addControlPoint()) and should override any changes from timing-points (added to the start of the list). + for (int i = pendingControlPoints.Count - 1; i >= 0; i--) + { + var type = pendingControlPoints[i].GetType(); + if (pendingControlPointTypes.Contains(type)) + continue; + + pendingControlPointTypes.Add(type); + beatmap.ControlPointInfo.Add(pendingControlPointsTime, pendingControlPoints[i]); + } pendingControlPoints.Clear(); + pendingControlPointTypes.Clear(); } private void handleHitObject(string line)