Merge branch 'master' into show-beatmap-id

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

@@ -110,7 +110,7 @@ namespace osu.Game.Rulesets.UI.Scrolling
             base.UpdateAfterChildrenLife();
 
             // We need to calculate this as soon as possible after lifetimes so that hitobjects get the final say in their positions
-            speedChangeVisualiser.ComputePositions(AliveObjects, direction, Time.Current, TimeRange, DrawSize);
+            speedChangeVisualiser.UpdatePositions(AliveObjects, direction, Time.Current, TimeRange, DrawSize);
         }
     }
 }

osu.Game/Rulesets/UI/Scrolling/Visualisers/ISpeedChangeVisualiser.cs

@@ -10,24 +10,23 @@ namespace osu.Game.Rulesets.UI.Scrolling.Visualisers
     public interface ISpeedChangeVisualiser
     {
         /// <summary>
-        /// Computes the states of <see cref="DrawableHitObject"/>s that are constant, such as lifetime and spatial length.
+        /// Computes the states of <see cref="DrawableHitObject"/>s that remain constant while scrolling, such as lifetime and spatial length.
         /// This is invoked once whenever <paramref name="timeRange"/> or <paramref name="length"/> changes.
         /// </summary>
         /// <param name="hitObjects">The <see cref="DrawableHitObject"/>s whose states should be computed.</param>
         /// <param name="direction">The scrolling direction.</param>
-        /// <param name="timeRange">The duration required to scroll through one length of the screen before any control point adjustments.</param>
+        /// <param name="timeRange">The duration required to scroll through one length of the screen before any speed adjustments.</param>
         /// <param name="length">The length of the screen that is scrolled through.</param>
         void ComputeInitialStates(IEnumerable<DrawableHitObject> hitObjects, ScrollingDirection direction, double timeRange, Vector2 length);
 
         /// <summary>
-        /// Computes the states of <see cref="DrawableHitObject"/>s that change depending on <paramref name="currentTime"/>, such as position.
+        /// Updates the positions of <see cref="DrawableHitObject"/>s, depending on the current time. This is invoked once per frame.
-        /// This is invoked once per frame.
         /// </summary>
-        /// <param name="hitObjects">The <see cref="DrawableHitObject"/>s whose states should be computed.</param>
+        /// <param name="hitObjects">The <see cref="DrawableHitObject"/>s whose positions should be computed.</param>
         /// <param name="direction">The scrolling direction.</param>
         /// <param name="currentTime">The current time.</param>
-        /// <param name="timeRange">The duration required to scroll through one length of the screen before any control point adjustments.</param>
+        /// <param name="timeRange">The duration required to scroll through one length of the screen before any speed adjustments.</param>
         /// <param name="length">The length of the screen that is scrolled through.</param>
-        void ComputePositions(IEnumerable<DrawableHitObject> hitObjects, ScrollingDirection direction, double currentTime, double timeRange, Vector2 length);
+        void UpdatePositions(IEnumerable<DrawableHitObject> hitObjects, ScrollingDirection direction, double currentTime, double timeRange, Vector2 length);
     }
 }

osu.Game/Rulesets/UI/Scrolling/Visualisers/OverlappingSpeedChangeVisualiser.cs

@@ -4,6 +4,7 @@
 using System.Collections.Generic;
 using osu.Framework.Lists;
 using osu.Game.Rulesets.Objects.Drawables;
+using osu.Game.Rulesets.Objects.Types;
 using osu.Game.Rulesets.Timing;
 using OpenTK;
 
