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

Merge pull request #10419 from peppy/fix-editor-batch-handling

Browse Source 
Increase reliability of editor beatmap change handling
This commit is contained in:
Dan Balasescu 2020-10-09 20:36:57 +09:00 committed by GitHub
commit f214cc6fb3
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 267 additions and 147 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/SliderSelectionBlueprint.cs
View File

@ -206,7 +206,7 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders
private void updatePath()
{
HitObject.Path.ExpectedDistance.Value = composer?.GetSnappedDistanceFromDistance(HitObject.StartTime, (float)HitObject.Path.CalculatedDistance) ?? (float)HitObject.Path.CalculatedDistance;
editorBeatmap?.UpdateHitObject(HitObject);
editorBeatmap?.Update(HitObject);
}
public override MenuItem[] ContextMenuItems => new MenuItem[]

10
osu.Game.Rulesets.Taiko/Edit/TaikoSelectionHandler.cs
View File

@ -54,30 +54,30 @@ namespace osu.Game.Rulesets.Taiko.Edit
{
var hits = SelectedHitObjects.OfType<Hit>();
ChangeHandler.BeginChange();
EditorBeatmap.BeginChange();
foreach (var h in hits)
{
if (h.IsStrong != state)
{
h.IsStrong = state;
EditorBeatmap.UpdateHitObject(h);
EditorBeatmap.Update(h);
}
}
ChangeHandler.EndChange();
EditorBeatmap.EndChange();
}
public void SetRimState(bool state)
{
var hits = SelectedHitObjects.OfType<Hit>();
ChangeHandler.BeginChange();
EditorBeatmap.BeginChange();
foreach (var h in hits)
h.Type = state ? HitType.Rim : HitType.Centre;
ChangeHandler.EndChange();
EditorBeatmap.EndChange();
}
protected override IEnumerable<MenuItem> GetContextMenuItemsForSelection(IEnumerable<SelectionBlueprint> selection)

100
osu.Game.Tests/Editing/TransactionalCommitComponentTest.cs Normal file
View File

@ -0,0 +1,100 @@
// 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;
using NUnit.Framework;
using osu.Game.Screens.Edit;
namespace osu.Game.Tests.Editing
{
[TestFixture]
public class TransactionalCommitComponentTest
{
private TestHandler handler;
[SetUp]
public void SetUp()
{
handler = new TestHandler();
}
[Test]
public void TestCommitTransaction()
{
Assert.That(handler.StateUpdateCount, Is.EqualTo(0));
handler.BeginChange();
Assert.That(handler.StateUpdateCount, Is.EqualTo(0));
handler.EndChange();
Assert.That(handler.StateUpdateCount, Is.EqualTo(1));
}
[Test]
public void TestSaveOutsideOfTransactionTriggersUpdates()
{
Assert.That(handler.StateUpdateCount, Is.EqualTo(0));
handler.SaveState();
Assert.That(handler.StateUpdateCount, Is.EqualTo(1));
handler.SaveState();
Assert.That(handler.StateUpdateCount, Is.EqualTo(2));
}
[Test]
public void TestEventsFire()
{
int transactionBegan = 0;
int transactionEnded = 0;
int stateSaved = 0;
handler.TransactionBegan += () => transactionBegan++;
handler.TransactionEnded += () => transactionEnded++;
handler.SaveStateTriggered += () => stateSaved++;
handler.BeginChange();
Assert.That(transactionBegan, Is.EqualTo(1));
handler.EndChange();
Assert.That(transactionEnded, Is.EqualTo(1));
Assert.That(stateSaved, Is.EqualTo(0));
handler.SaveState();
Assert.That(stateSaved, Is.EqualTo(1));
}
[Test]
public void TestSaveDuringTransactionDoesntTriggerUpdate()
{
Assert.That(handler.StateUpdateCount, Is.EqualTo(0));
handler.BeginChange();
handler.SaveState();
Assert.That(handler.StateUpdateCount, Is.EqualTo(0));
handler.EndChange();
Assert.That(handler.StateUpdateCount, Is.EqualTo(1));
}
[Test]
public void TestEndWithoutBeginThrows()
{
handler.BeginChange();
handler.EndChange();
Assert.That(() => handler.EndChange(), Throws.TypeOf<InvalidOperationException>());
}
private class TestHandler : TransactionalCommitComponent
{
public int StateUpdateCount { get; private set; }
protected override void UpdateState()
{
StateUpdateCount++;
}
}
}
}

6
osu.Game/Screens/Edit/Compose/Components/BlueprintContainer.cs
View File

