Merge pull request #28478 from bdach/timeline-drag-selection-crash

Fix crash on drag selection via timeline while placement is active

osu.Game.Rulesets.Mania.Tests/Editor/TestSceneManiaBeatSnapGrid.cs

@@ -1,6 +1,7 @@
 // Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
 // See the LICENCE file in the repository root for full licence text.
 
+using System;
 using System.Collections.Generic;
 using System.Linq;
 using osu.Framework.Allocation;
@@ -17,6 +18,7 @@ using osu.Game.Rulesets.Objects.Drawables;
 using osu.Game.Rulesets.UI;
 using osu.Game.Rulesets.UI.Scrolling;
 using osu.Game.Screens.Edit;
+using osu.Game.Screens.Edit.Compose.Components;
 using osu.Game.Tests.Visual;
 using osuTK;
 
@@ -84,6 +86,7 @@ namespace osu.Game.Rulesets.Mania.Tests.Editor
     public partial class TestHitObjectComposer : HitObjectComposer
     {
         public override Playfield Playfield { get; }
+        public override ComposeBlueprintContainer BlueprintContainer => throw new NotImplementedException();
         public override IEnumerable<DrawableHitObject> HitObjects => Enumerable.Empty<DrawableHitObject>();
         public override bool CursorInPlacementArea => false;
 
@@ -100,7 +103,7 @@ namespace osu.Game.Rulesets.Mania.Tests.Editor
 
         public override SnapResult FindSnappedPositionAndTime(Vector2 screenSpacePosition, SnapType snapType = SnapType.All)
         {
-            throw new System.NotImplementedException();
+            throw new NotImplementedException();
         }
     }
 }

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

@@ -14,6 +14,7 @@ using osu.Game.Graphics.UserInterface;
 using osu.Game.Rulesets;
 using osu.Game.Rulesets.Objects;
 using osu.Game.Rulesets.Osu;
+using osu.Game.Rulesets.Osu.Edit;
 using osu.Game.Rulesets.Osu.Objects;
 using osu.Game.Screens.Edit.Compose.Components.Timeline;
 using osu.Game.Tests.Beatmaps;
@@ -357,6 +358,51 @@ namespace osu.Game.Tests.Visual.Editing
             AddAssert("all blueprints are present", () => blueprintContainer.SelectionBlueprints.Count == EditorBeatmap.SelectedHitObjects.Count);
         }
 
+        [Test]
+        public void TestDragSelectionDuringPlacement()
+        {
+            var addedObjects = new[]
+            {
+                new Slider
+                {
+                    StartTime = 300,
+                    Path = new SliderPath([
+                        new PathControlPoint(),
+                        new PathControlPoint(new Vector2(200)),
+                    ])
+                },
+            };
+            AddStep("add hitobjects", () => EditorBeatmap.AddRange(addedObjects));
+
+            AddStep("seek to 700", () => EditorClock.Seek(700));
+            AddStep("select spinner placement tool", () =>
+            {
+                InputManager.Key(Key.Number4);
+                InputManager.MoveMouseTo(this.ChildrenOfType<OsuHitObjectComposer>().Single());
+            });
+            AddStep("begin spinner placement", () => InputManager.Click(MouseButton.Left));
+            AddStep("seek to 1500", () => EditorClock.Seek(1500));
+
+            AddStep("start dragging", () =>
+            {
+                var blueprintQuad = blueprintContainer.SelectionBlueprints[1].ScreenSpaceDrawQuad;
+                var dragStartPos = (blueprintQuad.TopLeft + blueprintQuad.BottomLeft) / 2 - new Vector2(30, 0);
+                InputManager.MoveMouseTo(dragStartPos);
+                InputManager.PressButton(MouseButton.Left);
+            });
+
+            AddStep("select entire object", () =>
+            {
+                var blueprintQuad = blueprintContainer.SelectionBlueprints[1].ScreenSpaceDrawQuad;
+                var dragStartPos = (blueprintQuad.TopRight + blueprintQuad.BottomRight) / 2 + new Vector2(30, 0);
+                InputManager.MoveMouseTo(dragStartPos);
+            });
+            AddStep("end drag", () => InputManager.ReleaseButton(MouseButton.Left));
+
+            AddUntilStep("hitobject selected", () => EditorBeatmap.SelectedHitObjects, () => NUnit.Framework.Contains.Item(addedObjects[0]));
+            AddAssert("placement committed", () => EditorBeatmap.HitObjects, () => Has.Count.EqualTo(2));
+        }
+
         private void assertSelectionIs(IEnumerable<HitObject> hitObjects)
             => AddAssert("correct hitobjects selected", () => EditorBeatmap.SelectedHitObjects.OrderBy(h => h.StartTime).SequenceEqual(hitObjects));
     }

osu.Game/Rulesets/Edit/HitObjectComposer.cs

@@ -67,7 +67,8 @@ namespace osu.Game.Rulesets.Edit
         [Resolved]
         private OverlayColourProvider colourProvider { get; set; }
 
-        protected ComposeBlueprintContainer BlueprintContainer { get; private set; }
+        public override ComposeBlueprintContainer BlueprintContainer => blueprintContainer;
+        private ComposeBlueprintContainer blueprintContainer;
 
         protected ExpandingToolboxContainer LeftToolbox { get; private set; }
 
@@ -143,7 +144,7 @@ namespace osu.Game.Rulesets.Edit
                         drawableRulesetWrapper,
                         // layers above playfield
                         drawableRulesetWrapper.CreatePlayfieldAdjustmentContainer()
-                                              .WithChild(BlueprintContainer = CreateBlueprintContainer())
+                                              .WithChild(blueprintContainer = CreateBlueprintContainer())
                     }
                 },
                 new Container
@@ -570,6 +571,8 @@ namespace osu.Game.Rulesets.Edit
         /// </summary>
         public abstract Playfield Playfield { get; }
 
+        public abstract ComposeBlueprintContainer BlueprintContainer { get; }
+
         /// <summary>
         /// All <see cref="DrawableHitObject"/>s in currently loaded beatmap.
         /// </summary>

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

@@ -372,7 +372,7 @@ namespace osu.Game.Screens.Edit.Compose.Components
             }
         }
 
-        private void commitIfPlacementActive()
+        public void CommitIfPlacementActive()
         {
             CurrentPlacement?.EndPlacement(CurrentPlacement.PlacementActive == PlacementBlueprint.PlacementState.Active);
             removePlacement();
@@ -402,7 +402,7 @@ namespace osu.Game.Screens.Edit.Compose.Components
                 currentTool = value;
 
                 // As per stable editor, when changing tools, we should forcefully commit any pending placement.
-                commitIfPlacementActive();
+                CommitIfPlacementActive();
             }
         }
     }

osu.Game/Screens/Edit/Compose/Components/Timeline/TimelineBlueprintContainer.cs

@@ -170,6 +170,8 @@ namespace osu.Game.Screens.Edit.Compose.Components.Timeline
 
         protected override void UpdateSelectionFromDragBox()
         {
+            Composer.BlueprintContainer.CommitIfPlacementActive();
+
             var dragBox = (TimelineDragBox)DragBox;
             double minTime = dragBox.MinTime;
             double maxTime = dragBox.MaxTime;