Merge pull request #30411 from frenzibyte/editor-slider-touch-support-2

Fix placing objects via touch in editor not working sometimes

osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderDrawing.cs

@@ -8,6 +8,7 @@
 using osu.Framework.Input;
 using osu.Framework.Testing;
 using osu.Game.Beatmaps;
+using osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders;
 using osu.Game.Rulesets.Osu.Objects;
 using osu.Game.Rulesets.UI;
 using osu.Game.Screens.Edit.Components.RadioButtons;
@@ -24,38 +25,57 @@ public partial class TestSceneSliderDrawing : TestSceneOsuEditor
         protected override IBeatmap CreateBeatmap(RulesetInfo ruleset) => new TestBeatmap(ruleset, false);
 
         [Test]
-        public void TestTouchInputAfterTouchingComposeArea()
+        public void TestTouchInputPlaceHitCircleDirectly()
         {
             AddStep("tap circle", () => tap(this.ChildrenOfType<EditorRadioButton>().Single(b => b.Button.Label == "HitCircle")));
 
-            // this input is just for interacting with compose area
+            AddStep("tap to place circle", () => tap(this.ChildrenOfType<Playfield>().Single()));
-            AddStep("tap playfield", () => tap(this.ChildrenOfType<Playfield>().Single()));
-
-            AddStep("move current time", () => InputManager.Key(Key.Right));
-
-            AddStep("tap to place circle", () => tap(this.ChildrenOfType<Playfield>().Single().ToScreenSpace(new Vector2(10, 10))));
             AddAssert("circle placed correctly", () =>
             {
                 var circle = (HitCircle)EditorBeatmap.HitObjects.Single(h => h.StartTime == EditorClock.CurrentTimeAccurate);
                 Assert.Multiple(() =>
                 {
-                    Assert.That(circle.Position.X, Is.EqualTo(10f).Within(0.01f));
+                    Assert.That(circle.Position.X, Is.EqualTo(256f).Within(0.01f));
-                    Assert.That(circle.Position.Y, Is.EqualTo(10f).Within(0.01f));
+                    Assert.That(circle.Position.Y, Is.EqualTo(192f).Within(0.01f));
                 });
 
                 return true;
             });
+        }
 
-            AddStep("tap slider", () => tap(this.ChildrenOfType<EditorRadioButton>().Single(b => b.Button.Label == "Slider")));
+        [Test]
+        public void TestTouchInputPlaceCircleAfterTouchingComposeArea()
+        {
+            AddStep("tap circle", () => tap(this.ChildrenOfType<EditorRadioButton>().Single(b => b.Button.Label == "HitCircle")));
 
-            // this input is just for interacting with compose area
             AddStep("tap playfield", () => tap(this.ChildrenOfType<Playfield>().Single()));
+            AddAssert("circle placed", () => EditorBeatmap.HitObjects.Single(h => h.StartTime == EditorClock.CurrentTimeAccurate) is HitCircle);
 
-            AddStep("move current time", () => InputManager.Key(Key.Right));
+            AddStep("move forward", () => InputManager.Key(Key.Right));
+
+            AddStep("tap to place circle", () => tap(this.ChildrenOfType<Playfield>().Single()));
+            AddAssert("circle placed correctly", () =>
+            {
+                var circle = (HitCircle)EditorBeatmap.HitObjects.Single(h => h.StartTime == EditorClock.CurrentTimeAccurate);
+                Assert.Multiple(() =>
+                {
+                    Assert.That(circle.Position.X, Is.EqualTo(256f).Within(0.01f));
+                    Assert.That(circle.Position.Y, Is.EqualTo(192f).Within(0.01f));
+                });
+
+                return true;
+            });
+        }
+
+        [Test]
+        public void TestTouchInputPlaceSliderDirectly()
+        {
+            AddStep("tap slider", () => tap(this.ChildrenOfType<EditorRadioButton>().Single(b => b.Button.Label == "Slider")));
 
             AddStep("hold to draw slider", () => InputManager.BeginTouch(new Touch(TouchSource.Touch1, this.ChildrenOfType<Playfield>().Single().ToScreenSpace(new Vector2(50, 20)))));
             AddStep("drag to draw", () => InputManager.MoveTouchTo(new Touch(TouchSource.Touch1, this.ChildrenOfType<Playfield>().Single().ToScreenSpace(new Vector2(200, 50)))));
             AddAssert("selection not initiated", () => this.ChildrenOfType<DragBox>().All(d => d.State == Visibility.Hidden));
+            AddAssert("blueprint visible", () => this.ChildrenOfType<SliderPlacementBlueprint>().Single().Alpha > 0);
             AddStep("end", () => InputManager.EndTouch(new Touch(TouchSource.Touch1, InputManager.CurrentState.Touch.GetTouchPosition(TouchSource.Touch1)!.Value)));
             AddAssert("slider placed correctly", () =>
             {
@@ -76,12 +96,55 @@ public void TestTouchInputAfterTouchingComposeArea()
             });
         }
 
