From f49aa4d8157eb6368263f919c662dd0afdaca1d0 Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Wed, 28 Feb 2024 22:01:39 +0800 Subject: [PATCH 01/11] Parse points and segments for path string --- .../Objects/Legacy/ConvertHitObjectParser.cs | 57 ++++++++++++------- 1 file changed, 38 insertions(+), 19 deletions(-) diff --git a/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs b/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs index f9e32fe26f..30e4101a84 100644 --- a/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs +++ b/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs @@ -266,30 +266,49 @@ namespace osu.Game.Rulesets.Objects.Legacy // This code takes on the responsibility of handling explicit segments of the path ("X" & "Y" from above). Implicit segments are handled by calls to convertPoints(). string[] pointSplit = pointString.Split('|'); - var controlPoints = new List>(); - int startIndex = 0; - int endIndex = 0; - bool first = true; + Span points = stackalloc Vector2[pointSplit.Length]; + Span<(PathType Type, int StartIndex)> segments = stackalloc (PathType Type, int StartIndex)[pointSplit.Length]; + int pointsCount = 0; + int segmentsCount = 0; - while (++endIndex < pointSplit.Length) + foreach (string s in pointSplit) { - // Keep incrementing endIndex while it's not the start of a new segment (indicated by having an alpha character at position 0). - if (!char.IsLetter(pointSplit[endIndex][0])) - continue; - - // Multi-segmented sliders DON'T contain the end point as part of the current segment as it's assumed to be the start of the next segment. - // The start of the next segment is the index after the type descriptor. - string endPoint = endIndex < pointSplit.Length - 1 ? pointSplit[endIndex + 1] : null; - - controlPoints.AddRange(convertPoints(pointSplit.AsMemory().Slice(startIndex, endIndex - startIndex), endPoint, first, offset)); - startIndex = endIndex; - first = false; + if (char.IsLetter(s[0])) + { + // The start of a new segment(indicated by having an alpha character at position 0). + var pathType = convertPathType(s); + segments[segmentsCount++] = (pathType, pointsCount); + } + else + { + points[pointsCount++] = readPoint(s, offset); + } } - if (endIndex > startIndex) - controlPoints.AddRange(convertPoints(pointSplit.AsMemory().Slice(startIndex, endIndex - startIndex), null, first, offset)); + var controlPoints = new List(pointsCount); - return mergePointsLists(controlPoints); + for (int i = 0; i < segmentsCount; i++) + { + int startIndex = segments[i].StartIndex; + int endIndex = i < segmentsCount - 1 ? segments[i + 1].StartIndex : pointsCount; + Vector2? endPoint = i < segmentsCount - 1 ? points[endIndex] : null; + controlPoints.AddRange(convertPoints(segments[i].Type, points[startIndex..endIndex], endPoint)); + } + + return controlPoints.ToArray(); + + static Vector2 readPoint(string value, Vector2 startPos) + { + string[] vertexSplit = value.Split(':'); + + Vector2 pos = new Vector2((int)Parsing.ParseDouble(vertexSplit[0], Parsing.MAX_COORDINATE_VALUE), (int)Parsing.ParseDouble(vertexSplit[1], Parsing.MAX_COORDINATE_VALUE)) - startPos; + return pos; + } + } + + private IEnumerable convertPoints(PathType type, ReadOnlySpan points, Vector2? endPoint) + { + throw new NotImplementedException(); } /// From 4bff54d35dbd60aab7cd8a255977fe95e5c3bde7 Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Wed, 28 Feb 2024 22:37:14 +0800 Subject: [PATCH 02/11] Add ToString on PathControlPoint for debugging --- osu.Game/Rulesets/Objects/PathControlPoint.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/osu.Game/Rulesets/Objects/PathControlPoint.cs b/osu.Game/Rulesets/Objects/PathControlPoint.cs index ae9fa08085..1f8e63b269 100644 --- a/osu.Game/Rulesets/Objects/PathControlPoint.cs +++ b/osu.Game/Rulesets/Objects/PathControlPoint.cs @@ -76,5 +76,9 @@ namespace osu.Game.Rulesets.Objects } public bool Equals(PathControlPoint other) => Position == other?.Position && Type == other.Type; + + public override string ToString() => type is null + ? $"Position={Position}" + : $"Position={Position}, Type={type}"; } } From fe34577ee2938c45d295800405a309ad3d55ac3e Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Wed, 28 Feb 2024 22:42:08 +0800 Subject: [PATCH 03/11] Update parsing. --- .../Objects/Legacy/ConvertHitObjectParser.cs | 86 ++++++------------- 1 file changed, 24 insertions(+), 62 deletions(-) diff --git a/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs b/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs index 30e4101a84..2d4d2865e6 100644 --- a/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs +++ b/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs @@ -278,6 +278,10 @@ namespace osu.Game.Rulesets.Objects.Legacy // The start of a new segment(indicated by having an alpha character at position 0). var pathType = convertPathType(s); segments[segmentsCount++] = (pathType, pointsCount); + + // First segment is prepended by an extra zero point + if (pointsCount == 0) + points[pointsCount++] = Vector2.Zero; } else { @@ -306,47 +310,30 @@ namespace osu.Game.Rulesets.Objects.Legacy } } - private IEnumerable convertPoints(PathType type, ReadOnlySpan points, Vector2? endPoint) - { - throw new NotImplementedException(); - } - /// /// Converts a given point list into a set of path segments. /// + /// The path type of the point list. /// The point list. /// Any extra endpoint to consider as part of the points. This will NOT be returned. - /// Whether this is the first segment in the set. If true the first of the returned segments will contain a zero point. - /// The positional offset to apply to the control points. - /// The set of points contained by as one or more segments of the path, prepended by an extra zero point if is true. - private IEnumerable> convertPoints(ReadOnlyMemory points, string endPoint, bool first, Vector2 offset) + /// The set of points contained by as one or more segments of the path. + private IEnumerable convertPoints(PathType type, ReadOnlySpan points, Vector2? endPoint) { - PathType type = convertPathType(points.Span[0]); - - int readOffset = first ? 1 : 0; // First control point is zero for the first segment. - int readablePoints = points.Length - 1; // Total points readable from the base point span. - int endPointLength = endPoint != null ? 1 : 0; // Extra length if an endpoint is given that lies outside the base point span. - - var vertices = new PathControlPoint[readOffset + readablePoints + endPointLength]; - - // Fill any non-read points. - for (int i = 0; i < readOffset; i++) - vertices[i] = new PathControlPoint(); + var vertices = new PathControlPoint[points.Length]; + var result = new List(); // Parse into control points. - for (int i = 1; i < points.Length; i++) - readPoint(points.Span[i], offset, out vertices[readOffset + i - 1]); - - // If an endpoint is given, add it to the end. - if (endPoint != null) - readPoint(endPoint, offset, out vertices[^1]); + for (int i = 0; i < points.Length; i++) + vertices[i] = new PathControlPoint { Position = points[i] }; // Edge-case rules (to match stable). if (type == PathType.PERFECT_CURVE) { - if (vertices.Length != 3) + int endPointLength = endPoint is null ? 0 : 1; + + if (vertices.Length + endPointLength != 3) type = PathType.BEZIER; - else if (isLinear(vertices)) + else if (isLinear(stackalloc[] { points[0], points[1], endPoint ?? points[2] })) { // osu-stable special-cased colinear perfect curves to a linear path type = PathType.LINEAR; @@ -365,7 +352,7 @@ namespace osu.Game.Rulesets.Objects.Legacy int startIndex = 0; int endIndex = 0; - while (++endIndex < vertices.Length - endPointLength) + while (++endIndex < vertices.Length) { // Keep incrementing while an implicit segment doesn't need to be started. if (vertices[endIndex].Position != vertices[endIndex - 1].Position) @@ -378,50 +365,25 @@ namespace osu.Game.Rulesets.Objects.Legacy continue; // The last control point of each segment is not allowed to start a new implicit segment. - if (endIndex == vertices.Length - endPointLength - 1) + if (endIndex == vertices.Length - 1) continue; // Force a type on the last point, and return the current control point set as a segment. vertices[endIndex - 1].Type = type; - yield return vertices.AsMemory().Slice(startIndex, endIndex - startIndex); + for (int i = startIndex; i < endIndex; i++) + result.Add(vertices[i]); // Skip the current control point - as it's the same as the one that's just been returned. startIndex = endIndex + 1; } - if (endIndex > startIndex) - yield return vertices.AsMemory().Slice(startIndex, endIndex - startIndex); + for (int i = startIndex; i < endIndex; i++) + result.Add(vertices[i]); - static void readPoint(string value, Vector2 startPos, out PathControlPoint point) - { - string[] vertexSplit = value.Split(':'); + return result; - Vector2 pos = new Vector2((int)Parsing.ParseDouble(vertexSplit[0], Parsing.MAX_COORDINATE_VALUE), (int)Parsing.ParseDouble(vertexSplit[1], Parsing.MAX_COORDINATE_VALUE)) - startPos; - point = new PathControlPoint { Position = pos }; - } - - static bool isLinear(PathControlPoint[] p) => Precision.AlmostEquals(0, (p[1].Position.Y - p[0].Position.Y) * (p[2].Position.X - p[0].Position.X) - - (p[1].Position.X - p[0].Position.X) * (p[2].Position.Y - p[0].Position.Y)); - } - - private PathControlPoint[] mergePointsLists(List> controlPointList) - { - int totalCount = 0; - - foreach (var arr in controlPointList) - totalCount += arr.Length; - - var mergedArray = new PathControlPoint[totalCount]; - var mergedArrayMemory = mergedArray.AsMemory(); - int copyIndex = 0; - - foreach (var arr in controlPointList) - { - arr.CopyTo(mergedArrayMemory.Slice(copyIndex)); - copyIndex += arr.Length; - } - - return mergedArray; + static bool isLinear(ReadOnlySpan p) => Precision.AlmostEquals(0, (p[1].Y - p[0].Y) * (p[2].X - p[0].X) + - (p[1].X - p[0].X) * (p[2].Y - p[0].Y)); } /// From bcb91f348d746bd4470240c23c72ec812fc54113 Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Wed, 28 Feb 2024 22:51:36 +0800 Subject: [PATCH 04/11] Use ArrayPool instead of stackalloc --- .../Objects/Legacy/ConvertHitObjectParser.cs | 82 ++++++++++--------- 1 file changed, 44 insertions(+), 38 deletions(-) diff --git a/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs b/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs index 2d4d2865e6..a3ca719ff9 100644 --- a/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs +++ b/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs @@ -18,6 +18,7 @@ using osu.Game.Beatmaps.ControlPoints; using osu.Game.Beatmaps.Legacy; using osu.Game.Skinning; using osu.Game.Utils; +using System.Buffers; namespace osu.Game.Rulesets.Objects.Legacy { @@ -266,41 +267,49 @@ namespace osu.Game.Rulesets.Objects.Legacy // This code takes on the responsibility of handling explicit segments of the path ("X" & "Y" from above). Implicit segments are handled by calls to convertPoints(). string[] pointSplit = pointString.Split('|'); - Span points = stackalloc Vector2[pointSplit.Length]; - Span<(PathType Type, int StartIndex)> segments = stackalloc (PathType Type, int StartIndex)[pointSplit.Length]; + var points = ArrayPool.Shared.Rent(pointSplit.Length); + var segments = ArrayPool<(PathType Type, int StartIndex)>.Shared.Rent(pointSplit.Length); int pointsCount = 0; int segmentsCount = 0; - - foreach (string s in pointSplit) + try { - if (char.IsLetter(s[0])) - { - // The start of a new segment(indicated by having an alpha character at position 0). - var pathType = convertPathType(s); - segments[segmentsCount++] = (pathType, pointsCount); - // First segment is prepended by an extra zero point - if (pointsCount == 0) - points[pointsCount++] = Vector2.Zero; - } - else + foreach (string s in pointSplit) { - points[pointsCount++] = readPoint(s, offset); + if (char.IsLetter(s[0])) + { + // The start of a new segment(indicated by having an alpha character at position 0). + var pathType = convertPathType(s); + segments[segmentsCount++] = (pathType, pointsCount); + + // First segment is prepended by an extra zero point + if (pointsCount == 0) + points[pointsCount++] = Vector2.Zero; + } + else + { + points[pointsCount++] = readPoint(s, offset); + } } + + var controlPoints = new List>(pointsCount); + + for (int i = 0; i < segmentsCount; i++) + { + int startIndex = segments[i].StartIndex; + int endIndex = i < segmentsCount - 1 ? segments[i + 1].StartIndex : pointsCount; + Vector2? endPoint = i < segmentsCount - 1 ? points[endIndex] : null; + controlPoints.AddRange(convertPoints(segments[i].Type, new ArraySegment(points, startIndex, endIndex - startIndex), endPoint)); + } + + return controlPoints.SelectMany(s => s).ToArray(); } - - var controlPoints = new List(pointsCount); - - for (int i = 0; i < segmentsCount; i++) + finally { - int startIndex = segments[i].StartIndex; - int endIndex = i < segmentsCount - 1 ? segments[i + 1].StartIndex : pointsCount; - Vector2? endPoint = i < segmentsCount - 1 ? points[endIndex] : null; - controlPoints.AddRange(convertPoints(segments[i].Type, points[startIndex..endIndex], endPoint)); + ArrayPool.Shared.Return(points); + ArrayPool<(PathType Type, int StartIndex)>.Shared.Return(segments); } - return controlPoints.ToArray(); - static Vector2 readPoint(string value, Vector2 startPos) { string[] vertexSplit = value.Split(':'); @@ -317,13 +326,12 @@ namespace osu.Game.Rulesets.Objects.Legacy /// The point list. /// Any extra endpoint to consider as part of the points. This will NOT be returned. /// The set of points contained by as one or more segments of the path. - private IEnumerable convertPoints(PathType type, ReadOnlySpan points, Vector2? endPoint) + private IEnumerable> convertPoints(PathType type, ArraySegment points, Vector2? endPoint) { - var vertices = new PathControlPoint[points.Length]; - var result = new List(); + var vertices = new PathControlPoint[points.Count]; // Parse into control points. - for (int i = 0; i < points.Length; i++) + for (int i = 0; i < points.Count; i++) vertices[i] = new PathControlPoint { Position = points[i] }; // Edge-case rules (to match stable). @@ -333,7 +341,7 @@ namespace osu.Game.Rulesets.Objects.Legacy if (vertices.Length + endPointLength != 3) type = PathType.BEZIER; - else if (isLinear(stackalloc[] { points[0], points[1], endPoint ?? points[2] })) + else if (isLinear(points[0], points[1], endPoint ?? points[2])) { // osu-stable special-cased colinear perfect curves to a linear path type = PathType.LINEAR; @@ -370,20 +378,18 @@ namespace osu.Game.Rulesets.Objects.Legacy // Force a type on the last point, and return the current control point set as a segment. vertices[endIndex - 1].Type = type; - for (int i = startIndex; i < endIndex; i++) - result.Add(vertices[i]); + yield return new ArraySegment(vertices, startIndex, endIndex - startIndex); // Skip the current control point - as it's the same as the one that's just been returned. startIndex = endIndex + 1; } - for (int i = startIndex; i < endIndex; i++) - result.Add(vertices[i]); + if (startIndex < endIndex) + yield return new ArraySegment(vertices, startIndex, endIndex - startIndex); - return result; - - static bool isLinear(ReadOnlySpan p) => Precision.AlmostEquals(0, (p[1].Y - p[0].Y) * (p[2].X - p[0].X) - - (p[1].X - p[0].X) * (p[2].Y - p[0].Y)); + static bool isLinear(Vector2 p0, Vector2 p1, Vector2 p2) + => Precision.AlmostEquals(0, (p1.Y - p0.Y) * (p2.X - p0.X) + - (p1.X - p0.X) * (p2.Y - p0.Y)); } /// From 470d2be2e1b4043953e067ef91e4c997138cc0ef Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Thu, 29 Feb 2024 00:07:00 +0800 Subject: [PATCH 05/11] The overhead of LINQ is not ignorable --- .../Objects/Legacy/ConvertHitObjectParser.cs | 21 ++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs b/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs index a3ca719ff9..72053648a0 100644 --- a/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs +++ b/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs @@ -302,7 +302,7 @@ namespace osu.Game.Rulesets.Objects.Legacy controlPoints.AddRange(convertPoints(segments[i].Type, new ArraySegment(points, startIndex, endIndex - startIndex), endPoint)); } - return controlPoints.SelectMany(s => s).ToArray(); + return mergePointsLists(controlPoints); } finally { @@ -392,6 +392,25 @@ namespace osu.Game.Rulesets.Objects.Legacy - (p1.X - p0.X) * (p2.Y - p0.Y)); } + private PathControlPoint[] mergePointsLists(List> controlPointList) + { + int totalCount = 0; + + foreach (var arr in controlPointList) + totalCount += arr.Count; + + var mergedArray = new PathControlPoint[totalCount]; + int copyIndex = 0; + + foreach (var arr in controlPointList) + { + arr.AsSpan().CopyTo(mergedArray.AsSpan(copyIndex)); + copyIndex += arr.Count; + } + + return mergedArray; + } + /// /// Creates a legacy Hit-type hit object. /// From e86ebd6cdba85e506a771aeab0fdeb0a71d74fab Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Thu, 29 Feb 2024 00:24:24 +0800 Subject: [PATCH 06/11] Fix formatting --- osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs b/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs index 72053648a0..f042e6ba26 100644 --- a/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs +++ b/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs @@ -271,9 +271,9 @@ namespace osu.Game.Rulesets.Objects.Legacy var segments = ArrayPool<(PathType Type, int StartIndex)>.Shared.Rent(pointSplit.Length); int pointsCount = 0; int segmentsCount = 0; + try { - foreach (string s in pointSplit) { if (char.IsLetter(s[0])) From f28f19ed7eeee1ecacc909f0f4f35edb07c7b8cb Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Thu, 29 Feb 2024 10:47:16 +0800 Subject: [PATCH 07/11] Fix method indent size Co-authored-by: Salman Ahmed --- osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs b/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs index f042e6ba26..db1fbe3fa4 100644 --- a/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs +++ b/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs @@ -389,7 +389,7 @@ namespace osu.Game.Rulesets.Objects.Legacy static bool isLinear(Vector2 p0, Vector2 p1, Vector2 p2) => Precision.AlmostEquals(0, (p1.Y - p0.Y) * (p2.X - p0.X) - - (p1.X - p0.X) * (p2.Y - p0.Y)); + - (p1.X - p0.X) * (p2.Y - p0.Y)); } private PathControlPoint[] mergePointsLists(List> controlPointList) From a11e63b184acc5030e3d167f13d85a3691f0b7dc Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Thu, 29 Feb 2024 20:02:04 +0800 Subject: [PATCH 08/11] Make the code more clear --- .../Objects/Legacy/ConvertHitObjectParser.cs | 42 ++++++++++++------- 1 file changed, 26 insertions(+), 16 deletions(-) diff --git a/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs b/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs index db1fbe3fa4..2b058d5e1f 100644 --- a/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs +++ b/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs @@ -265,49 +265,59 @@ namespace osu.Game.Rulesets.Objects.Legacy private PathControlPoint[] convertPathString(string pointString, Vector2 offset) { // This code takes on the responsibility of handling explicit segments of the path ("X" & "Y" from above). Implicit segments are handled by calls to convertPoints(). - string[] pointSplit = pointString.Split('|'); + string[] pointStringSplit = pointString.Split('|'); - var points = ArrayPool.Shared.Rent(pointSplit.Length); - var segments = ArrayPool<(PathType Type, int StartIndex)>.Shared.Rent(pointSplit.Length); - int pointsCount = 0; - int segmentsCount = 0; + var pointsBuffer = ArrayPool.Shared.Rent(pointStringSplit.Length); + var segmentsBuffer = ArrayPool<(PathType Type, int StartIndex)>.Shared.Rent(pointStringSplit.Length); + int currentPointsIndex = 0; + int currentSegmentsIndex = 0; try { - foreach (string s in pointSplit) + foreach (string s in pointStringSplit) { if (char.IsLetter(s[0])) { // The start of a new segment(indicated by having an alpha character at position 0). var pathType = convertPathType(s); - segments[segmentsCount++] = (pathType, pointsCount); + segmentsBuffer[currentSegmentsIndex++] = (pathType, currentPointsIndex); // First segment is prepended by an extra zero point - if (pointsCount == 0) - points[pointsCount++] = Vector2.Zero; + if (currentPointsIndex == 0) + pointsBuffer[currentPointsIndex++] = Vector2.Zero; } else { - points[pointsCount++] = readPoint(s, offset); + pointsBuffer[currentPointsIndex++] = readPoint(s, offset); } } + int pointsCount = currentPointsIndex; + int segmentsCount = currentSegmentsIndex; var controlPoints = new List>(pointsCount); + var allPoints = new ArraySegment(pointsBuffer, 0, pointsCount); for (int i = 0; i < segmentsCount; i++) { - int startIndex = segments[i].StartIndex; - int endIndex = i < segmentsCount - 1 ? segments[i + 1].StartIndex : pointsCount; - Vector2? endPoint = i < segmentsCount - 1 ? points[endIndex] : null; - controlPoints.AddRange(convertPoints(segments[i].Type, new ArraySegment(points, startIndex, endIndex - startIndex), endPoint)); + if (i < segmentsCount - 1) + { + int startIndex = segmentsBuffer[i].StartIndex; + int endIndex = segmentsBuffer[i + 1].StartIndex; + controlPoints.AddRange(convertPoints(segmentsBuffer[i].Type, allPoints[startIndex..endIndex], pointsBuffer[endIndex])); + } + else + { + int startIndex = segmentsBuffer[i].StartIndex; + controlPoints.AddRange(convertPoints(segmentsBuffer[i].Type, allPoints[startIndex..], null)); + } } return mergePointsLists(controlPoints); } finally { - ArrayPool.Shared.Return(points); - ArrayPool<(PathType Type, int StartIndex)>.Shared.Return(segments); + ArrayPool.Shared.Return(pointsBuffer); + ArrayPool<(PathType, int)>.Shared.Return(segmentsBuffer); } static Vector2 readPoint(string value, Vector2 startPos) From a8ce6a0bba5c65ab12360191d7589365d0996133 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Tue, 19 Mar 2024 02:43:34 +0300 Subject: [PATCH 09/11] Use `Slice` method instead of index range operators for readability --- .../Rulesets/Objects/Legacy/ConvertHitObjectParser.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs b/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs index 2b058d5e1f..283a59b7ed 100644 --- a/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs +++ b/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs @@ -303,16 +303,16 @@ namespace osu.Game.Rulesets.Objects.Legacy { int startIndex = segmentsBuffer[i].StartIndex; int endIndex = segmentsBuffer[i + 1].StartIndex; - controlPoints.AddRange(convertPoints(segmentsBuffer[i].Type, allPoints[startIndex..endIndex], pointsBuffer[endIndex])); + controlPoints.AddRange(convertPoints(segmentsBuffer[i].Type, allPoints.Slice(startIndex, endIndex - startIndex), pointsBuffer[endIndex])); } else { int startIndex = segmentsBuffer[i].StartIndex; - controlPoints.AddRange(convertPoints(segmentsBuffer[i].Type, allPoints[startIndex..], null)); + controlPoints.AddRange(convertPoints(segmentsBuffer[i].Type, allPoints.Slice(startIndex), null)); } } - return mergePointsLists(controlPoints); + return mergeControlPointsLists(controlPoints); } finally { @@ -402,7 +402,7 @@ namespace osu.Game.Rulesets.Objects.Legacy - (p1.X - p0.X) * (p2.Y - p0.Y)); } - private PathControlPoint[] mergePointsLists(List> controlPointList) + private PathControlPoint[] mergeControlPointsLists(List> controlPointList) { int totalCount = 0; From e2df0981843c3428fa11a232d341d97ac3e0505f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 22 Mar 2024 08:25:03 +0100 Subject: [PATCH 10/11] Add failing test case for desired artist sort behaviour --- .../SongSelect/TestSceneBeatmapCarousel.cs | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs b/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs index de2ae3708f..c0102b238c 100644 --- a/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs +++ b/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs @@ -666,6 +666,56 @@ namespace osu.Game.Tests.Visual.SongSelect AddAssert($"Check {zzz_lowercase} is second last", () => carousel.BeatmapSets.SkipLast(1).Last().Metadata.Artist == zzz_lowercase); } + [Test] + public void TestSortByArtistUsesTitleAsTiebreaker() + { + var sets = new List(); + + AddStep("Populuate beatmap sets", () => + { + sets.Clear(); + + for (int i = 0; i < 20; i++) + { + var set = TestResources.CreateTestBeatmapSetInfo(); + + if (i == 4) + { + set.Beatmaps.ForEach(b => + { + b.Metadata.Artist = "ZZZ"; + b.Metadata.Title = "AAA"; + }); + } + + if (i == 8) + { + set.Beatmaps.ForEach(b => + { + b.Metadata.Artist = "ZZZ"; + b.Metadata.Title = "ZZZ"; + }); + } + + sets.Add(set); + } + }); + + loadBeatmaps(sets); + + AddStep("Sort by artist", () => carousel.Filter(new FilterCriteria { Sort = SortMode.Artist }, false)); + AddAssert("Check last item", () => + { + var lastItem = carousel.BeatmapSets.Last(); + return lastItem.Metadata.Artist == "ZZZ" && lastItem.Metadata.Title == "ZZZ"; + }); + AddAssert("Check second last item", () => + { + var secondLastItem = carousel.BeatmapSets.SkipLast(1).Last(); + return secondLastItem.Metadata.Artist == "ZZZ" && secondLastItem.Metadata.Title == "AAA"; + }); + } + /// /// Ensures stability is maintained on different sort modes for items with equal properties. /// From a1880e89c299f55a8907050988b8200e854ed34b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 22 Mar 2024 08:31:59 +0100 Subject: [PATCH 11/11] Use title as tiebreaker when sorting beatmap carousel by artist Closes https://github.com/ppy/osu/issues/27548. Reference: https://github.com/peppy/osu-stable-reference/blob/e53980dd76857ee899f66ce519ba1597e7874f28/osu!/GameplayElements/Beatmaps/BeatmapTreeManager.cs#L341-L347 --- osu.Game/Screens/Select/Carousel/CarouselBeatmapSet.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/osu.Game/Screens/Select/Carousel/CarouselBeatmapSet.cs b/osu.Game/Screens/Select/Carousel/CarouselBeatmapSet.cs index 43c9c621e8..7e15699804 100644 --- a/osu.Game/Screens/Select/Carousel/CarouselBeatmapSet.cs +++ b/osu.Game/Screens/Select/Carousel/CarouselBeatmapSet.cs @@ -69,6 +69,8 @@ namespace osu.Game.Screens.Select.Carousel default: case SortMode.Artist: comparison = OrdinalSortByCaseStringComparer.DEFAULT.Compare(BeatmapSet.Metadata.Artist, otherSet.BeatmapSet.Metadata.Artist); + if (comparison == 0) + goto case SortMode.Title; break; case SortMode.Title: