Merge pull request #10825 from smoogipoo/hitobject-pooling-in-playfield

Move hitobject pooling into Playfield

osu.Game.Tests/Visual/Gameplay/TestScenePoolingRuleset.cs

@@ -134,41 +134,44 @@ namespace osu.Game.Tests.Visual.Gameplay
             {
             }
 
-            [BackgroundDependencyLoader]
-            private void load()
-            {
-                RegisterPool<TestHitObject, DrawableTestHitObject>(PoolSize);
-            }
-
-            protected override HitObjectLifetimeEntry CreateLifetimeEntry(TestHitObject hitObject) => new TestHitObjectLifetimeEntry(hitObject);
-
             public override DrawableHitObject<TestHitObject> CreateDrawableRepresentation(TestHitObject h) => null;
 
             protected override PassThroughInputManager CreateInputManager() => new PassThroughInputManager();
 
-            protected override Playfield CreatePlayfield() => new TestPlayfield();
+            protected override Playfield CreatePlayfield() => new TestPlayfield(PoolSize);
-
-            private class TestHitObjectLifetimeEntry : HitObjectLifetimeEntry
-            {
-                public TestHitObjectLifetimeEntry(HitObject hitObject)
-                    : base(hitObject)
-                {
-                }
-
-                protected override double InitialLifetimeOffset => 0;
-            }
         }
 
         private class TestPlayfield : Playfield
         {
-            public TestPlayfield()
+            private readonly int poolSize;
+
+            public TestPlayfield(int poolSize)
             {
+                this.poolSize = poolSize;
                 AddInternal(HitObjectContainer);
             }
 
+            [BackgroundDependencyLoader]
+            private void load()
+            {
+                RegisterPool<TestHitObject, DrawableTestHitObject>(poolSize);
+            }
+
+            protected override HitObjectLifetimeEntry CreateLifetimeEntry(HitObject hitObject) => new TestHitObjectLifetimeEntry(hitObject);
+
             protected override GameplayCursorContainer CreateCursor() => null;
         }
 
