From 56af009e7759d364b6ded4d57d0f9c32c1f6dbd0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Mon, 22 Jul 2024 14:47:33 +0200 Subject: [PATCH 1/3] Fix `EditablePath.UpdateHitObjectFromPath()` not automatically updating object This is important because the editable path conversions heavily depend on the value of `JuiceStream.Velocity` being correct. The value is only guaranteed to be correct after an `ApplyDefaults()` call, which is triggered by updating the object via `EditorBeatmap`. --- .../Blueprints/Components/EditablePath.cs | 5 ++++ .../Components/SelectionEditablePath.cs | 23 ++++++++----------- 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/osu.Game.Rulesets.Catch/Edit/Blueprints/Components/EditablePath.cs b/osu.Game.Rulesets.Catch/Edit/Blueprints/Components/EditablePath.cs index 86f92d16ca..857c00cd41 100644 --- a/osu.Game.Rulesets.Catch/Edit/Blueprints/Components/EditablePath.cs +++ b/osu.Game.Rulesets.Catch/Edit/Blueprints/Components/EditablePath.cs @@ -13,6 +13,7 @@ using osu.Game.Rulesets.Catch.Objects; using osu.Game.Rulesets.Edit; using osu.Game.Rulesets.Objects.Types; using osu.Game.Rulesets.UI.Scrolling; +using osu.Game.Screens.Edit; using osuTK; namespace osu.Game.Rulesets.Catch.Edit.Blueprints.Components @@ -42,6 +43,9 @@ namespace osu.Game.Rulesets.Catch.Edit.Blueprints.Components [Resolved] private IBeatSnapProvider? beatSnapProvider { get; set; } + [Resolved] + protected EditorBeatmap? EditorBeatmap { get; private set; } + protected EditablePath(Func positionToTime) { PositionToTime = positionToTime; @@ -112,6 +116,7 @@ namespace osu.Game.Rulesets.Catch.Edit.Blueprints.Components double endTime = hitObject.StartTime + path.Duration; double snappedEndTime = beatSnapProvider.SnapTime(endTime, hitObject.StartTime); hitObject.Path.ExpectedDistance.Value = (snappedEndTime - hitObject.StartTime) * hitObject.Velocity; + EditorBeatmap?.Update(hitObject); } public Vector2 ToRelativePosition(Vector2 screenSpacePosition) diff --git a/osu.Game.Rulesets.Catch/Edit/Blueprints/Components/SelectionEditablePath.cs b/osu.Game.Rulesets.Catch/Edit/Blueprints/Components/SelectionEditablePath.cs index 6a4e35b1f9..b2ee43ba16 100644 --- a/osu.Game.Rulesets.Catch/Edit/Blueprints/Components/SelectionEditablePath.cs +++ b/osu.Game.Rulesets.Catch/Edit/Blueprints/Components/SelectionEditablePath.cs @@ -4,13 +4,11 @@ using System; using System.Collections.Generic; using System.Linq; -using osu.Framework.Allocation; using osu.Framework.Graphics.Cursor; using osu.Framework.Graphics.UserInterface; using osu.Framework.Input.Events; using osu.Game.Graphics.UserInterface; using osu.Game.Rulesets.Catch.Objects; -using osu.Game.Screens.Edit; using osuTK; using osuTK.Input; @@ -25,9 +23,6 @@ namespace osu.Game.Rulesets.Catch.Edit.Blueprints.Components // To handle when the editor is scrolled while dragging. private Vector2 dragStartPosition; - [Resolved] - private IEditorChangeHandler? changeHandler { get; set; } - public SelectionEditablePath(JuiceStream juiceStream, Func positionToTime) : base(positionToTime) { @@ -36,12 +31,14 @@ namespace osu.Game.Rulesets.Catch.Edit.Blueprints.Components public void AddVertex(Vector2 relativePosition) { - changeHandler?.BeginChange(); + EditorBeatmap?.BeginChange(); + double time = Math.Max(0, PositionToTime(relativePosition.Y)); int index = AddVertex(time, relativePosition.X); UpdateHitObjectFromPath(juiceStream); selectOnly(index); - changeHandler?.EndChange(); + + EditorBeatmap?.EndChange(); } public override bool ReceivePositionalInputAt(Vector2 screenSpacePos) => InternalChildren.Any(d => d.ReceivePositionalInputAt(screenSpacePos)); @@ -54,10 +51,10 @@ namespace osu.Game.Rulesets.Catch.Edit.Blueprints.Components if (e.Button == MouseButton.Right && e.ShiftPressed) { - changeHandler?.BeginChange(); + EditorBeatmap?.BeginChange(); RemoveVertex(index); UpdateHitObjectFromPath(juiceStream); - changeHandler?.EndChange(); + EditorBeatmap?.EndChange(); return true; } @@ -85,7 +82,7 @@ namespace osu.Game.Rulesets.Catch.Edit.Blueprints.Components for (int i = 0; i < VertexCount; i++) VertexStates[i].VertexBeforeChange = Vertices[i]; - changeHandler?.BeginChange(); + EditorBeatmap?.BeginChange(); return true; } @@ -99,7 +96,7 @@ namespace osu.Game.Rulesets.Catch.Edit.Blueprints.Components protected override void OnDragEnd(DragEndEvent e) { - changeHandler?.EndChange(); + EditorBeatmap?.EndChange(); } private int getMouseTargetVertex(Vector2 screenSpacePosition) @@ -129,7 +126,7 @@ namespace osu.Game.Rulesets.Catch.Edit.Blueprints.Components private void deleteSelectedVertices() { - changeHandler?.BeginChange(); + EditorBeatmap?.BeginChange(); for (int i = VertexCount - 1; i >= 0; i--) { @@ -139,7 +136,7 @@ namespace osu.Game.Rulesets.Catch.Edit.Blueprints.Components UpdateHitObjectFromPath(juiceStream); - changeHandler?.EndChange(); + EditorBeatmap?.EndChange(); } } } From f3617eadad1f997d5cd4eea45eb28c22467c25f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Mon, 22 Jul 2024 14:51:45 +0200 Subject: [PATCH 2/3] Fix editing juice stream path sometimes changing its duration I'm not *super* sure why this works, but it appears to, and my educated guess as to why is that it counteracts the effects of a change in the SV of the juice stream by artificially increasing or decreasing the velocity when running the appropriate path conversions and expected distance calculations. The actual SV change takes effect on the next default application, which is triggered by the `Update()` call at the end of the method. --- .../Edit/Blueprints/Components/EditablePath.cs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/osu.Game.Rulesets.Catch/Edit/Blueprints/Components/EditablePath.cs b/osu.Game.Rulesets.Catch/Edit/Blueprints/Components/EditablePath.cs index 857c00cd41..e626392234 100644 --- a/osu.Game.Rulesets.Catch/Edit/Blueprints/Components/EditablePath.cs +++ b/osu.Game.Rulesets.Catch/Edit/Blueprints/Components/EditablePath.cs @@ -107,15 +107,22 @@ namespace osu.Game.Rulesets.Catch.Edit.Blueprints.Components // // The value is clamped here by the bindable min and max values. // In case the required velocity is too large, the path is not preserved. + double previousVelocity = svBindable.Value; svBindable.Value = Math.Ceiling(requiredVelocity / svToVelocityFactor); - path.ConvertToSliderPath(hitObject.Path, hitObject.LegacyConvertedY, hitObject.Velocity); + // adjust velocity locally, so that once the SV change is applied by applying defaults + // (triggered by `EditorBeatmap.Update()` call at end of method), + // it results in the outcome desired by the user. + double relativeChange = svBindable.Value / previousVelocity; + double localVelocity = hitObject.Velocity * relativeChange; + path.ConvertToSliderPath(hitObject.Path, hitObject.LegacyConvertedY, localVelocity); if (beatSnapProvider == null) return; double endTime = hitObject.StartTime + path.Duration; double snappedEndTime = beatSnapProvider.SnapTime(endTime, hitObject.StartTime); - hitObject.Path.ExpectedDistance.Value = (snappedEndTime - hitObject.StartTime) * hitObject.Velocity; + hitObject.Path.ExpectedDistance.Value = (snappedEndTime - hitObject.StartTime) * localVelocity; + EditorBeatmap?.Update(hitObject); } From 6100f5269d757177d557af714a17a8c116530dbd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Mon, 22 Jul 2024 14:57:36 +0200 Subject: [PATCH 3/3] Fix tests --- .../TestSceneJuiceStreamSelectionBlueprint.cs | 20 ++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/osu.Game.Rulesets.Catch.Tests/Editor/TestSceneJuiceStreamSelectionBlueprint.cs b/osu.Game.Rulesets.Catch.Tests/Editor/TestSceneJuiceStreamSelectionBlueprint.cs index 10cf294a36..7b665b1ff9 100644 --- a/osu.Game.Rulesets.Catch.Tests/Editor/TestSceneJuiceStreamSelectionBlueprint.cs +++ b/osu.Game.Rulesets.Catch.Tests/Editor/TestSceneJuiceStreamSelectionBlueprint.cs @@ -82,6 +82,7 @@ namespace osu.Game.Rulesets.Catch.Tests.Editor AddMouseMoveStep(-100, 100); addVertexCheckStep(3, 1, times[0], positions[0]); + addDragEndStep(); } [Test] @@ -100,6 +101,9 @@ namespace osu.Game.Rulesets.Catch.Tests.Editor AddMouseMoveStep(times[2] - 50, positions[2] - 50); addVertexCheckStep(4, 1, times[1] - 50, positions[1] - 50); addVertexCheckStep(4, 2, times[2] - 50, positions[2] - 50); + + AddStep("release control", () => InputManager.ReleaseKey(Key.ControlLeft)); + addDragEndStep(); } [Test] @@ -113,6 +117,7 @@ namespace osu.Game.Rulesets.Catch.Tests.Editor addDragStartStep(times[1], positions[1]); AddMouseMoveStep(times[1], 400); AddAssert("slider velocity changed", () => !hitObject.SliderVelocityMultiplierBindable.IsDefault); + addDragEndStep(); } [Test] @@ -129,6 +134,7 @@ namespace osu.Game.Rulesets.Catch.Tests.Editor AddStep("scroll playfield", () => manualClock.CurrentTime += 200); AddMouseMoveStep(times[1] + 200, positions[1] + 100); addVertexCheckStep(2, 1, times[1] + 200, positions[1] + 100); + addDragEndStep(); } [Test] @@ -158,21 +164,21 @@ namespace osu.Game.Rulesets.Catch.Tests.Editor float[] positions = { 200, 200 }; addBlueprintStep(times, positions, 0.2); - addAddVertexSteps(500, 180); - addVertexCheckStep(3, 1, 500, 180); + addAddVertexSteps(500, 150); + addVertexCheckStep(3, 1, 500, 150); - addAddVertexSteps(90, 200); - addVertexCheckStep(4, 1, times[0], positions[0]); + addAddVertexSteps(160, 200); + addVertexCheckStep(4, 1, 160, 200); - addAddVertexSteps(750, 200); - addVertexCheckStep(5, 4, 750, 200); + addAddVertexSteps(750, 180); + addVertexCheckStep(5, 4, 800, 160); AddAssert("duration is changed", () => Precision.AlmostEquals(hitObject.Duration, 800 - times[0], 1e-3)); } [Test] public void TestDeleteVertex() { - double[] times = { 100, 300, 500 }; + double[] times = { 100, 300, 400 }; float[] positions = { 100, 200, 150 }; addBlueprintStep(times, positions);