Merge pull request #22804 from peppy/skin-editor-cyclic-selection

Add support for cyclic selection in skin editor

osu.Game.Tests/Visual/Gameplay/TestSceneSkinEditor.cs

@@ -17,6 +17,8 @@ using osu.Game.Rulesets.Osu;
 using osu.Game.Screens.Edit;
 using osu.Game.Screens.Play.HUD.HitErrorMeters;
 using osu.Game.Skinning;
+using osu.Game.Skinning.Components;
+using osuTK;
 using osuTK.Input;
 
 namespace osu.Game.Tests.Visual.Gameplay
@@ -52,6 +54,134 @@ namespace osu.Game.Tests.Visual.Gameplay
             AddUntilStep("wait for loaded", () => skinEditor.IsLoaded);
         }
 
+        [Test]
+        public void TestDragSelection()
+        {
+            BigBlackBox box1 = null!;
+            BigBlackBox box2 = null!;
+            BigBlackBox box3 = null!;
+
+            AddStep("Add big black boxes", () =>
+            {
+                var target = Player.ChildrenOfType<SkinComponentsContainer>().First();
+                target.Add(box1 = new BigBlackBox
+                {
+                    Position = new Vector2(-90),
+                    Anchor = Anchor.Centre,
+                    Origin = Anchor.Centre,
+                });
+                target.Add(box2 = new BigBlackBox
+                {
+                    Anchor = Anchor.Centre,
+                    Origin = Anchor.Centre,
+                });
+                target.Add(box3 = new BigBlackBox
+                {
+                    Position = new Vector2(90),
+                    Anchor = Anchor.Centre,
+                    Origin = Anchor.Centre,
+                });
+            });
+
+            // This step is specifically added to reproduce an edge case which was found during cyclic selection development.
+            // If everything is working as expected it should not affect the subsequent drag selections.
+            AddRepeatStep("Select top left", () =>
+            {
+                InputManager.MoveMouseTo(box1.ScreenSpaceDrawQuad.TopLeft + new Vector2(box1.ScreenSpaceDrawQuad.Width / 8));
+                InputManager.Click(MouseButton.Left);
+            }, 2);
+
+            AddStep("Begin drag top left", () =>
+            {
+                InputManager.MoveMouseTo(box1.ScreenSpaceDrawQuad.TopLeft - new Vector2(box1.ScreenSpaceDrawQuad.Width / 4));
+                InputManager.PressButton(MouseButton.Left);
+            });
+
+            AddStep("Drag to bottom right", () =>
+            {
+                InputManager.MoveMouseTo(box3.ScreenSpaceDrawQuad.TopRight + new Vector2(-box3.ScreenSpaceDrawQuad.Width / 8, box3.ScreenSpaceDrawQuad.Height / 4));
+            });
+
+            AddStep("Release button", () =>
+            {
+                InputManager.ReleaseButton(MouseButton.Left);
+            });
+
+            AddAssert("First two boxes selected", () => skinEditor.SelectedComponents, () => Is.EqualTo(new[] { box1, box2 }));
+
+            AddStep("Begin drag bottom right", () =>
+            {
+                InputManager.MoveMouseTo(box3.ScreenSpaceDrawQuad.BottomRight + new Vector2(box3.ScreenSpaceDrawQuad.Width / 4));
+                InputManager.PressButton(MouseButton.Left);
+            });
+
+            AddStep("Drag to top left", () =>
+            {
+                InputManager.MoveMouseTo(box2.ScreenSpaceDrawQuad.Centre - new Vector2(box2.ScreenSpaceDrawQuad.Width / 4));
+            });
+
+            AddStep("Release button", () =>
+            {
+                InputManager.ReleaseButton(MouseButton.Left);
+            });
+
+            AddAssert("Last two boxes selected", () => skinEditor.SelectedComponents, () => Is.EqualTo(new[] { box2, box3 }));
+
+            // Test cyclic selection doesn't trigger in this state.
+            AddStep("click on black box stack", () => InputManager.Click(MouseButton.Left));
+            AddAssert("Last two boxes still selected", () => skinEditor.SelectedComponents, () => Is.EqualTo(new[] { box2, box3 }));
+        }
+
+        [Test]
+        public void TestCyclicSelection()
+        {
+            SkinBlueprint[] blueprints = null!;
+
+            AddStep("Add big black boxes", () =>
+            {
+                InputManager.MoveMouseTo(skinEditor.ChildrenOfType<BigBlackBox>().First());
+                InputManager.Click(MouseButton.Left);
+                InputManager.Click(MouseButton.Left);
+                InputManager.Click(MouseButton.Left);
+            });
+
+            AddAssert("Three black boxes added", () => targetContainer.Components.OfType<BigBlackBox>().Count(), () => Is.EqualTo(3));
+
+            AddStep("Store black box blueprints", () =>
+            {
+                blueprints = skinEditor.ChildrenOfType<SkinBlueprint>().Where(b => b.Item is BigBlackBox).ToArray();
+            });
+
+            AddAssert("Selection is black box 1", () => skinEditor.SelectedComponents.Single(), () => Is.EqualTo(blueprints[0].Item));
+
+            AddStep("move cursor to black box", () =>
+            {
+                // Slightly offset from centre to avoid random failures (see https://github.com/ppy/osu-framework/issues/5669).
+                InputManager.MoveMouseTo(((Drawable)blueprints[0].Item).ScreenSpaceDrawQuad.Centre + new Vector2(1));
+            });
+
+            AddStep("click on black box stack", () => InputManager.Click(MouseButton.Left));
+            AddAssert("Selection is black box 2", () => skinEditor.SelectedComponents.Single(), () => Is.EqualTo(blueprints[1].Item));
+
+            AddStep("click on black box stack", () => InputManager.Click(MouseButton.Left));
+            AddAssert("Selection is black box 3", () => skinEditor.SelectedComponents.Single(), () => Is.EqualTo(blueprints[2].Item));
+
+            AddStep("click on black box stack", () => InputManager.Click(MouseButton.Left));
+            AddAssert("Selection is black box 1", () => skinEditor.SelectedComponents.Single(), () => Is.EqualTo(blueprints[0].Item));
+
+            AddStep("select all boxes", () =>
+            {
+                skinEditor.SelectedComponents.Clear();
+                skinEditor.SelectedComponents.AddRange(targetContainer.Components.OfType<BigBlackBox>().Skip(1));
+            });
+
+            AddAssert("all boxes selected", () => skinEditor.SelectedComponents, () => Has.Count.EqualTo(2));
+            AddStep("click on black box stack", () => InputManager.Click(MouseButton.Left));
+            AddStep("click on black box stack", () => InputManager.Click(MouseButton.Left));
+            AddStep("click on black box stack", () => InputManager.Click(MouseButton.Left));
+            AddAssert("all boxes still selected", () => skinEditor.SelectedComponents, () => Has.Count.EqualTo(2));
+        }
+
         [TestCase(false)]
         [TestCase(true)]
         public void TestBringToFront(bool alterSelectionOrder)

