From de9b3d33ebfba7beefe36f659e3a8aa77ab6a7eb Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 5 May 2022 17:00:36 +0900 Subject: [PATCH 01/20] Rename misleading `DistanceSpacing` variable --- .../Editor/TestSceneOsuDistanceSnapGrid.cs | 16 +++------------- .../Visual/Editing/TestSceneDistanceSnapGrid.cs | 10 +++++----- .../Components/CircularDistanceSnapGrid.cs | 10 +++++----- .../Edit/Compose/Components/DistanceSnapGrid.cs | 8 ++++---- 4 files changed, 17 insertions(+), 27 deletions(-) diff --git a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneOsuDistanceSnapGrid.cs b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneOsuDistanceSnapGrid.cs index 368166157d..015a922719 100644 --- a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneOsuDistanceSnapGrid.cs +++ b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneOsuDistanceSnapGrid.cs @@ -37,7 +37,7 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor [Cached(typeof(IDistanceSnapProvider))] private readonly SnapProvider snapProvider = new SnapProvider(); - private TestOsuDistanceSnapGrid grid; + private OsuDistanceSnapGrid grid; public TestSceneOsuDistanceSnapGrid() { @@ -64,7 +64,7 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor RelativeSizeAxes = Axes.Both, Colour = Color4.SlateGray }, - grid = new TestOsuDistanceSnapGrid(new HitCircle { Position = grid_position }), + grid = new OsuDistanceSnapGrid(new HitCircle { Position = grid_position }), new SnappingCursorContainer { GetSnapPosition = v => grid.GetSnappedPosition(grid.ToLocalSpace(v)).position } }; }); @@ -115,7 +115,7 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor RelativeSizeAxes = Axes.Both, Colour = Color4.SlateGray }, - grid = new TestOsuDistanceSnapGrid(new HitCircle { Position = grid_position }, new HitCircle { StartTime = 200 }), + grid = new OsuDistanceSnapGrid(new HitCircle { Position = grid_position }, new HitCircle { StartTime = 200 }), new SnappingCursorContainer { GetSnapPosition = v => grid.GetSnappedPosition(grid.ToLocalSpace(v)).position } }; }); @@ -170,16 +170,6 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor } } - private class TestOsuDistanceSnapGrid : OsuDistanceSnapGrid - { - public new float DistanceSpacing => base.DistanceSpacing; - - public TestOsuDistanceSnapGrid(OsuHitObject hitObject, OsuHitObject nextHitObject = null) - : base(hitObject, nextHitObject) - { - } - } - private class SnapProvider : IDistanceSnapProvider { public SnapResult FindSnappedPosition(Vector2 screenSpacePosition) => diff --git a/osu.Game.Tests/Visual/Editing/TestSceneDistanceSnapGrid.cs b/osu.Game.Tests/Visual/Editing/TestSceneDistanceSnapGrid.cs index 38c0808a71..cdaa3739b7 100644 --- a/osu.Game.Tests/Visual/Editing/TestSceneDistanceSnapGrid.cs +++ b/osu.Game.Tests/Visual/Editing/TestSceneDistanceSnapGrid.cs @@ -87,7 +87,7 @@ namespace osu.Game.Tests.Visual.Editing private class TestDistanceSnapGrid : DistanceSnapGrid { - public new float DistanceSpacing => base.DistanceSpacing; + public new float DistanceBetweenTick => base.DistanceBetweenTick; public TestDistanceSnapGrid(double? endTime = null) : base(new HitObject(), grid_position, 0, endTime) @@ -105,7 +105,7 @@ namespace osu.Game.Tests.Visual.Editing int indexFromPlacement = 0; - for (float s = StartPosition.X + DistanceSpacing; s <= DrawWidth && indexFromPlacement < MaxIntervals; s += DistanceSpacing, indexFromPlacement++) + for (float s = StartPosition.X + DistanceBetweenTick; s <= DrawWidth && indexFromPlacement < MaxIntervals; s += DistanceBetweenTick, indexFromPlacement++) { AddInternal(new Circle { @@ -118,7 +118,7 @@ namespace osu.Game.Tests.Visual.Editing indexFromPlacement = 0; - for (float s = StartPosition.X - DistanceSpacing; s >= 0 && indexFromPlacement < MaxIntervals; s -= DistanceSpacing, indexFromPlacement++) + for (float s = StartPosition.X - DistanceBetweenTick; s >= 0 && indexFromPlacement < MaxIntervals; s -= DistanceBetweenTick, indexFromPlacement++) { AddInternal(new Circle { @@ -131,7 +131,7 @@ namespace osu.Game.Tests.Visual.Editing indexFromPlacement = 0; - for (float s = StartPosition.Y + DistanceSpacing; s <= DrawHeight && indexFromPlacement < MaxIntervals; s += DistanceSpacing, indexFromPlacement++) + for (float s = StartPosition.Y + DistanceBetweenTick; s <= DrawHeight && indexFromPlacement < MaxIntervals; s += DistanceBetweenTick, indexFromPlacement++) { AddInternal(new Circle { @@ -144,7 +144,7 @@ namespace osu.Game.Tests.Visual.Editing indexFromPlacement = 0; - for (float s = StartPosition.Y - DistanceSpacing; s >= 0 && indexFromPlacement < MaxIntervals; s -= DistanceSpacing, indexFromPlacement++) + for (float s = StartPosition.Y - DistanceBetweenTick; s >= 0 && indexFromPlacement < MaxIntervals; s -= DistanceBetweenTick, indexFromPlacement++) { AddInternal(new Circle { diff --git a/osu.Game/Screens/Edit/Compose/Components/CircularDistanceSnapGrid.cs b/osu.Game/Screens/Edit/Compose/Components/CircularDistanceSnapGrid.cs index 50d5f0389a..76e429dbeb 100644 --- a/osu.Game/Screens/Edit/Compose/Components/CircularDistanceSnapGrid.cs +++ b/osu.Game/Screens/Edit/Compose/Components/CircularDistanceSnapGrid.cs @@ -30,14 +30,14 @@ namespace osu.Game.Screens.Edit.Compose.Components Position = StartPosition, Width = crosshair_thickness, EdgeSmoothness = new Vector2(1), - Height = Math.Min(crosshair_max_size, DistanceSpacing * 2), + Height = Math.Min(crosshair_max_size, DistanceBetweenTick * 2), }, new Box { Origin = Anchor.Centre, Position = StartPosition, EdgeSmoothness = new Vector2(1), - Width = Math.Min(crosshair_max_size, DistanceSpacing * 2), + Width = Math.Min(crosshair_max_size, DistanceBetweenTick * 2), Height = crosshair_thickness, } }); @@ -45,11 +45,11 @@ namespace osu.Game.Screens.Edit.Compose.Components float dx = Math.Max(StartPosition.X, DrawWidth - StartPosition.X); float dy = Math.Max(StartPosition.Y, DrawHeight - StartPosition.Y); float maxDistance = new Vector2(dx, dy).Length; - int requiredCircles = Math.Min(MaxIntervals, (int)(maxDistance / DistanceSpacing)); + int requiredCircles = Math.Min(MaxIntervals, (int)(maxDistance / DistanceBetweenTick)); for (int i = 0; i < requiredCircles; i++) { - float radius = (i + 1) * DistanceSpacing * 2; + float radius = (i + 1) * DistanceBetweenTick * 2; AddInternal(new CircularProgress { @@ -74,7 +74,7 @@ namespace osu.Game.Screens.Edit.Compose.Components float distance = direction.Length; - float radius = DistanceSpacing; + float radius = DistanceBetweenTick; int radialCount = Math.Clamp((int)MathF.Round(distance / radius), 1, MaxIntervals); Vector2 normalisedDirection = direction * new Vector2(1f / distance); diff --git a/osu.Game/Screens/Edit/Compose/Components/DistanceSnapGrid.cs b/osu.Game/Screens/Edit/Compose/Components/DistanceSnapGrid.cs index 5568c15514..42bb8a813d 100644 --- a/osu.Game/Screens/Edit/Compose/Components/DistanceSnapGrid.cs +++ b/osu.Game/Screens/Edit/Compose/Components/DistanceSnapGrid.cs @@ -23,7 +23,7 @@ namespace osu.Game.Screens.Edit.Compose.Components /// /// The spacing between each tick of the beat snapping grid. /// - protected float DistanceSpacing { get; private set; } + protected float DistanceBetweenTick { get; private set; } /// /// The maximum number of distance snapping intervals allowed. @@ -32,7 +32,7 @@ namespace osu.Game.Screens.Edit.Compose.Components /// /// The position which the grid should start. - /// The first beat snapping tick is located at + away from this point. + /// The first beat snapping tick is located at + away from this point. /// protected readonly Vector2 StartPosition; @@ -92,7 +92,7 @@ namespace osu.Game.Screens.Edit.Compose.Components private void updateSpacing() { - DistanceSpacing = (float)(SnapProvider.GetBeatSnapDistanceAt(ReferenceObject) * distanceSpacingMultiplier.Value); + DistanceBetweenTick = (float)(SnapProvider.GetBeatSnapDistanceAt(ReferenceObject) * distanceSpacingMultiplier.Value); if (endTime == null) MaxIntervals = int.MaxValue; @@ -100,7 +100,7 @@ namespace osu.Game.Screens.Edit.Compose.Components { // +1 is added since a snapped hitobject may have its start time slightly less than the snapped time due to floating point errors double maxDuration = endTime.Value - StartTime + 1; - MaxIntervals = (int)(maxDuration / SnapProvider.DistanceToDuration(ReferenceObject, DistanceSpacing)); + MaxIntervals = (int)(maxDuration / SnapProvider.DistanceToDuration(ReferenceObject, DistanceBetweenTick)); } gridCache.Invalidate(); From 786c7f14d3d0a2ea1db09dea4528e04a4b53e217 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 5 May 2022 17:08:53 +0900 Subject: [PATCH 02/20] Expose `DistanceSpacingMultiplier` to distance --- .../Edit/Compose/Components/DistanceSnapGrid.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/osu.Game/Screens/Edit/Compose/Components/DistanceSnapGrid.cs b/osu.Game/Screens/Edit/Compose/Components/DistanceSnapGrid.cs index 42bb8a813d..a907608cbc 100644 --- a/osu.Game/Screens/Edit/Compose/Components/DistanceSnapGrid.cs +++ b/osu.Game/Screens/Edit/Compose/Components/DistanceSnapGrid.cs @@ -25,6 +25,8 @@ namespace osu.Game.Screens.Edit.Compose.Components /// protected float DistanceBetweenTick { get; private set; } + protected IBindable DistanceSpacingMultiplier { get; private set; } + /// /// The maximum number of distance snapping intervals allowed. /// @@ -53,8 +55,6 @@ namespace osu.Game.Screens.Edit.Compose.Components [Resolved] private BindableBeatDivisor beatDivisor { get; set; } - private IBindable distanceSpacingMultiplier; - private readonly LayoutValue gridCache = new LayoutValue(Invalidation.RequiredParentSizeToFit); private readonly double? endTime; @@ -86,13 +86,13 @@ namespace osu.Game.Screens.Edit.Compose.Components beatDivisor.BindValueChanged(_ => updateSpacing()); - distanceSpacingMultiplier = SnapProvider.DistanceSpacingMultiplier.GetBoundCopy(); - distanceSpacingMultiplier.BindValueChanged(_ => updateSpacing(), true); + DistanceSpacingMultiplier = SnapProvider.DistanceSpacingMultiplier.GetBoundCopy(); + DistanceSpacingMultiplier.BindValueChanged(_ => updateSpacing(), true); } private void updateSpacing() { - DistanceBetweenTick = (float)(SnapProvider.GetBeatSnapDistanceAt(ReferenceObject) * distanceSpacingMultiplier.Value); + DistanceBetweenTick = (float)(SnapProvider.GetBeatSnapDistanceAt(ReferenceObject) * DistanceSpacingMultiplier.Value); if (endTime == null) MaxIntervals = int.MaxValue; From 4c884aea5d8592b810b7549ab8d6966759edf60c Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 5 May 2022 17:09:14 +0900 Subject: [PATCH 03/20] Fix `CircularDistanceSnapGrid` returning an incorrect time value when distance spacing is not 1.0 --- .../Screens/Edit/Compose/Components/CircularDistanceSnapGrid.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Screens/Edit/Compose/Components/CircularDistanceSnapGrid.cs b/osu.Game/Screens/Edit/Compose/Components/CircularDistanceSnapGrid.cs index 76e429dbeb..1c6eb98521 100644 --- a/osu.Game/Screens/Edit/Compose/Components/CircularDistanceSnapGrid.cs +++ b/osu.Game/Screens/Edit/Compose/Components/CircularDistanceSnapGrid.cs @@ -80,7 +80,7 @@ namespace osu.Game.Screens.Edit.Compose.Components Vector2 normalisedDirection = direction * new Vector2(1f / distance); Vector2 snappedPosition = StartPosition + normalisedDirection * radialCount * radius; - return (snappedPosition, StartTime + SnapProvider.FindSnappedDuration(ReferenceObject, (snappedPosition - StartPosition).Length)); + return (snappedPosition, StartTime + SnapProvider.FindSnappedDuration(ReferenceObject, (float)((snappedPosition - StartPosition).Length / DistanceSpacingMultiplier.Value))); } } } From 947a68006a6afb4f166053401bc6b01f21e78f14 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 5 May 2022 17:50:17 +0900 Subject: [PATCH 04/20] Add note about `IDistanceSnapProvider` not multiplying `DistanceSpacing` itself --- osu.Game/Rulesets/Edit/IDistanceSnapProvider.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/osu.Game/Rulesets/Edit/IDistanceSnapProvider.cs b/osu.Game/Rulesets/Edit/IDistanceSnapProvider.cs index 8c599f8596..b12e1437dc 100644 --- a/osu.Game/Rulesets/Edit/IDistanceSnapProvider.cs +++ b/osu.Game/Rulesets/Edit/IDistanceSnapProvider.cs @@ -16,6 +16,7 @@ namespace osu.Game.Rulesets.Edit { /// /// A multiplier which changes the ratio of distance travelled per time unit. + /// Importantly, this is provided for manual usage, and not multiplied into any of the methods exposed by this interface. /// /// IBindable DistanceSpacingMultiplier { get; } From b2e9be70a5e3ae76a315fa346356a9348f0a590e Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 5 May 2022 17:50:33 +0900 Subject: [PATCH 05/20] Rewrite `CircularDistanceSnapGrid` snapping implementation to use snap provider --- .../Components/CircularDistanceSnapGrid.cs | 28 +++++++++++++------ 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/osu.Game/Screens/Edit/Compose/Components/CircularDistanceSnapGrid.cs b/osu.Game/Screens/Edit/Compose/Components/CircularDistanceSnapGrid.cs index 1c6eb98521..f34fa7328a 100644 --- a/osu.Game/Screens/Edit/Compose/Components/CircularDistanceSnapGrid.cs +++ b/osu.Game/Screens/Edit/Compose/Components/CircularDistanceSnapGrid.cs @@ -68,19 +68,29 @@ namespace osu.Game.Screens.Edit.Compose.Components if (MaxIntervals == 0) return (StartPosition, StartTime); - Vector2 direction = position - StartPosition; - if (direction == Vector2.Zero) - direction = new Vector2(0.001f, 0.001f); + // This grid implementation factors in the user's distance spacing specification, + // which is usually not considered by an `IDistanceSnapProvider`. + float distanceSpacing = (float)DistanceSpacingMultiplier.Value; - float distance = direction.Length; + Vector2 travelVector = (position - StartPosition); - float radius = DistanceBetweenTick; - int radialCount = Math.Clamp((int)MathF.Round(distance / radius), 1, MaxIntervals); + if (travelVector == Vector2.Zero) + return (StartPosition, StartTime); - Vector2 normalisedDirection = direction * new Vector2(1f / distance); - Vector2 snappedPosition = StartPosition + normalisedDirection * radialCount * radius; + float travelLength = travelVector.Length; - return (snappedPosition, StartTime + SnapProvider.FindSnappedDuration(ReferenceObject, (float)((snappedPosition - StartPosition).Length / DistanceSpacingMultiplier.Value))); + // FindSnappedDistance will always round down, but we want to potentially round upwards. + travelLength += DistanceBetweenTick / 2; + + // When interacting with the resolved snap provider, the distance spacing multiplier should first be removed + // to allow for snapping at a non-multiplied ratio. + float snappedDistance = SnapProvider.FindSnappedDistance(ReferenceObject, travelLength / distanceSpacing); + double snappedTime = StartTime + SnapProvider.DistanceToDuration(ReferenceObject, snappedDistance); + + // The multiplier can then be reapplied to the final position. + Vector2 snappedPosition = StartPosition + travelVector.Normalized() * snappedDistance * distanceSpacing; + + return (snappedPosition, snappedTime); } } } From 269e15c167dc53a20afd3162e936b578d8fbae4a Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Thu, 5 May 2022 11:42:50 +0300 Subject: [PATCH 06/20] Add test coverage of distance spacing multiplier working with distance snap grid --- .../Editor/TestSceneOsuDistanceSnapGrid.cs | 12 +++++- .../Editing/TestSceneDistanceSnapGrid.cs | 37 ++++++++++++++++--- 2 files changed, 42 insertions(+), 7 deletions(-) diff --git a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneOsuDistanceSnapGrid.cs b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneOsuDistanceSnapGrid.cs index 015a922719..1feb80414a 100644 --- a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneOsuDistanceSnapGrid.cs +++ b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneOsuDistanceSnapGrid.cs @@ -82,6 +82,14 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor AddStep($"set beat divisor = {divisor}", () => beatDivisor.Value = divisor); } + [TestCase(1.0f)] + [TestCase(2.0f)] + [TestCase(0.5f)] + public void TestDistanceSpacing(float multiplier) + { + AddStep($"set beat divisor = {multiplier}", () => snapProvider.DistanceSpacingMultiplier.Value = multiplier); + } + [Test] public void TestCursorInCentre() { @@ -177,7 +185,9 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor public SnapResult FindSnappedPositionAndTime(Vector2 screenSpacePosition) => new SnapResult(screenSpacePosition, 0); - public IBindable DistanceSpacingMultiplier { get; } = new BindableDouble(1); + public Bindable DistanceSpacingMultiplier { get; } = new BindableDouble(1); + + IBindable IDistanceSnapProvider.DistanceSpacingMultiplier => DistanceSpacingMultiplier; public float GetBeatSnapDistanceAt(HitObject referenceObject) => (float)beat_length; diff --git a/osu.Game.Tests/Visual/Editing/TestSceneDistanceSnapGrid.cs b/osu.Game.Tests/Visual/Editing/TestSceneDistanceSnapGrid.cs index cdaa3739b7..7d8089b435 100644 --- a/osu.Game.Tests/Visual/Editing/TestSceneDistanceSnapGrid.cs +++ b/osu.Game.Tests/Visual/Editing/TestSceneDistanceSnapGrid.cs @@ -21,8 +21,12 @@ namespace osu.Game.Tests.Visual.Editing public class TestSceneDistanceSnapGrid : EditorClockTestScene { private const double beat_length = 100; + private const int beat_snap_distance = 10; + private static readonly Vector2 grid_position = new Vector2(512, 384); + private TestDistanceSnapGrid grid; + [Cached(typeof(EditorBeatmap))] private readonly EditorBeatmap editorBeatmap; @@ -39,6 +43,7 @@ namespace osu.Game.Tests.Visual.Editing } }); editorBeatmap.ControlPointInfo.Add(0, new TimingControlPoint { BeatLength = beat_length }); + editorBeatmap.Difficulty.SliderMultiplier = 1; } [SetUp] @@ -51,7 +56,7 @@ namespace osu.Game.Tests.Visual.Editing RelativeSizeAxes = Axes.Both, Colour = Color4.SlateGray }, - new TestDistanceSnapGrid() + grid = new TestDistanceSnapGrid() }; }); @@ -68,9 +73,22 @@ namespace osu.Game.Tests.Visual.Editing AddStep($"set beat divisor = {divisor}", () => BeatDivisor.Value = divisor); } - [Test] - public void TestLimitedDistance() + [TestCase(1.0)] + [TestCase(2.0)] + [TestCase(0.5)] + public void TestDistanceSpacing(double multiplier) { + AddStep($"set distance spacing = {multiplier}", () => snapProvider.DistanceSpacingMultiplier.Value = multiplier); + AddAssert("distance spacing matches multiplier", () => grid.DistanceBetweenTick == beat_snap_distance * multiplier); + } + + [TestCase(1.0)] + [TestCase(2.0)] + [TestCase(0.5)] + public void TestLimitedDistance(double multiplier) + { + const int end_time = 100; + AddStep("create limited grid", () => { Children = new Drawable[] @@ -80,15 +98,20 @@ namespace osu.Game.Tests.Visual.Editing RelativeSizeAxes = Axes.Both, Colour = Color4.SlateGray }, - new TestDistanceSnapGrid(100) + grid = new TestDistanceSnapGrid(end_time) }; }); + + AddStep($"set distance spacing = {multiplier}", () => snapProvider.DistanceSpacingMultiplier.Value = multiplier); + AddAssert("check correct interval count", () => grid.MaxIntervals == (end_time / grid.DistanceBetweenTick)); } private class TestDistanceSnapGrid : DistanceSnapGrid { public new float DistanceBetweenTick => base.DistanceBetweenTick; + public new int MaxIntervals => base.MaxIntervals; + public TestDistanceSnapGrid(double? endTime = null) : base(new HitObject(), grid_position, 0, endTime) { @@ -167,9 +190,11 @@ namespace osu.Game.Tests.Visual.Editing public SnapResult FindSnappedPositionAndTime(Vector2 screenSpacePosition) => new SnapResult(screenSpacePosition, 0); - public IBindable DistanceSpacingMultiplier { get; } = new BindableDouble(1); + public Bindable DistanceSpacingMultiplier { get; } = new BindableDouble(1); - public float GetBeatSnapDistanceAt(HitObject referenceObject) => 10; + IBindable IDistanceSnapProvider.DistanceSpacingMultiplier => DistanceSpacingMultiplier; + + public float GetBeatSnapDistanceAt(HitObject referenceObject) => beat_snap_distance; public float DurationToDistance(HitObject referenceObject, double duration) => (float)duration; From 7b71fb860b39574c71cad2def3055750cec4bdad Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 5 May 2022 18:25:32 +0900 Subject: [PATCH 07/20] Expose `DistanceSpacingMultiplier` for test usage --- osu.Game/Rulesets/Edit/DistancedHitObjectComposer.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Rulesets/Edit/DistancedHitObjectComposer.cs b/osu.Game/Rulesets/Edit/DistancedHitObjectComposer.cs index bbcb702bd8..5e6d9dbe34 100644 --- a/osu.Game/Rulesets/Edit/DistancedHitObjectComposer.cs +++ b/osu.Game/Rulesets/Edit/DistancedHitObjectComposer.cs @@ -26,7 +26,7 @@ namespace osu.Game.Rulesets.Edit public abstract class DistancedHitObjectComposer : HitObjectComposer, IDistanceSnapProvider, IScrollBindingHandler where TObject : HitObject { - protected Bindable DistanceSpacingMultiplier { get; } = new BindableDouble(1.0) + public Bindable DistanceSpacingMultiplier { get; } = new BindableDouble(1.0) { MinValue = 0.1, MaxValue = 6.0, From b9d8b7e413462c45871419977b67a927ace19c41 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 5 May 2022 18:25:46 +0900 Subject: [PATCH 08/20] Fix end time extent not being accounted for in new snap implementation --- .../Edit/Compose/Components/CircularDistanceSnapGrid.cs | 5 +++++ .../Screens/Edit/Compose/Components/DistanceSnapGrid.cs | 9 +++++---- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/osu.Game/Screens/Edit/Compose/Components/CircularDistanceSnapGrid.cs b/osu.Game/Screens/Edit/Compose/Components/CircularDistanceSnapGrid.cs index f34fa7328a..2873d74f75 100644 --- a/osu.Game/Screens/Edit/Compose/Components/CircularDistanceSnapGrid.cs +++ b/osu.Game/Screens/Edit/Compose/Components/CircularDistanceSnapGrid.cs @@ -87,6 +87,11 @@ namespace osu.Game.Screens.Edit.Compose.Components float snappedDistance = SnapProvider.FindSnappedDistance(ReferenceObject, travelLength / distanceSpacing); double snappedTime = StartTime + SnapProvider.DistanceToDuration(ReferenceObject, snappedDistance); + if (snappedTime > LatestEndTime) + { + snappedDistance = SnapProvider.DurationToDistance(ReferenceObject, LatestEndTime.Value - ReferenceObject.StartTime); + } + // The multiplier can then be reapplied to the final position. Vector2 snappedPosition = StartPosition + travelVector.Normalized() * snappedDistance * distanceSpacing; diff --git a/osu.Game/Screens/Edit/Compose/Components/DistanceSnapGrid.cs b/osu.Game/Screens/Edit/Compose/Components/DistanceSnapGrid.cs index a907608cbc..b9e0cfef19 100644 --- a/osu.Game/Screens/Edit/Compose/Components/DistanceSnapGrid.cs +++ b/osu.Game/Screens/Edit/Compose/Components/DistanceSnapGrid.cs @@ -43,6 +43,8 @@ namespace osu.Game.Screens.Edit.Compose.Components /// protected readonly double StartTime; + protected readonly double? LatestEndTime; + [Resolved] protected OsuColour Colours { get; private set; } @@ -56,7 +58,6 @@ namespace osu.Game.Screens.Edit.Compose.Components private BindableBeatDivisor beatDivisor { get; set; } private readonly LayoutValue gridCache = new LayoutValue(Invalidation.RequiredParentSizeToFit); - private readonly double? endTime; protected readonly HitObject ReferenceObject; @@ -70,7 +71,7 @@ namespace osu.Game.Screens.Edit.Compose.Components protected DistanceSnapGrid(HitObject referenceObject, Vector2 startPosition, double startTime, double? endTime = null) { ReferenceObject = referenceObject; - this.endTime = endTime; + LatestEndTime = endTime; StartPosition = startPosition; StartTime = startTime; @@ -94,12 +95,12 @@ namespace osu.Game.Screens.Edit.Compose.Components { DistanceBetweenTick = (float)(SnapProvider.GetBeatSnapDistanceAt(ReferenceObject) * DistanceSpacingMultiplier.Value); - if (endTime == null) + if (LatestEndTime == null) MaxIntervals = int.MaxValue; else { // +1 is added since a snapped hitobject may have its start time slightly less than the snapped time due to floating point errors - double maxDuration = endTime.Value - StartTime + 1; + double maxDuration = LatestEndTime.Value - StartTime + 1; MaxIntervals = (int)(maxDuration / SnapProvider.DistanceToDuration(ReferenceObject, DistanceBetweenTick)); } From 4fe23bced2da57d8931991b3df12aab667c7c6d0 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 5 May 2022 18:32:37 +0900 Subject: [PATCH 09/20] Update tests with new assumptions and a better snap implementation --- .../Editor/TestSceneOsuDistanceSnapGrid.cs | 53 +++++++++---------- 1 file changed, 25 insertions(+), 28 deletions(-) diff --git a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneOsuDistanceSnapGrid.cs b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneOsuDistanceSnapGrid.cs index 1feb80414a..201d6b5d8f 100644 --- a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneOsuDistanceSnapGrid.cs +++ b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneOsuDistanceSnapGrid.cs @@ -4,7 +4,6 @@ using System; using NUnit.Framework; using osu.Framework.Allocation; -using osu.Framework.Bindables; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Framework.Graphics.Shapes; @@ -12,7 +11,6 @@ using osu.Framework.Input.Events; using osu.Framework.Utils; using osu.Game.Beatmaps.ControlPoints; using osu.Game.Rulesets.Edit; -using osu.Game.Rulesets.Objects; using osu.Game.Rulesets.Osu.Beatmaps; using osu.Game.Rulesets.Osu.Edit; using osu.Game.Rulesets.Osu.Objects; @@ -26,16 +24,26 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor public class TestSceneOsuDistanceSnapGrid : OsuManualInputManagerTestScene { private const double beat_length = 100; + private static readonly Vector2 grid_position = new Vector2(512, 384); [Cached(typeof(EditorBeatmap))] + [Cached(typeof(IBeatSnapProvider))] private readonly EditorBeatmap editorBeatmap; + [Cached] + private readonly EditorClock editorClock; + [Cached] private readonly BindableBeatDivisor beatDivisor = new BindableBeatDivisor(); [Cached(typeof(IDistanceSnapProvider))] - private readonly SnapProvider snapProvider = new SnapProvider(); + private readonly OsuHitObjectComposer snapProvider = new OsuHitObjectComposer(new OsuRuleset()) + { + // Just used for the snap implementation, so let's hide from vision. + AlwaysPresent = true, + Alpha = 0, + }; private OsuDistanceSnapGrid grid; @@ -48,14 +56,25 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor Ruleset = new OsuRuleset().RulesetInfo } }); + + editorClock = new EditorClock(editorBeatmap); + + base.Content.Children = new Drawable[] + { + snapProvider, + Content + }; } + protected override Container Content { get; } = new Container { RelativeSizeAxes = Axes.Both }; + [SetUp] public void Setup() => Schedule(() => { editorBeatmap.Difficulty.SliderMultiplier = 1; editorBeatmap.ControlPointInfo.Clear(); editorBeatmap.ControlPointInfo.Add(0, new TimingControlPoint { BeatLength = beat_length }); + snapProvider.DistanceSpacingMultiplier.Value = 1; Children = new Drawable[] { @@ -94,20 +113,20 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor public void TestCursorInCentre() { AddStep("move mouse to centre", () => InputManager.MoveMouseTo(grid.ToScreenSpace(grid_position))); - assertSnappedDistance((float)beat_length); + assertSnappedDistance(0); } [Test] public void TestCursorBeforeMovementPoint() { - AddStep("move mouse to just before movement point", () => InputManager.MoveMouseTo(grid.ToScreenSpace(grid_position + new Vector2((float)beat_length, 0) * 1.49f))); + AddStep("move mouse to just before movement point", () => InputManager.MoveMouseTo(grid.ToScreenSpace(grid_position + new Vector2((float)beat_length, 0) * 1.45f))); assertSnappedDistance((float)beat_length); } [Test] public void TestCursorAfterMovementPoint() { - AddStep("move mouse to just after movement point", () => InputManager.MoveMouseTo(grid.ToScreenSpace(grid_position + new Vector2((float)beat_length, 0) * 1.51f))); + AddStep("move mouse to just after movement point", () => InputManager.MoveMouseTo(grid.ToScreenSpace(grid_position + new Vector2((float)beat_length, 0) * 1.55f))); assertSnappedDistance((float)beat_length * 2); } @@ -177,27 +196,5 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor cursor.Position = GetSnapPosition.Invoke(screenSpacePosition); } } - - private class SnapProvider : IDistanceSnapProvider - { - public SnapResult FindSnappedPosition(Vector2 screenSpacePosition) => - new SnapResult(screenSpacePosition, null); - - public SnapResult FindSnappedPositionAndTime(Vector2 screenSpacePosition) => new SnapResult(screenSpacePosition, 0); - - public Bindable DistanceSpacingMultiplier { get; } = new BindableDouble(1); - - IBindable IDistanceSnapProvider.DistanceSpacingMultiplier => DistanceSpacingMultiplier; - - public float GetBeatSnapDistanceAt(HitObject referenceObject) => (float)beat_length; - - public float DurationToDistance(HitObject referenceObject, double duration) => (float)duration; - - public double DistanceToDuration(HitObject referenceObject, float distance) => distance; - - public double FindSnappedDuration(HitObject referenceObject, float distance) => 0; - - public float FindSnappedDistance(HitObject referenceObject, float distance) => 0; - } } } From 9fd98b80608e42182df12ced2ebec96b19d2e3db Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 5 May 2022 18:41:25 +0900 Subject: [PATCH 10/20] Also add test coverage of adjsuting the distance spacing multiplier --- .../Editor/TestSceneOsuDistanceSnapGrid.cs | 47 +++++++++++-------- 1 file changed, 28 insertions(+), 19 deletions(-) diff --git a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneOsuDistanceSnapGrid.cs b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneOsuDistanceSnapGrid.cs index 201d6b5d8f..6d0ed15e45 100644 --- a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneOsuDistanceSnapGrid.cs +++ b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneOsuDistanceSnapGrid.cs @@ -7,7 +7,7 @@ using osu.Framework.Allocation; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Framework.Graphics.Shapes; -using osu.Framework.Input.Events; +using osu.Framework.Input; using osu.Framework.Utils; using osu.Game.Beatmaps.ControlPoints; using osu.Game.Rulesets.Edit; @@ -23,7 +23,7 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor { public class TestSceneOsuDistanceSnapGrid : OsuManualInputManagerTestScene { - private const double beat_length = 100; + private const float beat_length = 100; private static readonly Vector2 grid_position = new Vector2(512, 384); @@ -119,15 +119,27 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor [Test] public void TestCursorBeforeMovementPoint() { - AddStep("move mouse to just before movement point", () => InputManager.MoveMouseTo(grid.ToScreenSpace(grid_position + new Vector2((float)beat_length, 0) * 1.45f))); - assertSnappedDistance((float)beat_length); + AddStep("move mouse to just before movement point", () => InputManager.MoveMouseTo(grid.ToScreenSpace(grid_position + new Vector2(beat_length, 0) * 1.45f))); + assertSnappedDistance(beat_length); } [Test] public void TestCursorAfterMovementPoint() { - AddStep("move mouse to just after movement point", () => InputManager.MoveMouseTo(grid.ToScreenSpace(grid_position + new Vector2((float)beat_length, 0) * 1.55f))); - assertSnappedDistance((float)beat_length * 2); + AddStep("move mouse to just after movement point", () => InputManager.MoveMouseTo(grid.ToScreenSpace(grid_position + new Vector2(beat_length, 0) * 1.55f))); + assertSnappedDistance(beat_length * 2); + } + + [TestCase(0.5f, beat_length * 2)] + [TestCase(1, beat_length * 2)] + [TestCase(1.5f, beat_length * 1.5f)] + [TestCase(2f, beat_length * 2)] + public void TestDistanceSpacingAdjust(float multiplier, float expectedDistance) + { + AddStep($"Set distance spacing to {multiplier}", () => snapProvider.DistanceSpacingMultiplier.Value = multiplier); + AddStep("move mouse to point", () => InputManager.MoveMouseTo(grid.ToScreenSpace(grid_position + new Vector2(beat_length, 0) * 2))); + + assertSnappedDistance(expectedDistance); } [Test] @@ -147,8 +159,8 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor }; }); - AddStep("move mouse outside grid", () => InputManager.MoveMouseTo(grid.ToScreenSpace(grid_position + new Vector2((float)beat_length, 0) * 3f))); - assertSnappedDistance((float)beat_length * 2); + AddStep("move mouse outside grid", () => InputManager.MoveMouseTo(grid.ToScreenSpace(grid_position + new Vector2(beat_length, 0) * 3f))); + assertSnappedDistance(beat_length * 2); } private void assertSnappedDistance(float expectedDistance) => AddAssert($"snap distance = {expectedDistance}", () => @@ -164,6 +176,10 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor private readonly Drawable cursor; + private InputManager inputManager; + + public override bool HandlePositionalInput => true; + public SnappingCursorContainer() { RelativeSizeAxes = Axes.Both; @@ -180,20 +196,13 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor { base.LoadComplete(); - updatePosition(GetContainingInputManager().CurrentState.Mouse.Position); + inputManager = GetContainingInputManager(); } - protected override bool OnMouseMove(MouseMoveEvent e) + protected override void Update() { - base.OnMouseMove(e); - - updatePosition(e.ScreenSpaceMousePosition); - return true; - } - - private void updatePosition(Vector2 screenSpacePosition) - { - cursor.Position = GetSnapPosition.Invoke(screenSpacePosition); + base.Update(); + cursor.Position = GetSnapPosition.Invoke(inputManager.CurrentState.Mouse.Position); } } } From 32b40bdabfaa99f6c01ecf22a07c222b064103cd Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 6 May 2022 15:45:36 +0900 Subject: [PATCH 11/20] Rename `DistanceBetweenTick` to be plural --- .../Visual/Editing/TestSceneDistanceSnapGrid.cs | 14 +++++++------- .../Compose/Components/CircularDistanceSnapGrid.cs | 10 +++++----- .../Edit/Compose/Components/DistanceSnapGrid.cs | 8 ++++---- 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/osu.Game.Tests/Visual/Editing/TestSceneDistanceSnapGrid.cs b/osu.Game.Tests/Visual/Editing/TestSceneDistanceSnapGrid.cs index 7d8089b435..9148965987 100644 --- a/osu.Game.Tests/Visual/Editing/TestSceneDistanceSnapGrid.cs +++ b/osu.Game.Tests/Visual/Editing/TestSceneDistanceSnapGrid.cs @@ -79,7 +79,7 @@ namespace osu.Game.Tests.Visual.Editing public void TestDistanceSpacing(double multiplier) { AddStep($"set distance spacing = {multiplier}", () => snapProvider.DistanceSpacingMultiplier.Value = multiplier); - AddAssert("distance spacing matches multiplier", () => grid.DistanceBetweenTick == beat_snap_distance * multiplier); + AddAssert("distance spacing matches multiplier", () => grid.DistanceBetweenTicks == beat_snap_distance * multiplier); } [TestCase(1.0)] @@ -103,12 +103,12 @@ namespace osu.Game.Tests.Visual.Editing }); AddStep($"set distance spacing = {multiplier}", () => snapProvider.DistanceSpacingMultiplier.Value = multiplier); - AddAssert("check correct interval count", () => grid.MaxIntervals == (end_time / grid.DistanceBetweenTick)); + AddAssert("check correct interval count", () => grid.MaxIntervals == (end_time / grid.DistanceBetweenTicks)); } private class TestDistanceSnapGrid : DistanceSnapGrid { - public new float DistanceBetweenTick => base.DistanceBetweenTick; + public new float DistanceBetweenTicks => base.DistanceBetweenTicks; public new int MaxIntervals => base.MaxIntervals; @@ -128,7 +128,7 @@ namespace osu.Game.Tests.Visual.Editing int indexFromPlacement = 0; - for (float s = StartPosition.X + DistanceBetweenTick; s <= DrawWidth && indexFromPlacement < MaxIntervals; s += DistanceBetweenTick, indexFromPlacement++) + for (float s = StartPosition.X + DistanceBetweenTicks; s <= DrawWidth && indexFromPlacement < MaxIntervals; s += DistanceBetweenTicks, indexFromPlacement++) { AddInternal(new Circle { @@ -141,7 +141,7 @@ namespace osu.Game.Tests.Visual.Editing indexFromPlacement = 0; - for (float s = StartPosition.X - DistanceBetweenTick; s >= 0 && indexFromPlacement < MaxIntervals; s -= DistanceBetweenTick, indexFromPlacement++) + for (float s = StartPosition.X - DistanceBetweenTicks; s >= 0 && indexFromPlacement < MaxIntervals; s -= DistanceBetweenTicks, indexFromPlacement++) { AddInternal(new Circle { @@ -154,7 +154,7 @@ namespace osu.Game.Tests.Visual.Editing indexFromPlacement = 0; - for (float s = StartPosition.Y + DistanceBetweenTick; s <= DrawHeight && indexFromPlacement < MaxIntervals; s += DistanceBetweenTick, indexFromPlacement++) + for (float s = StartPosition.Y + DistanceBetweenTicks; s <= DrawHeight && indexFromPlacement < MaxIntervals; s += DistanceBetweenTicks, indexFromPlacement++) { AddInternal(new Circle { @@ -167,7 +167,7 @@ namespace osu.Game.Tests.Visual.Editing indexFromPlacement = 0; - for (float s = StartPosition.Y - DistanceBetweenTick; s >= 0 && indexFromPlacement < MaxIntervals; s -= DistanceBetweenTick, indexFromPlacement++) + for (float s = StartPosition.Y - DistanceBetweenTicks; s >= 0 && indexFromPlacement < MaxIntervals; s -= DistanceBetweenTicks, indexFromPlacement++) { AddInternal(new Circle { diff --git a/osu.Game/Screens/Edit/Compose/Components/CircularDistanceSnapGrid.cs b/osu.Game/Screens/Edit/Compose/Components/CircularDistanceSnapGrid.cs index 2873d74f75..ee8ca80c01 100644 --- a/osu.Game/Screens/Edit/Compose/Components/CircularDistanceSnapGrid.cs +++ b/osu.Game/Screens/Edit/Compose/Components/CircularDistanceSnapGrid.cs @@ -30,14 +30,14 @@ namespace osu.Game.Screens.Edit.Compose.Components Position = StartPosition, Width = crosshair_thickness, EdgeSmoothness = new Vector2(1), - Height = Math.Min(crosshair_max_size, DistanceBetweenTick * 2), + Height = Math.Min(crosshair_max_size, DistanceBetweenTicks * 2), }, new Box { Origin = Anchor.Centre, Position = StartPosition, EdgeSmoothness = new Vector2(1), - Width = Math.Min(crosshair_max_size, DistanceBetweenTick * 2), + Width = Math.Min(crosshair_max_size, DistanceBetweenTicks * 2), Height = crosshair_thickness, } }); @@ -45,11 +45,11 @@ namespace osu.Game.Screens.Edit.Compose.Components float dx = Math.Max(StartPosition.X, DrawWidth - StartPosition.X); float dy = Math.Max(StartPosition.Y, DrawHeight - StartPosition.Y); float maxDistance = new Vector2(dx, dy).Length; - int requiredCircles = Math.Min(MaxIntervals, (int)(maxDistance / DistanceBetweenTick)); + int requiredCircles = Math.Min(MaxIntervals, (int)(maxDistance / DistanceBetweenTicks)); for (int i = 0; i < requiredCircles; i++) { - float radius = (i + 1) * DistanceBetweenTick * 2; + float radius = (i + 1) * DistanceBetweenTicks * 2; AddInternal(new CircularProgress { @@ -80,7 +80,7 @@ namespace osu.Game.Screens.Edit.Compose.Components float travelLength = travelVector.Length; // FindSnappedDistance will always round down, but we want to potentially round upwards. - travelLength += DistanceBetweenTick / 2; + travelLength += DistanceBetweenTicks / 2; // When interacting with the resolved snap provider, the distance spacing multiplier should first be removed // to allow for snapping at a non-multiplied ratio. diff --git a/osu.Game/Screens/Edit/Compose/Components/DistanceSnapGrid.cs b/osu.Game/Screens/Edit/Compose/Components/DistanceSnapGrid.cs index b9e0cfef19..742fbc99ca 100644 --- a/osu.Game/Screens/Edit/Compose/Components/DistanceSnapGrid.cs +++ b/osu.Game/Screens/Edit/Compose/Components/DistanceSnapGrid.cs @@ -23,7 +23,7 @@ namespace osu.Game.Screens.Edit.Compose.Components /// /// The spacing between each tick of the beat snapping grid. /// - protected float DistanceBetweenTick { get; private set; } + protected float DistanceBetweenTicks { get; private set; } protected IBindable DistanceSpacingMultiplier { get; private set; } @@ -34,7 +34,7 @@ namespace osu.Game.Screens.Edit.Compose.Components /// /// The position which the grid should start. - /// The first beat snapping tick is located at + away from this point. + /// The first beat snapping tick is located at + away from this point. /// protected readonly Vector2 StartPosition; @@ -93,7 +93,7 @@ namespace osu.Game.Screens.Edit.Compose.Components private void updateSpacing() { - DistanceBetweenTick = (float)(SnapProvider.GetBeatSnapDistanceAt(ReferenceObject) * DistanceSpacingMultiplier.Value); + DistanceBetweenTicks = (float)(SnapProvider.GetBeatSnapDistanceAt(ReferenceObject) * DistanceSpacingMultiplier.Value); if (LatestEndTime == null) MaxIntervals = int.MaxValue; @@ -101,7 +101,7 @@ namespace osu.Game.Screens.Edit.Compose.Components { // +1 is added since a snapped hitobject may have its start time slightly less than the snapped time due to floating point errors double maxDuration = LatestEndTime.Value - StartTime + 1; - MaxIntervals = (int)(maxDuration / SnapProvider.DistanceToDuration(ReferenceObject, DistanceBetweenTick)); + MaxIntervals = (int)(maxDuration / SnapProvider.DistanceToDuration(ReferenceObject, DistanceBetweenTicks)); } gridCache.Invalidate(); From 246479bf34c725d886742017099f9d509e58b2f7 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 6 May 2022 15:54:37 +0900 Subject: [PATCH 12/20] Fix snap extent not working correctly on sliders (and providing incorrect time values) --- .../Edit/Compose/Components/CircularDistanceSnapGrid.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/osu.Game/Screens/Edit/Compose/Components/CircularDistanceSnapGrid.cs b/osu.Game/Screens/Edit/Compose/Components/CircularDistanceSnapGrid.cs index ee8ca80c01..ffa4176a48 100644 --- a/osu.Game/Screens/Edit/Compose/Components/CircularDistanceSnapGrid.cs +++ b/osu.Game/Screens/Edit/Compose/Components/CircularDistanceSnapGrid.cs @@ -89,7 +89,8 @@ namespace osu.Game.Screens.Edit.Compose.Components if (snappedTime > LatestEndTime) { - snappedDistance = SnapProvider.DurationToDistance(ReferenceObject, LatestEndTime.Value - ReferenceObject.StartTime); + snappedDistance = SnapProvider.DurationToDistance(ReferenceObject, LatestEndTime.Value - ReferenceObject.GetEndTime()); + snappedTime = StartTime + SnapProvider.DistanceToDuration(ReferenceObject, snappedDistance); } // The multiplier can then be reapplied to the final position. From c7e7aa5962c12cda3e74b161e0f4b30130b5a026 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 6 May 2022 17:05:52 +0900 Subject: [PATCH 13/20] Don't draw distance snap grid on the start time of the next object --- osu.Game.Rulesets.Osu/Edit/OsuDistanceSnapGrid.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Rulesets.Osu/Edit/OsuDistanceSnapGrid.cs b/osu.Game.Rulesets.Osu/Edit/OsuDistanceSnapGrid.cs index 8a561f962a..b11929c1e8 100644 --- a/osu.Game.Rulesets.Osu/Edit/OsuDistanceSnapGrid.cs +++ b/osu.Game.Rulesets.Osu/Edit/OsuDistanceSnapGrid.cs @@ -11,7 +11,7 @@ namespace osu.Game.Rulesets.Osu.Edit public class OsuDistanceSnapGrid : CircularDistanceSnapGrid { public OsuDistanceSnapGrid(OsuHitObject hitObject, [CanBeNull] OsuHitObject nextHitObject = null) - : base(hitObject, hitObject.StackedEndPosition, hitObject.GetEndTime(), nextHitObject?.StartTime) + : base(hitObject, hitObject.StackedEndPosition, hitObject.GetEndTime(), nextHitObject?.StartTime - 1) { Masking = true; } From 684fef7f8cf813f616b96cb0c3abfe009ccf40d5 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 6 May 2022 17:09:02 +0900 Subject: [PATCH 14/20] Fix `MaxIntervals` incorrectly factoring distance spacing multipiler into snap calculation --- .../Edit/Compose/Components/DistanceSnapGrid.cs | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/osu.Game/Screens/Edit/Compose/Components/DistanceSnapGrid.cs b/osu.Game/Screens/Edit/Compose/Components/DistanceSnapGrid.cs index 742fbc99ca..2f39db06d4 100644 --- a/osu.Game/Screens/Edit/Compose/Components/DistanceSnapGrid.cs +++ b/osu.Game/Screens/Edit/Compose/Components/DistanceSnapGrid.cs @@ -52,7 +52,7 @@ namespace osu.Game.Screens.Edit.Compose.Components protected IDistanceSnapProvider SnapProvider { get; private set; } [Resolved] - private EditorBeatmap beatmap { get; set; } + protected EditorBeatmap Beatmap { get; private set; } [Resolved] private BindableBeatDivisor beatDivisor { get; set; } @@ -93,16 +93,15 @@ namespace osu.Game.Screens.Edit.Compose.Components private void updateSpacing() { - DistanceBetweenTicks = (float)(SnapProvider.GetBeatSnapDistanceAt(ReferenceObject) * DistanceSpacingMultiplier.Value); + float distanceSpacingMultiplier = (float)DistanceSpacingMultiplier.Value; + float beatSnapDistance = SnapProvider.GetBeatSnapDistanceAt(ReferenceObject); + + DistanceBetweenTicks = beatSnapDistance * distanceSpacingMultiplier; if (LatestEndTime == null) MaxIntervals = int.MaxValue; else - { - // +1 is added since a snapped hitobject may have its start time slightly less than the snapped time due to floating point errors - double maxDuration = LatestEndTime.Value - StartTime + 1; - MaxIntervals = (int)(maxDuration / SnapProvider.DistanceToDuration(ReferenceObject, DistanceBetweenTicks)); - } + MaxIntervals = (int)((LatestEndTime.Value - StartTime) / SnapProvider.DistanceToDuration(ReferenceObject, beatSnapDistance)); gridCache.Invalidate(); } @@ -138,7 +137,7 @@ namespace osu.Game.Screens.Edit.Compose.Components /// The applicable colour. protected ColourInfo GetColourForIndexFromPlacement(int placementIndex) { - var timingPoint = beatmap.ControlPointInfo.TimingPointAt(StartTime); + var timingPoint = Beatmap.ControlPointInfo.TimingPointAt(StartTime); double beatLength = timingPoint.BeatLength / beatDivisor.Value; int beatIndex = (int)Math.Round((StartTime - timingPoint.Time) / beatLength); From 37cbc792834b9128ae2dfdcb8deba7abe6129766 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 6 May 2022 17:09:38 +0900 Subject: [PATCH 15/20] Fix clamping logic to always clamp to the last displayed tick --- .../Compose/Components/CircularDistanceSnapGrid.cs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/osu.Game/Screens/Edit/Compose/Components/CircularDistanceSnapGrid.cs b/osu.Game/Screens/Edit/Compose/Components/CircularDistanceSnapGrid.cs index ffa4176a48..13d838234e 100644 --- a/osu.Game/Screens/Edit/Compose/Components/CircularDistanceSnapGrid.cs +++ b/osu.Game/Screens/Edit/Compose/Components/CircularDistanceSnapGrid.cs @@ -70,7 +70,7 @@ namespace osu.Game.Screens.Edit.Compose.Components // This grid implementation factors in the user's distance spacing specification, // which is usually not considered by an `IDistanceSnapProvider`. - float distanceSpacing = (float)DistanceSpacingMultiplier.Value; + float distanceSpacingMultiplier = (float)DistanceSpacingMultiplier.Value; Vector2 travelVector = (position - StartPosition); @@ -84,17 +84,19 @@ namespace osu.Game.Screens.Edit.Compose.Components // When interacting with the resolved snap provider, the distance spacing multiplier should first be removed // to allow for snapping at a non-multiplied ratio. - float snappedDistance = SnapProvider.FindSnappedDistance(ReferenceObject, travelLength / distanceSpacing); + float snappedDistance = SnapProvider.FindSnappedDistance(ReferenceObject, travelLength / distanceSpacingMultiplier); double snappedTime = StartTime + SnapProvider.DistanceToDuration(ReferenceObject, snappedDistance); if (snappedTime > LatestEndTime) { - snappedDistance = SnapProvider.DurationToDistance(ReferenceObject, LatestEndTime.Value - ReferenceObject.GetEndTime()); + double tickLength = Beatmap.GetBeatLengthAtTime(StartTime); + + snappedDistance = SnapProvider.DurationToDistance(ReferenceObject, MaxIntervals * tickLength); snappedTime = StartTime + SnapProvider.DistanceToDuration(ReferenceObject, snappedDistance); } // The multiplier can then be reapplied to the final position. - Vector2 snappedPosition = StartPosition + travelVector.Normalized() * snappedDistance * distanceSpacing; + Vector2 snappedPosition = StartPosition + travelVector.Normalized() * snappedDistance * distanceSpacingMultiplier; return (snappedPosition, snappedTime); } From 118e58888b77d38a2b202c97ad7fe5ed11ff3ab9 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 6 May 2022 17:51:51 +0900 Subject: [PATCH 16/20] Rename incorrectly named variable (radius is not diameter) --- .../Edit/Compose/Components/CircularDistanceSnapGrid.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/osu.Game/Screens/Edit/Compose/Components/CircularDistanceSnapGrid.cs b/osu.Game/Screens/Edit/Compose/Components/CircularDistanceSnapGrid.cs index 13d838234e..91fad08aff 100644 --- a/osu.Game/Screens/Edit/Compose/Components/CircularDistanceSnapGrid.cs +++ b/osu.Game/Screens/Edit/Compose/Components/CircularDistanceSnapGrid.cs @@ -49,15 +49,15 @@ namespace osu.Game.Screens.Edit.Compose.Components for (int i = 0; i < requiredCircles; i++) { - float radius = (i + 1) * DistanceBetweenTicks * 2; + float diameter = (i + 1) * DistanceBetweenTicks * 2; AddInternal(new CircularProgress { Origin = Anchor.Centre, Position = StartPosition, Current = { Value = 1 }, - Size = new Vector2(radius), - InnerRadius = 4 * 1f / radius, + Size = new Vector2(diameter), + InnerRadius = 4 * 1f / diameter, Colour = GetColourForIndexFromPlacement(i) }); } From daed42513e6476abe183fd6173ce328765970903 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 6 May 2022 18:44:25 +0900 Subject: [PATCH 17/20] Fix outdated test asserts --- .../Editor/TestSceneOsuDistanceSnapGrid.cs | 2 +- osu.Game.Tests/Visual/Editing/TestSceneDistanceSnapGrid.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneOsuDistanceSnapGrid.cs b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneOsuDistanceSnapGrid.cs index 6d0ed15e45..f6b8f4edc3 100644 --- a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneOsuDistanceSnapGrid.cs +++ b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneOsuDistanceSnapGrid.cs @@ -160,7 +160,7 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor }); AddStep("move mouse outside grid", () => InputManager.MoveMouseTo(grid.ToScreenSpace(grid_position + new Vector2(beat_length, 0) * 3f))); - assertSnappedDistance(beat_length * 2); + assertSnappedDistance(beat_length); } private void assertSnappedDistance(float expectedDistance) => AddAssert($"snap distance = {expectedDistance}", () => diff --git a/osu.Game.Tests/Visual/Editing/TestSceneDistanceSnapGrid.cs b/osu.Game.Tests/Visual/Editing/TestSceneDistanceSnapGrid.cs index 9148965987..3aa3481cbf 100644 --- a/osu.Game.Tests/Visual/Editing/TestSceneDistanceSnapGrid.cs +++ b/osu.Game.Tests/Visual/Editing/TestSceneDistanceSnapGrid.cs @@ -103,7 +103,7 @@ namespace osu.Game.Tests.Visual.Editing }); AddStep($"set distance spacing = {multiplier}", () => snapProvider.DistanceSpacingMultiplier.Value = multiplier); - AddAssert("check correct interval count", () => grid.MaxIntervals == (end_time / grid.DistanceBetweenTicks)); + AddStep("check correct interval count", () => Assert.That((end_time / grid.DistanceBetweenTicks) * multiplier, Is.EqualTo(grid.MaxIntervals))); } private class TestDistanceSnapGrid : DistanceSnapGrid From fad1f727bb26954f75a8eeea25ff333107f3325a Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 6 May 2022 20:34:44 +0900 Subject: [PATCH 18/20] Fix editor drag box visuals --- .../Edit/Compose/Components/DragBox.cs | 85 ++++++++++++++++--- 1 file changed, 73 insertions(+), 12 deletions(-) diff --git a/osu.Game/Screens/Edit/Compose/Components/DragBox.cs b/osu.Game/Screens/Edit/Compose/Components/DragBox.cs index eaee2cd1e2..a256adbe4a 100644 --- a/osu.Game/Screens/Edit/Compose/Components/DragBox.cs +++ b/osu.Game/Screens/Edit/Compose/Components/DragBox.cs @@ -9,7 +9,8 @@ using osu.Framework.Graphics.Containers; using osu.Framework.Graphics.Primitives; using osu.Framework.Graphics.Shapes; using osu.Framework.Input.Events; -using osuTK.Graphics; +using osu.Framework.Layout; +using osuTK; namespace osu.Game.Screens.Edit.Compose.Components { @@ -41,17 +42,7 @@ namespace osu.Game.Screens.Edit.Compose.Components InternalChild = Box = CreateBox(); } - protected virtual Drawable CreateBox() => new Container - { - Masking = true, - BorderColour = Color4.White, - BorderThickness = SelectionBox.BORDER_RADIUS, - Child = new Box - { - RelativeSizeAxes = Axes.Both, - Alpha = 0.1f - } - }; + protected virtual Drawable CreateBox() => new BoxWithBorders(); private RectangleF? dragRectangle; @@ -111,5 +102,75 @@ namespace osu.Game.Screens.Edit.Compose.Components public override void Show() => State = Visibility.Visible; public event Action StateChanged; + + public class BoxWithBorders : CompositeDrawable + { + private readonly LayoutValue cache = new LayoutValue(Invalidation.RequiredParentSizeToFit); + + public BoxWithBorders() + { + AddLayout(cache); + } + + protected override void Update() + { + base.Update(); + + if (!cache.IsValid) + { + createContent(); + cache.Validate(); + } + } + + private void createContent() + { + if (DrawSize == Vector2.Zero) + { + ClearInternal(); + return; + } + + // Make lines the same width independent of display resolution. + float lineThickness = DrawWidth > 0 + ? DrawWidth / ScreenSpaceDrawQuad.Width * 2 + : DrawHeight / ScreenSpaceDrawQuad.Height * 2; + + Padding = new MarginPadding(-lineThickness); + + InternalChildren = new Drawable[] + { + new Box + { + RelativeSizeAxes = Axes.X, + Height = lineThickness, + }, + new Box + { + RelativeSizeAxes = Axes.X, + Height = lineThickness, + Anchor = Anchor.BottomRight, + Origin = Anchor.BottomRight, + }, + new Box + { + RelativeSizeAxes = Axes.Y, + Width = lineThickness, + }, + new Box + { + RelativeSizeAxes = Axes.Y, + Width = lineThickness, + Anchor = Anchor.BottomRight, + Origin = Anchor.BottomRight, + }, + new Box + { + RelativeSizeAxes = Axes.Both, + Alpha = 0.1f + } + }; + } + } } } From b119726b289334df0666c15709a99245b2c0a28d Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Fri, 6 May 2022 15:36:52 +0300 Subject: [PATCH 19/20] Reword test step --- .../Editor/TestSceneOsuDistanceSnapGrid.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneOsuDistanceSnapGrid.cs b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneOsuDistanceSnapGrid.cs index f6b8f4edc3..3c3c5cb939 100644 --- a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneOsuDistanceSnapGrid.cs +++ b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneOsuDistanceSnapGrid.cs @@ -106,7 +106,7 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor [TestCase(0.5f)] public void TestDistanceSpacing(float multiplier) { - AddStep($"set beat divisor = {multiplier}", () => snapProvider.DistanceSpacingMultiplier.Value = multiplier); + AddStep($"set distance spacing = {multiplier}", () => snapProvider.DistanceSpacingMultiplier.Value = multiplier); } [Test] From 29a3ab7e7a018d895ddcc5f14d65d0f864812855 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 6 May 2022 15:21:25 +0200 Subject: [PATCH 20/20] Halve drag box padding Allows the drag box borders to collapse in on themselves to a single line if the drag selection has zero width or height. --- osu.Game/Screens/Edit/Compose/Components/DragBox.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Screens/Edit/Compose/Components/DragBox.cs b/osu.Game/Screens/Edit/Compose/Components/DragBox.cs index a256adbe4a..ecbac82db0 100644 --- a/osu.Game/Screens/Edit/Compose/Components/DragBox.cs +++ b/osu.Game/Screens/Edit/Compose/Components/DragBox.cs @@ -136,7 +136,7 @@ namespace osu.Game.Screens.Edit.Compose.Components ? DrawWidth / ScreenSpaceDrawQuad.Width * 2 : DrawHeight / ScreenSpaceDrawQuad.Height * 2; - Padding = new MarginPadding(-lineThickness); + Padding = new MarginPadding(-lineThickness / 2); InternalChildren = new Drawable[] {