Merge pull request #28630 from peppy/fix-long-note-break-woes

osu.Game.Tests/Editing/TestSceneEditorBeatmapProcessor.cs

@@ -5,6 +5,8 @@ using NUnit.Framework;
 using osu.Game.Beatmaps;
 using osu.Game.Beatmaps.ControlPoints;
 using osu.Game.Beatmaps.Timing;
+using osu.Game.Rulesets.Mania;
+using osu.Game.Rulesets.Mania.Objects;
 using osu.Game.Rulesets.Osu;
 using osu.Game.Rulesets.Osu.Objects;
 using osu.Game.Screens.Edit;
@@ -74,6 +76,50 @@ namespace osu.Game.Tests.Editing
             Assert.That(beatmap.Breaks, Is.Empty);
         }
 
+        [Test]
+        public void TestHoldNote()
+        {
+            var controlPoints = new ControlPointInfo();
+            controlPoints.Add(0, new TimingControlPoint { BeatLength = 500 });
+            var beatmap = new Beatmap
+            {
+                ControlPointInfo = controlPoints,
+                HitObjects =
+                {
+                    new HoldNote { StartTime = 1000, Duration = 10000 },
+                }
+            };
+
+            var beatmapProcessor = new EditorBeatmapProcessor(beatmap, new ManiaRuleset());
+            beatmapProcessor.PreProcess();
+            beatmapProcessor.PostProcess();
+
+            Assert.That(beatmap.Breaks, Has.Count.EqualTo(0));
+        }
+
+        [Test]
+        public void TestHoldNoteWithOverlappingNote()
+        {
+            var controlPoints = new ControlPointInfo();
+            controlPoints.Add(0, new TimingControlPoint { BeatLength = 500 });
+            var beatmap = new Beatmap
+            {
+                ControlPointInfo = controlPoints,
+                HitObjects =
+                {
+                    new HoldNote { StartTime = 1000, Duration = 10000 },
+                    new Note { StartTime = 2000 },
+                    new Note { StartTime = 12000 },
+                }
+            };
+
+            var beatmapProcessor = new EditorBeatmapProcessor(beatmap, new ManiaRuleset());
+            beatmapProcessor.PreProcess();
+            beatmapProcessor.PostProcess();
+
+            Assert.That(beatmap.Breaks, Has.Count.EqualTo(0));
+        }
+
         [Test]
         public void TestTwoObjectsFarApart()
         {
@@ -349,6 +395,32 @@ namespace osu.Game.Tests.Editing
             Assert.That(beatmap.Breaks, Is.Empty);
         }
 
+        [Test]
+        public void TestManualBreaksAtEndOfBeatmapAreRemovedCorrectlyEvenWithConcurrentObjects()
+        {
+            var controlPoints = new ControlPointInfo();
+            controlPoints.Add(0, new TimingControlPoint { BeatLength = 500 });
+            var beatmap = new Beatmap
+            {
+                ControlPointInfo = controlPoints,
+                HitObjects =
+                {
+                    new HoldNote { StartTime = 1000, EndTime = 20000 },
+                    new HoldNote { StartTime = 2000, EndTime = 3000 },
+                },
+                Breaks =
+                {
+                    new ManualBreakPeriod(10000, 15000),
+                }
+            };
+
+            var beatmapProcessor = new EditorBeatmapProcessor(beatmap, new OsuRuleset());
+            beatmapProcessor.PreProcess();
+            beatmapProcessor.PostProcess();
+
+            Assert.That(beatmap.Breaks, Is.Empty);
+        }
+
         [Test]
         public void TestBreaksAtStartOfBeatmapAreRemoved()
         {

osu.Game/Screens/Edit/EditorBeatmapProcessor.cs

@@ -41,22 +41,29 @@ namespace osu.Game.Screens.Edit
             foreach (var manualBreak in Beatmap.Breaks.ToList())
             {
                 if (manualBreak.EndTime <= Beatmap.HitObjects.FirstOrDefault()?.StartTime
-                    || manualBreak.StartTime >= Beatmap.HitObjects.LastOrDefault()?.GetEndTime()
+                    || manualBreak.StartTime >= Beatmap.GetLastObjectTime()
                     || Beatmap.HitObjects.Any(ho => ho.StartTime <= manualBreak.EndTime && ho.GetEndTime() >= manualBreak.StartTime))
                 {
                     Beatmap.Breaks.Remove(manualBreak);
                 }
             }
 
+            double currentMaxEndTime = double.MinValue;
+
             for (int i = 1; i < Beatmap.HitObjects.Count; ++i)
             {
-                double previousObjectEndTime = Beatmap.HitObjects[i - 1].GetEndTime();
+                // Keep track of the maximum end time encountered thus far.
+                // This handles cases like osu!mania's hold notes, which could have concurrent other objects after their start time.
+                // Note that we're relying on the implicit assumption that objects are sorted by start time,
+                // which is why similar tracking is not done for start time.
+                currentMaxEndTime = Math.Max(currentMaxEndTime, Beatmap.HitObjects[i - 1].GetEndTime());
+
                 double nextObjectStartTime = Beatmap.HitObjects[i].StartTime;
 
-                if (nextObjectStartTime - previousObjectEndTime < BreakPeriod.MIN_GAP_DURATION)
+                if (nextObjectStartTime - currentMaxEndTime < BreakPeriod.MIN_GAP_DURATION)
                     continue;
 
-                double breakStartTime = previousObjectEndTime + BreakPeriod.GAP_BEFORE_BREAK;
+                double breakStartTime = currentMaxEndTime + BreakPeriod.GAP_BEFORE_BREAK;
                 double breakEndTime = nextObjectStartTime - Math.Max(BreakPeriod.GAP_AFTER_BREAK, Beatmap.ControlPointInfo.TimingPointAt(nextObjectStartTime).BeatLength * 2);
 
                 if (breakEndTime - breakStartTime < BreakPeriod.MIN_BREAK_DURATION)