-        private void tap(Drawable drawable) => tap(drawable.ScreenSpaceDrawQuad.Centre);
+        [Test]
+        public void TestTouchInputPlaceSliderAfterTouchingComposeArea()
+        {
+            AddStep("tap slider", () => tap(this.ChildrenOfType<EditorRadioButton>().Single(b => b.Button.Label == "Slider")));
+
+            AddStep("tap playfield", () => tap(this.ChildrenOfType<Playfield>().Single()));
+            AddStep("tap and hold another spot", () => hold(this.ChildrenOfType<Playfield>().Single(), new Vector2(50, 0)));
+            AddUntilStep("wait for slider placement", () => EditorBeatmap.HitObjects.SingleOrDefault(h => h.StartTime == EditorClock.CurrentTimeAccurate) is Slider);
+            AddStep("end", () => InputManager.EndTouch(new Touch(TouchSource.Touch1, InputManager.CurrentState.Touch.GetTouchPosition(TouchSource.Touch1)!.Value)));
+
+            AddStep("move forward", () => InputManager.Key(Key.Right));
+
+            AddStep("hold to draw slider", () => InputManager.BeginTouch(new Touch(TouchSource.Touch1, this.ChildrenOfType<Playfield>().Single().ToScreenSpace(new Vector2(50, 20)))));
+            AddStep("drag to draw", () => InputManager.MoveTouchTo(new Touch(TouchSource.Touch1, this.ChildrenOfType<Playfield>().Single().ToScreenSpace(new Vector2(200, 50)))));
+            AddAssert("selection not initiated", () => this.ChildrenOfType<DragBox>().All(d => d.State == Visibility.Hidden));
+            AddAssert("blueprint visible", () => this.ChildrenOfType<SliderPlacementBlueprint>().Single().IsPresent);
+            AddStep("end", () => InputManager.EndTouch(new Touch(TouchSource.Touch1, InputManager.CurrentState.Touch.GetTouchPosition(TouchSource.Touch1)!.Value)));
+            AddAssert("slider placed correctly", () =>
+            {
+                var slider = (Slider)EditorBeatmap.HitObjects.Single(h => h.StartTime == EditorClock.CurrentTimeAccurate);
+                Assert.Multiple(() =>
+                {
+                    Assert.That(slider.Position.X, Is.EqualTo(50f).Within(0.01f));
+                    Assert.That(slider.Position.Y, Is.EqualTo(20f).Within(0.01f));
+                    Assert.That(slider.Path.ControlPoints.Count, Is.EqualTo(2));
+                    Assert.That(slider.Path.ControlPoints[0].Position, Is.EqualTo(Vector2.Zero));
+
+                    // the final position may be slightly off from the mouse position when drawing, account for that.
+                    Assert.That(slider.Path.ControlPoints[1].Position.X, Is.EqualTo(150).Within(5));
+                    Assert.That(slider.Path.ControlPoints[1].Position.Y, Is.EqualTo(30).Within(5));
+                });
+
+                return true;
+            });
+        }
+
+        private void tap(Drawable drawable, Vector2 offset = default) => tap(drawable.ToScreenSpace(drawable.LayoutRectangle.Centre + offset));
 
         private void tap(Vector2 position)
         {
-            InputManager.BeginTouch(new Touch(TouchSource.Touch1, position));
+            hold(position);
             InputManager.EndTouch(new Touch(TouchSource.Touch1, position));
         }
