Merge pull request #25420 from smoogipoo/hp-drain-fix-breaks

Fix osu! and base HP processor break time simulation

osu.Game.Rulesets.Osu.Tests/TestSceneOsuHealthProcessor.cs

@@ -0,0 +1,93 @@
+// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
+// See the LICENCE file in the repository root for full licence text.
+
+using NUnit.Framework;
+using osu.Game.Beatmaps;
+using osu.Game.Beatmaps.Timing;
+using osu.Game.Rulesets.Osu.Objects;
+using osu.Game.Rulesets.Osu.Scoring;
+
+namespace osu.Game.Rulesets.Osu.Tests
+{
+    [TestFixture]
+    public class TestSceneOsuHealthProcessor
+    {
+        [Test]
+        public void TestNoBreak()
+        {
+            OsuHealthProcessor hp = new OsuHealthProcessor(-1000);
+            hp.ApplyBeatmap(new Beatmap<OsuHitObject>
+            {
+                HitObjects =
+                {
+                    new HitCircle { StartTime = 0 },
+                    new HitCircle { StartTime = 2000 }
+                }
+            });
+
+            Assert.That(hp.DrainRate, Is.EqualTo(1.4E-5).Within(0.1E-5));
+        }
+
+        [Test]
+        public void TestSingleBreak()
+        {
+            OsuHealthProcessor hp = new OsuHealthProcessor(-1000);
+            hp.ApplyBeatmap(new Beatmap<OsuHitObject>
+            {
+                HitObjects =
+                {
+                    new HitCircle { StartTime = 0 },
+                    new HitCircle { StartTime = 2000 }
+                },
+                Breaks =
+                {
+                    new BreakPeriod(500, 1500)
+                }
+            });
+
+            Assert.That(hp.DrainRate, Is.EqualTo(4.3E-5).Within(0.1E-5));
+        }
+
+        [Test]
+        public void TestOverlappingBreak()
+        {
+            OsuHealthProcessor hp = new OsuHealthProcessor(-1000);
+            hp.ApplyBeatmap(new Beatmap<OsuHitObject>
+            {
+                HitObjects =
+                {
+                    new HitCircle { StartTime = 0 },
+                    new HitCircle { StartTime = 2000 }
+                },
+                Breaks =
+                {
+                    new BreakPeriod(500, 1400),
+                    new BreakPeriod(750, 1500),
+                }
+            });
+
+            Assert.That(hp.DrainRate, Is.EqualTo(4.3E-5).Within(0.1E-5));
+        }
+
+        [Test]
+        public void TestSequentialBreak()
+        {
+            OsuHealthProcessor hp = new OsuHealthProcessor(-1000);
+            hp.ApplyBeatmap(new Beatmap<OsuHitObject>
+            {
+                HitObjects =
+                {
+                    new HitCircle { StartTime = 0 },
+                    new HitCircle { StartTime = 2000 }
+                },
+                Breaks =
+                {
+                    new BreakPeriod(500, 1000),
+                    new BreakPeriod(1000, 1500),
+                }
+            });
+
+            Assert.That(hp.DrainRate, Is.EqualTo(4.3E-5).Within(0.1E-5));
+        }
+    }
+}

osu.Game.Rulesets.Osu/Scoring/OsuHealthProcessor.cs

@@ -4,7 +4,6 @@
 using System;
 using System.Linq;
 using osu.Game.Beatmaps;
-using osu.Game.Beatmaps.Timing;
 using osu.Game.Rulesets.Judgements;
 using osu.Game.Rulesets.Objects;
 using osu.Game.Rulesets.Osu.Objects;
