Replace hit objects when placing at same time in editor (#37485)

Matches stable and significantly improves UX when mapping. In addition,
current behavior makes it too easy to place stacked objects which is
something we should not encourage.

---------

Co-authored-by: Bartłomiej Dach <dach.bartlomiej@gmail.com>

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

@@ -57,6 +57,28 @@ namespace osu.Game.Rulesets.Mania.Tests.Editor
             AddAssert("time is unchanged", () => EditorClock.CurrentTime, () => Is.EqualTo(initialTime));
         }
 
+        [Test]
+        public void TestNoTwoObjectsAtSameTimeAndColumn()
+        {
+            AddStep("change seek setting to false", () => config.SetValue(OsuSetting.EditorAutoSeekOnPlacement, false));
+            AddStep("clear beatmap", () => EditorBeatmap.Clear());
+
+            AddStep("select note placement tool", () => InputManager.Key(Key.Number2));
+            AddStep("move mouse to centre of last column", () => InputManager.MoveMouseTo(this.ChildrenOfType<Column>().Last().ScreenSpaceDrawQuad.Centre));
+            AddStep("place note", () => InputManager.Click(MouseButton.Left));
+            AddAssert("beatmap has 1 object", () => EditorBeatmap.HitObjects, () => Has.Count.EqualTo(1));
+
+            AddStep("select note placement tool", () => InputManager.Key(Key.Number2));
+            AddStep("move mouse to centre of first column", () => InputManager.MoveMouseTo(this.ChildrenOfType<Column>().First().ScreenSpaceDrawQuad.Centre));
+            AddStep("place note", () => InputManager.Click(MouseButton.Left));
+            AddAssert("beatmap has 2 objects", () => EditorBeatmap.HitObjects, () => Has.Count.EqualTo(2));
+
+            AddStep("select note placement tool", () => InputManager.Key(Key.Number2));
+            AddStep("move mouse to centre of last column", () => InputManager.MoveMouseTo(this.ChildrenOfType<Column>().Last().ScreenSpaceDrawQuad.Centre));
+            AddStep("place note", () => InputManager.Click(MouseButton.Left));
+            AddAssert("beatmap has 2 objects", () => EditorBeatmap.HitObjects, () => Has.Count.EqualTo(2));
+        }
+
         private void placeObject()
         {
             AddStep("select note placement tool", () => InputManager.Key(Key.Number2));

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

@@ -73,6 +73,12 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor
 
             AddStep("end slider placement", () => InputManager.Click(MouseButton.Right));
 
+            AddStep("seek to slider end", () =>
+            {
+                var slider = (Slider)EditorBeatmap.HitObjects.Single();
+                EditorClock.Seek(slider.EndTime);
+            });
+
             AddStep("enter circle placement mode", () => InputManager.Key(Key.Number2));
 
             AddStep("move mouse slightly", () => InputManager.MoveMouseTo(playfield.ScreenSpaceDrawQuad.Centre + new Vector2(playfield.ScreenSpaceDrawQuad.Width * 0.205f, 0)));

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

@@ -100,8 +100,10 @@ namespace osu.Game.Tests.Visual.Editing
         [Test]
         public void TestPlacementOfConcurrentObjectWithDuration()
         {
-            AddStep("seek to timing point", () => EditorClock.Seek(2170));
-            AddStep("add hit circle", () => EditorBeatmap.Add(createHitCircle(2170, Vector2.Zero)));
+            const double spinner_start_time = 2170;
+            const double spinner_end_seek_time = 2500;
+
+            AddStep("seek to timing point", () => EditorClock.Seek(spinner_start_time));
 
             AddStep("choose spinner placement tool", () =>
             {
@@ -116,10 +118,15 @@ namespace osu.Game.Tests.Visual.Editing
             });
             AddStep("end placing spinner", () =>
             {
-                EditorClock.Seek(2500);
+                EditorClock.Seek(spinner_end_seek_time);
                 InputManager.Click(MouseButton.Right);
             });
 
+            AddStep("add hit circle mid-spinner", () =>
+            {
+                EditorBeatmap.Add(createHitCircle((spinner_start_time + spinner_end_seek_time) / 2, Vector2.Zero));
+            });
+
             AddAssert("two timeline blueprints present", () => Editor.ChildrenOfType<TimelineHitObjectBlueprint>().Count() == 2);
         }
 

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

@@ -5,6 +5,7 @@ using System.Linq;
 using NUnit.Framework;
 using osu.Framework.Screens;
 using osu.Framework.Testing;
+using osu.Framework.Utils;
 using osu.Game.Audio;
 using osu.Game.Beatmaps;
 using osu.Game.Beatmaps.ControlPoints;
@@ -12,10 +13,11 @@ using osu.Game.Graphics.UserInterface;
 using osu.Game.Input.Bindings;
 using osu.Game.Rulesets;
 using osu.Game.Rulesets.Edit;
+using osu.Game.Rulesets.Objects;
 using osu.Game.Rulesets.Objects.Types;
+using osu.Game.Rulesets.UI;
 using osu.Game.Rulesets.Osu;
 using osu.Game.Rulesets.Osu.Objects;
-using osu.Game.Rulesets.UI;
 using osu.Game.Screens.Edit.Components.TernaryButtons;
 using osu.Game.Screens.Edit.Compose.Components;
 using osu.Game.Tests.Beatmaps;
@@ -51,6 +53,90 @@ namespace osu.Game.Tests.Visual.Editing
             AddAssert("circle removed", () => EditorBeatmap.HitObjects, () => Is.Empty);
         }
 