+
+        private void hold(Drawable drawable, Vector2 offset = default) => hold(drawable.ToScreenSpace(drawable.LayoutRectangle.Centre + offset));
+
+        private void hold(Vector2 position)
+        {
+            InputManager.BeginTouch(new Touch(TouchSource.Touch1, position));
+        }
     }
 }

osu.Game.Rulesets.Osu/Edit/Blueprints/GridPlacementBlueprint.cs

@@ -2,6 +2,7 @@
 // See the LICENCE file in the repository root for full licence text.
 
 using osu.Framework.Allocation;
+using osu.Framework.Graphics.Containers;
 using osu.Framework.Input.Events;
 using osu.Game.Rulesets.Edit;
 using osuTK;
@@ -31,12 +32,7 @@ private void load(OsuGridToolboxGroup gridToolboxGroup)
         public override void EndPlacement(bool commit)
         {
             if (!commit && PlacementActive != PlacementState.Finished)
-            {
+                resetGridState();
-                gridToolboxGroup.StartPosition.Value = originalOrigin;
-                gridToolboxGroup.Spacing.Value = originalSpacing;
-                if (!gridToolboxGroup.GridLinesRotation.Disabled)
-                    gridToolboxGroup.GridLinesRotation.Value = originalRotation;
-            }
 
             base.EndPlacement(commit);
 
@@ -103,6 +99,9 @@ protected override void OnDragEnd(DragEndEvent e)
 
         public override void UpdateTimeAndPosition(SnapResult result)
         {
+            if (State.Value == Visibility.Hidden)
+                return;
+
             var pos = ToLocalSpace(result.ScreenSpacePosition);
 
             if (PlacementActive != PlacementState.Active)
@@ -122,5 +121,19 @@ public override void UpdateTimeAndPosition(SnapResult result)
                 }
             }
         }
+
+        protected override void PopOut()
+        {
+            base.PopOut();
+            resetGridState();
+        }
+
+        private void resetGridState()
+        {
+            gridToolboxGroup.StartPosition.Value = originalOrigin;
+            gridToolboxGroup.Spacing.Value = originalSpacing;
+            if (!gridToolboxGroup.GridLinesRotation.Disabled)
+                gridToolboxGroup.GridLinesRotation.Value = originalRotation;
+        }
     }
 }

osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/SliderPlacementBlueprint.cs

@@ -156,6 +156,13 @@ protected override bool OnMouseDown(MouseDownEvent e)
             return true;
         }
 
+        // this allows sliders to be drawn outside compose area (after starting from a point within the compose area).
+        public override bool ReceivePositionalInputAt(Vector2 screenSpacePos) => base.ReceivePositionalInputAt(screenSpacePos) || PlacementActive == PlacementState.Active;
+
+        // ReceivePositionalInputAtSubTree generally always returns true when masking is disabled, but we don't want that,
+        // otherwise a slider path tooltip will be displayed anywhere in the editor (outside compose area).
+        protected override bool ReceivePositionalInputAtSubTree(Vector2 screenSpacePos) => ReceivePositionalInputAt(screenSpacePos);
+
         private void beginNewSegment(PathControlPoint lastPoint)
         {
             segmentStart = lastPoint;

osu.Game.Tests/Visual/Editing/TestSceneHitObjectComposer.cs

@@ -19,6 +19,7 @@
 using osu.Game.Rulesets.Osu;
 using osu.Game.Rulesets.Osu.Edit;
 using osu.Game.Rulesets.Osu.Objects;
+using osu.Game.Rulesets.UI;
 using osu.Game.Screens.Edit;
 using osu.Game.Screens.Edit.Components.RadioButtons;
 using osu.Game.Screens.Edit.Components.TernaryButtons;
@@ -82,6 +83,45 @@ public void SetUpSteps()
             });
         }
 
