Merge pull request #16354 from bdach/slider-snapping

Apply slider snapping to current beat divisor more liberally to match user expectations
This commit is contained in:
Dean Herbert 2022-01-12 11:18:18 +09:00 committed by GitHub
commit d8c52740cb
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 262 additions and 9 deletions

View File

@ -0,0 +1,225 @@
// 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.Linq;
using NUnit.Framework;
using osu.Framework.Input.Events;
using osu.Framework.Testing;
using osu.Framework.Utils;
using osu.Game.Beatmaps;
using osu.Game.Beatmaps.ControlPoints;
using osu.Game.Input.Bindings;
using osu.Game.Rulesets.Objects;
using osu.Game.Rulesets.Osu.Edit;
using osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components;
using osu.Game.Rulesets.Osu.Objects;
using osu.Game.Rulesets.Osu.UI;
using osu.Game.Screens.Edit;
using osu.Game.Screens.Edit.Compose.Components;
using osu.Game.Tests.Beatmaps;
using osu.Game.Tests.Visual;
using osuTK;
using osuTK.Input;
namespace osu.Game.Rulesets.Osu.Tests.Editor
{
public class TestSceneSliderSnapping : EditorTestScene
{
private const double beat_length = 1000;
protected override Ruleset CreateEditorRuleset() => new OsuRuleset();
protected override IBeatmap CreateBeatmap(RulesetInfo ruleset)
{
var controlPointInfo = new ControlPointInfo();
controlPointInfo.Add(0, new TimingControlPoint { BeatLength = beat_length });
return new TestBeatmap(ruleset, false)
{
ControlPointInfo = controlPointInfo
};
}
private Slider slider;
public override void SetUpSteps()
{
base.SetUpSteps();
AddStep("add unsnapped slider", () => EditorBeatmap.Add(slider = new Slider
{
StartTime = 0,
Position = OsuPlayfield.BASE_SIZE / 5,
Path = new SliderPath
{
ControlPoints =
{
new PathControlPoint(Vector2.Zero),
new PathControlPoint(OsuPlayfield.BASE_SIZE * 2 / 5),
new PathControlPoint(OsuPlayfield.BASE_SIZE * 3 / 5)
}
}
}));
AddStep("set beat divisor to 1/1", () =>
{
var beatDivisor = (BindableBeatDivisor)Editor.Dependencies.Get(typeof(BindableBeatDivisor));
beatDivisor.Value = 1;
});
}
[Test]
public void TestMovingUnsnappedSliderNodesSnaps()
{
PathControlPointPiece sliderEnd = null;
assertSliderSnapped(false);
AddStep("select slider", () => EditorBeatmap.SelectedHitObjects.Add(slider));
AddStep("select slider end", () =>
{
sliderEnd = this.ChildrenOfType<PathControlPointPiece>().Single(piece => piece.ControlPoint == slider.Path.ControlPoints.Last());
InputManager.MoveMouseTo(sliderEnd.ScreenSpaceDrawQuad.Centre);
});
AddStep("move slider end", () =>
{
InputManager.PressButton(MouseButton.Left);
InputManager.MoveMouseTo(sliderEnd.ScreenSpaceDrawQuad.Centre - new Vector2(0, 20));
InputManager.ReleaseButton(MouseButton.Left);
});
assertSliderSnapped(true);
}
[Test]
public void TestAddingControlPointToUnsnappedSliderNodesSnaps()
{
assertSliderSnapped(false);
AddStep("select slider", () => EditorBeatmap.SelectedHitObjects.Add(slider));
AddStep("move mouse to new point location", () =>
{
var firstPiece = this.ChildrenOfType<PathControlPointPiece>().Single(piece => piece.ControlPoint == slider.Path.ControlPoints[0]);
var secondPiece = this.ChildrenOfType<PathControlPointPiece>().Single(piece => piece.ControlPoint == slider.Path.ControlPoints[1]);
InputManager.MoveMouseTo((firstPiece.ScreenSpaceDrawQuad.Centre + secondPiece.ScreenSpaceDrawQuad.Centre) / 2);
});
AddStep("move slider end", () =>
{
InputManager.PressKey(Key.ControlLeft);
InputManager.Click(MouseButton.Left);
InputManager.ReleaseKey(Key.ControlLeft);
});
assertSliderSnapped(true);
}
[Test]
public void TestRemovingControlPointFromUnsnappedSliderNodesSnaps()
{
assertSliderSnapped(false);
AddStep("select slider", () => EditorBeatmap.SelectedHitObjects.Add(slider));
AddStep("move mouse to second control point", () =>
{
var secondPiece = this.ChildrenOfType<PathControlPointPiece>().Single(piece => piece.ControlPoint == slider.Path.ControlPoints[1]);
InputManager.MoveMouseTo(secondPiece);
});
AddStep("quick delete", () =>
{
InputManager.PressKey(Key.ShiftLeft);
InputManager.PressButton(MouseButton.Right);
InputManager.ReleaseKey(Key.ShiftLeft);
});
assertSliderSnapped(true);
}
[Test]
public void TestResizingUnsnappedSliderSnaps()
{
SelectionBoxScaleHandle handle = null;
assertSliderSnapped(false);
AddStep("select slider", () => EditorBeatmap.SelectedHitObjects.Add(slider));
AddStep("move mouse to scale handle", () =>
{
handle = this.ChildrenOfType<SelectionBoxScaleHandle>().First();
InputManager.MoveMouseTo(handle.ScreenSpaceDrawQuad.Centre);
});
AddStep("scale slider", () =>
{
InputManager.PressButton(MouseButton.Left);
InputManager.MoveMouseTo(handle.ScreenSpaceDrawQuad.Centre + new Vector2(20, 20));
InputManager.ReleaseButton(MouseButton.Left);
});
assertSliderSnapped(true);
}
[Test]
public void TestRotatingUnsnappedSliderDoesNotSnap()
{
SelectionBoxRotationHandle handle = null;
assertSliderSnapped(false);
AddStep("select slider", () => EditorBeatmap.SelectedHitObjects.Add(slider));
AddStep("move mouse to rotate handle", () =>
{
handle = this.ChildrenOfType<SelectionBoxRotationHandle>().First();
InputManager.MoveMouseTo(handle.ScreenSpaceDrawQuad.Centre);
});
AddStep("scale slider", () =>
{
InputManager.PressButton(MouseButton.Left);
InputManager.MoveMouseTo(handle.ScreenSpaceDrawQuad.Centre + new Vector2(0, 20));
InputManager.ReleaseButton(MouseButton.Left);
});
assertSliderSnapped(false);
}
[Test]
public void TestFlippingSliderDoesNotSnap()
{
OsuSelectionHandler selectionHandler = null;
assertSliderSnapped(false);
AddStep("select slider", () => EditorBeatmap.SelectedHitObjects.Add(slider));
AddStep("flip slider horizontally", () =>
{
selectionHandler = this.ChildrenOfType<OsuSelectionHandler>().Single();
selectionHandler.OnPressed(new KeyBindingPressEvent<GlobalAction>(InputManager.CurrentState, GlobalAction.EditorFlipHorizontally));
});
assertSliderSnapped(false);
AddStep("flip slider vertically", () =>
{
selectionHandler = this.ChildrenOfType<OsuSelectionHandler>().Single();
selectionHandler.OnPressed(new KeyBindingPressEvent<GlobalAction>(InputManager.CurrentState, GlobalAction.EditorFlipVertically));
});
assertSliderSnapped(false);
}
[Test]
public void TestReversingSliderDoesNotSnap()
{
assertSliderSnapped(false);
AddStep("select slider", () => EditorBeatmap.SelectedHitObjects.Add(slider));
AddStep("reverse slider", () =>
{
InputManager.PressKey(Key.ControlLeft);
InputManager.Key(Key.G);
InputManager.ReleaseKey(Key.ControlLeft);
});
assertSliderSnapped(false);
}
private void assertSliderSnapped(bool snapped)
=> AddAssert($"slider is {(snapped ? "" : "not ")}snapped", () =>
{
double durationInBeatLengths = slider.Duration / beat_length;
double fractionalPart = durationInBeatLengths - (int)durationInBeatLengths;
return Precision.AlmostEquals(fractionalPart, 0) == snapped;
});
}
}

View File

@ -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;
}

View File

@ -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)

View File

@ -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)

View File

@ -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>