+        [Test]
+        public void TestPlacementReplacesObjectAtSameStartTime()
+        {
+            HitCircle existing = null!;
+            var existingPosition = new Vector2(128, 160);
+            var replacementPosition = new Vector2(400, 280);
+            Playfield playfield = null!;
+
+            AddStep("add existing circle", () =>
+            {
+                EditorBeatmap.Add(existing = new HitCircle
+                {
+                    StartTime = 500,
+                    Position = existingPosition,
+                });
+            });
+
+            AddStep("seek to same time", () => EditorClock.Seek(500));
+            AddStep("select circle placement tool", () => InputManager.Key(Key.Number2));
+            AddStep("grab playfield", () => playfield = this.ChildrenOfType<Playfield>().Single());
+            AddStep("move mouse to replacement coordinates", () => InputManager.MoveMouseTo(playfield.GamefieldToScreenSpace(replacementPosition)));
+            AddStep("place circle", () => InputManager.Click(MouseButton.Left));
+
+            AddAssert("only one hit object", () => EditorBeatmap.HitObjects.Count, () => Is.EqualTo(1));
+            AddAssert("original instance removed from beatmap", () => EditorBeatmap.HitObjects.Single(), () => Is.Not.SameAs(existing));
+            AddAssert("start time unchanged", () => Precision.AlmostEquals(EditorBeatmap.HitObjects.Single().StartTime, 500));
+            AddAssert("circle at new coordinates", () =>
+            {
+                var circle = (HitCircle)EditorBeatmap.HitObjects.Single();
+                return circle != null
+                       && Precision.AlmostEquals(circle.Position.X, replacementPosition.X)
+                       && Precision.AlmostEquals(circle.Position.Y, replacementPosition.Y);
+            });
+        }
+
+        [Test]
+        public void TestPlacementOnSliderBodyDoesNotRemoveSlider()
+        {
+            Slider originalSlider = null!;
+
+            AddStep("add slider", () =>
+            {
+                EditorBeatmap.Add(originalSlider = new Slider
+                {
+                    StartTime = 0,
+                    Position = new Vector2(256, 192),
+                    Path = new SliderPath(new[]
+                    {
+                        new PathControlPoint(Vector2.Zero),
+                        new PathControlPoint(new Vector2(256, 0)),
+                    }),
+                });
+            });
+
+            AddUntilStep("slider duration resolved", () => originalSlider.EndTime > originalSlider.StartTime + 1);
+
+            double midTime = 0;
+            double endTime = 0;
+
+            AddStep("capture slider times", () =>
+            {
+                midTime = originalSlider.StartTime + originalSlider.Duration / 2;
+                endTime = originalSlider.EndTime;
+            });
+
+            Playfield playfield = null!;
+
+            AddStep("seek to slider mid", () => EditorClock.Seek(midTime));
+            AddStep("select circle placement tool", () => InputManager.Key(Key.Number2));
+            AddStep("grab playfield", () => playfield = this.ChildrenOfType<Playfield>().Single());
+            AddStep("move mouse for mid placement", () => InputManager.MoveMouseTo(playfield.GamefieldToScreenSpace(new Vector2(300, 200))));
+            AddStep("place circle at slider mid time", () => InputManager.Click(MouseButton.Left));
+
+            AddAssert("slider preserved after mid placement", () => EditorBeatmap.HitObjects.Contains(originalSlider));
+            AddAssert("one circle after mid placement", () => EditorBeatmap.HitObjects.Count(h => h is HitCircle), () => Is.EqualTo(1));
+
+            AddStep("seek to slider end", () => EditorClock.Seek(endTime));
+            AddStep("place circle at slider end time", () => InputManager.Click(MouseButton.Left));
+
+            AddAssert("slider still preserved", () => EditorBeatmap.HitObjects.Contains(originalSlider));
+            AddAssert("three hit objects total", () => EditorBeatmap.HitObjects.Count, () => Is.EqualTo(3));
+            AddAssert("two circles placed", () => EditorBeatmap.HitObjects.Count(h => h is HitCircle), () => Is.EqualTo(2));
+        }
+
         [Test]
         public void TestTimingLost()
         {

osu.Game/Rulesets/Edit/HitObjectComposer.cs

@@ -541,7 +541,12 @@ namespace osu.Game.Rulesets.Edit
         public void CommitPlacement(HitObject hitObject)
         {
             EditorBeatmap.PlacementObject.Value = null;
+
+            EditorBeatmap.BeginChange();
+            foreach (var h in EditorBeatmap.HitObjects.Where(ho => HitObjectPlacementBlueprint.PlacementReplacesExisting(ho, hitObject)).ToArray())
+                EditorBeatmap.Remove(h);
             EditorBeatmap.Add(hitObject);
+            EditorBeatmap.EndChange();
 
             if (autoSeekOnPlacement.Value && EditorClock.CurrentTime < hitObject.StartTime)
                 EditorClock.SeekSmoothlyTo(hitObject.StartTime);

osu.Game/Rulesets/Edit/HitObjectPlacementBlueprint.cs

@@ -7,6 +7,7 @@ using osu.Framework.Allocation;
 using osu.Framework.Bindables;
 using osu.Framework.Graphics;
 using osu.Framework.Graphics.Containers;
+using osu.Framework.Utils;
 using osu.Game.Audio;
 using osu.Game.Beatmaps;
 using osu.Game.Beatmaps.ControlPoints;
@@ -53,6 +54,12 @@ namespace osu.Game.Rulesets.Edit
         [Resolved]
         private IPlacementHandler placementHandler { get; set; } = null!;
 
+        /// <summary>
+        /// Acceptable leniency to account for rounding errors and minor unsnaps that we generally
+        /// don't consider a problem, but still need to account for in certain operations.
+        /// </summary>
+        private const double placement_replace_start_time_leniency_ms = 2;
+
         protected HitObjectPlacementBlueprint(HitObject hitObject)
         {
             HitObject = hitObject;
@@ -61,6 +68,23 @@ namespace osu.Game.Rulesets.Edit
             HitObject.Samples.Add(new HitSampleInfo(HitSampleInfo.HIT_NORMAL));
         }
 
+        /// <summary>
+        /// Whether <paramref name="existing"/> should be removed because <paramref name="placement"/> is being placed on top of it.
+        /// </summary>
+        /// <remarks>
+        /// Matches when start times are within ±<see cref="placement_replace_start_time_leniency_ms"/> ms of each other.
+        /// </remarks>
+        public static bool PlacementReplacesExisting(HitObject existing, HitObject placement)
+        {
+            if (!Precision.AlmostEquals(existing.StartTime, placement.StartTime, placement_replace_start_time_leniency_ms))
+                return false;
+
+            if (placement is IHasColumn placementColumn && existing is IHasColumn existingColumn)
+                return existingColumn.Column == placementColumn.Column;
+
+            return true;
+        }
+
         [BackgroundDependencyLoader]
         private void load()
         {