osu.Game/Overlays/SkinEditor/SkinBlueprintContainer.cs

@@ -25,6 +25,8 @@ namespace osu.Game.Overlays.SkinEditor
         [Resolved]
         private SkinEditor editor { get; set; } = null!;
 
+        protected override bool AllowCyclicSelection => true;
+
         public SkinBlueprintContainer(ISerialisableDrawableContainer targetContainer)
         {
             this.targetContainer = targetContainer;

osu.Game/Screens/Edit/Compose/Components/BlueprintContainer.cs

@@ -45,6 +45,15 @@ namespace osu.Game.Screens.Edit.Compose.Components
 
         protected readonly BindableList<T> SelectedItems = new BindableList<T>();
 
+        /// <summary>
+        /// Whether to allow cyclic selection on clicking multiple times.
+        /// </summary>
+        /// <remarks>
+        /// Disabled by default as it does not work well with editors that support double-clicking or other advanced interactions.
+        /// Can probably be made to work with more thought.
+        /// </remarks>
+        protected virtual bool AllowCyclicSelection => false;
+
         protected BlueprintContainer()
         {
             RelativeSizeAxes = Axes.Both;
@@ -167,8 +176,9 @@ namespace osu.Game.Screens.Edit.Compose.Components
             Schedule(() =>
             {
                 endClickSelection(e);
-                clickSelectionBegan = false;
+                clickSelectionHandled = false;
                 isDraggingBlueprint = false;
+                wasDragStarted = false;
             });
 
             finishSelectionMovement();
@@ -182,6 +192,7 @@ namespace osu.Game.Screens.Edit.Compose.Components
                 return false;
 
             lastDragEvent = e;
+            wasDragStarted = true;
 
             if (movementBlueprints != null)
             {
@@ -339,7 +350,12 @@ namespace osu.Game.Screens.Edit.Compose.Components
         /// <summary>
         /// Whether a blueprint was selected by a previous click event.
         /// </summary>
-        private bool clickSelectionBegan;
+        private bool clickSelectionHandled;
+
+        /// <summary>
+        /// Whether the selected blueprint(s) were already selected on mouse down. Generally used to perform selection cycling on mouse up in such a case.
+        /// </summary>
+        private bool selectedBlueprintAlreadySelectedOnMouseDown;
 
         /// <summary>
         /// Attempts to select any hovered blueprints.
@@ -354,7 +370,8 @@ namespace osu.Game.Screens.Edit.Compose.Components
             {
                 if (!blueprint.IsHovered) continue;
 
-                return clickSelectionBegan = SelectionHandler.MouseDownSelectionRequested(blueprint, e);
+                selectedBlueprintAlreadySelectedOnMouseDown = blueprint.State == SelectionState.Selected;
+                return clickSelectionHandled = SelectionHandler.MouseDownSelectionRequested(blueprint, e);
             }
 
             return false;
@@ -367,25 +384,48 @@ namespace osu.Game.Screens.Edit.Compose.Components
         /// <returns>Whether a click selection was active.</returns>
         private bool endClickSelection(MouseButtonEvent e)
         {
-            if (!clickSelectionBegan && !isDraggingBlueprint)
+            // If already handled a selection or drag, we don't want to perform a mouse up / click action.
+            if (clickSelectionHandled || isDraggingBlueprint) return true;
+
+            if (e.Button != MouseButton.Left) return false;
+
+            if (e.ControlPressed)
             {
                 // if a selection didn't occur, we may want to trigger a deselection.
-                if (e.ControlPressed && e.Button == MouseButton.Left)
-                {
-                    // Iterate from the top of the input stack (blueprints closest to the front of the screen first).
-                    // Priority is given to already-selected blueprints.
-                    foreach (SelectionBlueprint<T> blueprint in SelectionBlueprints.AliveChildren.Reverse().OrderByDescending(b => b.IsSelected))
-                    {
-                        if (!blueprint.IsHovered) continue;
 
-                        return clickSelectionBegan = SelectionHandler.MouseUpSelectionRequested(blueprint, e);
+                // Iterate from the top of the input stack (blueprints closest to the front of the screen first).
-                    }
+                // Priority is given to already-selected blueprints.
-                }
+                foreach (SelectionBlueprint<T> blueprint in SelectionBlueprints.AliveChildren.Where(b => b.IsHovered).OrderByDescending(b => b.IsSelected))
+                    return clickSelectionHandled = SelectionHandler.MouseUpSelectionRequested(blueprint, e);
 
                 return false;
             }
 
-            return true;
+            if (!wasDragStarted && selectedBlueprintAlreadySelectedOnMouseDown && SelectedItems.Count == 1 && AllowCyclicSelection)
+            {
+                // If a click occurred and was handled by the currently selected blueprint but didn't result in a drag,
+                // cycle between other blueprints which are also under the cursor.
+
+                // The depth of blueprints is constantly changing (see above where selected blueprints are brought to the front).
+                // For this logic, we want a stable sort order so we can correctly cycle, thus using the blueprintMap instead.
+                IEnumerable<SelectionBlueprint<T>> cyclingSelectionBlueprints = blueprintMap.Values;
+
+                // If there's already a selection, let's start from the blueprint after the selection.
+                cyclingSelectionBlueprints = cyclingSelectionBlueprints.SkipWhile(b => !b.IsSelected).Skip(1);
+
+                // Add the blueprints from before the selection to the end of the enumerable to allow for cyclic selection.
+                cyclingSelectionBlueprints = cyclingSelectionBlueprints.Concat(blueprintMap.Values.TakeWhile(b => !b.IsSelected));
+
+                foreach (SelectionBlueprint<T> blueprint in cyclingSelectionBlueprints)
+                {
+                    if (!blueprint.IsHovered) continue;
+
+                    // We are performing a mouse up, but selection handlers perform selection on mouse down, so we need to call that instead.
+                    return clickSelectionHandled = SelectionHandler.MouseDownSelectionRequested(blueprint, e);
+                }
+            }
+
+            return false;
         }
 
         /// <summary>
@@ -441,8 +481,17 @@ namespace osu.Game.Screens.Edit.Compose.Components
 
         private Vector2[][] movementBlueprintsOriginalPositions;
         private SelectionBlueprint<T>[] movementBlueprints;
+
+        /// <summary>
+        /// Whether a blueprint is currently being dragged.
+        /// </summary>
         private bool isDraggingBlueprint;
 
+        /// <summary>
+        /// Whether a drag operation was started at all.
+        /// </summary>
+        private bool wasDragStarted;
+
         /// <summary>
         /// Attempts to begin the movement of any selected blueprints.
         /// </summary>
@@ -454,7 +503,7 @@ namespace osu.Game.Screens.Edit.Compose.Components
 
             // Any selected blueprint that is hovered can begin the movement of the group, however only the first item (according to SortForMovement) is used for movement.
             // A special case is added for when a click selection occurred before the drag
-            if (!clickSelectionBegan && !SelectionHandler.SelectedBlueprints.Any(b => b.IsHovered))
+            if (!clickSelectionHandled && !SelectionHandler.SelectedBlueprints.Any(b => b.IsHovered))
                 return false;
 
             // Movement is tracked from the blueprint of the earliest item, since it only makes sense to distance snap from that item