From b0ca82e1e530c73a31324586da2d42bd505db547 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Mon, 20 Dec 2021 20:56:06 +0100 Subject: [PATCH 01/22] Move slider path point drag handling to visualiser --- .../Components/PathControlPointPiece.cs | 60 +++--------------- .../Components/PathControlPointVisualiser.cs | 62 +++++++++++++++++++ 2 files changed, 70 insertions(+), 52 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointPiece.cs b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointPiece.cs index b3e0566a98..dd1a2d69fb 100644 --- a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointPiece.cs +++ b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointPiece.cs @@ -16,11 +16,9 @@ using osu.Framework.Input.Events; using osu.Framework.Localisation; using osu.Framework.Utils; using osu.Game.Graphics; -using osu.Game.Rulesets.Edit; using osu.Game.Rulesets.Objects; using osu.Game.Rulesets.Objects.Types; using osu.Game.Rulesets.Osu.Objects; -using osu.Game.Screens.Edit; using osuTK; using osuTK.Graphics; using osuTK.Input; @@ -33,6 +31,11 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components public class PathControlPointPiece : BlueprintPiece, IHasTooltip { public Action RequestSelection; + + public Action DragStarted; + public Action DragInProgress; + public Action DragEnded; + public List PointsInSegment; public readonly BindableBool IsSelected = new BindableBool(); @@ -42,12 +45,6 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components private readonly Container marker; private readonly Drawable markerRing; - [Resolved(CanBeNull = true)] - private IEditorChangeHandler changeHandler { get; set; } - - [Resolved(CanBeNull = true)] - private IPositionSnapProvider snapProvider { get; set; } - [Resolved] private OsuColour colours { get; set; } @@ -160,9 +157,6 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components protected override bool OnClick(ClickEvent e) => RequestSelection != null; - private Vector2 dragStartPosition; - private PathType? dragPathType; - protected override bool OnDragStart(DragStartEvent e) { if (RequestSelection == null) @@ -170,54 +164,16 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components if (e.Button == MouseButton.Left) { - dragStartPosition = ControlPoint.Position; - dragPathType = PointsInSegment[0].Type; - - changeHandler?.BeginChange(); + DragStarted?.Invoke(ControlPoint); return true; } return false; } - protected override void OnDrag(DragEvent e) - { - Vector2[] oldControlPoints = slider.Path.ControlPoints.Select(cp => cp.Position).ToArray(); - var oldPosition = slider.Position; - double oldStartTime = slider.StartTime; + protected override void OnDrag(DragEvent e) => DragInProgress?.Invoke(ControlPoint, e); - if (ControlPoint == slider.Path.ControlPoints[0]) - { - // Special handling for the head control point - the position of the slider changes which means the snapped position and time have to be taken into account - var result = snapProvider?.SnapScreenSpacePositionToValidTime(e.ScreenSpaceMousePosition); - - Vector2 movementDelta = Parent.ToLocalSpace(result?.ScreenSpacePosition ?? e.ScreenSpaceMousePosition) - slider.Position; - - slider.Position += movementDelta; - slider.StartTime = result?.Time ?? slider.StartTime; - - // Since control points are relative to the position of the slider, they all need to be offset backwards by the delta - for (int i = 1; i < slider.Path.ControlPoints.Count; i++) - slider.Path.ControlPoints[i].Position -= movementDelta; - } - else - ControlPoint.Position = dragStartPosition + (e.MousePosition - e.MouseDownPosition); - - if (!slider.Path.HasValidLength) - { - for (int i = 0; i < slider.Path.ControlPoints.Count; i++) - slider.Path.ControlPoints[i].Position = oldControlPoints[i]; - - slider.Position = oldPosition; - slider.StartTime = oldStartTime; - return; - } - - // Maintain the path type in case it got defaulted to bezier at some point during the drag. - PointsInSegment[0].Type = dragPathType; - } - - protected override void OnDragEnd(DragEndEvent e) => changeHandler?.EndChange(); + protected override void OnDragEnd(DragEndEvent e) => DragEnded?.Invoke(); private void cachePoints(Slider slider) => PointsInSegment = slider.Path.PointsInSegment(ControlPoint); diff --git a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointVisualiser.cs b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointVisualiser.cs index 1be9b5bf2e..017bc395f0 100644 --- a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointVisualiser.cs +++ b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointVisualiser.cs @@ -16,6 +16,7 @@ using osu.Framework.Input; using osu.Framework.Input.Bindings; using osu.Framework.Input.Events; using osu.Game.Graphics.UserInterface; +using osu.Game.Rulesets.Edit; using osu.Game.Rulesets.Objects; using osu.Game.Rulesets.Objects.Types; using osu.Game.Rulesets.Osu.Objects; @@ -40,6 +41,9 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components public Action> RemoveControlPointsRequested; + [Resolved(CanBeNull = true)] + private IPositionSnapProvider snapProvider { get; set; } + public PathControlPointVisualiser(Slider slider, bool allowSelection) { this.slider = slider; @@ -88,6 +92,10 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components { if (allowSelection) d.RequestSelection = selectPiece; + + d.DragStarted = dragStarted; + d.DragInProgress = dragInProgress; + d.DragEnded = dragEnded; })); Connections.Add(new PathControlPointConnectionPiece(slider, e.NewStartingIndex + i)); @@ -203,6 +211,60 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components return true; } + #region Drag handling + + private Vector2 dragStartPosition; + private PathType? dragPathType; + + private void dragStarted(PathControlPoint controlPoint) + { + dragStartPosition = controlPoint.Position; + dragPathType = slider.Path.PointsInSegment(controlPoint).First().Type; + + changeHandler?.BeginChange(); + } + + private void dragInProgress(PathControlPoint controlPoint, DragEvent e) + { + Vector2[] oldControlPoints = slider.Path.ControlPoints.Select(cp => cp.Position).ToArray(); + var oldPosition = slider.Position; + double oldStartTime = slider.StartTime; + + if (controlPoint == slider.Path.ControlPoints[0]) + { + // Special handling for the head control point - the position of the slider changes which means the snapped position and time have to be taken into account + var result = snapProvider?.SnapScreenSpacePositionToValidTime(e.ScreenSpaceMousePosition); + + Vector2 movementDelta = Parent.ToLocalSpace(result?.ScreenSpacePosition ?? e.ScreenSpaceMousePosition) - slider.Position; + + slider.Position += movementDelta; + slider.StartTime = result?.Time ?? slider.StartTime; + + // Since control points are relative to the position of the slider, they all need to be offset backwards by the delta + for (int i = 1; i < slider.Path.ControlPoints.Count; i++) + slider.Path.ControlPoints[i].Position -= movementDelta; + } + else + controlPoint.Position = dragStartPosition + (e.MousePosition - e.MouseDownPosition); + + if (!slider.Path.HasValidLength) + { + for (int i = 0; i < slider.Path.ControlPoints.Count; i++) + slider.Path.ControlPoints[i].Position = oldControlPoints[i]; + + slider.Position = oldPosition; + slider.StartTime = oldStartTime; + return; + } + + // Maintain the path type in case it got defaulted to bezier at some point during the drag. + slider.Path.PointsInSegment(controlPoint).First().Type = dragPathType; + } + + private void dragEnded() => changeHandler?.EndChange(); + + #endregion + public MenuItem[] ContextMenuItems { get From fbba8293c76965b49a1db3cc4e6853cd1df7be89 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Mon, 20 Dec 2021 21:01:11 +0100 Subject: [PATCH 02/22] Add failing test for expected multiple path drag UX --- .../TestSceneSliderControlPointPiece.cs | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderControlPointPiece.cs b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderControlPointPiece.cs index 6bfe7f892b..282e7a479e 100644 --- a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderControlPointPiece.cs +++ b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderControlPointPiece.cs @@ -60,6 +60,34 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor assertControlPointType(0, PathType.PerfectCurve); } + [Test] + public void TestDragMultipleControlPoints() + { + moveMouseToControlPoint(2); + AddStep("click", () => InputManager.Click(MouseButton.Left)); + + AddStep("hold control", () => InputManager.PressKey(Key.LControl)); + + moveMouseToControlPoint(3); + AddStep("click", () => InputManager.Click(MouseButton.Left)); + + moveMouseToControlPoint(4); + AddStep("click", () => InputManager.Click(MouseButton.Left)); + + moveMouseToControlPoint(2); + addMovementStep(new Vector2(450, 50)); + AddStep("release", () => InputManager.ReleaseButton(MouseButton.Left)); + + assertControlPointPosition(2, new Vector2(450, 50)); + assertControlPointType(2, PathType.PerfectCurve); + + assertControlPointPosition(3, new Vector2(550, 50)); + + assertControlPointPosition(4, new Vector2(550, 200)); + + AddStep("release control", () => InputManager.ReleaseKey(Key.LControl)); + } + [Test] public void TestDragControlPointAlmostLinearlyExterior() { From a9408485cc9fc00f5d85d7a835fb260834ea1426 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Mon, 20 Dec 2021 21:18:38 +0100 Subject: [PATCH 03/22] Change control point piece selection logic to allow dragging multiple --- .../Components/PathControlPointPiece.cs | 23 ++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointPiece.cs b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointPiece.cs index dd1a2d69fb..d219fb0b2d 100644 --- a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointPiece.cs +++ b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointPiece.cs @@ -135,6 +135,10 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components updateMarkerDisplay(); } + // Used to pair up mouse down events which caused this piece to be selected + // with their corresponding mouse up events. + private bool selectionPerformed; + protected override bool OnMouseDown(MouseDownEvent e) { if (RequestSelection == null) @@ -143,7 +147,12 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components switch (e.Button) { case MouseButton.Left: - RequestSelection.Invoke(this, e); + if (!IsSelected.Value) + { + RequestSelection.Invoke(this, e); + selectionPerformed = true; + } + return true; case MouseButton.Right: @@ -155,6 +164,18 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components return false; } + protected override void OnMouseUp(MouseUpEvent e) + { + base.OnMouseUp(e); + + // ctrl+click deselects this piece, but only if this event + // wasn't immediately preceded by a matching mouse down. + if (IsSelected.Value && e.ControlPressed && !selectionPerformed) + IsSelected.Value = false; + + selectionPerformed = false; + } + protected override bool OnClick(ClickEvent e) => RequestSelection != null; protected override bool OnDragStart(DragStartEvent e) From d2417beeacc82da7802ce0b2499114a4c8d615a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Mon, 20 Dec 2021 21:29:57 +0100 Subject: [PATCH 04/22] Implement drag operation for multiple path control points --- .../TestSceneSliderControlPointPiece.cs | 9 ++-- .../Components/PathControlPointPiece.cs | 8 ++-- .../Components/PathControlPointVisualiser.cs | 43 +++++++++++++------ 3 files changed, 39 insertions(+), 21 deletions(-) diff --git a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderControlPointPiece.cs b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderControlPointPiece.cs index 282e7a479e..91c75c376d 100644 --- a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderControlPointPiece.cs +++ b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderControlPointPiece.cs @@ -64,19 +64,20 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor public void TestDragMultipleControlPoints() { moveMouseToControlPoint(2); - AddStep("click", () => InputManager.Click(MouseButton.Left)); + AddStep("click left mouse", () => InputManager.Click(MouseButton.Left)); AddStep("hold control", () => InputManager.PressKey(Key.LControl)); moveMouseToControlPoint(3); - AddStep("click", () => InputManager.Click(MouseButton.Left)); + AddStep("click left mouse", () => InputManager.Click(MouseButton.Left)); moveMouseToControlPoint(4); - AddStep("click", () => InputManager.Click(MouseButton.Left)); + AddStep("click left mouse", () => InputManager.Click(MouseButton.Left)); moveMouseToControlPoint(2); + AddStep("hold left mouse", () => InputManager.PressButton(MouseButton.Left)); addMovementStep(new Vector2(450, 50)); - AddStep("release", () => InputManager.ReleaseButton(MouseButton.Left)); + AddStep("release left mouse", () => InputManager.ReleaseButton(MouseButton.Left)); assertControlPointPosition(2, new Vector2(450, 50)); assertControlPointType(2, PathType.PerfectCurve); diff --git a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointPiece.cs b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointPiece.cs index d219fb0b2d..da9b8ec9e4 100644 --- a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointPiece.cs +++ b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointPiece.cs @@ -32,8 +32,8 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components { public Action RequestSelection; - public Action DragStarted; - public Action DragInProgress; + public Action DragStarted; + public Action DragInProgress; public Action DragEnded; public List PointsInSegment; @@ -185,14 +185,14 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components if (e.Button == MouseButton.Left) { - DragStarted?.Invoke(ControlPoint); + DragStarted?.Invoke(); return true; } return false; } - protected override void OnDrag(DragEvent e) => DragInProgress?.Invoke(ControlPoint, e); + protected override void OnDrag(DragEvent e) => DragInProgress?.Invoke(e); protected override void OnDragEnd(DragEndEvent e) => DragEnded?.Invoke(); diff --git a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointVisualiser.cs b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointVisualiser.cs index 017bc395f0..97775d8122 100644 --- a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointVisualiser.cs +++ b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointVisualiser.cs @@ -213,26 +213,28 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components #region Drag handling - private Vector2 dragStartPosition; - private PathType? dragPathType; + private Vector2[] dragStartPositions; + private PathType?[] dragPathTypes; + private HashSet draggedControlPoints; - private void dragStarted(PathControlPoint controlPoint) + private void dragStarted() { - dragStartPosition = controlPoint.Position; - dragPathType = slider.Path.PointsInSegment(controlPoint).First().Type; + dragStartPositions = slider.Path.ControlPoints.Select(point => point.Position).ToArray(); + dragPathTypes = slider.Path.ControlPoints.Select(point => point.Type).ToArray(); + draggedControlPoints = new HashSet(Pieces.Where(piece => piece.IsSelected.Value).Select(piece => piece.ControlPoint)); changeHandler?.BeginChange(); } - private void dragInProgress(PathControlPoint controlPoint, DragEvent e) + private void dragInProgress(DragEvent e) { Vector2[] oldControlPoints = slider.Path.ControlPoints.Select(cp => cp.Position).ToArray(); var oldPosition = slider.Position; double oldStartTime = slider.StartTime; - if (controlPoint == slider.Path.ControlPoints[0]) + if (draggedControlPoints.Contains(slider.Path.ControlPoints[0])) { - // Special handling for the head control point - the position of the slider changes which means the snapped position and time have to be taken into account + // Special handling for selections containing head control point - the position of the slider changes which means the snapped position and time have to be taken into account var result = snapProvider?.SnapScreenSpacePositionToValidTime(e.ScreenSpaceMousePosition); Vector2 movementDelta = Parent.ToLocalSpace(result?.ScreenSpacePosition ?? e.ScreenSpaceMousePosition) - slider.Position; @@ -240,12 +242,26 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components slider.Position += movementDelta; slider.StartTime = result?.Time ?? slider.StartTime; - // Since control points are relative to the position of the slider, they all need to be offset backwards by the delta for (int i = 1; i < slider.Path.ControlPoints.Count; i++) - slider.Path.ControlPoints[i].Position -= movementDelta; + { + var controlPoint = slider.Path.ControlPoints[i]; + // Since control points are relative to the position of the slider, all points that are _not_ selected + // need to be offset _back_ by the delta corresponding to the movement of the head point. + // All other selected control points (if any) will move together with the head point + // (and so they will not move at all, relative to each other). + if (!draggedControlPoints.Contains(controlPoint)) + controlPoint.Position -= movementDelta; + } } else - controlPoint.Position = dragStartPosition + (e.MousePosition - e.MouseDownPosition); + { + for (int i = 0; i < controlPoints.Count; ++i) + { + var controlPoint = controlPoints[i]; + if (draggedControlPoints.Contains(controlPoint)) + controlPoint.Position = dragStartPositions[i] + (e.MousePosition - e.MouseDownPosition); + } + } if (!slider.Path.HasValidLength) { @@ -257,8 +273,9 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components return; } - // Maintain the path type in case it got defaulted to bezier at some point during the drag. - slider.Path.PointsInSegment(controlPoint).First().Type = dragPathType; + // Maintain the path types in case they got defaulted to bezier at some point during the drag. + for (int i = 0; i < slider.Path.ControlPoints.Count; i++) + slider.Path.ControlPoints[i].Type = dragPathTypes[i]; } private void dragEnded() => changeHandler?.EndChange(); From cdb587d9562aa484707dfad642559b0eeb55c7b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Mon, 20 Dec 2021 21:51:56 +0100 Subject: [PATCH 05/22] Add more test steps for input handling edge cases --- .../Editor/TestSceneSliderControlPointPiece.cs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderControlPointPiece.cs b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderControlPointPiece.cs index 91c75c376d..7afe9d91e0 100644 --- a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderControlPointPiece.cs +++ b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderControlPointPiece.cs @@ -1,7 +1,9 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. +using System.Linq; using NUnit.Framework; +using osu.Framework.Testing; using osu.Framework.Utils; using osu.Game.Beatmaps; using osu.Game.Beatmaps.ControlPoints; @@ -76,9 +78,14 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor moveMouseToControlPoint(2); AddStep("hold left mouse", () => InputManager.PressButton(MouseButton.Left)); + + AddAssert("three control point pieces selected", () => this.ChildrenOfType().Count(piece => piece.IsSelected.Value) == 3); + addMovementStep(new Vector2(450, 50)); AddStep("release left mouse", () => InputManager.ReleaseButton(MouseButton.Left)); + AddAssert("three control point pieces selected", () => this.ChildrenOfType().Count(piece => piece.IsSelected.Value) == 3); + assertControlPointPosition(2, new Vector2(450, 50)); assertControlPointType(2, PathType.PerfectCurve); @@ -87,6 +94,10 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor assertControlPointPosition(4, new Vector2(550, 200)); AddStep("release control", () => InputManager.ReleaseKey(Key.LControl)); + + moveMouseToControlPoint(3); + AddStep("click left mouse", () => InputManager.Click(MouseButton.Left)); + AddAssert("only one control point piece selected", () => this.ChildrenOfType().Count(piece => piece.IsSelected.Value) == 1); } [Test] From bf8c87e9b7295e0d47e8166150867d6232c1df25 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Mon, 20 Dec 2021 21:53:51 +0100 Subject: [PATCH 06/22] Fix releasing mouse after drag deselecting dragged point --- .../Sliders/Components/PathControlPointPiece.cs | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointPiece.cs b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointPiece.cs index da9b8ec9e4..57a65a84e3 100644 --- a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointPiece.cs +++ b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointPiece.cs @@ -135,9 +135,9 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components updateMarkerDisplay(); } - // Used to pair up mouse down events which caused this piece to be selected - // with their corresponding mouse up events. - private bool selectionPerformed; + // Used to pair up mouse down/drag events with their corresponding mouse up events, + // to avoid deselecting the piece by accident when the mouse up corresponding to the mouse down/drag fires. + private bool keepSelection; protected override bool OnMouseDown(MouseDownEvent e) { @@ -150,7 +150,7 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components if (!IsSelected.Value) { RequestSelection.Invoke(this, e); - selectionPerformed = true; + keepSelection = true; } return true; @@ -169,11 +169,11 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components base.OnMouseUp(e); // ctrl+click deselects this piece, but only if this event - // wasn't immediately preceded by a matching mouse down. - if (IsSelected.Value && e.ControlPressed && !selectionPerformed) + // wasn't immediately preceded by a matching mouse down or drag. + if (IsSelected.Value && e.ControlPressed && !keepSelection) IsSelected.Value = false; - selectionPerformed = false; + keepSelection = false; } protected override bool OnClick(ClickEvent e) => RequestSelection != null; @@ -186,6 +186,7 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components if (e.Button == MouseButton.Left) { DragStarted?.Invoke(); + keepSelection = true; return true; } From 5ef4e2333573408c5fc73475d289e6bae5b2e94f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Mon, 20 Dec 2021 21:56:06 +0100 Subject: [PATCH 07/22] Fix selecting control points without control not deselecting other selected points --- .../Sliders/Components/PathControlPointPiece.cs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointPiece.cs b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointPiece.cs index 57a65a84e3..7b9b6cb976 100644 --- a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointPiece.cs +++ b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointPiece.cs @@ -147,11 +147,14 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components switch (e.Button) { case MouseButton.Left: - if (!IsSelected.Value) - { - RequestSelection.Invoke(this, e); - keepSelection = true; - } + // if control is pressed, do not do anything as the user may be adding to current selection + // or dragging all currently selected control points. + // if it isn't and the user's intent is to deselect, deselection will happen on mouse up. + if (e.ControlPressed && IsSelected.Value) + return true; + + RequestSelection.Invoke(this, e); + keepSelection = true; return true; From b311308ada7bd524a0557323eaa8a4f3c197d6b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Mon, 20 Dec 2021 22:09:01 +0100 Subject: [PATCH 08/22] Add more comprehensive test of multiple selection --- .../TestSceneSliderControlPointPiece.cs | 45 +++++++++++++++++-- 1 file changed, 41 insertions(+), 4 deletions(-) diff --git a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderControlPointPiece.cs b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderControlPointPiece.cs index 7afe9d91e0..7ae66b7ba7 100644 --- a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderControlPointPiece.cs +++ b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderControlPointPiece.cs @@ -2,6 +2,7 @@ // See the LICENCE file in the repository root for full licence text. using System.Linq; +using Humanizer; using NUnit.Framework; using osu.Framework.Testing; using osu.Framework.Utils; @@ -49,6 +50,46 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor AddBlueprint(new TestSliderBlueprint(slider), drawableObject); }); + [Test] + public void TestSelection() + { + moveMouseToControlPoint(0); + AddStep("click left mouse", () => InputManager.Click(MouseButton.Left)); + assertSelectionCount(1); + assertSelected(0); + + moveMouseToControlPoint(3); + AddStep("click left mouse", () => InputManager.Click(MouseButton.Left)); + assertSelectionCount(1); + assertSelected(3); + + AddStep("press control", () => InputManager.PressKey(Key.ControlLeft)); + moveMouseToControlPoint(2); + AddStep("click left mouse", () => InputManager.Click(MouseButton.Left)); + assertSelectionCount(2); + assertSelected(2); + assertSelected(3); + + moveMouseToControlPoint(0); + AddStep("click left mouse", () => InputManager.Click(MouseButton.Left)); + assertSelectionCount(3); + assertSelected(0); + assertSelected(2); + assertSelected(3); + + AddStep("release control", () => InputManager.ReleaseKey(Key.ControlLeft)); + AddStep("click left mouse", () => InputManager.Click(MouseButton.Left)); + assertSelectionCount(1); + assertSelected(0); + + void assertSelectionCount(int count) => + AddAssert($"{count} control point pieces selected", () => this.ChildrenOfType().Count(piece => piece.IsSelected.Value) == count); + + void assertSelected(int index) => + AddAssert($"{(index + 1).ToOrdinalWords()} control point piece selected", + () => this.ChildrenOfType().Single(piece => piece.ControlPoint == slider.Path.ControlPoints[index]).IsSelected.Value); + } + [Test] public void TestDragControlPoint() { @@ -94,10 +135,6 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor assertControlPointPosition(4, new Vector2(550, 200)); AddStep("release control", () => InputManager.ReleaseKey(Key.LControl)); - - moveMouseToControlPoint(3); - AddStep("click left mouse", () => InputManager.Click(MouseButton.Left)); - AddAssert("only one control point piece selected", () => this.ChildrenOfType().Count(piece => piece.IsSelected.Value) == 1); } [Test] From adfadc13f7af4ca14d0b252ed05000d39c7330e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Tue, 21 Dec 2021 12:34:55 +0100 Subject: [PATCH 09/22] Add test case for dragging selection including slider head --- .../TestSceneSliderControlPointPiece.cs | 43 +++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderControlPointPiece.cs b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderControlPointPiece.cs index 7ae66b7ba7..a0bdcb144f 100644 --- a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderControlPointPiece.cs +++ b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderControlPointPiece.cs @@ -137,6 +137,49 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor AddStep("release control", () => InputManager.ReleaseKey(Key.LControl)); } + [Test] + public void TestDragMultipleControlPointsIncludingHead() + { + moveMouseToControlPoint(0); + AddStep("click left mouse", () => InputManager.Click(MouseButton.Left)); + + AddStep("hold control", () => InputManager.PressKey(Key.LControl)); + + moveMouseToControlPoint(3); + AddStep("click left mouse", () => InputManager.Click(MouseButton.Left)); + + moveMouseToControlPoint(4); + AddStep("click left mouse", () => InputManager.Click(MouseButton.Left)); + + moveMouseToControlPoint(3); + AddStep("hold left mouse", () => InputManager.PressButton(MouseButton.Left)); + + AddAssert("three control point pieces selected", () => this.ChildrenOfType().Count(piece => piece.IsSelected.Value) == 3); + + addMovementStep(new Vector2(550, 50)); + AddStep("release left mouse", () => InputManager.ReleaseButton(MouseButton.Left)); + + AddAssert("three control point pieces selected", () => this.ChildrenOfType().Count(piece => piece.IsSelected.Value) == 3); + + // note: if the head is part of the selection being moved, the entire slider is moved. + // the unselected nodes will therefore change position relative to the slider head. + + AddAssert("slider moved", () => Precision.AlmostEquals(slider.Position, new Vector2(256, 192) + new Vector2(150, 50))); + + assertControlPointPosition(0, Vector2.Zero); + assertControlPointType(0, PathType.PerfectCurve); + + assertControlPointPosition(1, new Vector2(0, 100)); + + assertControlPointPosition(2, new Vector2(150, -50)); + + assertControlPointPosition(3, new Vector2(400, 0)); + + assertControlPointPosition(4, new Vector2(400, 150)); + + AddStep("release control", () => InputManager.ReleaseKey(Key.LControl)); + } + [Test] public void TestDragControlPointAlmostLinearlyExterior() { From e715bff535a3d2431db3a39adff28d33b6793f2f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Tue, 21 Dec 2021 12:35:48 +0100 Subject: [PATCH 10/22] Fix selections including head not correctly dragging if not started from head --- .../Components/PathControlPointPiece.cs | 4 ++-- .../Components/PathControlPointVisualiser.cs | 22 ++++++++++++------- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointPiece.cs b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointPiece.cs index 7b9b6cb976..3db3e85c11 100644 --- a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointPiece.cs +++ b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointPiece.cs @@ -32,7 +32,7 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components { public Action RequestSelection; - public Action DragStarted; + public Action DragStarted; public Action DragInProgress; public Action DragEnded; @@ -188,7 +188,7 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components if (e.Button == MouseButton.Left) { - DragStarted?.Invoke(); + DragStarted?.Invoke(ControlPoint); keepSelection = true; return true; } diff --git a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointVisualiser.cs b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointVisualiser.cs index 97775d8122..e06fc1510c 100644 --- a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointVisualiser.cs +++ b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointVisualiser.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.Collections.Specialized; +using System.Diagnostics; using System.Linq; using Humanizer; using osu.Framework.Allocation; @@ -215,13 +216,17 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components private Vector2[] dragStartPositions; private PathType?[] dragPathTypes; - private HashSet draggedControlPoints; + private int draggedControlPointIndex; + private HashSet selectedControlPoints; - private void dragStarted() + private void dragStarted(PathControlPoint controlPoint) { dragStartPositions = slider.Path.ControlPoints.Select(point => point.Position).ToArray(); dragPathTypes = slider.Path.ControlPoints.Select(point => point.Type).ToArray(); - draggedControlPoints = new HashSet(Pieces.Where(piece => piece.IsSelected.Value).Select(piece => piece.ControlPoint)); + draggedControlPointIndex = slider.Path.ControlPoints.IndexOf(controlPoint); + selectedControlPoints = new HashSet(Pieces.Where(piece => piece.IsSelected.Value).Select(piece => piece.ControlPoint)); + + Debug.Assert(draggedControlPointIndex >= 0); changeHandler?.BeginChange(); } @@ -232,12 +237,13 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components var oldPosition = slider.Position; double oldStartTime = slider.StartTime; - if (draggedControlPoints.Contains(slider.Path.ControlPoints[0])) + if (selectedControlPoints.Contains(slider.Path.ControlPoints[0])) { // Special handling for selections containing head control point - the position of the slider changes which means the snapped position and time have to be taken into account - var result = snapProvider?.SnapScreenSpacePositionToValidTime(e.ScreenSpaceMousePosition); + Vector2 newHeadPosition = Parent.ToScreenSpace(e.MousePosition + (dragStartPositions[0] - dragStartPositions[draggedControlPointIndex])); + var result = snapProvider?.SnapScreenSpacePositionToValidTime(newHeadPosition); - Vector2 movementDelta = Parent.ToLocalSpace(result?.ScreenSpacePosition ?? e.ScreenSpaceMousePosition) - slider.Position; + Vector2 movementDelta = Parent.ToLocalSpace(result?.ScreenSpacePosition ?? newHeadPosition) - slider.Position; slider.Position += movementDelta; slider.StartTime = result?.Time ?? slider.StartTime; @@ -249,7 +255,7 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components // need to be offset _back_ by the delta corresponding to the movement of the head point. // All other selected control points (if any) will move together with the head point // (and so they will not move at all, relative to each other). - if (!draggedControlPoints.Contains(controlPoint)) + if (!selectedControlPoints.Contains(controlPoint)) controlPoint.Position -= movementDelta; } } @@ -258,7 +264,7 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components for (int i = 0; i < controlPoints.Count; ++i) { var controlPoint = controlPoints[i]; - if (draggedControlPoints.Contains(controlPoint)) + if (selectedControlPoints.Contains(controlPoint)) controlPoint.Position = dragStartPositions[i] + (e.MousePosition - e.MouseDownPosition); } } From 9973db3981327fb507a03cf270320b8637bbd81e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 22 Dec 2021 08:40:17 +0100 Subject: [PATCH 11/22] Add failing test steps for right click behaviour --- .../Editor/TestSceneSliderControlPointPiece.cs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderControlPointPiece.cs b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderControlPointPiece.cs index a0bdcb144f..d8618ee289 100644 --- a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderControlPointPiece.cs +++ b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderControlPointPiece.cs @@ -58,6 +58,10 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor assertSelectionCount(1); assertSelected(0); + AddStep("click right mouse", () => InputManager.Click(MouseButton.Right)); + assertSelectionCount(1); + assertSelected(0); + moveMouseToControlPoint(3); AddStep("click left mouse", () => InputManager.Click(MouseButton.Left)); assertSelectionCount(1); @@ -77,6 +81,12 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor assertSelected(2); assertSelected(3); + AddStep("click right mouse", () => InputManager.Click(MouseButton.Right)); + assertSelectionCount(3); + assertSelected(0); + assertSelected(2); + assertSelected(3); + AddStep("release control", () => InputManager.ReleaseKey(Key.ControlLeft)); AddStep("click left mouse", () => InputManager.Click(MouseButton.Left)); assertSelectionCount(1); From e22745397dc0716636b7af6b9006e61d68ba64c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 22 Dec 2021 08:55:26 +0100 Subject: [PATCH 12/22] Fix right click deselecting clicked path piece with control held --- .../Edit/Blueprints/Sliders/Components/PathControlPointPiece.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointPiece.cs b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointPiece.cs index 3db3e85c11..3709cd3a7d 100644 --- a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointPiece.cs +++ b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointPiece.cs @@ -161,6 +161,8 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components case MouseButton.Right: if (!IsSelected.Value) RequestSelection.Invoke(this, e); + + keepSelection = true; return false; // Allow context menu to show } From 307d3709e06cea42cddfe80fc2219c4dcc12d33b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 22 Dec 2021 09:32:38 +0100 Subject: [PATCH 13/22] Add failing test steps for selection behaviour on new point creation --- .../Editor/TestSceneSliderControlPointPiece.cs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderControlPointPiece.cs b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderControlPointPiece.cs index d8618ee289..74d6d23348 100644 --- a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderControlPointPiece.cs +++ b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderControlPointPiece.cs @@ -92,6 +92,21 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor assertSelectionCount(1); assertSelected(0); + AddStep("move mouse to new point position", () => + { + Vector2 position = slider.Position + (slider.Path.ControlPoints[2].Position + slider.Path.ControlPoints[3].Position) / 2; + InputManager.MoveMouseTo(drawableObject.Parent.ToScreenSpace(position)); + }); + AddStep("ctrl+click to create new point", () => + { + InputManager.PressKey(Key.ControlLeft); + InputManager.Click(MouseButton.Left); + InputManager.ReleaseKey(Key.ControlLeft); + }); + AddAssert("slider has 6 control points", () => slider.Path.ControlPoints.Count == 6); + assertSelectionCount(1); + assertSelected(3); + void assertSelectionCount(int count) => AddAssert($"{count} control point pieces selected", () => this.ChildrenOfType().Count(piece => piece.IsSelected.Value) == count); From 6330fa5dc5f2300e2b48244a91e2d59ec79d202c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 22 Dec 2021 10:03:58 +0100 Subject: [PATCH 14/22] Select newly created control point --- .../Components/PathControlPointVisualiser.cs | 22 +++++++++++----- .../Sliders/SliderSelectionBlueprint.cs | 26 +++++++++++-------- 2 files changed, 31 insertions(+), 17 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointVisualiser.cs b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointVisualiser.cs index e06fc1510c..b72e2d786b 100644 --- a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointVisualiser.cs +++ b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointVisualiser.cs @@ -92,7 +92,7 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components Pieces.Add(new PathControlPointPiece(slider, point).With(d => { if (allowSelection) - d.RequestSelection = selectPiece; + d.RequestSelection = selectionRequested; d.DragStarted = dragStarted; d.DragInProgress = dragInProgress; @@ -128,6 +128,9 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components protected override bool OnClick(ClickEvent e) { + if (Pieces.Any(piece => piece.IsHovered)) + return false; + foreach (var piece in Pieces) { piece.IsSelected.Value = false; @@ -151,15 +154,22 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components { } - private void selectPiece(PathControlPointPiece piece, MouseButtonEvent e) + private void selectionRequested(PathControlPointPiece piece, MouseButtonEvent e) { if (e.Button == MouseButton.Left && inputManager.CurrentState.Keyboard.ControlPressed) piece.IsSelected.Toggle(); else - { - foreach (var p in Pieces) - p.IsSelected.Value = p == piece; - } + SetSelectionTo(piece.ControlPoint); + } + + /// + /// Selects the corresponding to the given , + /// and deselects all other s. + /// + public void SetSelectionTo(PathControlPoint pathControlPoint) + { + foreach (var p in Pieces) + p.IsSelected.Value = p.ControlPoint == pathControlPoint; } /// diff --git a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/SliderSelectionBlueprint.cs b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/SliderSelectionBlueprint.cs index 17a62fc61c..de8c0d3b78 100644 --- a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/SliderSelectionBlueprint.cs +++ b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/SliderSelectionBlueprint.cs @@ -140,7 +140,8 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders case MouseButton.Left: if (e.ControlPressed && IsSelected) { - placementControlPointIndex = addControlPoint(e.MousePosition); + placementControlPoint = addControlPoint(e.MousePosition); + ControlPointVisualiser?.SetSelectionTo(placementControlPoint); return true; // Stop input from being handled and modifying the selection } @@ -150,11 +151,12 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders return false; } - private int? placementControlPointIndex; + [CanBeNull] + private PathControlPoint placementControlPoint; protected override bool OnDragStart(DragStartEvent e) { - if (placementControlPointIndex != null) + if (placementControlPoint != null) { changeHandler?.BeginChange(); return true; @@ -165,16 +167,16 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders protected override void OnDrag(DragEvent e) { - Debug.Assert(placementControlPointIndex != null); + Debug.Assert(placementControlPoint != null); - HitObject.Path.ControlPoints[placementControlPointIndex.Value].Position = e.MousePosition - HitObject.Position; + placementControlPoint.Position = e.MousePosition - HitObject.Position; } protected override void OnDragEnd(DragEndEvent e) { - if (placementControlPointIndex != null) + if (placementControlPoint != null) { - placementControlPointIndex = null; + placementControlPoint = null; changeHandler?.EndChange(); } } @@ -193,7 +195,7 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders return false; } - private int addControlPoint(Vector2 position) + private PathControlPoint addControlPoint(Vector2 position) { position -= HitObject.Position; @@ -211,10 +213,12 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders } } - // Move the control points from the insertion index onwards to make room for the insertion - controlPoints.Insert(insertionIndex, new PathControlPoint { Position = position }); + var pathControlPoint = new PathControlPoint { Position = position }; - return insertionIndex; + // Move the control points from the insertion index onwards to make room for the insertion + controlPoints.Insert(insertionIndex, pathControlPoint); + + return pathControlPoint; } private void removeControlPoints(List toRemove) From c3fada1926a2c0ca2c3d6352a1548e2a51fdcab1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 22 Dec 2021 10:28:35 +0100 Subject: [PATCH 15/22] Replace assertion with soft null check Surrounding `OnDrag{Start,End}` methods did so already. --- .../Edit/Blueprints/Sliders/SliderSelectionBlueprint.cs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/SliderSelectionBlueprint.cs b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/SliderSelectionBlueprint.cs index de8c0d3b78..9caed970b1 100644 --- a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/SliderSelectionBlueprint.cs +++ b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/SliderSelectionBlueprint.cs @@ -2,7 +2,6 @@ // See the LICENCE file in the repository root for full licence text. using System.Collections.Generic; -using System.Diagnostics; using System.Linq; using JetBrains.Annotations; using osu.Framework.Allocation; @@ -167,9 +166,8 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders protected override void OnDrag(DragEvent e) { - Debug.Assert(placementControlPoint != null); - - placementControlPoint.Position = e.MousePosition - HitObject.Position; + if (placementControlPoint != null) + placementControlPoint.Position = e.MousePosition - HitObject.Position; } protected override void OnDragEnd(DragEndEvent e) From ef20182a34845d18b22a6e90648123c4465d6c4f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 22 Dec 2021 10:57:39 +0100 Subject: [PATCH 16/22] Rewrite test to check selection state during and after new control point placement --- .../Editor/TestSceneSliderControlPointPiece.cs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderControlPointPiece.cs b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderControlPointPiece.cs index 74d6d23348..93ee9b515b 100644 --- a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderControlPointPiece.cs +++ b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderControlPointPiece.cs @@ -100,13 +100,20 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor AddStep("ctrl+click to create new point", () => { InputManager.PressKey(Key.ControlLeft); - InputManager.Click(MouseButton.Left); - InputManager.ReleaseKey(Key.ControlLeft); + InputManager.PressButton(MouseButton.Left); }); AddAssert("slider has 6 control points", () => slider.Path.ControlPoints.Count == 6); assertSelectionCount(1); assertSelected(3); + AddStep("release ctrl+click", () => + { + InputManager.ReleaseButton(MouseButton.Left); + InputManager.ReleaseKey(Key.ControlLeft); + }); + assertSelectionCount(1); + assertSelected(3); + void assertSelectionCount(int count) => AddAssert($"{count} control point pieces selected", () => this.ChildrenOfType().Count(piece => piece.IsSelected.Value) == count); From b0df787b1addc5b36f616e2ee19bb1fbb538ce04 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 23 Dec 2021 14:13:36 +0900 Subject: [PATCH 17/22] Move public method up and add xmldoc to second public method --- .../Components/PathControlPointVisualiser.cs | 62 ++++++++++--------- 1 file changed, 33 insertions(+), 29 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointVisualiser.cs b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointVisualiser.cs index b72e2d786b..065d4737a5 100644 --- a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointVisualiser.cs +++ b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointVisualiser.cs @@ -69,6 +69,39 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components controlPoints.BindTo(slider.Path.ControlPoints); } + /// + /// Selects the corresponding to the given , + /// and deselects all other s. + /// + public void SetSelectionTo(PathControlPoint pathControlPoint) + { + foreach (var p in Pieces) + p.IsSelected.Value = p.ControlPoint == pathControlPoint; + } + + /// + /// Delete all visually selected s. + /// + /// + public bool DeleteSelected() + { + List toRemove = Pieces.Where(p => p.IsSelected.Value).Select(p => p.ControlPoint).ToList(); + + // Ensure that there are any points to be deleted + if (toRemove.Count == 0) + return false; + + 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; + } + private void onControlPointsChanged(object sender, NotifyCollectionChangedEventArgs e) { switch (e.Action) @@ -162,16 +195,6 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components SetSelectionTo(piece.ControlPoint); } - /// - /// Selects the corresponding to the given , - /// and deselects all other s. - /// - public void SetSelectionTo(PathControlPoint pathControlPoint) - { - foreach (var p in Pieces) - p.IsSelected.Value = p.ControlPoint == pathControlPoint; - } - /// /// Attempts to set the given control point piece to the given path type. /// If that would fail, try to change the path such that it instead succeeds @@ -203,25 +226,6 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components [Resolved(CanBeNull = true)] private IEditorChangeHandler changeHandler { get; set; } - public bool DeleteSelected() - { - List toRemove = Pieces.Where(p => p.IsSelected.Value).Select(p => p.ControlPoint).ToList(); - - // Ensure that there are any points to be deleted - if (toRemove.Count == 0) - return false; - - 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; - } - #region Drag handling private Vector2[] dragStartPositions; From 29b42402a3e4c1eacbebec3699f37e93107c364e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Thu, 23 Dec 2021 09:06:03 +0100 Subject: [PATCH 18/22] Add failing test for drag after placement moving last placed point sometimes --- .../TestSceneSliderControlPointPiece.cs | 74 +++++++++++++++---- 1 file changed, 61 insertions(+), 13 deletions(-) diff --git a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderControlPointPiece.cs b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderControlPointPiece.cs index 93ee9b515b..e7bcd2cadc 100644 --- a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderControlPointPiece.cs +++ b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderControlPointPiece.cs @@ -92,17 +92,12 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor assertSelectionCount(1); assertSelected(0); - AddStep("move mouse to new point position", () => - { - Vector2 position = slider.Position + (slider.Path.ControlPoints[2].Position + slider.Path.ControlPoints[3].Position) / 2; - InputManager.MoveMouseTo(drawableObject.Parent.ToScreenSpace(position)); - }); + moveMouseToRelativePosition(new Vector2(350, 0)); AddStep("ctrl+click to create new point", () => { InputManager.PressKey(Key.ControlLeft); InputManager.PressButton(MouseButton.Left); }); - AddAssert("slider has 6 control points", () => slider.Path.ControlPoints.Count == 6); assertSelectionCount(1); assertSelected(3); @@ -113,15 +108,68 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor }); assertSelectionCount(1); assertSelected(3); - - void assertSelectionCount(int count) => - AddAssert($"{count} control point pieces selected", () => this.ChildrenOfType().Count(piece => piece.IsSelected.Value) == count); - - void assertSelected(int index) => - AddAssert($"{(index + 1).ToOrdinalWords()} control point piece selected", - () => this.ChildrenOfType().Single(piece => piece.ControlPoint == slider.Path.ControlPoints[index]).IsSelected.Value); } + [Test] + public void TestNewControlPointCreation() + { + moveMouseToRelativePosition(new Vector2(350, 0)); + AddStep("ctrl+click to create new point", () => + { + InputManager.PressKey(Key.ControlLeft); + InputManager.PressButton(MouseButton.Left); + }); + AddAssert("slider has 6 control points", () => slider.Path.ControlPoints.Count == 6); + AddStep("release ctrl+click", () => + { + InputManager.ReleaseButton(MouseButton.Left); + InputManager.ReleaseKey(Key.ControlLeft); + }); + + // ensure that the next drag doesn't attempt to move the placement that just finished. + moveMouseToRelativePosition(new Vector2(0, 50)); + AddStep("press left mouse", () => InputManager.PressButton(MouseButton.Left)); + moveMouseToRelativePosition(new Vector2(0, 100)); + AddStep("release left mouse", () => InputManager.ReleaseButton(MouseButton.Left)); + assertControlPointPosition(3, new Vector2(350, 0)); + + moveMouseToRelativePosition(new Vector2(400, 75)); + AddStep("ctrl+click to create new point", () => + { + InputManager.PressKey(Key.ControlLeft); + InputManager.PressButton(MouseButton.Left); + }); + AddAssert("slider has 7 control points", () => slider.Path.ControlPoints.Count == 7); + moveMouseToRelativePosition(new Vector2(350, 75)); + AddStep("release ctrl+click", () => + { + InputManager.ReleaseButton(MouseButton.Left); + InputManager.ReleaseKey(Key.ControlLeft); + }); + assertControlPointPosition(5, new Vector2(350, 75)); + + // ensure that the next drag doesn't attempt to move the placement that just finished. + moveMouseToRelativePosition(new Vector2(0, 50)); + AddStep("press left mouse", () => InputManager.PressButton(MouseButton.Left)); + moveMouseToRelativePosition(new Vector2(0, 100)); + AddStep("release left mouse", () => InputManager.ReleaseButton(MouseButton.Left)); + assertControlPointPosition(5, new Vector2(350, 75)); + } + + private void assertSelectionCount(int count) => + AddAssert($"{count} control point pieces selected", () => this.ChildrenOfType().Count(piece => piece.IsSelected.Value) == count); + + private void assertSelected(int index) => + AddAssert($"{(index + 1).ToOrdinalWords()} control point piece selected", + () => this.ChildrenOfType().Single(piece => piece.ControlPoint == slider.Path.ControlPoints[index]).IsSelected.Value); + + private void moveMouseToRelativePosition(Vector2 relativePosition) => + AddStep($"move mouse to {relativePosition}", () => + { + Vector2 position = slider.Position + relativePosition; + InputManager.MoveMouseTo(drawableObject.Parent.ToScreenSpace(position)); + }); + [Test] public void TestDragControlPoint() { From cbda637d66be33dec4f2ccb574f3c1a8747c2da3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Thu, 23 Dec 2021 09:09:14 +0100 Subject: [PATCH 19/22] Fix drag after placement moving last placed point sometimes More specifically, if the left mouse button was just pressed without a drag, `OnDragEnd()` wouldn't fire, and the next drag would start moving the last placed control point around regardless of where the mouse was. --- .../Edit/Blueprints/Sliders/SliderSelectionBlueprint.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/SliderSelectionBlueprint.cs b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/SliderSelectionBlueprint.cs index 9caed970b1..d5dd411600 100644 --- a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/SliderSelectionBlueprint.cs +++ b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/SliderSelectionBlueprint.cs @@ -170,7 +170,7 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders placementControlPoint.Position = e.MousePosition - HitObject.Position; } - protected override void OnDragEnd(DragEndEvent e) + protected override void OnMouseUp(MouseUpEvent e) { if (placementControlPoint != null) { From c6a5ac1c5f259391167d3f3eaf8bc37c8b05546c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Thu, 23 Dec 2021 09:19:21 +0100 Subject: [PATCH 20/22] Fix control point additions without a drag not being undoable --- .../Blueprints/Sliders/SliderSelectionBlueprint.cs | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/SliderSelectionBlueprint.cs b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/SliderSelectionBlueprint.cs index d5dd411600..2aebe05c2f 100644 --- a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/SliderSelectionBlueprint.cs +++ b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/SliderSelectionBlueprint.cs @@ -139,6 +139,7 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders case MouseButton.Left: if (e.ControlPressed && IsSelected) { + changeHandler?.BeginChange(); placementControlPoint = addControlPoint(e.MousePosition); ControlPointVisualiser?.SetSelectionTo(placementControlPoint); return true; // Stop input from being handled and modifying the selection @@ -153,16 +154,7 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders [CanBeNull] private PathControlPoint placementControlPoint; - protected override bool OnDragStart(DragStartEvent e) - { - if (placementControlPoint != null) - { - changeHandler?.BeginChange(); - return true; - } - - return false; - } + protected override bool OnDragStart(DragStartEvent e) => placementControlPoint != null; protected override void OnDrag(DragEvent e) { From 0732a9e6da034b6121dc719c78d8177b6ff77897 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 23 Dec 2021 18:08:44 +0900 Subject: [PATCH 21/22] Update framework --- osu.Android.props | 2 +- osu.Game/osu.Game.csproj | 2 +- osu.iOS.props | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/osu.Android.props b/osu.Android.props index ca4d88a8a7..4f0125bed2 100644 --- a/osu.Android.props +++ b/osu.Android.props @@ -52,7 +52,7 @@ - + diff --git a/osu.Game/osu.Game.csproj b/osu.Game/osu.Game.csproj index c538f55e89..565f78e2d4 100644 --- a/osu.Game/osu.Game.csproj +++ b/osu.Game/osu.Game.csproj @@ -36,7 +36,7 @@ runtime; build; native; contentfiles; analyzers; buildtransitive - + diff --git a/osu.iOS.props b/osu.iOS.props index 5cc1857dfe..bdb3b0b149 100644 --- a/osu.iOS.props +++ b/osu.iOS.props @@ -60,7 +60,7 @@ - + @@ -83,7 +83,7 @@ - + From 10405908440576b576cc4c5063d3e6d190d1aa32 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Thu, 23 Dec 2021 10:33:17 +0100 Subject: [PATCH 22/22] Add cancellation support to game-side `IResourceStore`s --- osu.Game.Tests/Gameplay/TestSceneStoryboardSamples.cs | 4 +++- .../Rulesets/TestSceneDrawableRulesetDependencies.cs | 3 ++- osu.Game/IO/Archives/ArchiveReader.cs | 5 +++-- osu.Game/Localisation/ResourceManagerLocalisationStore.cs | 3 ++- osu.Game/Rulesets/UI/DrawableRulesetDependencies.cs | 3 ++- osu.Game/Tests/Beatmaps/HitObjectSampleTest.cs | 3 ++- osu.Game/Tests/Visual/OsuTestScene.cs | 3 ++- 7 files changed, 16 insertions(+), 8 deletions(-) diff --git a/osu.Game.Tests/Gameplay/TestSceneStoryboardSamples.cs b/osu.Game.Tests/Gameplay/TestSceneStoryboardSamples.cs index 3aab28886e..28915b2c4a 100644 --- a/osu.Game.Tests/Gameplay/TestSceneStoryboardSamples.cs +++ b/osu.Game.Tests/Gameplay/TestSceneStoryboardSamples.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Generic; using System.IO; using System.Linq; +using System.Threading; using System.Threading.Tasks; using NUnit.Framework; using osu.Framework.Audio; @@ -184,7 +185,8 @@ namespace osu.Game.Tests.Gameplay public byte[] Get(string name) => name == resourceName ? TestResources.GetStore().Get("Resources/Samples/test-sample.mp3") : null; - public Task GetAsync(string name) => name == resourceName ? TestResources.GetStore().GetAsync("Resources/Samples/test-sample.mp3") : null; + public Task GetAsync(string name, CancellationToken cancellationToken = default) + => name == resourceName ? TestResources.GetStore().GetAsync("Resources/Samples/test-sample.mp3", cancellationToken) : null; public Stream GetStream(string name) => name == resourceName ? TestResources.GetStore().GetStream("Resources/Samples/test-sample.mp3") : null; diff --git a/osu.Game.Tests/Rulesets/TestSceneDrawableRulesetDependencies.cs b/osu.Game.Tests/Rulesets/TestSceneDrawableRulesetDependencies.cs index c357fccd27..20439ac969 100644 --- a/osu.Game.Tests/Rulesets/TestSceneDrawableRulesetDependencies.cs +++ b/osu.Game.Tests/Rulesets/TestSceneDrawableRulesetDependencies.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.IO; +using System.Threading; using System.Threading.Tasks; using NUnit.Framework; using osu.Framework.Allocation; @@ -114,7 +115,7 @@ namespace osu.Game.Tests.Rulesets public Sample Get(string name) => null; - public Task GetAsync(string name) => null; + public Task GetAsync(string name, CancellationToken cancellationToken = default) => null; public Stream GetStream(string name) => null; diff --git a/osu.Game/IO/Archives/ArchiveReader.cs b/osu.Game/IO/Archives/ArchiveReader.cs index 679ab40402..f787534e2d 100644 --- a/osu.Game/IO/Archives/ArchiveReader.cs +++ b/osu.Game/IO/Archives/ArchiveReader.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using System.IO; +using System.Threading; using System.Threading.Tasks; using osu.Framework.IO.Stores; @@ -33,7 +34,7 @@ namespace osu.Game.IO.Archives public virtual byte[] Get(string name) => GetAsync(name).Result; - public async Task GetAsync(string name) + public async Task GetAsync(string name, CancellationToken cancellationToken = default) { using (Stream input = GetStream(name)) { @@ -41,7 +42,7 @@ namespace osu.Game.IO.Archives return null; byte[] buffer = new byte[input.Length]; - await input.ReadAsync(buffer).ConfigureAwait(false); + await input.ReadAsync(buffer, cancellationToken).ConfigureAwait(false); return buffer; } } diff --git a/osu.Game/Localisation/ResourceManagerLocalisationStore.cs b/osu.Game/Localisation/ResourceManagerLocalisationStore.cs index 82dc0ad110..0fb85e4a19 100644 --- a/osu.Game/Localisation/ResourceManagerLocalisationStore.cs +++ b/osu.Game/Localisation/ResourceManagerLocalisationStore.cs @@ -7,6 +7,7 @@ using System.Globalization; using System.IO; using System.Linq; using System.Resources; +using System.Threading; using System.Threading.Tasks; using osu.Framework.Localisation; @@ -75,7 +76,7 @@ namespace osu.Game.Localisation } } - public Task GetAsync(string lookup) + public Task GetAsync(string lookup, CancellationToken cancellationToken = default) { return Task.FromResult(Get(lookup)); } diff --git a/osu.Game/Rulesets/UI/DrawableRulesetDependencies.cs b/osu.Game/Rulesets/UI/DrawableRulesetDependencies.cs index e49e515d2e..aacb8dbe83 100644 --- a/osu.Game/Rulesets/UI/DrawableRulesetDependencies.cs +++ b/osu.Game/Rulesets/UI/DrawableRulesetDependencies.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.IO; +using System.Threading; using System.Threading.Tasks; using osu.Framework.Allocation; using osu.Framework.Audio; @@ -115,7 +116,7 @@ namespace osu.Game.Rulesets.UI public Sample Get(string name) => primary.Get(name) ?? fallback.Get(name); - public Task GetAsync(string name) => primary.GetAsync(name) ?? fallback.GetAsync(name); + public Task GetAsync(string name, CancellationToken cancellationToken = default) => primary.GetAsync(name, cancellationToken) ?? fallback.GetAsync(name, cancellationToken); public Stream GetStream(string name) => primary.GetStream(name) ?? fallback.GetStream(name); diff --git a/osu.Game/Tests/Beatmaps/HitObjectSampleTest.cs b/osu.Game/Tests/Beatmaps/HitObjectSampleTest.cs index f919edecf7..27d1de83ec 100644 --- a/osu.Game/Tests/Beatmaps/HitObjectSampleTest.cs +++ b/osu.Game/Tests/Beatmaps/HitObjectSampleTest.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Generic; using System.IO; using System.Linq; +using System.Threading; using System.Threading.Tasks; using osu.Framework.Allocation; using osu.Framework.Audio; @@ -166,7 +167,7 @@ namespace osu.Game.Tests.Beatmaps return Array.Empty(); } - public Task GetAsync(string name) + public Task GetAsync(string name, CancellationToken cancellationToken = default) { markLookup(name); return Task.FromResult(Array.Empty()); diff --git a/osu.Game/Tests/Visual/OsuTestScene.cs b/osu.Game/Tests/Visual/OsuTestScene.cs index 7bc63a8c26..1b2c1e72c4 100644 --- a/osu.Game/Tests/Visual/OsuTestScene.cs +++ b/osu.Game/Tests/Visual/OsuTestScene.cs @@ -6,6 +6,7 @@ using System.Collections.Generic; using System.Diagnostics; using System.IO; using System.Linq; +using System.Threading; using System.Threading.Tasks; using JetBrains.Annotations; using osu.Framework.Allocation; @@ -352,7 +353,7 @@ namespace osu.Game.Tests.Visual public Track Get(string name) => throw new NotImplementedException(); - public Task GetAsync(string name) => throw new NotImplementedException(); + public Task GetAsync(string name, CancellationToken cancellationToken = default) => throw new NotImplementedException(); public Stream GetStream(string name) => throw new NotImplementedException();