Merge pull request #12437 from peppy/fix-editor-placement-commit-false

Fix placement blueprints not being correctly removed after a rolled back placement
ppy · Apr 16, 2021 · d3acbcc · d3acbcc
2 parents 1ebd997 + 119c9b4
commit d3acbcc
Show file tree

Hide file tree

Showing 6 changed files with 54 additions and 26 deletions.
diff --git a/osu.Game.Rulesets.Mania/Edit/Blueprints/HoldNotePlacementBlueprint.cs b/osu.Game.Rulesets.Mania/Edit/Blueprints/HoldNotePlacementBlueprint.cs
@@ -82,7 +82,7 @@ public override void UpdateTimeAndPosition(SnapResult result)
         {
             base.UpdateTimeAndPosition(result);
 
-            if (PlacementActive)
+            if (PlacementActive == PlacementState.Active)
             {
                 if (result.Time is double endTime)
                 {

diff --git a/osu.Game.Rulesets.Mania/Edit/Blueprints/ManiaPlacementBlueprint.cs b/osu.Game.Rulesets.Mania/Edit/Blueprints/ManiaPlacementBlueprint.cs
@@ -52,7 +52,7 @@ public override void UpdateTimeAndPosition(SnapResult result)
         {
             base.UpdateTimeAndPosition(result);
 
-            if (!PlacementActive)
+            if (PlacementActive == PlacementState.Waiting)
                 Column = result.Playfield as Column;
         }
     }

diff --git a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/SliderPlacementBlueprint.cs b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/SliderPlacementBlueprint.cs
@@ -30,7 +30,7 @@ public class SliderPlacementBlueprint : PlacementBlueprint
 
         private InputManager inputManager;
 
-        private PlacementState state;
+        private SliderPlacementState state;
         private PathControlPoint segmentStart;
         private PathControlPoint cursor;
         private int currentSegmentLength;
@@ -58,7 +58,7 @@ private void load(OsuColour colours)
                 controlPointVisualiser = new PathControlPointVisualiser(HitObject, false)
             };
 
-            setState(PlacementState.Initial);
+            setState(SliderPlacementState.Initial);
         }
 
         protected override void LoadComplete()
@@ -73,12 +73,12 @@ public override void UpdateTimeAndPosition(SnapResult result)
 
             switch (state)
             {
-                case PlacementState.Initial:
+                case SliderPlacementState.Initial:
                     BeginPlacement();
                     HitObject.Position = ToLocalSpace(result.ScreenSpacePosition);
                     break;
 
-                case PlacementState.Body:
+                case SliderPlacementState.Body:
                     updateCursor();
                     break;
             }
