Refactor follow points to support dynamic updates (#6721)

Refactor follow points to support dynamic updates

Co-authored-by: Dean Herbert <pe@ppy.sh>

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

@@ -0,0 +1,230 @@
+// 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 System;
+using NUnit.Framework;
+using osu.Framework.Graphics;
+using osu.Framework.Graphics.Containers;
+using osu.Game.Beatmaps;
+using osu.Game.Beatmaps.ControlPoints;
+using osu.Game.Rulesets.Osu.Objects;
+using osu.Game.Rulesets.Osu.Objects.Drawables;
+using osu.Game.Rulesets.Osu.Objects.Drawables.Connections;
+using osu.Game.Tests.Visual;
+using osuTK;
+
+namespace osu.Game.Rulesets.Osu.Tests
+{
+    public class TestSceneFollowPoints : OsuTestScene
+    {
+        private Container<DrawableOsuHitObject> hitObjectContainer;
+        private FollowPointRenderer followPointRenderer;
+
+        [SetUp]
+        public void Setup() => Schedule(() =>
+        {
+            Children = new Drawable[]
+            {
+                hitObjectContainer = new TestHitObjectContainer { RelativeSizeAxes = Axes.Both },
+                followPointRenderer = new FollowPointRenderer { RelativeSizeAxes = Axes.Both }
+            };
+        });
+
+        [Test]
+        public void TestAddObject()
+        {
+            addObjectsStep(() => new OsuHitObject[] { new HitCircle { Position = new Vector2(100, 100) } });
+
+            assertGroups();
+        }
+
+        [Test]
+        public void TestRemoveObject()
+        {
+            addObjectsStep(() => new OsuHitObject[] { new HitCircle { Position = new Vector2(100, 100) } });
+
+            removeObjectStep(() => getObject(0));
+
+            assertGroups();
+        }
+
+        [Test]
+        public void TestAddMultipleObjects()
+        {
+            addMultipleObjectsStep();
+
+            assertGroups();
+        }
+
+        [Test]
+        public void TestRemoveEndObject()
+        {
+            addMultipleObjectsStep();
+
+            removeObjectStep(() => getObject(4));
+
+            assertGroups();
+        }
+
+        [Test]
+        public void TestRemoveStartObject()
+        {
+            addMultipleObjectsStep();
+
+            removeObjectStep(() => getObject(0));
+
+            assertGroups();
+        }
+
+        [Test]
+        public void TestRemoveMiddleObject()
+        {
+            addMultipleObjectsStep();
+
+            removeObjectStep(() => getObject(2));
+
+            assertGroups();
+        }
+
+        [Test]
+        public void TestMoveObject()
+        {
+            addMultipleObjectsStep();
+
+            AddStep("move hitobject", () => getObject(2).HitObject.Position = new Vector2(300, 100));
+
+            assertGroups();
+        }
+
+        [TestCase(0, 0)] // Start -> Start
+        [TestCase(0, 2)] // Start -> Middle
+        [TestCase(0, 5)] // Start -> End
+        [TestCase(2, 0)] // Middle -> Start
+        [TestCase(1, 3)] // Middle -> Middle (forwards)
+        [TestCase(3, 1)] // Middle -> Middle (backwards)
+        [TestCase(4, 0)] // End -> Start
+        [TestCase(4, 2)] // End -> Middle
+        [TestCase(4, 4)] // End -> End
+        public void TestReorderObjects(int startIndex, int endIndex)
+        {
+            addMultipleObjectsStep();
+
+            reorderObjectStep(startIndex, endIndex);
+
+            assertGroups();
+        }
+
+        private void addMultipleObjectsStep() => addObjectsStep(() => new OsuHitObject[]
+        {
+            new HitCircle { Position = new Vector2(100, 100) },
+            new HitCircle { Position = new Vector2(200, 200) },
+            new HitCircle { Position = new Vector2(300, 300) },
+            new HitCircle { Position = new Vector2(400, 400) },
+            new HitCircle { Position = new Vector2(500, 500) },
+        });
+
+        private void addObjectsStep(Func<OsuHitObject[]> ctorFunc)
+        {
+            AddStep("add hitobjects", () =>
+            {
+                var objects = ctorFunc();
+
+                for (int i = 0; i < objects.Length; i++)
+                {
+                    objects[i].StartTime = Time.Current + 1000 + 500 * (i + 1);
+                    objects[i].ApplyDefaults(new ControlPointInfo(), new BeatmapDifficulty());
+
+                    DrawableOsuHitObject drawableObject = null;
+
+                    switch (objects[i])
+                    {
+                        case HitCircle circle:
+                            drawableObject = new DrawableHitCircle(circle);
+                            break;
+
+                        case Slider slider:
+                            drawableObject = new DrawableSlider(slider);
+                            break;
+
+                        case Spinner spinner:
+                            drawableObject = new DrawableSpinner(spinner);
+                            break;
+                    }
+
+                    hitObjectContainer.Add(drawableObject);
+                    followPointRenderer.AddFollowPoints(drawableObject);
+                }
+            });
+        }
+
+        private void removeObjectStep(Func<DrawableOsuHitObject> getFunc)
+        {
+            AddStep("remove hitobject", () =>
+            {
+                var drawableObject = getFunc?.Invoke();
+
+                hitObjectContainer.Remove(drawableObject);
+                followPointRenderer.RemoveFollowPoints(drawableObject);
+            });
+        }
+
+        private void reorderObjectStep(int startIndex, int endIndex)
+        {
+            AddStep($"move object {startIndex} to {endIndex}", () =>
+            {
+                DrawableOsuHitObject toReorder = getObject(startIndex);
+
+                double targetTime;
+                if (endIndex < hitObjectContainer.Count)
+                    targetTime = getObject(endIndex).HitObject.StartTime - 1;
+                else
+                    targetTime = getObject(hitObjectContainer.Count - 1).HitObject.StartTime + 1;
+
+                hitObjectContainer.Remove(toReorder);
+                toReorder.HitObject.StartTime = targetTime;
+                hitObjectContainer.Add(toReorder);
+            });
+        }
+
+        private void assertGroups()
+        {
+            AddAssert("has correct group count", () => followPointRenderer.Connections.Count == hitObjectContainer.Count);
+            AddAssert("group endpoints are correct", () =>
+            {
+                for (int i = 0; i < hitObjectContainer.Count; i++)
+                {
+                    DrawableOsuHitObject expectedStart = getObject(i);
+                    DrawableOsuHitObject expectedEnd = i < hitObjectContainer.Count - 1 ? getObject(i + 1) : null;
+
+                    if (getGroup(i).Start != expectedStart)
+                        throw new AssertionException($"Object {i} expected to be the start of group {i}.");
+
+                    if (getGroup(i).End != expectedEnd)
+                        throw new AssertionException($"Object {(expectedEnd == null ? "null" : i.ToString())} expected to be the end of group {i}.");
+                }
+
+                return true;
+            });
+        }
+
+        private DrawableOsuHitObject getObject(int index) => hitObjectContainer[index];
+
+        private FollowPointConnection getGroup(int index) => followPointRenderer.Connections[index];
+
+        private class TestHitObjectContainer : Container<DrawableOsuHitObject>
+        {
+            protected override int Compare(Drawable x, Drawable y)
+            {
+                var osuX = (DrawableOsuHitObject)x;
+                var osuY = (DrawableOsuHitObject)y;
+
+                int compare = osuX.HitObject.StartTime.CompareTo(osuY.HitObject.StartTime);
+
+                if (compare == 0)
+                    return base.Compare(x, y);
+
+                return compare;
+            }
+        }
+    }
+}

osu.Game.Rulesets.Osu/Objects/Drawables/Connections/ConnectionRenderer.cs

@@ -1,21 +0,0 @@
-// 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 osu.Framework.Graphics.Containers;
-using osu.Game.Rulesets.Objects;
-using System.Collections.Generic;
-
-namespace osu.Game.Rulesets.Osu.Objects.Drawables.Connections
-{
-    /// <summary>
-    /// Connects hit objects visually, for example with follow points.
-    /// </summary>
-    public abstract class ConnectionRenderer<T> : LifetimeManagementContainer
-        where T : HitObject
-    {
-        /// <summary>
-        /// Hit objects to create connections for
-        /// </summary>
-        public abstract IEnumerable<T> HitObjects { get; set; }
-    }
-}

osu.Game.Rulesets.Osu/Objects/Drawables/Connections/FollowPoint.cs

@@ -12,6 +12,9 @@ using osu.Game.Skinning;
 
 namespace osu.Game.Rulesets.Osu.Objects.Drawables.Connections
 {
+    /// <summary>
+    /// A single follow point positioned between two adjacent <see cref="DrawableOsuHitObject"/>s.
+    /// </summary>
     public class FollowPoint : Container
     {
         private const float width = 8;

osu.Game.Rulesets.Osu/Objects/Drawables/Connections/FollowPointConnection.cs

@@ -0,0 +1,140 @@
+// 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 System;
+using JetBrains.Annotations;
+using osu.Framework.Bindables;
+using osu.Framework.Graphics;
+using osu.Framework.Graphics.Containers;
+using osu.Game.Rulesets.Objects.Types;
+using osuTK;
+
+namespace osu.Game.Rulesets.Osu.Objects.Drawables.Connections
+{
+    /// <summary>
+    /// Visualises the <see cref="FollowPoint"/>s between two <see cref="DrawableOsuHitObject"/>s.
+    /// </summary>
+    public class FollowPointConnection : CompositeDrawable
+    {
+        // Todo: These shouldn't be constants
+        private const int spacing = 32;
+        private const double preempt = 800;
+
+        /// <summary>
+        /// The start time of <see cref="Start"/>.
+        /// </summary>
+        public readonly Bindable<double> StartTime = new Bindable<double>();
+
+        /// <summary>
+        /// The <see cref="DrawableOsuHitObject"/> which <see cref="FollowPoint"/>s will exit from.
+        /// </summary>
+        [NotNull]
+        public readonly DrawableOsuHitObject Start;
+
+        /// <summary>
+        /// Creates a new <see cref="FollowPointConnection"/>.
+        /// </summary>
+        /// <param name="start">The <see cref="DrawableOsuHitObject"/> which <see cref="FollowPoint"/>s will exit from.</param>
+        public FollowPointConnection([NotNull] DrawableOsuHitObject start)
+        {
+            Start = start;
+
+            RelativeSizeAxes = Axes.Both;
+
+            StartTime.BindTo(Start.HitObject.StartTimeBindable);
+        }
+
+        protected override void LoadComplete()
+        {
+            base.LoadComplete();
+            bindEvents(Start);
+        }
+
+        private DrawableOsuHitObject end;
+
+        /// <summary>
+        /// The <see cref="DrawableOsuHitObject"/> which <see cref="FollowPoint"/>s will enter.
+        /// </summary>
+        [CanBeNull]
+        public DrawableOsuHitObject End
+        {
+            get => end;
+            set
+            {
+                end = value;
+
+                if (end != null)
+                    bindEvents(end);
+
+                if (IsLoaded)
+                    scheduleRefresh();
+                else
+                    refresh();
+            }
+        }
+
+        private void bindEvents(DrawableOsuHitObject drawableObject)
+        {
+            drawableObject.HitObject.PositionBindable.BindValueChanged(_ => scheduleRefresh());
+            drawableObject.HitObject.DefaultsApplied += scheduleRefresh;
+        }
+
+        private void scheduleRefresh() => Scheduler.AddOnce(refresh);
+
+        private void refresh()
+        {
+            ClearInternal();
+
+            if (End == null)
+                return;
+
+            OsuHitObject osuStart = Start.HitObject;
+            OsuHitObject osuEnd = End.HitObject;
+
+            if (osuEnd.NewCombo)
+                return;
+
+            if (osuStart is Spinner || osuEnd is Spinner)
+                return;
+
+            Vector2 startPosition = osuStart.EndPosition;
+            Vector2 endPosition = osuEnd.Position;
+            double startTime = (osuStart as IHasEndTime)?.EndTime ?? osuStart.StartTime;
+            double endTime = osuEnd.StartTime;
+
+            Vector2 distanceVector = endPosition - startPosition;
+            int distance = (int)distanceVector.Length;
+            float rotation = (float)(Math.Atan2(distanceVector.Y, distanceVector.X) * (180 / Math.PI));
+            double duration = endTime - startTime;
+
+            for (int d = (int)(spacing * 1.5); d < distance - spacing; d += spacing)
+            {
+                float fraction = (float)d / distance;
+                Vector2 pointStartPosition = startPosition + (fraction - 0.1f) * distanceVector;
+                Vector2 pointEndPosition = startPosition + fraction * distanceVector;
+                double fadeOutTime = startTime + fraction * duration;
+                double fadeInTime = fadeOutTime - preempt;
+
+                FollowPoint fp;
+
+                AddInternal(fp = new FollowPoint
+                {
+                    Position = pointStartPosition,
+                    Rotation = rotation,
+                    Alpha = 0,
+                    Scale = new Vector2(1.5f * osuEnd.Scale),
+                });
+
+                using (fp.BeginAbsoluteSequence(fadeInTime))
+                {
+                    fp.FadeIn(osuEnd.TimeFadeIn);
+                    fp.ScaleTo(osuEnd.Scale, osuEnd.TimeFadeIn, Easing.Out);
+                    fp.MoveTo(pointEndPosition, osuEnd.TimeFadeIn, Easing.Out);
+                    fp.Delay(fadeOutTime - fadeInTime).FadeOut(osuEnd.TimeFadeIn);
+                }
+
+                fp.Expire(true);
+            }
+        }
+    }
+}

osu.Game.Rulesets.Osu/Objects/Drawables/Connections/FollowPointRenderer.cs

@@ -1,121 +1,110 @@
 // 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 System;
 using System.Collections.Generic;
-using osuTK;
+using System.Linq;
+using osu.Framework.Extensions;
 using osu.Framework.Graphics;
-using osu.Game.Rulesets.Objects.Types;
+using osu.Framework.Graphics.Containers;
 
 namespace osu.Game.Rulesets.Osu.Objects.Drawables.Connections
 {
-    public class FollowPointRenderer : ConnectionRenderer<OsuHitObject>
+    /// <summary>
+    /// Visualises connections between <see cref="DrawableOsuHitObject"/>s.
+    /// </summary>
+    public class FollowPointRenderer : CompositeDrawable
     {
-        private int pointDistance = 32;
-
         /// <summary>
-        /// Determines how much space there is between points.
+        /// All the <see cref="FollowPointConnection"/>s contained by this <see cref="FollowPointRenderer"/>.
         /// </summary>
-        public int PointDistance
+        internal IReadOnlyList<FollowPointConnection> Connections => connections;
-        {
-            get => pointDistance;
-            set
-            {
-                if (pointDistance == value) return;
 
-                pointDistance = value;
+        private readonly List<FollowPointConnection> connections = new List<FollowPointConnection>();
-                update();
-            }
-        }
-
-        private int preEmpt = 800;
-
-        /// <summary>
-        /// Follow points to the next hitobject start appearing for this many milliseconds before an hitobject's end time.
-        /// </summary>
-        public int PreEmpt
-        {
-            get => preEmpt;
-            set
-            {
-                if (preEmpt == value) return;
-
-                preEmpt = value;
-                update();
-            }
-        }
-
-        private IEnumerable<OsuHitObject> hitObjects;
-
-        public override IEnumerable<OsuHitObject> HitObjects
-        {
-            get => hitObjects;
-            set
-            {
-                hitObjects = value;
-                update();
-            }
-        }
 
         public override bool RemoveCompletedTransforms => false;
 
-        private void update()
+        /// <summary>
+        /// Adds the <see cref="FollowPoint"/>s around a <see cref="DrawableOsuHitObject"/>.
+        /// This includes <see cref="FollowPoint"/>s leading into <paramref name="hitObject"/>, and <see cref="FollowPoint"/>s exiting <paramref name="hitObject"/>.
+        /// </summary>
+        /// <param name="hitObject">The <see cref="DrawableOsuHitObject"/> to add <see cref="FollowPoint"/>s for.</param>
+        public void AddFollowPoints(DrawableOsuHitObject hitObject)
+            => addConnection(new FollowPointConnection(hitObject).With(g => g.StartTime.BindValueChanged(_ => onStartTimeChanged(g))));
+
+        /// <summary>
+        /// Removes the <see cref="FollowPoint"/>s around a <see cref="DrawableOsuHitObject"/>.
+        /// This includes <see cref="FollowPoint"/>s leading into <paramref name="hitObject"/>, and <see cref="FollowPoint"/>s exiting <paramref name="hitObject"/>.
+        /// </summary>
+        /// <param name="hitObject">The <see cref="DrawableOsuHitObject"/> to remove <see cref="FollowPoint"/>s for.</param>
+        public void RemoveFollowPoints(DrawableOsuHitObject hitObject) => removeGroup(connections.Single(g => g.Start == hitObject));
+
+        /// <summary>
+        /// Adds a <see cref="FollowPointConnection"/> to this <see cref="FollowPointRenderer"/>.
+        /// </summary>
+        /// <param name="connection">The <see cref="FollowPointConnection"/> to add.</param>
+        /// <returns>The index of <paramref name="connection"/> in <see cref="connections"/>.</returns>
+        private void addConnection(FollowPointConnection connection)
         {
-            ClearInternal();
+            AddInternal(connection);
 
-            if (hitObjects == null)
+            // Groups are sorted by their start time when added such that the index can be used to post-process other surrounding connections
-                return;
+            int index = connections.AddInPlace(connection, Comparer<FollowPointConnection>.Create((g1, g2) => g1.StartTime.Value.CompareTo(g2.StartTime.Value)));
 
-            OsuHitObject prevHitObject = null;
+            if (index < connections.Count - 1)
-
-            foreach (var currHitObject in hitObjects)
             {
-                if (prevHitObject != null && !currHitObject.NewCombo && !(prevHitObject is Spinner) && !(currHitObject is Spinner))
+                // Update the connection's end point to the next connection's start point
-                {
+                //     h1 -> -> -> h2
-                    Vector2 startPosition = prevHitObject.EndPosition;
+                //    connection    nextGroup
-                    Vector2 endPosition = currHitObject.Position;
-                    double startTime = (prevHitObject as IHasEndTime)?.EndTime ?? prevHitObject.StartTime;
-                    double endTime = currHitObject.StartTime;
 
-                    Vector2 distanceVector = endPosition - startPosition;
+                FollowPointConnection nextConnection = connections[index + 1];
-                    int distance = (int)distanceVector.Length;
+                connection.End = nextConnection.Start;
-                    float rotation = (float)(Math.Atan2(distanceVector.Y, distanceVector.X) * (180 / Math.PI));
-                    double duration = endTime - startTime;
-
-                    for (int d = (int)(PointDistance * 1.5); d < distance - PointDistance; d += PointDistance)
-                    {
-                        float fraction = (float)d / distance;
-                        Vector2 pointStartPosition = startPosition + (fraction - 0.1f) * distanceVector;
-                        Vector2 pointEndPosition = startPosition + fraction * distanceVector;
-                        double fadeOutTime = startTime + fraction * duration;
-                        double fadeInTime = fadeOutTime - PreEmpt;
-
-                        FollowPoint fp;
-
-                        AddInternal(fp = new FollowPoint
-                        {
-                            Position = pointStartPosition,
-                            Rotation = rotation,
-                            Alpha = 0,
-                            Scale = new Vector2(1.5f * currHitObject.Scale),
-                        });
-
-                        using (fp.BeginAbsoluteSequence(fadeInTime))
-                        {
-                            fp.FadeIn(currHitObject.TimeFadeIn);
-                            fp.ScaleTo(currHitObject.Scale, currHitObject.TimeFadeIn, Easing.Out);
-
-                            fp.MoveTo(pointEndPosition, currHitObject.TimeFadeIn, Easing.Out);
-
-                            fp.Delay(fadeOutTime - fadeInTime).FadeOut(currHitObject.TimeFadeIn);
-                        }
-
-                        fp.Expire(true);
-                    }
-                }
-
-                prevHitObject = currHitObject;
             }
+            else
+            {
+                // The end point may be non-null during re-ordering
+                connection.End = null;
+            }
+
+            if (index > 0)
+            {
+                // Update the previous connection's end point to the current connection's start point
+                //     h1 -> -> -> h2
+                //  prevGroup    connection
+
+                FollowPointConnection previousConnection = connections[index - 1];
+                previousConnection.End = connection.Start;
+            }
+        }
+
+        /// <summary>
+        /// Removes a <see cref="FollowPointConnection"/> from this <see cref="FollowPointRenderer"/>.
+        /// </summary>
+        /// <param name="connection">The <see cref="FollowPointConnection"/> to remove.</param>
+        /// <returns>Whether <paramref name="connection"/> was removed.</returns>
+        private void removeGroup(FollowPointConnection connection)
+        {
+            RemoveInternal(connection);
+
+            int index = connections.IndexOf(connection);
+
+            if (index > 0)
+            {
+                // Update the previous connection's end point to the next connection's start point
+                //     h1 -> -> -> h2 -> -> -> h3
+                //  prevGroup    connection       nextGroup
+                // The current connection's end point is used since there may not be a next connection
+                FollowPointConnection previousConnection = connections[index - 1];
+                previousConnection.End = connection.End;
+            }
+
+            connections.Remove(connection);
+        }
+
+        private void onStartTimeChanged(FollowPointConnection connection)
+        {
+            // Naive but can be improved if performance becomes an issue
+            removeGroup(connection);
+            addConnection(connection);
         }
     }
 }

osu.Game.Rulesets.Osu/UI/OsuPlayfield.cs

@@ -9,7 +9,6 @@ using osu.Game.Rulesets.Osu.Objects;
 using osu.Game.Rulesets.Osu.Objects.Drawables;
 using osu.Game.Rulesets.Osu.Objects.Drawables.Connections;
 using osu.Game.Rulesets.UI;
-using System.Linq;
 using osu.Game.Rulesets.Judgements;
 using osu.Game.Rulesets.Osu.UI.Cursor;
 using osu.Game.Skinning;
@@ -20,7 +19,7 @@ namespace osu.Game.Rulesets.Osu.UI
     {
         private readonly ApproachCircleProxyContainer approachCircles;
         private readonly JudgementContainer<DrawableOsuJudgement> judgementLayer;
-        private readonly ConnectionRenderer<OsuHitObject> connectionLayer;
+        private readonly FollowPointRenderer followPoints;
 
         public static readonly Vector2 BASE_SIZE = new Vector2(512, 384);
 
@@ -30,7 +29,7 @@ namespace osu.Game.Rulesets.Osu.UI
         {
             InternalChildren = new Drawable[]
             {
-                connectionLayer = new FollowPointRenderer
+                followPoints = new FollowPointRenderer
                 {
                     RelativeSizeAxes = Axes.Both,
                     Depth = 2,
@@ -64,11 +63,18 @@ namespace osu.Game.Rulesets.Osu.UI
             };
 
             base.Add(h);
+
+            followPoints.AddFollowPoints((DrawableOsuHitObject)h);
         }
 
-        public override void PostProcess()
+        public override bool Remove(DrawableHitObject h)
         {
-            connectionLayer.HitObjects = HitObjectContainer.Objects.Select(d => d.HitObject).OfType<OsuHitObject>();
+            bool result = base.Remove(h);
+
+            if (result)
+                followPoints.RemoveFollowPoints((DrawableOsuHitObject)h);
+
+            return result;
         }
 
         private void onNewResult(DrawableHitObject judgedObject, JudgementResult result)