+        [Test]
+        public void TestPlacementOutsideComposeScreen()
+        {
+            AddStep("clear all control points and hitobjects", () =>
+            {
+                editorBeatmap.ControlPointInfo.Clear();
+                editorBeatmap.Clear();
+            });
+
+            AddStep("Add timing point", () => editorBeatmap.ControlPointInfo.Add(0, new TimingControlPoint()));
+            AddStep("select circle", () => hitObjectComposer.ChildrenOfType<EditorRadioButton>().First(d => d.Button.Label == "HitCircle").TriggerClick());
+            AddStep("move mouse to compose", () => InputManager.MoveMouseTo(hitObjectComposer.ChildrenOfType<HitObjectContainer>().Single()));
+            AddStep("click", () => InputManager.Click(MouseButton.Left));
+            AddAssert("circle placed", () => editorBeatmap.HitObjects.Count == 1);
+
+            AddStep("move mouse outside compose", () => InputManager.MoveMouseTo(hitObjectComposer.ChildrenOfType<HitObjectContainer>().Single().ScreenSpaceDrawQuad.TopLeft - new Vector2(0f, 20f)));
+            AddStep("click", () => InputManager.Click(MouseButton.Left));
+            AddAssert("no circle placed", () => editorBeatmap.HitObjects.Count == 1);
+        }
+
+        [Test]
+        public void TestDragSliderOutsideComposeScreen()
+        {
+            AddStep("clear all control points and hitobjects", () =>
+            {
+                editorBeatmap.ControlPointInfo.Clear();
+                editorBeatmap.Clear();
+            });
+
+            AddStep("Add timing point", () => editorBeatmap.ControlPointInfo.Add(0, new TimingControlPoint()));
+            AddStep("select slider", () => hitObjectComposer.ChildrenOfType<EditorRadioButton>().First(d => d.Button.Label == "Slider").TriggerClick());
+
+            AddStep("move mouse to compose", () => InputManager.MoveMouseTo(hitObjectComposer.ChildrenOfType<HitObjectContainer>().Single()));
+            AddStep("hold", () => InputManager.PressButton(MouseButton.Left));
+            AddStep("move mouse outside compose", () => InputManager.MoveMouseTo(hitObjectComposer.ChildrenOfType<HitObjectContainer>().Single().ScreenSpaceDrawQuad.TopLeft - new Vector2(0f, 80f)));
+            AddStep("release", () => InputManager.ReleaseButton(MouseButton.Left));
+            AddAssert("slider placed", () => editorBeatmap.HitObjects.Count == 1);
+        }
+
         [Test]
         public void TestPlacementOnlyWorksWithTiming()
         {

osu.Game/Rulesets/Edit/HitObjectComposer.cs

@@ -537,22 +537,23 @@ private void toolSelected(CompositionTool tool)
 
         #region IPlacementHandler
 
-        public void BeginPlacement(HitObject hitObject)
+        public void ShowPlacement(HitObject hitObject)
         {
             EditorBeatmap.PlacementObject.Value = hitObject;
         }
 
-        public void EndPlacement(HitObject hitObject, bool commit)
+        public void HidePlacement()
         {
             EditorBeatmap.PlacementObject.Value = null;
+        }
 
-            if (commit)
+        public void CommitPlacement(HitObject hitObject)
-            {
+        {
-                EditorBeatmap.Add(hitObject);
+            EditorBeatmap.PlacementObject.Value = null;
+            EditorBeatmap.Add(hitObject);
 
-                if (autoSeekOnPlacement.Value && EditorClock.CurrentTime < hitObject.StartTime)
+            if (autoSeekOnPlacement.Value && EditorClock.CurrentTime < hitObject.StartTime)
-                    EditorClock.SeekSmoothlyTo(hitObject.StartTime);
+                EditorClock.SeekSmoothlyTo(hitObject.StartTime);
-            }
         }
 
         public void Delete(HitObject hitObject) => EditorBeatmap.Remove(hitObject);

osu.Game/Rulesets/Edit/HitObjectPlacementBlueprint.cs

@@ -5,6 +5,7 @@
 using System.Threading;
 using osu.Framework.Allocation;
 using osu.Framework.Bindables;
+using osu.Framework.Graphics.Containers;
 using osu.Game.Audio;
 using osu.Game.Beatmaps;
 using osu.Game.Beatmaps.ControlPoints;
@@ -63,18 +64,26 @@ private void load()
             startTimeBindable.BindValueChanged(_ => ApplyDefaultsToHitObject(), true);
         }
 
