Merge pull request #92 from winnerspiros/timing-screen-selection-retention-17106411480944945514

Retain control point selection over undo/redo operations

Author: winnerspiros

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

@@ -75,10 +75,8 @@ public void SetUpSteps()
             AddUntilStep("Wait for rows to load", () => Child.ChildrenOfType<EffectRowAttribute>().Any());
         }
 
-        // TODO: this is best-effort for now, but the comment out test below should probably be how things should work.
-        // Was originally working as of https://github.com/ppy/osu/pull/26141; Regressed at some point.
         [Test]
-        public void TestSelectionDismissedOnUndo()
+        public void TestSelectedRetainedOverUndo()
         {
             AddStep("Select first timing point", () =>
             {
@@ -97,54 +95,26 @@ public void TestSelectionDismissedOnUndo()
 
             AddUntilStep("wait for offset changed", () =>
             {
-                return timingScreen.SelectedGroup.Value.ControlPoints.Any(c => c is TimingControlPoint) && timingScreen.SelectedGroup.Value?.Time > 2170;
+                return timingScreen.SelectedGroup.Value?.ControlPoints.Any(c => c is TimingControlPoint) == true && timingScreen.SelectedGroup.Value?.Time > 2170;
             });
 
             AddStep("undo", () => changeHandler?.RestoreState(-1));
 
-            AddUntilStep("selection dismissed", () => timingScreen.SelectedGroup.Value, () => Is.Null);
-        }
+            AddUntilStep("selection retained", () =>
+            {
+                return timingScreen.SelectedGroup.Value?.ControlPoints.Any(c => c is TimingControlPoint) == true && timingScreen.SelectedGroup.Value?.Time == 2170;
+            });
 
-        // [Test]
-        // public void TestSelectedRetainedOverUndo()
-        // {
-        //     AddStep("Select first timing point", () =>
-        //     {
-        //         InputManager.MoveMouseTo(Child.ChildrenOfType<TimingRowAttribute>().First());
-        //         InputManager.Click(MouseButton.Left);
-        //     });
-        //
-        //     AddUntilStep("Selection changed", () => timingScreen.SelectedGroup.Value?.Time == 2170);
-        //     AddUntilStep("Ensure seeked to correct time", () => EditorClock.CurrentTimeAccurate == 2170);
-        //
-        //     AddStep("Adjust offset", () =>
-        //     {
-        //         InputManager.MoveMouseTo(timingScreen.ChildrenOfType<TimingAdjustButton>().First().ScreenSpaceDrawQuad.Centre + new Vector2(20, 0));
-        //         InputManager.Click(MouseButton.Left);
-        //     });
-        //
-        //     AddUntilStep("wait for offset changed", () =>
-        //     {
-        //         return timingScreen.SelectedGroup.Value.ControlPoints.Any(c => c is TimingControlPoint) && timingScreen.SelectedGroup.Value?.Time > 2170;
-        //     });
-        //
-        //     AddStep("undo", () => changeHandler?.RestoreState(-1));
-        //
-        //     AddUntilStep("selection retained", () =>
-        //     {
-        //         return timingScreen.SelectedGroup.Value.ControlPoints.Any(c => c is TimingControlPoint) && timingScreen.SelectedGroup.Value?.Time > 2170;
-        //     });
-        //
-        //     AddAssert("check group count", () => editorBeatmap.ControlPointInfo.Groups.Count, () => Is.EqualTo(10));
-        //
-        //     AddStep("Adjust offset", () =>
-        //     {
-        //         InputManager.MoveMouseTo(timingScreen.ChildrenOfType<TimingAdjustButton>().First().ScreenSpaceDrawQuad.Centre + new Vector2(20, 0));
-        //         InputManager.Click(MouseButton.Left);
-        //     });
-        //
-        //     AddAssert("check group count", () => editorBeatmap.ControlPointInfo.Groups.Count, () => Is.EqualTo(10));
-        // }
+            AddAssert("check group count", () => editorBeatmap.ControlPointInfo.Groups.Count, () => Is.EqualTo(9));
+
+            AddStep("Adjust offset", () =>
+            {
+                InputManager.MoveMouseTo(timingScreen.ChildrenOfType<TimingAdjustButton>().First().ScreenSpaceDrawQuad.Centre + new Vector2(20, 0));
+                InputManager.Click(MouseButton.Left);
+            });
+
+            AddAssert("check group count", () => editorBeatmap.ControlPointInfo.Groups.Count, () => Is.EqualTo(9));
+        }
 
         [Test]
         public void TestScrollControlGroupIntoView()

osu.Game/Screens/Edit/Timing/ControlPointList.cs

@@ -187,8 +187,25 @@ private void onUndoRedo()
         {
             // Best effort. We have no tracking of control points through undo/redo changes.
             // If we don't deselect, things like offset changes could spawn groups to be added from previous states (see https://github.com/ppy/osu/issues/31098).
-            if (selectedGroup.Value != null && !Beatmap.ControlPointInfo.Groups.Contains(selectedGroup.Value))
-                selectedGroup.Value = null;
+            var lastSelected = selectedGroup.Value;
+
+            if (lastSelected == null)
+            {
+                SelectClosestTimingPoint?.Invoke();
+
+                return;
+            }
+
+            // Always clear the old (potentially orphaned) reference.
+            selectedGroup.Value = null;
+
+            // Try and find a group at the exact same time.
+            var matchingGroup = Beatmap.ControlPointInfo.GroupAt(lastSelected.Time);
+
+            if (matchingGroup != null)
+                selectedGroup.Value = matchingGroup;
+            else
+                SelectClosestTimingPoint?.Invoke();
         }
 
         protected override void Dispose(bool isDisposing)