From b9a66ad7b346aa26d49a875a522deb8932b8dca9 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 19 Jul 2023 14:58:45 +0900 Subject: [PATCH 1/4] Add test coverage of incorrect selection behaviour --- .../Editor/TestSceneOsuComposerSelection.cs | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneOsuComposerSelection.cs b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneOsuComposerSelection.cs index 8641663ce859..623cefff6bcd 100644 --- a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneOsuComposerSelection.cs +++ b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneOsuComposerSelection.cs @@ -25,6 +25,35 @@ public partial class TestSceneOsuComposerSelection : TestSceneOsuEditor { protected override IBeatmap CreateBeatmap(RulesetInfo ruleset) => new TestBeatmap(ruleset, false); + [Test] + public void TestSelectAfterFadedOut() + { + var slider = new Slider + { + StartTime = 0, + Position = new Vector2(100, 100), + Path = new SliderPath + { + ControlPoints = + { + new PathControlPoint(), + new PathControlPoint(new Vector2(100)) + } + } + }; + AddStep("add slider", () => EditorBeatmap.Add(slider)); + + moveMouseToObject(() => slider); + + AddStep("seek after end", () => EditorClock.Seek(750)); + AddStep("left click", () => InputManager.Click(MouseButton.Left)); + AddAssert("slider not selected", () => EditorBeatmap.SelectedHitObjects.Count == 0); + + AddStep("seek to visible", () => EditorClock.Seek(650)); + AddStep("left click", () => InputManager.Click(MouseButton.Left)); + AddUntilStep("slider selected", () => EditorBeatmap.SelectedHitObjects.Single() == slider); + } + [Test] public void TestContextMenuShownCorrectlyForSelectedSlider() { From 4a6a5b174a3255b8479edb2ab4683fd779040925 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 19 Jul 2023 14:52:31 +0900 Subject: [PATCH 2/4] Fix editor blueprints being selectable for too long when hit markers are enabled Addresses https://github.com/ppy/osu/discussions/24163. --- .../Edit/Blueprints/OsuSelectionBlueprint.cs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/osu.Game.Rulesets.Osu/Edit/Blueprints/OsuSelectionBlueprint.cs b/osu.Game.Rulesets.Osu/Edit/Blueprints/OsuSelectionBlueprint.cs index bdd19f91065c..178b809d8bec 100644 --- a/osu.Game.Rulesets.Osu/Edit/Blueprints/OsuSelectionBlueprint.cs +++ b/osu.Game.Rulesets.Osu/Edit/Blueprints/OsuSelectionBlueprint.cs @@ -22,7 +22,16 @@ public abstract partial class OsuSelectionBlueprint : HitObjectSelectionBluep protected override bool AlwaysShowWhenSelected => true; protected override bool ShouldBeAlive => base.ShouldBeAlive - || (DrawableObject is not DrawableSpinner && ShowHitMarkers.Value && editorClock.CurrentTime >= Item.StartTime && editorClock.CurrentTime - Item.GetEndTime() < HitCircleOverlapMarker.FADE_OUT_EXTENSION); + || (DrawableObject is not DrawableSpinner && ShowHitMarkers.Value && editorClock.CurrentTime >= Item.StartTime + && editorClock.CurrentTime - Item.GetEndTime() < HitCircleOverlapMarker.FADE_OUT_EXTENSION); + + public override bool HandlePositionalInput => + // Bypass fade out extension from hit markers for input handling purposes. + // This is to match stable, where even when the afterimage hit markers are still visible, objects are not selectable. + // + // Note that we are intentionally overriding HandlePositionalInput here and not ReceivePositionalInputAt + // as individual blueprint implementations override that. + base.ShouldBeAlive; protected OsuSelectionBlueprint(T hitObject) : base(hitObject) From eac6271bd011359afbe5658e664427d09479c44c Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 25 Jul 2023 18:13:35 +0900 Subject: [PATCH 3/4] Add new property to avoid overlapping usages --- .../Edit/Blueprints/OsuSelectionBlueprint.cs | 7 ++----- osu.Game/Rulesets/Edit/SelectionBlueprint.cs | 7 ++++++- .../Screens/Edit/Compose/Components/BlueprintContainer.cs | 2 +- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Edit/Blueprints/OsuSelectionBlueprint.cs b/osu.Game.Rulesets.Osu/Edit/Blueprints/OsuSelectionBlueprint.cs index 178b809d8bec..8e8972a665d2 100644 --- a/osu.Game.Rulesets.Osu/Edit/Blueprints/OsuSelectionBlueprint.cs +++ b/osu.Game.Rulesets.Osu/Edit/Blueprints/OsuSelectionBlueprint.cs @@ -25,12 +25,9 @@ public abstract partial class OsuSelectionBlueprint : HitObjectSelectionBluep || (DrawableObject is not DrawableSpinner && ShowHitMarkers.Value && editorClock.CurrentTime >= Item.StartTime && editorClock.CurrentTime - Item.GetEndTime() < HitCircleOverlapMarker.FADE_OUT_EXTENSION); - public override bool HandlePositionalInput => - // Bypass fade out extension from hit markers for input handling purposes. + public override bool IsSelectable => + // Bypass fade out extension from hit markers for selection purposes. // This is to match stable, where even when the afterimage hit markers are still visible, objects are not selectable. - // - // Note that we are intentionally overriding HandlePositionalInput here and not ReceivePositionalInputAt - // as individual blueprint implementations override that. base.ShouldBeAlive; protected OsuSelectionBlueprint(T hitObject) diff --git a/osu.Game/Rulesets/Edit/SelectionBlueprint.cs b/osu.Game/Rulesets/Edit/SelectionBlueprint.cs index 3c878ffd3366..158c3c102cea 100644 --- a/osu.Game/Rulesets/Edit/SelectionBlueprint.cs +++ b/osu.Game/Rulesets/Edit/SelectionBlueprint.cs @@ -32,7 +32,7 @@ public abstract partial class SelectionBlueprint : CompositeDrawable, IStatef /// public event Action> Deselected; - public override bool HandlePositionalInput => ShouldBeAlive; + public override bool HandlePositionalInput => IsSelectable; public override bool RemoveWhenNotAlive => false; protected SelectionBlueprint(T item) @@ -125,6 +125,11 @@ protected virtual void OnSelected() /// public virtual MenuItem[] ContextMenuItems => Array.Empty(); + /// + /// Whether the can be currently selected via a click or a drag box. + /// + public virtual bool IsSelectable => ShouldBeAlive && IsPresent; + /// /// The screen-space main point that causes this to be selected via a drag. /// diff --git a/osu.Game/Screens/Edit/Compose/Components/BlueprintContainer.cs b/osu.Game/Screens/Edit/Compose/Components/BlueprintContainer.cs index 2ee162bf3b28..1de6c8364ced 100644 --- a/osu.Game/Screens/Edit/Compose/Components/BlueprintContainer.cs +++ b/osu.Game/Screens/Edit/Compose/Components/BlueprintContainer.cs @@ -487,7 +487,7 @@ protected virtual void UpdateSelectionFromDragBox() break; case SelectionState.NotSelected: - if (blueprint.IsAlive && blueprint.IsPresent && quad.Contains(blueprint.ScreenSpaceSelectionPoint)) + if (blueprint.IsSelectable && quad.Contains(blueprint.ScreenSpaceSelectionPoint)) blueprint.Select(); break; } From d75e6f78d6bc6f6f549591da37974b547952105c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Tue, 25 Jul 2023 18:54:13 +0200 Subject: [PATCH 4/4] Adjust cyclic selection test to hitcircle lifetime adjustments As it turns out, the tightening of hitcircle lifetimes in editor caused the test to fail, since the objects were too far apart and at the starting time of the test, the first object was fully faded out and as such not alive, therefore leading cyclic selection to fail to select it. To fix, bring all three objects closer together time-wise so that this does not happen and the test can continue to exercise its original behaviour. --- osu.Game.Tests/Visual/Editing/TestSceneComposerSelection.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/osu.Game.Tests/Visual/Editing/TestSceneComposerSelection.cs b/osu.Game.Tests/Visual/Editing/TestSceneComposerSelection.cs index b1a7ef976a90..3884a3108fee 100644 --- a/osu.Game.Tests/Visual/Editing/TestSceneComposerSelection.cs +++ b/osu.Game.Tests/Visual/Editing/TestSceneComposerSelection.cs @@ -340,14 +340,14 @@ public void TestCyclicSelectionOutwards() public void TestCyclicSelectionBackwards() { var firstObject = new HitCircle { Position = new Vector2(256, 192), StartTime = 0 }; - var secondObject = new HitCircle { Position = new Vector2(256, 192), StartTime = 300 }; - var thirdObject = new HitCircle { Position = new Vector2(256, 192), StartTime = 600 }; + var secondObject = new HitCircle { Position = new Vector2(256, 192), StartTime = 200 }; + var thirdObject = new HitCircle { Position = new Vector2(256, 192), StartTime = 400 }; AddStep("add hitobjects", () => EditorBeatmap.AddRange(new[] { firstObject, secondObject, thirdObject })); moveMouseToObject(() => firstObject); - AddStep("seek to third", () => EditorClock.Seek(600)); + AddStep("seek to third", () => EditorClock.Seek(350)); AddStep("left click", () => InputManager.Click(MouseButton.Left)); AddAssert("third selected", () => EditorBeatmap.SelectedHitObjects.Single(), () => Is.EqualTo(thirdObject));