Merge branch 'master' into song-select-track-selected-better-2

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

@@ -96,6 +96,11 @@ namespace osu.Game.Rulesets.Mania.Tests.Editor
             throw new System.NotImplementedException();
         }
 
+        public override SnapResult SnapScreenSpacePositionToValidPosition(Vector2 screenSpacePosition)
+        {
+            throw new System.NotImplementedException();
+        }
+
         public override float GetBeatSnapDistanceAt(double referenceTime)
         {
             throw new System.NotImplementedException();

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

@@ -51,7 +51,7 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor
                 var first = (OsuHitObject)objects.First();
                 var second = (OsuHitObject)objects.Last();
 
-                return first.Position == second.Position;
+                return Precision.AlmostEquals(first.EndPosition, second.Position);
             });
         }
 
@@ -86,5 +86,64 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor
                 return Precision.AlmostEquals(first.EndPosition, second.Position);
             });
         }
+
+        [Test]
+        public void TestSecondCircleInSelectionAlsoSnaps()
+        {
+            AddStep("move mouse to centre", () => InputManager.MoveMouseTo(playfield.ScreenSpaceDrawQuad.Centre));
+
+            AddStep("disable distance snap", () => InputManager.Key(Key.Q));
+
+            AddStep("enter placement mode", () => InputManager.Key(Key.Number2));
+
+            AddStep("place first object", () => InputManager.Click(MouseButton.Left));
+
+            AddStep("increment time", () => EditorClock.SeekForward(true));
+
+            AddStep("move mouse right", () => InputManager.MoveMouseTo(playfield.ScreenSpaceDrawQuad.Centre + new Vector2(playfield.ScreenSpaceDrawQuad.Width * 0.2f, 0)));
+            AddStep("place second object", () => InputManager.Click(MouseButton.Left));
+
+            AddStep("increment time", () => EditorClock.SeekForward(true));
+
+            AddStep("move mouse down", () => InputManager.MoveMouseTo(playfield.ScreenSpaceDrawQuad.Centre + new Vector2(0, playfield.ScreenSpaceDrawQuad.Width * 0.2f)));
+            AddStep("place third object", () => InputManager.Click(MouseButton.Left));
+
+            AddStep("enter selection mode", () => InputManager.Key(Key.Number1));
+
+            AddStep("select objects 2 and 3", () =>
+            {
+                // add selection backwards to test non-sequential time ordering
+                EditorBeatmap.SelectedHitObjects.Add(EditorBeatmap.HitObjects[2]);
+                EditorBeatmap.SelectedHitObjects.Add(EditorBeatmap.HitObjects[1]);
+            });
+
+            AddStep("begin drag", () => InputManager.PressButton(MouseButton.Left));
+
+            AddStep("move mouse slightly off centre", () => InputManager.MoveMouseTo(playfield.ScreenSpaceDrawQuad.Centre + new Vector2(playfield.ScreenSpaceDrawQuad.Width * 0.02f, 0)));
+
+            AddAssert("object 3 snapped to 1", () =>
+            {
+                var objects = EditorBeatmap.HitObjects;
+
+                var first = (OsuHitObject)objects.First();
+                var third = (OsuHitObject)objects.Last();
+
+                return Precision.AlmostEquals(first.EndPosition, third.Position);
+            });
+
+            AddStep("move mouse slightly off centre", () => InputManager.MoveMouseTo(playfield.ScreenSpaceDrawQuad.Centre + new Vector2(playfield.ScreenSpaceDrawQuad.Width * -0.22f, playfield.ScreenSpaceDrawQuad.Width * 0.21f)));
+
+            AddAssert("object 2 snapped to 1", () =>
+            {
+                var objects = EditorBeatmap.HitObjects;
+
+                var first = (OsuHitObject)objects.First();
+                var second = (OsuHitObject)objects.ElementAt(1);
+
+                return Precision.AlmostEquals(first.EndPosition, second.Position);
+            });
+
+            AddStep("end drag", () => InputManager.ReleaseButton(MouseButton.Left));
+        }
     }
 }

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

@@ -174,6 +174,9 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor
 
         private class SnapProvider : IPositionSnapProvider
         {
+            public SnapResult SnapScreenSpacePositionToValidPosition(Vector2 screenSpacePosition) =>
+                new SnapResult(screenSpacePosition, null);
+
             public SnapResult SnapScreenSpacePositionToValidTime(Vector2 screenSpacePosition) => new SnapResult(screenSpacePosition, 0);
 
             public float GetBeatSnapDistanceAt(double referenceTime) => (float)beat_length;

osu.Game.Rulesets.Osu/Edit/OsuHitObjectComposer.cs

@@ -105,11 +105,20 @@ namespace osu.Game.Rulesets.Osu.Edit
             }
         }
 
