Merge pull request #12280 from bdach/fix-editor-arrow-seek-snapping

Fix editor arrow seek snapping not updating after control point changes

osu.Game.Tests/Visual/Editing/TestSceneEditorSeeking.cs

@@ -0,0 +1,79 @@
+// 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 NUnit.Framework;
+using osu.Framework.Utils;
+using osu.Game.Beatmaps;
+using osu.Game.Beatmaps.ControlPoints;
+using osu.Game.Rulesets;
+using osu.Game.Rulesets.Osu;
+using osuTK.Input;
+
+namespace osu.Game.Tests.Visual.Editing
+{
+    public class TestSceneEditorSeeking : EditorTestScene
+    {
+        protected override Ruleset CreateEditorRuleset() => new OsuRuleset();
+
+        protected override IBeatmap CreateBeatmap(RulesetInfo ruleset)
+        {
+            var beatmap = base.CreateBeatmap(ruleset);
+
+            beatmap.BeatmapInfo.BeatDivisor = 1;
+
+            beatmap.ControlPointInfo = new ControlPointInfo();
+            beatmap.ControlPointInfo.Add(0, new TimingControlPoint { BeatLength = 1000 });
+            beatmap.ControlPointInfo.Add(2000, new TimingControlPoint { BeatLength = 500 });
+
+            return beatmap;
+        }
+
+        [Test]
+        public void TestSnappedSeeking()
+        {
+            AddStep("seek to 0", () => EditorClock.Seek(0));
+            AddAssert("time is 0", () => EditorClock.CurrentTime == 0);
+
+            pressAndCheckTime(Key.Right, 1000);
+            pressAndCheckTime(Key.Right, 2000);
+            pressAndCheckTime(Key.Right, 2500);
+            pressAndCheckTime(Key.Right, 3000);
+
+            pressAndCheckTime(Key.Left, 2500);
+            pressAndCheckTime(Key.Left, 2000);
+            pressAndCheckTime(Key.Left, 1000);
+        }
+
+        [Test]
+        public void TestSnappedSeekingAfterControlPointChange()
+        {
+            AddStep("seek to 0", () => EditorClock.Seek(0));
+            AddAssert("time is 0", () => EditorClock.CurrentTime == 0);
+
+            pressAndCheckTime(Key.Right, 1000);
+            pressAndCheckTime(Key.Right, 2000);
+            pressAndCheckTime(Key.Right, 2500);
+            pressAndCheckTime(Key.Right, 3000);
+
+            AddStep("remove 2nd timing point", () =>
+            {
+                EditorBeatmap.BeginChange();
+                var group = EditorBeatmap.ControlPointInfo.GroupAt(2000);
+                EditorBeatmap.ControlPointInfo.RemoveGroup(group);
+                EditorBeatmap.EndChange();
+            });
+
+            pressAndCheckTime(Key.Left, 2000);
+            pressAndCheckTime(Key.Left, 1000);
+
+            pressAndCheckTime(Key.Right, 2000);
+            pressAndCheckTime(Key.Right, 3000);
+        }
+
+        private void pressAndCheckTime(Key key, double expectedTime)
+        {
+            AddStep($"press {key}", () => InputManager.Key(key));
+            AddUntilStep($"time is {expectedTime}", () => Precision.AlmostEquals(expectedTime, EditorClock.CurrentTime, 1));
+        }
+    }
+}

osu.Game/Screens/Edit/Editor.cs

@@ -122,22 +122,6 @@ namespace osu.Game.Screens.Edit
                     return;
             }
 
-            beatDivisor.Value = loadableBeatmap.BeatmapInfo.BeatDivisor;
-            beatDivisor.BindValueChanged(divisor => loadableBeatmap.BeatmapInfo.BeatDivisor = divisor.NewValue);
-
-            // Todo: should probably be done at a DrawableRuleset level to share logic with Player.
-            clock = new EditorClock(loadableBeatmap, beatDivisor) { IsCoupled = false };
-
-            UpdateClockSource();
-
-            dependencies.CacheAs(clock);
-            AddInternal(clock);
-
-            clock.SeekingOrStopped.BindValueChanged(_ => updateSampleDisabledState());
-
-            // todo: remove caching of this and consume via editorBeatmap?
-            dependencies.Cache(beatDivisor);
-
             try
             {
                 playableBeatmap = loadableBeatmap.GetPlayableBeatmap(loadableBeatmap.BeatmapInfo.Ruleset);
@@ -154,6 +138,22 @@ namespace osu.Game.Screens.Edit
                 return;
             }
 
+            beatDivisor.Value = playableBeatmap.BeatmapInfo.BeatDivisor;
+            beatDivisor.BindValueChanged(divisor => playableBeatmap.BeatmapInfo.BeatDivisor = divisor.NewValue);
+
+            // Todo: should probably be done at a DrawableRuleset level to share logic with Player.
+            clock = new EditorClock(playableBeatmap, beatDivisor) { IsCoupled = false };
+
+            UpdateClockSource();
+
+            dependencies.CacheAs(clock);
+            AddInternal(clock);
+
+            clock.SeekingOrStopped.BindValueChanged(_ => updateSampleDisabledState());
+
+            // todo: remove caching of this and consume via editorBeatmap?
+            dependencies.Cache(beatDivisor);
+
             AddInternal(editorBeatmap = new EditorBeatmap(playableBeatmap, loadableBeatmap.Skin));
             dependencies.CacheAs(editorBeatmap);
             changeHandler = new EditorChangeHandler(editorBeatmap);

osu.Game/Screens/Edit/EditorClock.cs

@@ -42,12 +42,12 @@ namespace osu.Game.Screens.Edit
         /// </summary>
         public bool IsSeeking { get; private set; }
 
-        public EditorClock(WorkingBeatmap beatmap, BindableBeatDivisor beatDivisor)
+        public EditorClock(IBeatmap beatmap, BindableBeatDivisor beatDivisor)
-            : this(beatmap.Beatmap.ControlPointInfo, beatmap.Track.Length, beatDivisor)
+            : this(beatmap.ControlPointInfo, beatDivisor)
         {
         }
 
-        public EditorClock(ControlPointInfo controlPointInfo, double trackLength, BindableBeatDivisor beatDivisor)
+        public EditorClock(ControlPointInfo controlPointInfo, BindableBeatDivisor beatDivisor)
         {
             this.beatDivisor = beatDivisor;
 
@@ -57,7 +57,7 @@ namespace osu.Game.Screens.Edit
         }
 
         public EditorClock()
-            : this(new ControlPointInfo(), 1000, new BindableBeatDivisor())
+            : this(new ControlPointInfo(), new BindableBeatDivisor())
         {
         }
 

osu.Game/Tests/Visual/EditorClockTestScene.cs

@@ -24,7 +24,7 @@ namespace osu.Game.Tests.Visual
 
         protected EditorClockTestScene()
         {
-            Clock = new EditorClock(new ControlPointInfo(), 5000, BeatDivisor) { IsCoupled = false };
+            Clock = new EditorClock(new ControlPointInfo(), BeatDivisor) { IsCoupled = false };
         }
 
         protected override IReadOnlyDependencyContainer CreateChildDependencies(IReadOnlyDependencyContainer parent)