@ -203,7 +203,7 @@ namespace osu.Game.Screens.Edit.Compose.Components
{
// handle positional change etc.
foreach (var obj in selectedHitObjects)
Beatmap.UpdateHitObject(obj);
Beatmap.Update(obj);
changeHandler?.EndChange();
isDraggingBlueprint = false;
@ -436,8 +436,12 @@ namespace osu.Game.Screens.Edit.Compose.Components
{
// Apply the start time at the newly snapped-to position
double offset = result.Time.Value - draggedObject.StartTime;
foreach (HitObject obj in SelectionHandler.SelectedHitObjects)
{
obj.StartTime += offset;
Beatmap.Update(obj);
}
}
return true;

16
osu.Game/Screens/Edit/Compose/Components/SelectionHandler.cs
View File

@ -245,9 +245,7 @@ namespace osu.Game.Screens.Edit.Compose.Components
private void deleteSelected()
{
ChangeHandler?.BeginChange();
EditorBeatmap?.RemoveRange(selectedBlueprints.Select(b => b.HitObject));
ChangeHandler?.EndChange();
}
#endregion
@ -314,7 +312,7 @@ namespace osu.Game.Screens.Edit.Compose.Components
/// <param name="sampleName">The name of the hit sample.</param>
public void AddHitSample(string sampleName)
{
ChangeHandler?.BeginChange();
EditorBeatmap?.BeginChange();
foreach (var h in SelectedHitObjects)
{
@ -325,7 +323,7 @@ namespace osu.Game.Screens.Edit.Compose.Components
h.Samples.Add(new HitSampleInfo { Name = sampleName });
}
ChangeHandler?.EndChange();
EditorBeatmap?.EndChange();
}
/// <summary>
@ -335,7 +333,7 @@ namespace osu.Game.Screens.Edit.Compose.Components
/// <exception cref="InvalidOperationException">Throws if any selected object doesn't implement <see cref="IHasComboInformation"/></exception>
public void SetNewCombo(bool state)
{
ChangeHandler?.BeginChange();
EditorBeatmap?.BeginChange();
foreach (var h in SelectedHitObjects)
{
@ -344,10 +342,10 @@ namespace osu.Game.Screens.Edit.Compose.Components
if (comboInfo == null || comboInfo.NewCombo == state) continue;
comboInfo.NewCombo = state;
EditorBeatmap?.UpdateHitObject(h);
EditorBeatmap?.Update(h);
}
ChangeHandler?.EndChange();
EditorBeatmap?.EndChange();
}
/// <summary>
@ -356,12 +354,12 @@ namespace osu.Game.Screens.Edit.Compose.Components
/// <param name="sampleName">The name of the hit sample.</param>
public void RemoveHitSample(string sampleName)
{
ChangeHandler?.BeginChange();
EditorBeatmap?.BeginChange();
foreach (var h in SelectedHitObjects)
h.SamplesBindable.RemoveAll(s => s.Name == sampleName);
ChangeHandler?.EndChange();
EditorBeatmap?.EndChange();
}
#endregion

4
osu.Game/Screens/Edit/Compose/Components/Timeline/TimelineHitObjectBlueprint.cs
View File

@ -392,6 +392,7 @@ namespace osu.Game.Screens.Edit.Compose.Components.Timeline
return;
repeatHitObject.RepeatCount = proposedCount;
beatmap.Update(hitObject);
break;
case IHasDuration endTimeHitObject:
@ -401,10 +402,9 @@ namespace osu.Game.Screens.Edit.Compose.Components.Timeline
return;
endTimeHitObject.Duration = snappedTime - hitObject.StartTime;
beatmap.Update(hitObject);
break;
}
beatmap.UpdateHitObject(hitObject);
}
}

4
osu.Game/Screens/Edit/Editor.cs
View File

@ -516,14 +516,14 @@ namespace osu.Game.Screens.Edit
foreach (var h in objects)
h.StartTime += timeOffset;
changeHandler.BeginChange();
editorBeatmap.BeginChange();
editorBeatmap.SelectedHitObjects.Clear();
editorBeatmap.AddRange(objects);
editorBeatmap.SelectedHitObjects.AddRange(objects);
changeHandler.EndChange();
editorBeatmap.EndChange();
}
protected void Undo() => changeHandler.RestoreState(-1);

139
osu.Game/Screens/Edit/EditorBeatmap.cs
View File

