Adjust editor seeking to be fairer when track is playing

[peppy]

The previous multiplier was supposed to account for seeking backwards being "slower" because the track is playing forwards, but it really didn't work amazingly.

Rather than trying to pull off some magic, let's just ensure that seeks in both directions feel correct, even if that means that time gradually moves forwards.

Closes #35998 (hopefully). Now when rapidly scrolling over a known range of mouse wheel like the user was, things gradually move forwards, but the drift is far lower than what they were seeing.

Of note, this still doesn't match stable completely. I fully implemented the full stable seeking algorithm but concluded it's objectively worse in other scenarios, so I'd rather just tweak what we have until the majority of users are happy.

Here's more-than-twenty-minutes-spent-making-things-work-like-stable. Was not worth it.

diff --git a/osu.Game.Rulesets.Catch.Tests/Editor/CatchSelectionBlueprintTestScene.cs b/osu.Game.Rulesets.Catch.Tests/Editor/CatchSelectionBlueprintTestScene.cs
index 6dfc74e75c..2a7cb8b799 100644
--- a/osu.Game.Rulesets.Catch.Tests/Editor/CatchSelectionBlueprintTestScene.cs
+++ b/osu.Game.Rulesets.Catch.Tests/Editor/CatchSelectionBlueprintTestScene.cs
@@ -60,7 +60,7 @@ protected void AddMouseMoveStep(double time, float x) => AddStep($"move to time=
             InputManager.MoveMouseTo(pos);
         });
 
-        private partial class EditorBeatmapDependencyContainer : Container
+        private partial class EditorBeatmapDependencyContainer : Container, IBeatSnapProvider
         {
             [Cached]
             private readonly EditorClock editorClock;
@@ -76,10 +76,14 @@ public EditorBeatmapDependencyContainer(IBeatmap beatmap, BindableBeatDivisor be
 
                 InternalChildren = new Drawable[]
                 {
-                    editorClock = new EditorClock(beatmap, beatDivisor),
+                    editorClock = new EditorClock(beatmap, this),
                     Content,
                 };
             }
+
+            public double SnapTime(double time, double? referenceTime = null) => time;
+            public double GetBeatLengthAtTime(double referenceTime) => 10000;
+            int IBeatSnapProvider.BeatDivisor => beatDivisor.Value;
         }
     }
 }
diff --git a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneObjectObjectSnap.cs b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneObjectObjectSnap.cs
index 9af1855167..1d8a9756c3 100644
--- a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneObjectObjectSnap.cs
+++ b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneObjectObjectSnap.cs
@@ -101,12 +101,12 @@ public void TestSecondCircleInSelectionAlsoSnaps()
 
             AddStep("place first object", () => InputManager.Click(MouseButton.Left));
 
-            AddStep("increment time", () => EditorClock.SeekForward(true));
+            AddStep("increment time", () => EditorClock.SeekForward());
 
             AddStep("move mouse right", () => InputManager.MoveMouseTo(playfield.ScreenSpaceDrawQuad.Centre + new Vector2(playfield.ScreenSpaceDrawQuad.Width * 0.2f, 0)));
             AddStep("place second object", () => InputManager.Click(MouseButton.Left));
 
-            AddStep("increment time", () => EditorClock.SeekForward(true));
+            AddStep("increment time", () => EditorClock.SeekForward());
 
             AddStep("move mouse down", () => InputManager.MoveMouseTo(playfield.ScreenSpaceDrawQuad.Centre + new Vector2(0, playfield.ScreenSpaceDrawQuad.Width * 0.2f)));
             AddStep("place third object", () => InputManager.Click(MouseButton.Left));