@@ -22,24 +23,45 @@ namespace osu.Game.Rulesets.UI.Scrolling.Visualisers
         {
             foreach (var obj in hitObjects)
             {
-                var controlPoint = controlPointAt(obj.HitObject.StartTime);
+                // The total amount of time that the hitobject will remain visible within the timeRange, which decreases as the speed multiplier increases
-                obj.LifetimeStart = obj.HitObject.StartTime - timeRange / controlPoint.Multiplier;
+                double visibleDuration = timeRange / controlPointAt(obj.HitObject.StartTime).Multiplier;
+
+                obj.LifetimeStart = obj.HitObject.StartTime - visibleDuration;
+
+                if (obj.HitObject is IHasEndTime endTime)
+                {
+                    // At the hitobject's end time, the hitobject will be positioned such that its end rests at the origin.
+                    // This results in a negative-position value, and the absolute of it indicates the length of the hitobject.
+                    var hitObjectLength = -hitObjectPositionAt(obj, endTime.EndTime, timeRange);
+
+                    switch (direction)
+                    {
+                        case ScrollingDirection.Up:
+                        case ScrollingDirection.Down:
+                            obj.Height = (float)(hitObjectLength * length.Y);
+                            break;
+                        case ScrollingDirection.Left:
+                        case ScrollingDirection.Right:
+                            obj.Width = (float)(hitObjectLength * length.X);
+                            break;
+                    }
+                }
 
                 if (obj.HasNestedHitObjects)
                 {
                     ComputeInitialStates(obj.NestedHitObjects, direction, timeRange, length);
-                    ComputePositions(obj.NestedHitObjects, direction, obj.HitObject.StartTime, timeRange, length);
+
+                    // Nested hitobjects don't need to scroll, but they do need accurate positions
+                    UpdatePositions(obj.NestedHitObjects, direction, obj.HitObject.StartTime, timeRange, length);
                 }
             }
         }
 
