From 5dbbe11fc668b3a54ae2be4a190479ec5aa42126 Mon Sep 17 00:00:00 2001
From: smoogipoo <smoogipoo@smgi.me>
Date: Thu, 12 Nov 2020 14:04:16 +0900
Subject: [PATCH] Remove PoolHitObjects, use return value of
 CreateDrawableRepresentation() instead

---
 .../Gameplay/TestScenePoolingRuleset.cs       |  2 -
 osu.Game/Rulesets/UI/DrawableRuleset.cs       | 49 +++++++++++--------
 osu.Game/Rulesets/UI/Playfield.cs             | 22 ++++++---
 3 files changed, 44 insertions(+), 29 deletions(-)

diff --git a/osu.Game.Tests/Visual/Gameplay/TestScenePoolingRuleset.cs b/osu.Game.Tests/Visual/Gameplay/TestScenePoolingRuleset.cs
index bb09aec416..c3ae753eae 100644
--- a/osu.Game.Tests/Visual/Gameplay/TestScenePoolingRuleset.cs
+++ b/osu.Game.Tests/Visual/Gameplay/TestScenePoolingRuleset.cs
@@ -140,8 +140,6 @@ namespace osu.Game.Tests.Visual.Gameplay
                 RegisterPool<TestHitObject, DrawableTestHitObject>(PoolSize);
             }
 
-            protected override bool PoolHitObjects => true;
-
             protected override HitObjectLifetimeEntry CreateLifetimeEntry(TestHitObject hitObject) => new TestHitObjectLifetimeEntry(hitObject);
 
             protected override PassThroughInputManager CreateInputManager() => new PassThroughInputManager();
diff --git a/osu.Game/Rulesets/UI/DrawableRuleset.cs b/osu.Game/Rulesets/UI/DrawableRuleset.cs
index 357f2d27d9..ce4cef4977 100644
--- a/osu.Game/Rulesets/UI/DrawableRuleset.cs
+++ b/osu.Game/Rulesets/UI/DrawableRuleset.cs
@@ -242,29 +242,38 @@ namespace osu.Game.Rulesets.UI
         /// <param name="hitObject">The <see cref="HitObject"/> to add.</param>
         public void AddHitObject(TObject hitObject)
         {
-            if (PoolHitObjects)
-                Playfield.Add(GetLifetimeEntry(hitObject));
+            var drawableRepresentation = CreateDrawableRepresentation(hitObject);
+
+            // If a drawable representation exists, use it, otherwise assume the hitobject is being pooled.
+            if (drawableRepresentation != null)
+                Playfield.Add(drawableRepresentation);
             else
-                Playfield.Add(CreateDrawableRepresentation(hitObject));
+                Playfield.Add(GetLifetimeEntry(hitObject));
         }
 
         /// <summary>
-        /// Removes a <see cref="HitObject"/> from this <see cref="HitObject"/>.
+        /// Removes a <see cref="HitObject"/> from this <see cref="DrawableRuleset"/>.
         /// </summary>
         /// <remarks>
         /// This does not remove the <see cref="HitObject"/> from the beatmap.
         /// </remarks>
         /// <param name="hitObject">The <see cref="HitObject"/> to remove.</param>
-        public void RemoveHitObject(TObject hitObject)
+        public bool RemoveHitObject(TObject hitObject)
         {
-            if (PoolHitObjects)
-                Playfield.Remove(GetLifetimeEntry(hitObject));
-            else
-            {
-                var drawableObject = Playfield.AllHitObjects.SingleOrDefault(d => d.HitObject == hitObject);
-                if (drawableObject != null)
-                    Playfield.Remove(drawableObject);
-            }
+            var entry = GetLifetimeEntry(hitObject);
+
+            // May have been newly-created by the above call - remove it anyway.
+            RemoveLifetimeEntry(hitObject);
+
+            if (Playfield.Remove(entry))
+                return true;
+
+            // If the entry was not removed from the playfield, assume the hitobject is not being pooled and attempt a direct removal.
+            var drawableObject = Playfield.AllHitObjects.SingleOrDefault(d => d.HitObject == hitObject);
+            if (drawableObject != null)
+                return Playfield.Remove(drawableObject);
+
+            return false;
         }
 
         protected sealed override HitObjectLifetimeEntry CreateLifetimeEntry(HitObject hitObject)
