From a294f328fb4f1471273b5ebe2e49b8e7e2003f21 Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Sun, 21 Mar 2021 06:30:17 +0100 Subject: [PATCH 01/52] Add linear circular arc test --- .../TestSceneSliderPlacementBlueprint.cs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderPlacementBlueprint.cs b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderPlacementBlueprint.cs index 67a2e5a47c..f9b6fb924e 100644 --- a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderPlacementBlueprint.cs +++ b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderPlacementBlueprint.cs @@ -276,6 +276,24 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor assertControlPointType(0, PathType.Linear); } + [Test] + public void TestPlacePerfectCurveSegmentAlmostLinearly() + { + addMovementStep(new Vector2(0)); + addClickStep(MouseButton.Left); + + addMovementStep(new Vector2(61.382935f, 6.423767f)); + addClickStep(MouseButton.Left); + + addMovementStep(new Vector2(217.69522f, 22.84021f)); + addClickStep(MouseButton.Right); + + assertPlaced(true); + assertControlPointCount(3); + // Will be > 10000 if not falling back to Bezier path calc. + assertLength(218.8901f); + } + private void addMovementStep(Vector2 position) => AddStep($"move mouse to {position}", () => InputManager.MoveMouseTo(InputManager.ToScreenSpace(position))); private void addClickStep(MouseButton button) From fcd1f4930f9118445a0976a785db75282bf9763b Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Sun, 21 Mar 2021 06:34:55 +0100 Subject: [PATCH 02/52] Fix freeze due to large circular arc radius Seems to stem from the osu!framework's PathApproximator not catching a few edge cases wherein the radius approaches infinity. --- osu.Game/Rulesets/Objects/SliderPath.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/osu.Game/Rulesets/Objects/SliderPath.cs b/osu.Game/Rulesets/Objects/SliderPath.cs index 3083fcfccb..94815fe0ea 100644 --- a/osu.Game/Rulesets/Objects/SliderPath.cs +++ b/osu.Game/Rulesets/Objects/SliderPath.cs @@ -219,8 +219,10 @@ namespace osu.Game.Rulesets.Objects List subpath = PathApproximator.ApproximateCircularArc(subControlPoints); - // If for some reason a circular arc could not be fit to the 3 given points, fall back to a numerically stable bezier approximation. - if (subpath.Count == 0) + // If for some reason a circular arc could not be fit to the 3 given points, or the + // resulting radius is too large (e.g. points arranged almost linearly), fall back + // to a numerically stable bezier approximation. + if (subpath.Count == 0 || subpath.Count > 400) break; return subpath; From bee2f55d007bad3e83fd2d010f709ff720242784 Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Mon, 22 Mar 2021 15:54:33 +0100 Subject: [PATCH 03/52] Undo subpath limiting --- osu.Game/Rulesets/Objects/SliderPath.cs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/osu.Game/Rulesets/Objects/SliderPath.cs b/osu.Game/Rulesets/Objects/SliderPath.cs index 94815fe0ea..3083fcfccb 100644 --- a/osu.Game/Rulesets/Objects/SliderPath.cs +++ b/osu.Game/Rulesets/Objects/SliderPath.cs @@ -219,10 +219,8 @@ namespace osu.Game.Rulesets.Objects List subpath = PathApproximator.ApproximateCircularArc(subControlPoints); - // If for some reason a circular arc could not be fit to the 3 given points, or the - // resulting radius is too large (e.g. points arranged almost linearly), fall back - // to a numerically stable bezier approximation. - if (subpath.Count == 0 || subpath.Count > 400) + // If for some reason a circular arc could not be fit to the 3 given points, fall back to a numerically stable bezier approximation. + if (subpath.Count == 0) break; return subpath; From 7a2cb526e4bd9e04f5e0720336cf278551d4efac Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Mon, 22 Mar 2021 15:55:30 +0100 Subject: [PATCH 04/52] Add PointsInSegment method --- osu.Game/Rulesets/Objects/SliderPath.cs | 32 +++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/osu.Game/Rulesets/Objects/SliderPath.cs b/osu.Game/Rulesets/Objects/SliderPath.cs index 3083fcfccb..e76699ac04 100644 --- a/osu.Game/Rulesets/Objects/SliderPath.cs +++ b/osu.Game/Rulesets/Objects/SliderPath.cs @@ -156,6 +156,38 @@ namespace osu.Game.Rulesets.Objects return interpolateVertices(indexOfDistance(d), d); } + /// + /// Returns the control points belonging to the same segment as the one given. + /// The first point has a PathType which all other points inherit. + /// + /// One of the control points in the segment. + /// + public List PointsInSegment(PathControlPoint controlPoint) + { + bool found = false; + List pointsInCurrentSegment = new List(); + foreach (PathControlPoint point in ControlPoints) + { + if (point.Type.Value is PathType) + { + if (!found) + pointsInCurrentSegment.Clear(); + else + { + pointsInCurrentSegment.Add(point); + break; + } + } + + pointsInCurrentSegment.Add(point); + + if (point == controlPoint) + found = true; + } + + return pointsInCurrentSegment; + } + private void invalidate() { pathCache.Invalidate(); From c82218627f914ca51875422de48811dacb136c14 Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Mon, 22 Mar 2021 15:57:57 +0100 Subject: [PATCH 05/52] Add path type update logic Only attempts to change points to bezier if points in the slider are modified. --- osu.Game/Rulesets/Objects/SliderPath.cs | 53 +++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/osu.Game/Rulesets/Objects/SliderPath.cs b/osu.Game/Rulesets/Objects/SliderPath.cs index e76699ac04..42f1a33dfc 100644 --- a/osu.Game/Rulesets/Objects/SliderPath.cs +++ b/osu.Game/Rulesets/Objects/SliderPath.cs @@ -54,13 +54,19 @@ namespace osu.Game.Rulesets.Objects { case NotifyCollectionChangedAction.Add: foreach (var c in args.NewItems.Cast()) + { c.Changed += invalidate; + c.Changed += updatePathTypes; + } break; case NotifyCollectionChangedAction.Reset: case NotifyCollectionChangedAction.Remove: foreach (var c in args.OldItems.Cast()) + { c.Changed -= invalidate; + c.Changed -= updatePathTypes; + } break; } @@ -188,6 +194,53 @@ namespace osu.Game.Rulesets.Objects return pointsInCurrentSegment; } + private void updatePathTypes() + { + // Updates each segment of the slider once + foreach (PathControlPoint controlPoint in ControlPoints.Where(p => p.Type.Value is PathType)) + updatePathType(controlPoint); + } + + private void updatePathType(PathControlPoint controlPoint) + { + List pointsInSegment = PointsInSegment(controlPoint); + PathType? pathType = pointsInSegment[0].Type.Value; + + // An almost linear arrangement of points results in radius approaching infinity, + // we should prevent that to avoid memory exhaustion when drawing / approximating + if (pathType == PathType.PerfectCurve) + { + Vector2[] points = pointsInSegment.Select(point => point.Position.Value).ToArray(); + if (points.Length < 3) + return; + + Vector2 a = points[0]; + Vector2 b = points[1]; + Vector2 c = points[2]; + + float maxLength = points.Max(p => p.Length); + + Vector2 normA = new Vector2(a.X / maxLength, a.Y / maxLength); + Vector2 normB = new Vector2(b.X / maxLength, b.Y / maxLength); + Vector2 normC = new Vector2(c.X / maxLength, c.Y / maxLength); + + float det = (normA.X - normB.X) * (normB.Y - normC.Y) - (normB.X - normC.X) * (normA.Y - normB.Y); + + float acSq = (a - c).LengthSquared; + float abSq = (a - b).LengthSquared; + float bcSq = (b - c).LengthSquared; + + // Exterior = curve wraps around the long way between end-points + // Exterior bottleneck is drawing-related, interior bottleneck is approximation-related, + // where the latter is much faster, hence differing thresholds + bool exterior = abSq > acSq || bcSq > acSq; + float threshold = exterior ? 0.05f : 0.001f; + + if (Math.Abs(det) < threshold) + pointsInSegment[0].Type.Value = PathType.Bezier; + } + } + private void invalidate() { pathCache.Invalidate(); From 067178e53779be114ffd067a28144bb6dee94dd4 Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Mon, 22 Mar 2021 15:59:06 +0100 Subject: [PATCH 06/52] Maintain path type when dragging/placing --- .../Sliders/Components/PathControlPointPiece.cs | 10 ++++++++++ .../Blueprints/Sliders/SliderPlacementBlueprint.cs | 3 +++ 2 files changed, 13 insertions(+) diff --git a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointPiece.cs b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointPiece.cs index 1390675a1a..42a7d246ea 100644 --- a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointPiece.cs +++ b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointPiece.cs @@ -2,6 +2,7 @@ // See the LICENCE file in the repository root for full licence text. using System; +using System.Collections.Generic; using osu.Framework.Allocation; using osu.Framework.Bindables; using osu.Framework.Extensions.Color4Extensions; @@ -31,6 +32,7 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components public readonly BindableBool IsSelected = new BindableBool(); public readonly PathControlPoint ControlPoint; + public readonly List PointsInSegment; private readonly Slider slider; private readonly Container marker; @@ -53,6 +55,7 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components { this.slider = slider; ControlPoint = controlPoint; + PointsInSegment = slider.Path.PointsInSegment(controlPoint); controlPoint.Type.BindValueChanged(_ => updateMarkerDisplay()); @@ -150,6 +153,7 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components protected override bool OnClick(ClickEvent e) => RequestSelection != null; private Vector2 dragStartPosition; + private PathType? dragPathType; protected override bool OnDragStart(DragStartEvent e) { @@ -159,6 +163,8 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components if (e.Button == MouseButton.Left) { dragStartPosition = ControlPoint.Position.Value; + dragPathType = PointsInSegment[0].Type.Value; + changeHandler?.BeginChange(); return true; } @@ -184,6 +190,10 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components } else ControlPoint.Position.Value = dragStartPosition + (e.MousePosition - e.MouseDownPosition); + + // Maintain the path type in case it got defaulted to bezier at some point during the drag. + if (PointsInSegment[0].Type.Value != dragPathType) + PointsInSegment[0].Type.Value = dragPathType; } protected override void OnDragEnd(DragEndEvent e) => changeHandler?.EndChange(); diff --git a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/SliderPlacementBlueprint.cs b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/SliderPlacementBlueprint.cs index b71e1914f7..16e2a52279 100644 --- a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/SliderPlacementBlueprint.cs +++ b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/SliderPlacementBlueprint.cs @@ -142,6 +142,9 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders { base.Update(); updateSlider(); + + // Maintain the path type in case it got defaulted to bezier at some point during the drag. + updatePathType(); } private void updatePathType() From 3bddc4a75d2f312541aad38e6408d956c32f9d81 Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Mon, 22 Mar 2021 15:59:45 +0100 Subject: [PATCH 07/52] Add path type test --- .../Editor/TestSceneSliderPlacementBlueprint.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderPlacementBlueprint.cs b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderPlacementBlueprint.cs index f9b6fb924e..4f716a31b7 100644 --- a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderPlacementBlueprint.cs +++ b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderPlacementBlueprint.cs @@ -290,6 +290,7 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor assertPlaced(true); assertControlPointCount(3); + assertControlPointType(0, PathType.Bezier); // Will be > 10000 if not falling back to Bezier path calc. assertLength(218.8901f); } From 15af57de95ec260afef3a59bedc736202c06f297 Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Mon, 22 Mar 2021 15:59:59 +0100 Subject: [PATCH 08/52] Add path type recovery test --- .../TestSceneSliderPlacementBlueprint.cs | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderPlacementBlueprint.cs b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderPlacementBlueprint.cs index 4f716a31b7..db7b3aa973 100644 --- a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderPlacementBlueprint.cs +++ b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderPlacementBlueprint.cs @@ -295,6 +295,25 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor assertLength(218.8901f); } + [Test] + public void TestPlacePerfectCurveSegmentAlmostLinearlyRecovery() + { + addMovementStep(new Vector2(0)); + addClickStep(MouseButton.Left); + + addMovementStep(new Vector2(61.382935f, 6.423767f)); + addClickStep(MouseButton.Left); + + addMovementStep(new Vector2(217.69522f, 22.84021f)); // Should convert to bezier + addMovementStep(new Vector2(210.0f, 30.0f)); // Should convert back to perfect + addClickStep(MouseButton.Right); + + assertPlaced(true); + assertControlPointCount(3); + assertControlPointType(0, PathType.PerfectCurve); + assertLength(212.2276f); + } + private void addMovementStep(Vector2 position) => AddStep($"move mouse to {position}", () => InputManager.MoveMouseTo(InputManager.ToScreenSpace(position))); private void addClickStep(MouseButton button) From 323b875cea686e080c3b58fc04de91e73d24b437 Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Mon, 22 Mar 2021 17:32:40 +0100 Subject: [PATCH 09/52] Fix newlines/spaces --- .../Editor/TestSceneSliderPlacementBlueprint.cs | 4 ++-- osu.Game/Rulesets/Objects/SliderPath.cs | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderPlacementBlueprint.cs b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderPlacementBlueprint.cs index db7b3aa973..e7ea12e538 100644 --- a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderPlacementBlueprint.cs +++ b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderPlacementBlueprint.cs @@ -304,8 +304,8 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor addMovementStep(new Vector2(61.382935f, 6.423767f)); addClickStep(MouseButton.Left); - addMovementStep(new Vector2(217.69522f, 22.84021f)); // Should convert to bezier - addMovementStep(new Vector2(210.0f, 30.0f)); // Should convert back to perfect + addMovementStep(new Vector2(217.69522f, 22.84021f)); // Should convert to bezier + addMovementStep(new Vector2(210.0f, 30.0f)); // Should convert back to perfect addClickStep(MouseButton.Right); assertPlaced(true); diff --git a/osu.Game/Rulesets/Objects/SliderPath.cs b/osu.Game/Rulesets/Objects/SliderPath.cs index 42f1a33dfc..639cd4c72e 100644 --- a/osu.Game/Rulesets/Objects/SliderPath.cs +++ b/osu.Game/Rulesets/Objects/SliderPath.cs @@ -58,6 +58,7 @@ namespace osu.Game.Rulesets.Objects c.Changed += invalidate; c.Changed += updatePathTypes; } + break; case NotifyCollectionChangedAction.Reset: @@ -67,6 +68,7 @@ namespace osu.Game.Rulesets.Objects c.Changed -= invalidate; c.Changed -= updatePathTypes; } + break; } From a7076c329c66752955ee7501b09a1a17b7554b30 Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Mon, 22 Mar 2021 17:32:55 +0100 Subject: [PATCH 10/52] Fix null checks --- osu.Game/Rulesets/Objects/SliderPath.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game/Rulesets/Objects/SliderPath.cs b/osu.Game/Rulesets/Objects/SliderPath.cs index 639cd4c72e..47cc2c6044 100644 --- a/osu.Game/Rulesets/Objects/SliderPath.cs +++ b/osu.Game/Rulesets/Objects/SliderPath.cs @@ -176,7 +176,7 @@ namespace osu.Game.Rulesets.Objects List pointsInCurrentSegment = new List(); foreach (PathControlPoint point in ControlPoints) { - if (point.Type.Value is PathType) + if (point.Type.Value != null) { if (!found) pointsInCurrentSegment.Clear(); @@ -199,7 +199,7 @@ namespace osu.Game.Rulesets.Objects private void updatePathTypes() { // Updates each segment of the slider once - foreach (PathControlPoint controlPoint in ControlPoints.Where(p => p.Type.Value is PathType)) + foreach (PathControlPoint controlPoint in ControlPoints.Where(p => p.Type.Value != null)) updatePathType(controlPoint); } From 6911a1b41546c04252b4d9e92bcbbf511befbf2a Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Mon, 22 Mar 2021 19:03:55 +0100 Subject: [PATCH 11/52] Fix missing newline --- osu.Game/Rulesets/Objects/SliderPath.cs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/osu.Game/Rulesets/Objects/SliderPath.cs b/osu.Game/Rulesets/Objects/SliderPath.cs index 47cc2c6044..d8fbb46b76 100644 --- a/osu.Game/Rulesets/Objects/SliderPath.cs +++ b/osu.Game/Rulesets/Objects/SliderPath.cs @@ -174,6 +174,7 @@ namespace osu.Game.Rulesets.Objects { bool found = false; List pointsInCurrentSegment = new List(); + foreach (PathControlPoint point in ControlPoints) { if (point.Type.Value != null) @@ -241,6 +242,12 @@ namespace osu.Game.Rulesets.Objects if (Math.Abs(det) < threshold) pointsInSegment[0].Type.Value = PathType.Bezier; } + // where the latter is much faster, hence differing thresholds + bool exterior = abSq > acSq || bcSq > acSq; + float threshold = exterior ? 0.05f : 0.001f; + + if (Math.Abs(det) < threshold) + pointsInSegment[0].Type.Value = PathType.Bezier; } private void invalidate() From 80e7c3aba7e804ae1f1696219ee8c9a48871abb0 Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Mon, 22 Mar 2021 19:09:28 +0100 Subject: [PATCH 12/52] Invert if statement --- osu.Game/Rulesets/Objects/SliderPath.cs | 42 +++++++++++-------------- 1 file changed, 18 insertions(+), 24 deletions(-) diff --git a/osu.Game/Rulesets/Objects/SliderPath.cs b/osu.Game/Rulesets/Objects/SliderPath.cs index d8fbb46b76..767f7909d5 100644 --- a/osu.Game/Rulesets/Objects/SliderPath.cs +++ b/osu.Game/Rulesets/Objects/SliderPath.cs @@ -211,37 +211,31 @@ namespace osu.Game.Rulesets.Objects // An almost linear arrangement of points results in radius approaching infinity, // we should prevent that to avoid memory exhaustion when drawing / approximating - if (pathType == PathType.PerfectCurve) - { - Vector2[] points = pointsInSegment.Select(point => point.Position.Value).ToArray(); - if (points.Length < 3) - return; + if (pathType != PathType.PerfectCurve) + return; - Vector2 a = points[0]; - Vector2 b = points[1]; - Vector2 c = points[2]; + Vector2[] points = pointsInSegment.Select(point => point.Position.Value).ToArray(); + if (points.Length < 3) + return; - float maxLength = points.Max(p => p.Length); + Vector2 a = points[0]; + Vector2 b = points[1]; + Vector2 c = points[2]; - Vector2 normA = new Vector2(a.X / maxLength, a.Y / maxLength); - Vector2 normB = new Vector2(b.X / maxLength, b.Y / maxLength); - Vector2 normC = new Vector2(c.X / maxLength, c.Y / maxLength); + float maxLength = points.Max(p => p.Length); - float det = (normA.X - normB.X) * (normB.Y - normC.Y) - (normB.X - normC.X) * (normA.Y - normB.Y); + Vector2 normA = new Vector2(a.X / maxLength, a.Y / maxLength); + Vector2 normB = new Vector2(b.X / maxLength, b.Y / maxLength); + Vector2 normC = new Vector2(c.X / maxLength, c.Y / maxLength); - float acSq = (a - c).LengthSquared; - float abSq = (a - b).LengthSquared; - float bcSq = (b - c).LengthSquared; + float det = (normA.X - normB.X) * (normB.Y - normC.Y) - (normB.X - normC.X) * (normA.Y - normB.Y); - // Exterior = curve wraps around the long way between end-points - // Exterior bottleneck is drawing-related, interior bottleneck is approximation-related, - // where the latter is much faster, hence differing thresholds - bool exterior = abSq > acSq || bcSq > acSq; - float threshold = exterior ? 0.05f : 0.001f; + float acSq = (a - c).LengthSquared; + float abSq = (a - b).LengthSquared; + float bcSq = (b - c).LengthSquared; - if (Math.Abs(det) < threshold) - pointsInSegment[0].Type.Value = PathType.Bezier; - } + // Exterior = curve wraps around the long way between end-points + // Exterior bottleneck is drawing-related, interior bottleneck is approximation-related, // where the latter is much faster, hence differing thresholds bool exterior = abSq > acSq || bcSq > acSq; float threshold = exterior ? 0.05f : 0.001f; From 92f713a30e57d86b9c6408004be911f8c40755ca Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Mon, 22 Mar 2021 19:10:56 +0100 Subject: [PATCH 13/52] Improve fallback conditions It's possible to create a `PerfectCurve` type path with more than 3 points currently, so this accounts for that. --- osu.Game/Rulesets/Objects/SliderPath.cs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/osu.Game/Rulesets/Objects/SliderPath.cs b/osu.Game/Rulesets/Objects/SliderPath.cs index 767f7909d5..23511be5e4 100644 --- a/osu.Game/Rulesets/Objects/SliderPath.cs +++ b/osu.Game/Rulesets/Objects/SliderPath.cs @@ -211,12 +211,10 @@ namespace osu.Game.Rulesets.Objects // An almost linear arrangement of points results in radius approaching infinity, // we should prevent that to avoid memory exhaustion when drawing / approximating - if (pathType != PathType.PerfectCurve) + if (pathType != PathType.PerfectCurve || pointsInSegment.Count != 3) return; Vector2[] points = pointsInSegment.Select(point => point.Position.Value).ToArray(); - if (points.Length < 3) - return; Vector2 a = points[0]; Vector2 b = points[1]; From b11fd7972aafea70905a1f0f73addffe82442d91 Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Mon, 22 Mar 2021 19:41:48 +0100 Subject: [PATCH 14/52] Separate condition logic from math logic --- osu.Game/Rulesets/Objects/SliderPath.cs | 27 ++++++++++--------------- 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/osu.Game/Rulesets/Objects/SliderPath.cs b/osu.Game/Rulesets/Objects/SliderPath.cs index 23511be5e4..ae08660404 100644 --- a/osu.Game/Rulesets/Objects/SliderPath.cs +++ b/osu.Game/Rulesets/Objects/SliderPath.cs @@ -199,23 +199,19 @@ namespace osu.Game.Rulesets.Objects private void updatePathTypes() { - // Updates each segment of the slider once - foreach (PathControlPoint controlPoint in ControlPoints.Where(p => p.Type.Value != null)) - updatePathType(controlPoint); + foreach (PathControlPoint segmentStartPoint in ControlPoints.Where(p => p.Type.Value != null)) + { + if (segmentStartPoint.Type.Value != PathType.PerfectCurve) + continue; + + Vector2[] points = PointsInSegment(segmentStartPoint).Select(p => p.Position.Value).ToArray(); + if (points.Length == 3 && !validCircularArcSegment(points)) + segmentStartPoint.Type.Value = PathType.Bezier; + } } - private void updatePathType(PathControlPoint controlPoint) + private bool validCircularArcSegment(IReadOnlyList points) { - List pointsInSegment = PointsInSegment(controlPoint); - PathType? pathType = pointsInSegment[0].Type.Value; - - // An almost linear arrangement of points results in radius approaching infinity, - // we should prevent that to avoid memory exhaustion when drawing / approximating - if (pathType != PathType.PerfectCurve || pointsInSegment.Count != 3) - return; - - Vector2[] points = pointsInSegment.Select(point => point.Position.Value).ToArray(); - Vector2 a = points[0]; Vector2 b = points[1]; Vector2 c = points[2]; @@ -238,8 +234,7 @@ namespace osu.Game.Rulesets.Objects bool exterior = abSq > acSq || bcSq > acSq; float threshold = exterior ? 0.05f : 0.001f; - if (Math.Abs(det) < threshold) - pointsInSegment[0].Type.Value = PathType.Bezier; + return Math.Abs(det) < threshold; } private void invalidate() From 3fa5852e00cbb6d43e0383e5044dcd487ab43b2a Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Mon, 22 Mar 2021 19:42:27 +0100 Subject: [PATCH 15/52] Add method documentation --- osu.Game/Rulesets/Objects/SliderPath.cs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/osu.Game/Rulesets/Objects/SliderPath.cs b/osu.Game/Rulesets/Objects/SliderPath.cs index ae08660404..b048bf2a84 100644 --- a/osu.Game/Rulesets/Objects/SliderPath.cs +++ b/osu.Game/Rulesets/Objects/SliderPath.cs @@ -197,6 +197,9 @@ namespace osu.Game.Rulesets.Objects return pointsInCurrentSegment; } + /// + /// Handles correction of invalid path types. + /// private void updatePathTypes() { foreach (PathControlPoint segmentStartPoint in ControlPoints.Where(p => p.Type.Value != null)) @@ -210,6 +213,13 @@ namespace osu.Game.Rulesets.Objects } } + /// + /// Returns whether the given points are arranged in a valid way. Invalid if points + /// are almost entirely linear - as this causes the radius to approach infinity, + /// which would exhaust memory when drawing / approximating. + /// + /// The three points that make up this circular arc segment. + /// private bool validCircularArcSegment(IReadOnlyList points) { Vector2 a = points[0]; From e922e67c9847b8dfea62bed7c85d673acd6278bd Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Mon, 22 Mar 2021 19:48:21 +0100 Subject: [PATCH 16/52] Fix inverted return statement Forgot to run tests, all passing now. --- osu.Game/Rulesets/Objects/SliderPath.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Rulesets/Objects/SliderPath.cs b/osu.Game/Rulesets/Objects/SliderPath.cs index b048bf2a84..c7931b440b 100644 --- a/osu.Game/Rulesets/Objects/SliderPath.cs +++ b/osu.Game/Rulesets/Objects/SliderPath.cs @@ -244,7 +244,7 @@ namespace osu.Game.Rulesets.Objects bool exterior = abSq > acSq || bcSq > acSq; float threshold = exterior ? 0.05f : 0.001f; - return Math.Abs(det) < threshold; + return Math.Abs(det) >= threshold; } private void invalidate() From 5ee280f941b5738adf98211646b8f12cc3719702 Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Wed, 24 Mar 2021 02:56:32 +0100 Subject: [PATCH 17/52] Update PointsInSegment when adding/removing points There was a bug where if you created a slider, moved the last point, and then added a point such that it became a PerfectCurve, it would fail to recover after becoming a Bezier. This fixes that. --- .../Blueprints/Sliders/Components/PathControlPointPiece.cs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointPiece.cs b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointPiece.cs index 42a7d246ea..f34403b377 100644 --- a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointPiece.cs +++ b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointPiece.cs @@ -29,10 +29,10 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components public class PathControlPointPiece : BlueprintPiece, IHasTooltip { public Action RequestSelection; + public List PointsInSegment; public readonly BindableBool IsSelected = new BindableBool(); public readonly PathControlPoint ControlPoint; - public readonly List PointsInSegment; private readonly Slider slider; private readonly Container marker; @@ -55,7 +55,10 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components { this.slider = slider; ControlPoint = controlPoint; - PointsInSegment = slider.Path.PointsInSegment(controlPoint); + slider.Path.ControlPoints.BindCollectionChanged((_, args) => + { + PointsInSegment = slider.Path.PointsInSegment(controlPoint); + }, runOnceImmediately: true); controlPoint.Type.BindValueChanged(_ => updateMarkerDisplay()); From 0bcd38e6613fff3009a7e59f55c422751cf9e6e5 Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Wed, 24 Mar 2021 02:57:47 +0100 Subject: [PATCH 18/52] Simplify path type maintenance when dragging --- .../Blueprints/Sliders/Components/PathControlPointPiece.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointPiece.cs b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointPiece.cs index f34403b377..4459308ea5 100644 --- a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointPiece.cs +++ b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointPiece.cs @@ -195,8 +195,7 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components ControlPoint.Position.Value = dragStartPosition + (e.MousePosition - e.MouseDownPosition); // Maintain the path type in case it got defaulted to bezier at some point during the drag. - if (PointsInSegment[0].Type.Value != dragPathType) - PointsInSegment[0].Type.Value = dragPathType; + PointsInSegment[0].Type.Value = dragPathType; } protected override void OnDragEnd(DragEndEvent e) => changeHandler?.EndChange(); From 4ae3eaaac6265b54c649fc64789c84ebd87f4400 Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Wed, 24 Mar 2021 03:02:19 +0100 Subject: [PATCH 19/52] Move path type correction This is better because `PathControlPointVisualizer` is local to the editor, meaning there is no chance that this could affect gameplay. --- .../Components/PathControlPointVisualiser.cs | 54 +++++++++++++++++ osu.Game/Rulesets/Objects/PathControlPoint.cs | 2 +- osu.Game/Rulesets/Objects/SliderPath.cs | 58 ------------------- 3 files changed, 55 insertions(+), 59 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointVisualiser.cs b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointVisualiser.cs index ce5dc4855e..0c9163ae41 100644 --- a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointVisualiser.cs +++ b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointVisualiser.cs @@ -91,6 +91,8 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components })); Connections.Add(new PathControlPointConnectionPiece(slider, e.NewStartingIndex + i)); + + point.Changed += updatePathTypes; } break; @@ -100,6 +102,8 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components { Pieces.RemoveAll(p => p.ControlPoint == point); Connections.RemoveAll(c => c.ControlPoint == point); + + point.Changed -= updatePathTypes; } // If removing before the end of the path, @@ -142,6 +146,56 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components { } + /// + /// Handles correction of invalid path types. + /// + private void updatePathTypes() + { + foreach (PathControlPoint segmentStartPoint in slider.Path.ControlPoints.Where(p => p.Type.Value != null)) + { + if (segmentStartPoint.Type.Value != PathType.PerfectCurve) + continue; + + Vector2[] points = slider.Path.PointsInSegment(segmentStartPoint).Select(p => p.Position.Value).ToArray(); + if (points.Length == 3 && !validCircularArcSegment(points)) + segmentStartPoint.Type.Value = PathType.Bezier; + } + } + + /// + /// Returns whether the given points are arranged in a valid way. Invalid if points + /// are almost entirely linear - as this causes the radius to approach infinity, + /// which would exhaust memory when drawing / approximating. + /// + /// The three points that make up this circular arc segment. + /// + private bool validCircularArcSegment(IReadOnlyList points) + { + Vector2 a = points[0]; + Vector2 b = points[1]; + Vector2 c = points[2]; + + float maxLength = points.Max(p => p.Length); + + Vector2 normA = new Vector2(a.X / maxLength, a.Y / maxLength); + Vector2 normB = new Vector2(b.X / maxLength, b.Y / maxLength); + Vector2 normC = new Vector2(c.X / maxLength, c.Y / maxLength); + + float det = (normA.X - normB.X) * (normB.Y - normC.Y) - (normB.X - normC.X) * (normA.Y - normB.Y); + + float acSq = (a - c).LengthSquared; + float abSq = (a - b).LengthSquared; + float bcSq = (b - c).LengthSquared; + + // Exterior = curve wraps around the long way between end-points + // Exterior bottleneck is drawing-related, interior bottleneck is approximation-related, + // where the latter is much faster, hence differing thresholds + bool exterior = abSq > acSq || bcSq > acSq; + float threshold = exterior ? 0.05f : 0.001f; + + return Math.Abs(det) >= threshold; + } + private void selectPiece(PathControlPointPiece piece, MouseButtonEvent e) { if (e.Button == MouseButton.Left && inputManager.CurrentState.Keyboard.ControlPressed) diff --git a/osu.Game/Rulesets/Objects/PathControlPoint.cs b/osu.Game/Rulesets/Objects/PathControlPoint.cs index f11917f4f4..2e4100ee0b 100644 --- a/osu.Game/Rulesets/Objects/PathControlPoint.cs +++ b/osu.Game/Rulesets/Objects/PathControlPoint.cs @@ -27,7 +27,7 @@ namespace osu.Game.Rulesets.Objects /// /// Invoked when any property of this is changed. /// - internal event Action Changed; + public event Action Changed; /// /// Creates a new . diff --git a/osu.Game/Rulesets/Objects/SliderPath.cs b/osu.Game/Rulesets/Objects/SliderPath.cs index c7931b440b..61f5f94142 100644 --- a/osu.Game/Rulesets/Objects/SliderPath.cs +++ b/osu.Game/Rulesets/Objects/SliderPath.cs @@ -54,21 +54,13 @@ namespace osu.Game.Rulesets.Objects { case NotifyCollectionChangedAction.Add: foreach (var c in args.NewItems.Cast()) - { c.Changed += invalidate; - c.Changed += updatePathTypes; - } - break; case NotifyCollectionChangedAction.Reset: case NotifyCollectionChangedAction.Remove: foreach (var c in args.OldItems.Cast()) - { c.Changed -= invalidate; - c.Changed -= updatePathTypes; - } - break; } @@ -197,56 +189,6 @@ namespace osu.Game.Rulesets.Objects return pointsInCurrentSegment; } - /// - /// Handles correction of invalid path types. - /// - private void updatePathTypes() - { - foreach (PathControlPoint segmentStartPoint in ControlPoints.Where(p => p.Type.Value != null)) - { - if (segmentStartPoint.Type.Value != PathType.PerfectCurve) - continue; - - Vector2[] points = PointsInSegment(segmentStartPoint).Select(p => p.Position.Value).ToArray(); - if (points.Length == 3 && !validCircularArcSegment(points)) - segmentStartPoint.Type.Value = PathType.Bezier; - } - } - - /// - /// Returns whether the given points are arranged in a valid way. Invalid if points - /// are almost entirely linear - as this causes the radius to approach infinity, - /// which would exhaust memory when drawing / approximating. - /// - /// The three points that make up this circular arc segment. - /// - private bool validCircularArcSegment(IReadOnlyList points) - { - Vector2 a = points[0]; - Vector2 b = points[1]; - Vector2 c = points[2]; - - float maxLength = points.Max(p => p.Length); - - Vector2 normA = new Vector2(a.X / maxLength, a.Y / maxLength); - Vector2 normB = new Vector2(b.X / maxLength, b.Y / maxLength); - Vector2 normC = new Vector2(c.X / maxLength, c.Y / maxLength); - - float det = (normA.X - normB.X) * (normB.Y - normC.Y) - (normB.X - normC.X) * (normA.Y - normB.Y); - - float acSq = (a - c).LengthSquared; - float abSq = (a - b).LengthSquared; - float bcSq = (b - c).LengthSquared; - - // Exterior = curve wraps around the long way between end-points - // Exterior bottleneck is drawing-related, interior bottleneck is approximation-related, - // where the latter is much faster, hence differing thresholds - bool exterior = abSq > acSq || bcSq > acSq; - float threshold = exterior ? 0.05f : 0.001f; - - return Math.Abs(det) >= threshold; - } - private void invalidate() { pathCache.Invalidate(); From 7bae4ff43d1d90c30dd874e54183e60934dd7fd3 Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Wed, 24 Mar 2021 05:06:04 +0100 Subject: [PATCH 20/52] Add control point dragging tests --- .../TestSceneSliderControlPointPiece.cs | 170 ++++++++++++++++++ 1 file changed, 170 insertions(+) create mode 100644 osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderControlPointPiece.cs diff --git a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderControlPointPiece.cs b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderControlPointPiece.cs new file mode 100644 index 0000000000..c4f103e40c --- /dev/null +++ b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderControlPointPiece.cs @@ -0,0 +1,170 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using NUnit.Framework; +using osu.Framework.Utils; +using osu.Game.Beatmaps; +using osu.Game.Beatmaps.ControlPoints; +using osu.Game.Rulesets.Objects; +using osu.Game.Rulesets.Objects.Types; +using osu.Game.Rulesets.Osu.Edit.Blueprints.HitCircles.Components; +using osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders; +using osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components; +using osu.Game.Rulesets.Osu.Objects; +using osu.Game.Rulesets.Osu.Objects.Drawables; +using osu.Game.Tests.Visual; +using osuTK; +using osuTK.Input; + +namespace osu.Game.Rulesets.Osu.Tests.Editor +{ + public class TestSceneSliderControlPointPiece : SelectionBlueprintTestScene + { + private Slider slider; + private DrawableSlider drawableObject; + private TestSliderBlueprint blueprint; + + [SetUp] + public void Setup() => Schedule(() => + { + Clear(); + + slider = new Slider + { + Position = new Vector2(256, 192), + Path = new SliderPath(new[] + { + new PathControlPoint(Vector2.Zero, PathType.PerfectCurve), + new PathControlPoint(new Vector2(150, 150)), + new PathControlPoint(new Vector2(300, 0), PathType.PerfectCurve), + new PathControlPoint(new Vector2(400, 0)), + new PathControlPoint(new Vector2(400, 150)) + }) + }; + + slider.ApplyDefaults(new ControlPointInfo(), new BeatmapDifficulty { CircleSize = 2 }); + + Add(drawableObject = new DrawableSlider(slider)); + AddBlueprint(blueprint = new TestSliderBlueprint(drawableObject)); + }); + + [Test] + public void TestDragControlPoint() + { + moveMouseToControlPoint(1); + AddStep("hold", () => InputManager.PressButton(MouseButton.Left)); + + addMovementStep(new Vector2(150, 50)); + AddStep("release", () => InputManager.ReleaseButton(MouseButton.Left)); + + assertControlPointPosition(1, new Vector2(150, 50)); + assertControlPointType(0, PathType.PerfectCurve); + } + + [Test] + public void TestDragControlPointAlmostLinearly() + { + moveMouseToControlPoint(1); + AddStep("hold", () => InputManager.PressButton(MouseButton.Left)); + + addMovementStep(new Vector2(150, 0.01f)); + AddStep("release", () => InputManager.ReleaseButton(MouseButton.Left)); + + assertControlPointPosition(1, new Vector2(150, 0.01f)); + assertControlPointType(0, PathType.Bezier); + } + + [Test] + public void TestDragControlPointAlmostLinearlyExterior() + { + moveMouseToControlPoint(1); + AddStep("hold", () => InputManager.PressButton(MouseButton.Left)); + + addMovementStep(new Vector2(400, 0.01f)); + AddStep("release", () => InputManager.ReleaseButton(MouseButton.Left)); + + assertControlPointPosition(1, new Vector2(400, 0.01f)); + assertControlPointType(0, PathType.Bezier); + } + + [Test] + public void TestDragControlPointPathRecovery() + { + moveMouseToControlPoint(1); + AddStep("hold", () => InputManager.PressButton(MouseButton.Left)); + + addMovementStep(new Vector2(150, 0.01f)); + assertControlPointType(0, PathType.Bezier); + + addMovementStep(new Vector2(150, 50)); + AddStep("release", () => InputManager.ReleaseButton(MouseButton.Left)); + + assertControlPointPosition(1, new Vector2(150, 50)); + assertControlPointType(0, PathType.PerfectCurve); + } + + [Test] + public void TestDragControlPointPathRecoveryOtherSegment() + { + moveMouseToControlPoint(4); + AddStep("hold", () => InputManager.PressButton(MouseButton.Left)); + + addMovementStep(new Vector2(350, 0)); + assertControlPointType(2, PathType.Bezier); + + addMovementStep(new Vector2(150, 150)); + AddStep("release", () => InputManager.ReleaseButton(MouseButton.Left)); + + assertControlPointPosition(1, new Vector2(150, 150)); + assertControlPointType(2, PathType.PerfectCurve); + } + + private void addMovementStep(Vector2 relativePosition) + { + AddStep($"move mouse to {relativePosition}", () => + { + Vector2 position = slider.Position + relativePosition; + InputManager.MoveMouseTo(drawableObject.Parent.ToScreenSpace(position)); + }); + } + + private void moveMouseToControlPoint(int index) + { + AddStep($"move mouse to control point {index}", () => + { + Vector2 position = slider.Position + slider.Path.ControlPoints[index].Position.Value; + InputManager.MoveMouseTo(drawableObject.Parent.ToScreenSpace(position)); + }); + } + + private void assertControlPointType(int index, PathType type) => AddAssert($"control point {index} is {type}", () => slider.Path.ControlPoints[index].Type.Value == type); + + private void assertControlPointPosition(int index, Vector2 position) => + AddAssert($"control point {index} at {position}", () => Precision.AlmostEquals(position, slider.Path.ControlPoints[index].Position.Value, 1)); + + private class TestSliderBlueprint : SliderSelectionBlueprint + { + public new SliderBodyPiece BodyPiece => base.BodyPiece; + public new TestSliderCircleBlueprint HeadBlueprint => (TestSliderCircleBlueprint)base.HeadBlueprint; + public new TestSliderCircleBlueprint TailBlueprint => (TestSliderCircleBlueprint)base.TailBlueprint; + public new PathControlPointVisualiser ControlPointVisualiser => base.ControlPointVisualiser; + + public TestSliderBlueprint(DrawableSlider slider) + : base(slider) + { + } + + protected override SliderCircleSelectionBlueprint CreateCircleSelectionBlueprint(DrawableSlider slider, SliderPosition position) => new TestSliderCircleBlueprint(slider, position); + } + + private class TestSliderCircleBlueprint : SliderCircleSelectionBlueprint + { + public new HitCirclePiece CirclePiece => base.CirclePiece; + + public TestSliderCircleBlueprint(DrawableSlider slider, SliderPosition position) + : base(slider, position) + { + } + } + } +} From 847d44c7d9998b9d2429fcc9bcef967004d314d4 Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Wed, 24 Mar 2021 05:13:37 +0100 Subject: [PATCH 21/52] Remove unnecessary length asserts We don't actually care about the length (as this isn't what we're testing), just the type of the slider. --- .../Editor/TestSceneSliderPlacementBlueprint.cs | 3 --- 1 file changed, 3 deletions(-) diff --git a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderPlacementBlueprint.cs b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderPlacementBlueprint.cs index e7ea12e538..e47d86428b 100644 --- a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderPlacementBlueprint.cs +++ b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderPlacementBlueprint.cs @@ -291,8 +291,6 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor assertPlaced(true); assertControlPointCount(3); assertControlPointType(0, PathType.Bezier); - // Will be > 10000 if not falling back to Bezier path calc. - assertLength(218.8901f); } [Test] @@ -311,7 +309,6 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor assertPlaced(true); assertControlPointCount(3); assertControlPointType(0, PathType.PerfectCurve); - assertLength(212.2276f); } private void addMovementStep(Vector2 position) => AddStep($"move mouse to {position}", () => InputManager.MoveMouseTo(InputManager.ToScreenSpace(position))); From 6fbe5300163bea139a466c1be2bb09a63e2bd830 Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Wed, 24 Mar 2021 05:14:35 +0100 Subject: [PATCH 22/52] Fix coordinates --- .../TestSceneSliderPlacementBlueprint.cs | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderPlacementBlueprint.cs b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderPlacementBlueprint.cs index e47d86428b..6eec5edfbe 100644 --- a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderPlacementBlueprint.cs +++ b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderPlacementBlueprint.cs @@ -279,13 +279,15 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor [Test] public void TestPlacePerfectCurveSegmentAlmostLinearly() { - addMovementStep(new Vector2(0)); + Vector2 startPosition = new Vector2(200); + + addMovementStep(startPosition); addClickStep(MouseButton.Left); - addMovementStep(new Vector2(61.382935f, 6.423767f)); + addMovementStep(startPosition + new Vector2(300, 0)); addClickStep(MouseButton.Left); - addMovementStep(new Vector2(217.69522f, 22.84021f)); + addMovementStep(startPosition + new Vector2(150, 0.1f)); addClickStep(MouseButton.Right); assertPlaced(true); @@ -296,14 +298,16 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor [Test] public void TestPlacePerfectCurveSegmentAlmostLinearlyRecovery() { - addMovementStep(new Vector2(0)); + Vector2 startPosition = new Vector2(200); + + addMovementStep(startPosition); addClickStep(MouseButton.Left); - addMovementStep(new Vector2(61.382935f, 6.423767f)); + addMovementStep(startPosition + new Vector2(61.382935f, 6.423767f)); addClickStep(MouseButton.Left); - addMovementStep(new Vector2(217.69522f, 22.84021f)); // Should convert to bezier - addMovementStep(new Vector2(210.0f, 30.0f)); // Should convert back to perfect + addMovementStep(startPosition + new Vector2(217.69522f, 22.84021f)); // Should convert to bezier + addMovementStep(startPosition + new Vector2(210.0f, 0.0f)); // Should convert back to perfect addClickStep(MouseButton.Right); assertPlaced(true); From 23a4d1c1350ef03a05b3c53b2c73be37ef18a4a7 Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Wed, 24 Mar 2021 05:15:28 +0100 Subject: [PATCH 23/52] Shorten recovery test name --- .../Editor/TestSceneSliderPlacementBlueprint.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderPlacementBlueprint.cs b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderPlacementBlueprint.cs index 6eec5edfbe..e42ceaa4ac 100644 --- a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderPlacementBlueprint.cs +++ b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderPlacementBlueprint.cs @@ -296,7 +296,7 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor } [Test] - public void TestPlacePerfectCurveSegmentAlmostLinearlyRecovery() + public void TestPlacePerfectCurveSegmentRecovery() { Vector2 startPosition = new Vector2(200); From 7b395ed783e2811e9e29f73e287965b9bafe8b57 Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Wed, 24 Mar 2021 05:15:50 +0100 Subject: [PATCH 24/52] Add exterior arc test --- .../TestSceneSliderPlacementBlueprint.cs | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderPlacementBlueprint.cs b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderPlacementBlueprint.cs index e42ceaa4ac..0433acb25c 100644 --- a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderPlacementBlueprint.cs +++ b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderPlacementBlueprint.cs @@ -284,6 +284,25 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor addMovementStep(startPosition); addClickStep(MouseButton.Left); + addMovementStep(startPosition + new Vector2(61.382935f, 6.423767f)); + addClickStep(MouseButton.Left); + + addMovementStep(startPosition + new Vector2(217.69522f, 22.84021f)); + addClickStep(MouseButton.Right); + + assertPlaced(true); + assertControlPointCount(3); + assertControlPointType(0, PathType.Bezier); + } + + [Test] + public void TestPlacePerfectCurveSegmentAlmostLinearlyExterior() + { + Vector2 startPosition = new Vector2(200); + + addMovementStep(startPosition); + addClickStep(MouseButton.Left); + addMovementStep(startPosition + new Vector2(300, 0)); addClickStep(MouseButton.Left); From f80b3ada25f1fe688262c95df3dc65f7862479ce Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Wed, 24 Mar 2021 05:54:48 +0100 Subject: [PATCH 25/52] Add circular arc size tests --- .../TestSceneSliderPlacementBlueprint.cs | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderPlacementBlueprint.cs b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderPlacementBlueprint.cs index 0433acb25c..20df46cd2c 100644 --- a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderPlacementBlueprint.cs +++ b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderPlacementBlueprint.cs @@ -334,6 +334,40 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor assertControlPointType(0, PathType.PerfectCurve); } + [Test] + public void TestPlacePerfectCurveSegmentLarge() + { + addMovementStep(new Vector2(200)); + addClickStep(MouseButton.Left); + + addMovementStep(new Vector2(200, 800)); + addClickStep(MouseButton.Left); + + addMovementStep(new Vector2(600, 200)); + addClickStep(MouseButton.Right); + + assertPlaced(true); + assertControlPointCount(3); + assertControlPointType(0, PathType.PerfectCurve); + } + + [Test] + public void TestPlacePerfectCurveSegmentTooLarge() + { + addMovementStep(new Vector2(200)); + addClickStep(MouseButton.Left); + + addMovementStep(new Vector2(200, 800)); + addClickStep(MouseButton.Left); + + addMovementStep(new Vector2(400, 200)); + addClickStep(MouseButton.Right); + + assertPlaced(true); + assertControlPointCount(3); + assertControlPointType(0, PathType.Bezier); + } + private void addMovementStep(Vector2 position) => AddStep($"move mouse to {position}", () => InputManager.MoveMouseTo(InputManager.ToScreenSpace(position))); private void addClickStep(MouseButton button) From e0240ab9d93db5e4d2467c4002e9a2784ff0e1a1 Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Wed, 24 Mar 2021 05:55:34 +0100 Subject: [PATCH 26/52] Increase exterior threshold --- .../Blueprints/Sliders/Components/PathControlPointVisualiser.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointVisualiser.cs b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointVisualiser.cs index 0c9163ae41..c4a16f9c2a 100644 --- a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointVisualiser.cs +++ b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointVisualiser.cs @@ -191,7 +191,7 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components // Exterior bottleneck is drawing-related, interior bottleneck is approximation-related, // where the latter is much faster, hence differing thresholds bool exterior = abSq > acSq || bcSq > acSq; - float threshold = exterior ? 0.05f : 0.001f; + float threshold = exterior ? 0.35f : 0.001f; return Math.Abs(det) >= threshold; } From 415797aadd3c77de155ebdefc53d1c22cb83a723 Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Wed, 24 Mar 2021 06:01:12 +0100 Subject: [PATCH 27/52] Fix broken control point drag test Broken for 2 reasons: - Assert checks the wrong control point. - The exterior arc is now too big. This fixes both. --- .../Editor/TestSceneSliderControlPointPiece.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderControlPointPiece.cs b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderControlPointPiece.cs index c4f103e40c..c3492fc843 100644 --- a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderControlPointPiece.cs +++ b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderControlPointPiece.cs @@ -112,10 +112,10 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor addMovementStep(new Vector2(350, 0)); assertControlPointType(2, PathType.Bezier); - addMovementStep(new Vector2(150, 150)); + addMovementStep(new Vector2(400, 50)); AddStep("release", () => InputManager.ReleaseButton(MouseButton.Left)); - assertControlPointPosition(1, new Vector2(150, 150)); + assertControlPointPosition(4, new Vector2(400, 50)); assertControlPointType(2, PathType.PerfectCurve); } From b4dc35f66ba3c8c6761fdb8dc381057bb13ce73a Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Wed, 24 Mar 2021 17:24:05 +0100 Subject: [PATCH 28/52] Update large arc tests Should now be more robust and readable. --- .../TestSceneSliderControlPointPiece.cs | 4 ++-- .../TestSceneSliderPlacementBlueprint.cs | 20 +++++++++++++------ 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderControlPointPiece.cs b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderControlPointPiece.cs index c3492fc843..7ca86335ce 100644 --- a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderControlPointPiece.cs +++ b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderControlPointPiece.cs @@ -112,10 +112,10 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor addMovementStep(new Vector2(350, 0)); assertControlPointType(2, PathType.Bezier); - addMovementStep(new Vector2(400, 50)); + addMovementStep(new Vector2(150, 150)); AddStep("release", () => InputManager.ReleaseButton(MouseButton.Left)); - assertControlPointPosition(4, new Vector2(400, 50)); + assertControlPointPosition(4, new Vector2(150, 150)); assertControlPointType(2, PathType.PerfectCurve); } diff --git a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderPlacementBlueprint.cs b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderPlacementBlueprint.cs index 20df46cd2c..46fe7b7576 100644 --- a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderPlacementBlueprint.cs +++ b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderPlacementBlueprint.cs @@ -337,13 +337,17 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor [Test] public void TestPlacePerfectCurveSegmentLarge() { - addMovementStep(new Vector2(200)); + Vector2 startPosition = new Vector2(400); + + addMovementStep(startPosition); addClickStep(MouseButton.Left); - addMovementStep(new Vector2(200, 800)); + addMovementStep(startPosition + new Vector2(240, 240)); addClickStep(MouseButton.Left); - addMovementStep(new Vector2(600, 200)); + // Playfield dimensions are 640 x 480. + // So a 480 * 480 bounding box area should be ok. + addMovementStep(startPosition + new Vector2(-240, 240)); addClickStep(MouseButton.Right); assertPlaced(true); @@ -354,13 +358,17 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor [Test] public void TestPlacePerfectCurveSegmentTooLarge() { - addMovementStep(new Vector2(200)); + Vector2 startPosition = new Vector2(480, 200); + + addMovementStep(startPosition); addClickStep(MouseButton.Left); - addMovementStep(new Vector2(200, 800)); + addMovementStep(startPosition + new Vector2(400, 400)); addClickStep(MouseButton.Left); - addMovementStep(new Vector2(400, 200)); + // Playfield dimensions are 640 x 480. + // So an 800 * 800 bounding box area should not be ok. + addMovementStep(startPosition + new Vector2(-400, 400)); addClickStep(MouseButton.Right); assertPlaced(true); From 0f4314c1d8e6741037df4407326d186fb8c3210b Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Wed, 24 Mar 2021 17:24:33 +0100 Subject: [PATCH 29/52] Add complete arc test Ensures we can still make smaller circles properly. --- .../Editor/TestSceneSliderPlacementBlueprint.cs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderPlacementBlueprint.cs b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderPlacementBlueprint.cs index 46fe7b7576..937034fc23 100644 --- a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderPlacementBlueprint.cs +++ b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderPlacementBlueprint.cs @@ -376,6 +376,23 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor assertControlPointType(0, PathType.Bezier); } + [Test] + public void TestPlacePerfectCurveSegmentCompleteArc() + { + addMovementStep(new Vector2(400)); + addClickStep(MouseButton.Left); + + addMovementStep(new Vector2(600, 400)); + addClickStep(MouseButton.Left); + + addMovementStep(new Vector2(400, 410)); + addClickStep(MouseButton.Right); + + assertPlaced(true); + assertControlPointCount(3); + assertControlPointType(0, PathType.PerfectCurve); + } + private void addMovementStep(Vector2 position) => AddStep($"move mouse to {position}", () => InputManager.MoveMouseTo(InputManager.ToScreenSpace(position))); private void addClickStep(MouseButton button) From 9df059b01decf63403094a3c071a6f54bea7fe7b Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Wed, 24 Mar 2021 17:25:28 +0100 Subject: [PATCH 30/52] Add bounding box limit --- .../Components/PathControlPointVisualiser.cs | 101 ++++++++++++++++-- 1 file changed, 90 insertions(+), 11 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointVisualiser.cs b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointVisualiser.cs index c4a16f9c2a..0270485f31 100644 --- a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointVisualiser.cs +++ b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointVisualiser.cs @@ -11,6 +11,7 @@ using osu.Framework.Bindables; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Framework.Graphics.Cursor; +using osu.Framework.Graphics.Primitives; using osu.Framework.Graphics.UserInterface; using osu.Framework.Input; using osu.Framework.Input.Bindings; @@ -165,11 +166,26 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components /// /// Returns whether the given points are arranged in a valid way. Invalid if points /// are almost entirely linear - as this causes the radius to approach infinity, - /// which would exhaust memory when drawing / approximating. + /// or if the bounding box of the arc is too large. /// /// The three points that make up this circular arc segment. /// private bool validCircularArcSegment(IReadOnlyList points) + { + float det = circularArcDeterminant(points); + RectangleF boundingBox = circularArcBoundingBox(points); + + // Determinant limit prevents memory exhaustion as a result of approximating the subpath. + // Bounding box area limit prevents memory exhaustion as a result of drawing the texture. + return Math.Abs(det) > 0.001f && boundingBox.Area < 640 * 480; + } + + /// + /// Computes the determinant of the circular arc segment defined by the given points. + /// + /// The three points defining the circular arc. + /// + private float circularArcDeterminant(IReadOnlyList points) { Vector2 a = points[0]; Vector2 b = points[1]; @@ -181,19 +197,82 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components Vector2 normB = new Vector2(b.X / maxLength, b.Y / maxLength); Vector2 normC = new Vector2(c.X / maxLength, c.Y / maxLength); - float det = (normA.X - normB.X) * (normB.Y - normC.Y) - (normB.X - normC.X) * (normA.Y - normB.Y); + return (normA.X - normB.X) * (normB.Y - normC.Y) - (normB.X - normC.X) * (normA.Y - normB.Y); + } - float acSq = (a - c).LengthSquared; - float abSq = (a - b).LengthSquared; - float bcSq = (b - c).LengthSquared; + /// + /// Computes the bounding box of the circular arc segment defined by the given points. + /// + /// The three points defining the circular arc. + /// + private RectangleF circularArcBoundingBox(IReadOnlyList points) + { + Vector2 a = points[0]; + Vector2 b = points[1]; + Vector2 c = points[2]; - // Exterior = curve wraps around the long way between end-points - // Exterior bottleneck is drawing-related, interior bottleneck is approximation-related, - // where the latter is much faster, hence differing thresholds - bool exterior = abSq > acSq || bcSq > acSq; - float threshold = exterior ? 0.35f : 0.001f; + // See: https://en.wikipedia.org/wiki/Circumscribed_circle#Cartesian_coordinates_2 + float d = 2 * (a.X * (b - c).Y + b.X * (c - a).Y + c.X * (a - b).Y); + float aSq = a.LengthSquared; + float bSq = b.LengthSquared; + float cSq = c.LengthSquared; - return Math.Abs(det) >= threshold; + Vector2 center = new Vector2( + aSq * (b - c).Y + bSq * (c - a).Y + cSq * (a - b).Y, + aSq * (c - b).X + bSq * (a - c).X + cSq * (b - a).X) / d; + + Vector2 dA = a - center; + Vector2 dC = c - center; + + float r = dA.Length; + + double thetaStart = Math.Atan2(dA.Y, dA.X); + double thetaEnd = Math.Atan2(dC.Y, dC.X); + + while (thetaEnd < thetaStart) + thetaEnd += 2 * Math.PI; + + // Decide in which direction to draw the circle, depending on which side of + // AC B lies. + Vector2 orthoAtoC = c - a; + orthoAtoC = new Vector2(orthoAtoC.Y, -orthoAtoC.X); + bool clockwise = Vector2.Dot(orthoAtoC, b - a) >= 0; + + if (clockwise) + { + if (thetaEnd < thetaStart) + thetaEnd += Math.PI * 2; + } + else + { + if (thetaStart < thetaEnd) + thetaStart += Math.PI * 2; + } + + List boundingBoxPoints = new List(points); + + bool includes0Degrees = 0 > thetaStart && 0 < thetaEnd; + bool includes90Degrees = Math.PI / 2 > thetaStart && Math.PI / 2 < thetaEnd; + bool includes180Degrees = Math.PI > thetaStart && Math.PI < thetaEnd; + bool includes270Degrees = Math.PI * 1.5f > thetaStart && Math.PI * 1.5f < thetaEnd; + + if (!clockwise) + { + includes0Degrees = 0 < thetaStart && 0 > thetaEnd; + includes90Degrees = Math.PI / 2 < thetaStart && Math.PI / 2 > thetaEnd; + includes180Degrees = Math.PI < thetaStart && Math.PI > thetaEnd; + includes270Degrees = Math.PI * 1.5f < thetaStart && Math.PI * 1.5f > thetaEnd; + } + + if (includes0Degrees) boundingBoxPoints.Add(center + new Vector2(1, 0) * r); + if (includes90Degrees) boundingBoxPoints.Add(center + new Vector2(0, 1) * r); + if (includes180Degrees) boundingBoxPoints.Add(center + new Vector2(-1, 0) * r); + if (includes270Degrees) boundingBoxPoints.Add(center + new Vector2(0, -1) * r); + + float width = Math.Abs(boundingBoxPoints.Max(p => p.X) - boundingBoxPoints.Min(p => p.X)); + float height = Math.Abs(boundingBoxPoints.Max(p => p.Y) - boundingBoxPoints.Min(p => p.Y)); + + return new RectangleF(slider.Position, new Vector2(width, height)); } private void selectPiece(PathControlPointPiece piece, MouseButtonEvent e) From ce9130ca5065f4278af498e3949049c03d43dbbe Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Thu, 25 Mar 2021 17:38:55 +0100 Subject: [PATCH 31/52] Remove determinant limit This has since been added into the framework through https://github.com/ppy/osu-framework/pull/4302 --- .../Components/PathControlPointVisualiser.cs | 39 +------------------ 1 file changed, 1 insertion(+), 38 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointVisualiser.cs b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointVisualiser.cs index 0270485f31..348ec30266 100644 --- a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointVisualiser.cs +++ b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointVisualiser.cs @@ -158,48 +158,11 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components continue; Vector2[] points = slider.Path.PointsInSegment(segmentStartPoint).Select(p => p.Position.Value).ToArray(); - if (points.Length == 3 && !validCircularArcSegment(points)) + if (points.Length == 3 && circularArcBoundingBox(points).Area >= 640 * 480) segmentStartPoint.Type.Value = PathType.Bezier; } } - /// - /// Returns whether the given points are arranged in a valid way. Invalid if points - /// are almost entirely linear - as this causes the radius to approach infinity, - /// or if the bounding box of the arc is too large. - /// - /// The three points that make up this circular arc segment. - /// - private bool validCircularArcSegment(IReadOnlyList points) - { - float det = circularArcDeterminant(points); - RectangleF boundingBox = circularArcBoundingBox(points); - - // Determinant limit prevents memory exhaustion as a result of approximating the subpath. - // Bounding box area limit prevents memory exhaustion as a result of drawing the texture. - return Math.Abs(det) > 0.001f && boundingBox.Area < 640 * 480; - } - - /// - /// Computes the determinant of the circular arc segment defined by the given points. - /// - /// The three points defining the circular arc. - /// - private float circularArcDeterminant(IReadOnlyList points) - { - Vector2 a = points[0]; - Vector2 b = points[1]; - Vector2 c = points[2]; - - float maxLength = points.Max(p => p.Length); - - Vector2 normA = new Vector2(a.X / maxLength, a.Y / maxLength); - Vector2 normB = new Vector2(b.X / maxLength, b.Y / maxLength); - Vector2 normC = new Vector2(c.X / maxLength, c.Y / maxLength); - - return (normA.X - normB.X) * (normB.Y - normC.Y) - (normB.X - normC.X) * (normA.Y - normB.Y); - } - /// /// Computes the bounding box of the circular arc segment defined by the given points. /// From 51f0477df4744b6702dff18740974e3a17913be3 Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Fri, 26 Mar 2021 04:42:46 +0100 Subject: [PATCH 32/52] Move bounding box logic to framework --- .../Components/PathControlPointVisualiser.cs | 81 +------------------ 1 file changed, 4 insertions(+), 77 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointVisualiser.cs b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointVisualiser.cs index 348ec30266..dd39014bb6 100644 --- a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointVisualiser.cs +++ b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointVisualiser.cs @@ -16,6 +16,7 @@ using osu.Framework.Graphics.UserInterface; using osu.Framework.Input; using osu.Framework.Input.Bindings; using osu.Framework.Input.Events; +using osu.Framework.Utils; using osu.Game.Graphics.UserInterface; using osu.Game.Rulesets.Objects; using osu.Game.Rulesets.Objects.Types; @@ -157,87 +158,13 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components if (segmentStartPoint.Type.Value != PathType.PerfectCurve) continue; - Vector2[] points = slider.Path.PointsInSegment(segmentStartPoint).Select(p => p.Position.Value).ToArray(); - if (points.Length == 3 && circularArcBoundingBox(points).Area >= 640 * 480) + ReadOnlySpan points = slider.Path.PointsInSegment(segmentStartPoint).Select(p => p.Position.Value).ToArray(); + RectangleF boundingBox = PathApproximator.CircularArcBoundingBox(points); + if (points.Length == 3 && boundingBox.Area >= 640 * 480) segmentStartPoint.Type.Value = PathType.Bezier; } } - /// - /// Computes the bounding box of the circular arc segment defined by the given points. - /// - /// The three points defining the circular arc. - /// - private RectangleF circularArcBoundingBox(IReadOnlyList points) - { - Vector2 a = points[0]; - Vector2 b = points[1]; - Vector2 c = points[2]; - - // See: https://en.wikipedia.org/wiki/Circumscribed_circle#Cartesian_coordinates_2 - float d = 2 * (a.X * (b - c).Y + b.X * (c - a).Y + c.X * (a - b).Y); - float aSq = a.LengthSquared; - float bSq = b.LengthSquared; - float cSq = c.LengthSquared; - - Vector2 center = new Vector2( - aSq * (b - c).Y + bSq * (c - a).Y + cSq * (a - b).Y, - aSq * (c - b).X + bSq * (a - c).X + cSq * (b - a).X) / d; - - Vector2 dA = a - center; - Vector2 dC = c - center; - - float r = dA.Length; - - double thetaStart = Math.Atan2(dA.Y, dA.X); - double thetaEnd = Math.Atan2(dC.Y, dC.X); - - while (thetaEnd < thetaStart) - thetaEnd += 2 * Math.PI; - - // Decide in which direction to draw the circle, depending on which side of - // AC B lies. - Vector2 orthoAtoC = c - a; - orthoAtoC = new Vector2(orthoAtoC.Y, -orthoAtoC.X); - bool clockwise = Vector2.Dot(orthoAtoC, b - a) >= 0; - - if (clockwise) - { - if (thetaEnd < thetaStart) - thetaEnd += Math.PI * 2; - } - else - { - if (thetaStart < thetaEnd) - thetaStart += Math.PI * 2; - } - - List boundingBoxPoints = new List(points); - - bool includes0Degrees = 0 > thetaStart && 0 < thetaEnd; - bool includes90Degrees = Math.PI / 2 > thetaStart && Math.PI / 2 < thetaEnd; - bool includes180Degrees = Math.PI > thetaStart && Math.PI < thetaEnd; - bool includes270Degrees = Math.PI * 1.5f > thetaStart && Math.PI * 1.5f < thetaEnd; - - if (!clockwise) - { - includes0Degrees = 0 < thetaStart && 0 > thetaEnd; - includes90Degrees = Math.PI / 2 < thetaStart && Math.PI / 2 > thetaEnd; - includes180Degrees = Math.PI < thetaStart && Math.PI > thetaEnd; - includes270Degrees = Math.PI * 1.5f < thetaStart && Math.PI * 1.5f > thetaEnd; - } - - if (includes0Degrees) boundingBoxPoints.Add(center + new Vector2(1, 0) * r); - if (includes90Degrees) boundingBoxPoints.Add(center + new Vector2(0, 1) * r); - if (includes180Degrees) boundingBoxPoints.Add(center + new Vector2(-1, 0) * r); - if (includes270Degrees) boundingBoxPoints.Add(center + new Vector2(0, -1) * r); - - float width = Math.Abs(boundingBoxPoints.Max(p => p.X) - boundingBoxPoints.Min(p => p.X)); - float height = Math.Abs(boundingBoxPoints.Max(p => p.Y) - boundingBoxPoints.Min(p => p.Y)); - - return new RectangleF(slider.Position, new Vector2(width, height)); - } - private void selectPiece(PathControlPointPiece piece, MouseButtonEvent e) { if (e.Button == MouseButton.Left && inputManager.CurrentState.Keyboard.ControlPressed) From 70d5b616f28bb8916bb9dda0a9e1810b942d8afb Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Mon, 29 Mar 2021 15:49:49 +0200 Subject: [PATCH 33/52] Add scaling path type recovery --- osu.Game.Rulesets.Osu/Edit/OsuSelectionHandler.cs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/osu.Game.Rulesets.Osu/Edit/OsuSelectionHandler.cs b/osu.Game.Rulesets.Osu/Edit/OsuSelectionHandler.cs index 871339ae7b..da484b9782 100644 --- a/osu.Game.Rulesets.Osu/Edit/OsuSelectionHandler.cs +++ b/osu.Game.Rulesets.Osu/Edit/OsuSelectionHandler.cs @@ -33,6 +33,7 @@ namespace osu.Game.Rulesets.Osu.Edit { base.OnOperationEnded(); referenceOrigin = null; + referencePathTypes = null; } public override bool HandleMovement(MoveSelectionEvent moveEvent) => @@ -43,6 +44,12 @@ namespace osu.Game.Rulesets.Osu.Edit /// private Vector2? referenceOrigin; + /// + /// During a transform, the initial path types of a single selected slider are stored so they + /// can be maintained throughout the operation. + /// + private List referencePathTypes; + public override bool HandleReverse() { var hitObjects = EditorBeatmap.SelectedHitObjects; @@ -141,11 +148,17 @@ namespace osu.Game.Rulesets.Osu.Edit // is not looking to change the duration of the slider but expand the whole pattern. if (hitObjects.Length == 1 && hitObjects.First() is Slider slider) { + referencePathTypes ??= slider.Path.ControlPoints.Select(p => p.Type.Value).ToList(); + Quad quad = getSurroundingQuad(slider.Path.ControlPoints.Select(p => p.Position.Value)); Vector2 pathRelativeDeltaScale = new Vector2(1 + scale.X / quad.Width, 1 + scale.Y / quad.Height); foreach (var point in slider.Path.ControlPoints) point.Position.Value *= pathRelativeDeltaScale; + + // Maintain the path types in case they were defaulted to bezier at some point during scaling + for (int i = 0; i < slider.Path.ControlPoints.Count; ++i) + slider.Path.ControlPoints[i].Type.Value = referencePathTypes[i]; } else { From 1718084dbc55761c9f0a2cc49bccde6d88ccb3f3 Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Wed, 31 Mar 2021 20:08:39 +0200 Subject: [PATCH 34/52] Update/remove determinant tests We now only change the path type based on the bounding box. If the control points are too linear, the framework now handles the fallback to Bezier. --- .../TestSceneSliderControlPointPiece.cs | 17 ++----------- .../TestSceneSliderPlacementBlueprint.cs | 25 +++---------------- 2 files changed, 5 insertions(+), 37 deletions(-) diff --git a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderControlPointPiece.cs b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderControlPointPiece.cs index 7ca86335ce..b4eba7070b 100644 --- a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderControlPointPiece.cs +++ b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderControlPointPiece.cs @@ -61,19 +61,6 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor assertControlPointType(0, PathType.PerfectCurve); } - [Test] - public void TestDragControlPointAlmostLinearly() - { - moveMouseToControlPoint(1); - AddStep("hold", () => InputManager.PressButton(MouseButton.Left)); - - addMovementStep(new Vector2(150, 0.01f)); - AddStep("release", () => InputManager.ReleaseButton(MouseButton.Left)); - - assertControlPointPosition(1, new Vector2(150, 0.01f)); - assertControlPointType(0, PathType.Bezier); - } - [Test] public void TestDragControlPointAlmostLinearlyExterior() { @@ -93,7 +80,7 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor moveMouseToControlPoint(1); AddStep("hold", () => InputManager.PressButton(MouseButton.Left)); - addMovementStep(new Vector2(150, 0.01f)); + addMovementStep(new Vector2(400, 0.01f)); assertControlPointType(0, PathType.Bezier); addMovementStep(new Vector2(150, 50)); @@ -109,7 +96,7 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor moveMouseToControlPoint(4); AddStep("hold", () => InputManager.PressButton(MouseButton.Left)); - addMovementStep(new Vector2(350, 0)); + addMovementStep(new Vector2(350, 0.01f)); assertControlPointType(2, PathType.Bezier); addMovementStep(new Vector2(150, 150)); diff --git a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderPlacementBlueprint.cs b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderPlacementBlueprint.cs index 937034fc23..24382d2c65 100644 --- a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderPlacementBlueprint.cs +++ b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderPlacementBlueprint.cs @@ -276,25 +276,6 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor assertControlPointType(0, PathType.Linear); } - [Test] - public void TestPlacePerfectCurveSegmentAlmostLinearly() - { - Vector2 startPosition = new Vector2(200); - - addMovementStep(startPosition); - addClickStep(MouseButton.Left); - - addMovementStep(startPosition + new Vector2(61.382935f, 6.423767f)); - addClickStep(MouseButton.Left); - - addMovementStep(startPosition + new Vector2(217.69522f, 22.84021f)); - addClickStep(MouseButton.Right); - - assertPlaced(true); - assertControlPointCount(3); - assertControlPointType(0, PathType.Bezier); - } - [Test] public void TestPlacePerfectCurveSegmentAlmostLinearlyExterior() { @@ -322,10 +303,10 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor addMovementStep(startPosition); addClickStep(MouseButton.Left); - addMovementStep(startPosition + new Vector2(61.382935f, 6.423767f)); + addMovementStep(startPosition + new Vector2(300, 0)); addClickStep(MouseButton.Left); - addMovementStep(startPosition + new Vector2(217.69522f, 22.84021f)); // Should convert to bezier + addMovementStep(startPosition + new Vector2(150, 0.1f)); // Should convert to bezier addMovementStep(startPosition + new Vector2(210.0f, 0.0f)); // Should convert back to perfect addClickStep(MouseButton.Right); @@ -346,7 +327,7 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor addClickStep(MouseButton.Left); // Playfield dimensions are 640 x 480. - // So a 480 * 480 bounding box area should be ok. + // So a 480 x 480 bounding box should be ok. addMovementStep(startPosition + new Vector2(-240, 240)); addClickStep(MouseButton.Right); From 75b8f2535ff3c024b1cc4ab219989658c7003f1e Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Wed, 31 Mar 2021 20:09:56 +0200 Subject: [PATCH 35/52] Move updatePathTypes to PathControlPointPiece Here we produce a local bound copy of the path version, and bind it to update the path type. This way, if the path version updates (i.e. any control point changes type or position), we check that all control points have a well-defined path. Additionally, if the control point piece is disposed of, the GB should also swoop up the subscription because of the local bound copy. --- .../Components/PathControlPointPiece.cs | 24 +++++++++++++++++++ .../Components/PathControlPointVisualiser.cs | 23 ------------------ 2 files changed, 24 insertions(+), 23 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointPiece.cs b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointPiece.cs index 4459308ea5..8059fc910c 100644 --- a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointPiece.cs +++ b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointPiece.cs @@ -3,14 +3,17 @@ using System; using System.Collections.Generic; +using System.Linq; using osu.Framework.Allocation; using osu.Framework.Bindables; using osu.Framework.Extensions.Color4Extensions; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Framework.Graphics.Cursor; +using osu.Framework.Graphics.Primitives; using osu.Framework.Graphics.Shapes; using osu.Framework.Input.Events; +using osu.Framework.Utils; using osu.Game.Graphics; using osu.Game.Rulesets.Edit; using osu.Game.Rulesets.Objects; @@ -47,6 +50,7 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components [Resolved] private OsuColour colours { get; set; } + private IBindable sliderVersion; private IBindable sliderPosition; private IBindable sliderScale; private IBindable controlPointPosition; @@ -105,6 +109,9 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components { base.LoadComplete(); + sliderVersion = slider.Path.Version.GetBoundCopy(); + sliderVersion.BindValueChanged(_ => updatePathType()); + sliderPosition = slider.PositionBindable.GetBoundCopy(); sliderPosition.BindValueChanged(_ => updateMarkerDisplay()); @@ -200,6 +207,23 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components protected override void OnDragEnd(DragEndEvent e) => changeHandler?.EndChange(); + /// + /// Handles correction of invalid path types. + /// + private void updatePathType() + { + if (PointsInSegment[0].Type.Value != PathType.PerfectCurve) + return; + + ReadOnlySpan points = PointsInSegment.Select(p => p.Position.Value).ToArray(); + if (points.Length != 3) + return; + + RectangleF boundingBox = PathApproximator.CircularArcBoundingBox(points); + if (boundingBox.Width >= 640 || boundingBox.Height >= 480) + PointsInSegment[0].Type.Value = PathType.Bezier; + } + /// /// Updates the state of the circular control point marker. /// diff --git a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointVisualiser.cs b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointVisualiser.cs index dd39014bb6..ce5dc4855e 100644 --- a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointVisualiser.cs +++ b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointVisualiser.cs @@ -11,12 +11,10 @@ using osu.Framework.Bindables; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Framework.Graphics.Cursor; -using osu.Framework.Graphics.Primitives; using osu.Framework.Graphics.UserInterface; using osu.Framework.Input; using osu.Framework.Input.Bindings; using osu.Framework.Input.Events; -using osu.Framework.Utils; using osu.Game.Graphics.UserInterface; using osu.Game.Rulesets.Objects; using osu.Game.Rulesets.Objects.Types; @@ -93,8 +91,6 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components })); Connections.Add(new PathControlPointConnectionPiece(slider, e.NewStartingIndex + i)); - - point.Changed += updatePathTypes; } break; @@ -104,8 +100,6 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components { Pieces.RemoveAll(p => p.ControlPoint == point); Connections.RemoveAll(c => c.ControlPoint == point); - - point.Changed -= updatePathTypes; } // If removing before the end of the path, @@ -148,23 +142,6 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components { } - /// - /// Handles correction of invalid path types. - /// - private void updatePathTypes() - { - foreach (PathControlPoint segmentStartPoint in slider.Path.ControlPoints.Where(p => p.Type.Value != null)) - { - if (segmentStartPoint.Type.Value != PathType.PerfectCurve) - continue; - - ReadOnlySpan points = slider.Path.PointsInSegment(segmentStartPoint).Select(p => p.Position.Value).ToArray(); - RectangleF boundingBox = PathApproximator.CircularArcBoundingBox(points); - if (points.Length == 3 && boundingBox.Area >= 640 * 480) - segmentStartPoint.Type.Value = PathType.Bezier; - } - } - private void selectPiece(PathControlPointPiece piece, MouseButtonEvent e) { if (e.Button == MouseButton.Left && inputManager.CurrentState.Keyboard.ControlPressed) From 5022a78e80b1a07c54ab55176570a7e4be2debaa Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Wed, 31 Mar 2021 20:25:46 +0200 Subject: [PATCH 36/52] Check current point instead of start point Since each control point will call this when the path updates, the previous would correct the start segment 3 times instead of just once. This fixes that. --- .../Blueprints/Sliders/Components/PathControlPointPiece.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointPiece.cs b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointPiece.cs index 8059fc910c..394d2b039d 100644 --- a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointPiece.cs +++ b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointPiece.cs @@ -212,7 +212,7 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components /// private void updatePathType() { - if (PointsInSegment[0].Type.Value != PathType.PerfectCurve) + if (ControlPoint.Type.Value != PathType.PerfectCurve) return; ReadOnlySpan points = PointsInSegment.Select(p => p.Position.Value).ToArray(); @@ -221,7 +221,7 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components RectangleF boundingBox = PathApproximator.CircularArcBoundingBox(points); if (boundingBox.Width >= 640 || boundingBox.Height >= 480) - PointsInSegment[0].Type.Value = PathType.Bezier; + ControlPoint.Type.Value = PathType.Bezier; } /// From 7b684339ed64703c169dfccc27c3abfec521e586 Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Wed, 31 Mar 2021 20:32:49 +0200 Subject: [PATCH 37/52] Undo public -> internal for `PathControlPoint.Changed` No longer used. --- osu.Game/Rulesets/Objects/PathControlPoint.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Rulesets/Objects/PathControlPoint.cs b/osu.Game/Rulesets/Objects/PathControlPoint.cs index 2e4100ee0b..f11917f4f4 100644 --- a/osu.Game/Rulesets/Objects/PathControlPoint.cs +++ b/osu.Game/Rulesets/Objects/PathControlPoint.cs @@ -27,7 +27,7 @@ namespace osu.Game.Rulesets.Objects /// /// Invoked when any property of this is changed. /// - public event Action Changed; + internal event Action Changed; /// /// Creates a new . From 25afae5671514f88966f06b59eda1ef82b824ecf Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Wed, 31 Mar 2021 20:48:17 +0200 Subject: [PATCH 38/52] Fix broken test case Seems this technically works, but only because of the edge case of being entirely linear, which the framework catches. This fixes that. --- .../Editor/TestSceneSliderPlacementBlueprint.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderPlacementBlueprint.cs b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderPlacementBlueprint.cs index 24382d2c65..462abf2edb 100644 --- a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderPlacementBlueprint.cs +++ b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderPlacementBlueprint.cs @@ -307,7 +307,7 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor addClickStep(MouseButton.Left); addMovementStep(startPosition + new Vector2(150, 0.1f)); // Should convert to bezier - addMovementStep(startPosition + new Vector2(210.0f, 0.0f)); // Should convert back to perfect + addMovementStep(startPosition + new Vector2(400.0f, 50.0f)); // Should convert back to perfect addClickStep(MouseButton.Right); assertPlaced(true); From b8479a979f1ae1453e4488ec555a3cdc0e49f082 Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Thu, 1 Apr 2021 18:06:12 +0200 Subject: [PATCH 39/52] Remove unused blueprint variable --- .../Editor/TestSceneSliderControlPointPiece.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderControlPointPiece.cs b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderControlPointPiece.cs index b4eba7070b..9b67d18db6 100644 --- a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderControlPointPiece.cs +++ b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderControlPointPiece.cs @@ -22,7 +22,6 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor { private Slider slider; private DrawableSlider drawableObject; - private TestSliderBlueprint blueprint; [SetUp] public void Setup() => Schedule(() => @@ -45,7 +44,7 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor slider.ApplyDefaults(new ControlPointInfo(), new BeatmapDifficulty { CircleSize = 2 }); Add(drawableObject = new DrawableSlider(slider)); - AddBlueprint(blueprint = new TestSliderBlueprint(drawableObject)); + AddBlueprint(new TestSliderBlueprint(drawableObject)); }); [Test] From 8621a6b4fe8b84e50e2a1d56b25e3bed9824be6f Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Thu, 1 Apr 2021 20:34:04 +0200 Subject: [PATCH 40/52] Add margin to large segment test Test ran fine on my end, but apparently not on the CI. This should make results a bit more consistent, hopefully. --- .../Editor/TestSceneSliderPlacementBlueprint.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderPlacementBlueprint.cs b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderPlacementBlueprint.cs index 462abf2edb..d2c37061f0 100644 --- a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderPlacementBlueprint.cs +++ b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderPlacementBlueprint.cs @@ -323,12 +323,12 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor addMovementStep(startPosition); addClickStep(MouseButton.Left); - addMovementStep(startPosition + new Vector2(240, 240)); + addMovementStep(startPosition + new Vector2(220, 220)); addClickStep(MouseButton.Left); // Playfield dimensions are 640 x 480. - // So a 480 x 480 bounding box should be ok. - addMovementStep(startPosition + new Vector2(-240, 240)); + // So a 440 x 440 bounding box should be ok. + addMovementStep(startPosition + new Vector2(-220, 220)); addClickStep(MouseButton.Right); assertPlaced(true); From 8cc1e8b8b0605e981b1d0be53cb9a0590383e532 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 7 Apr 2021 23:11:01 +0900 Subject: [PATCH 41/52] Update framework --- .idea/.idea.osu.Desktop/.idea/indexLayout.xml | 2 +- osu.Android.props | 2 +- osu.Game/osu.Game.csproj | 2 +- osu.iOS.props | 4 ++-- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/.idea/.idea.osu.Desktop/.idea/indexLayout.xml b/.idea/.idea.osu.Desktop/.idea/indexLayout.xml index 27ba142e96..7b08163ceb 100644 --- a/.idea/.idea.osu.Desktop/.idea/indexLayout.xml +++ b/.idea/.idea.osu.Desktop/.idea/indexLayout.xml @@ -1,6 +1,6 @@ - + diff --git a/osu.Android.props b/osu.Android.props index 73ee1d9d10..cba3975209 100644 --- a/osu.Android.props +++ b/osu.Android.props @@ -52,6 +52,6 @@ - + diff --git a/osu.Game/osu.Game.csproj b/osu.Game/osu.Game.csproj index 931b55222a..292e5b932f 100644 --- a/osu.Game/osu.Game.csproj +++ b/osu.Game/osu.Game.csproj @@ -29,7 +29,7 @@ - + diff --git a/osu.iOS.props b/osu.iOS.props index 64e9a01a92..36e581a80c 100644 --- a/osu.iOS.props +++ b/osu.iOS.props @@ -70,7 +70,7 @@ - + @@ -93,7 +93,7 @@ - + From 544fff5af6f9fa20e1aa04e5aaa0a5ea3865b220 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 7 Apr 2021 23:18:45 +0900 Subject: [PATCH 42/52] Undo rider EAP changes for the time being --- .idea/.idea.osu.Desktop/.idea/indexLayout.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.idea/.idea.osu.Desktop/.idea/indexLayout.xml b/.idea/.idea.osu.Desktop/.idea/indexLayout.xml index 7b08163ceb..27ba142e96 100644 --- a/.idea/.idea.osu.Desktop/.idea/indexLayout.xml +++ b/.idea/.idea.osu.Desktop/.idea/indexLayout.xml @@ -1,6 +1,6 @@ - + From 9d0293070971e17132b201448afbb42c5f40b295 Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Wed, 7 Apr 2021 17:18:55 +0200 Subject: [PATCH 43/52] Add regression test for type changes --- .../TestSceneSliderControlPointPiece.cs | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderControlPointPiece.cs b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderControlPointPiece.cs index 9b67d18db6..d7dfc3bd42 100644 --- a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderControlPointPiece.cs +++ b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderControlPointPiece.cs @@ -105,6 +105,25 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor assertControlPointType(2, PathType.PerfectCurve); } + [Test] + public void TestDragControlPointPathAfterChangingType() + { + AddStep("change type to bezier", () => slider.Path.ControlPoints[2].Type.Value = PathType.Bezier); + AddStep("add point", () => slider.Path.ControlPoints.Add(new PathControlPoint(new Vector2(500, 10)))); + AddStep("change type to perfect", () => slider.Path.ControlPoints[3].Type.Value = PathType.PerfectCurve); + + moveMouseToControlPoint(4); + AddStep("hold", () => InputManager.PressButton(MouseButton.Left)); + + assertControlPointType(3, PathType.PerfectCurve); + + addMovementStep(new Vector2(350, 0.01f)); + AddStep("release", () => InputManager.ReleaseButton(MouseButton.Left)); + + assertControlPointPosition(4, new Vector2(350, 0.01f)); + assertControlPointType(3, PathType.Bezier); + } + private void addMovementStep(Vector2 relativePosition) { AddStep($"move mouse to {relativePosition}", () => From b8ab1c768253402bdfb3644808cbfde05234ef07 Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Wed, 7 Apr 2021 17:19:12 +0200 Subject: [PATCH 44/52] Track path type changes for `PointsInSegment` --- .../Sliders/Components/PathControlPointPiece.cs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointPiece.cs b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointPiece.cs index 394d2b039d..5fcf0656c5 100644 --- a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointPiece.cs +++ b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointPiece.cs @@ -50,6 +50,8 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components [Resolved] private OsuColour colours { get; set; } + private readonly List> pathTypes; + private IBindable sliderVersion; private IBindable sliderPosition; private IBindable sliderScale; @@ -59,8 +61,19 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components { this.slider = slider; ControlPoint = controlPoint; + pathTypes = new List>(); + slider.Path.ControlPoints.BindCollectionChanged((_, args) => { + pathTypes.Clear(); + + foreach (var point in slider.Path.ControlPoints) + { + IBindable boundTypeCopy = point.Type.GetBoundCopy(); + pathTypes.Add(boundTypeCopy); + boundTypeCopy.BindValueChanged(_ => PointsInSegment = slider.Path.PointsInSegment(controlPoint)); + } + PointsInSegment = slider.Path.PointsInSegment(controlPoint); }, runOnceImmediately: true); From d1d56c636a04a48e5cafdc3cf0cb1d1996d2162e Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Thu, 8 Apr 2021 01:43:06 +0200 Subject: [PATCH 45/52] Convert `pathTypes` to local variable Not entirely sure what holds the reference to pathTypes now (the binding to`slider.Path.ControlPoints` maybe?), but this does seem to work still. --- .../Blueprints/Sliders/Components/PathControlPointPiece.cs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointPiece.cs b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointPiece.cs index 5fcf0656c5..bb50fec5dc 100644 --- a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointPiece.cs +++ b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointPiece.cs @@ -50,8 +50,6 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components [Resolved] private OsuColour colours { get; set; } - private readonly List> pathTypes; - private IBindable sliderVersion; private IBindable sliderPosition; private IBindable sliderScale; @@ -61,7 +59,7 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components { this.slider = slider; ControlPoint = controlPoint; - pathTypes = new List>(); + var pathTypes = new List>(); slider.Path.ControlPoints.BindCollectionChanged((_, args) => { From d6490899e2946294bbdab7f0d0db6a5bd014ce5d Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Thu, 8 Apr 2021 03:21:56 +0200 Subject: [PATCH 46/52] Simplify slider path bindings Adds a slight performance overhead, but solves the memory leak and makes the code much easier to follow. --- .../Components/PathControlPointPiece.cs | 19 +++---------------- 1 file changed, 3 insertions(+), 16 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointPiece.cs b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointPiece.cs index bb50fec5dc..20d4fe4ea9 100644 --- a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointPiece.cs +++ b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointPiece.cs @@ -50,7 +50,6 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components [Resolved] private OsuColour colours { get; set; } - private IBindable sliderVersion; private IBindable sliderPosition; private IBindable sliderScale; private IBindable controlPointPosition; @@ -59,20 +58,11 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components { this.slider = slider; ControlPoint = controlPoint; - var pathTypes = new List>(); - slider.Path.ControlPoints.BindCollectionChanged((_, args) => + slider.Path.Version.BindValueChanged(_ => { - pathTypes.Clear(); - - foreach (var point in slider.Path.ControlPoints) - { - IBindable boundTypeCopy = point.Type.GetBoundCopy(); - pathTypes.Add(boundTypeCopy); - boundTypeCopy.BindValueChanged(_ => PointsInSegment = slider.Path.PointsInSegment(controlPoint)); - } - - PointsInSegment = slider.Path.PointsInSegment(controlPoint); + PointsInSegment = slider.Path.PointsInSegment(ControlPoint); + updatePathType(); }, runOnceImmediately: true); controlPoint.Type.BindValueChanged(_ => updateMarkerDisplay()); @@ -120,9 +110,6 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components { base.LoadComplete(); - sliderVersion = slider.Path.Version.GetBoundCopy(); - sliderVersion.BindValueChanged(_ => updatePathType()); - sliderPosition = slider.PositionBindable.GetBoundCopy(); sliderPosition.BindValueChanged(_ => updateMarkerDisplay()); From 8aff53172de5f153b5926926bc0bf1c8e75f3ef5 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 8 Apr 2021 15:17:53 +0900 Subject: [PATCH 47/52] Remove necessity for nested PassThroughInputManger --- .../Gameplay/TestSceneGameplayMenuOverlay.cs | 2 +- .../Input/Bindings/GlobalActionContainer.cs | 23 ++++++++++----- osu.Game/Input/Bindings/GlobalInputManager.cs | 29 ------------------- osu.Game/OsuGameBase.cs | 15 ++++++---- .../Visual/OsuManualInputManagerTestScene.cs | 2 +- 5 files changed, 26 insertions(+), 45 deletions(-) delete mode 100644 osu.Game/Input/Bindings/GlobalInputManager.cs diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneGameplayMenuOverlay.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneGameplayMenuOverlay.cs index 75e8194708..d69ac665cc 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneGameplayMenuOverlay.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneGameplayMenuOverlay.cs @@ -27,7 +27,7 @@ namespace osu.Game.Tests.Visual.Gameplay [BackgroundDependencyLoader] private void load(OsuGameBase game) { - Child = globalActionContainer = new GlobalActionContainer(game, null); + Child = globalActionContainer = new GlobalActionContainer(game); } [SetUp] diff --git a/osu.Game/Input/Bindings/GlobalActionContainer.cs b/osu.Game/Input/Bindings/GlobalActionContainer.cs index 042960d54c..671c3bc8bc 100644 --- a/osu.Game/Input/Bindings/GlobalActionContainer.cs +++ b/osu.Game/Input/Bindings/GlobalActionContainer.cs @@ -4,7 +4,6 @@ using System.Collections.Generic; using System.ComponentModel; using System.Linq; -using JetBrains.Annotations; using osu.Framework.Graphics; using osu.Framework.Input; using osu.Framework.Input.Bindings; @@ -13,20 +12,23 @@ namespace osu.Game.Input.Bindings { public class GlobalActionContainer : DatabasedKeyBindingContainer, IHandleGlobalKeyboardInput { - [CanBeNull] - private readonly GlobalInputManager globalInputManager; - private readonly Drawable handler; + private InputManager parentInputManager; - public GlobalActionContainer(OsuGameBase game, [CanBeNull] GlobalInputManager globalInputManager) + public GlobalActionContainer(OsuGameBase game) : base(matchingMode: KeyCombinationMatchingMode.Modifiers) { - this.globalInputManager = globalInputManager; - if (game is IKeyBindingHandler) handler = game; } + protected override void LoadComplete() + { + base.LoadComplete(); + + parentInputManager = GetContainingInputManager(); + } + public override IEnumerable DefaultKeyBindings => GlobalKeyBindings .Concat(EditorKeyBindings) .Concat(InGameKeyBindings) @@ -113,7 +115,12 @@ namespace osu.Game.Input.Bindings { get { - var inputQueue = globalInputManager?.NonPositionalInputQueue ?? base.KeyBindingInputQueue; + // To ensure the global actions are handled with priority, this GlobalActionContainer is actually placed after game content. + // It does not contain children as expected, so we need to forward the NonPositionalInputQueue from the parent input manager to correctly + // allow the whole game to handle these actions. + + // An eventual solution to this hack is to create localised action containers for individual components like SongSelect, but this will take some rearranging. + var inputQueue = parentInputManager?.NonPositionalInputQueue ?? base.KeyBindingInputQueue; return handler != null ? inputQueue.Prepend(handler) : inputQueue; } diff --git a/osu.Game/Input/Bindings/GlobalInputManager.cs b/osu.Game/Input/Bindings/GlobalInputManager.cs deleted file mode 100644 index 91373712fb..0000000000 --- a/osu.Game/Input/Bindings/GlobalInputManager.cs +++ /dev/null @@ -1,29 +0,0 @@ -// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. -// See the LICENCE file in the repository root for full licence text. - -using osu.Framework.Graphics; -using osu.Framework.Graphics.Containers; -using osu.Framework.Input; - -namespace osu.Game.Input.Bindings -{ - public class GlobalInputManager : PassThroughInputManager - { - public readonly GlobalActionContainer GlobalBindings; - - protected override Container Content { get; } - - public GlobalInputManager(OsuGameBase game) - { - InternalChildren = new Drawable[] - { - Content = new Container - { - RelativeSizeAxes = Axes.Both, - }, - // to avoid positional input being blocked by children, ensure the GlobalActionContainer is above everything. - GlobalBindings = new GlobalActionContainer(game, this) - }; - } - } -} diff --git a/osu.Game/OsuGameBase.cs b/osu.Game/OsuGameBase.cs index 21313d0596..ca132df552 100644 --- a/osu.Game/OsuGameBase.cs +++ b/osu.Game/OsuGameBase.cs @@ -308,18 +308,21 @@ namespace osu.Game AddInternal(RulesetConfigCache); - var globalInput = new GlobalInputManager(this) + GlobalActionContainer globalBindings; + + var mainContent = new Drawable[] { - RelativeSizeAxes = Axes.Both, - Child = MenuCursorContainer = new MenuCursorContainer { RelativeSizeAxes = Axes.Both } + MenuCursorContainer = new MenuCursorContainer { RelativeSizeAxes = Axes.Both }, + // to avoid positional input being blocked by children, ensure the GlobalActionContainer is above everything. + globalBindings = new GlobalActionContainer(this) }; MenuCursorContainer.Child = content = new OsuTooltipContainer(MenuCursorContainer.Cursor) { RelativeSizeAxes = Axes.Both }; - base.Content.Add(CreateScalingContainer().WithChild(globalInput)); + base.Content.Add(CreateScalingContainer().WithChildren(mainContent)); - KeyBindingStore.Register(globalInput.GlobalBindings); - dependencies.Cache(globalInput.GlobalBindings); + KeyBindingStore.Register(globalBindings); + dependencies.Cache(globalBindings); PreviewTrackManager previewTrackManager; dependencies.Cache(previewTrackManager = new PreviewTrackManager()); diff --git a/osu.Game/Tests/Visual/OsuManualInputManagerTestScene.cs b/osu.Game/Tests/Visual/OsuManualInputManagerTestScene.cs index 7dad636da7..01dd7a25c8 100644 --- a/osu.Game/Tests/Visual/OsuManualInputManagerTestScene.cs +++ b/osu.Game/Tests/Visual/OsuManualInputManagerTestScene.cs @@ -40,7 +40,7 @@ namespace osu.Game.Tests.Visual if (CreateNestedActionContainer) { - mainContent = new GlobalActionContainer(null, null).WithChild(mainContent); + mainContent = new GlobalActionContainer(null).WithChild(mainContent); } base.Content.AddRange(new Drawable[] From 545156d15ca19a67bc22c43aa6dded4cee3cd877 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 8 Apr 2021 15:20:53 +0900 Subject: [PATCH 48/52] Add regression test coverage --- .../Visual/Navigation/TestSceneScreenNavigation.cs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs b/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs index 00f9bf3432..859cefe3a9 100644 --- a/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs +++ b/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs @@ -44,6 +44,20 @@ namespace osu.Game.Tests.Visual.Navigation exitViaEscapeAndConfirm(); } + /// + /// This tests that the F1 key will open the mod select overlay, and not be handled / blocked by the music controller (which has the same default binding + /// but should be handled *after* song select). + /// + [Test] + public void TestOpenModSelectOverlayUsingAction() + { + TestSongSelect songSelect = null; + + PushAndConfirm(() => songSelect = new TestSongSelect()); + AddStep("Show mods overlay", () => InputManager.Key(Key.F1)); + AddAssert("Overlay was shown", () => songSelect.ModSelectOverlay.State.Value == Visibility.Visible); + } + [Test] public void TestRetryCountIncrements() { From b73860cb5f6de153df50f3dc64b067dd611a6f40 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 8 Apr 2021 15:47:55 +0900 Subject: [PATCH 49/52] Slightly alter button colour scheme to make text more legible and reduce saturation --- .../Multiplayer/Match/MultiplayerSpectateButton.cs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/Match/MultiplayerSpectateButton.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/Match/MultiplayerSpectateButton.cs index 50be7719d9..4b3fb5d00f 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/Match/MultiplayerSpectateButton.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/Match/MultiplayerSpectateButton.cs @@ -68,16 +68,16 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Match { default: button.Text = "Spectate"; - button.BackgroundColour = colours.Blue; - button.Triangles.ColourDark = colours.Blue; - button.Triangles.ColourLight = colours.BlueLight; + button.BackgroundColour = colours.BlueDark; + button.Triangles.ColourDark = colours.BlueDarker; + button.Triangles.ColourLight = colours.Blue; break; case MultiplayerUserState.Spectating: button.Text = "Stop spectating"; - button.BackgroundColour = colours.Red; - button.Triangles.ColourDark = colours.Red; - button.Triangles.ColourLight = colours.RedLight; + button.BackgroundColour = colours.Gray4; + button.Triangles.ColourDark = colours.Gray5; + button.Triangles.ColourLight = colours.Gray6; break; } From a55e62188ebe87648bcc1ffe9f6af93f167072a3 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 8 Apr 2021 15:54:58 +0900 Subject: [PATCH 50/52] Change state icon to binoculars so the eye isn't staring at me --- .../Screens/OnlinePlay/Multiplayer/Participants/StateDisplay.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/Participants/StateDisplay.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/Participants/StateDisplay.cs index e6a407acec..2616b07c1f 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/Participants/StateDisplay.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/Participants/StateDisplay.cs @@ -137,7 +137,7 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Participants case MultiplayerUserState.Spectating: text.Text = "spectating"; - icon.Icon = FontAwesome.Solid.Eye; + icon.Icon = FontAwesome.Solid.Binoculars; icon.Colour = colours.BlueLight; break; From b38d332268bf678cda1746bb264a7adb32d7a7e5 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Thu, 8 Apr 2021 16:29:11 +0900 Subject: [PATCH 51/52] Fix broken test --- .../Multiplayer/TestSceneMultiplayerReadyButton.cs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerReadyButton.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerReadyButton.cs index dad1237991..dfb4306e67 100644 --- a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerReadyButton.cs +++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerReadyButton.cs @@ -209,9 +209,16 @@ namespace osu.Game.Tests.Visual.Multiplayer { addClickButtonStep(); AddAssert("user waiting for load", () => Client.Room?.Users[0].State == MultiplayerUserState.WaitingForLoad); - AddAssert("ready button disabled", () => !button.ChildrenOfType().Single().Enabled.Value); + AddAssert("ready button disabled", () => !button.ChildrenOfType().Single().Enabled.Value); AddStep("transitioned to gameplay", () => readyClickOperation.Dispose()); + + AddStep("finish gameplay", () => + { + Client.ChangeUserState(Client.Room?.Users[0].UserID ?? 0, MultiplayerUserState.Loaded); + Client.ChangeUserState(Client.Room?.Users[0].UserID ?? 0, MultiplayerUserState.FinishedPlay); + }); + AddAssert("ready button enabled", () => button.ChildrenOfType().Single().Enabled.Value); } } From fd2a14a0bf07fd5e8b940f2f666455fc4eb923ba Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Thu, 8 Apr 2021 16:30:48 +0900 Subject: [PATCH 52/52] Only set button state once --- .../Multiplayer/Match/MultiplayerReadyButton.cs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/Match/MultiplayerReadyButton.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/Match/MultiplayerReadyButton.cs index 6919be2d56..f2dd9a6f25 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/Match/MultiplayerReadyButton.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/Match/MultiplayerReadyButton.cs @@ -105,14 +105,13 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Match break; } - button.Enabled.Value = Client.Room?.State == MultiplayerRoomState.Open && !operationInProgress.Value; + bool enableButton = Client.Room?.State == MultiplayerRoomState.Open && !operationInProgress.Value; - // When the local user is the host and spectating the match, the "start match" state should be enabled. + // When the local user is the host and spectating the match, the "start match" state should be enabled if any users are ready. if (localUser.State == MultiplayerUserState.Spectating) - { - button.Enabled.Value &= Room?.Host?.Equals(localUser) == true; - button.Enabled.Value &= newCountReady > 0; - } + enableButton &= Room?.Host?.Equals(localUser) == true && newCountReady > 0; + + button.Enabled.Value = enableButton; if (newCountReady != countReady) {