Merge pull request #21479 from peppy/fix-wrong-last-hit-object-time

Fix some components not accounting for final hold notes in osu!mania beatmaps

osu.Game.Rulesets.Osu/Mods/OsuModTargetPractice.cs

@@ -17,7 +17,6 @@ using osu.Game.Configuration;
 using osu.Game.Graphics;
 using osu.Game.Overlays.Settings;
 using osu.Game.Rulesets.Mods;
-using osu.Game.Rulesets.Objects;
 using osu.Game.Rulesets.Objects.Drawables;
 using osu.Game.Rulesets.Objects.Types;
 using osu.Game.Rulesets.Osu.Beatmaps;
@@ -196,8 +195,8 @@ namespace osu.Game.Rulesets.Osu.Mods
 
         private IEnumerable<double> generateBeats(IBeatmap beatmap, IReadOnlyCollection<OsuHitObject> originalHitObjects)
         {
-            double startTime = originalHitObjects.First().StartTime;
+            double startTime = beatmap.HitObjects.First().StartTime;
-            double endTime = originalHitObjects.Last().GetEndTime();
+            double endTime = beatmap.GetLastObjectTime();
 
             var beats = beatmap.ControlPointInfo.TimingPoints
                                // Ignore timing points after endTime

osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapDecoderTest.cs

@@ -314,6 +314,24 @@ namespace osu.Game.Tests.Beatmaps.Formats
             }
         }
 
+        [Test]
+        public void TestGetLastObjectTime()
+        {
+            var decoder = new LegacyBeatmapDecoder();
+
+            using (var resStream = TestResources.OpenResource("mania-last-object-not-latest.osu"))
+            using (var stream = new LineBufferedReader(resStream))
+            {
+                var beatmap = decoder.Decode(stream);
+
+                Assert.That(beatmap.HitObjects.Last().StartTime, Is.EqualTo(2494));
+                Assert.That(beatmap.HitObjects.Last().GetEndTime(), Is.EqualTo(2494));
+
+                Assert.That(beatmap.HitObjects.Max(h => h.GetEndTime()), Is.EqualTo(2582));
+                Assert.That(beatmap.GetLastObjectTime(), Is.EqualTo(2582));
+            }
+        }
+
         [Test]
         public void TestDecodeBeatmapComboOffsetsOsu()
         {

osu.Game.Tests/Resources/mania-last-object-not-latest.osu

@@ -0,0 +1,39 @@
+osu file format v14
+
+[General]
+SampleSet: Normal
+StackLeniency: 0.7
+Mode: 3
+
+[Difficulty]
+HPDrainRate:3
+CircleSize:5
+OverallDifficulty:8
+ApproachRate:8
+SliderMultiplier:3.59999990463257
+SliderTickRate:2
+
+[TimingPoints]
+24,352.941176470588,4,1,1,100,1,0
+6376,-50,4,1,1,100,0,0
+
+[HitObjects]
+51,192,24,1,0,0:0:0:0:
+153,192,200,1,0,0:0:0:0:
+358,192,376,1,0,0:0:0:0:
+460,192,553,1,0,0:0:0:0:
+460,192,729,128,0,1435:0:0:0:0:
+358,192,906,128,0,1612:0:0:0:0:
+256,192,1082,128,0,1788:0:0:0:0:
+153,192,1259,128,0,1965:0:0:0:0:
+51,192,1435,128,0,2141:0:0:0:0:
+51,192,2318,1,12,0:0:0:0:
+153,192,2318,1,4,0:0:0:0:
+256,192,2318,1,6,0:0:0:0:
+358,192,2318,1,14,0:0:0:0:
+460,192,2318,1,0,0:0:0:0:
+51,192,2494,128,0,2582:0:0:0:0:
+153,192,2494,128,14,2582:0:0:0:0:
+256,192,2494,128,6,2582:0:0:0:0:
+358,192,2494,128,4,2582:0:0:0:0:
+460,192,2494,1,12,0:0:0:0:0:

osu.Game/Beatmaps/Beatmap.cs

@@ -81,9 +81,14 @@ namespace osu.Game.Beatmaps
 
         public double GetMostCommonBeatLength()
         {
+            double lastTime;
+
             // The last playable time in the beatmap - the last timing point extends to this time.
             // Note: This is more accurate and may present different results because osu-stable didn't have the ability to calculate slider durations in this context.
-            double lastTime = HitObjects.LastOrDefault()?.GetEndTime() ?? ControlPointInfo.TimingPoints.LastOrDefault()?.Time ?? 0;
+            if (!HitObjects.Any())
+                lastTime = ControlPointInfo.TimingPoints.LastOrDefault()?.Time ?? 0;
+            else
+                lastTime = this.GetLastObjectTime();
 
             var mostCommon =
                 // Construct a set of (beatLength, duration) tuples for each individual timing point.

osu.Game/Beatmaps/IBeatmap.cs

@@ -4,6 +4,7 @@
 #nullable disable
 
 using System.Collections.Generic;
+using System.Linq;
 using osu.Game.Beatmaps.ControlPoints;
 using osu.Game.Beatmaps.Timing;
 using osu.Game.Rulesets.Objects;
@@ -102,5 +103,16 @@ namespace osu.Game.Beatmaps
                     addCombo(nested, ref combo);
             }
         }