@@ -539,14 +548,6 @@ namespace osu.Game.Rulesets.UI
         private readonly Dictionary<Type, IDrawablePool> pools = new Dictionary<Type, IDrawablePool>();
         private readonly Dictionary<HitObject, HitObjectLifetimeEntry> lifetimeEntries = new Dictionary<HitObject, HitObjectLifetimeEntry>();
 
-        /// <summary>
-        /// Whether this <see cref="DrawableRuleset"/> should retrieve pooled <see cref="DrawableHitObject"/>s.
-        /// </summary>
-        /// <remarks>
-        /// Pools must be registered with this <see cref="DrawableRuleset"/> via <see cref="RegisterPool{TObject,TDrawable}"/> in order for <see cref="DrawableHitObject"/>s to be retrieved.
-        /// </remarks>
-        protected virtual bool PoolHitObjects => false;
-
         /// <summary>
         /// Registers a <see cref="DrawableHitObject"/> pool with this <see cref="DrawableRuleset"/> which is to be used whenever
         /// <see cref="DrawableHitObject"/> representations are requested for the given <typeparamref name="TObject"/> type (via <see cref="GetPooledDrawableRepresentation"/>).
@@ -627,6 +628,12 @@ namespace osu.Game.Rulesets.UI
 
             return lifetimeEntries[hitObject] = CreateLifetimeEntry(hitObject);
         }
+
+        /// <summary>
+        /// Removes the <see cref="HitObjectLifetimeEntry"/> for a <see cref="HitObject"/>.
+        /// </summary>
+        /// <param name="hitObject">The <see cref="HitObject"/> to remove the <see cref="HitObjectLifetimeEntry"/> for.</param>
+        internal void RemoveLifetimeEntry([NotNull] HitObject hitObject) => lifetimeEntries.Remove(hitObject);
     }
 
     public class BeatmapInvalidForRulesetException : ArgumentException
diff --git a/osu.Game/Rulesets/UI/Playfield.cs b/osu.Game/Rulesets/UI/Playfield.cs
index cdaf9364af..7c47f046dc 100644
--- a/osu.Game/Rulesets/UI/Playfield.cs
+++ b/osu.Game/Rulesets/UI/Playfield.cs
@@ -143,7 +143,7 @@ namespace osu.Game.Rulesets.UI
         private readonly Dictionary<HitObject, HitObjectLifetimeEntry> lifetimeEntryMap = new Dictionary<HitObject, HitObjectLifetimeEntry>();
 
         /// <summary>
-        /// Adds a <see cref="HitObject"/> to this <see cref="Playfield"/>.
+        /// Adds a <see cref="HitObjectLifetimeEntry"/> for a pooled <see cref="HitObject"/> to this <see cref="Playfield"/>.
         /// </summary>
         /// <param name="entry">The <see cref="HitObjectLifetimeEntry"/> controlling the lifetime of the <see cref="HitObject"/>.</param>
         public void Add(HitObjectLifetimeEntry entry)
@@ -154,14 +154,24 @@ namespace osu.Game.Rulesets.UI
         }
 
         /// <summary>
-        /// Removes a <see cref="HitObject"/> to this <see cref="Playfield"/>.
+        /// Removes a <see cref="HitObjectLifetimeEntry"/> for a pooled <see cref="HitObject"/> from this <see cref="Playfield"/>.
         /// </summary>
         /// <param name="entry">The <see cref="HitObjectLifetimeEntry"/> controlling the lifetime of the <see cref="HitObject"/>.</param>
-        public void Remove(HitObjectLifetimeEntry entry)
+        /// <returns>Whether the <see cref="HitObject"/> was successfully removed.</returns>
+        public bool Remove(HitObjectLifetimeEntry entry)
         {
-            if (HitObjectContainer.Remove(entry))
-                OnHitObjectRemoved(entry.HitObject);
-            lifetimeEntryMap.Remove(entry.HitObject);
+            if (lifetimeEntryMap.Remove(entry.HitObject))
+            {
+                HitObjectContainer.Remove(entry);
+                return true;
+            }
+
+            bool removedFromNested = false;
+
+            if (nestedPlayfields.IsValueCreated)
+                removedFromNested = nestedPlayfields.Value.Any(p => p.Remove(entry));
+
+            return removedFromNested;
         }
 
         /// <summary>