From 5aa186c987a08cb563f1c9f7e3d3fd1f99bc67fa Mon Sep 17 00:00:00 2001 From: Salman Alshamrani Date: Tue, 5 Aug 2025 11:31:49 +0300 Subject: [PATCH 1/3] Add failing test case --- .../Editor/TestSceneOsuEditorGrids.cs | 66 +++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneOsuEditorGrids.cs b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneOsuEditorGrids.cs index f257ed5987..c6893a5bdf 100644 --- a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneOsuEditorGrids.cs +++ b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneOsuEditorGrids.cs @@ -9,6 +9,7 @@ using osu.Framework.Utils; using osu.Game.Rulesets.Edit; using osu.Game.Rulesets.Osu.Edit; using osu.Game.Rulesets.Osu.Edit.Blueprints.HitCircles; +using osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders; using osu.Game.Rulesets.Osu.Objects; using osu.Game.Screens.Edit.Compose.Components; using osu.Game.Tests.Visual; @@ -284,5 +285,70 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor && Precision.AlmostEquals(composer.GridLineRotation.Value, 0.09, 0.01); }); } + + [Test] + public void TestGridPlacementCommittedByDragSelection() + { + AddStep("add circle", () => EditorBeatmap.Add(new HitCircle + { + Position = new Vector2(64, 64), + StartTime = EditorClock.CurrentTime, + })); + + AddStep("select circle tool", () => InputManager.Key(Key.Number2)); + AddStep("select grid tool", () => InputManager.Key(Key.Number5)); + AddStep("move cursor to centre", () => InputManager.MoveMouseTo(Editor)); + AddStep("click", () => InputManager.Click(MouseButton.Left)); + AddStep("move cursor to (-1, -1)", () => + { + var composer = Editor.ChildrenOfType().Single(); + InputManager.MoveMouseTo(composer.ToScreenSpace(new Vector2(-1, -1))); + }); + AddStep("drag to center", () => + { + InputManager.PressButton(MouseButton.Left); + InputManager.MoveMouseTo(Editor); + }); + AddStep("release left", () => InputManager.ReleaseButton(MouseButton.Left)); + + AddAssert("one selection", () => Editor.ChildrenOfType().Single().SelectedBlueprints, () => Has.One.Items); + AddAssert("selection is circle", () => Editor.ChildrenOfType().Single().SelectedBlueprints.Single(), Is.TypeOf); + + AddStep("move cursor to slider", () => + { + var composer = Editor.ChildrenOfType().Single(); + InputManager.MoveMouseTo(composer.ToScreenSpace(((Slider)EditorBeatmap.HitObjects.ElementAt(1)).EndPosition + new Vector2(1, 1))); + }); + AddStep("click", () => InputManager.Click(MouseButton.Left)); + + AddAssert("one selection", () => Editor.ChildrenOfType().Single().SelectedBlueprints, () => Has.One.Items); + AddAssert("selection is slider", () => Editor.ChildrenOfType().Single().SelectedBlueprints.Single(), Is.TypeOf); + } + + [Test] + public void TestGridPlacementRevertsToLastTool() + { + AddStep("select circle tool", () => InputManager.Key(Key.Number2)); + AddStep("select grid tool", () => InputManager.Key(Key.Number5)); + AddStep("move cursor to centre", () => InputManager.MoveMouseTo(Editor)); + AddStep("start grid placement", () => InputManager.Click(MouseButton.Left)); + AddStep("end grid placement", () => InputManager.Click(MouseButton.Left)); + AddAssert("tool reverted to circle", () => getComposer().BlueprintContainer.CurrentTool, Is.TypeOf); + + HitObjectComposer getComposer() => Editor.ChildrenOfType().Single(); + } + + [Test] + public void TestGridPlacementDoesNotOverrideToolChange() + { + AddStep("select circle tool", () => InputManager.Key(Key.Number2)); + AddStep("select grid tool", () => InputManager.Key(Key.Number5)); + AddStep("move cursor to centre", () => InputManager.MoveMouseTo(Editor)); + AddStep("start grid placement", () => InputManager.Click(MouseButton.Left)); + AddStep("select circle tool again", () => InputManager.Key(Key.Number2)); + AddAssert("circle tool selected", () => getComposer().BlueprintContainer.CurrentTool, Is.TypeOf); + + HitObjectComposer getComposer() => Editor.ChildrenOfType().Single(); + } } } From 6001d6418b7defeb0c9496aa103d36c5538ce974 Mon Sep 17 00:00:00 2001 From: Salman Alshamrani Date: Tue, 5 Aug 2025 14:22:38 +0300 Subject: [PATCH 2/3] Fix grid placement aggressively changing tool selection --- .../Edit/Blueprints/GridPlacementBlueprint.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Edit/Blueprints/GridPlacementBlueprint.cs b/osu.Game.Rulesets.Osu/Edit/Blueprints/GridPlacementBlueprint.cs index d9edc8dbd4..54e34f98ab 100644 --- a/osu.Game.Rulesets.Osu/Edit/Blueprints/GridPlacementBlueprint.cs +++ b/osu.Game.Rulesets.Osu/Edit/Blueprints/GridPlacementBlueprint.cs @@ -37,8 +37,9 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints base.EndPlacement(commit); // You typically only place the grid once, so we switch back to the last tool after placement. - if (commit && hitObjectComposer is OsuHitObjectComposer osuHitObjectComposer) - osuHitObjectComposer.SetLastTool(); + // This may be committed due to switching to another tool, we don't want to change the tool if so. + if (commit && hitObjectComposer?.BlueprintContainer.CurrentTool is GridFromPointsTool) + hitObjectComposer.SetLastTool(); } protected override bool OnClick(ClickEvent e) From f1f77842727fb50efbc6f103a2d34c8f546fd026 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 6 Aug 2025 15:03:38 +0200 Subject: [PATCH 3/3] Reword comment to be more coherent --- .../Edit/Blueprints/GridPlacementBlueprint.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Edit/Blueprints/GridPlacementBlueprint.cs b/osu.Game.Rulesets.Osu/Edit/Blueprints/GridPlacementBlueprint.cs index 54e34f98ab..f54dc2c85b 100644 --- a/osu.Game.Rulesets.Osu/Edit/Blueprints/GridPlacementBlueprint.cs +++ b/osu.Game.Rulesets.Osu/Edit/Blueprints/GridPlacementBlueprint.cs @@ -36,8 +36,8 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints base.EndPlacement(commit); - // You typically only place the grid once, so we switch back to the last tool after placement. - // This may be committed due to switching to another tool, we don't want to change the tool if so. + // You typically only place the grid once, so we switch back to the last tool after placement - + // but only if the tool hasn't changed from under us (which is possible, as external tool changes will commit any ongoing placements, including this one) if (commit && hitObjectComposer?.BlueprintContainer.CurrentTool is GridFromPointsTool) hitObjectComposer.SetLastTool(); }