From 4d2bc790fd96b68234b39e623d6147e09a444ffb Mon Sep 17 00:00:00 2001 From: kamp Date: Sat, 14 Nov 2020 13:20:16 +0100 Subject: [PATCH 1/9] Fix crash on shift+right-click deleting objects --- osu.Game/Screens/Edit/Compose/Components/BlueprintContainer.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Screens/Edit/Compose/Components/BlueprintContainer.cs b/osu.Game/Screens/Edit/Compose/Components/BlueprintContainer.cs index 3229719d5a..c9043ccef3 100644 --- a/osu.Game/Screens/Edit/Compose/Components/BlueprintContainer.cs +++ b/osu.Game/Screens/Edit/Compose/Components/BlueprintContainer.cs @@ -338,7 +338,7 @@ namespace osu.Game.Screens.Edit.Compose.Components /// Whether a selection was performed. private bool beginClickSelection(MouseButtonEvent e) { - foreach (SelectionBlueprint blueprint in SelectionBlueprints.AliveChildren) + foreach (SelectionBlueprint blueprint in SelectionBlueprints.AliveChildren.ToList()) { if (!blueprint.IsHovered) continue; From 83727a2e85a868d612a80bdc7a65bb0bf7b6befd Mon Sep 17 00:00:00 2001 From: kamp Date: Sun, 15 Nov 2020 16:06:29 +0100 Subject: [PATCH 2/9] Add quick-delete tests --- .../Editing/TestSceneEditorQuickDelete.cs | 93 +++++++++++++++++++ 1 file changed, 93 insertions(+) create mode 100644 osu.Game.Tests/Visual/Editing/TestSceneEditorQuickDelete.cs diff --git a/osu.Game.Tests/Visual/Editing/TestSceneEditorQuickDelete.cs b/osu.Game.Tests/Visual/Editing/TestSceneEditorQuickDelete.cs new file mode 100644 index 0000000000..cb921fd857 --- /dev/null +++ b/osu.Game.Tests/Visual/Editing/TestSceneEditorQuickDelete.cs @@ -0,0 +1,93 @@ +// 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.Game.Beatmaps; +using osu.Game.Rulesets; +using osu.Game.Rulesets.Objects; +using osu.Game.Rulesets.Osu; +using osu.Game.Rulesets.Osu.Objects; +using osu.Game.Rulesets.Osu.Edit.Blueprints.HitCircles; +using osu.Game.Rulesets.Osu.Edit.Blueprints.HitCircles.Components; +using osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders; +using osu.Game.Tests.Beatmaps; +using osu.Game.Rulesets.Edit; +using osu.Game.Screens.Edit.Compose; +using osu.Game.Screens.Edit.Compose.Components; +using osuTK; +using osuTK.Input; + +namespace osu.Game.Tests.Visual.Editing +{ + public class TestSceneEditorQuickDelete : EditorTestScene + { + protected override Ruleset CreateEditorRuleset() => new OsuRuleset(); + + protected override IBeatmap CreateBeatmap(RulesetInfo ruleset) => new TestBeatmap(ruleset, false); + + [Test] + public void TestQuickDeleteRemovesObject() + { + var addedObject = new HitCircle { StartTime = 1000 }; + + AddStep("add hitobject", () => EditorBeatmap.Add(addedObject)); + + AddStep("select added object", () => EditorBeatmap.SelectedHitObjects.Add(addedObject)); + + AddStep("move mouse to object", () => + { + var pos = getBlueprintContainer.SelectionBlueprints + .ChildrenOfType().First() + .ChildrenOfType().First().ScreenSpaceDrawQuad.Centre; + InputManager.MoveMouseTo(pos); + }); + AddStep("hold shift", () => InputManager.PressKey(Key.ShiftLeft)); + AddStep("rightclick", () => InputManager.Click(MouseButton.Right)); + AddStep("release shift", () => InputManager.ReleaseKey(Key.ShiftLeft)); + + AddAssert("no hitobjects in beatmap", () => EditorBeatmap.HitObjects.Count == 0); + } + + [Test] + public void TestQuickDeleteRemovesSliderControlPoint() + { + Slider slider = new Slider { StartTime = 1000 }; + + PathControlPoint[] points = new PathControlPoint[] + { + new PathControlPoint(), + new PathControlPoint(new Vector2(50, 0)), + new PathControlPoint(new Vector2(100, 0)) + }; + + AddStep("add slider", () => + { + slider.Path = new SliderPath(points); + EditorBeatmap.Add(slider); + }); + + AddStep("select added slider", () => EditorBeatmap.SelectedHitObjects.Add(slider)); + + AddStep("move mouse to controlpoint", () => + { + // This doesn't get the HitCirclePiece corresponding to the last control point on consecutive runs, + // causing the slider to translate by 50 every time and go off the screen after about 10 runs. + var pos = getBlueprintContainer.SelectionBlueprints + .ChildrenOfType().First() + .ChildrenOfType().Last().ScreenSpaceDrawQuad.Centre; + InputManager.MoveMouseTo(pos); + }); + AddStep("hold shift", () => InputManager.PressKey(Key.ShiftLeft)); + AddStep("rightclick", () => InputManager.Click(MouseButton.Right)); + AddStep("release shift", () => InputManager.ReleaseKey(Key.ShiftLeft)); + + AddAssert("slider has 2 points", () => slider.Path.ControlPoints.Count == 2); + } + + private BlueprintContainer getBlueprintContainer => Editor.ChildrenOfType().First() + .ChildrenOfType().First() + .ChildrenOfType().First(); + } +} From 1db303b15924cba91b828d553a858508c6840b38 Mon Sep 17 00:00:00 2001 From: kamp Date: Sun, 15 Nov 2020 16:54:48 +0100 Subject: [PATCH 3/9] Revert beginClickSelection logic --- .../Edit/Compose/Components/BlueprintContainer.cs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/osu.Game/Screens/Edit/Compose/Components/BlueprintContainer.cs b/osu.Game/Screens/Edit/Compose/Components/BlueprintContainer.cs index c9043ccef3..4390ed4f10 100644 --- a/osu.Game/Screens/Edit/Compose/Components/BlueprintContainer.cs +++ b/osu.Game/Screens/Edit/Compose/Components/BlueprintContainer.cs @@ -338,15 +338,18 @@ namespace osu.Game.Screens.Edit.Compose.Components /// Whether a selection was performed. private bool beginClickSelection(MouseButtonEvent e) { - foreach (SelectionBlueprint blueprint in SelectionBlueprints.AliveChildren.ToList()) + bool selectedPerformed = true; + + foreach (SelectionBlueprint blueprint in SelectionBlueprints.AliveChildren) { if (!blueprint.IsHovered) continue; - if (SelectionHandler.HandleSelectionRequested(blueprint, e)) - return clickSelectionBegan = true; + selectedPerformed &= SelectionHandler.HandleSelectionRequested(blueprint, e); + clickSelectionBegan = true; + break; } - return false; + return selectedPerformed; } /// From c77ec3e905adc7ffd5bf2e74001cea184081887d Mon Sep 17 00:00:00 2001 From: kamp Date: Sun, 15 Nov 2020 17:42:52 +0100 Subject: [PATCH 4/9] Fix slider control point quickdelete test --- osu.Game.Tests/Visual/Editing/TestSceneEditorQuickDelete.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/osu.Game.Tests/Visual/Editing/TestSceneEditorQuickDelete.cs b/osu.Game.Tests/Visual/Editing/TestSceneEditorQuickDelete.cs index cb921fd857..55daf130a8 100644 --- a/osu.Game.Tests/Visual/Editing/TestSceneEditorQuickDelete.cs +++ b/osu.Game.Tests/Visual/Editing/TestSceneEditorQuickDelete.cs @@ -12,6 +12,7 @@ using osu.Game.Rulesets.Osu.Objects; using osu.Game.Rulesets.Osu.Edit.Blueprints.HitCircles; using osu.Game.Rulesets.Osu.Edit.Blueprints.HitCircles.Components; using osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders; +using osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components; using osu.Game.Tests.Beatmaps; using osu.Game.Rulesets.Edit; using osu.Game.Screens.Edit.Compose; @@ -72,11 +73,10 @@ namespace osu.Game.Tests.Visual.Editing AddStep("move mouse to controlpoint", () => { - // This doesn't get the HitCirclePiece corresponding to the last control point on consecutive runs, - // causing the slider to translate by 50 every time and go off the screen after about 10 runs. var pos = getBlueprintContainer.SelectionBlueprints .ChildrenOfType().First() - .ChildrenOfType().Last().ScreenSpaceDrawQuad.Centre; + .ChildrenOfType().First() + .ChildrenOfType().ElementAt(1).ScreenSpaceDrawQuad.Centre; InputManager.MoveMouseTo(pos); }); AddStep("hold shift", () => InputManager.PressKey(Key.ShiftLeft)); From 4e77800b98aa35758c555d4c690e34d4aa6312aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Sun, 15 Nov 2020 20:51:35 +0100 Subject: [PATCH 5/9] Rename & simplify property --- .../Visual/Editing/TestSceneEditorQuickDelete.cs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/osu.Game.Tests/Visual/Editing/TestSceneEditorQuickDelete.cs b/osu.Game.Tests/Visual/Editing/TestSceneEditorQuickDelete.cs index 55daf130a8..5bae6ec5f7 100644 --- a/osu.Game.Tests/Visual/Editing/TestSceneEditorQuickDelete.cs +++ b/osu.Game.Tests/Visual/Editing/TestSceneEditorQuickDelete.cs @@ -28,6 +28,9 @@ namespace osu.Game.Tests.Visual.Editing protected override IBeatmap CreateBeatmap(RulesetInfo ruleset) => new TestBeatmap(ruleset, false); + private BlueprintContainer blueprintContainer + => Editor.ChildrenOfType().First(); + [Test] public void TestQuickDeleteRemovesObject() { @@ -39,7 +42,7 @@ namespace osu.Game.Tests.Visual.Editing AddStep("move mouse to object", () => { - var pos = getBlueprintContainer.SelectionBlueprints + var pos = blueprintContainer.SelectionBlueprints .ChildrenOfType().First() .ChildrenOfType().First().ScreenSpaceDrawQuad.Centre; InputManager.MoveMouseTo(pos); @@ -73,7 +76,7 @@ namespace osu.Game.Tests.Visual.Editing AddStep("move mouse to controlpoint", () => { - var pos = getBlueprintContainer.SelectionBlueprints + var pos = blueprintContainer.SelectionBlueprints .ChildrenOfType().First() .ChildrenOfType().First() .ChildrenOfType().ElementAt(1).ScreenSpaceDrawQuad.Centre; @@ -85,9 +88,5 @@ namespace osu.Game.Tests.Visual.Editing AddAssert("slider has 2 points", () => slider.Path.ControlPoints.Count == 2); } - - private BlueprintContainer getBlueprintContainer => Editor.ChildrenOfType().First() - .ChildrenOfType().First() - .ChildrenOfType().First(); } } From 1f0945d4deef4678fcc785a7931bdd6fcf7c9665 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Sun, 15 Nov 2020 20:52:33 +0100 Subject: [PATCH 6/9] Simplify accesses via ChildrenOfType() --- .../Visual/Editing/TestSceneEditorQuickDelete.cs | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/osu.Game.Tests/Visual/Editing/TestSceneEditorQuickDelete.cs b/osu.Game.Tests/Visual/Editing/TestSceneEditorQuickDelete.cs index 5bae6ec5f7..9d35185f7e 100644 --- a/osu.Game.Tests/Visual/Editing/TestSceneEditorQuickDelete.cs +++ b/osu.Game.Tests/Visual/Editing/TestSceneEditorQuickDelete.cs @@ -9,13 +9,9 @@ using osu.Game.Rulesets; using osu.Game.Rulesets.Objects; using osu.Game.Rulesets.Osu; using osu.Game.Rulesets.Osu.Objects; -using osu.Game.Rulesets.Osu.Edit.Blueprints.HitCircles; using osu.Game.Rulesets.Osu.Edit.Blueprints.HitCircles.Components; -using osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders; using osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components; using osu.Game.Tests.Beatmaps; -using osu.Game.Rulesets.Edit; -using osu.Game.Screens.Edit.Compose; using osu.Game.Screens.Edit.Compose.Components; using osuTK; using osuTK.Input; @@ -42,9 +38,7 @@ namespace osu.Game.Tests.Visual.Editing AddStep("move mouse to object", () => { - var pos = blueprintContainer.SelectionBlueprints - .ChildrenOfType().First() - .ChildrenOfType().First().ScreenSpaceDrawQuad.Centre; + var pos = blueprintContainer.ChildrenOfType().First().ScreenSpaceDrawQuad.Centre; InputManager.MoveMouseTo(pos); }); AddStep("hold shift", () => InputManager.PressKey(Key.ShiftLeft)); @@ -76,10 +70,7 @@ namespace osu.Game.Tests.Visual.Editing AddStep("move mouse to controlpoint", () => { - var pos = blueprintContainer.SelectionBlueprints - .ChildrenOfType().First() - .ChildrenOfType().First() - .ChildrenOfType().ElementAt(1).ScreenSpaceDrawQuad.Centre; + var pos = blueprintContainer.ChildrenOfType().ElementAt(1).ScreenSpaceDrawQuad.Centre; InputManager.MoveMouseTo(pos); }); AddStep("hold shift", () => InputManager.PressKey(Key.ShiftLeft)); From 337311c3236c848c9ead6c885991a227f288a581 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Sun, 15 Nov 2020 20:52:58 +0100 Subject: [PATCH 7/9] Remove redundant type specification --- osu.Game.Tests/Visual/Editing/TestSceneEditorQuickDelete.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Tests/Visual/Editing/TestSceneEditorQuickDelete.cs b/osu.Game.Tests/Visual/Editing/TestSceneEditorQuickDelete.cs index 9d35185f7e..9bcb056f25 100644 --- a/osu.Game.Tests/Visual/Editing/TestSceneEditorQuickDelete.cs +++ b/osu.Game.Tests/Visual/Editing/TestSceneEditorQuickDelete.cs @@ -53,7 +53,7 @@ namespace osu.Game.Tests.Visual.Editing { Slider slider = new Slider { StartTime = 1000 }; - PathControlPoint[] points = new PathControlPoint[] + PathControlPoint[] points = { new PathControlPoint(), new PathControlPoint(new Vector2(50, 0)), From 399a1a16a070d31e36a2076f505dd9111de8f381 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Sun, 15 Nov 2020 21:06:47 +0100 Subject: [PATCH 8/9] Refactor beginClickSelection in a slightly different way --- .../Screens/Edit/Compose/Components/BlueprintContainer.cs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/osu.Game/Screens/Edit/Compose/Components/BlueprintContainer.cs b/osu.Game/Screens/Edit/Compose/Components/BlueprintContainer.cs index 4390ed4f10..8f3c86b98a 100644 --- a/osu.Game/Screens/Edit/Compose/Components/BlueprintContainer.cs +++ b/osu.Game/Screens/Edit/Compose/Components/BlueprintContainer.cs @@ -338,18 +338,14 @@ namespace osu.Game.Screens.Edit.Compose.Components /// Whether a selection was performed. private bool beginClickSelection(MouseButtonEvent e) { - bool selectedPerformed = true; - foreach (SelectionBlueprint blueprint in SelectionBlueprints.AliveChildren) { if (!blueprint.IsHovered) continue; - selectedPerformed &= SelectionHandler.HandleSelectionRequested(blueprint, e); - clickSelectionBegan = true; - break; + return clickSelectionBegan = SelectionHandler.HandleSelectionRequested(blueprint, e); } - return selectedPerformed; + return false; } /// From 7169dc91735feb4250636432ee7f5a07114cd94a Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 16 Nov 2020 14:06:37 +0900 Subject: [PATCH 9/9] Add extra step checking slider deletion on second click --- .../Visual/Editing/TestSceneEditorQuickDelete.cs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/osu.Game.Tests/Visual/Editing/TestSceneEditorQuickDelete.cs b/osu.Game.Tests/Visual/Editing/TestSceneEditorQuickDelete.cs index 9bcb056f25..9efd299fba 100644 --- a/osu.Game.Tests/Visual/Editing/TestSceneEditorQuickDelete.cs +++ b/osu.Game.Tests/Visual/Editing/TestSceneEditorQuickDelete.cs @@ -42,7 +42,7 @@ namespace osu.Game.Tests.Visual.Editing InputManager.MoveMouseTo(pos); }); AddStep("hold shift", () => InputManager.PressKey(Key.ShiftLeft)); - AddStep("rightclick", () => InputManager.Click(MouseButton.Right)); + AddStep("right click", () => InputManager.Click(MouseButton.Right)); AddStep("release shift", () => InputManager.ReleaseKey(Key.ShiftLeft)); AddAssert("no hitobjects in beatmap", () => EditorBeatmap.HitObjects.Count == 0); @@ -74,10 +74,15 @@ namespace osu.Game.Tests.Visual.Editing InputManager.MoveMouseTo(pos); }); AddStep("hold shift", () => InputManager.PressKey(Key.ShiftLeft)); - AddStep("rightclick", () => InputManager.Click(MouseButton.Right)); - AddStep("release shift", () => InputManager.ReleaseKey(Key.ShiftLeft)); + AddStep("right click", () => InputManager.Click(MouseButton.Right)); AddAssert("slider has 2 points", () => slider.Path.ControlPoints.Count == 2); + + // second click should nuke the object completely. + AddStep("right click", () => InputManager.Click(MouseButton.Right)); + AddAssert("no hitobjects in beatmap", () => EditorBeatmap.HitObjects.Count == 0); + + AddStep("release shift", () => InputManager.ReleaseKey(Key.ShiftLeft)); } } }