+        private bool placementBegun;
+
         protected override void BeginPlacement(bool commitStart = false)
         {
             base.BeginPlacement(commitStart);
 
-            placementHandler.BeginPlacement(HitObject);
+            if (State.Value == Visibility.Visible)
+                placementHandler.ShowPlacement(HitObject);
+
+            placementBegun = true;
         }
 
         public override void EndPlacement(bool commit)
         {
             base.EndPlacement(commit);
 
-            placementHandler.EndPlacement(HitObject, IsValidForPlacement && commit);
+            if (IsValidForPlacement && commit)
+                placementHandler.CommitPlacement(HitObject);
+            else
+                placementHandler.HidePlacement();
         }
 
         /// <summary>
@@ -127,5 +136,19 @@ public override void UpdateTimeAndPosition(SnapResult result)
         /// refreshing <see cref="Objects.HitObject.NestedHitObjects"/> and parameters for the <see cref="HitObject"/>.
         /// </summary>
         protected void ApplyDefaultsToHitObject() => HitObject.ApplyDefaults(beatmap.ControlPointInfo, beatmap.Difficulty);
+
+        protected override void PopIn()
+        {
+            base.PopIn();
+
+            if (placementBegun)
+                placementHandler.ShowPlacement(HitObject);
+        }
+
+        protected override void PopOut()
+        {
+            base.PopOut();
+            placementHandler.HidePlacement();
+        }
     }
 }

osu.Game/Rulesets/Edit/PlacementBlueprint.cs

@@ -7,7 +7,6 @@
 using osu.Framework.Input.Events;
 using osu.Game.Input.Bindings;
 using osu.Game.Rulesets.Objects;
-using osuTK;
 using osuTK.Input;
 
 namespace osu.Game.Rulesets.Edit
@@ -15,7 +14,7 @@ namespace osu.Game.Rulesets.Edit
     /// <summary>
     /// A blueprint which governs the placement of something.
     /// </summary>
-    public abstract partial class PlacementBlueprint : CompositeDrawable, IKeyBindingHandler<GlobalAction>
+    public abstract partial class PlacementBlueprint : VisibilityContainer, IKeyBindingHandler<GlobalAction>
     {
         /// <summary>
         /// Whether the <see cref="HitObject"/> is currently mid-placement, but has not necessarily finished being placed.
@@ -31,12 +30,17 @@ public abstract partial class PlacementBlueprint : CompositeDrawable, IKeyBindin
         /// </remarks>
         protected virtual bool IsValidForPlacement => true;
 
+        // the blueprint should still be considered for input even if it is hidden,
+        // especially when such input is the reason for making the blueprint become visible.
+        public override bool PropagatePositionalInputSubTree => true;
+        public override bool PropagateNonPositionalInputSubTree => true;
+
         protected PlacementBlueprint()
         {
             RelativeSizeAxes = Axes.Both;
 
-            // This is required to allow the blueprint's position to be updated via OnMouseMove/Handle
+            // the blueprint should still be considered for input even if it is hidden,
-            // on the same frame it is made visible via a PlacementState change.
+            // especially when such input is the reason for making the blueprint become visible.
             AlwaysPresent = true;
         }
 
@@ -104,8 +108,6 @@ public void OnReleased(KeyBindingReleaseEvent<GlobalAction> e)
         {
         }
 
-        public override bool ReceivePositionalInputAt(Vector2 screenSpacePos) => true;
-
         protected override bool Handle(UIEvent e)
         {
             base.Handle(e);
@@ -127,6 +129,9 @@ protected override bool Handle(UIEvent e)
             }
         }
 
+        protected override void PopIn() => this.FadeIn();
+        protected override void PopOut() => this.FadeOut();
+
         public enum PlacementState
         {
             Waiting,

osu.Game/Screens/Edit/Compose/Components/ComposeBlueprintContainer.cs

@@ -316,9 +316,12 @@ private void updateAdditionBankTernaryButtonTooltips()
         /// <summary>
         /// Refreshes the current placement tool.
         /// </summary>
-        private void refreshTool()
+        private void refreshPlacement()
         {
-            removePlacement();
+            CurrentPlacement?.EndPlacement(false);
+            CurrentPlacement?.Expire();
+            CurrentPlacement = null;
+
             ensurePlacementCreated();
         }
 
@@ -344,21 +347,24 @@ protected override void Update()
                 {
                     case PlacementBlueprint.PlacementState.Waiting:
                         if (!Composer.CursorInPlacementArea)
-                            removePlacement();
+                            CurrentPlacement.Hide();
+                        else
+                            CurrentPlacement.Show();
+
+                        break;
+
+                    case PlacementBlueprint.PlacementState.Active:
+                        CurrentPlacement.Show();
                         break;
 
                     case PlacementBlueprint.PlacementState.Finished:
-                        removePlacement();
+                        refreshPlacement();
                         break;
                 }
-            }
 
