h1or4dme8/osu-lazer
1
0
Fork 0
mirror of https://github.com/ppy/osu.git synced 2025-02-06 20:22:56 +08:00
Code Issues Releases Wiki Activity

Fix adjusting control point offset after undo/redo causing catastrophic failure

Browse Source 
Closes https://github.com/ppy/osu/issues/31098.

Low effort fix because it was already half broken. The test was testing
in isolation but in actual editor usage it wasn't working as expected.
This commit is contained in:
Dean Herbert 2024-12-13 19:33:47 +09:00
parent 35c70ceb18
commit 64555debc2
No known key found for this signature in database
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 TimingScreen timingScreen;
private EditorBeatmap editorBeatmap; private EditorBeatmap editorBeatmap;
private BeatmapEditorChangeHandler changeHandler;
protected override bool ScrollUsingMouseWheel => false; protected override bool ScrollUsingMouseWheel => false;
@ -46,6 +47,7 @@ namespace osu.Game.Tests.Visual.Editing
private void reloadEditorBeatmap() private void reloadEditorBeatmap()
{ {
editorBeatmap = new EditorBeatmap(Beatmap.Value.GetPlayableBeatmap(Ruleset.Value)); editorBeatmap = new EditorBeatmap(Beatmap.Value.GetPlayableBeatmap(Ruleset.Value));
changeHandler = new BeatmapEditorChangeHandler(editorBeatmap);
Child = new DependencyProvidingContainer Child = new DependencyProvidingContainer
{ {
@ -53,6 +55,7 @@ namespace osu.Game.Tests.Visual.Editing
CachedDependencies = new (Type, object)[] CachedDependencies = new (Type, object)[]
{ {
(typeof(EditorBeatmap), editorBeatmap), (typeof(EditorBeatmap), editorBeatmap),
(typeof(IEditorChangeHandler), changeHandler),
(typeof(IBeatSnapProvider), editorBeatmap) (typeof(IBeatSnapProvider), editorBeatmap)
}, },
Child = timingScreen = new TimingScreen Child = timingScreen = new TimingScreen
@ -72,8 +75,10 @@ namespace osu.Game.Tests.Visual.Editing
AddUntilStep("Wait for rows to load", () => Child.ChildrenOfType<EffectRowAttribute>().Any()); 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] [Test]
public void TestSelectedRetainedOverUndo() public void TestSelectionDismissedOnUndo()
{ {
AddStep("Select first timing point", () => 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; return timingScreen.SelectedGroup.Value.ControlPoints.Any(c => c is TimingControlPoint) && timingScreen.SelectedGroup.Value.Time > 2170;
}); });
AddStep("simulate undo", () => AddStep("undo", () => changeHandler?.RestoreState(-1));
{
var clone = editorBeatmap.ControlPointInfo.DeepClone();
editorBeatmap.ControlPointInfo.Clear(); AddUntilStep("selection dismissed", () => timingScreen.SelectedGroup.Value, () => Is.Null);
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;
});
} }
// [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] [Test]
public void TestScrollControlGroupIntoView() public void TestScrollControlGroupIntoView()
{ {

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

@ -34,6 +34,9 @@ namespace osu.Game.Screens.Edit.Timing
[Resolved] [Resolved]
private Bindable<ControlPointGroup?> selectedGroup { get; set; } = null!; private Bindable<ControlPointGroup?> selectedGroup { get; set; } = null!;
[Resolved]
private IEditorChangeHandler? editorChangeHandler { get; set; }
[BackgroundDependencyLoader] [BackgroundDependencyLoader]
private void load(OsuColour colours, OverlayColourProvider colourProvider) 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() protected override void LoadComplete()
@ -185,5 +191,21 @@ namespace osu.Game.Screens.Edit.Timing
selectedGroup.Value = group; 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;
}
} }
} }