From bbf2ec369b748dcb424f370178ec5402a5c46493 Mon Sep 17 00:00:00 2001 From: ekrctb Date: Tue, 20 Apr 2021 17:13:59 +0900 Subject: [PATCH 1/5] Remove SkinReloadableDrawable inheritance from DHO --- .../Objects/Drawables/DrawableHitObject.cs | 24 +++++++++++++------ 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs b/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs index ba2b8423d0..1369623a62 100644 --- a/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs +++ b/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs @@ -11,6 +11,7 @@ using osu.Framework.Allocation; using osu.Framework.Bindables; using osu.Framework.Extensions.TypeExtensions; using osu.Framework.Graphics; +using osu.Framework.Graphics.Pooling; using osu.Framework.Graphics.Primitives; using osu.Framework.Threading; using osu.Game.Audio; @@ -25,7 +26,7 @@ using osuTK.Graphics; namespace osu.Game.Rulesets.Objects.Drawables { [Cached(typeof(DrawableHitObject))] - public abstract class DrawableHitObject : SkinReloadableDrawable + public abstract class DrawableHitObject : PoolableDrawable { /// /// Invoked after this 's applied has had its defaults applied. @@ -178,12 +179,15 @@ namespace osu.Game.Rulesets.Objects.Drawables } [BackgroundDependencyLoader] - private void load(OsuConfigManager config) + private void load(OsuConfigManager config, ISkinSource skinSource) { config.BindWith(OsuSetting.PositionalHitSounds, userPositionalHitSounds); // Explicit non-virtual function call. base.AddInternal(Samples = new PausableSkinnableSound()); + + CurrentSkin = skinSource; + CurrentSkin.SourceChanged += onSkinSourceChanged; } protected override void LoadAsyncComplete() @@ -536,17 +540,19 @@ namespace osu.Game.Rulesets.Objects.Drawables #endregion - protected sealed override void SkinChanged(ISkinSource skin, bool allowFallback) - { - base.SkinChanged(skin, allowFallback); + #region Skinning + protected ISkinSource CurrentSkin { get; private set; } + + private void onSkinSourceChanged() => Scheduler.AddOnce(() => + { UpdateComboColour(); - ApplySkin(skin, allowFallback); + ApplySkin(CurrentSkin, true); if (IsLoaded) updateState(State.Value, true); - } + }); protected void UpdateComboColour() { @@ -616,6 +622,8 @@ namespace osu.Game.Rulesets.Objects.Drawables Samples.Stop(); } + #endregion + protected override void Update() { base.Update(); @@ -811,6 +819,8 @@ namespace osu.Game.Rulesets.Objects.Drawables if (HitObject != null) HitObject.DefaultsApplied -= onDefaultsApplied; + + CurrentSkin.SourceChanged -= onSkinSourceChanged; } } From b877a2973739c4cb8a32393bbb021392f536be82 Mon Sep 17 00:00:00 2001 From: ekrctb Date: Tue, 20 Apr 2021 17:55:01 +0900 Subject: [PATCH 2/5] Factor out pooling and lifetime management logic of DHO to a base class --- .../Objects/Drawables/DrawableHitObject.cs | 116 +++-------------- .../Objects/Pooling/DrawableObject.cs | 121 ++++++++++++++++++ 2 files changed, 141 insertions(+), 96 deletions(-) create mode 100644 osu.Game/Rulesets/Objects/Pooling/DrawableObject.cs diff --git a/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs b/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs index 1369623a62..312ed93e45 100644 --- a/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs +++ b/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs @@ -11,7 +11,6 @@ using osu.Framework.Allocation; using osu.Framework.Bindables; using osu.Framework.Extensions.TypeExtensions; using osu.Framework.Graphics; -using osu.Framework.Graphics.Pooling; using osu.Framework.Graphics.Primitives; using osu.Framework.Threading; using osu.Game.Audio; @@ -20,13 +19,14 @@ using osu.Game.Rulesets.Objects.Types; using osu.Game.Rulesets.Scoring; using osu.Game.Skinning; using osu.Game.Configuration; +using osu.Game.Rulesets.Objects.Pooling; using osu.Game.Rulesets.UI; using osuTK.Graphics; namespace osu.Game.Rulesets.Objects.Drawables { [Cached(typeof(DrawableHitObject))] - public abstract class DrawableHitObject : PoolableDrawable + public abstract class DrawableHitObject : DrawableObject { /// /// Invoked after this 's applied has had its defaults applied. @@ -41,7 +41,7 @@ namespace osu.Game.Rulesets.Objects.Drawables /// /// The currently represented by this . /// - public HitObject HitObject => lifetimeEntry?.HitObject; + public HitObject HitObject => Entry?.HitObject; /// /// The parenting , if any. @@ -110,7 +110,7 @@ namespace osu.Game.Rulesets.Objects.Drawables /// /// The scoring result of this . /// - public JudgementResult Result => lifetimeEntry?.Result; + public JudgementResult Result => Entry?.Result; /// /// The relative X position of this hit object for sample playback balance adjustment. @@ -126,8 +126,6 @@ namespace osu.Game.Rulesets.Objects.Drawables private readonly Bindable userPositionalHitSounds = new Bindable(); private readonly Bindable comboIndexBindable = new Bindable(); - public override bool RemoveWhenNotAlive => false; - public override bool RemoveCompletedTransforms => false; protected override bool RequiresChildrenUpdate => true; public override bool IsPresent => base.IsPresent || (State.Value == ArmedState.Idle && Clock?.CurrentTime >= LifetimeStart); @@ -142,18 +140,6 @@ namespace osu.Game.Rulesets.Objects.Drawables /// public IBindable State => state; - /// - /// Whether a is currently applied. - /// - private bool hasEntryApplied; - - /// - /// The controlling the lifetime of the currently-attached . - /// - /// Even if it is not null, it may not be fully applied until loaded ( is false). - [CanBeNull] - private HitObjectLifetimeEntry lifetimeEntry; - [Resolved(CanBeNull = true)] private IPooledHitObjectProvider pooledObjectProvider { get; set; } @@ -167,15 +153,13 @@ namespace osu.Game.Rulesets.Objects.Drawables /// /// /// The to be initially applied to this . - /// If null, a hitobject is expected to be later applied via (or automatically via pooling). + /// If null, a hitobject is expected to be later applied via (or automatically via pooling). /// protected DrawableHitObject([CanBeNull] HitObject initialHitObject = null) + : base(initialHitObject != null ? new SyntheticHitObjectEntry(initialHitObject) : null) { - if (initialHitObject != null) - { - lifetimeEntry = new SyntheticHitObjectEntry(initialHitObject); + if (Entry != null) ensureEntryHasResult(); - } } [BackgroundDependencyLoader] @@ -190,14 +174,6 @@ namespace osu.Game.Rulesets.Objects.Drawables CurrentSkin.SourceChanged += onSkinSourceChanged; } - protected override void LoadAsyncComplete() - { - base.LoadAsyncComplete(); - - if (lifetimeEntry != null && !hasEntryApplied) - Apply(lifetimeEntry); - } - protected override void LoadComplete() { base.LoadComplete(); @@ -231,22 +207,15 @@ namespace osu.Game.Rulesets.Objects.Drawables Apply(new SyntheticHitObjectEntry(hitObject)); } - /// - /// Applies a new to be represented by this . - /// - public void Apply([NotNull] HitObjectLifetimeEntry newEntry) + protected sealed override void OnApply(HitObjectLifetimeEntry entry) { - free(); - - lifetimeEntry = newEntry; - // 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 (entry is SyntheticHitObjectEntry) + entry.LifetimeStart = HitObject.StartTime - InitialLifetimeOffset; - LifetimeStart = lifetimeEntry.LifetimeStart; - LifetimeEnd = lifetimeEntry.LifetimeEnd; + LifetimeStart = entry.LifetimeStart; + LifetimeEnd = entry.LifetimeEnd; ensureEntryHasResult(); @@ -297,17 +266,10 @@ namespace osu.Game.Rulesets.Objects.Drawables else updateState(ArmedState.Idle, true); } - - hasEntryApplied = true; } - /// - /// Removes the currently applied - /// - private void free() + protected sealed override void OnFree(HitObjectLifetimeEntry entry) { - if (!hasEntryApplied) return; - StartTimeBindable.UnbindFrom(HitObject.StartTimeBindable); if (HitObject is IHasComboInformation combo) comboIndexBindable.UnbindFrom(combo.ComboIndexBindable); @@ -339,22 +301,8 @@ namespace osu.Game.Rulesets.Objects.Drawables OnFree(); ParentHitObject = null; - lifetimeEntry = null; clearExistingStateTransforms(); - - hasEntryApplied = false; - } - - protected sealed override void FreeAfterUse() - { - base.FreeAfterUse(); - - // Freeing while not in a pool would cause the DHO to not be usable elsewhere in the hierarchy without being re-applied. - if (!IsInPool) - return; - - free(); } /// @@ -402,8 +350,8 @@ namespace osu.Game.Rulesets.Objects.Drawables private void onDefaultsApplied(HitObject hitObject) { - Debug.Assert(lifetimeEntry != null); - Apply(lifetimeEntry); + Debug.Assert(Entry != null); + Apply(Entry); DefaultsApplied?.Invoke(this); } @@ -486,7 +434,7 @@ namespace osu.Game.Rulesets.Objects.Drawables /// /// Apply (generally fade-in) transforms leading into the start time. - /// The local drawable hierarchy is recursively delayed to for convenience. + /// The local drawable hierarchy is recursively delayed to for convenience. /// /// By default this will fade in the object from zero with no duration. /// @@ -661,30 +609,6 @@ namespace osu.Game.Rulesets.Objects.Drawables /// protected internal new ScheduledDelegate Schedule(Action action) => base.Schedule(action); - public override double LifetimeStart - { - get => base.LifetimeStart; - set => setLifetime(value, LifetimeEnd); - } - - public override double LifetimeEnd - { - get => base.LifetimeEnd; - set => setLifetime(LifetimeStart, value); - } - - private void setLifetime(double lifetimeStart, double lifetimeEnd) - { - base.LifetimeStart = lifetimeStart; - base.LifetimeEnd = lifetimeEnd; - - if (lifetimeEntry != null) - { - lifetimeEntry.LifetimeStart = lifetimeStart; - lifetimeEntry.LifetimeEnd = lifetimeEnd; - } - } - /// /// A safe offset prior to the start time of at which this may begin displaying contents. /// By default, s are assumed to display their contents within 10 seconds prior to the start time of . @@ -692,7 +616,7 @@ namespace osu.Game.Rulesets.Objects.Drawables /// /// This is only used as an optimisation to delay the initial update of this and may be tuned more aggressively if required. /// It is indirectly used to decide the automatic transform offset provided to . - /// A more accurate should be set for further optimisation (in , for example). + /// A more accurate should be set for further optimisation (in , for example). /// /// Only has an effect if this is not being pooled. /// For pooled s, use instead. @@ -808,9 +732,9 @@ namespace osu.Game.Rulesets.Objects.Drawables 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)}."); + Debug.Assert(Entry != null); + Entry.Result ??= CreateResult(HitObject.CreateJudgement()) + ?? throw new InvalidOperationException($"{GetType().ReadableName()} must provide a {nameof(JudgementResult)} through {nameof(CreateResult)}."); } protected override void Dispose(bool isDisposing) diff --git a/osu.Game/Rulesets/Objects/Pooling/DrawableObject.cs b/osu.Game/Rulesets/Objects/Pooling/DrawableObject.cs new file mode 100644 index 0000000000..b29e6a6c3c --- /dev/null +++ b/osu.Game/Rulesets/Objects/Pooling/DrawableObject.cs @@ -0,0 +1,121 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +#nullable enable + +using System.Diagnostics; +using osu.Framework.Graphics.Performance; +using osu.Framework.Graphics.Pooling; + +namespace osu.Game.Rulesets.Objects.Pooling +{ + /// + /// A that is controlled by to implement drawable pooling and replay rewinding. + /// + /// The type storing state and controlling this drawable. + public abstract class DrawableObject : PoolableDrawable where TEntry : LifetimeEntry + { + /// + /// The entry holding essential state of this . + /// + protected TEntry? Entry { get; private set; } + + /// + /// Whether is applied to this . + /// When an initial entry is specified in the constructor, is set but not applied until loading is completed. + /// + protected bool HasEntryApplied { get; private set; } + + public override double LifetimeStart + { + get => base.LifetimeStart; + set => setLifetime(value, LifetimeEnd); + } + + public override double LifetimeEnd + { + get => base.LifetimeEnd; + set => setLifetime(LifetimeStart, value); + } + + public override bool RemoveWhenNotAlive => false; + public override bool RemoveCompletedTransforms => false; + + protected DrawableObject(TEntry? initialEntry = null) + { + Entry = initialEntry; + } + + protected override void LoadAsyncComplete() + { + base.LoadAsyncComplete(); + + if (Entry != null && !HasEntryApplied) + Apply(Entry); + } + + /// + /// Applies a new entry to be represented by this drawable. + /// If there is an existing entry applied, the entry will be replaced. + /// + public void Apply(TEntry entry) + { + freeIfInUse(); + + setLifetime(entry.LifetimeStart, entry.LifetimeEnd); + Entry = entry; + + OnApply(entry); + + HasEntryApplied = true; + } + + protected sealed override void FreeAfterUse() + { + base.FreeAfterUse(); + + if (IsInPool) + freeIfInUse(); + } + + /// + /// Invoked to apply a new entry to this drawable. + /// + protected virtual void OnApply(TEntry entry) + { + } + + /// + /// Invoked to revert application of the entry to this drawable. + /// + protected virtual void OnFree(TEntry entry) + { + } + + private void setLifetime(double start, double end) + { + base.LifetimeStart = start; + base.LifetimeEnd = end; + + if (Entry != null) + { + Entry.LifetimeStart = start; + Entry.LifetimeEnd = end; + } + } + + private void freeIfInUse() + { + if (!HasEntryApplied) return; + + Debug.Assert(Entry != null); + + OnFree(Entry); + + Entry = null; + setLifetime(double.MaxValue, double.MaxValue); + + HasEntryApplied = false; + } + } +} From e6474e6ff73b968970dc0c9e943130a6f865af06 Mon Sep 17 00:00:00 2001 From: ekrctb Date: Mon, 26 Apr 2021 11:47:38 +0900 Subject: [PATCH 3/5] Remove redundant statement (lifetime is set in base) --- osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs b/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs index 312ed93e45..6ab6a4b984 100644 --- a/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs +++ b/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs @@ -212,10 +212,7 @@ namespace osu.Game.Rulesets.Objects.Drawables // LifetimeStart is already computed using HitObjectLifetimeEntry's InitialLifetimeOffset. // We override this with DHO's InitialLifetimeOffset for a non-pooled DHO. if (entry is SyntheticHitObjectEntry) - entry.LifetimeStart = HitObject.StartTime - InitialLifetimeOffset; - - LifetimeStart = entry.LifetimeStart; - LifetimeEnd = entry.LifetimeEnd; + LifetimeStart = HitObject.StartTime - InitialLifetimeOffset; ensureEntryHasResult(); From 20e3cadd30e6c2e9e49b0f880915646e7caa9624 Mon Sep 17 00:00:00 2001 From: ekrctb Date: Mon, 26 Apr 2021 12:04:59 +0900 Subject: [PATCH 4/5] freeIfInUse -> free, and add comments --- .../Rulesets/Objects/Pooling/DrawableObject.cs | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/osu.Game/Rulesets/Objects/Pooling/DrawableObject.cs b/osu.Game/Rulesets/Objects/Pooling/DrawableObject.cs index b29e6a6c3c..27ed4c04f2 100644 --- a/osu.Game/Rulesets/Objects/Pooling/DrawableObject.cs +++ b/osu.Game/Rulesets/Objects/Pooling/DrawableObject.cs @@ -50,6 +50,7 @@ namespace osu.Game.Rulesets.Objects.Pooling { base.LoadAsyncComplete(); + // Apply the initial entry given in the constructor. if (Entry != null && !HasEntryApplied) Apply(Entry); } @@ -60,7 +61,8 @@ namespace osu.Game.Rulesets.Objects.Pooling /// public void Apply(TEntry entry) { - freeIfInUse(); + if (HasEntryApplied) + free(); setLifetime(entry.LifetimeStart, entry.LifetimeEnd); Entry = entry; @@ -74,8 +76,9 @@ namespace osu.Game.Rulesets.Objects.Pooling { base.FreeAfterUse(); - if (IsInPool) - freeIfInUse(); + // We preserve the existing entry in case we want to move a non-pooled drawable between different parent drawables. + if (HasEntryApplied && IsInPool) + free(); } /// @@ -104,11 +107,9 @@ namespace osu.Game.Rulesets.Objects.Pooling } } - private void freeIfInUse() + private void free() { - if (!HasEntryApplied) return; - - Debug.Assert(Entry != null); + Debug.Assert(Entry != null && HasEntryApplied); OnFree(Entry); From 6561a7c7d697dafd15d3f35b82d7ccd86552ca0b Mon Sep 17 00:00:00 2001 From: ekrctb Date: Mon, 26 Apr 2021 12:06:21 +0900 Subject: [PATCH 5/5] Rename DrawableObject -> PoolableDrawableWithLifetime --- osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs | 4 ++-- ...{DrawableObject.cs => PoolableDrawableWithLifetime.cs} | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) rename osu.Game/Rulesets/Objects/Pooling/{DrawableObject.cs => PoolableDrawableWithLifetime.cs} (92%) diff --git a/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs b/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs index 6ab6a4b984..7739994527 100644 --- a/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs +++ b/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs @@ -26,7 +26,7 @@ using osuTK.Graphics; namespace osu.Game.Rulesets.Objects.Drawables { [Cached(typeof(DrawableHitObject))] - public abstract class DrawableHitObject : DrawableObject + public abstract class DrawableHitObject : PoolableDrawableWithLifetime { /// /// Invoked after this 's applied has had its defaults applied. @@ -153,7 +153,7 @@ namespace osu.Game.Rulesets.Objects.Drawables /// /// /// The to be initially applied to this . - /// If null, a hitobject is expected to be later applied via (or automatically via pooling). + /// If null, a hitobject is expected to be later applied via (or automatically via pooling). /// protected DrawableHitObject([CanBeNull] HitObject initialHitObject = null) : base(initialHitObject != null ? new SyntheticHitObjectEntry(initialHitObject) : null) diff --git a/osu.Game/Rulesets/Objects/Pooling/DrawableObject.cs b/osu.Game/Rulesets/Objects/Pooling/PoolableDrawableWithLifetime.cs similarity index 92% rename from osu.Game/Rulesets/Objects/Pooling/DrawableObject.cs rename to osu.Game/Rulesets/Objects/Pooling/PoolableDrawableWithLifetime.cs index 27ed4c04f2..93e476be76 100644 --- a/osu.Game/Rulesets/Objects/Pooling/DrawableObject.cs +++ b/osu.Game/Rulesets/Objects/Pooling/PoolableDrawableWithLifetime.cs @@ -13,15 +13,15 @@ namespace osu.Game.Rulesets.Objects.Pooling /// A that is controlled by to implement drawable pooling and replay rewinding. /// /// The type storing state and controlling this drawable. - public abstract class DrawableObject : PoolableDrawable where TEntry : LifetimeEntry + public abstract class PoolableDrawableWithLifetime : PoolableDrawable where TEntry : LifetimeEntry { /// - /// The entry holding essential state of this . + /// The entry holding essential state of this . /// protected TEntry? Entry { get; private set; } /// - /// Whether is applied to this . + /// Whether is applied to this . /// When an initial entry is specified in the constructor, is set but not applied until loading is completed. /// protected bool HasEntryApplied { get; private set; } @@ -41,7 +41,7 @@ namespace osu.Game.Rulesets.Objects.Pooling public override bool RemoveWhenNotAlive => false; public override bool RemoveCompletedTransforms => false; - protected DrawableObject(TEntry? initialEntry = null) + protected PoolableDrawableWithLifetime(TEntry? initialEntry = null) { Entry = initialEntry; }