h1or4dme8/osu-lazer
1
0
Fork 0
mirror of https://github.com/ppy/osu.git synced 2025-01-18 10:53:21 +08:00
Code Issues Releases Wiki Activity

Merge pull request #31109 from peppy/fix-editor-timing-undo-redo

Browse Source 
Fix adjusting control point offset after undo/redo causing catastrophic failure
This commit is contained in:
Bartłomiej Dach 2024-12-16 12:40:22 +09:00 committed by GitHub
commit 573d709fae
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 71 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

66
osu.Game.Tests/Visual/Editing/TestSceneTimingScreen.cs
View File

@ -32,6 +32,7 @@ namespace osu.Game.Tests.Visual.Editing
private TimingScreen timingScreen;
private EditorBeatmap editorBeatmap;
private BeatmapEditorChangeHandler changeHandler;
protected override bool ScrollUsingMouseWheel => false;
@ -46,6 +47,7 @@ namespace osu.Game.Tests.Visual.Editing
private void reloadEditorBeatmap()
{
editorBeatmap = new EditorBeatmap(Beatmap.Value.GetPlayableBeatmap(Ruleset.Value));
changeHandler = new BeatmapEditorChangeHandler(editorBeatmap);
Child = new DependencyProvidingContainer
{
@ -53,6 +55,7 @@ namespace osu.Game.Tests.Visual.Editing
CachedDependencies = new (Type, object)[]
{
(typeof(EditorBeatmap), editorBeatmap),
(typeof(IEditorChangeHandler), changeHandler),
(typeof(IBeatSnapProvider), editorBeatmap)
},
Child = timingScreen = new TimingScreen
@ -72,8 +75,10 @@ namespace osu.Game.Tests.Visual.Editing
AddUntilStep("Wait for rows to load", () => Child.ChildrenOfType<EffectRowAttribute>().Any());
}
// TODO: this is best-effort for now, but the comment out test below should probably be how things should work.
// Was originally working as of https://github.com/ppy/osu/pull/26141; Regressed at some point.
[Test]
public void TestSelectedRetainedOverUndo()
public void TestSelectionDismissedOnUndo()
{
AddStep("Select first timing point", () =>
{
@ -95,25 +100,52 @@ namespace osu.Game.Tests.Visual.Editing
return timingScreen.SelectedGroup.Value.ControlPoints.Any(c => c is TimingControlPoint) && timingScreen.SelectedGroup.Value.Time > 2170;
});
AddStep("simulate undo", () =>
{
var clone = editorBeatmap.ControlPointInfo.DeepClone();
AddStep("undo", () => changeHandler?.RestoreState(-1));
editorBeatmap.ControlPointInfo.Clear();
foreach (var group in clone.Groups)
{
foreach (var cp in group.ControlPoints)
editorBeatmap.ControlPointInfo.Add(group.Time, cp);
}
});
AddUntilStep("selection retained", () =>
{
return timingScreen.SelectedGroup.Value.ControlPoints.Any(c => c is TimingControlPoint) && timingScreen.SelectedGroup.Value.Time > 2170;
});
AddUntilStep("selection dismissed", () => timingScreen.SelectedGroup.Value, () => Is.Null);
}
// [Test]
// public void TestSelectedRetainedOverUndo()
// {
// AddStep("Select first timing point", () =>
// {
// InputManager.MoveMouseTo(Child.ChildrenOfType<TimingRowAttribute>().First());
// InputManager.Click(MouseButton.Left);
// });
//
// AddUntilStep("Selection changed", () => timingScreen.SelectedGroup.Value.Time == 2170);
// AddUntilStep("Ensure seeked to correct time", () => EditorClock.CurrentTimeAccurate == 2170);
//
// AddStep("Adjust offset", () =>
// {
// InputManager.MoveMouseTo(timingScreen.ChildrenOfType<TimingAdjustButton>().First().ScreenSpaceDrawQuad.Centre + new Vector2(20, 0));
// InputManager.Click(MouseButton.Left);
// });
//
// AddUntilStep("wait for offset changed", () =>
// {
// return timingScreen.SelectedGroup.Value.ControlPoints.Any(c => c is TimingControlPoint) && timingScreen.SelectedGroup.Value.Time > 2170;
// });
//
// AddStep("undo", () => changeHandler?.RestoreState(-1));
//
// AddUntilStep("selection retained", () =>
// {
// return timingScreen.SelectedGroup.Value.ControlPoints.Any(c => c is TimingControlPoint) && timingScreen.SelectedGroup.Value.Time > 2170;
// });
//
// AddAssert("check group count", () => editorBeatmap.ControlPointInfo.Groups.Count, () => Is.EqualTo(10));
//
// AddStep("Adjust offset", () =>
// {
// InputManager.MoveMouseTo(timingScreen.ChildrenOfType<TimingAdjustButton>().First().ScreenSpaceDrawQuad.Centre + new Vector2(20, 0));
// InputManager.Click(MouseButton.Left);
// });
//
// AddAssert("check group count", () => editorBeatmap.ControlPointInfo.Groups.Count, () => Is.EqualTo(10));
// }
[Test]
public void TestScrollControlGroupIntoView()
{

22
osu.Game/Screens/Edit/Timing/ControlPointList.cs
View File

@ -34,6 +34,9 @@ namespace osu.Game.Screens.Edit.Timing
[Resolved]
private Bindable<ControlPointGroup?> selectedGroup { get; set; } = null!;
[Resolved]
private IEditorChangeHandler? editorChangeHandler { get; set; }
[BackgroundDependencyLoader]
private void load(OsuColour colours, OverlayColourProvider colourProvider)
{
@ -110,6 +113,9 @@ namespace osu.Game.Screens.Edit.Timing
}
},
};
if (editorChangeHandler != null)
editorChangeHandler.OnStateChange += onUndoRedo;
}
protected override void LoadComplete()
@ -185,5 +191,21 @@ namespace osu.Game.Screens.Edit.Timing
selectedGroup.Value = group;
}
private void onUndoRedo()
{
// Best effort. We have no tracking of control points through undo/redo changes.
// If we don't deselect, things like offset changes could spawn groups to be added from previous states (see https://github.com/ppy/osu/issues/31098).
if (selectedGroup.Value != null && !Beatmap.ControlPointInfo.Groups.Contains(selectedGroup.Value))
selectedGroup.Value = null;
}
protected override void Dispose(bool isDisposing)
{
base.Dispose(isDisposing);
if (editorChangeHandler != null)
editorChangeHandler.OnStateChange -= onUndoRedo;
}
}
}