From 843d2903d237a12c92781ced262576f7a65c9ff9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Mon, 22 May 2023 21:17:21 +0200 Subject: [PATCH 1/5] Add failing test case for slider velocity undo --- ...TestSceneHitObjectDifficultyPointAdjustments.cs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/osu.Game.Tests/Visual/Editing/TestSceneHitObjectDifficultyPointAdjustments.cs b/osu.Game.Tests/Visual/Editing/TestSceneHitObjectDifficultyPointAdjustments.cs index 3b998b4219..c874b39028 100644 --- a/osu.Game.Tests/Visual/Editing/TestSceneHitObjectDifficultyPointAdjustments.cs +++ b/osu.Game.Tests/Visual/Editing/TestSceneHitObjectDifficultyPointAdjustments.cs @@ -92,6 +92,20 @@ namespace osu.Game.Tests.Visual.Editing hitObjectHasVelocity(1, 5); } + [Test] + public void TestUndo() + { + clickDifficultyPiece(1); + velocityPopoverHasSingleValue(2); + + setVelocityViaPopover(5); + hitObjectHasVelocity(1, 5); + dismissPopover(); + + AddStep("undo", () => Editor.Undo()); + hitObjectHasVelocity(1, 2); + } + [Test] public void TestMultipleSelectionWithSameSliderVelocity() { From f253d17a7f0af2d21985fbf92ca8835a510e9ee5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Mon, 22 May 2023 22:19:10 +0200 Subject: [PATCH 2/5] Fix slider velocity changes not being applied in patcher --- .../Screens/Edit/LegacyEditorBeatmapPatcher.cs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/osu.Game/Screens/Edit/LegacyEditorBeatmapPatcher.cs b/osu.Game/Screens/Edit/LegacyEditorBeatmapPatcher.cs index b4647c2b64..33f8cd5c78 100644 --- a/osu.Game/Screens/Edit/LegacyEditorBeatmapPatcher.cs +++ b/osu.Game/Screens/Edit/LegacyEditorBeatmapPatcher.cs @@ -15,7 +15,9 @@ using osu.Framework.Graphics.Textures; using osu.Game.Beatmaps; using osu.Game.Beatmaps.ControlPoints; using osu.Game.Beatmaps.Formats; +using osu.Game.Beatmaps.Legacy; using osu.Game.IO; +using osu.Game.Rulesets.Objects.Types; using osu.Game.Skinning; using Decoder = osu.Game.Beatmaps.Formats.Decoder; @@ -42,6 +44,7 @@ namespace osu.Game.Screens.Edit editorBeatmap.BeginChange(); processHitObjects(result, () => newBeatmap ??= readBeatmap(newState)); processTimingPoints(() => newBeatmap ??= readBeatmap(newState)); + processSliderVelocity(() => newBeatmap ??= readBeatmap(newState)); editorBeatmap.EndChange(); } @@ -71,6 +74,17 @@ namespace osu.Game.Screens.Edit } } + private void processSliderVelocity(Func getNewBeatmap) + { + var legacyControlPoints = (LegacyControlPointInfo)getNewBeatmap().ControlPointInfo; + + foreach (var hitObject in editorBeatmap.HitObjects.Where(ho => ho is IHasSliderVelocity)) + { + var difficultyPoint = legacyControlPoints.DifficultyPointAt(hitObject.StartTime); + ((IHasSliderVelocity)hitObject).SliderVelocity = difficultyPoint.SliderVelocity; + } + } + private void processHitObjects(DiffResult result, Func getNewBeatmap) { findChangedIndices(result, LegacyDecoder.Section.HitObjects, out var removedIndices, out var addedIndices); From 2ce150ba2bbff1763a4be142ec802a5d942c96fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Mon, 22 May 2023 22:23:05 +0200 Subject: [PATCH 3/5] Add failing test case for sample undo --- .../TestSceneHitObjectSampleAdjustments.cs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/osu.Game.Tests/Visual/Editing/TestSceneHitObjectSampleAdjustments.cs b/osu.Game.Tests/Visual/Editing/TestSceneHitObjectSampleAdjustments.cs index 530bd5eb20..d812aed0f6 100644 --- a/osu.Game.Tests/Visual/Editing/TestSceneHitObjectSampleAdjustments.cs +++ b/osu.Game.Tests/Visual/Editing/TestSceneHitObjectSampleAdjustments.cs @@ -109,6 +109,21 @@ namespace osu.Game.Tests.Visual.Editing hitObjectHasSampleBank(1, "drum"); } + [Test] + public void TestUndo() + { + clickSamplePiece(1); + samplePopoverHasSingleBank("soft"); + samplePopoverHasSingleVolume(60); + + setVolumeViaPopover(90); + hitObjectHasSampleVolume(1, 90); + dismissPopover(); + + AddStep("undo", () => Editor.Undo()); + hitObjectHasSampleVolume(1, 60); + } + [Test] public void TestMultipleSelectionWithSameSampleVolume() { From e0b7539c2a1338f079b8052bff0cf1eeb5e1de37 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Mon, 22 May 2023 22:33:41 +0200 Subject: [PATCH 4/5] Fix sample changes not being applied in patcher --- .../Edit/LegacyEditorBeatmapPatcher.cs | 46 +++++++++++++------ 1 file changed, 33 insertions(+), 13 deletions(-) diff --git a/osu.Game/Screens/Edit/LegacyEditorBeatmapPatcher.cs b/osu.Game/Screens/Edit/LegacyEditorBeatmapPatcher.cs index 33f8cd5c78..ae7105ee34 100644 --- a/osu.Game/Screens/Edit/LegacyEditorBeatmapPatcher.cs +++ b/osu.Game/Screens/Edit/LegacyEditorBeatmapPatcher.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.IO; using System.Linq; using System.Text; @@ -15,7 +16,7 @@ using osu.Framework.Graphics.Textures; using osu.Game.Beatmaps; using osu.Game.Beatmaps.ControlPoints; using osu.Game.Beatmaps.Formats; -using osu.Game.Beatmaps.Legacy; +using osu.Game.Extensions; using osu.Game.IO; using osu.Game.Rulesets.Objects.Types; using osu.Game.Skinning; @@ -44,7 +45,7 @@ namespace osu.Game.Screens.Edit editorBeatmap.BeginChange(); processHitObjects(result, () => newBeatmap ??= readBeatmap(newState)); processTimingPoints(() => newBeatmap ??= readBeatmap(newState)); - processSliderVelocity(() => newBeatmap ??= readBeatmap(newState)); + processHitObjectLocalData(() => newBeatmap ??= readBeatmap(newState)); editorBeatmap.EndChange(); } @@ -74,17 +75,6 @@ namespace osu.Game.Screens.Edit } } - private void processSliderVelocity(Func getNewBeatmap) - { - var legacyControlPoints = (LegacyControlPointInfo)getNewBeatmap().ControlPointInfo; - - foreach (var hitObject in editorBeatmap.HitObjects.Where(ho => ho is IHasSliderVelocity)) - { - var difficultyPoint = legacyControlPoints.DifficultyPointAt(hitObject.StartTime); - ((IHasSliderVelocity)hitObject).SliderVelocity = difficultyPoint.SliderVelocity; - } - } - private void processHitObjects(DiffResult result, Func getNewBeatmap) { findChangedIndices(result, LegacyDecoder.Section.HitObjects, out var removedIndices, out var addedIndices); @@ -101,6 +91,36 @@ namespace osu.Game.Screens.Edit } } + private void processHitObjectLocalData(Func getNewBeatmap) + { + // This method handles data that are stored in control points in the legacy format, + // but were moved to the hitobjects themselves in lazer. + // Specifically, the data being referred to here consists of: slider velocity and sample information. + + // For simplicity, this implementation relies on the editor beatmap already having the same hitobjects in sequence as the new beatmap. + // To guarantee that, `processHitObjects()` must be ran prior to this method for correct operation. + // This is done to avoid the necessity of reimplementing/reusing parts of LegacyBeatmapDecoder that already treat this data correctly. + + var oldObjects = editorBeatmap.HitObjects; + var newObjects = getNewBeatmap().HitObjects; + + Debug.Assert(oldObjects.Count == newObjects.Count); + + foreach (var (oldObject, newObject) in oldObjects.Zip(newObjects)) + { + if (oldObject is IHasSliderVelocity oldWithVelocity && newObject is IHasSliderVelocity newWithVelocity) + oldWithVelocity.SliderVelocity = newWithVelocity.SliderVelocity; + + oldObject.Samples = newObject.Samples; + + if (oldObject is IHasRepeats oldWithRepeats && newObject is IHasRepeats newWithRepeats) + { + oldWithRepeats.NodeSamples.Clear(); + oldWithRepeats.NodeSamples.AddRange(newWithRepeats.NodeSamples); + } + } + } + private void findChangedIndices(DiffResult result, LegacyDecoder.Section section, out List removedIndices, out List addedIndices) { removedIndices = new List(); From 38b4bd8aefd738af26e26c1bf2309995f7253aee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Mon, 22 May 2023 22:45:39 +0200 Subject: [PATCH 5/5] Fix undo not behaving as expected sometimes --- osu.Game/Screens/Edit/LegacyEditorBeatmapPatcher.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/osu.Game/Screens/Edit/LegacyEditorBeatmapPatcher.cs b/osu.Game/Screens/Edit/LegacyEditorBeatmapPatcher.cs index ae7105ee34..2cf823ca0c 100644 --- a/osu.Game/Screens/Edit/LegacyEditorBeatmapPatcher.cs +++ b/osu.Game/Screens/Edit/LegacyEditorBeatmapPatcher.cs @@ -108,6 +108,11 @@ namespace osu.Game.Screens.Edit foreach (var (oldObject, newObject) in oldObjects.Zip(newObjects)) { + // if `oldObject` and `newObject` are the same, it means that `oldObject` was inserted into `editorBeatmap` by `processHitObjects()`. + // in that case, there is nothing to do (and some of the subsequent changes may even prove destructive). + if (ReferenceEquals(oldObject, newObject)) + continue; + if (oldObject is IHasSliderVelocity oldWithVelocity && newObject is IHasSliderVelocity newWithVelocity) oldWithVelocity.SliderVelocity = newWithVelocity.SliderVelocity;