Merge pull request #31348 from bdach/fix-slider-quick-delete

Fix quick-deleting unselected slider path control point also deleting all selected control points

osu.Game.Rulesets.Osu.Tests/Editor/TestSceneOsuComposerSelection.cs

@@ -10,6 +10,7 @@ using osu.Framework.Testing;
 using osu.Game.Beatmaps;
 using osu.Game.Graphics.UserInterface;
 using osu.Game.Rulesets.Objects;
+using osu.Game.Rulesets.Objects.Types;
 using osu.Game.Rulesets.Osu.Edit.Blueprints.HitCircles.Components;
 using osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components;
 using osu.Game.Rulesets.Osu.Objects;
@@ -261,6 +262,90 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor
             AddAssert("selection preserved", () => EditorBeatmap.SelectedHitObjects.Count, () => Is.EqualTo(1));
         }
 
+        [Test]
+        public void TestQuickDeleteOnUnselectedControlPointOnlyRemovesThatControlPoint()
+        {
+            var slider = new Slider
+            {
+                StartTime = 0,
+                Position = new Vector2(100, 100),
+                Path = new SliderPath
+                {
+                    ControlPoints =
+                    {
+                        new PathControlPoint { Type = PathType.LINEAR },
+                        new PathControlPoint(new Vector2(100, 0)),
+                        new PathControlPoint(new Vector2(100)),
+                        new PathControlPoint(new Vector2(0, 100))
+                    }
+                }
+            };
+            AddStep("add slider", () => EditorBeatmap.Add(slider));
+            AddStep("select slider", () => EditorBeatmap.SelectedHitObjects.Add(slider));
+
+            AddStep("select second node", () =>
+            {
+                InputManager.MoveMouseTo(this.ChildrenOfType<PathControlPointPiece<Slider>>().ElementAt(1));
+                InputManager.Click(MouseButton.Left);
+            });
+            AddStep("also select third node", () =>
+            {
+                InputManager.PressKey(Key.ControlLeft);
+                InputManager.MoveMouseTo(this.ChildrenOfType<PathControlPointPiece<Slider>>().ElementAt(2));
+                InputManager.Click(MouseButton.Left);
+                InputManager.ReleaseKey(Key.ControlLeft);
+            });
+            AddStep("quick-delete fourth node", () =>
+            {
+                InputManager.MoveMouseTo(this.ChildrenOfType<PathControlPointPiece<Slider>>().ElementAt(3));
+                InputManager.Click(MouseButton.Middle);
+            });
+            AddUntilStep("slider not deleted", () => EditorBeatmap.HitObjects.OfType<Slider>().Count(), () => Is.EqualTo(1));
+            AddUntilStep("slider path has 3 nodes", () => EditorBeatmap.HitObjects.OfType<Slider>().Single().Path.ControlPoints.Count, () => Is.EqualTo(3));
+        }
+
+        [Test]
+        public void TestQuickDeleteOnSelectedControlPointRemovesEntireSelection()
+        {
+            var slider = new Slider
+            {
+                StartTime = 0,
+                Position = new Vector2(100, 100),
+                Path = new SliderPath
+                {
+                    ControlPoints =
+                    {
+                        new PathControlPoint { Type = PathType.LINEAR },
+                        new PathControlPoint(new Vector2(100, 0)),
+                        new PathControlPoint(new Vector2(100)),
+                        new PathControlPoint(new Vector2(0, 100))
+                    }
+                }
+            };
+            AddStep("add slider", () => EditorBeatmap.Add(slider));
+            AddStep("select slider", () => EditorBeatmap.SelectedHitObjects.Add(slider));
+
+            AddStep("select second node", () =>
+            {
+                InputManager.MoveMouseTo(this.ChildrenOfType<PathControlPointPiece<Slider>>().ElementAt(1));
+                InputManager.Click(MouseButton.Left);
+            });
+            AddStep("also select third node", () =>
+            {
+                InputManager.PressKey(Key.ControlLeft);
+                InputManager.MoveMouseTo(this.ChildrenOfType<PathControlPointPiece<Slider>>().ElementAt(2));
+                InputManager.Click(MouseButton.Left);
+                InputManager.ReleaseKey(Key.ControlLeft);
+            });
+            AddStep("quick-delete second node", () =>
+            {
+                InputManager.MoveMouseTo(this.ChildrenOfType<PathControlPointPiece<Slider>>().ElementAt(1));
+                InputManager.Click(MouseButton.Middle);
+            });
+            AddUntilStep("slider not deleted", () => EditorBeatmap.HitObjects.OfType<Slider>().Count(), () => Is.EqualTo(1));
+            AddUntilStep("slider path has 2 nodes", () => EditorBeatmap.HitObjects.OfType<Slider>().Single().Path.ControlPoints.Count, () => Is.EqualTo(2));
+        }
+
         private ComposeBlueprintContainer blueprintContainer
             => Editor.ChildrenOfType<ComposeBlueprintContainer>().First();
 

osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointVisualiser.cs

@@ -137,11 +137,27 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components
         /// <summary>
         /// Delete all visually selected <see cref="PathControlPoint"/>s.
         /// </summary>
-        /// <returns></returns>
+        /// <returns>Whether any change actually took place.</returns>
         public bool DeleteSelected()
         {
             List<PathControlPoint> toRemove = Pieces.Where(p => p.IsSelected.Value).Select(p => p.ControlPoint).ToList();
 
+            if (!Delete(toRemove))
+                return false;
+
+            // Since pieces are re-used, they will not point to the deleted control points while remaining selected
+            foreach (var piece in Pieces)
+                piece.IsSelected.Value = false;
+
+            return true;
+        }
+
+        /// <summary>
+        /// Delete the specified <see cref="PathControlPoint"/>s.
+        /// </summary>
+        /// <returns>Whether any change actually took place.</returns>
+        public bool Delete(List<PathControlPoint> toRemove)
+        {
             // Ensure that there are any points to be deleted
             if (toRemove.Count == 0)
                 return false;
@@ -149,11 +165,6 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components
             changeHandler?.BeginChange();
             RemoveControlPointsRequested?.Invoke(toRemove);
             changeHandler?.EndChange();
-
-            // Since pieces are re-used, they will not point to the deleted control points while remaining selected
-            foreach (var piece in Pieces)
-                piece.IsSelected.Value = false;
-
             return true;
         }
 

osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/SliderSelectionBlueprint.cs

@@ -140,8 +140,11 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders
             if (hoveredControlPoint == null)
                 return false;
 
-            hoveredControlPoint.IsSelected.Value = true;
+            if (hoveredControlPoint.IsSelected.Value)
                 ControlPointVisualiser?.DeleteSelected();
+            else
+                ControlPointVisualiser?.Delete([hoveredControlPoint.ControlPoint]);
+
             return true;
         }