diff --git a/osu.Game.Tests/Visual/Editing/TestSceneEditorSeekSnapping.cs b/osu.Game.Tests/Visual/Editing/TestSceneEditorSeekSnapping.cs
index 9df5df6b39..5612711d89 100644
--- a/osu.Game.Tests/Visual/Editing/TestSceneEditorSeekSnapping.cs
+++ b/osu.Game.Tests/Visual/Editing/TestSceneEditorSeekSnapping.cs
@@ -152,17 +152,17 @@ public void TestSeekForwardSnappingOnBeat()
         {
             reset();
 
-            AddStep("SeekForward, Snap", () => EditorClock.SeekForward(true));
+            AddStep("SeekForward, Snap", () => EditorClock.SeekForward());
             checkTime(50);
-            AddStep("SeekForward, Snap", () => EditorClock.SeekForward(true));
+            AddStep("SeekForward, Snap", () => EditorClock.SeekForward());
             checkTime(100);
-            AddStep("SeekForward, Snap", () => EditorClock.SeekForward(true));
+            AddStep("SeekForward, Snap", () => EditorClock.SeekForward());
             checkTime(175);
-            AddStep("SeekForward, Snap", () => EditorClock.SeekForward(true));
+            AddStep("SeekForward, Snap", () => EditorClock.SeekForward());
             checkTime(350);
-            AddStep("SeekForward, Snap", () => EditorClock.SeekForward(true));
+            AddStep("SeekForward, Snap", () => EditorClock.SeekForward());
             checkTime(400);
-            AddStep("SeekForward, Snap", () => EditorClock.SeekForward(true));
+            AddStep("SeekForward, Snap", () => EditorClock.SeekForward());
             checkTime(450);
         }
 
@@ -177,28 +177,28 @@ public void TestSeekForwardSnappingFromInBetweenBeat()
 
             AddStep("Seek(49)", () => EditorClock.Seek(49));
             checkTime(49);
-            AddStep("SeekForward, Snap", () => EditorClock.SeekForward(true));
+            AddStep("SeekForward, Snap", () => EditorClock.SeekForward());
             checkTime(50);
             AddStep("Seek(49.999)", () => EditorClock.Seek(49.999));
-            AddStep("SeekForward, Snap", () => EditorClock.SeekForward(true));
+            AddStep("SeekForward, Snap", () => EditorClock.SeekForward());
             checkTime(100);
             AddStep("Seek(99)", () => EditorClock.Seek(99));
-            AddStep("SeekForward, Snap", () => EditorClock.SeekForward(true));
+            AddStep("SeekForward, Snap", () => EditorClock.SeekForward());
             checkTime(100);
             AddStep("Seek(99.999)", () => EditorClock.Seek(99.999));
-            AddStep("SeekForward, Snap", () => EditorClock.SeekForward(true));
+            AddStep("SeekForward, Snap", () => EditorClock.SeekForward());
             checkTime(150);
             AddStep("Seek(174)", () => EditorClock.Seek(174));
-            AddStep("SeekForward, Snap", () => EditorClock.SeekForward(true));
+            AddStep("SeekForward, Snap", () => EditorClock.SeekForward());
             checkTime(175);
             AddStep("Seek(349)", () => EditorClock.Seek(349));
-            AddStep("SeekForward, Snap", () => EditorClock.SeekForward(true));
+            AddStep("SeekForward, Snap", () => EditorClock.SeekForward());
             checkTime(350);
             AddStep("Seek(399)", () => EditorClock.Seek(399));
-            AddStep("SeekForward, Snap", () => EditorClock.SeekForward(true));
+            AddStep("SeekForward, Snap", () => EditorClock.SeekForward());
             checkTime(400);
             AddStep("Seek(449)", () => EditorClock.Seek(449));
-            AddStep("SeekForward, Snap", () => EditorClock.SeekForward(true));
+            AddStep("SeekForward, Snap", () => EditorClock.SeekForward());
             checkTime(450);
         }
 
@@ -234,17 +234,17 @@ public void TestSeekBackwardSnappingOnBeat()
 
             AddStep("Seek(450)", () => EditorClock.Seek(450));
             checkTime(450);
-            AddStep("SeekBackward, Snap", () => EditorClock.SeekBackward(true));
+            AddStep("SeekBackward, Snap", () => EditorClock.SeekBackward());
             checkTime(400);
-            AddStep("SeekBackward, Snap", () => EditorClock.SeekBackward(true));
+            AddStep("SeekBackward, Snap", () => EditorClock.SeekBackward());
             checkTime(350);
-            AddStep("SeekBackward, Snap", () => EditorClock.SeekBackward(true));
+            AddStep("SeekBackward, Snap", () => EditorClock.SeekBackward());
             checkTime(175);
-            AddStep("SeekBackward, Snap", () => EditorClock.SeekBackward(true));
+            AddStep("SeekBackward, Snap", () => EditorClock.SeekBackward());
             checkTime(100);
-            AddStep("SeekBackward, Snap", () => EditorClock.SeekBackward(true));
+            AddStep("SeekBackward, Snap", () => EditorClock.SeekBackward());
             checkTime(50);