+        private class TestHitObjectLifetimeEntry : HitObjectLifetimeEntry
+        {
+            public TestHitObjectLifetimeEntry(HitObject hitObject)
+                : base(hitObject)
+            {
+            }
+
+            protected override double InitialLifetimeOffset => 0;
+        }
+
         private class TestBeatmapConverter : BeatmapConverter<TestHitObject>
         {
             public TestBeatmapConverter(IBeatmap beatmap, Ruleset ruleset)

osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs

@@ -137,7 +137,7 @@ namespace osu.Game.Rulesets.Objects.Drawables
         private HitObjectLifetimeEntry lifetimeEntry;
 
         [Resolved(CanBeNull = true)]
-        private DrawableRuleset drawableRuleset { get; set; }
+        private IPooledHitObjectProvider pooledObjectProvider { get; set; }
 
         private Container<PausableSkinnableSound> samplesContainer;
 
@@ -212,7 +212,7 @@ namespace osu.Game.Rulesets.Objects.Drawables
 
             foreach (var h in HitObject.NestedHitObjects)
             {
-                var drawableNested = drawableRuleset?.GetPooledDrawableRepresentation(h)
+                var drawableNested = pooledObjectProvider?.GetPooledDrawableRepresentation(h)
                                      ?? CreateNestedHitObject(h)
                                      ?? throw new InvalidOperationException($"{nameof(CreateNestedHitObject)} returned null for {h.GetType().ReadableName()}.");
 

osu.Game/Rulesets/UI/DrawableRuleset.cs

@@ -15,10 +15,7 @@ using System.Linq;
 using System.Threading;
 using JetBrains.Annotations;
 using osu.Framework.Bindables;
-using osu.Framework.Extensions.IEnumerableExtensions;
-using osu.Framework.Extensions.TypeExtensions;
 using osu.Framework.Graphics.Cursor;
-using osu.Framework.Graphics.Pooling;
 using osu.Framework.Input;
 using osu.Framework.Input.Events;
 using osu.Game.Configuration;
@@ -248,7 +245,7 @@ namespace osu.Game.Rulesets.UI
             if (drawableRepresentation != null)
                 Playfield.Add(drawableRepresentation);
             else
-                Playfield.Add(GetLifetimeEntry(hitObject));
+                Playfield.Add(hitObject);
         }
 
         /// <summary>
@@ -260,15 +257,10 @@ namespace osu.Game.Rulesets.UI
         /// <param name="hitObject">The <see cref="HitObject"/> to remove.</param>
         public bool RemoveHitObject(TObject hitObject)
         {
-            var entry = GetLifetimeEntry(hitObject);
+            if (Playfield.Remove(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.
+            // If the entry was not removed from the playfield, assume the hitobject is not being pooled and attempt a direct drawable removal.
             var drawableObject = Playfield.AllHitObjects.SingleOrDefault(d => d.HitObject == hitObject);
             if (drawableObject != null)
                 return Playfield.Remove(drawableObject);
@@ -276,16 +268,6 @@ namespace osu.Game.Rulesets.UI
             return false;
         }
 
-        protected sealed override HitObjectLifetimeEntry CreateLifetimeEntry(HitObject hitObject)
-        {
-            if (!(hitObject is TObject tHitObject))
-                throw new InvalidOperationException($"Unexpected hitobject type: {hitObject.GetType().ReadableName()}");
-
-            return CreateLifetimeEntry(tHitObject);
-        }
-
-        protected virtual HitObjectLifetimeEntry CreateLifetimeEntry(TObject hitObject) => new HitObjectLifetimeEntry(hitObject);
-
         public override void SetRecordTarget(Replay recordingReplay)
         {
             if (!(KeyBindingInputManager is IHasRecordingHandler recordingInputManager))
@@ -327,9 +309,8 @@ namespace osu.Game.Rulesets.UI
         /// Creates a <see cref="DrawableHitObject{TObject}"/> to represent a <see cref="HitObject"/>.
         /// </summary>
         /// <remarks>
-        /// If this method returns <c>null</c>, then this <see cref="DrawableRuleset"/> will assume the requested <see cref="HitObject"/> type is being pooled,
+        /// If this method returns <c>null</c>, then this <see cref="DrawableRuleset"/> will assume the requested <see cref="HitObject"/> type is being pooled inside the <see cref="Playfield"/>,
-        /// and will instead attempt to retrieve the <see cref="DrawableHitObject"/>s at the point they should become alive via pools registered through
+        /// and will instead attempt to retrieve the <see cref="DrawableHitObject"/>s at the point they should become alive via pools registered in the <see cref="Playfield"/>.
-        /// <see cref="DrawableRuleset.RegisterPool{TObject, TDrawable}(int, int?)"/> or  <see cref="DrawableRuleset.RegisterPool{TObject, TDrawable}(DrawablePool{TDrawable})"/>.
         /// </remarks>
         /// <param name="h">The <see cref="HitObject"/> to represent.</param>
         /// <returns>The representing <see cref="DrawableHitObject{TObject}"/>.</returns>
@@ -549,99 +530,6 @@ namespace osu.Game.Rulesets.UI
         /// Invoked when the user requests to pause while the resume overlay is active.
         /// </summary>
         public abstract void CancelResume();
-
-        private readonly Dictionary<Type, IDrawablePool> pools = new Dictionary<Type, IDrawablePool>();
-        private readonly Dictionary<HitObject, HitObjectLifetimeEntry> lifetimeEntries = new Dictionary<HitObject, HitObjectLifetimeEntry>();
-
-        /// <summary>
-        /// Registers a default <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"/>).
-        /// </summary>
-        /// <param name="initialSize">The number of <see cref="DrawableHitObject"/>s to be initially stored in the pool.</param>
-        /// <param name="maximumSize">
-        /// The maximum number of <see cref="DrawableHitObject"/>s that can be stored in the pool.
-        /// If this limit is exceeded, every subsequent <see cref="DrawableHitObject"/> will be created anew instead of being retrieved from the pool,
-        /// until some of the existing <see cref="DrawableHitObject"/>s are returned to the pool.
-        /// </param>
-        /// <typeparam name="TObject">The <see cref="HitObject"/> type.</typeparam>
-        /// <typeparam name="TDrawable">The <see cref="DrawableHitObject"/> receiver for <typeparamref name="TObject"/>s.</typeparam>
-        protected void RegisterPool<TObject, TDrawable>(int initialSize, int? maximumSize = null)
-            where TObject : HitObject
-            where TDrawable : DrawableHitObject, new()
-            => RegisterPool<TObject, TDrawable>(new DrawablePool<TDrawable>(initialSize, maximumSize));
-
-        /// <summary>
-        /// Registers a custom <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"/>).
-        /// </summary>
-        /// <param name="pool">The <see cref="DrawablePool{T}"/> to register.</param>
-        /// <typeparam name="TObject">The <see cref="HitObject"/> type.</typeparam>
-        /// <typeparam name="TDrawable">The <see cref="DrawableHitObject"/> receiver for <typeparamref name="TObject"/>s.</typeparam>
-        protected void RegisterPool<TObject, TDrawable>([NotNull] DrawablePool<TDrawable> pool)
-            where TObject : HitObject
-            where TDrawable : DrawableHitObject, new()
-        {
-            pools[typeof(TObject)] = pool;
-            AddInternal(pool);
-        }
-
-        /// <summary>
-        /// Attempts to retrieve the poolable <see cref="DrawableHitObject"/> representation of a <see cref="HitObject"/>.
-        /// </summary>
-        /// <param name="hitObject">The <see cref="HitObject"/> to retrieve the <see cref="DrawableHitObject"/> representation of.</param>
-        /// <returns>The <see cref="DrawableHitObject"/> representing <see cref="HitObject"/>, or <c>null</c> if no poolable representation exists.</returns>
-        [CanBeNull]
-        public DrawableHitObject GetPooledDrawableRepresentation([NotNull] HitObject hitObject)
-        {
-            if (!pools.TryGetValue(hitObject.GetType(), out var pool))
-                return null;
-
-            return (DrawableHitObject)pool.Get(d =>
-            {
-                var dho = (DrawableHitObject)d;
-
-                // If this is the first time this DHO is being used (not loaded), then apply the DHO mods.
-                // This is done before Apply() so that the state is updated once when the hitobject is applied.
-                if (!dho.IsLoaded)
-                {
-                    foreach (var m in Mods.OfType<IApplicableToDrawableHitObjects>())
-                        m.ApplyToDrawableHitObjects(dho.Yield());
-                }
-
-                dho.Apply(hitObject, GetLifetimeEntry(hitObject));
-            });
-        }
-
-        /// <summary>
-        /// Creates the <see cref="HitObjectLifetimeEntry"/> for a given <see cref="HitObject"/>.
-        /// </summary>
-        /// <remarks>
-        /// This may be overridden to provide custom lifetime control (e.g. via <see cref="HitObjectLifetimeEntry.InitialLifetimeOffset"/>.
-        /// </remarks>
-        /// <param name="hitObject">The <see cref="HitObject"/> to create the entry for.</param>
-        /// <returns>The <see cref="HitObjectLifetimeEntry"/>.</returns>
-        [NotNull]
-        protected abstract HitObjectLifetimeEntry CreateLifetimeEntry([NotNull] HitObject hitObject);
-
-        /// <summary>
-        /// Retrieves or creates the <see cref="HitObjectLifetimeEntry"/> for a given <see cref="HitObject"/>.
-        /// </summary>
-        /// <param name="hitObject">The <see cref="HitObject"/> to retrieve or create the <see cref="HitObjectLifetimeEntry"/> for.</param>
-        /// <returns>The <see cref="HitObjectLifetimeEntry"/> for <paramref name="hitObject"/>.</returns>
-        [NotNull]
-        protected HitObjectLifetimeEntry GetLifetimeEntry([NotNull] HitObject hitObject)
-        {
-            if (lifetimeEntries.TryGetValue(hitObject, out var entry))
-                return entry;
-
-            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

osu.Game/Rulesets/UI/HitObjectContainer.cs

@@ -73,7 +73,7 @@ namespace osu.Game.Rulesets.UI
         private readonly LifetimeEntryManager lifetimeManager = new LifetimeEntryManager();
 
         [Resolved(CanBeNull = true)]
-        private DrawableRuleset drawableRuleset { get; set; }
+        private IPooledHitObjectProvider pooledObjectProvider { get; set; }
 
         public HitObjectContainer()
         {
@@ -105,7 +105,7 @@ namespace osu.Game.Rulesets.UI
         {
             Debug.Assert(!drawableMap.ContainsKey(entry));
 
-            var drawable = drawableRuleset.GetPooledDrawableRepresentation(entry.HitObject);
+            var drawable = pooledObjectProvider.GetPooledDrawableRepresentation(entry.HitObject);
             if (drawable == null)
                 throw new InvalidOperationException($"A drawable representation could not be retrieved for hitobject type: {entry.HitObject.GetType().ReadableName()}.");
 

osu.Game/Rulesets/UI/IPooledHitObjectProvider.cs

@@ -0,0 +1,20 @@
+// 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 JetBrains.Annotations;
+using osu.Game.Rulesets.Objects;
+using osu.Game.Rulesets.Objects.Drawables;
+
+namespace osu.Game.Rulesets.UI
+{
+    internal interface IPooledHitObjectProvider
+    {
+        /// <summary>
+        /// Attempts to retrieve the poolable <see cref="DrawableHitObject"/> representation of a <see cref="HitObject"/>.
+        /// </summary>
+        /// <param name="hitObject">The <see cref="HitObject"/> to retrieve the <see cref="DrawableHitObject"/> representation of.</param>
+        /// <returns>The <see cref="DrawableHitObject"/> representing <see cref="HitObject"/>, or <c>null</c> if no poolable representation exists.</returns>
+        [CanBeNull]
+        DrawableHitObject GetPooledDrawableRepresentation([NotNull] HitObject hitObject);
+    }
+}

osu.Game/Rulesets/UI/Playfield.cs

@@ -4,12 +4,14 @@
 using System;
 using System.Collections.Generic;
 using System.Linq;
+using JetBrains.Annotations;
 using osu.Framework.Graphics;
 using osu.Game.Rulesets.Objects.Drawables;
 using osu.Framework.Allocation;
 using osu.Framework.Bindables;
 using osu.Framework.Extensions.IEnumerableExtensions;
 using osu.Framework.Graphics.Containers;
+using osu.Framework.Graphics.Pooling;
 using osu.Game.Rulesets.Judgements;
 using osu.Game.Rulesets.Mods;
 using osu.Game.Rulesets.Objects;
@@ -17,7 +19,8 @@ using osuTK;
 
 namespace osu.Game.Rulesets.UI
 {
-    public abstract class Playfield : CompositeDrawable
+    [Cached(typeof(IPooledHitObjectProvider))]
+    public abstract class Playfield : CompositeDrawable, IPooledHitObjectProvider
     {
         /// <summary>
         /// Invoked when a <see cref="DrawableHitObject"/> is judged.
@@ -138,39 +141,6 @@ namespace osu.Game.Rulesets.UI
             return false;
         }
 
-        /// <summary>
-        /// 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 virtual void Add(HitObjectLifetimeEntry entry)
-        {
-            HitObjectContainer.Add(entry);
-            lifetimeEntryMap[entry.HitObject] = entry;
-            OnHitObjectAdded(entry.HitObject);
-        }
-
-        /// <summary>
-        /// 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>
-        /// <returns>Whether the <see cref="HitObject"/> was successfully removed.</returns>
-        public virtual bool Remove(HitObjectLifetimeEntry entry)
-        {
-            if (HitObjectContainer.Remove(entry))
-            {
-                lifetimeEntryMap.Remove(entry.HitObject);
-                OnHitObjectRemoved(entry.HitObject);
-                return true;
-            }
-
-            bool removedFromNested = false;
-
-            if (nestedPlayfields.IsValueCreated)
-                removedFromNested = nestedPlayfields.Value.Any(p => p.Remove(entry));
-
-            return removedFromNested;
-        }
-
         /// <summary>
         /// Invoked when a <see cref="HitObject"/> is added to this <see cref="Playfield"/>.
         /// </summary>
@@ -246,6 +216,134 @@ namespace osu.Game.Rulesets.UI
         /// </summary>
         protected virtual HitObjectContainer CreateHitObjectContainer() => new HitObjectContainer();
 
+        #region Pooling support
+
+        [Resolved(CanBeNull = true)]
+        private IPooledHitObjectProvider parentPooledObjectProvider { get; set; }
+
+        private readonly Dictionary<Type, IDrawablePool> pools = new Dictionary<Type, IDrawablePool>();
+
+        /// <summary>
+        /// Adds a <see cref="HitObjectLifetimeEntry"/> for a pooled <see cref="HitObject"/> to this <see cref="Playfield"/>.
+        /// </summary>
+        /// <param name="hitObject"></param>
+        public virtual void Add(HitObject hitObject)
+        {
+            var entry = CreateLifetimeEntry(hitObject);
+            lifetimeEntryMap[entry.HitObject] = entry;
+
+            HitObjectContainer.Add(entry);
+            OnHitObjectAdded(entry.HitObject);
+        }
+
+        /// <summary>
+        /// Removes a <see cref="HitObjectLifetimeEntry"/> for a pooled <see cref="HitObject"/> from this <see cref="Playfield"/>.
+        /// </summary>
+        /// <param name="hitObject"></param>
+        /// <returns>Whether the <see cref="HitObject"/> was successfully removed.</returns>
+        public virtual bool Remove(HitObject hitObject)
+        {
+            if (lifetimeEntryMap.Remove(hitObject, out var entry))
+            {
+                HitObjectContainer.Remove(entry);
+                OnHitObjectRemoved(hitObject);
+                return true;
+            }
+
+            bool removedFromNested = false;
+
+            if (nestedPlayfields.IsValueCreated)
+                removedFromNested = nestedPlayfields.Value.Any(p => p.Remove(hitObject));
+
+            return removedFromNested;
+        }
+
+        /// <summary>
+        /// Creates the <see cref="HitObjectLifetimeEntry"/> for a given <see cref="HitObject"/>.
+        /// </summary>
+        /// <remarks>
+        /// This may be overridden to provide custom lifetime control (e.g. via <see cref="HitObjectLifetimeEntry.InitialLifetimeOffset"/>.
+        /// </remarks>
+        /// <param name="hitObject">The <see cref="HitObject"/> to create the entry for.</param>
+        /// <returns>The <see cref="HitObjectLifetimeEntry"/>.</returns>
+        [NotNull]
+        protected virtual HitObjectLifetimeEntry CreateLifetimeEntry([NotNull] HitObject hitObject) => new HitObjectLifetimeEntry(hitObject);
+
+        /// <summary>
+        /// Registers a default <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.
+        /// </summary>
+        /// <param name="initialSize">The number of <see cref="DrawableHitObject"/>s to be initially stored in the pool.</param>
+        /// <param name="maximumSize">
+        /// The maximum number of <see cref="DrawableHitObject"/>s that can be stored in the pool.
+        /// If this limit is exceeded, every subsequent <see cref="DrawableHitObject"/> will be created anew instead of being retrieved from the pool,
+        /// until some of the existing <see cref="DrawableHitObject"/>s are returned to the pool.
+        /// </param>
+        /// <typeparam name="TObject">The <see cref="HitObject"/> type.</typeparam>
+        /// <typeparam name="TDrawable">The <see cref="DrawableHitObject"/> receiver for <typeparamref name="TObject"/>s.</typeparam>
+        protected void RegisterPool<TObject, TDrawable>(int initialSize, int? maximumSize = null)
+            where TObject : HitObject
+            where TDrawable : DrawableHitObject, new()
+            => RegisterPool<TObject, TDrawable>(new DrawablePool<TDrawable>(initialSize, maximumSize));
+
+        /// <summary>
+        /// Registers a custom <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.
+        /// </summary>
+        /// <param name="pool">The <see cref="DrawablePool{T}"/> to register.</param>
+        /// <typeparam name="TObject">The <see cref="HitObject"/> type.</typeparam>
+        /// <typeparam name="TDrawable">The <see cref="DrawableHitObject"/> receiver for <typeparamref name="TObject"/>s.</typeparam>
+        protected void RegisterPool<TObject, TDrawable>([NotNull] DrawablePool<TDrawable> pool)
+            where TObject : HitObject
+            where TDrawable : DrawableHitObject, new()
+        {
+            pools[typeof(TObject)] = pool;
+            AddInternal(pool);
+        }
+
+        DrawableHitObject IPooledHitObjectProvider.GetPooledDrawableRepresentation(HitObject hitObject)
+        {
+            var lookupType = hitObject.GetType();
+
+            IDrawablePool pool;
+
+            // Tests may add derived hitobject instances for which pools don't exist. Try to find any applicable pool and dynamically assign the type if the pool exists.
+            if (!pools.TryGetValue(lookupType, out pool))
+            {
+                foreach (var (t, p) in pools)
+                {
+                    if (!t.IsInstanceOfType(hitObject))
+                        continue;
+
+                    pools[lookupType] = pool = p;
+                    break;
+                }
+            }
+
+            if (pool == null)
+                return parentPooledObjectProvider?.GetPooledDrawableRepresentation(hitObject);
+
+            return (DrawableHitObject)pool.Get(d =>
+            {
+                var dho = (DrawableHitObject)d;
+
+                // If this is the first time this DHO is being used (not loaded), then apply the DHO mods.
+                // This is done before Apply() so that the state is updated once when the hitobject is applied.
+                if (!dho.IsLoaded)
+                {
+                    foreach (var m in mods.OfType<IApplicableToDrawableHitObjects>())
+                        m.ApplyToDrawableHitObjects(dho.Yield());
+                }
+
+                if (!lifetimeEntryMap.TryGetValue(hitObject, out var entry))
+                    lifetimeEntryMap[hitObject] = entry = CreateLifetimeEntry(hitObject);
+
+                dho.Apply(hitObject, entry);
+            });
+        }
+
+        #endregion
+
         #region Editor logic
 
         /// <summary>