Add B-Spline slider curve type #24457
Conversation
| string name = type switch | ||
| { | ||
| null => "Inherit", | ||
| PathType.BSpline => "B-Spline", | ||
| _ => type.ToString().Humanize() | ||
| }; |
There was a problem hiding this comment.
Rather than do this I'd much prefer to stop relying on humanizer at all and instead explicitly annotate the enum members with [Description] or [LocalisableDescription].
| if (segmentCount == 0 || (segmentCount == 1 && hasPath.Path.ControlPoints[0].Type != PathType.BSpline)) | ||
| continue; |
There was a problem hiding this comment.
I'm pretty sure this should be something like
| if (segmentCount == 0 || (segmentCount == 1 && hasPath.Path.ControlPoints[0].Type != PathType.BSpline)) | |
| continue; | |
| if (segmentCount <= 1 && hasPath.Path.ControlPoints.All(p => p.Type != PathType.BSpline)) |
because you can have multiple B-spline segments in a slider. And as soon as you get one you need to convert (but you also need to convert if you have more, or if it isn't first, which the current condition isn't checking).
| @@ -87,6 +87,11 @@ public static List<Vector2> ConvertToLegacyBezier(IList<PathControlPoint> contro | |||
|
|
|||
| break; | |||
|
|
|||
| case PathType.BSpline: | |||
| result.AddRange(from segment in ConvertBSplineToBezierAnchors(segmentVertices) from v in segment select v + position); | |||
There was a problem hiding this comment.
Ew it's the fake sql syntax. I'll look away because it's already being used elsewhere but I kinda hate it.
|
TO note before we get too deep in this: I am not sure this should exist as a new option. We already have enough curve types that the learning curve (ha) is quite high. Adding a new one which is very specialised to people who know how to manipulate it, where there is no inherit value as the same shapes can be made with existing multi segment is not necessarily a great direction IMO. Not to say this can't exist, but I'm not sure how it should fit in the UX. |
Calling the B-splines "very specialised to people who know how to manipulate it" is not fair, because using B-spline to create smooth slider shapes is actually much more intuitive than Bézier due to the anchors in B-splines having a fixed and local area of influence. For that reason, I think B-spline is suitable as a replacement for Bézier as the default curve type in path segments with >3 anchors. Though, adding an option to choose whether you want to have Bézier or B-spline as the default will be required, because a lot of mappers right now are so used to the quirks of Bézier already. |
|
This PR is probably redundant since #25409 |
Summary
RFC. Added b-splines as a curve type for sliders.
Changes made
SliderPath.csPathControlPiece.csControlPointVisualiser.csLegacyBeatmapEncoder.csLegacyBeatmapExporter.csBezierConverter.csConvertHitObjectParser.csTestSceneSlider.csTestSceneBezierConverter.csReasoning
B-spline curves combine some of the benefits of both bezier & catmull sliders. They follow the control points more strictly, similar to catmull sliders, while maintaining the aesthetics of bezier sliders. Since they follow the control points more closely, they are easier to work with for new mappers, compared to bezier sliders, where the generated curve can vary widely from the actual control points.
Examples
Thanks to @OliBomby for helping me with this