@@ -62,26 +61,16 @@ protected override double ComputeDrainRate()
                 {
                     HitObject h = Beatmap.HitObjects[i];
 
-                    // Find active break (between current and lastTime)
-                    double localLastTime = lastTime;
-                    double breakTime = 0;
-
-                    // TODO: This doesn't handle overlapping/sequential breaks correctly (/b/614).
-                    // Subtract any break time from the duration since the last object
-                    // Note that this method is a bit convoluted, but matches stable code for compatibility.
-                    if (Beatmap.Breaks.Count > 0 && currentBreak < Beatmap.Breaks.Count)
+                    while (currentBreak < Beatmap.Breaks.Count && Beatmap.Breaks[currentBreak].EndTime <= h.StartTime)
                     {
-                        BreakPeriod e = Beatmap.Breaks[currentBreak];
-
-                        if (e.StartTime >= localLastTime && e.EndTime <= h.StartTime)
-                        {
-                            // consider break start equal to object end time for version 8+ since drain stops during this time
-                            breakTime = (Beatmap.BeatmapInfo.BeatmapVersion < 8) ? (e.EndTime - e.StartTime) : e.EndTime - localLastTime;
-                            currentBreak++;
-                        }
+                        // If two hitobjects are separated by a break period, there is no drain for the full duration between the hitobjects.
+                        // This differs from legacy (version < 8) beatmaps which continue draining until the break section is entered,
+                        // but this shouldn't have a noticeable impact in practice.
+                        lastTime = h.StartTime;
+                        currentBreak++;
                     }
 
-                    reduceHp(testDrop * (h.StartTime - lastTime - breakTime));
+                    reduceHp(testDrop * (h.StartTime - lastTime));
 
                     lastTime = h.GetEndTime();
 

osu.Game.Tests/Gameplay/TestSceneDrainingHealthProcessor.cs

@@ -192,7 +192,8 @@ public void TestMultipleFailConditions(HitResult resultApplied)
             AddStep("apply perfect hit result", () => processor.ApplyResult(new JudgementResult(beatmap.HitObjects[0], new Judgement()) { Type = HitResult.Perfect }));
             AddAssert("not failed", () => !processor.HasFailed);
 
-            AddStep($"apply {resultApplied.ToString().ToLowerInvariant()} hit result", () => processor.ApplyResult(new JudgementResult(beatmap.HitObjects[0], new Judgement()) { Type = resultApplied }));
+            AddStep($"apply {resultApplied.ToString().ToLowerInvariant()} hit result",
+                () => processor.ApplyResult(new JudgementResult(beatmap.HitObjects[0], new Judgement()) { Type = resultApplied }));
             AddAssert("failed", () => processor.HasFailed);
         }
 
@@ -232,6 +233,84 @@ public void TestSingleLongObjectDoesNotDrain()
             assertHealthEqualTo(1);
         }
 
