Use slider snapping more liberally to match user expectations

Previously the slider path length would be snapped using the current
beat snap setting on *every* change of the slider path. As it turns out
this is unexpected behaviour in some situations (e.g. when reversing a
path, which is expected to preserve the previous duration, even though
the slider may be technically "unsnapped" at that point in time due to a
different beat snap setting being selected afterwards).

osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointVisualiser.cs

@@ -283,6 +283,9 @@ private void dragInProgress(DragEvent e)
                 }
             }
 
+            // Snap the path to the current beat divisor before checking length validity.
+            slider.SnapTo(snapProvider);
+
             if (!slider.Path.HasValidLength)
             {
                 for (int i = 0; i < slider.Path.ControlPoints.Count; i++)
@@ -290,6 +293,8 @@ private void dragInProgress(DragEvent e)
 
                 slider.Position = oldPosition;
                 slider.StartTime = oldStartTime;
+                // Snap the path length again to undo the invalid length.
+                slider.SnapTo(snapProvider);
                 return;
             }
 

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

@@ -80,7 +80,7 @@ protected override void LoadComplete()
             controlPoints.BindTo(HitObject.Path.ControlPoints);
 
             pathVersion.BindTo(HitObject.Path.Version);
-            pathVersion.BindValueChanged(_ => updatePath());
+            pathVersion.BindValueChanged(_ => editorBeatmap?.Update(HitObject));
 
             BodyPiece.UpdateFrom(HitObject);
         }
@@ -208,6 +208,8 @@ private PathControlPoint addControlPoint(Vector2 position)
             // Move the control points from the insertion index onwards to make room for the insertion
             controlPoints.Insert(insertionIndex, pathControlPoint);
 
+            HitObject.SnapTo(composer);
+
             return pathControlPoint;
         }
 
@@ -227,7 +229,10 @@ private void removeControlPoints(List<PathControlPoint> toRemove)
                 controlPoints.Remove(c);
             }
 
-            // If there are 0 or 1 remaining control points, the slider is in a degenerate (single point) form and should be deleted
+            // Snap the slider to the current beat divisor before checking length validity.
+            HitObject.SnapTo(composer);
+
+            // If there are 0 or 1 remaining control points, or the slider has an invalid length, it is in a degenerate form and should be deleted
             if (controlPoints.Count <= 1 || !HitObject.Path.HasValidLength)
             {
                 placementHandler?.Delete(HitObject);
@@ -242,12 +247,6 @@ private void removeControlPoints(List<PathControlPoint> toRemove)
             HitObject.Position += first;
         }
 
-        private void updatePath()
-        {
-            HitObject.Path.ExpectedDistance.Value = composer?.GetSnappedDistanceFromDistance(HitObject, (float)HitObject.Path.CalculatedDistance) ?? (float)HitObject.Path.CalculatedDistance;
-            editorBeatmap?.Update(HitObject);
-        }
-
         private void convertToStream()
         {
             if (editorBeatmap == null || changeHandler == null || beatDivisor == null)

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

@@ -1,12 +1,16 @@
 // 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.
 
+#nullable enable
+
 using System.Collections.Generic;
 using System.Linq;
+using osu.Framework.Allocation;
 using osu.Framework.Graphics;
 using osu.Framework.Graphics.Primitives;
 using osu.Framework.Utils;
 using osu.Game.Extensions;
+using osu.Game.Rulesets.Edit;
 using osu.Game.Rulesets.Objects;
 using osu.Game.Rulesets.Objects.Types;
 using osu.Game.Rulesets.Osu.Objects;
@@ -18,6 +22,9 @@ namespace osu.Game.Rulesets.Osu.Edit
 {
     public class OsuSelectionHandler : EditorSelectionHandler
     {
+        [Resolved(CanBeNull = true)]
+        private IPositionSnapProvider? positionSnapProvider { get; set; }
+
         /// <summary>
         /// During a transform, the initial origin is stored so it can be used throughout the operation.
         /// </summary>
@@ -27,7 +34,7 @@ public class OsuSelectionHandler : EditorSelectionHandler
         /// During a transform, the initial path types of a single selected slider are stored so they
         /// can be maintained throughout the operation.
         /// </summary>
-        private List<PathType?> referencePathTypes;
+        private List<PathType?>? referencePathTypes;
 
         protected override void OnSelectionChanged()
         {
@@ -197,6 +204,10 @@ private void scaleSlider(Slider slider, Vector2 scale)
             for (int i = 0; i < slider.Path.ControlPoints.Count; ++i)
                 slider.Path.ControlPoints[i].Type = referencePathTypes[i];
 
+            // Snap the slider's length to the current beat divisor
+            // to calculate the final resulting duration / bounding box before the final checks.
+            slider.SnapTo(positionSnapProvider);
+
             //if sliderhead or sliderend end up outside playfield, revert scaling.
             Quad scaledQuad = getSurroundingQuad(new OsuHitObject[] { slider });
             (bool xInBounds, bool yInBounds) = isQuadInBounds(scaledQuad);
@@ -206,6 +217,9 @@ private void scaleSlider(Slider slider, Vector2 scale)
 
             foreach (var point in slider.Path.ControlPoints)
                 point.Position = oldControlPoints.Dequeue();
+
+            // Snap the slider's length again to undo the potentially-invalid length applied by the previous snap.
+            slider.SnapTo(positionSnapProvider);
         }
 
         private void scaleHitObjects(OsuHitObject[] hitObjects, Anchor reference, Vector2 scale)

osu.Game/Rulesets/Objects/SliderPathExtensions.cs

@@ -2,6 +2,7 @@
 // See the LICENCE file in the repository root for full licence text.
 
 using System.Linq;
+using osu.Game.Rulesets.Edit;
 using osu.Game.Rulesets.Objects.Types;
 using osuTK;
 
@@ -11,6 +12,15 @@ namespace osu.Game.Rulesets.Objects
 {
     public static class SliderPathExtensions
     {
+        /// <summary>
+        /// Snaps the provided <paramref name="hitObject"/>'s duration using the <paramref name="snapProvider"/>.
+        /// </summary>
+        public static void SnapTo<THitObject>(this THitObject hitObject, IPositionSnapProvider? snapProvider)
+            where THitObject : HitObject, IHasPath
+        {
+            hitObject.Path.ExpectedDistance.Value = snapProvider?.GetSnappedDistanceFromDistance(hitObject, (float)hitObject.Path.CalculatedDistance) ?? hitObject.Path.CalculatedDistance;
+        }
+
         /// <summary>
         /// Reverse the direction of this path.
         /// </summary>