Merge pull request #12486 from ekrctb/unmanaged-hit-object-entry

Associate DrawableHitObject with HitObjectLifetimeEntry rather than HitObject

osu.Game.Rulesets.Osu.Tests/TestSceneHitCircleApplication.cs

@@ -32,7 +32,7 @@ namespace osu.Game.Rulesets.Osu.Tests
             {
                 Position = new Vector2(128, 128),
                 ComboIndex = 1,
-            }), null));
+            })));
         }
 
         private HitCircle prepareObject(HitCircle circle)

osu.Game.Rulesets.Osu.Tests/TestSceneSliderApplication.cs

@@ -57,7 +57,7 @@ namespace osu.Game.Rulesets.Osu.Tests
                     new Vector2(300, 0),
                 }),
                 RepeatCount = 1
-            }), null));
+            })));
         }
 
         [Test]

osu.Game.Rulesets.Osu.Tests/TestSceneSpinnerApplication.cs

@@ -37,7 +37,7 @@ namespace osu.Game.Rulesets.Osu.Tests
                 Position = new Vector2(256, 192),
                 ComboIndex = 1,
                 Duration = 1000,
-            }), null));
+            })));
 
             AddAssert("rotation is reset", () => dho.Result.RateAdjustedRotation == 0);
         }

osu.Game.Rulesets.Taiko.Tests/TestSceneBarLineApplication.cs

@@ -18,7 +18,7 @@ namespace osu.Game.Rulesets.Taiko.Tests
             {
                 StartTime = 400,
                 Major = true
-            }), null));
+            })));
             AddHitObject(barLine);
             RemoveHitObject(barLine);
 
@@ -26,7 +26,7 @@ namespace osu.Game.Rulesets.Taiko.Tests
             {
                 StartTime = 200,
                 Major = false
-            }), null));
+            })));
             AddHitObject(barLine);
         }
     }

osu.Game.Rulesets.Taiko.Tests/TestSceneDrumRollApplication.cs

@@ -20,7 +20,7 @@ namespace osu.Game.Rulesets.Taiko.Tests
                 Duration = 500,
                 IsStrong = false,
                 TickRate = 2
-            }), null));
+            })));
 
             AddHitObject(drumRoll);
             RemoveHitObject(drumRoll);
@@ -31,7 +31,7 @@ namespace osu.Game.Rulesets.Taiko.Tests
                 Duration = 400,
                 IsStrong = true,
                 TickRate = 16
-            }), null));
+            })));
 
             AddHitObject(drumRoll);
         }

osu.Game.Rulesets.Taiko.Tests/TestSceneHitApplication.cs

@@ -19,7 +19,7 @@ namespace osu.Game.Rulesets.Taiko.Tests
                 Type = HitType.Rim,
                 IsStrong = false,
                 StartTime = 300
-            }), null));
+            })));
 
             AddHitObject(hit);
             RemoveHitObject(hit);
@@ -29,7 +29,7 @@ namespace osu.Game.Rulesets.Taiko.Tests
                 Type = HitType.Centre,
                 IsStrong = true,
                 StartTime = 500
-            }), null));
+            })));
 
             AddHitObject(hit);
         }

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

@@ -4,6 +4,7 @@
 using System;
 using System.Collections.Generic;
 using System.Collections.Specialized;
+using System.Diagnostics;
 using System.Linq;
 using JetBrains.Annotations;
 using osu.Framework.Allocation;
@@ -39,7 +40,7 @@ namespace osu.Game.Rulesets.Objects.Drawables
         /// <summary>
         /// The <see cref="HitObject"/> currently represented by this <see cref="DrawableHitObject"/>.
         /// </summary>
-        public HitObject HitObject { get; private set; }
+        public HitObject HitObject => lifetimeEntry?.HitObject;
 
         /// <summary>
         /// The parenting <see cref="DrawableHitObject"/>, if any.
@@ -108,7 +109,7 @@ namespace osu.Game.Rulesets.Objects.Drawables
         /// <summary>
         /// The scoring result of this <see cref="DrawableHitObject"/>.
         /// </summary>
-        public JudgementResult Result { get; private set; }
+        public JudgementResult Result => lifetimeEntry?.Result;
 
         /// <summary>
         /// The relative X position of this hit object for sample playback balance adjustment.
@@ -141,13 +142,14 @@ namespace osu.Game.Rulesets.Objects.Drawables
         public IBindable<ArmedState> State => state;
 
         /// <summary>