+        [Test]
+        public void TestNoBreakDrainRate()
+        {
+            DrainingHealthProcessor hp = new DrainingHealthProcessor(-1000);
+            hp.ApplyBeatmap(new Beatmap<JudgeableHitObject>
+            {
+                HitObjects =
+                {
+                    new JudgeableHitObject { StartTime = 0 },
+                    new JudgeableHitObject { StartTime = 2000 }
+                }
+            });
+
+            Assert.That(hp.DrainRate, Is.EqualTo(4.5E-5).Within(0.1E-5));
+        }
+
+        [Test]
+        public void TestSingleBreakDrainRate()
+        {
+            DrainingHealthProcessor hp = new DrainingHealthProcessor(-1000);
+            hp.ApplyBeatmap(new Beatmap<JudgeableHitObject>
+            {
+                HitObjects =
+                {
+                    new JudgeableHitObject { StartTime = 0 },
+                    new JudgeableHitObject { StartTime = 2000 }
+                },
+                Breaks =
+                {
+                    new BreakPeriod(500, 1500)
+                }
+            });
+
+            Assert.That(hp.DrainRate, Is.EqualTo(9.1E-5).Within(0.1E-5));
+        }
+
+        [Test]
+        public void TestOverlappingBreakDrainRate()
+        {
+            DrainingHealthProcessor hp = new DrainingHealthProcessor(-1000);
+            hp.ApplyBeatmap(new Beatmap<JudgeableHitObject>
+            {
+                HitObjects =
+                {
+                    new JudgeableHitObject { StartTime = 0 },
+                    new JudgeableHitObject { StartTime = 2000 }
+                },
+                Breaks =
+                {
+                    new BreakPeriod(500, 1400),
+                    new BreakPeriod(750, 1500),
+                }
+            });
+
+            Assert.That(hp.DrainRate, Is.EqualTo(9.1E-5).Within(0.1E-5));
+        }
+
+        [Test]
+        public void TestSequentialBreakDrainRate()
+        {
+            DrainingHealthProcessor hp = new DrainingHealthProcessor(-1000);
+            hp.ApplyBeatmap(new Beatmap<JudgeableHitObject>
+            {
+                HitObjects =
+                {
+                    new JudgeableHitObject { StartTime = 0 },
+                    new JudgeableHitObject { StartTime = 2000 }
+                },
+                Breaks =
+                {
+                    new BreakPeriod(500, 1000),
+                    new BreakPeriod(1000, 1500),
+                }
+            });
+
+            Assert.That(hp.DrainRate, Is.EqualTo(9.1E-5).Within(0.1E-5));
+        }
+
         private Beatmap createBeatmap(double startTime, double endTime, params BreakPeriod[] breaks)
         {
             var beatmap = new Beatmap

osu.Game/Rulesets/Scoring/DrainingHealthProcessor.cs

@@ -103,18 +103,20 @@ public override void ApplyBeatmap(IBeatmap beatmap)
             if (beatmap.HitObjects.Count > 0)
                 gameplayEndTime = beatmap.HitObjects[^1].GetEndTime();
 
-            noDrainPeriodTracker = new PeriodTracker(beatmap.Breaks.Select(breakPeriod => new Period(
-                beatmap.HitObjects
-                       .Select(hitObject => hitObject.GetEndTime())
-                       .Where(endTime => endTime <= breakPeriod.StartTime)
-                       .DefaultIfEmpty(double.MinValue)
-                       .Last(),
-                beatmap.HitObjects
-                       .Select(hitObject => hitObject.StartTime)
-                       .Where(startTime => startTime >= breakPeriod.EndTime)
-                       .DefaultIfEmpty(double.MaxValue)
-                       .First()
-            )));
+            noDrainPeriodTracker = new PeriodTracker(
+                beatmap.Breaks.Select(breakPeriod =>
+                    new Period(
+                        beatmap.HitObjects
+                               .Select(hitObject => hitObject.GetEndTime())
+                               .Where(endTime => endTime <= breakPeriod.StartTime)
+                               .DefaultIfEmpty(double.MinValue)
+                               .Last(),
+                        beatmap.HitObjects
+                               .Select(hitObject => hitObject.StartTime)
+                               .Where(startTime => startTime >= breakPeriod.EndTime)
+                               .DefaultIfEmpty(double.MaxValue)
+                               .First()
+                    )));
 
             targetMinimumHealth = IBeatmapDifficultyInfo.DifficultyRange(beatmap.Difficulty.DrainRate, min_health_target, mid_health_target, max_health_target);
 
@@ -159,26 +161,24 @@ protected virtual double ComputeDrainRate()
             {
                 double currentHealth = 1;
                 double lowestHealth = 1;
-                int currentBreak = -1;
+                int currentBreak = 0;
 
                 for (int i = 0; i < healthIncreases.Count; i++)
                 {
                     double currentTime = healthIncreases[i].time;
                     double lastTime = i > 0 ? healthIncreases[i - 1].time : DrainStartTime;
 
-                    // Subtract any break time from the duration since the last object
-                    if (Beatmap.Breaks.Count > 0)
+                    while (currentBreak < Beatmap.Breaks.Count && Beatmap.Breaks[currentBreak].EndTime <= currentTime)
                     {
-                        // Advance the last break occuring before the current time
-                        while (currentBreak + 1 < Beatmap.Breaks.Count && Beatmap.Breaks[currentBreak + 1].EndTime < currentTime)
-                            currentBreak++;
-
-                        if (currentBreak >= 0)
-                            lastTime = Math.Max(lastTime, Beatmap.Breaks[currentBreak].EndTime);
+                        // If two hitobjects are separated by a break period, there is no drain for the full duration between the hitobjects.
+                        // This differs from legacy (version < 8) beatmaps which continue draining until the break section is entered,
+                        // but this shouldn't have a noticeable impact in practice.
+                        lastTime = currentTime;
+                        currentBreak++;
                     }
 
                     // Apply health adjustments
-                    currentHealth -= (healthIncreases[i].time - lastTime) * result;
+                    currentHealth -= (currentTime - lastTime) * result;
                     lowestHealth = Math.Min(lowestHealth, currentHealth);
                     currentHealth = Math.Min(1, currentHealth + healthIncreases[i].health);