From 749704344c5fbb0d46b153d98e60798e331a3965 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Thu, 30 Jan 2025 13:11:05 +0100 Subject: [PATCH 1/3] Move implicit slider path segment handling logic to Bezier converter The logic in `LegacyBeatmapEncoder` that was supposed to handle the lazer-exclusive feature of supporting multiple slider segment types in a single slider was interfering rather badly with the Bezier converter. Generally it was a bit difficult to follow, too. The nice thing about `BezierConverter` is that it is *guaranteed* to only output Bezier control points. In light of this, the same double-up- -the-control-point logic that was supposed to make multiple slider segment types backwards-compatible with stable can be placed in the Bezier conversion logic, and be *much* more understandable, too. --- .../Beatmaps/Formats/LegacyBeatmapEncoder.cs | 59 +++++-------------- osu.Game/Rulesets/Objects/BezierConverter.cs | 4 ++ 2 files changed, 19 insertions(+), 44 deletions(-) diff --git a/osu.Game/Beatmaps/Formats/LegacyBeatmapEncoder.cs b/osu.Game/Beatmaps/Formats/LegacyBeatmapEncoder.cs index 6c855e1346..07e88ab956 100644 --- a/osu.Game/Beatmaps/Formats/LegacyBeatmapEncoder.cs +++ b/osu.Game/Beatmaps/Formats/LegacyBeatmapEncoder.cs @@ -447,60 +447,31 @@ namespace osu.Game.Beatmaps.Formats private void addPathData(TextWriter writer, IHasPath pathData, Vector2 position) { - PathType? lastType = null; - for (int i = 0; i < pathData.Path.ControlPoints.Count; i++) { PathControlPoint point = pathData.Path.ControlPoints[i]; + // Note that lazer's encoding format supports specifying multiple curve types for a slider path, which is not supported by stable. + // Backwards compatibility with stable is handled by `LegacyBeatmapExporter` and `BezierConverter.ConvertToModernBezier()`. if (point.Type != null) { - // We've reached a new (explicit) segment! - - // Explicit segments have a new format in which the type is injected into the middle of the control point string. - // To preserve compatibility with osu-stable as much as possible, explicit segments with the same type are converted to use implicit segments by duplicating the control point. - // One exception are consecutive perfect curves, which aren't supported in osu!stable and can lead to decoding issues if encoded as implicit segments - bool needsExplicitSegment = point.Type != lastType || point.Type == PathType.PERFECT_CURVE || i == pathData.Path.ControlPoints.Count - 1; - - // Another exception to this is when the last two control points of the last segment were duplicated. This is not a scenario supported by osu!stable. - // Lazer does not add implicit segments for the last two control points of _any_ explicit segment, so an explicit segment is forced in order to maintain consistency with the decoder. - if (i > 1) + switch (point.Type?.Type) { - // We need to use the absolute control point position to determine equality, otherwise floating point issues may arise. - Vector2 p1 = position + pathData.Path.ControlPoints[i - 1].Position; - Vector2 p2 = position + pathData.Path.ControlPoints[i - 2].Position; + case SplineType.BSpline: + writer.Write(point.Type.Value.Degree > 0 ? $"B{point.Type.Value.Degree}|" : "B|"); + break; - if ((int)p1.X == (int)p2.X && (int)p1.Y == (int)p2.Y) - needsExplicitSegment = true; - } + case SplineType.Catmull: + writer.Write("C|"); + break; - if (needsExplicitSegment) - { - switch (point.Type?.Type) - { - case SplineType.BSpline: - writer.Write(point.Type.Value.Degree > 0 ? $"B{point.Type.Value.Degree}|" : "B|"); - break; + case SplineType.PerfectCurve: + writer.Write("P|"); + break; - case SplineType.Catmull: - writer.Write("C|"); - break; - - case SplineType.PerfectCurve: - writer.Write("P|"); - break; - - case SplineType.Linear: - writer.Write("L|"); - break; - } - - lastType = point.Type; - } - else - { - // New segment with the same type - duplicate the control point - writer.Write(FormattableString.Invariant($"{position.X + point.Position.X}:{position.Y + point.Position.Y}|")); + case SplineType.Linear: + writer.Write("L|"); + break; } } diff --git a/osu.Game/Rulesets/Objects/BezierConverter.cs b/osu.Game/Rulesets/Objects/BezierConverter.cs index 638975630e..384c686167 100644 --- a/osu.Game/Rulesets/Objects/BezierConverter.cs +++ b/osu.Game/Rulesets/Objects/BezierConverter.cs @@ -136,6 +136,7 @@ namespace osu.Game.Rulesets.Objects { for (int j = 0; j < segment.Length - 1; j++) { + if (result.Count > 0 && j == 0) result.Add(new PathControlPoint(segment[j])); result.Add(new PathControlPoint(segment[j], j == 0 ? PathType.BEZIER : null)); } } @@ -147,6 +148,7 @@ namespace osu.Game.Rulesets.Objects { for (int j = 0; j < segment.Length - 1; j++) { + if (result.Count > 0 && j == 0) result.Add(new PathControlPoint(segment[j])); result.Add(new PathControlPoint(segment[j], j == 0 ? PathType.BEZIER : null)); } } @@ -158,6 +160,7 @@ namespace osu.Game.Rulesets.Objects for (int j = 0; j < circleResult.Length - 1; j++) { + if (result.Count > 0 && j == 0) result.Add(new PathControlPoint(circleResult[j])); result.Add(new PathControlPoint(circleResult[j], j == 0 ? PathType.BEZIER : null)); } @@ -170,6 +173,7 @@ namespace osu.Game.Rulesets.Objects for (int j = 0; j < bSplineResult.Length - 1; j++) { + if (result.Count > 0 && j == 0) result.Add(new PathControlPoint(bSplineResult[j])); result.Add(new PathControlPoint(bSplineResult[j], j == 0 ? PathType.BEZIER : null)); } From b4f63da048e16c9f0fd0d339ea13f33637dade9c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Thu, 30 Jan 2025 15:23:22 +0100 Subject: [PATCH 2/3] Move control point double-up logic to `LegacyBeatmapExporter` Done for two reasons: - During review it was requested for the logic to be moved out of `BezierConverter` as `BezierConverter` was intended to produce "lazer style" sliders with per-control-point curve types, as a future usability / code layering concern. - It is also relevant for encode-decode stability. With how the logic was structured between the Bezier converter and the legacy beatmap encoder, the encoder would leave behind per-control-point Bezier curve specs that stable ignored, but subsequent encodes and decodes in lazer would end up multiplying the doubled-up control points ad nauseam. Instead, it is sufficient to only specify the curve type for the head control point as Bezier, not specify any further curve types later on, and instead just keep the double-up-control-point for new implicit segment logic which is enough to make stable cooperate (and also as close to outputting the slider exactly as stable would have produced it as we've ever been) --- osu.Game/Database/LegacyBeatmapExporter.cs | 32 ++++++++++++++------ osu.Game/Rulesets/Objects/BezierConverter.cs | 4 --- 2 files changed, 22 insertions(+), 14 deletions(-) diff --git a/osu.Game/Database/LegacyBeatmapExporter.cs b/osu.Game/Database/LegacyBeatmapExporter.cs index 24e752da31..9bb90ab461 100644 --- a/osu.Game/Database/LegacyBeatmapExporter.cs +++ b/osu.Game/Database/LegacyBeatmapExporter.cs @@ -120,18 +120,30 @@ namespace osu.Game.Database if (BezierConverter.CountSegments(hasPath.Path.ControlPoints) <= 1 && hasPath.Path.ControlPoints[0].Type!.Value.Degree == null) continue; - var newControlPoints = BezierConverter.ConvertToModernBezier(hasPath.Path.ControlPoints); - - // Truncate control points to integer positions - foreach (var pathControlPoint in newControlPoints) - { - pathControlPoint.Position = new Vector2( - (float)Math.Floor(pathControlPoint.Position.X), - (float)Math.Floor(pathControlPoint.Position.Y)); - } + var convertedToBezier = BezierConverter.ConvertToModernBezier(hasPath.Path.ControlPoints); hasPath.Path.ControlPoints.Clear(); - hasPath.Path.ControlPoints.AddRange(newControlPoints); + + for (int i = 0; i < convertedToBezier.Count; i++) + { + var convertedPoint = convertedToBezier[i]; + + // Truncate control points to integer positions + var position = new Vector2( + (float)Math.Floor(convertedPoint.Position.X), + (float)Math.Floor(convertedPoint.Position.Y)); + + // stable only supports a single curve type specification per slider. + // we exploit the fact that the converted-to-Bézier path only has Bézier segments, + // and thus we specify the Bézier curve type once ever at the start of the slider. + hasPath.Path.ControlPoints.Add(new PathControlPoint(position, i == 0 ? PathType.BEZIER : null)); + + // however, the Bézier path as output by the converter has multiple segments. + // `LegacyBeatmapEncoder` will attempt to encode this by emitting per-control-point curve type specs which don't do anything for stable. + // instead, stable expects control points that start a segment to be present in the path twice in succession. + if (convertedPoint.Type == PathType.BEZIER) + hasPath.Path.ControlPoints.Add(new PathControlPoint(position)); + } } // Encode to legacy format diff --git a/osu.Game/Rulesets/Objects/BezierConverter.cs b/osu.Game/Rulesets/Objects/BezierConverter.cs index 384c686167..638975630e 100644 --- a/osu.Game/Rulesets/Objects/BezierConverter.cs +++ b/osu.Game/Rulesets/Objects/BezierConverter.cs @@ -136,7 +136,6 @@ namespace osu.Game.Rulesets.Objects { for (int j = 0; j < segment.Length - 1; j++) { - if (result.Count > 0 && j == 0) result.Add(new PathControlPoint(segment[j])); result.Add(new PathControlPoint(segment[j], j == 0 ? PathType.BEZIER : null)); } } @@ -148,7 +147,6 @@ namespace osu.Game.Rulesets.Objects { for (int j = 0; j < segment.Length - 1; j++) { - if (result.Count > 0 && j == 0) result.Add(new PathControlPoint(segment[j])); result.Add(new PathControlPoint(segment[j], j == 0 ? PathType.BEZIER : null)); } } @@ -160,7 +158,6 @@ namespace osu.Game.Rulesets.Objects for (int j = 0; j < circleResult.Length - 1; j++) { - if (result.Count > 0 && j == 0) result.Add(new PathControlPoint(circleResult[j])); result.Add(new PathControlPoint(circleResult[j], j == 0 ? PathType.BEZIER : null)); } @@ -173,7 +170,6 @@ namespace osu.Game.Rulesets.Objects for (int j = 0; j < bSplineResult.Length - 1; j++) { - if (result.Count > 0 && j == 0) result.Add(new PathControlPoint(bSplineResult[j])); result.Add(new PathControlPoint(bSplineResult[j], j == 0 ? PathType.BEZIER : null)); } From 20280cd1959d0ceecff45f1e11a7aff3cedd5768 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 31 Jan 2025 09:01:42 +0100 Subject: [PATCH 3/3] Do not double up first control point of path --- osu.Game/Database/LegacyBeatmapExporter.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Database/LegacyBeatmapExporter.cs b/osu.Game/Database/LegacyBeatmapExporter.cs index 9bb90ab461..8f94fc9e63 100644 --- a/osu.Game/Database/LegacyBeatmapExporter.cs +++ b/osu.Game/Database/LegacyBeatmapExporter.cs @@ -141,7 +141,7 @@ namespace osu.Game.Database // however, the Bézier path as output by the converter has multiple segments. // `LegacyBeatmapEncoder` will attempt to encode this by emitting per-control-point curve type specs which don't do anything for stable. // instead, stable expects control points that start a segment to be present in the path twice in succession. - if (convertedPoint.Type == PathType.BEZIER) + if (convertedPoint.Type == PathType.BEZIER && i > 0) hasPath.Path.ControlPoints.Add(new PathControlPoint(position)); } }