-        /// Whether <see cref="HitObject"/> is currently applied.
+        /// Whether a <see cref="HitObjectLifetimeEntry"/> is currently applied.
         /// </summary>
-        private bool hasHitObjectApplied;
+        private bool hasEntryApplied;
 
         /// <summary>
         /// The <see cref="HitObjectLifetimeEntry"/> controlling the lifetime of the currently-attached <see cref="HitObject"/>.
         /// </summary>
+        /// <remarks>Even if it is not null, it may not be fully applied until loaded (<see cref="hasEntryApplied"/> is false).</remarks>
         [CanBeNull]
         private HitObjectLifetimeEntry lifetimeEntry;
 
@@ -164,11 +166,15 @@ namespace osu.Game.Rulesets.Objects.Drawables
         /// </summary>
         /// <param name="initialHitObject">
         /// The <see cref="HitObject"/> to be initially applied to this <see cref="DrawableHitObject"/>.
-        /// If <c>null</c>, a hitobject is expected to be later applied via <see cref="Apply"/> (or automatically via pooling).
+        /// If <c>null</c>, a hitobject is expected to be later applied via <see cref="Apply(osu.Game.Rulesets.Objects.HitObjectLifetimeEntry)"/> (or automatically via pooling).
         /// </param>
         protected DrawableHitObject([CanBeNull] HitObject initialHitObject = null)
         {
-            HitObject = initialHitObject;
+            if (initialHitObject != null)
+            {
+                lifetimeEntry = new SyntheticHitObjectEntry(initialHitObject);
+                ensureEntryHasResult();
+            }
         }
 
         [BackgroundDependencyLoader]
@@ -184,8 +190,8 @@ namespace osu.Game.Rulesets.Objects.Drawables
         {
             base.LoadAsyncComplete();
 
-            if (HitObject != null)
-                Apply(HitObject, lifetimeEntry);
+            if (lifetimeEntry != null && !hasEntryApplied)
+                Apply(lifetimeEntry);
         }
 
         protected override void LoadComplete()
@@ -198,37 +204,47 @@ namespace osu.Game.Rulesets.Objects.Drawables
         }
 
         /// <summary>
-        /// Applies a new <see cref="HitObject"/> to be represented by this <see cref="DrawableHitObject"/>.
+        /// Applies a hit object to be represented by this <see cref="DrawableHitObject"/>.
         /// </summary>
-        /// <param name="hitObject">The <see cref="HitObject"/> to apply.</param>
-        /// <param name="lifetimeEntry">The <see cref="HitObjectLifetimeEntry"/> controlling the lifetime of <paramref name="hitObject"/>.</param>
+        [Obsolete("Use either overload of Apply that takes a single argument of type HitObject or HitObjectLifetimeEntry")]
         public void Apply([NotNull] HitObject hitObject, [CanBeNull] HitObjectLifetimeEntry lifetimeEntry)
+        {
+            if (lifetimeEntry != null)
+                Apply(lifetimeEntry);
+            else
+                Apply(hitObject);
+        }
+
+        /// <summary>
+        /// Applies a new <see cref="HitObject"/> to be represented by this <see cref="DrawableHitObject"/>.
+        /// A new <see cref="HitObjectLifetimeEntry"/> is automatically created and applied to this <see cref="DrawableHitObject"/>.
+        /// </summary>
+        public void Apply([NotNull] HitObject hitObject)
+        {
+            if (hitObject == null)
+                throw new ArgumentNullException($"Cannot apply a null {nameof(HitObject)}.");
+
+            Apply(new SyntheticHitObjectEntry(hitObject));
+        }
+
+        /// <summary>
+        /// Applies a new <see cref="HitObjectLifetimeEntry"/> to be represented by this <see cref="DrawableHitObject"/>.
+        /// </summary>
+        public void Apply([NotNull] HitObjectLifetimeEntry newEntry)
         {
             free();
 
-            HitObject = hitObject ?? throw new InvalidOperationException($"Cannot apply a null {nameof(HitObject)}.");
+            lifetimeEntry = newEntry;
 
-            this.lifetimeEntry = lifetimeEntry;
+            // LifetimeStart is already computed using HitObjectLifetimeEntry's InitialLifetimeOffset.
+            // We override this with DHO's InitialLifetimeOffset for a non-pooled DHO.
+            if (newEntry is SyntheticHitObjectEntry)
+                lifetimeEntry.LifetimeStart = HitObject.StartTime - InitialLifetimeOffset;
 
-            if (lifetimeEntry != null)
-            {
-                // Transfer lifetime from the entry.
-                LifetimeStart = lifetimeEntry.LifetimeStart;
-                LifetimeEnd = lifetimeEntry.LifetimeEnd;
+            LifetimeStart = lifetimeEntry.LifetimeStart;
+            LifetimeEnd = lifetimeEntry.LifetimeEnd;
 
-                // Copy any existing result from the entry (required for rewind / judgement revert).
-                Result = lifetimeEntry.Result;
-            }
-            else
-                LifetimeStart = HitObject.StartTime - InitialLifetimeOffset;
-
-            // Ensure this DHO has a result.
-            Result ??= CreateResult(HitObject.CreateJudgement())
-                       ?? throw new InvalidOperationException($"{GetType().ReadableName()} must provide a {nameof(JudgementResult)} through {nameof(CreateResult)}.");
-
-            // Copy back the result to the entry for potential future retrieval.
-            if (lifetimeEntry != null)
-                lifetimeEntry.Result = Result;
+            ensureEntryHasResult();
 
             foreach (var h in HitObject.NestedHitObjects)
             {
@@ -278,16 +294,15 @@ namespace osu.Game.Rulesets.Objects.Drawables
                     updateState(ArmedState.Idle, true);
             }
 
-            hasHitObjectApplied = true;
+            hasEntryApplied = true;
         }
 
         /// <summary>