-        public override SnapResult SnapScreenSpacePositionToValidTime(Vector2 screenSpacePosition)
+        public override SnapResult SnapScreenSpacePositionToValidPosition(Vector2 screenSpacePosition)
         {
             if (snapToVisibleBlueprints(screenSpacePosition, out var snapResult))
                 return snapResult;
 
+            return new SnapResult(screenSpacePosition, null);
+        }
+
+        public override SnapResult SnapScreenSpacePositionToValidTime(Vector2 screenSpacePosition)
+        {
+            var positionSnap = SnapScreenSpacePositionToValidPosition(screenSpacePosition);
+            if (positionSnap.ScreenSpacePosition != screenSpacePosition)
+                return positionSnap;
+
             // will be null if distance snap is disabled or not feasible for the current time value.
             if (distanceSnapGrid == null)
                 return base.SnapScreenSpacePositionToValidTime(screenSpacePosition);

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

@@ -153,6 +153,9 @@ namespace osu.Game.Tests.Visual.Editing
 
         private class SnapProvider : IPositionSnapProvider
         {
+            public SnapResult SnapScreenSpacePositionToValidPosition(Vector2 screenSpacePosition) =>
+                new SnapResult(screenSpacePosition, null);
+
             public SnapResult SnapScreenSpacePositionToValidTime(Vector2 screenSpacePosition) => new SnapResult(screenSpacePosition, 0);
 
             public float GetBeatSnapDistanceAt(double referenceTime) => 10;

osu.Game/Rulesets/Edit/HitObjectComposer.cs

@@ -442,6 +442,9 @@ namespace osu.Game.Rulesets.Edit
 
         public abstract SnapResult SnapScreenSpacePositionToValidTime(Vector2 screenSpacePosition);
 
+        public virtual SnapResult SnapScreenSpacePositionToValidPosition(Vector2 screenSpacePosition) =>
+            new SnapResult(screenSpacePosition, null);
+
         public abstract float GetBeatSnapDistanceAt(double referenceTime);
 
         public abstract float DurationToDistance(double referenceTime, double duration);

osu.Game/Rulesets/Edit/IPositionSnapProvider.cs

@@ -8,12 +8,22 @@ namespace osu.Game.Rulesets.Edit
     public interface IPositionSnapProvider
     {
         /// <summary>
-        /// Given a position, find a valid time snap.
+        /// Given a position, find a valid time and position snap.
         /// </summary>
+        /// <remarks>
+        /// This call should be equivalent to running <see cref="SnapScreenSpacePositionToValidPosition"/> with any additional logic that can be performed without the time immutability restriction.
+        /// </remarks>
         /// <param name="screenSpacePosition">The screen-space position to be snapped.</param>
         /// <returns>The time and position post-snapping.</returns>
         SnapResult SnapScreenSpacePositionToValidTime(Vector2 screenSpacePosition);
 
+        /// <summary>
+        /// Given a position, find a value position snap, restricting time to its input value.
+        /// </summary>
+        /// <param name="screenSpacePosition">The screen-space position to be snapped.</param>
+        /// <returns>The position post-snapping. Time will always be null.</returns>
+        SnapResult SnapScreenSpacePositionToValidPosition(Vector2 screenSpacePosition);
+
         /// <summary>
         /// Retrieves the distance between two points within a timing point that are one beat length apart.
         /// </summary>

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

@@ -187,7 +187,7 @@ namespace osu.Game.Screens.Edit.Compose.Components
             if (e.Button == MouseButton.Right)
                 return false;
 
-            if (movementBlueprint != null)
+            if (movementBlueprints != null)
             {
                 isDraggingBlueprint = true;
                 changeHandler?.BeginChange();
@@ -299,7 +299,7 @@ namespace osu.Game.Screens.Edit.Compose.Components
 
             SelectionBlueprints.Remove(blueprint);
 
-            if (movementBlueprint == blueprint)
+            if (movementBlueprints?.Contains(blueprint) == true)
                 finishSelectionMovement();
 
             OnBlueprintRemoved(hitObject);
@@ -424,8 +424,8 @@ namespace osu.Game.Screens.Edit.Compose.Components
 
         #region Selection Movement
 
-        private Vector2? movementBlueprintOriginalPosition;
+        private Vector2[] movementBlueprintOriginalPositions;
-        private SelectionBlueprint movementBlueprint;
+        private SelectionBlueprint[] movementBlueprints;
         private bool isDraggingBlueprint;
 
         /// <summary>
@@ -442,8 +442,8 @@ namespace osu.Game.Screens.Edit.Compose.Components
                 return;
 
             // Movement is tracked from the blueprint of the earliest hitobject, since it only makes sense to distance snap from that hitobject
-            movementBlueprint = SelectionHandler.SelectedBlueprints.OrderBy(b => b.HitObject.StartTime).First();
+            movementBlueprints = SelectionHandler.SelectedBlueprints.OrderBy(b => b.HitObject.StartTime).ToArray();
-            movementBlueprintOriginalPosition = movementBlueprint.ScreenSpaceSelectionPoint; // todo: unsure if correct
+            movementBlueprintOriginalPositions = movementBlueprints.Select(m => m.ScreenSpaceSelectionPoint).ToArray();
         }
 
         /// <summary>
@@ -453,30 +453,47 @@ namespace osu.Game.Screens.Edit.Compose.Components
         /// <returns>Whether a movement was active.</returns>
         private bool moveCurrentSelection(DragEvent e)
         {
-            if (movementBlueprint == null)
+            if (movementBlueprints == null)
                 return false;
 
             if (snapProvider == null)
                 return true;
 
-            Debug.Assert(movementBlueprintOriginalPosition != null);
+            Debug.Assert(movementBlueprintOriginalPositions != null);
 
-            HitObject draggedObject = movementBlueprint.HitObject;
+            Vector2 distanceTravelled = e.ScreenSpaceMousePosition - e.ScreenSpaceMouseDownPosition;
+
+            // check for positional snap for every object in selection (for things like object-object snapping)
+            for (var i = 0; i < movementBlueprintOriginalPositions.Length; i++)
+            {
+                var testPosition = movementBlueprintOriginalPositions[i] + distanceTravelled;
+
+                var positionalResult = snapProvider.SnapScreenSpacePositionToValidPosition(testPosition);
+
+                if (positionalResult.ScreenSpacePosition == testPosition) continue;
+
+                // attempt to move the objects, and abort any time based snapping if we can.
+                if (SelectionHandler.HandleMovement(new MoveSelectionEvent(movementBlueprints[i], positionalResult.ScreenSpacePosition)))
+                    return true;
+            }
+
+            // if no positional snapping could be performed, try unrestricted snapping from the earliest
+            // hitobject in the selection.
 
             // The final movement position, relative to movementBlueprintOriginalPosition.
-            Vector2 movePosition = movementBlueprintOriginalPosition.Value + e.ScreenSpaceMousePosition - e.ScreenSpaceMouseDownPosition;
+            Vector2 movePosition = movementBlueprintOriginalPositions.First() + distanceTravelled;
 
             // Retrieve a snapped position.
             var result = snapProvider.SnapScreenSpacePositionToValidTime(movePosition);
 
             // Move the hitobjects.
-            if (!SelectionHandler.HandleMovement(new MoveSelectionEvent(movementBlueprint, result.ScreenSpacePosition)))
+            if (!SelectionHandler.HandleMovement(new MoveSelectionEvent(movementBlueprints.First(), result.ScreenSpacePosition)))
                 return true;
 
             if (result.Time.HasValue)
             {
                 // Apply the start time at the newly snapped-to position
-                double offset = result.Time.Value - draggedObject.StartTime;
+                double offset = result.Time.Value - movementBlueprints.First().HitObject.StartTime;
 
                 foreach (HitObject obj in Beatmap.SelectedHitObjects)
                 {
@@ -494,11 +511,11 @@ namespace osu.Game.Screens.Edit.Compose.Components
         /// <returns>Whether a movement was active.</returns>
         private bool finishSelectionMovement()
         {
-            if (movementBlueprint == null)
+            if (movementBlueprints == null)
                 return false;
 
-            movementBlueprintOriginalPosition = null;
+            movementBlueprintOriginalPositions = null;
-            movementBlueprint = null;
+            movementBlueprints = null;
 
             return true;
         }

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

@@ -224,6 +224,9 @@ namespace osu.Game.Screens.Edit.Compose.Components.Timeline
         /// </summary>
         public double VisibleRange => track.Length / Zoom;
 
+        public SnapResult SnapScreenSpacePositionToValidPosition(Vector2 screenSpacePosition) =>
+            new SnapResult(screenSpacePosition, null);
+
         public SnapResult SnapScreenSpacePositionToValidTime(Vector2 screenSpacePosition) =>
             new SnapResult(screenSpacePosition, beatSnapProvider.SnapTime(getTimeFromPosition(Content.ToLocalSpace(screenSpacePosition))));