diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneSkinEditor.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneSkinEditor.cs index 9690d00d4c..119b753d70 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneSkinEditor.cs +++ b/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().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().First()); + InputManager.Click(MouseButton.Left); + InputManager.Click(MouseButton.Left); + InputManager.Click(MouseButton.Left); + }); + + AddAssert("Three black boxes added", () => targetContainer.Components.OfType().Count(), () => Is.EqualTo(3)); + + AddStep("Store black box blueprints", () => + { + blueprints = skinEditor.ChildrenOfType().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().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) diff --git a/osu.Game/Overlays/SkinEditor/SkinBlueprintContainer.cs b/osu.Game/Overlays/SkinEditor/SkinBlueprintContainer.cs index 3f8d9f80d4..db27e20010 100644 --- a/osu.Game/Overlays/SkinEditor/SkinBlueprintContainer.cs +++ b/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; diff --git a/osu.Game/Screens/Edit/Compose/Components/BlueprintContainer.cs b/osu.Game/Screens/Edit/Compose/Components/BlueprintContainer.cs index e4e67d10d7..cb7c083d87 100644 --- a/osu.Game/Screens/Edit/Compose/Components/BlueprintContainer.cs +++ b/osu.Game/Screens/Edit/Compose/Components/BlueprintContainer.cs @@ -45,6 +45,15 @@ namespace osu.Game.Screens.Edit.Compose.Components protected readonly BindableList SelectedItems = new BindableList(); + /// + /// Whether to allow cyclic selection on clicking multiple times. + /// + /// + /// 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. + /// + 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 /// /// Whether a blueprint was selected by a previous click event. /// - private bool clickSelectionBegan; + private bool clickSelectionHandled; + + /// + /// Whether the selected blueprint(s) were already selected on mouse down. Generally used to perform selection cycling on mouse up in such a case. + /// + private bool selectedBlueprintAlreadySelectedOnMouseDown; /// /// 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 /// Whether a click selection was active. 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 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 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> 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 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; } /// @@ -441,8 +481,17 @@ namespace osu.Game.Screens.Edit.Compose.Components private Vector2[][] movementBlueprintsOriginalPositions; private SelectionBlueprint[] movementBlueprints; + + /// + /// Whether a blueprint is currently being dragged. + /// private bool isDraggingBlueprint; + /// + /// Whether a drag operation was started at all. + /// + private bool wasDragStarted; + /// /// Attempts to begin the movement of any selected blueprints. /// @@ -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