+
+        /// <summary>
+        /// Find the absolute end time of the latest <see cref="HitObject"/> in a beatmap. Will throw if beatmap contains no objects.
+        /// </summary>
+        /// <remarks>
+        /// This correctly accounts for rulesets which have concurrent hitobjects which may have durations, causing the .Last() object
+        /// to not necessarily have the latest end time.
+        ///
+        /// It's not super efficient so calls should be kept to a minimum.
+        /// </remarks>
+        public static double GetLastObjectTime(this IBeatmap beatmap) => beatmap.HitObjects.Max(h => h.GetEndTime());
     }
 }

osu.Game/Rulesets/Mods/ModTimeRamp.cs

@@ -7,7 +7,6 @@ using osu.Framework.Audio;
 using osu.Framework.Bindables;
 using osu.Game.Beatmaps;
 using osu.Game.Configuration;
-using osu.Game.Rulesets.Objects;
 using osu.Game.Rulesets.UI;
 
 namespace osu.Game.Rulesets.Mods
@@ -71,7 +70,7 @@ namespace osu.Game.Rulesets.Mods
             SpeedChange.SetDefault();
 
             double firstObjectStart = beatmap.HitObjects.FirstOrDefault()?.StartTime ?? 0;
-            double lastObjectEnd = beatmap.HitObjects.LastOrDefault()?.GetEndTime() ?? 0;
+            double lastObjectEnd = beatmap.HitObjects.Any() ? beatmap.GetLastObjectTime() : 0;
 
             beginRampTime = firstObjectStart;
             finalRateTime = firstObjectStart + FINAL_RATE_PROGRESS * (lastObjectEnd - firstObjectStart);

osu.Game/Rulesets/Objects/BarLineGenerator.cs

@@ -27,11 +27,8 @@ namespace osu.Game.Rulesets.Objects
             if (beatmap.HitObjects.Count == 0)
                 return;
 
-            HitObject firstObject = beatmap.HitObjects.First();
+            double firstHitTime = beatmap.HitObjects.First().StartTime;
-            HitObject lastObject = beatmap.HitObjects.Last();
+            double lastHitTime = 1 + beatmap.GetLastObjectTime();
-
-            double firstHitTime = firstObject.StartTime;
-            double lastHitTime = 1 + lastObject.GetEndTime();
 
             var timingPoints = beatmap.ControlPointInfo.TimingPoints;
 

osu.Game/Rulesets/UI/Scrolling/DrawableScrollingRuleset.cs

@@ -114,7 +114,7 @@ namespace osu.Game.Rulesets.UI.Scrolling
                     break;
             }
 
-            double lastObjectTime = Objects.LastOrDefault()?.GetEndTime() ?? double.MaxValue;
+            double lastObjectTime = Beatmap.HitObjects.Any() ? Beatmap.GetLastObjectTime() : double.MaxValue;
             double baseBeatLength = TimingControlPoint.DEFAULT_BEAT_LENGTH;
 
             if (RelativeScaleBeatLengths)

osu.Game/Screens/Edit/Editor.cs

@@ -40,7 +40,6 @@ using osu.Game.Overlays.Notifications;
 using osu.Game.Overlays.OSD;
 using osu.Game.Rulesets;
 using osu.Game.Rulesets.Edit;
-using osu.Game.Rulesets.Objects;
 using osu.Game.Screens.Edit.Components.Menus;
 using osu.Game.Screens.Edit.Compose;
 using osu.Game.Screens.Edit.Compose.Components.Timeline;
@@ -538,12 +537,14 @@ namespace osu.Game.Screens.Edit
                     // Seek to last object time, or track end if already there.
                     // Note that in osu-stable subsequent presses when at track end won't return to last object.
                     // This has intentionally been changed to make it more useful.
-                    double? lastObjectTime = editorBeatmap.HitObjects.LastOrDefault()?.GetEndTime();
+                    if (!editorBeatmap.HitObjects.Any())
-
+                    {
-                    if (lastObjectTime == null || clock.CurrentTime == lastObjectTime)
                         clock.Seek(clock.TrackLength);
-                    else
+                        return true;
-                        clock.Seek(lastObjectTime.Value);
+                    }
+
+                    double lastObjectTime = editorBeatmap.GetLastObjectTime();
+                    clock.Seek(clock.CurrentTime == lastObjectTime ? clock.TrackLength : lastObjectTime);
                     return true;
             }
 

osu.Game/Screens/Play/ReplayPlayer.cs

@@ -5,7 +5,6 @@
 
 using System;
 using System.Collections.Generic;
-using System.Linq;
 using System.Threading.Tasks;
 using osu.Framework.Bindables;
 using osu.Framework.Input.Bindings;
@@ -13,7 +12,6 @@ using osu.Framework.Input.Events;
 using osu.Game.Beatmaps;
 using osu.Game.Input.Bindings;
 using osu.Game.Rulesets.Mods;
-using osu.Game.Rulesets.Objects;
 using osu.Game.Scoring;
 using osu.Game.Screens.Play.HUD;
 using osu.Game.Screens.Ranking;
@@ -94,7 +92,7 @@ namespace osu.Game.Screens.Play
 
             void keyboardSeek(int direction)
             {
-                double target = Math.Clamp(GameplayClockContainer.CurrentTime + direction * keyboard_seek_amount, 0, GameplayState.Beatmap.HitObjects.Last().GetEndTime());
+                double target = Math.Clamp(GameplayClockContainer.CurrentTime + direction * keyboard_seek_amount, 0, GameplayState.Beatmap.GetLastObjectTime());
 
                 Seek(target);
             }