Add ability to split sliders in osu! editor #19858
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,193 @@ | ||
| // Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence. | ||
| // See the LICENCE file in the repository root for full licence text. | ||
|
|
||
| using System.Linq; | ||
| using NUnit.Framework; | ||
| using osu.Framework.Graphics.UserInterface; | ||
| using osu.Framework.Testing; | ||
| using osu.Framework.Utils; | ||
| using osu.Game.Beatmaps; | ||
| using osu.Game.Rulesets.Objects; | ||
| using osu.Game.Rulesets.Objects.Types; | ||
| using osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components; | ||
| using osu.Game.Rulesets.Osu.Objects; | ||
| using osu.Game.Screens.Edit.Compose.Components; | ||
| using osu.Game.Tests.Beatmaps; | ||
| using osu.Game.Tests.Visual; | ||
| using osuTK; | ||
|
|
||
| namespace osu.Game.Rulesets.Osu.Tests.Editor | ||
| { | ||
| public class TestSceneSliderSplitting : EditorTestScene | ||
| { | ||
| protected override Ruleset CreateEditorRuleset() => new OsuRuleset(); | ||
|
|
||
| protected override IBeatmap CreateBeatmap(RulesetInfo ruleset) => new TestBeatmap(ruleset, false); | ||
|
|
||
| private ComposeBlueprintContainer blueprintContainer | ||
| => Editor.ChildrenOfType<ComposeBlueprintContainer>().First(); | ||
|
|
||
| private Slider? slider; | ||
| private PathControlPointVisualiser? visualiser; | ||
|
|
||
| [Test] | ||
| public void TestBasicSplit() | ||
| { | ||
| double endTime = 0; | ||
|
|
||
| AddStep("add slider", () => | ||
| { | ||
| slider = new Slider | ||
| { | ||
| Position = new Vector2(0, 50), | ||
| 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)) | ||
| }) | ||
| }; | ||
|
|
||
| EditorBeatmap.Add(slider); | ||
|
|
||
| endTime = slider.EndTime; | ||
| }); | ||
|
|
||
| AddStep("select added slider", () => | ||
| { | ||
| EditorBeatmap.SelectedHitObjects.Add(slider); | ||
| visualiser = blueprintContainer.SelectionBlueprints.First(o => o.Item == slider).ChildrenOfType<PathControlPointVisualiser>().First(); | ||
| }); | ||
|
|
||
| moveMouseToControlPoint(2); | ||
| AddStep("select control point", () => | ||
| { | ||
| if (visualiser is not null) visualiser.Pieces[2].IsSelected.Value = true; | ||
| }); | ||
| addContextMenuItemStep("Split control point"); | ||
|
|
||
| AddAssert("slider split", () => slider is not null && EditorBeatmap.HitObjects.Count == 2 && | ||
| sliderCreatedFor((Slider)EditorBeatmap.HitObjects[0], 0, slider.StartTime, | ||
| (new Vector2(0, 50), PathType.PerfectCurve), | ||
| (new Vector2(150, 200), null), | ||
| (new Vector2(300, 50), null) | ||
| ) && sliderCreatedFor((Slider)EditorBeatmap.HitObjects[1], slider.StartTime, endTime, | ||
| (new Vector2(300, 50), PathType.PerfectCurve), | ||
| (new Vector2(400, 50), null), | ||
| (new Vector2(400, 200), null) | ||
| )); | ||
|
|
||
| AddStep("undo", () => Editor.Undo()); | ||
| AddAssert("original slider restored", () => EditorBeatmap.HitObjects.Count == 1 && sliderCreatedFor((Slider)EditorBeatmap.HitObjects[0], 0, endTime, | ||
| (new Vector2(0, 50), PathType.PerfectCurve), | ||
| (new Vector2(150, 200), null), | ||
| (new Vector2(300, 50), PathType.PerfectCurve), | ||
| (new Vector2(400, 50), null), | ||
| (new Vector2(400, 200), null) | ||
| )); | ||
| } | ||
|
|
||
| [Test] | ||
| public void TestDoubleSplit() | ||
| { | ||
| double endTime = 0; | ||
|
|
||
| AddStep("add slider", () => | ||
| { | ||
| slider = new Slider | ||
| { | ||
| Position = new Vector2(0, 50), | ||
| Path = new SliderPath(new[] | ||
| { | ||
| new PathControlPoint(Vector2.Zero, PathType.PerfectCurve), | ||
| new PathControlPoint(new Vector2(150, 150)), | ||
| new PathControlPoint(new Vector2(300, 0), PathType.Bezier), | ||
| new PathControlPoint(new Vector2(400, 0)), | ||
| new PathControlPoint(new Vector2(400, 150), PathType.Catmull), | ||
| new PathControlPoint(new Vector2(300, 200)), | ||
| new PathControlPoint(new Vector2(400, 250)) | ||
| }) | ||
| }; | ||
|
|
||
| EditorBeatmap.Add(slider); | ||
|
|
||
| endTime = slider.EndTime; | ||
| }); | ||
|
|
||
| AddStep("select added slider", () => | ||
| { | ||
| EditorBeatmap.SelectedHitObjects.Add(slider); | ||
| visualiser = blueprintContainer.SelectionBlueprints.First(o => o.Item == slider).ChildrenOfType<PathControlPointVisualiser>().First(); | ||
| }); | ||
|
|
||
| moveMouseToControlPoint(2); | ||
| AddStep("select first control point", () => | ||
| { | ||
| if (visualiser is not null) visualiser.Pieces[2].IsSelected.Value = true; | ||
| }); | ||
| moveMouseToControlPoint(4); | ||
| AddStep("select second control point", () => | ||
| { | ||
| if (visualiser is not null) visualiser.Pieces[4].IsSelected.Value = true; | ||
| }); | ||
| addContextMenuItemStep("Split 2 control points"); | ||
|
|
||
| AddAssert("slider split", () => slider is not null && EditorBeatmap.HitObjects.Count == 3 && | ||
| sliderCreatedFor((Slider)EditorBeatmap.HitObjects[0], 0, EditorBeatmap.HitObjects[1].StartTime, | ||
| (new Vector2(0, 50), PathType.PerfectCurve), | ||
| (new Vector2(150, 200), null), | ||
| (new Vector2(300, 50), null) | ||
| ) && sliderCreatedFor((Slider)EditorBeatmap.HitObjects[1], EditorBeatmap.HitObjects[0].GetEndTime(), slider.StartTime, | ||
| (new Vector2(300, 50), PathType.Bezier), | ||
| (new Vector2(400, 50), null), | ||
| (new Vector2(400, 200), null) | ||
| ) && sliderCreatedFor((Slider)EditorBeatmap.HitObjects[2], EditorBeatmap.HitObjects[1].GetEndTime(), endTime, | ||
| (new Vector2(400, 200), PathType.Catmull), | ||
| (new Vector2(300, 250), null), | ||
| (new Vector2(400, 300), null) | ||
| )); | ||
| } | ||
|
|
||
| private bool sliderCreatedFor(Slider s, double startTime, double endTime, params (Vector2 pos, PathType? pathType)[] expectedControlPoints) | ||
| { | ||
| if (!Precision.AlmostEquals(s.StartTime, startTime, 1) || !Precision.AlmostEquals(s.EndTime, endTime, 1)) return false; | ||
|
|
||
| int i = 0; | ||
|
|
||
| foreach ((Vector2 pos, PathType? pathType) in expectedControlPoints) | ||
| { | ||
| var controlPoint = s.Path.ControlPoints[i++]; | ||
|
|
||
| if (!Precision.AlmostEquals(controlPoint.Position + s.Position, pos) || controlPoint.Type != pathType) | ||
| return false; | ||
| } | ||
|
|
||
| return true; | ||
| } | ||
|
|
||
| private void moveMouseToControlPoint(int index) | ||
| { | ||
| AddStep($"move mouse to control point {index}", () => | ||
| { | ||
| if (slider is null || visualiser is null) return; | ||
|
|
||
| Vector2 position = slider.Path.ControlPoints[index].Position + slider.Position; | ||
| InputManager.MoveMouseTo(visualiser.Pieces[0].Parent.ToScreenSpace(position)); | ||
| }); | ||
| } | ||
|
|
||
| private void addContextMenuItemStep(string contextMenuText) | ||
| { | ||
| AddStep($"click context menu item \"{contextMenuText}\"", () => | ||
| { | ||
| if (visualiser is null) return; | ||
|
|
||
| MenuItem? item = visualiser.ContextMenuItems.FirstOrDefault(menuItem => menuItem.Text.Value == contextMenuText); | ||
|
|
||
| item?.Action?.Value(); | ||
| }); | ||
| } | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,6 +13,7 @@ | |
| using osu.Framework.Graphics.UserInterface; | ||
| using osu.Framework.Input.Events; | ||
| using osu.Framework.Utils; | ||
| using osu.Game.Audio; | ||
| using osu.Game.Beatmaps.ControlPoints; | ||
| using osu.Game.Graphics.UserInterface; | ||
| using osu.Game.Rulesets.Edit; | ||
|
|
@@ -111,7 +112,8 @@ protected override void OnSelected() | |
| { | ||
| AddInternal(ControlPointVisualiser = new PathControlPointVisualiser(HitObject, true) | ||
| { | ||
| RemoveControlPointsRequested = removeControlPoints, | ||
| SplitControlPointsRequested = splitControlPoints | ||
| }); | ||
|
|
||
| base.OnSelected(); | ||
|
|
@@ -249,6 +251,75 @@ private void removeControlPoints(List<PathControlPoint> toRemove) | |
| HitObject.Position += first; | ||
| } | ||
|
|
||
| private void splitControlPoints(List<PathControlPoint> toSplit) | ||
| { | ||
| // Ensure that there are any points to be split | ||
| if (toSplit.Count == 0) | ||
| return; | ||
|
|
||
| foreach (var c in toSplit) | ||
| { | ||
| if (c == controlPoints[0] || c == controlPoints[^1] || c.Type is null) | ||
| continue; | ||
|
|
||
| // Split off the section of slider before this control point so the remaining control points to split are in the latter part of the slider. | ||
| var splitControlPoints = controlPoints.TakeWhile(current => current != c).ToList(); | ||
|
|
||
| if (splitControlPoints.Count == 0) | ||
| continue; | ||
|
|
||
| foreach (var current in splitControlPoints) | ||
| { | ||
| controlPoints.Remove(current); | ||
| } | ||
|
|
||
| splitControlPoints.Add(c); | ||
|
|
||
| // Turn the control points which were split off into a new slider. | ||
| var samplePoint = (SampleControlPoint)HitObject.SampleControlPoint.DeepClone(); | ||
| samplePoint.Time = HitObject.StartTime; | ||
| var difficultyPoint = (DifficultyControlPoint)HitObject.DifficultyControlPoint.DeepClone(); | ||
| difficultyPoint.Time = HitObject.StartTime; | ||
|
|
||
| var newSlider = new Slider | ||
| { | ||
| StartTime = HitObject.StartTime, | ||
| Position = HitObject.Position + splitControlPoints[0].Position, | ||
| NewCombo = HitObject.NewCombo, | ||
| SampleControlPoint = samplePoint, | ||
| DifficultyControlPoint = difficultyPoint, | ||
| Samples = HitObject.Samples.Select(s => s.With()).ToList(), | ||
| RepeatCount = HitObject.RepeatCount, | ||
| NodeSamples = HitObject.NodeSamples.Select(n => (IList<HitSampleInfo>)n.Select(s => s.With()).ToList()).ToList(), | ||
| Path = new SliderPath(splitControlPoints.Select(o => new PathControlPoint(o.Position - splitControlPoints[0].Position, o == splitControlPoints[^1] ? null : o.Type)).ToArray()) | ||
| }; | ||
|
|
||
| // Increase the start time of the slider before adding the new slider so the new slider is immediately inserted at the correct index and internal state remains valid. | ||
| HitObject.StartTime += 1; | ||
|
There was a problem hiding this comment. Of note, I haven't finished reviewing this because of this code. I'm very unsure of how correct this is (for instance, the If possible I'd like to see this moved into a method inside There was a problem hiding this comment.
I'm not sure what you mean by this. I saw that the sample control point should be 1 ms after the end of the slider, but internal updating stuff already makes it so that the sample control point is updated to this correct time. I did some testing however and noticed that the sample set of the splitted sliders do not get retained after a save and reload, so maybe something is indeed wrong with this code, but I don't know why this happens, I don't know enough about how hitsounds work in lazer. I tried to copy as much as possible from the convert-to-stream code.
Do you think a There was a problem hiding this comment.
I now understand why this happens and it's just There was a problem hiding this comment.
Then why are the control point times even set in the code above? It just seems misleading as written if those lines aren't even doing anything and the values assigned there aren't even final. There was a problem hiding this comment. ill remove those lines |
||
|
|
||
| editorBeatmap.Add(newSlider); | ||
|
|
||
| HitObject.NewCombo = false; | ||
| HitObject.Path.ExpectedDistance.Value -= newSlider.Path.CalculatedDistance; | ||
| HitObject.StartTime += newSlider.SpanDuration - 1; | ||
|
|
||
| // In case the remainder of the slider has no length left over, give it length anyways so we don't get a 0 length slider. | ||
| if (HitObject.Path.ExpectedDistance.Value <= Precision.DOUBLE_EPSILON) | ||
| { | ||
| HitObject.Path.ExpectedDistance.Value = null; | ||
| } | ||
| } | ||
|
|
||
| // The path will have a non-zero offset if the head is removed, but sliders don't support this behaviour since the head is positioned at the slider's position | ||
| // So the slider needs to be offset by this amount instead, and all control points offset backwards such that the path is re-positioned at (0, 0) | ||
| Vector2 first = controlPoints[0].Position; | ||
| foreach (var c in controlPoints) | ||
| c.Position -= first; | ||
| HitObject.Position += first; | ||
|
|
||
| editorBeatmap.SelectedHitObjects.Clear(); | ||
| } | ||
|
|
||
| private void convertToStream() | ||
| { | ||
| if (editorBeatmap == null || beatDivisor == null) | ||
|
|
||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Related, but isn't
splitControlPoints[0].Positionalways... zero? As I understand it,newSlideris the first part of the slider, not the second - so when would the first control point not be at (0,0)? It's confusing me as it looks like something you'd do when you wantednewSliderto become the second part of the slider rather than the first.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Slider control points are removed from the start of
HitObjectto createnewSlider, so at that pointsplitControlPoints[0].Positionis not zero. There is nothing that really forces the first control point to be zero.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But that would imply that the
PositionofnewSlideris not the same as the position of the original slider? How can this be ifnewSlideris supposed to be the starting segment?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My explanation was not completely right.
splitControlPoints[0].Positionis zero on the first split, but it can split in multiple points at the same time. On the second split, theHitObjectis missing some of the control points, sosplitControlPoints[0].Positionis not zero, and theHitObject.Positionhas not changed, thusHitObject.Position + splitControlPoints[0].Positionis the actual starting position of the new slider. It accounts for theHitObject.Positionnot updating in the previous splits.