Standardise control point search logic in OverlappingScrollAlgorithm

Was using a very local algorithm which I cannot guarantee is correct.
I'd rather it just use the one used everywhere else.

osu.Game.Tests/Visual/Gameplay/TestSceneScrollingHitObjects.cs

@@ -214,7 +214,7 @@ namespace osu.Game.Tests.Visual.Gameplay
 
         private void addControlPoints(IList<MultiplierControlPoint> controlPoints, double sequenceStartTime)
         {
-            controlPoints.ForEach(point => point.StartTime += sequenceStartTime);
+            controlPoints.ForEach(point => point.Time += sequenceStartTime);
 
             scrollContainers.ForEach(container =>
             {
@@ -224,7 +224,7 @@ namespace osu.Game.Tests.Visual.Gameplay
             foreach (var playfield in playfields)
             {
                 foreach (var controlPoint in controlPoints)
-                    playfield.Add(createDrawablePoint(playfield, controlPoint.StartTime));
+                    playfield.Add(createDrawablePoint(playfield, controlPoint.Time));
             }
         }
 

osu.Game/Beatmaps/ControlPoints/ControlPoint.cs

@@ -9,11 +9,8 @@ using osuTK.Graphics;
 
 namespace osu.Game.Beatmaps.ControlPoints
 {
-    public abstract class ControlPoint : IComparable<ControlPoint>, IDeepCloneable<ControlPoint>, IEquatable<ControlPoint>
+    public abstract class ControlPoint : IComparable<ControlPoint>, IDeepCloneable<ControlPoint>, IEquatable<ControlPoint>, IControlPoint
     {
-        /// <summary>
-        /// The time at which the control point takes effect.
-        /// </summary>
         [JsonIgnore]
         public double Time { get; set; }
 

osu.Game/Beatmaps/ControlPoints/ControlPointInfo.cs

@@ -196,8 +196,8 @@ namespace osu.Game.Beatmaps.ControlPoints
         /// <param name="time">The time to find the control point at.</param>
         /// <param name="fallback">The control point to use when <paramref name="time"/> is before any control points.</param>
         /// <returns>The active control point at <paramref name="time"/>, or a fallback <see cref="ControlPoint"/> if none found.</returns>
-        protected T BinarySearchWithFallback<T>(IReadOnlyList<T> list, double time, T fallback)
+        internal static T BinarySearchWithFallback<T>(IReadOnlyList<T> list, double time, T fallback)
-            where T : ControlPoint
+            where T : class, IControlPoint
         {
             return BinarySearch(list, time) ?? fallback;
         }
@@ -208,8 +208,8 @@ namespace osu.Game.Beatmaps.ControlPoints
         /// <param name="list">The list to search.</param>
         /// <param name="time">The time to find the control point at.</param>
         /// <returns>The active control point at <paramref name="time"/>.</returns>
-        protected virtual T BinarySearch<T>(IReadOnlyList<T> list, double time)
+        internal static T BinarySearch<T>(IReadOnlyList<T> list, double time)
-            where T : ControlPoint
+            where T : class, IControlPoint
         {
             if (list == null)
                 throw new ArgumentNullException(nameof(list));

osu.Game/Beatmaps/ControlPoints/IControlPoint.cs

@@ -0,0 +1,13 @@
+// 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.
+
+namespace osu.Game.Beatmaps.ControlPoints
+{
+    public interface IControlPoint
+    {
+        /// <summary>
+        /// The time at which the control point takes effect.
+        /// </summary>
+        public double Time { get; }
+    }
+}

osu.Game/Rulesets/Timing/MultiplierControlPoint.cs

@@ -11,12 +11,12 @@ namespace osu.Game.Rulesets.Timing
     /// <summary>
     /// A control point which adds an aggregated multiplier based on the provided <see cref="TimingPoint"/>'s BeatLength and <see cref="EffectPoint"/>'s SpeedMultiplier.
     /// </summary>
-    public class MultiplierControlPoint : IComparable<MultiplierControlPoint>
+    public class MultiplierControlPoint : IComparable<MultiplierControlPoint>, IControlPoint
     {
         /// <summary>
         /// The time in milliseconds at which this <see cref="MultiplierControlPoint"/> starts.
         /// </summary>
-        public double StartTime;
+        public double Time { get; set; }
 
         /// <summary>
         /// The aggregate multiplier which this <see cref="MultiplierControlPoint"/> provides.
@@ -54,13 +54,13 @@ namespace osu.Game.Rulesets.Timing
         /// <summary>
         /// Creates a <see cref="MultiplierControlPoint"/>.
         /// </summary>
-        /// <param name="startTime">The start time of this <see cref="MultiplierControlPoint"/>.</param>
+        /// <param name="time">The start time of this <see cref="MultiplierControlPoint"/>.</param>
-        public MultiplierControlPoint(double startTime)
+        public MultiplierControlPoint(double time)
         {
-            StartTime = startTime;
+            Time = time;
         }
 
         // ReSharper disable once ImpureMethodCallOnReadonlyValueField
-        public int CompareTo(MultiplierControlPoint other) => StartTime.CompareTo(other?.StartTime);
+        public int CompareTo(MultiplierControlPoint other) => Time.CompareTo(other?.Time);
     }
 }

osu.Game/Rulesets/UI/Scrolling/Algorithms/OverlappingScrollAlgorithm.cs

@@ -5,21 +5,18 @@
 
 using System;
 using osu.Framework.Lists;
+using osu.Game.Beatmaps.ControlPoints;
 using osu.Game.Rulesets.Timing;
 
 namespace osu.Game.Rulesets.UI.Scrolling.Algorithms
 {
     public class OverlappingScrollAlgorithm : IScrollAlgorithm
     {
-        private readonly MultiplierControlPoint searchPoint;
-
         private readonly SortedList<MultiplierControlPoint> controlPoints;
 
         public OverlappingScrollAlgorithm(SortedList<MultiplierControlPoint> controlPoints)
         {
             this.controlPoints = controlPoints;
-
-            searchPoint = new MultiplierControlPoint();
         }
 
         public double GetDisplayStartTime(double originTime, float offset, double timeRange, float scrollLength)
@@ -52,7 +49,7 @@ namespace osu.Game.Rulesets.UI.Scrolling.Algorithms
             for (; i < controlPoints.Count; i++)
             {
                 float lastPos = pos;
-                pos = PositionAt(controlPoints[i].StartTime, currentTime, timeRange, scrollLength);
+                pos = PositionAt(controlPoints[i].Time, currentTime, timeRange, scrollLength);
 
                 if (pos > position)
                 {
@@ -64,7 +61,7 @@ namespace osu.Game.Rulesets.UI.Scrolling.Algorithms
 
             i = Math.Clamp(i, 0, controlPoints.Count - 1);
 
-            return controlPoints[i].StartTime + (position - pos) * timeRange / controlPoints[i].Multiplier / scrollLength;
+            return controlPoints[i].Time + (position - pos) * timeRange / controlPoints[i].Multiplier / scrollLength;
         }
 
         public void Reset()
@@ -76,21 +73,6 @@ namespace osu.Game.Rulesets.UI.Scrolling.Algorithms
         /// </summary>
         /// <param name="time">The time which the <see cref="MultiplierControlPoint"/> should affect.</param>
         /// <returns>The <see cref="MultiplierControlPoint"/>.</returns>
-        private MultiplierControlPoint controlPointAt(double time)
+        private MultiplierControlPoint controlPointAt(double time) => ControlPointInfo.BinarySearch(controlPoints, time) ?? new MultiplierControlPoint(double.NegativeInfinity);
-        {
-            if (controlPoints.Count == 0)
-                return new MultiplierControlPoint(double.NegativeInfinity);
-
-            if (time < controlPoints[0].StartTime)
-                return controlPoints[0];
-
-            searchPoint.StartTime = time;
-            int index = controlPoints.BinarySearch(searchPoint);
-
-            if (index < 0)
-                index = ~index - 1;
-
-            return controlPoints[index];
-        }
     }
 }

osu.Game/Rulesets/UI/Scrolling/Algorithms/SequentialScrollAlgorithm.cs

@@ -121,7 +121,7 @@ namespace osu.Game.Rulesets.UI.Scrolling.Algorithms
             if (controlPoints.Count == 0)
                 return;
 
-            positionMappings.Add(new PositionMapping(controlPoints[0].StartTime, controlPoints[0]));
+            positionMappings.Add(new PositionMapping(controlPoints[0].Time, controlPoints[0]));
 
             for (int i = 0; i < controlPoints.Count - 1; i++)
             {
@@ -129,9 +129,9 @@ namespace osu.Game.Rulesets.UI.Scrolling.Algorithms
                 var next = controlPoints[i + 1];
 
                 // Figure out how much of the time range the duration represents, and adjust it by the speed multiplier
-                float length = (float)((next.StartTime - current.StartTime) / timeRange * current.Multiplier);
+                float length = (float)((next.Time - current.Time) / timeRange * current.Multiplier);
 
-                positionMappings.Add(new PositionMapping(next.StartTime, next, positionMappings[^1].Position + length));
+                positionMappings.Add(new PositionMapping(next.Time, next, positionMappings[^1].Position + length));
             }
         }
 

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

@@ -158,9 +158,9 @@ namespace osu.Game.Rulesets.UI.Scrolling
             // Trim unwanted sequences of timing changes
             timingChanges = timingChanges
                             // Collapse sections after the last hit object
-                            .Where(s => s.StartTime <= lastObjectTime)
+                            .Where(s => s.Time <= lastObjectTime)
                             // Collapse sections with the same start time
-                            .GroupBy(s => s.StartTime).Select(g => g.Last()).OrderBy(s => s.StartTime);
+                            .GroupBy(s => s.Time).Select(g => g.Last()).OrderBy(s => s.Time);
 
             ControlPoints.AddRange(timingChanges);