-            AddStep("SeekBackward, Snap", () => EditorClock.SeekBackward(true));
+            AddStep("SeekBackward, Snap", () => EditorClock.SeekBackward());
             checkTime(0);
         }
 
@@ -259,16 +259,16 @@ public void TestSeekBackwardSnappingFromInBetweenBeat()
 
             AddStep("Seek(451)", () => EditorClock.Seek(451));
             checkTime(451);
-            AddStep("SeekBackward, Snap", () => EditorClock.SeekBackward(true));
+            AddStep("SeekBackward, Snap", () => EditorClock.SeekBackward());
             checkTime(450);
             AddStep("Seek(450.999)", () => EditorClock.Seek(450.999));
-            AddStep("SeekBackward, Snap", () => EditorClock.SeekBackward(true));
+            AddStep("SeekBackward, Snap", () => EditorClock.SeekBackward());
             checkTime(450);
             AddStep("Seek(401)", () => EditorClock.Seek(401));
-            AddStep("SeekBackward, Snap", () => EditorClock.SeekBackward(true));
+            AddStep("SeekBackward, Snap", () => EditorClock.SeekBackward());
             checkTime(400);
             AddStep("Seek(401.999)", () => EditorClock.Seek(401.999));
-            AddStep("SeekBackward, Snap", () => EditorClock.SeekBackward(true));
+            AddStep("SeekBackward, Snap", () => EditorClock.SeekBackward());
             checkTime(400);
         }
 
@@ -290,7 +290,7 @@ public void TestSeekingWithFloatingPointBeatLength()
                 AddStep("SeekForward, Snap", () =>
                 {
                     lastTime = EditorClock.CurrentTime;
-                    EditorClock.SeekForward(true);
+                    EditorClock.SeekForward();
                 });
                 AddAssert("Time > lastTime", () => EditorClock.CurrentTime > lastTime);
             }
@@ -300,7 +300,7 @@ public void TestSeekingWithFloatingPointBeatLength()
                 AddStep("SeekBackward, Snap", () =>
                 {
                     lastTime = EditorClock.CurrentTime;
-                    EditorClock.SeekBackward(true);
+                    EditorClock.SeekBackward();
                 });
                 AddAssert("Time < lastTime", () => EditorClock.CurrentTime < lastTime);
             }
diff --git a/osu.Game/Screens/Edit/Editor.cs b/osu.Game/Screens/Edit/Editor.cs
index d40d36530a..c6af97da80 100644
--- a/osu.Game/Screens/Edit/Editor.cs
+++ b/osu.Game/Screens/Edit/Editor.cs
@@ -286,7 +286,7 @@ private void load(OsuConfigManager config)
             }
 
             // Todo: should probably be done at a DrawableRuleset level to share logic with Player.
-            clock = new EditorClock(playableBeatmap, beatDivisor);
+            clock = new EditorClock(playableBeatmap, this);
             clock.ChangeSource(loadableBeatmap.Track);
 
             dependencies.CacheAs(clock);
@@ -1248,21 +1248,10 @@ private void seek(UIEvent e, int direction)
         {
             double amount = e.ShiftPressed ? 4 : 1;
 
-            bool trackPlaying = clock.IsRunning;
-
-            if (trackPlaying)
-            {
-                // generally users are not looking to perform tiny seeks when the track is playing.
-                // this multiplication undoes the division that will be applied in the underlying seek operation.
-                // scale by BPM to keep the seek amount constant across all BPMs.
-                var timingPoint = editorBeatmap.ControlPointInfo.TimingPointAt(clock.CurrentTimeAccurate);
-                amount *= beatDivisor.Value * (timingPoint.BPM / 120);
-            }
-
             if (direction < 1)
-                clock.SeekBackward(!trackPlaying, amount);
+                clock.SeekBackward(amount);
             else
-                clock.SeekForward(!trackPlaying, amount);
+                clock.SeekForward(amount);
         }
 
         private void updateLastSavedHash()
diff --git a/osu.Game/Screens/Edit/EditorClock.cs b/osu.Game/Screens/Edit/EditorClock.cs
index 8b9bdb595d..2cc5580848 100644
--- a/osu.Game/Screens/Edit/EditorClock.cs
+++ b/osu.Game/Screens/Edit/EditorClock.cs
@@ -17,6 +17,7 @@
 using osu.Framework.Utils;
 using osu.Game.Beatmaps;
 using osu.Game.Beatmaps.ControlPoints;