-        /// Removes the currently applied <see cref="HitObject"/>
+        /// Removes the currently applied <see cref="lifetimeEntry"/>
         /// </summary>
         private void free()
         {
-            if (!hasHitObjectApplied)
-                return;
+            if (!hasEntryApplied) return;
 
             StartTimeBindable.UnbindFrom(HitObject.StartTimeBindable);
             if (HitObject is IHasComboInformation combo)
@@ -319,14 +334,12 @@ namespace osu.Game.Rulesets.Objects.Drawables
 
             OnFree();
 
-            HitObject = null;
             ParentHitObject = null;
-            Result = null;
             lifetimeEntry = null;
 
             clearExistingStateTransforms();
 
-            hasHitObjectApplied = false;
+            hasEntryApplied = false;
         }
 
         protected sealed override void FreeAfterUse()
@@ -385,7 +398,9 @@ namespace osu.Game.Rulesets.Objects.Drawables
 
         private void onDefaultsApplied(HitObject hitObject)
         {
-            Apply(hitObject, lifetimeEntry);
+            Debug.Assert(lifetimeEntry != null);
+            Apply(lifetimeEntry);
+
             DefaultsApplied?.Invoke(this);
         }
 
@@ -783,6 +798,13 @@ namespace osu.Game.Rulesets.Objects.Drawables
         /// <param name="judgement">The <see cref="Judgement"/> that provides the scoring information.</param>
         protected virtual JudgementResult CreateResult(Judgement judgement) => new JudgementResult(HitObject, judgement);
 
+        private void ensureEntryHasResult()
+        {
+            Debug.Assert(lifetimeEntry != null);
+            lifetimeEntry.Result ??= CreateResult(HitObject.CreateJudgement())
+                                     ?? throw new InvalidOperationException($"{GetType().ReadableName()} must provide a {nameof(JudgementResult)} through {nameof(CreateResult)}.");
+        }
+
         protected override void Dispose(bool isDisposing)
         {
             base.Dispose(isDisposing);

osu.Game/Rulesets/Objects/SyntheticHitObjectEntry.cs

@@ -0,0 +1,19 @@
+// 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 osu.Game.Rulesets.Objects.Drawables;
+
+namespace osu.Game.Rulesets.Objects
+{
+    /// <summary>
+    /// Created for a <see cref="DrawableHitObject"/> when only <see cref="HitObject"/> is given
+    /// to make sure a <see cref="DrawableHitObject"/> is always associated with a <see cref="HitObjectLifetimeEntry"/>.
+    /// </summary>
+    internal class SyntheticHitObjectEntry : HitObjectLifetimeEntry
+    {
+        public SyntheticHitObjectEntry(HitObject hitObject)
+            : base(hitObject)
+        {
+        }
+    }
+}

osu.Game/Rulesets/UI/Playfield.cs

@@ -362,7 +362,7 @@ namespace osu.Game.Rulesets.UI
                     lifetimeEntryMap[hitObject] = entry = CreateLifetimeEntry(hitObject);
 
                 dho.ParentHitObject = parent;
-                dho.Apply(hitObject, entry);
+                dho.Apply(entry);
             });
         }