@ -8,7 +8,6 @@ using System.Linq;
using JetBrains.Annotations;
using osu.Framework.Allocation;
using osu.Framework.Bindables;
using osu.Framework.Graphics;
using osu.Game.Beatmaps;
using osu.Game.Beatmaps.ControlPoints;
using osu.Game.Beatmaps.Timing;
@ -18,7 +17,7 @@ using osu.Game.Skinning;
namespace osu.Game.Screens.Edit
{
public class EditorBeatmap : Component, IBeatmap, IBeatSnapProvider
public class EditorBeatmap : TransactionalCommitComponent, IBeatmap, IBeatSnapProvider
{
/// <summary>
/// Invoked when a <see cref="HitObject"/> is added to this <see cref="EditorBeatmap"/>.
@ -89,9 +88,11 @@ namespace osu.Game.Screens.Edit
private IList mutableHitObjects => (IList)PlayableBeatmap.HitObjects;
private readonly HashSet<HitObject> pendingUpdates = new HashSet<HitObject>();
private readonly List<HitObject> batchPendingInserts = new List<HitObject>();
private bool isBatchApplying;
private readonly List<HitObject> batchPendingDeletes = new List<HitObject>();
private readonly HashSet<HitObject> batchPendingUpdates = new HashSet<HitObject>();
/// <summary>
/// Adds a collection of <see cref="HitObject"/>s to this <see cref="EditorBeatmap"/>.
@ -99,11 +100,10 @@ namespace osu.Game.Screens.Edit
/// <param name="hitObjects">The <see cref="HitObject"/>s to add.</param>
public void AddRange(IEnumerable<HitObject> hitObjects)
{
ApplyBatchChanges(_ =>
{
foreach (var h in hitObjects)
Add(h);
});
BeginChange();
foreach (var h in hitObjects)
Add(h);
EndChange();
}
/// <summary>
@ -131,26 +131,28 @@ namespace osu.Game.Screens.Edit
mutableHitObjects.Insert(index, hitObject);
if (isBatchApplying)
batchPendingInserts.Add(hitObject);
else
{
// must be run after any change to hitobject ordering
beatmapProcessor?.PreProcess();
processHitObject(hitObject);
beatmapProcessor?.PostProcess();
HitObjectAdded?.Invoke(hitObject);
}
BeginChange();
batchPendingInserts.Add(hitObject);
EndChange();
}
/// <summary>
/// Updates a <see cref="HitObject"/>, invoking <see cref="HitObject.ApplyDefaults"/> and re-processing the beatmap.
/// </summary>
/// <param name="hitObject">The <see cref="HitObject"/> to update.</param>
public void UpdateHitObject([NotNull] HitObject hitObject)
public void Update([NotNull] HitObject hitObject)
{
pendingUpdates.Add(hitObject);
// updates are debounced regardless of whether a batch is active.
batchPendingUpdates.Add(hitObject);
}
/// <summary>
/// Update all hit objects with potentially changed difficulty or control point data.
/// </summary>
public void UpdateAllHitObjects()
{
foreach (var h in HitObjects)
batchPendingUpdates.Add(h);
}
/// <summary>
@ -175,11 +177,10 @@ namespace osu.Game.Screens.Edit
/// <param name="hitObjects">The <see cref="HitObject"/>s to remove.</param>
public void RemoveRange(IEnumerable<HitObject> hitObjects)
{
ApplyBatchChanges(_ =>
{
foreach (var h in hitObjects)
Remove(h);
});
BeginChange();
foreach (var h in hitObjects)
Remove(h);
EndChange();
}
/// <summary>
@ -203,50 +204,45 @@ namespace osu.Game.Screens.Edit
bindable.UnbindAll();
startTimeBindables.Remove(hitObject);
if (isBatchApplying)
batchPendingDeletes.Add(hitObject);
else
{
// must be run after any change to hitobject ordering
beatmapProcessor?.PreProcess();
processHitObject(hitObject);
beatmapProcessor?.PostProcess();
HitObjectRemoved?.Invoke(hitObject);
}
BeginChange();
batchPendingDeletes.Add(hitObject);
EndChange();
}
private readonly List<HitObject> batchPendingInserts = new List<HitObject>();
private readonly List<HitObject> batchPendingDeletes = new List<HitObject>();
/// <summary>
/// Apply a batch of operations in one go, without performing Pre/Postprocessing each time.
/// </summary>
/// <param name="applyFunction">The function which will apply the batch changes.</param>
public void ApplyBatchChanges(Action<EditorBeatmap> applyFunction)
protected override void Update()
{
if (isBatchApplying)
throw new InvalidOperationException("Attempting to perform a batch application from within an existing batch");
base.Update();
isBatchApplying = true;
if (batchPendingUpdates.Count > 0)
UpdateState();
}
applyFunction(this);
protected override void UpdateState()
{
if (batchPendingUpdates.Count == 0 && batchPendingDeletes.Count == 0 && batchPendingInserts.Count == 0)
return;
beatmapProcessor?.PreProcess();
foreach (var h in batchPendingDeletes) processHitObject(h);
foreach (var h in batchPendingInserts) processHitObject(h);
foreach (var h in batchPendingUpdates) processHitObject(h);
beatmapProcessor?.PostProcess();
foreach (var h in batchPendingDeletes) HitObjectRemoved?.Invoke(h);
foreach (var h in batchPendingInserts) HitObjectAdded?.Invoke(h);
// callbacks may modify the lists so let's be safe about it
var deletes = batchPendingDeletes.ToArray();
batchPendingDeletes.Clear();
var inserts = batchPendingInserts.ToArray();
batchPendingInserts.Clear();
isBatchApplying = false;
var updates = batchPendingUpdates.ToArray();
batchPendingUpdates.Clear();
foreach (var h in deletes) HitObjectRemoved?.Invoke(h);
foreach (var h in inserts) HitObjectAdded?.Invoke(h);
foreach (var h in updates) HitObjectUpdated?.Invoke(h);
}
/// <summary>
@ -254,28 +250,6 @@ namespace osu.Game.Screens.Edit
/// </summary>
public void Clear() => RemoveRange(HitObjects.ToArray());
protected override void Update()
{
base.Update();
// debounce updates as they are common and may come from input events, which can run needlessly many times per update frame.
if (pendingUpdates.Count > 0)
{
beatmapProcessor?.PreProcess();
foreach (var hitObject in pendingUpdates)
processHitObject(hitObject);
beatmapProcessor?.PostProcess();
// explicitly needs to be fired after PostProcess
foreach (var hitObject in pendingUpdates)
HitObjectUpdated?.Invoke(hitObject);
pendingUpdates.Clear();
}
}
private void processHitObject(HitObject hitObject) => hitObject.ApplyDefaults(ControlPointInfo, BeatmapInfo.BaseDifficulty);
private void trackStartTime(HitObject hitObject)
@ -289,7 +263,7 @@ namespace osu.Game.Screens.Edit
var insertionIndex = findInsertionIndex(PlayableBeatmap.HitObjects, hitObject.StartTime);
mutableHitObjects.Insert(insertionIndex + 1, hitObject);
UpdateHitObject(hitObject);
Update(hitObject);
};
}
@ -315,14 +289,5 @@ namespace osu.Game.Screens.Edit
public double GetBeatLengthAtTime(double referenceTime) => ControlPointInfo.TimingPointAt(referenceTime).BeatLength / BeatDivisor;
public int BeatDivisor => beatDivisor?.Value ?? 1;
/// <summary>
/// Update all hit objects with potentially changed difficulty or control point data.
/// </summary>
public void UpdateBeatmap()
{
foreach (var h in HitObjects)
pendingUpdates.Add(h);
}
}
}

