From 86042e176387aaf259d1ac4a378058af74f4c6cb Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Thu, 13 May 2021 20:56:38 +0900 Subject: [PATCH 01/11] Implement HitObjectContainerEventQueue --- .../Compose/HitObjectContainerEventQueue.cs | 102 ++++++++++++++++++ 1 file changed, 102 insertions(+) create mode 100644 osu.Game/Screens/Edit/Compose/HitObjectContainerEventQueue.cs diff --git a/osu.Game/Screens/Edit/Compose/HitObjectContainerEventQueue.cs b/osu.Game/Screens/Edit/Compose/HitObjectContainerEventQueue.cs new file mode 100644 index 0000000000..b22d0a75e9 --- /dev/null +++ b/osu.Game/Screens/Edit/Compose/HitObjectContainerEventQueue.cs @@ -0,0 +1,102 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System; +using System.Collections.Generic; +using System.Diagnostics; +using System.Linq; +using osu.Framework.Graphics; +using osu.Game.Rulesets.Objects; +using osu.Game.Rulesets.Objects.Drawables; +using osu.Game.Rulesets.UI; + +namespace osu.Game.Screens.Edit.Compose +{ + /// + /// A queue which processes events from the many s in a nested hierarchy. + /// + internal class HitObjectContainerEventQueue : Component + { + /// + /// Invoked when a becomes used by a . + /// + /// + /// If the ruleset uses pooled objects, this represents the time when the s become alive. + /// + public event Action HitObjectUsageBegan; + + /// + /// Invoked when a becomes unused by a . + /// + /// + /// If the ruleset uses pooled objects, this represents the time when the s become dead. + /// + public event Action HitObjectUsageFinished; + + /// + /// Invoked when a has been transferred to another . + /// + public event Action HitObjectUsageTransferred; + + private readonly Playfield playfield; + + /// + /// Creates a new . + /// + /// The most top-level . + public HitObjectContainerEventQueue(Playfield playfield) + { + this.playfield = playfield; + + bindPlayfieldRecursive(playfield); + } + + private void bindPlayfieldRecursive(Playfield p) + { + p.HitObjectContainer.HitObjectUsageBegan += onHitObjectUsageBegan; + p.HitObjectContainer.HitObjectUsageFinished += onHitObjectUsageFinished; + + foreach (var nested in p.NestedPlayfields) + bindPlayfieldRecursive(nested); + } + + private readonly Dictionary pendingUsagesBegan = new Dictionary(); + private readonly Dictionary pendingUsagesFinished = new Dictionary(); + + private void onHitObjectUsageBegan(HitObject hitObject) => pendingUsagesBegan[hitObject] = pendingUsagesBegan.GetValueOrDefault(hitObject, 0) + 1; + + private void onHitObjectUsageFinished(HitObject hitObject) => pendingUsagesFinished[hitObject] = pendingUsagesFinished.GetValueOrDefault(hitObject, 0) + 1; + + protected override void Update() + { + base.Update(); + + foreach (var (hitObject, countBegan) in pendingUsagesBegan) + { + if (pendingUsagesFinished.TryGetValue(hitObject, out int countFinished)) + { + Debug.Assert(countFinished > 0); + + if (countBegan > countFinished) + { + // The hitobject is still in use, but transferred to a different HOC. + HitObjectUsageTransferred?.Invoke(hitObject, playfield.AllHitObjects.Single(d => d.HitObject == hitObject)); + pendingUsagesFinished.Remove(hitObject); + } + } + else + { + // This is a new usage of the hitobject. + HitObjectUsageBegan?.Invoke(hitObject, playfield.AllHitObjects.Single(d => d.HitObject == hitObject)); + } + } + + // Go through any remaining pending finished usages. + foreach (var (hitObject, _) in pendingUsagesFinished) + HitObjectUsageFinished?.Invoke(hitObject); + + pendingUsagesBegan.Clear(); + pendingUsagesFinished.Clear(); + } + } +} From aaf31af32668d4fd9560c7645c6139a3e021b9c6 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Thu, 13 May 2021 21:16:19 +0900 Subject: [PATCH 02/11] Add blueprint transferral --- .../Compose/Components/BlueprintContainer.cs | 7 +++++ .../Components/ComposeBlueprintContainer.cs | 9 +++++++ .../Components/EditorBlueprintContainer.cs | 18 +++++++------ .../Compose/HitObjectContainerEventQueue.cs | 27 ++++++++++--------- 4 files changed, 40 insertions(+), 21 deletions(-) diff --git a/osu.Game/Screens/Edit/Compose/Components/BlueprintContainer.cs b/osu.Game/Screens/Edit/Compose/Components/BlueprintContainer.cs index 361e98e0dd..951cc99c85 100644 --- a/osu.Game/Screens/Edit/Compose/Components/BlueprintContainer.cs +++ b/osu.Game/Screens/Edit/Compose/Components/BlueprintContainer.cs @@ -276,6 +276,13 @@ namespace osu.Game.Screens.Edit.Compose.Components { } + /// + /// Retrieves an item's blueprint. + /// + /// The item to retrieve the blueprint of. + /// The blueprint. + protected SelectionBlueprint GetBlueprintFor(T item) => blueprintMap[item]; + #endregion #region Selection diff --git a/osu.Game/Screens/Edit/Compose/Components/ComposeBlueprintContainer.cs b/osu.Game/Screens/Edit/Compose/Components/ComposeBlueprintContainer.cs index 95f4069edb..e231f7f648 100644 --- a/osu.Game/Screens/Edit/Compose/Components/ComposeBlueprintContainer.cs +++ b/osu.Game/Screens/Edit/Compose/Components/ComposeBlueprintContainer.cs @@ -16,6 +16,7 @@ using osu.Game.Graphics.UserInterface; using osu.Game.Rulesets.Edit; using osu.Game.Rulesets.Edit.Tools; using osu.Game.Rulesets.Objects; +using osu.Game.Rulesets.Objects.Drawables; using osu.Game.Rulesets.Objects.Types; using osu.Game.Screens.Edit.Components.TernaryButtons; using osuTK; @@ -73,6 +74,14 @@ namespace osu.Game.Screens.Edit.Compose.Components } } + protected override void TransferBlueprintFor(HitObject hitObject, DrawableHitObject drawableObject) + { + base.TransferBlueprintFor(hitObject, drawableObject); + + var blueprint = (HitObjectSelectionBlueprint)GetBlueprintFor(hitObject); + blueprint.DrawableObject = drawableObject; + } + protected override bool OnKeyDown(KeyDownEvent e) { if (e.ControlPressed) diff --git a/osu.Game/Screens/Edit/Compose/Components/EditorBlueprintContainer.cs b/osu.Game/Screens/Edit/Compose/Components/EditorBlueprintContainer.cs index db322faf65..063023c849 100644 --- a/osu.Game/Screens/Edit/Compose/Components/EditorBlueprintContainer.cs +++ b/osu.Game/Screens/Edit/Compose/Components/EditorBlueprintContainer.cs @@ -11,6 +11,7 @@ using osu.Framework.Graphics.Containers; using osu.Framework.Input.Events; using osu.Game.Rulesets.Edit; using osu.Game.Rulesets.Objects; +using osu.Game.Rulesets.Objects.Drawables; namespace osu.Game.Screens.Edit.Compose.Components { @@ -65,8 +66,11 @@ namespace osu.Game.Screens.Edit.Compose.Components foreach (var obj in Composer.HitObjects) AddBlueprintFor(obj.HitObject); - Composer.Playfield.HitObjectUsageBegan += AddBlueprintFor; - Composer.Playfield.HitObjectUsageFinished += RemoveBlueprintFor; + var eventQueue = new HitObjectContainerEventQueue(Composer.Playfield); + eventQueue.HitObjectUsageBegan += AddBlueprintFor; + eventQueue.HitObjectUsageFinished += RemoveBlueprintFor; + eventQueue.HitObjectUsageTransferred += TransferBlueprintFor; + AddInternal(eventQueue); } } @@ -100,6 +104,10 @@ namespace osu.Game.Screens.Edit.Compose.Components base.AddBlueprintFor(item); } + protected virtual void TransferBlueprintFor(HitObject hitObject, DrawableHitObject drawableObject) + { + } + protected override void DragOperationCompleted() { base.DragOperationCompleted(); @@ -152,12 +160,6 @@ namespace osu.Game.Screens.Edit.Compose.Components Beatmap.HitObjectAdded -= AddBlueprintFor; Beatmap.HitObjectRemoved -= RemoveBlueprintFor; } - - if (Composer != null) - { - Composer.Playfield.HitObjectUsageBegan -= AddBlueprintFor; - Composer.Playfield.HitObjectUsageFinished -= RemoveBlueprintFor; - } } } } diff --git a/osu.Game/Screens/Edit/Compose/HitObjectContainerEventQueue.cs b/osu.Game/Screens/Edit/Compose/HitObjectContainerEventQueue.cs index b22d0a75e9..26f5a28113 100644 --- a/osu.Game/Screens/Edit/Compose/HitObjectContainerEventQueue.cs +++ b/osu.Game/Screens/Edit/Compose/HitObjectContainerEventQueue.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Generic; using System.Diagnostics; using System.Linq; +using JetBrains.Annotations; using osu.Framework.Graphics; using osu.Game.Rulesets.Objects; using osu.Game.Rulesets.Objects.Drawables; @@ -23,7 +24,7 @@ namespace osu.Game.Screens.Edit.Compose /// /// If the ruleset uses pooled objects, this represents the time when the s become alive. /// - public event Action HitObjectUsageBegan; + public event Action HitObjectUsageBegan; /// /// Invoked when a becomes unused by a . @@ -44,20 +45,12 @@ namespace osu.Game.Screens.Edit.Compose /// Creates a new . /// /// The most top-level . - public HitObjectContainerEventQueue(Playfield playfield) + public HitObjectContainerEventQueue([NotNull] Playfield playfield) { this.playfield = playfield; - bindPlayfieldRecursive(playfield); - } - - private void bindPlayfieldRecursive(Playfield p) - { - p.HitObjectContainer.HitObjectUsageBegan += onHitObjectUsageBegan; - p.HitObjectContainer.HitObjectUsageFinished += onHitObjectUsageFinished; - - foreach (var nested in p.NestedPlayfields) - bindPlayfieldRecursive(nested); + playfield.HitObjectUsageBegan += onHitObjectUsageBegan; + playfield.HitObjectUsageFinished += onHitObjectUsageFinished; } private readonly Dictionary pendingUsagesBegan = new Dictionary(); @@ -87,7 +80,7 @@ namespace osu.Game.Screens.Edit.Compose else { // This is a new usage of the hitobject. - HitObjectUsageBegan?.Invoke(hitObject, playfield.AllHitObjects.Single(d => d.HitObject == hitObject)); + HitObjectUsageBegan?.Invoke(hitObject); } } @@ -98,5 +91,13 @@ namespace osu.Game.Screens.Edit.Compose pendingUsagesBegan.Clear(); pendingUsagesFinished.Clear(); } + + protected override void Dispose(bool isDisposing) + { + base.Dispose(isDisposing); + + playfield.HitObjectUsageBegan -= onHitObjectUsageBegan; + playfield.HitObjectUsageFinished -= onHitObjectUsageFinished; + } } } From 362a09ca73d491890698e1f84c6de2239ee6b129 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Thu, 13 May 2021 21:41:18 +0900 Subject: [PATCH 03/11] Fix up + reduce complexity of HOCEventQueue --- .../Compose/HitObjectContainerEventQueue.cs | 70 ++++++++++++------- 1 file changed, 46 insertions(+), 24 deletions(-) diff --git a/osu.Game/Screens/Edit/Compose/HitObjectContainerEventQueue.cs b/osu.Game/Screens/Edit/Compose/HitObjectContainerEventQueue.cs index 26f5a28113..7c21573b18 100644 --- a/osu.Game/Screens/Edit/Compose/HitObjectContainerEventQueue.cs +++ b/osu.Game/Screens/Edit/Compose/HitObjectContainerEventQueue.cs @@ -3,7 +3,6 @@ using System; using System.Collections.Generic; -using System.Diagnostics; using System.Linq; using JetBrains.Annotations; using osu.Framework.Graphics; @@ -53,43 +52,59 @@ namespace osu.Game.Screens.Edit.Compose playfield.HitObjectUsageFinished += onHitObjectUsageFinished; } - private readonly Dictionary pendingUsagesBegan = new Dictionary(); - private readonly Dictionary pendingUsagesFinished = new Dictionary(); + private readonly Dictionary pendingEvents = new Dictionary(); - private void onHitObjectUsageBegan(HitObject hitObject) => pendingUsagesBegan[hitObject] = pendingUsagesBegan.GetValueOrDefault(hitObject, 0) + 1; + private void onHitObjectUsageBegan(HitObject hitObject) => updateEvent(hitObject, EventType.Began); - private void onHitObjectUsageFinished(HitObject hitObject) => pendingUsagesFinished[hitObject] = pendingUsagesFinished.GetValueOrDefault(hitObject, 0) + 1; + private void onHitObjectUsageFinished(HitObject hitObject) => updateEvent(hitObject, EventType.Finished); + + private void updateEvent(HitObject hitObject, EventType newEvent) + { + if (!pendingEvents.TryGetValue(hitObject, out EventType existingEvent)) + { + pendingEvents[hitObject] = newEvent; + return; + } + + switch (existingEvent, newEvent) + { + case (EventType.Transferred, EventType.Finished): + pendingEvents[hitObject] = EventType.Finished; + break; + + case (EventType.Began, EventType.Finished): + case (EventType.Finished, EventType.Began): + pendingEvents[hitObject] = EventType.Transferred; + break; + + default: + throw new ArgumentOutOfRangeException($"Unexpected event update ({existingEvent} => {newEvent})."); + } + } protected override void Update() { base.Update(); - foreach (var (hitObject, countBegan) in pendingUsagesBegan) + foreach (var (hitObject, e) in pendingEvents) { - if (pendingUsagesFinished.TryGetValue(hitObject, out int countFinished)) + switch (e) { - Debug.Assert(countFinished > 0); + case EventType.Began: + HitObjectUsageBegan?.Invoke(hitObject); + break; - if (countBegan > countFinished) - { - // The hitobject is still in use, but transferred to a different HOC. + case EventType.Transferred: HitObjectUsageTransferred?.Invoke(hitObject, playfield.AllHitObjects.Single(d => d.HitObject == hitObject)); - pendingUsagesFinished.Remove(hitObject); - } - } - else - { - // This is a new usage of the hitobject. - HitObjectUsageBegan?.Invoke(hitObject); + break; + + case EventType.Finished: + HitObjectUsageFinished?.Invoke(hitObject); + break; } } - // Go through any remaining pending finished usages. - foreach (var (hitObject, _) in pendingUsagesFinished) - HitObjectUsageFinished?.Invoke(hitObject); - - pendingUsagesBegan.Clear(); - pendingUsagesFinished.Clear(); + pendingEvents.Clear(); } protected override void Dispose(bool isDisposing) @@ -99,5 +114,12 @@ namespace osu.Game.Screens.Edit.Compose playfield.HitObjectUsageBegan -= onHitObjectUsageBegan; playfield.HitObjectUsageFinished -= onHitObjectUsageFinished; } + + private enum EventType + { + Began, + Finished, + Transferred + } } } From ed957df162bbbfafdecaacab836ba6394c94e3a6 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 18 May 2021 16:40:56 +0900 Subject: [PATCH 04/11] Add simple xmldoc to `TransferBlueprintFor` method --- .../Edit/Compose/Components/EditorBlueprintContainer.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/osu.Game/Screens/Edit/Compose/Components/EditorBlueprintContainer.cs b/osu.Game/Screens/Edit/Compose/Components/EditorBlueprintContainer.cs index 6825e0f6a0..c62ea43331 100644 --- a/osu.Game/Screens/Edit/Compose/Components/EditorBlueprintContainer.cs +++ b/osu.Game/Screens/Edit/Compose/Components/EditorBlueprintContainer.cs @@ -84,6 +84,11 @@ namespace osu.Game.Screens.Edit.Compose.Components base.AddBlueprintFor(item); } + /// + /// Invoked when a has been transferred to another . + /// + /// The hit object which has been assigned to a new drawable. + /// The new drawable that is representing the hit object. protected virtual void TransferBlueprintFor(HitObject hitObject, DrawableHitObject drawableObject) { } From 61a41d97a4a61df9e95f47489b2ba69b23891f2d Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Tue, 18 May 2021 17:39:45 +0900 Subject: [PATCH 05/11] Add some xmldocs + comments --- .../Compose/HitObjectContainerEventQueue.cs | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/osu.Game/Screens/Edit/Compose/HitObjectContainerEventQueue.cs b/osu.Game/Screens/Edit/Compose/HitObjectContainerEventQueue.cs index 7c21573b18..363f08cb41 100644 --- a/osu.Game/Screens/Edit/Compose/HitObjectContainerEventQueue.cs +++ b/osu.Game/Screens/Edit/Compose/HitObjectContainerEventQueue.cs @@ -68,6 +68,9 @@ namespace osu.Game.Screens.Edit.Compose switch (existingEvent, newEvent) { + // This mostly exists as a safeguard to ensure that the sequence: Began -> { Finished -> Began } -> Finished, where { ... } indicates a transferral within a single frame, + // correctly leads into a final "Finished" state. It's unlikely for this to happen normally as it requires the hitobject usage to finish (for the final time) + // immediately after the HitObjectContainer updates lifetime, but it's not inconceivable to occur with the Editor's scheduling and execution order. case (EventType.Transferred, EventType.Finished): pendingEvents[hitObject] = EventType.Finished; break; @@ -117,8 +120,24 @@ namespace osu.Game.Screens.Edit.Compose private enum EventType { + /// + /// A has started being used by a . + /// Began, + + /// + /// A has finished being used by a . + /// Finished, + + /// + /// An internal intermediate state that occurs when a has finished being used by one + /// and started being used by another in the same frame. The may be the same instance in both cases. + /// + /// + /// This usually occurs when a is transferred between s, + /// but also occurs if the dies and becomes alive again in the same frame within the same . + /// Transferred } } From a31a6947bb4771472686cfd195d69747d1106c0d Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Tue, 18 May 2021 18:49:05 +0900 Subject: [PATCH 06/11] Add test --- .../TestSceneHitObjectContainerEventQueue.cs | 168 ++++++++++++++++++ osu.Game/Rulesets/UI/Playfield.cs | 7 +- 2 files changed, 173 insertions(+), 2 deletions(-) create mode 100644 osu.Game.Tests/Editing/TestSceneHitObjectContainerEventQueue.cs diff --git a/osu.Game.Tests/Editing/TestSceneHitObjectContainerEventQueue.cs b/osu.Game.Tests/Editing/TestSceneHitObjectContainerEventQueue.cs new file mode 100644 index 0000000000..3f4d1b835f --- /dev/null +++ b/osu.Game.Tests/Editing/TestSceneHitObjectContainerEventQueue.cs @@ -0,0 +1,168 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System; +using NUnit.Framework; +using osu.Framework.Graphics; +using osu.Game.Rulesets.Objects; +using osu.Game.Rulesets.Objects.Drawables; +using osu.Game.Rulesets.UI; +using osu.Game.Screens.Edit.Compose; +using osu.Game.Tests.Visual; + +namespace osu.Game.Tests.Editing +{ + public class TestSceneHitObjectContainerEventQueue : OsuTestScene + { + private readonly TestHitObject testObj = new TestHitObject(); + + private TestPlayfield playfield1; + private TestPlayfield playfield2; + private TestDrawable intermediateDrawable; + private HitObjectContainerEventQueue eventQueue; + + private HitObject beganUsage; + private HitObject finishedUsage; + private HitObject transferredUsage; + + [SetUp] + public void Setup() => Schedule(() => + { + reset(); + + if (eventQueue != null) + { + eventQueue.HitObjectUsageBegan -= onHitObjectUsageBegan; + eventQueue.HitObjectUsageFinished -= onHitObjectUsageFinished; + eventQueue.HitObjectUsageTransferred -= onHitObjectUsageTransferred; + } + + var topPlayfield = new TestPlayfield(); + topPlayfield.AddNested(playfield1 = new TestPlayfield()); + topPlayfield.AddNested(playfield2 = new TestPlayfield()); + + eventQueue = new HitObjectContainerEventQueue(topPlayfield); + eventQueue.HitObjectUsageBegan += onHitObjectUsageBegan; + eventQueue.HitObjectUsageFinished += onHitObjectUsageFinished; + eventQueue.HitObjectUsageTransferred += onHitObjectUsageTransferred; + + Children = new Drawable[] + { + topPlayfield, + intermediateDrawable = new TestDrawable(), + eventQueue + }; + }); + + private void onHitObjectUsageBegan(HitObject obj) => beganUsage = obj; + + private void onHitObjectUsageFinished(HitObject obj) => finishedUsage = obj; + + private void onHitObjectUsageTransferred(HitObject obj, DrawableHitObject drawableObj) => transferredUsage = obj; + + [Test] + public void TestUsageBeganAfterAdd() + { + AddStep("add hitobject", () => playfield1.Add(testObj)); + addCheckStep(began: true); + } + + [Test] + public void TestUsageFinishedAfterRemove() + { + AddStep("add hitobject", () => playfield1.Add(testObj)); + addResetStep(); + AddStep("remove hitobject", () => playfield1.Remove(testObj)); + addCheckStep(finished: true); + } + + [Test] + public void TestUsageTransferredWhenMovedBetweenPlayfields() + { + AddStep("add hitobject", () => playfield1.Add(testObj)); + addResetStep(); + AddStep("transfer hitobject to other playfield", () => + { + playfield1.Remove(testObj); + playfield2.Add(testObj); + }); + + addCheckStep(transferred: true); + } + + [Test] + public void TestRemoveImmediatelyAfterUsageBegan() + { + AddStep("add hitobject and schedule removal", () => + { + playfield1.Add(testObj); + intermediateDrawable.Schedule(() => playfield1.Remove(testObj)); + }); + + addCheckStep(); + } + + [Test] + public void TestRemoveImmediatelyAfterTransferred() + { + AddStep("add hitobject", () => playfield1.Add(testObj)); + addResetStep(); + AddStep("transfer hitobject to other playfield and schedule removal", () => + { + playfield1.Remove(testObj); + playfield2.Add(testObj); + intermediateDrawable.Schedule(() => playfield2.Remove(testObj)); + }); + + addCheckStep(finished: true); + } + + private void addResetStep() => AddStep("reset", reset); + + private void reset() + { + beganUsage = null; + finishedUsage = null; + transferredUsage = null; + } + + private void addCheckStep(bool began = false, bool finished = false, bool transferred = false) + => AddAssert($"began = {began}, finished = {finished}, transferred = {transferred}", + () => (beganUsage == testObj) == began && (finishedUsage == testObj) == finished && (transferredUsage == testObj) == transferred); + + private class TestPlayfield : Playfield + { + public TestPlayfield() + { + RegisterPool(1); + } + + public new void AddNested(Playfield playfield) + { + AddInternal(playfield); + base.AddNested(playfield); + } + + protected override HitObjectLifetimeEntry CreateLifetimeEntry(HitObject hitObject) + { + var entry = base.CreateLifetimeEntry(hitObject); + entry.KeepAlive = true; + return entry; + } + } + + private class TestHitObject : HitObject + { + public override string ToString() => "TestHitObject"; + } + + private class TestDrawableHitObject : DrawableHitObject + { + } + + private class TestDrawable : Drawable + { + public new void Schedule(Action action) => base.Schedule(action); + } + } +} diff --git a/osu.Game/Rulesets/UI/Playfield.cs b/osu.Game/Rulesets/UI/Playfield.cs index 17d3cf01a4..b154288dba 100644 --- a/osu.Game/Rulesets/UI/Playfield.cs +++ b/osu.Game/Rulesets/UI/Playfield.cs @@ -354,8 +354,11 @@ namespace osu.Game.Rulesets.UI // If this is the first time this DHO is being used, then apply the DHO mods. // This is done before Apply() so that the state is updated once when the hitobject is applied. - foreach (var m in mods.OfType()) - m.ApplyToDrawableHitObjects(dho.Yield()); + if (mods != null) + { + foreach (var m in mods.OfType()) + m.ApplyToDrawableHitObjects(dho.Yield()); + } } if (!lifetimeEntryMap.TryGetValue(hitObject, out var entry)) From bfc0205e9bf378039a87d6d67c05102a7aa4fed3 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Tue, 18 May 2021 18:49:11 +0900 Subject: [PATCH 07/11] Fix (began, finished) event --- .../Edit/Compose/HitObjectContainerEventQueue.cs | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/osu.Game/Screens/Edit/Compose/HitObjectContainerEventQueue.cs b/osu.Game/Screens/Edit/Compose/HitObjectContainerEventQueue.cs index 363f08cb41..6c1a3b06bb 100644 --- a/osu.Game/Screens/Edit/Compose/HitObjectContainerEventQueue.cs +++ b/osu.Game/Screens/Edit/Compose/HitObjectContainerEventQueue.cs @@ -68,14 +68,20 @@ namespace osu.Game.Screens.Edit.Compose switch (existingEvent, newEvent) { - // This mostly exists as a safeguard to ensure that the sequence: Began -> { Finished -> Began } -> Finished, where { ... } indicates a transferral within a single frame, - // correctly leads into a final "Finished" state. It's unlikely for this to happen normally as it requires the hitobject usage to finish (for the final time) - // immediately after the HitObjectContainer updates lifetime, but it's not inconceivable to occur with the Editor's scheduling and execution order. + // This exists as a safeguard to ensure that the sequence: { Began -> Finished }, where { ... } indicates a sequence within a single frame, does not trigger any events. + // This is unlikely to occur in practice as it requires the usage to finish immediately after the HitObjectContainer updates hitobject lifetimes, + // however, an Editor action scheduled somewhere between the lifetime update and this event queue's own Update() could cause this. + case (EventType.Began, EventType.Finished): + pendingEvents.Remove(hitObject); + break; + + // This exists as a safeguard to ensure that the sequence: Began -> { Finished -> Began -> Finished }, where { ... } indicates a sequence within a single frame, + // correctly leads into a final "finished" state rather than remaining in the intermediate "transferred" state. + // As above, this is unlikely to occur in practice. case (EventType.Transferred, EventType.Finished): pendingEvents[hitObject] = EventType.Finished; break; - case (EventType.Began, EventType.Finished): case (EventType.Finished, EventType.Began): pendingEvents[hitObject] = EventType.Transferred; break; From 633f841a0f691ecb89965cc4762c510e7f3cd4b8 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Tue, 18 May 2021 18:57:02 +0900 Subject: [PATCH 08/11] Rename to HitObjectUsageEventBuffer --- .../TestSceneHitObjectContainerEventQueue.cs | 20 +++++++++---------- .../Components/EditorBlueprintContainer.cs | 2 +- ...tQueue.cs => HitObjectUsageEventBuffer.cs} | 10 +++++----- 3 files changed, 16 insertions(+), 16 deletions(-) rename osu.Game/Screens/Edit/Compose/{HitObjectContainerEventQueue.cs => HitObjectUsageEventBuffer.cs} (93%) diff --git a/osu.Game.Tests/Editing/TestSceneHitObjectContainerEventQueue.cs b/osu.Game.Tests/Editing/TestSceneHitObjectContainerEventQueue.cs index 3f4d1b835f..ebf98c4c56 100644 --- a/osu.Game.Tests/Editing/TestSceneHitObjectContainerEventQueue.cs +++ b/osu.Game.Tests/Editing/TestSceneHitObjectContainerEventQueue.cs @@ -19,7 +19,7 @@ namespace osu.Game.Tests.Editing private TestPlayfield playfield1; private TestPlayfield playfield2; private TestDrawable intermediateDrawable; - private HitObjectContainerEventQueue eventQueue; + private HitObjectUsageEventBuffer eventBuffer; private HitObject beganUsage; private HitObject finishedUsage; @@ -30,27 +30,27 @@ namespace osu.Game.Tests.Editing { reset(); - if (eventQueue != null) + if (eventBuffer != null) { - eventQueue.HitObjectUsageBegan -= onHitObjectUsageBegan; - eventQueue.HitObjectUsageFinished -= onHitObjectUsageFinished; - eventQueue.HitObjectUsageTransferred -= onHitObjectUsageTransferred; + eventBuffer.HitObjectUsageBegan -= onHitObjectUsageBegan; + eventBuffer.HitObjectUsageFinished -= onHitObjectUsageFinished; + eventBuffer.HitObjectUsageTransferred -= onHitObjectUsageTransferred; } var topPlayfield = new TestPlayfield(); topPlayfield.AddNested(playfield1 = new TestPlayfield()); topPlayfield.AddNested(playfield2 = new TestPlayfield()); - eventQueue = new HitObjectContainerEventQueue(topPlayfield); - eventQueue.HitObjectUsageBegan += onHitObjectUsageBegan; - eventQueue.HitObjectUsageFinished += onHitObjectUsageFinished; - eventQueue.HitObjectUsageTransferred += onHitObjectUsageTransferred; + eventBuffer = new HitObjectUsageEventBuffer(topPlayfield); + eventBuffer.HitObjectUsageBegan += onHitObjectUsageBegan; + eventBuffer.HitObjectUsageFinished += onHitObjectUsageFinished; + eventBuffer.HitObjectUsageTransferred += onHitObjectUsageTransferred; Children = new Drawable[] { topPlayfield, intermediateDrawable = new TestDrawable(), - eventQueue + eventBuffer }; }); diff --git a/osu.Game/Screens/Edit/Compose/Components/EditorBlueprintContainer.cs b/osu.Game/Screens/Edit/Compose/Components/EditorBlueprintContainer.cs index c62ea43331..fdea26d92b 100644 --- a/osu.Game/Screens/Edit/Compose/Components/EditorBlueprintContainer.cs +++ b/osu.Game/Screens/Edit/Compose/Components/EditorBlueprintContainer.cs @@ -46,7 +46,7 @@ namespace osu.Game.Screens.Edit.Compose.Components foreach (var obj in Composer.HitObjects) AddBlueprintFor(obj.HitObject); - var eventQueue = new HitObjectContainerEventQueue(Composer.Playfield); + var eventQueue = new HitObjectUsageEventBuffer(Composer.Playfield); eventQueue.HitObjectUsageBegan += AddBlueprintFor; eventQueue.HitObjectUsageFinished += RemoveBlueprintFor; eventQueue.HitObjectUsageTransferred += TransferBlueprintFor; diff --git a/osu.Game/Screens/Edit/Compose/HitObjectContainerEventQueue.cs b/osu.Game/Screens/Edit/Compose/HitObjectUsageEventBuffer.cs similarity index 93% rename from osu.Game/Screens/Edit/Compose/HitObjectContainerEventQueue.cs rename to osu.Game/Screens/Edit/Compose/HitObjectUsageEventBuffer.cs index 6c1a3b06bb..8c69e9e707 100644 --- a/osu.Game/Screens/Edit/Compose/HitObjectContainerEventQueue.cs +++ b/osu.Game/Screens/Edit/Compose/HitObjectUsageEventBuffer.cs @@ -13,9 +13,9 @@ using osu.Game.Rulesets.UI; namespace osu.Game.Screens.Edit.Compose { /// - /// A queue which processes events from the many s in a nested hierarchy. + /// Buffers events from the many s in a nested hierarchy. /// - internal class HitObjectContainerEventQueue : Component + internal class HitObjectUsageEventBuffer : Component { /// /// Invoked when a becomes used by a . @@ -41,10 +41,10 @@ namespace osu.Game.Screens.Edit.Compose private readonly Playfield playfield; /// - /// Creates a new . + /// Creates a new . /// /// The most top-level . - public HitObjectContainerEventQueue([NotNull] Playfield playfield) + public HitObjectUsageEventBuffer([NotNull] Playfield playfield) { this.playfield = playfield; @@ -70,7 +70,7 @@ namespace osu.Game.Screens.Edit.Compose { // This exists as a safeguard to ensure that the sequence: { Began -> Finished }, where { ... } indicates a sequence within a single frame, does not trigger any events. // This is unlikely to occur in practice as it requires the usage to finish immediately after the HitObjectContainer updates hitobject lifetimes, - // however, an Editor action scheduled somewhere between the lifetime update and this event queue's own Update() could cause this. + // however, an Editor action scheduled somewhere between the lifetime update and this buffer's own Update() could cause this. case (EventType.Began, EventType.Finished): pendingEvents.Remove(hitObject); break; From 97f4f7bbd1473b7664ab9d07cc8a38382b72391a Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Tue, 18 May 2021 18:59:45 +0900 Subject: [PATCH 09/11] Remove Component inheritance --- ...TestSceneHitObjectContainerEventBuffer.cs} | 9 +++++++-- .../Components/EditorBlueprintContainer.cs | 19 ++++++++++++++----- .../Edit/Compose/HitObjectUsageEventBuffer.cs | 18 ++++++++---------- 3 files changed, 29 insertions(+), 17 deletions(-) rename osu.Game.Tests/Editing/{TestSceneHitObjectContainerEventQueue.cs => TestSceneHitObjectContainerEventBuffer.cs} (96%) diff --git a/osu.Game.Tests/Editing/TestSceneHitObjectContainerEventQueue.cs b/osu.Game.Tests/Editing/TestSceneHitObjectContainerEventBuffer.cs similarity index 96% rename from osu.Game.Tests/Editing/TestSceneHitObjectContainerEventQueue.cs rename to osu.Game.Tests/Editing/TestSceneHitObjectContainerEventBuffer.cs index ebf98c4c56..30e72150f1 100644 --- a/osu.Game.Tests/Editing/TestSceneHitObjectContainerEventQueue.cs +++ b/osu.Game.Tests/Editing/TestSceneHitObjectContainerEventBuffer.cs @@ -12,7 +12,7 @@ using osu.Game.Tests.Visual; namespace osu.Game.Tests.Editing { - public class TestSceneHitObjectContainerEventQueue : OsuTestScene + public class TestSceneHitObjectContainerEventBuffer : OsuTestScene { private readonly TestHitObject testObj = new TestHitObject(); @@ -50,7 +50,6 @@ namespace osu.Game.Tests.Editing { topPlayfield, intermediateDrawable = new TestDrawable(), - eventBuffer }; }); @@ -117,6 +116,12 @@ namespace osu.Game.Tests.Editing addCheckStep(finished: true); } + protected override void UpdateAfterChildren() + { + base.UpdateAfterChildren(); + eventBuffer.Update(); + } + private void addResetStep() => AddStep("reset", reset); private void reset() diff --git a/osu.Game/Screens/Edit/Compose/Components/EditorBlueprintContainer.cs b/osu.Game/Screens/Edit/Compose/Components/EditorBlueprintContainer.cs index fdea26d92b..5a6f98f504 100644 --- a/osu.Game/Screens/Edit/Compose/Components/EditorBlueprintContainer.cs +++ b/osu.Game/Screens/Edit/Compose/Components/EditorBlueprintContainer.cs @@ -23,6 +23,8 @@ namespace osu.Game.Screens.Edit.Compose.Components protected readonly HitObjectComposer Composer; + private HitObjectUsageEventBuffer usageEventBuffer; + protected EditorBlueprintContainer(HitObjectComposer composer) { Composer = composer; @@ -46,14 +48,19 @@ namespace osu.Game.Screens.Edit.Compose.Components foreach (var obj in Composer.HitObjects) AddBlueprintFor(obj.HitObject); - var eventQueue = new HitObjectUsageEventBuffer(Composer.Playfield); - eventQueue.HitObjectUsageBegan += AddBlueprintFor; - eventQueue.HitObjectUsageFinished += RemoveBlueprintFor; - eventQueue.HitObjectUsageTransferred += TransferBlueprintFor; - AddInternal(eventQueue); + usageEventBuffer = new HitObjectUsageEventBuffer(Composer.Playfield); + usageEventBuffer.HitObjectUsageBegan += AddBlueprintFor; + usageEventBuffer.HitObjectUsageFinished += RemoveBlueprintFor; + usageEventBuffer.HitObjectUsageTransferred += TransferBlueprintFor; } } + protected override void Update() + { + base.Update(); + usageEventBuffer?.Update(); + } + protected override IEnumerable> SortForMovement(IReadOnlyList> blueprints) => blueprints.OrderBy(b => b.Item.StartTime); @@ -145,6 +152,8 @@ namespace osu.Game.Screens.Edit.Compose.Components Beatmap.HitObjectAdded -= AddBlueprintFor; Beatmap.HitObjectRemoved -= RemoveBlueprintFor; } + + usageEventBuffer?.Dispose(); } } } diff --git a/osu.Game/Screens/Edit/Compose/HitObjectUsageEventBuffer.cs b/osu.Game/Screens/Edit/Compose/HitObjectUsageEventBuffer.cs index 8c69e9e707..cbaf9b4f26 100644 --- a/osu.Game/Screens/Edit/Compose/HitObjectUsageEventBuffer.cs +++ b/osu.Game/Screens/Edit/Compose/HitObjectUsageEventBuffer.cs @@ -5,7 +5,6 @@ using System; using System.Collections.Generic; using System.Linq; using JetBrains.Annotations; -using osu.Framework.Graphics; using osu.Game.Rulesets.Objects; using osu.Game.Rulesets.Objects.Drawables; using osu.Game.Rulesets.UI; @@ -15,7 +14,7 @@ namespace osu.Game.Screens.Edit.Compose /// /// Buffers events from the many s in a nested hierarchy. /// - internal class HitObjectUsageEventBuffer : Component + internal class HitObjectUsageEventBuffer : IDisposable { /// /// Invoked when a becomes used by a . @@ -91,10 +90,8 @@ namespace osu.Game.Screens.Edit.Compose } } - protected override void Update() + public void Update() { - base.Update(); - foreach (var (hitObject, e) in pendingEvents) { switch (e) @@ -116,12 +113,13 @@ namespace osu.Game.Screens.Edit.Compose pendingEvents.Clear(); } - protected override void Dispose(bool isDisposing) + public void Dispose() { - base.Dispose(isDisposing); - - playfield.HitObjectUsageBegan -= onHitObjectUsageBegan; - playfield.HitObjectUsageFinished -= onHitObjectUsageFinished; + if (playfield != null) + { + playfield.HitObjectUsageBegan -= onHitObjectUsageBegan; + playfield.HitObjectUsageFinished -= onHitObjectUsageFinished; + } } private enum EventType From ab6a79f84cccfe9b72f7dcb9966010366f4b0cda Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Tue, 18 May 2021 19:10:45 +0900 Subject: [PATCH 10/11] Simplify --- .../TestSceneHitObjectContainerEventBuffer.cs | 4 +- .../Edit/Compose/HitObjectUsageEventBuffer.cs | 88 +++---------------- 2 files changed, 13 insertions(+), 79 deletions(-) diff --git a/osu.Game.Tests/Editing/TestSceneHitObjectContainerEventBuffer.cs b/osu.Game.Tests/Editing/TestSceneHitObjectContainerEventBuffer.cs index 30e72150f1..5233cbc0be 100644 --- a/osu.Game.Tests/Editing/TestSceneHitObjectContainerEventBuffer.cs +++ b/osu.Game.Tests/Editing/TestSceneHitObjectContainerEventBuffer.cs @@ -98,7 +98,7 @@ namespace osu.Game.Tests.Editing intermediateDrawable.Schedule(() => playfield1.Remove(testObj)); }); - addCheckStep(); + addCheckStep(began: true, finished: true); } [Test] @@ -113,7 +113,7 @@ namespace osu.Game.Tests.Editing intermediateDrawable.Schedule(() => playfield2.Remove(testObj)); }); - addCheckStep(finished: true); + addCheckStep(transferred: true, finished: true); } protected override void UpdateAfterChildren() diff --git a/osu.Game/Screens/Edit/Compose/HitObjectUsageEventBuffer.cs b/osu.Game/Screens/Edit/Compose/HitObjectUsageEventBuffer.cs index cbaf9b4f26..fce5aa42ac 100644 --- a/osu.Game/Screens/Edit/Compose/HitObjectUsageEventBuffer.cs +++ b/osu.Game/Screens/Edit/Compose/HitObjectUsageEventBuffer.cs @@ -51,66 +51,23 @@ namespace osu.Game.Screens.Edit.Compose playfield.HitObjectUsageFinished += onHitObjectUsageFinished; } - private readonly Dictionary pendingEvents = new Dictionary(); + private readonly List usageFinishedHitObjects = new List(); - private void onHitObjectUsageBegan(HitObject hitObject) => updateEvent(hitObject, EventType.Began); - - private void onHitObjectUsageFinished(HitObject hitObject) => updateEvent(hitObject, EventType.Finished); - - private void updateEvent(HitObject hitObject, EventType newEvent) + private void onHitObjectUsageBegan(HitObject hitObject) { - if (!pendingEvents.TryGetValue(hitObject, out EventType existingEvent)) - { - pendingEvents[hitObject] = newEvent; - return; - } - - switch (existingEvent, newEvent) - { - // This exists as a safeguard to ensure that the sequence: { Began -> Finished }, where { ... } indicates a sequence within a single frame, does not trigger any events. - // This is unlikely to occur in practice as it requires the usage to finish immediately after the HitObjectContainer updates hitobject lifetimes, - // however, an Editor action scheduled somewhere between the lifetime update and this buffer's own Update() could cause this. - case (EventType.Began, EventType.Finished): - pendingEvents.Remove(hitObject); - break; - - // This exists as a safeguard to ensure that the sequence: Began -> { Finished -> Began -> Finished }, where { ... } indicates a sequence within a single frame, - // correctly leads into a final "finished" state rather than remaining in the intermediate "transferred" state. - // As above, this is unlikely to occur in practice. - case (EventType.Transferred, EventType.Finished): - pendingEvents[hitObject] = EventType.Finished; - break; - - case (EventType.Finished, EventType.Began): - pendingEvents[hitObject] = EventType.Transferred; - break; - - default: - throw new ArgumentOutOfRangeException($"Unexpected event update ({existingEvent} => {newEvent})."); - } + if (usageFinishedHitObjects.Remove(hitObject)) + HitObjectUsageTransferred?.Invoke(hitObject, playfield.AllHitObjects.Single(d => d.HitObject == hitObject)); + else + HitObjectUsageBegan?.Invoke(hitObject); } + private void onHitObjectUsageFinished(HitObject hitObject) => usageFinishedHitObjects.Add(hitObject); + public void Update() { - foreach (var (hitObject, e) in pendingEvents) - { - switch (e) - { - case EventType.Began: - HitObjectUsageBegan?.Invoke(hitObject); - break; - - case EventType.Transferred: - HitObjectUsageTransferred?.Invoke(hitObject, playfield.AllHitObjects.Single(d => d.HitObject == hitObject)); - break; - - case EventType.Finished: - HitObjectUsageFinished?.Invoke(hitObject); - break; - } - } - - pendingEvents.Clear(); + foreach (var hitObject in usageFinishedHitObjects) + HitObjectUsageFinished?.Invoke(hitObject); + usageFinishedHitObjects.Clear(); } public void Dispose() @@ -121,28 +78,5 @@ namespace osu.Game.Screens.Edit.Compose playfield.HitObjectUsageFinished -= onHitObjectUsageFinished; } } - - private enum EventType - { - /// - /// A has started being used by a . - /// - Began, - - /// - /// A has finished being used by a . - /// - Finished, - - /// - /// An internal intermediate state that occurs when a has finished being used by one - /// and started being used by another in the same frame. The may be the same instance in both cases. - /// - /// - /// This usually occurs when a is transferred between s, - /// but also occurs if the dies and becomes alive again in the same frame within the same . - /// - Transferred - } } } From d93ac7ac9842d08a0f0995779e1c179ed031870e Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Tue, 18 May 2021 19:13:13 +0900 Subject: [PATCH 11/11] Change class xmldoc a bit --- osu.Game/Screens/Edit/Compose/HitObjectUsageEventBuffer.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/osu.Game/Screens/Edit/Compose/HitObjectUsageEventBuffer.cs b/osu.Game/Screens/Edit/Compose/HitObjectUsageEventBuffer.cs index fce5aa42ac..621c901fb9 100644 --- a/osu.Game/Screens/Edit/Compose/HitObjectUsageEventBuffer.cs +++ b/osu.Game/Screens/Edit/Compose/HitObjectUsageEventBuffer.cs @@ -12,7 +12,8 @@ using osu.Game.Rulesets.UI; namespace osu.Game.Screens.Edit.Compose { /// - /// Buffers events from the many s in a nested hierarchy. + /// Buffers events from the many s in a nested hierarchy + /// to ensure correct ordering of events. /// internal class HitObjectUsageEventBuffer : IDisposable {