-            if (Composer.CursorInPlacementArea)
+                // updates the placement with the latest editor clock time.
-                ensurePlacementCreated();
-
-            // updates the placement with the latest editor clock time.
-            if (CurrentPlacement != null)
                 updatePlacementTimeAndPosition();
+            }
         }
 
         protected override bool OnMouseMove(MouseMoveEvent e)
@@ -386,7 +392,7 @@ protected sealed override SelectionBlueprint<HitObject> CreateBlueprintFor(HitOb
         private void hitObjectAdded(HitObject obj)
         {
             // refresh the tool to handle the case of placement completing.
-            refreshTool();
+            refreshPlacement();
 
             // on successful placement, the new combo button should be reset as this is the most common user interaction.
             if (Beatmap.SelectedHitObjects.Count == 0)
@@ -415,14 +421,7 @@ private void ensurePlacementCreated()
         public void CommitIfPlacementActive()
         {
             CurrentPlacement?.EndPlacement(CurrentPlacement.PlacementActive == PlacementBlueprint.PlacementState.Active);
-            removePlacement();
+            refreshPlacement();
-        }
-
-        private void removePlacement()
-        {
-            CurrentPlacement?.EndPlacement(false);
-            CurrentPlacement?.Expire();
-            CurrentPlacement = null;
         }
 
         private CompositionTool currentTool;

osu.Game/Screens/Edit/Compose/IPlacementHandler.cs

@@ -10,17 +10,21 @@ namespace osu.Game.Screens.Edit.Compose
     public interface IPlacementHandler
     {
         /// <summary>
-        /// Notifies that a placement has begun.
+        /// Notifies that a placement blueprint became visible on the screen.
         /// </summary>
-        /// <param name="hitObject">The <see cref="HitObject"/> being placed.</param>
+        /// <param name="hitObject">The <see cref="HitObject"/> representing the placement.</param>
-        void BeginPlacement(HitObject hitObject);
+        void ShowPlacement(HitObject hitObject);
 
         /// <summary>
-        /// Notifies that a placement has finished.
+        /// Notifies that a visible placement blueprint has been hidden.
+        /// </summary>
+        void HidePlacement();
+
+        /// <summary>
+        /// Notifies that a placement has been committed.
         /// </summary>
         /// <param name="hitObject">The <see cref="HitObject"/> that has been placed.</param>
-        /// <param name="commit">Whether the object should be committed.</param>
+        void CommitPlacement(HitObject hitObject);
-        void EndPlacement(HitObject hitObject, bool commit);
 
         /// <summary>
         /// Deletes a <see cref="HitObject"/>.

osu.Game/Tests/Visual/PlacementBlueprintTestScene.cs

@@ -59,20 +59,20 @@ protected virtual IBeatmap GetPlayableBeatmap()
         protected override void LoadComplete()
         {
             base.LoadComplete();
-
             ResetPlacement();
         }
 
-        public void BeginPlacement(HitObject hitObject)
+        public void ShowPlacement(HitObject hitObject)
         {
         }
 
-        public void EndPlacement(HitObject hitObject, bool commit)
+        public void HidePlacement()
         {
-            if (commit)
+        }
-                AddHitObject(CreateHitObject(hitObject));
 
-            ResetPlacement();
+        public void CommitPlacement(HitObject hitObject)
+        {
+            AddHitObject(CreateHitObject(hitObject));
         }
 
         protected void ResetPlacement()
@@ -89,6 +89,10 @@ public void Delete(HitObject hitObject)
         protected override void Update()
         {
             base.Update();
+
+            if (CurrentBlueprint.PlacementActive == PlacementBlueprint.PlacementState.Finished)
+                ResetPlacement();
+
             updatePlacementTimeAndPosition();
         }