+using osu.Game.Rulesets.Edit;
 
 namespace osu.Game.Screens.Edit
 {
@@ -38,7 +39,8 @@ public partial class EditorClock : CompositeComponent, IFrameBasedClock, IAdjust
 
         public IBeatmap Beatmap { get; set; }
 
-        private readonly BindableBeatDivisor beatDivisor;
+        [CanBeNull]
+        private readonly IBeatSnapProvider beatSnapProvider;
 
         private readonly FramedBeatmapClock underlyingClock;
 
@@ -53,11 +55,11 @@ public partial class EditorClock : CompositeComponent, IFrameBasedClock, IAdjust
         /// </summary>
         public bool IsSeeking { get; private set; }
 
-        public EditorClock(IBeatmap beatmap = null, BindableBeatDivisor beatDivisor = null)
+        public EditorClock(IBeatmap beatmap = null, IBeatSnapProvider beatSnapProvider = null)
         {
             Beatmap = beatmap ?? new Beatmap();
 
-            this.beatDivisor = beatDivisor ?? new BindableBeatDivisor();
+            this.beatSnapProvider = beatSnapProvider;
 
             underlyingClock = new FramedBeatmapClock(applyOffsets: true, requireDecoupling: true);
             AddInternal(underlyingClock);
@@ -73,7 +75,7 @@ public EditorClock(IBeatmap beatmap = null, BindableBeatDivisor beatDivisor = nu
         public bool SeekSnapped(double position)
         {
             var timingPoint = ControlPointInfo.TimingPointAt(position);
-            double beatSnapLength = timingPoint.BeatLength / beatDivisor.Value;
+            double beatSnapLength = timingPoint.BeatLength / (beatSnapProvider?.BeatDivisor ?? 1);
 
             // We will be snapping to beats within the timing point
             position -= timingPoint.Time;
@@ -94,67 +96,58 @@ public bool SeekSnapped(double position)
         /// <summary>
         /// Seeks backwards by one beat length.
         /// </summary>
-        /// <param name="snapped">Whether to snap to the closest beat after seeking.</param>
         /// <param name="amount">The relative amount (magnitude) which should be seeked.</param>
-        public void SeekBackward(bool snapped = false, double amount = 1) => seek(-1, snapped, amount + (IsRunning ? 1.5 : 0));
+        public void SeekBackward(double amount = 1) => seek(-1, amount);
 
         /// <summary>
         /// Seeks forwards by one beat length.
         /// </summary>
-        /// <param name="snapped">Whether to snap to the closest beat after seeking.</param>
         /// <param name="amount">The relative amount (magnitude) which should be seeked.</param>
-        public void SeekForward(bool snapped = false, double amount = 1) => seek(1, snapped, amount);
+        public void SeekForward(double amount = 1) => seek(1, amount);
 
-        private void seek(int direction, bool snapped, double amount = 1)
+        private void seek(int direction, double rate = 1)
         {
-            double current = CurrentTimeAccurate;
-
-            if (amount <= 0) throw new ArgumentException("Value should be greater than zero", nameof(amount));
-
-            var timingPoint = ControlPointInfo.TimingPointAt(current);
+            if (rate <= 0) throw new ArgumentException("Value should be greater than zero", nameof(rate));
 
-            if (direction < 0 && timingPoint.Time == current)
-                // When going backwards and we're at the boundary of two timing points, we compute the seek distance with the timing point which we are seeking into
-                timingPoint = ControlPointInfo.TimingPointAt(current - 1);
-
-            double seekAmount = timingPoint.BeatLength / beatDivisor.Value * amount;
-            double seekTime = current + seekAmount * direction;
+            double current = CurrentTimeAccurate;
+            double seekTime = current;
+            double beatLength = getBeatLength(current);
 
-            if (!snapped || ControlPointInfo.TimingPoints.Count == 0)
+            if (IsRunning)
             {
-                SeekSmoothlyTo(seekTime);
-                return;
+                // seek multiplier when track is playing. matches stable because don't-break-what-works
+                // https://github.com/peppy/osu-stable-reference/blob/7519cafd1823f1879c0d9c991ba0e5c7fd3bfa02/osu!/GameModes/Edit/Editor.cs#L1639-L1640
+                //
+                // ReSharper disable once PossibleLossOfFraction
+                rate *= 1 + 250 / (int)beatLength;
             }
 
-            // We will be snapping to beats within timingPoint
-            seekTime -= timingPoint.Time;
+            // the actual beat length we should use is the one at the proposed seek target.
+            beatLength = getBeatLength(current + direction * beatLength * rate);
 
-            // Determine the index from timingPoint of the closest beat to seekTime, accounting for scrolling direction
-            int closestBeat;
-            if (direction > 0)
-                closestBeat = (int)Math.Floor(seekTime / seekAmount);
-            else
-                closestBeat = (int)Math.Ceiling(seekTime / seekAmount);
+            seekTime += direction * beatLength * rate;
 
-            seekTime = timingPoint.Time + closestBeat * seekAmount;
+            if (beatSnapProvider != null)
+                seekTime = beatSnapProvider.SnapTime(seekTime);
 
-            // limit forward seeking to only up to the next timing point's start time.
-            var nextTimingPoint = ControlPointInfo.TimingPointAfter(timingPoint.Time);
-            if (seekTime > nextTimingPoint?.Time)
-                seekTime = nextTimingPoint.Time;
+            SeekSmoothlyTo(seekTime);
 
-            // Due to the rounding above, we may end up on the current beat. This will effectively cause 0 seeking to happen, but we don't want this.
-            // Instead, we'll go to the next beat in the direction when this is the case
-            if (Precision.AlmostEquals(current, seekTime, 0.5f))
+            double getBeatLength(double time)
             {
-                closestBeat += direction > 0 ? 1 : -1;
-                seekTime = timingPoint.Time + closestBeat * seekAmount;
-            }
+                if (beatSnapProvider == null)
+                    return 1000;
 
-            if (seekTime < timingPoint.Time && !ReferenceEquals(timingPoint, ControlPointInfo.TimingPoints.First()))
-                seekTime = timingPoint.Time;
+                double length = beatSnapProvider.GetBeatLengthAtTime(time);
 
-            SeekSmoothlyTo(seekTime);
+                if (IsRunning)
+                {
+                    // when playing, always seek at beat divisor 1.
+                    // this undoes the divisor application inside the `GetBeatLengthAt` call above.
+                    length *= (beatSnapProvider?.BeatDivisor ?? 1);
+                }
+
+                return length;
+            }
         }
 
         /// <summary>
diff --git a/osu.Game/Tests/Visual/EditorClockTestScene.cs b/osu.Game/Tests/Visual/EditorClockTestScene.cs
index 204a817b3a..312dffc733 100644
--- a/osu.Game/Tests/Visual/EditorClockTestScene.cs
+++ b/osu.Game/Tests/Visual/EditorClockTestScene.cs
@@ -11,6 +11,7 @@
 using osu.Framework.Input.Events;
 using osu.Game.Beatmaps;
 using osu.Game.Overlays;
+using osu.Game.Rulesets.Edit;
 using osu.Game.Screens.Edit;
 
 namespace osu.Game.Tests.Visual
@@ -19,7 +20,7 @@ namespace osu.Game.Tests.Visual
     /// Provides a clock, beat-divisor, and scrolling capability for test cases of editor components that
     /// are preferrably tested within the presence of a clock and seek controls.
     /// </summary>
-    public abstract partial class EditorClockTestScene : OsuManualInputManagerTestScene
+    public abstract partial class EditorClockTestScene : OsuManualInputManagerTestScene, IBeatSnapProvider
     {
         [Cached]
         private readonly OverlayColourProvider overlayColour = new OverlayColourProvider(OverlayColourScheme.Aquamarine);
@@ -45,7 +46,7 @@ protected override IReadOnlyDependencyContainer CreateChildDependencies(IReadOnl
 
             base.Content.AddRange(new Drawable[]
             {
-                EditorClock = new EditorClock(editorClockBeatmap, BeatDivisor),
+                EditorClock = new EditorClock(editorClockBeatmap, this),
                 content
             });
 
@@ -81,11 +82,15 @@ protected override bool OnScroll(ScrollEvent e)
                 return false;
 
             if (e.ScrollDelta.Y > 0)
-                EditorClock.SeekBackward(true);
+                EditorClock.SeekBackward();
             else
-                EditorClock.SeekForward(true);
+                EditorClock.SeekForward();
 
             return true;
         }
+
+        public double SnapTime(double time, double? referenceTime = null) => time;
+        public double GetBeatLengthAtTime(double referenceTime) => 10000;
+        int IBeatSnapProvider.BeatDivisor => BeatDivisor.Value;
     }
 }

[bdach]

Seems to mostly address #35281 too.

I don't have opinions on the resulting behaviour here so I'm okay with users providing some.