Merge pull request #12783 from smoogipoo/hoc-event-queue

Add a HitObjectUsageEventBuffer to handle HOC events appropriately

osu.Game.Tests/Editing/TestSceneHitObjectContainerEventBuffer.cs

@@ -0,0 +1,173 @@
+// 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.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 TestSceneHitObjectContainerEventBuffer : OsuTestScene
+    {
+        private readonly TestHitObject testObj = new TestHitObject();
+
+        private TestPlayfield playfield1;
+        private TestPlayfield playfield2;
+        private TestDrawable intermediateDrawable;
+        private HitObjectUsageEventBuffer eventBuffer;
+
+        private HitObject beganUsage;
+        private HitObject finishedUsage;
+        private HitObject transferredUsage;
+
+        [SetUp]
+        public void Setup() => Schedule(() =>
+        {
+            reset();
+
+            if (eventBuffer != null)
+            {
+                eventBuffer.HitObjectUsageBegan -= onHitObjectUsageBegan;
+                eventBuffer.HitObjectUsageFinished -= onHitObjectUsageFinished;
+                eventBuffer.HitObjectUsageTransferred -= onHitObjectUsageTransferred;
+            }
+
+            var topPlayfield = new TestPlayfield();
+            topPlayfield.AddNested(playfield1 = new TestPlayfield());
+            topPlayfield.AddNested(playfield2 = new TestPlayfield());
+
+            eventBuffer = new HitObjectUsageEventBuffer(topPlayfield);
+            eventBuffer.HitObjectUsageBegan += onHitObjectUsageBegan;
+            eventBuffer.HitObjectUsageFinished += onHitObjectUsageFinished;
+            eventBuffer.HitObjectUsageTransferred += onHitObjectUsageTransferred;
+
+            Children = new Drawable[]
+            {
+                topPlayfield,
+                intermediateDrawable = new TestDrawable(),
+            };
+        });
+
+        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(began: true, finished: true);
+        }
+
+        [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(transferred: true, finished: true);
+        }
+
+        protected override void UpdateAfterChildren()
+        {
+            base.UpdateAfterChildren();
+            eventBuffer.Update();
+        }
+
+        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<TestHitObject, TestDrawableHitObject>(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);
+        }
+    }
+}

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<IApplicableToDrawableHitObjects>())
-                        m.ApplyToDrawableHitObjects(dho.Yield());
+                    if (mods != null)
+                    {
+                        foreach (var m in mods.OfType<IApplicableToDrawableHitObjects>())
+                            m.ApplyToDrawableHitObjects(dho.Yield());
+                    }
                 }
 
                 if (!lifetimeEntryMap.TryGetValue(hitObject, out var entry))

osu.Game/Screens/Edit/Compose/Components/BlueprintContainer.cs

@@ -299,6 +299,13 @@ namespace osu.Game.Screens.Edit.Compose.Components
         {
         }
 
+        /// <summary>
+        /// Retrieves an item's blueprint.
+        /// </summary>
+        /// <param name="item">The item to retrieve the blueprint of.</param>
+        /// <returns>The blueprint.</returns>
+        protected SelectionBlueprint<T> GetBlueprintFor(T item) => blueprintMap[item];
+
         #endregion
 
         #region Selection

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)

osu.Game/Screens/Edit/Compose/Components/EditorBlueprintContainer.cs