33
osu.Game/Screens/Edit/EditorChangeHandler.cs
View File

@ -16,7 +16,7 @@ namespace osu.Game.Screens.Edit
/// <summary>
/// Tracks changes to the <see cref="Editor"/>.
/// </summary>
public class EditorChangeHandler : IEditorChangeHandler
public class EditorChangeHandler : TransactionalCommitComponent, IEditorChangeHandler
{
public readonly Bindable<bool> CanUndo = new Bindable<bool>();
public readonly Bindable<bool> CanRedo = new Bindable<bool>();
@ -41,7 +41,6 @@ namespace osu.Game.Screens.Edit
}
private readonly EditorBeatmap editorBeatmap;
private int bulkChangesStarted;
private bool isRestoring;
public const int MAX_SAVED_STATES = 50;
@ -54,9 +53,9 @@ namespace osu.Game.Screens.Edit
{
this.editorBeatmap = editorBeatmap;
editorBeatmap.HitObjectAdded += hitObjectAdded;
editorBeatmap.HitObjectRemoved += hitObjectRemoved;
editorBeatmap.HitObjectUpdated += hitObjectUpdated;
editorBeatmap.TransactionBegan += BeginChange;
editorBeatmap.TransactionEnded += EndChange;
editorBeatmap.SaveStateTriggered += SaveState;
patcher = new LegacyEditorBeatmapPatcher(editorBeatmap);
@ -64,28 +63,8 @@ namespace osu.Game.Screens.Edit
SaveState();
}
private void hitObjectAdded(HitObject obj) => SaveState();
private void hitObjectRemoved(HitObject obj) => SaveState();
private void hitObjectUpdated(HitObject obj) => SaveState();
public void BeginChange() => bulkChangesStarted++;
public void EndChange()
protected override void UpdateState()
{
if (bulkChangesStarted == 0)
throw new InvalidOperationException($"Cannot call {nameof(EndChange)} without a previous call to {nameof(BeginChange)}.");
if (--bulkChangesStarted == 0)
SaveState();
}
public void SaveState()
{
if (bulkChangesStarted > 0)
return;
if (isRestoring)
return;
@ -120,7 +99,7 @@ namespace osu.Game.Screens.Edit
/// <param name="direction">The direction to restore in. If less than 0, an older state will be used. If greater than 0, a newer state will be used.</param>
public void RestoreState(int direction)
{
if (bulkChangesStarted > 0)
if (TransactionActive)
return;
if (savedStates.Count == 0)

25
osu.Game/Screens/Edit/LegacyEditorBeatmapPatcher.cs
View File

@ -68,19 +68,20 @@ namespace osu.Game.Screens.Edit
toRemove.Sort();
toAdd.Sort();
editorBeatmap.ApplyBatchChanges(eb =>
{
// Apply the changes.
for (int i = toRemove.Count - 1; i >= 0; i--)
eb.RemoveAt(toRemove[i]);
editorBeatmap.BeginChange();
if (toAdd.Count > 0)
{
IBeatmap newBeatmap = readBeatmap(newState);
foreach (var i in toAdd)
eb.Insert(i, newBeatmap.HitObjects[i]);
}
});
// Apply the changes.
for (int i = toRemove.Count - 1; i >= 0; i--)
editorBeatmap.RemoveAt(toRemove[i]);
if (toAdd.Count > 0)
{
IBeatmap newBeatmap = readBeatmap(newState);
foreach (var i in toAdd)
editorBeatmap.Insert(i, newBeatmap.HitObjects[i]);
}
editorBeatmap.EndChange();
}
private string readString(byte[] state) => Encoding.UTF8.GetString(state);

2
osu.Game/Screens/Edit/Setup/DifficultySection.cs
View File

@ -93,7 +93,7 @@ namespace osu.Game.Screens.Edit.Setup
Beatmap.Value.BeatmapInfo.BaseDifficulty.ApproachRate = approachRateSlider.Current.Value;
Beatmap.Value.BeatmapInfo.BaseDifficulty.OverallDifficulty = overallDifficultySlider.Current.Value;
editorBeatmap.UpdateBeatmap();
editorBeatmap.UpdateAllHitObjects();
}
}
}

