From c031aeb14c95431f9f7cc5e875b4a22248e0070f Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sat, 26 Oct 2019 00:06:05 +0900 Subject: [PATCH 1/5] Fix inspection --- osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs b/osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs index 61f70a8c27..b5f763fc8d 100644 --- a/osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs +++ b/osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs @@ -379,7 +379,7 @@ namespace osu.Game.Beatmaps.Formats beatmap.ControlPointInfo.Add(time, new DifficultyControlPoint { SpeedMultiplier = speedMultiplier, - AutoGenerated = timingChange + AutoGenerated = false }); } From d25f7f4c275997c168659ce9f0143eb830531083 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sat, 26 Oct 2019 01:19:23 +0900 Subject: [PATCH 2/5] Correctly clear other lists --- osu.Game/Beatmaps/ControlPoints/ControlPointInfo.cs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/osu.Game/Beatmaps/ControlPoints/ControlPointInfo.cs b/osu.Game/Beatmaps/ControlPoints/ControlPointInfo.cs index 6a760343c3..8f7777daff 100644 --- a/osu.Game/Beatmaps/ControlPoints/ControlPointInfo.cs +++ b/osu.Game/Beatmaps/ControlPoints/ControlPointInfo.cs @@ -240,6 +240,13 @@ namespace osu.Game.Beatmaps.ControlPoints } } - public void Clear() => groups.Clear(); + public void Clear() + { + groups.Clear(); + timingPoints.Clear(); + difficultyPoints.Clear(); + samplePoints.Clear(); + effectPoints.Clear(); + } } } From 7100319858fd510e07eff7538c45d321ffd8ec99 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sat, 26 Oct 2019 08:31:41 +0900 Subject: [PATCH 3/5] Fix incorrect control point retrieval in non-lookup cases --- .../ControlPoints/ControlPointInfo.cs | 37 +++++++++++++------ 1 file changed, 25 insertions(+), 12 deletions(-) diff --git a/osu.Game/Beatmaps/ControlPoints/ControlPointInfo.cs b/osu.Game/Beatmaps/ControlPoints/ControlPointInfo.cs index 8f7777daff..0122ee5cdc 100644 --- a/osu.Game/Beatmaps/ControlPoints/ControlPointInfo.cs +++ b/osu.Game/Beatmaps/ControlPoints/ControlPointInfo.cs @@ -62,28 +62,28 @@ namespace osu.Game.Beatmaps.ControlPoints /// /// The time to find the difficulty control point at. /// The difficulty control point. - public DifficultyControlPoint DifficultyPointAt(double time) => binarySearch(DifficultyPoints, time); + public DifficultyControlPoint DifficultyPointAt(double time) => binarySearchWithFallback(DifficultyPoints, time); /// /// Finds the effect control point that is active at . /// /// The time to find the effect control point at. /// The effect control point. - public EffectControlPoint EffectPointAt(double time) => binarySearch(EffectPoints, time); + public EffectControlPoint EffectPointAt(double time) => binarySearchWithFallback(EffectPoints, time); /// /// Finds the sound control point that is active at . /// /// The time to find the sound control point at. /// The sound control point. - public SampleControlPoint SamplePointAt(double time) => binarySearch(SamplePoints, time, SamplePoints.Count > 0 ? SamplePoints[0] : null); + public SampleControlPoint SamplePointAt(double time) => binarySearchWithFallback(SamplePoints, time, SamplePoints.Count > 0 ? SamplePoints[0] : null); /// /// Finds the timing control point that is active at . /// /// The time to find the timing control point at. /// The timing control point. - public TimingControlPoint TimingPointAt(double time) => binarySearch(TimingPoints, time, TimingPoints.Count > 0 ? TimingPoints[0] : null); + public TimingControlPoint TimingPointAt(double time) => binarySearchWithFallback(TimingPoints, time, TimingPoints.Count > 0 ? TimingPoints[0] : null); /// /// Finds the closest of the same type as that is active at . @@ -95,13 +95,13 @@ namespace osu.Game.Beatmaps.ControlPoints { switch (referencePoint) { - case TimingControlPoint _: return TimingPointAt(time); + case TimingControlPoint _: return binarySearch(TimingPoints, time); - case EffectControlPoint _: return EffectPointAt(time); + case EffectControlPoint _: return binarySearch(EffectPoints, time); - case SampleControlPoint _: return SamplePointAt(time); + case SampleControlPoint _: return binarySearch(SamplePoints, time); - case DifficultyControlPoint _: return DifficultyPointAt(time); + case DifficultyControlPoint _: return binarySearch(DifficultyPoints, time); } return null; @@ -130,22 +130,35 @@ namespace osu.Game.Beatmaps.ControlPoints /// /// Binary searches one of the control point lists to find the active control point at . + /// Includes logic for returning a specific point when no matching point is found. /// /// The list to search. /// The time to find the control point at. /// The control point to use when is before any control points. If null, a new control point will be constructed. - /// The active control point at . - private T binarySearch(IReadOnlyList list, double time, T prePoint = null) + /// The active control point at , or a fallback if none found. + private T binarySearchWithFallback(IReadOnlyList list, double time, T prePoint = null) where T : ControlPoint, new() + { + return binarySearch(list, time) ?? prePoint ?? new T(); + } + + /// + /// Binary searches one of the control point lists to find the active control point at . + /// + /// The list to search. + /// The time to find the control point at. + /// The active control point at . + private T binarySearch(IReadOnlyList list, double time) + where T : ControlPoint { if (list == null) throw new ArgumentNullException(nameof(list)); if (list.Count == 0) - return new T(); + return null; if (time < list[0].Time) - return prePoint ?? new T(); + return null; if (time >= list[list.Count - 1].Time) return list[list.Count - 1]; From d6a49b9e93eecd41c6c6e4fc91b599b93050b6bf Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sat, 26 Oct 2019 10:25:13 +0900 Subject: [PATCH 4/5] Add back autogeneration rules Will be removed in https://github.com/ppy/osu/pull/6604 --- .../Beatmaps/ControlPoints/ControlPointGroup.cs | 13 +++++++++++-- osu.Game/Beatmaps/ControlPoints/ControlPointInfo.cs | 3 --- osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs | 12 +++++------- 3 files changed, 16 insertions(+), 12 deletions(-) diff --git a/osu.Game/Beatmaps/ControlPoints/ControlPointGroup.cs b/osu.Game/Beatmaps/ControlPoints/ControlPointGroup.cs index c4b990675e..8b71fc1897 100644 --- a/osu.Game/Beatmaps/ControlPoints/ControlPointGroup.cs +++ b/osu.Game/Beatmaps/ControlPoints/ControlPointGroup.cs @@ -30,10 +30,19 @@ namespace osu.Game.Beatmaps.ControlPoints public void Add(ControlPoint point) { - point.AttachGroup(this); + var existing = controlPoints.FirstOrDefault(p => p.GetType() == point.GetType()); + + if (existing != null) + { + // autogenerated points should not replace non-autogenerated. + // this allows for incorrectly ordered timing points to still be correctly handled. + if (point.AutoGenerated && !existing.AutoGenerated) + return; - foreach (var existing in controlPoints.Where(p => p.GetType() == point.GetType()).ToArray()) Remove(existing); + } + + point.AttachGroup(this); controlPoints.Add(point); ItemAdded?.Invoke(point); diff --git a/osu.Game/Beatmaps/ControlPoints/ControlPointInfo.cs b/osu.Game/Beatmaps/ControlPoints/ControlPointInfo.cs index 0122ee5cdc..d6db4c8d10 100644 --- a/osu.Game/Beatmaps/ControlPoints/ControlPointInfo.cs +++ b/osu.Game/Beatmaps/ControlPoints/ControlPointInfo.cs @@ -184,9 +184,6 @@ namespace osu.Game.Beatmaps.ControlPoints public void Add(double time, ControlPoint newPoint, bool force = false) { - if (!force && SimilarPointAt(time, newPoint)?.EquivalentTo(newPoint) == true) - return; - GroupAt(time, true).Add(newPoint); } diff --git a/osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs b/osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs index b5f763fc8d..24422199e5 100644 --- a/osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs +++ b/osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs @@ -374,14 +374,12 @@ namespace osu.Game.Beatmaps.Formats beatmap.ControlPointInfo.Add(time, controlPoint); } - else + + beatmap.ControlPointInfo.Add(time, new DifficultyControlPoint { - beatmap.ControlPointInfo.Add(time, new DifficultyControlPoint - { - SpeedMultiplier = speedMultiplier, - AutoGenerated = false - }); - } + SpeedMultiplier = speedMultiplier, + AutoGenerated = timingChange + }); beatmap.ControlPointInfo.Add(time, new EffectControlPoint { From 8ccff0e9cf6e42f21a23b324bc12cc6bea4105fd Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sat, 26 Oct 2019 11:20:07 +0900 Subject: [PATCH 5/5] temp --- .../Beatmaps/Formats/LegacyBeatmapDecoderTest.cs | 6 +++--- osu.Game/Beatmaps/ControlPoints/ControlPointInfo.cs | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapDecoderTest.cs b/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapDecoderTest.cs index c50250159e..7317771bac 100644 --- a/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapDecoderTest.cs +++ b/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapDecoderTest.cs @@ -167,9 +167,9 @@ namespace osu.Game.Tests.Beatmaps.Formats var controlPoints = beatmap.ControlPointInfo; Assert.AreEqual(4, controlPoints.TimingPoints.Count); - Assert.AreEqual(5, controlPoints.DifficultyPoints.Count); + Assert.AreEqual(6, controlPoints.DifficultyPoints.Count); Assert.AreEqual(34, controlPoints.SamplePoints.Count); - Assert.AreEqual(8, controlPoints.EffectPoints.Count); + Assert.AreEqual(9, controlPoints.EffectPoints.Count); var timingPoint = controlPoints.TimingPointAt(0); Assert.AreEqual(956, timingPoint.Time); @@ -191,7 +191,7 @@ namespace osu.Game.Tests.Beatmaps.Formats Assert.AreEqual(1.0, difficultyPoint.SpeedMultiplier); difficultyPoint = controlPoints.DifficultyPointAt(48428); - Assert.AreEqual(0, difficultyPoint.Time); + Assert.AreEqual(956, difficultyPoint.Time); Assert.AreEqual(1.0, difficultyPoint.SpeedMultiplier); difficultyPoint = controlPoints.DifficultyPointAt(116999); diff --git a/osu.Game/Beatmaps/ControlPoints/ControlPointInfo.cs b/osu.Game/Beatmaps/ControlPoints/ControlPointInfo.cs index 24e3c3d8ca..f8c6d3aa3b 100644 --- a/osu.Game/Beatmaps/ControlPoints/ControlPointInfo.cs +++ b/osu.Game/Beatmaps/ControlPoints/ControlPointInfo.cs @@ -95,13 +95,13 @@ namespace osu.Game.Beatmaps.ControlPoints { switch (referencePoint) { - case TimingControlPoint _: return TimingPointAt(time); + case TimingControlPoint _: return binarySearch(TimingPoints, time); - case EffectControlPoint _: return EffectPointAt(time); + case EffectControlPoint _: return binarySearch(EffectPoints, time); - case SampleControlPoint _: return SamplePointAt(time); + case SampleControlPoint _: return binarySearch(SamplePoints, time); - case DifficultyControlPoint _: return DifficultyPointAt(time); + case DifficultyControlPoint _: return binarySearch(DifficultyPoints, time); } return null;