@@ -91,11 +91,11 @@ protected override bool OnMouseDown(MouseDownEvent e)
 
             switch (state)
             {
-                case PlacementState.Initial:
+                case SliderPlacementState.Initial:
                     beginCurve();
                     break;
 
-                case PlacementState.Body:
+                case SliderPlacementState.Body:
                     if (canPlaceNewControlPoint(out var lastPoint))
                     {
                         // Place a new point by detatching the current cursor.
@@ -121,15 +121,15 @@ protected override bool OnMouseDown(MouseDownEvent e)
 
         protected override void OnMouseUp(MouseUpEvent e)
         {
-            if (state == PlacementState.Body && e.Button == MouseButton.Right)
+            if (state == SliderPlacementState.Body && e.Button == MouseButton.Right)
                 endCurve();
             base.OnMouseUp(e);
         }
 
         private void beginCurve()
         {
             BeginPlacement(commitStart: true);
-            setState(PlacementState.Body);
+            setState(SliderPlacementState.Body);
         }
 
         private void endCurve()
@@ -219,12 +219,12 @@ private void updateSlider()
             tailCirclePiece.UpdateFrom(HitObject.TailCircle);
         }
 
-        private void setState(PlacementState newState)
+        private void setState(SliderPlacementState newState)
         {
             state = newState;
         }
 
-        private enum PlacementState
+        private enum SliderPlacementState
         {
             Initial,
             Body,

diff --git a/osu.Game.Rulesets.Taiko/Edit/Blueprints/TaikoSpanPlacementBlueprint.cs b/osu.Game.Rulesets.Taiko/Edit/Blueprints/TaikoSpanPlacementBlueprint.cs
@@ -72,7 +72,7 @@ public override void UpdateTimeAndPosition(SnapResult result)
         {
             base.UpdateTimeAndPosition(result);
 
-            if (PlacementActive)
+            if (PlacementActive == PlacementState.Active)
             {
                 if (result.Time is double dragTime)
                 {

diff --git a/osu.Game/Rulesets/Edit/PlacementBlueprint.cs b/osu.Game/Rulesets/Edit/PlacementBlueprint.cs
@@ -25,7 +25,7 @@ public abstract class PlacementBlueprint : CompositeDrawable
         /// <summary>
         /// Whether the <see cref="HitObject"/> is currently mid-placement, but has not necessarily finished being placed.
         /// </summary>
-        public bool PlacementActive { get; private set; }
+        public PlacementState PlacementActive { get; private set; }
 
         /// <summary>
         /// The <see cref="HitObject"/> that is being placed.
@@ -72,7 +72,8 @@ private void load(IBindable<WorkingBeatmap> beatmap)
         protected void BeginPlacement(bool commitStart = false)
         {
             placementHandler.BeginPlacement(HitObject);
-            PlacementActive |= commitStart;
+            if (commitStart)
+                PlacementActive = PlacementState.Active;
         }
 
         /// <summary>
@@ -82,10 +83,19 @@ protected void BeginPlacement(bool commitStart = false)
         /// <param name="commit">Whether the object should be committed.</param>
         public void EndPlacement(bool commit)
         {
-            if (!PlacementActive)
-                BeginPlacement();
+            switch (PlacementActive)
+            {
+                case PlacementState.Finished:
+                    return;
+
+                case PlacementState.Waiting:
+                    // ensure placement was started before ending to make state handling simpler.
+                    BeginPlacement();
+                    break;
+            }
+
             placementHandler.EndPlacement(HitObject, commit);
-            PlacementActive = false;
+            PlacementActive = PlacementState.Finished;
         }
 
         /// <summary>
@@ -94,7 +104,7 @@ public void EndPlacement(bool commit)
         /// <param name="result">The snap result information.</param>
         public virtual void UpdateTimeAndPosition(SnapResult result)
         {
-            if (!PlacementActive)
+            if (PlacementActive == PlacementState.Waiting)
                 HitObject.StartTime = result.Time ?? EditorClock?.CurrentTime ?? Time.Current;
         }
 
@@ -125,5 +135,12 @@ protected override bool Handle(UIEvent e)
                     return false;
             }
         }
+
+        public enum PlacementState
+        {
+            Waiting,
+            Active,
+            Finished
+        }
     }
 }
diff --git a/osu.Game/Screens/Edit/Compose/Components/ComposeBlueprintContainer.cs b/osu.Game/Screens/Edit/Compose/Components/ComposeBlueprintContainer.cs
@@ -196,7 +196,7 @@ private Drawable getIconForSample(string sampleName)
         private void refreshTool()
         {
             removePlacement();
-            createPlacement();
+            ensurePlacementCreated();
         }
 
         private void updatePlacementPosition()
@@ -215,15 +215,26 @@ protected override void Update()
         {
             base.Update();
 
+            if (currentPlacement != null)
+            {
+                switch (currentPlacement.PlacementActive)
+                {
+                    case PlacementBlueprint.PlacementState.Waiting:
+                        if (!Composer.CursorInPlacementArea)
+                            removePlacement();
+                        break;
+
+                    case PlacementBlueprint.PlacementState.Finished:
+                        removePlacement();
+                        break;
+                }
+            }
+
             if (Composer.CursorInPlacementArea)
-                createPlacement();
-            else if (currentPlacement?.PlacementActive == false)
-                removePlacement();
+                ensurePlacementCreated();
 
             if (currentPlacement != null)
-            {
                 updatePlacementPosition();
-            }
         }
 
         protected sealed override SelectionBlueprint CreateBlueprintFor(HitObject hitObject)
@@ -249,7 +260,7 @@ protected override void OnBlueprintAdded(HitObject hitObject)
                 NewCombo.Value = TernaryState.False;
         }
 
-        private void createPlacement()
+        private void ensurePlacementCreated()
         {
             if (currentPlacement != null) return;