73
osu.Game/Screens/Edit/TransactionalCommitComponent.cs Normal file
View File

@ -0,0 +1,73 @@
// 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;
using osu.Framework.Graphics;
namespace osu.Game.Screens.Edit
{
/// <summary>
/// A component that tracks a batch change, only applying after all active changes are completed.
/// </summary>
public abstract class TransactionalCommitComponent : Component
{
/// <summary>
/// Fires whenever a transaction begins. Will not fire on nested transactions.
/// </summary>
public event Action TransactionBegan;
/// <summary>
/// Fires when the last transaction completes.
/// </summary>
public event Action TransactionEnded;
/// <summary>
/// Fires when <see cref="SaveState"/> is called and results in a non-transactional state save.
/// </summary>
public event Action SaveStateTriggered;
public bool TransactionActive => bulkChangesStarted > 0;
private int bulkChangesStarted;
/// <summary>
/// Signal the beginning of a change.
/// </summary>
public void BeginChange()
{
if (bulkChangesStarted++ == 0)
TransactionBegan?.Invoke();
}
/// <summary>
/// Signal the end of a change.
/// </summary>
/// <exception cref="InvalidOperationException">Throws if <see cref="BeginChange"/> was not first called.</exception>
public void EndChange()
{
if (bulkChangesStarted == 0)
throw new InvalidOperationException($"Cannot call {nameof(EndChange)} without a previous call to {nameof(BeginChange)}.");
if (--bulkChangesStarted == 0)
{
UpdateState();
TransactionEnded?.Invoke();
}
}
/// <summary>
/// Force an update of the state with no attached transaction.
/// This is a no-op if a transaction is already active. Should generally be used as a safety measure to ensure granular changes are not left outside a transaction.
/// </summary>
public void SaveState()
{
if (bulkChangesStarted > 0)
return;
SaveStateTriggered?.Invoke();
UpdateState();
}
protected abstract void UpdateState();
}
}