@@ -9,6 +9,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
 {
@@ -22,6 +23,8 @@ namespace osu.Game.Screens.Edit.Compose.Components
 
         protected readonly HitObjectComposer Composer;
 
+        private HitObjectUsageEventBuffer usageEventBuffer;
+
         protected EditorBlueprintContainer(HitObjectComposer composer)
         {
             Composer = composer;
@@ -45,11 +48,19 @@ namespace osu.Game.Screens.Edit.Compose.Components
                 foreach (var obj in Composer.HitObjects)
                     AddBlueprintFor(obj.HitObject);
 
-                Composer.Playfield.HitObjectUsageBegan += AddBlueprintFor;
-                Composer.Playfield.HitObjectUsageFinished += RemoveBlueprintFor;
+                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<SelectionBlueprint<HitObject>> SortForMovement(IReadOnlyList<SelectionBlueprint<HitObject>> blueprints)
             => blueprints.OrderBy(b => b.Item.StartTime);
 
@@ -80,6 +91,15 @@ namespace osu.Game.Screens.Edit.Compose.Components
             base.AddBlueprintFor(item);
         }
 
+        /// <summary>
+        /// Invoked when a <see cref="HitObject"/> has been transferred to another <see cref="DrawableHitObject"/>.
+        /// </summary>
+        /// <param name="hitObject">The hit object which has been assigned to a new drawable.</param>
+        /// <param name="drawableObject">The new drawable that is representing the hit object.</param>
+        protected virtual void TransferBlueprintFor(HitObject hitObject, DrawableHitObject drawableObject)
+        {
+        }
+
         protected override void DragOperationCompleted()
         {
             base.DragOperationCompleted();
@@ -133,11 +153,7 @@ namespace osu.Game.Screens.Edit.Compose.Components
                 Beatmap.HitObjectRemoved -= RemoveBlueprintFor;
             }
 
-            if (Composer != null)
-            {
-                Composer.Playfield.HitObjectUsageBegan -= AddBlueprintFor;
-                Composer.Playfield.HitObjectUsageFinished -= RemoveBlueprintFor;
-            }
+            usageEventBuffer?.Dispose();
         }
     }
 }

osu.Game/Screens/Edit/Compose/HitObjectUsageEventBuffer.cs

@@ -0,0 +1,83 @@
+// 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 System.Collections.Generic;
+using System.Linq;
+using JetBrains.Annotations;
+using osu.Game.Rulesets.Objects;
+using osu.Game.Rulesets.Objects.Drawables;
+using osu.Game.Rulesets.UI;
+
+namespace osu.Game.Screens.Edit.Compose
+{
+    /// <summary>
+    /// Buffers events from the many <see cref="HitObjectContainer"/>s in a nested <see cref="Playfield"/> hierarchy
+    /// to ensure correct ordering of events.
+    /// </summary>
+    internal class HitObjectUsageEventBuffer : IDisposable
+    {
+        /// <summary>
+        /// Invoked when a <see cref="HitObject"/> becomes used by a <see cref="DrawableHitObject"/>.
+        /// </summary>
+        /// <remarks>
+        /// If the ruleset uses pooled objects, this represents the time when the <see cref="HitObject"/>s become alive.
+        /// </remarks>
+        public event Action<HitObject> HitObjectUsageBegan;
+
+        /// <summary>
+        /// Invoked when a <see cref="HitObject"/> becomes unused by a <see cref="DrawableHitObject"/>.
+        /// </summary>
+        /// <remarks>
+        /// If the ruleset uses pooled objects, this represents the time when the <see cref="HitObject"/>s become dead.
+        /// </remarks>
+        public event Action<HitObject> HitObjectUsageFinished;
+
+        /// <summary>
+        /// Invoked when a <see cref="HitObject"/> has been transferred to another <see cref="DrawableHitObject"/>.
+        /// </summary>
+        public event Action<HitObject, DrawableHitObject> HitObjectUsageTransferred;
+
+        private readonly Playfield playfield;
+
+        /// <summary>
+        /// Creates a new <see cref="HitObjectUsageEventBuffer"/>.
+        /// </summary>
+        /// <param name="playfield">The most top-level <see cref="Playfield"/>.</param>
+        public HitObjectUsageEventBuffer([NotNull] Playfield playfield)
+        {
+            this.playfield = playfield;
+
+            playfield.HitObjectUsageBegan += onHitObjectUsageBegan;
+            playfield.HitObjectUsageFinished += onHitObjectUsageFinished;
+        }
+
+        private readonly List<HitObject> usageFinishedHitObjects = new List<HitObject>();
+
+        private void onHitObjectUsageBegan(HitObject hitObject)
+        {
+            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 in usageFinishedHitObjects)
+                HitObjectUsageFinished?.Invoke(hitObject);
+            usageFinishedHitObjects.Clear();
+        }
+
+        public void Dispose()
+        {
+            if (playfield != null)
+            {
+                playfield.HitObjectUsageBegan -= onHitObjectUsageBegan;
+                playfield.HitObjectUsageFinished -= onHitObjectUsageFinished;
+            }
+        }
+    }
+}