-        public void ComputePositions(IEnumerable<DrawableHitObject> hitObjects, ScrollingDirection direction, double currentTime, double timeRange, Vector2 length)
+        public void UpdatePositions(IEnumerable<DrawableHitObject> hitObjects, ScrollingDirection direction, double currentTime, double timeRange, Vector2 length)
         {
             foreach (var obj in hitObjects)
             {
-                var controlPoint = controlPointAt(obj.HitObject.StartTime);
+                var position = hitObjectPositionAt(obj, currentTime, timeRange);
-
-                var position = (obj.HitObject.StartTime - currentTime) * controlPoint.Multiplier / timeRange;
 
                 switch (direction)
                 {
@@ -59,7 +81,28 @@ namespace osu.Game.Rulesets.UI.Scrolling.Visualisers
             }
         }
 
+        /// <summary>
+        /// Computes the position of a <see cref="DrawableHitObject"/> at a point in time.
+        /// <para>
+        /// At t &lt; startTime, position &gt; 0. <br />
+        /// At t = startTime, position = 0. <br />
+        /// At t &gt; startTime, position &lt; 0.
+        /// </para>
+        /// </summary>
+        /// <param name="obj">The <see cref="DrawableHitObject"/>.</param>
+        /// <param name="time">The time to find the position of <paramref name="obj"/> at.</param>
+        /// <param name="timeRange">The amount of time visualised by the scrolling area.</param>
+        /// <returns>The position of <paramref name="obj"/> in the scrolling area at time = <paramref name="time"/>.</returns>
+        private double hitObjectPositionAt(DrawableHitObject obj, double time, double timeRange)
+            => (obj.HitObject.StartTime - time) / timeRange * controlPointAt(obj.HitObject.StartTime).Multiplier;
+
         private readonly MultiplierControlPoint searchPoint = new MultiplierControlPoint();
+
+        /// <summary>
+        /// Finds the <see cref="MultiplierControlPoint"/> which affects the speed of hitobjects at a specific time.
+        /// </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)
         {
             if (controlPoints.Count == 0)

osu.Game/Rulesets/UI/Scrolling/Visualisers/SequentialSpeedChangeVisualiser.cs

@@ -25,23 +25,25 @@ namespace osu.Game.Rulesets.UI.Scrolling.Visualisers
         {
             foreach (var obj in hitObjects)
             {
+                // To reduce iterations when updating hitobject positions later on, their initial positions are cached
                 var startPosition = hitObjectPositions[obj] = positionAt(obj.HitObject.StartTime, timeRange);
 
+                // Todo: This is approximate and will be incorrect in the case of extreme speed changes
                 obj.LifetimeStart = obj.HitObject.StartTime - timeRange - 1000;
 
                 if (obj.HitObject is IHasEndTime endTime)
                 {
-                    var diff = positionAt(endTime.EndTime, timeRange) - startPosition;
+                    var hitObjectLength = positionAt(endTime.EndTime, timeRange) - startPosition;
 
                     switch (direction)
                     {
                         case ScrollingDirection.Up:
                         case ScrollingDirection.Down:
-                            obj.Height = (float)(diff * length.Y);
+                            obj.Height = (float)(hitObjectLength * length.Y);
                             break;
                         case ScrollingDirection.Left:
                         case ScrollingDirection.Right:
-                            obj.Width = (float)(diff * length.X);
+                            obj.Width = (float)(hitObjectLength * length.X);
                             break;
                     }
                 }
@@ -49,12 +51,14 @@ namespace osu.Game.Rulesets.UI.Scrolling.Visualisers
                 if (obj.HasNestedHitObjects)
                 {
                     ComputeInitialStates(obj.NestedHitObjects, direction, timeRange, length);
-                    ComputePositions(obj.NestedHitObjects, direction, obj.HitObject.StartTime, timeRange, length);
+
+                    // Nested hitobjects don't need to scroll, but they do need accurate positions
+                    UpdatePositions(obj.NestedHitObjects, direction, obj.HitObject.StartTime, timeRange, length);
                 }
             }
         }
 
-        public void ComputePositions(IEnumerable<DrawableHitObject> hitObjects, ScrollingDirection direction, double currentTime, double timeRange, Vector2 length)
+        public void UpdatePositions(IEnumerable<DrawableHitObject> hitObjects, ScrollingDirection direction, double currentTime, double timeRange, Vector2 length)
         {
             var timelinePosition = positionAt(currentTime, timeRange);
 
@@ -80,21 +84,38 @@ namespace osu.Game.Rulesets.UI.Scrolling.Visualisers
             }
         }
 
+        /// <summary>
+        /// Finds the position which corresponds to a point in time.
+        /// This is a non-linear operation that depends on all the control points up to and including the one active at the time value.
+        /// </summary>
+        /// <param name="time">The time to find the position at.</param>
+        /// <param name="timeRange">The amount of time visualised by the scrolling area.</param>
+        /// <returns>A positive value indicating the position at <paramref name="time"/>.</returns>
         private double positionAt(double time, double timeRange)
         {
             double length = 0;
+
+            // We need to consider all timing points until the specified time and not just the currently-active one,
+            // since each timing point individually affects the positions of _all_ hitobjects after its start time
             for (int i = 0; i < controlPoints.Count; i++)
             {
                 var current = controlPoints[i];
                 var next = i < controlPoints.Count - 1 ? controlPoints[i + 1] : null;
 
+                // We don't need to consider any control points beyond the current time, since it will not yet
+                // affect any hitobjects
                 if (i > 0 && current.StartTime > time)
                     continue;
 
                 // Duration of the current control point
                 var currentDuration = (next?.StartTime ?? double.PositiveInfinity) - current.StartTime;
 
-                length += Math.Min(currentDuration, time - current.StartTime) * current.Multiplier / timeRange;
+                // We want to consider the minimal amount of time that this control point has affected,
+                // which may be either its duration, or the amount of time that has passed within it
+                var durationInCurrent = Math.Min(currentDuration, time - current.StartTime);
+
+                // Figure out how much of the time range the duration represents, and adjust it by the speed multiplier
+                length += durationInCurrent / timeRange * current.Multiplier;
             }
 
             return length;