From 248d342a2f0e487c6b5c000aee604b5337c25c57 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Fri, 6 Nov 2020 22:15:00 +0900 Subject: [PATCH] Initial Apply()/FreeAfterUse() DHO implementation --- .../Objects/Drawables/DrawableHitObject.cs | 169 ++++++++++++------ osu.Game/Rulesets/Objects/HitObject.cs | 6 + osu.Game/Rulesets/UI/HitObjectContainer.cs | 89 ++++----- osu.Game/Skinning/SkinReloadableDrawable.cs | 1 - 4 files changed, 168 insertions(+), 97 deletions(-) diff --git a/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs b/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs index 5c3f57c2d0..43688a7332 100644 --- a/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs +++ b/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Collections.Specialized; using System.Linq; using JetBrains.Annotations; using osu.Framework.Allocation; @@ -27,7 +28,10 @@ namespace osu.Game.Rulesets.Objects.Drawables { public event Action DefaultsApplied; - public readonly HitObject HitObject; + /// + /// The currently represented by this . + /// + public HitObject HitObject { get; private set; } /// /// The colour used for various elements of this DrawableHitObject. @@ -96,10 +100,10 @@ namespace osu.Game.Rulesets.Objects.Drawables /// protected virtual float SamplePlaybackPosition => 0.5f; - private BindableList samplesBindable; - private Bindable startTimeBindable; - private Bindable userPositionalHitSounds; - private Bindable comboIndexBindable; + public readonly Bindable StartTimeBindable = new Bindable(); + private readonly BindableList samplesBindable = new BindableList(); + private readonly Bindable userPositionalHitSounds = new Bindable(); + private readonly Bindable comboIndexBindable = new Bindable(); public override bool RemoveWhenNotAlive => false; public override bool RemoveCompletedTransforms => false; @@ -111,52 +115,120 @@ namespace osu.Game.Rulesets.Objects.Drawables public IBindable State => state; - protected DrawableHitObject([NotNull] HitObject hitObject) + /// + /// Creates a new . + /// + /// + /// The to be initially applied to this . + /// If null, a hitobject is expected to be later applied via (or automatically via pooling). + /// + protected DrawableHitObject([CanBeNull] HitObject initialHitObject = null) { - HitObject = hitObject ?? throw new ArgumentNullException(nameof(hitObject)); + HitObject = initialHitObject; } [BackgroundDependencyLoader] private void load(OsuConfigManager config) { - userPositionalHitSounds = config.GetBindable(OsuSetting.PositionalHitSounds); - var judgement = HitObject.CreateJudgement(); - - Result = CreateResult(judgement); - if (Result == null) - throw new InvalidOperationException($"{GetType().ReadableName()} must provide a {nameof(JudgementResult)} through {nameof(CreateResult)}."); + config.BindWith(OsuSetting.PositionalHitSounds, userPositionalHitSounds); } protected override void LoadAsyncComplete() { base.LoadAsyncComplete(); - LoadSamples(); - - HitObject.DefaultsApplied += onDefaultsApplied; - - startTimeBindable = HitObject.StartTimeBindable.GetBoundCopy(); - startTimeBindable.BindValueChanged(_ => updateState(State.Value, true)); - - if (HitObject is IHasComboInformation combo) - { - comboIndexBindable = combo.ComboIndexBindable.GetBoundCopy(); - comboIndexBindable.BindValueChanged(_ => updateComboColour(), true); - } - - samplesBindable = HitObject.SamplesBindable.GetBoundCopy(); - samplesBindable.CollectionChanged += (_, __) => LoadSamples(); - - apply(HitObject); + if (HitObject != null) + Apply(HitObject); } protected override void LoadComplete() { base.LoadComplete(); + StartTimeBindable.BindValueChanged(_ => updateState(State.Value, true)); + comboIndexBindable.BindValueChanged(_ => updateComboColour(), true); + updateState(ArmedState.Idle, true); } + /// + /// Removes the currently applied to this , + /// + protected override void FreeAfterUse() + { + StartTimeBindable.UnbindFrom(HitObject.StartTimeBindable); + if (HitObject is IHasComboInformation combo) + comboIndexBindable.UnbindFrom(combo.ComboIndexBindable); + + samplesBindable.UnbindFrom(HitObject.SamplesBindable); + + // When a new hitobject is applied, the samples will be cleared before re-populating. + // In order to stop this needless update, the event is unbound and re-bound as late as possible in Apply(). + samplesBindable.CollectionChanged -= onSamplesChanged; + + if (nestedHitObjects.IsValueCreated) + { + foreach (var obj in nestedHitObjects.Value) + { + obj.OnNewResult -= onNewResult; + obj.OnRevertResult -= onRevertResult; + obj.ApplyCustomUpdateState -= onApplyCustomUpdateState; + } + + nestedHitObjects.Value.Clear(); + ClearNestedHitObjects(); + } + + HitObject.DefaultsApplied -= onDefaultsApplied; + HitObject = null; + + base.FreeAfterUse(); + } + + /// + /// Applies a new to be represented by this . + /// + /// + public virtual void Apply(HitObject hitObject) + { + HitObject = hitObject; + + // Copy any existing result from the hitobject (required for rewind / judgement revert). + Result = HitObject.Result; + + // Ensure this DHO has a result. + Result ??= CreateResult(HitObject.CreateJudgement()) + ?? throw new InvalidOperationException($"{GetType().ReadableName()} must provide a {nameof(JudgementResult)} through {nameof(CreateResult)}."); + + // Ensure the hitobject has a result. + HitObject.Result = Result; + + foreach (var h in HitObject.NestedHitObjects) + { + var drawableNested = CreateNestedHitObject(h) ?? throw new InvalidOperationException($"{nameof(CreateNestedHitObject)} returned null for {h.GetType().ReadableName()}."); + + drawableNested.OnNewResult += (d, r) => OnNewResult?.Invoke(d, r); + drawableNested.OnRevertResult += (d, r) => OnRevertResult?.Invoke(d, r); + drawableNested.ApplyCustomUpdateState += (d, j) => ApplyCustomUpdateState?.Invoke(d, j); + + nestedHitObjects.Value.Add(drawableNested); + AddNestedHitObject(drawableNested); + } + + StartTimeBindable.BindTo(HitObject.StartTimeBindable); + if (HitObject is IHasComboInformation combo) + comboIndexBindable.BindTo(combo.ComboIndexBindable); + + samplesBindable.BindTo(HitObject.SamplesBindable); + samplesBindable.BindCollectionChanged(onSamplesChanged, true); + + HitObject.DefaultsApplied += onDefaultsApplied; + + // If not loaded, the state update happens in LoadComplete(). Otherwise, the update is scheduled to allow for lifetime updates. + if (IsLoaded) + Schedule(() => updateState(ArmedState.Idle, true)); + } + /// /// Invoked by the base to populate samples, once on initial load and potentially again on any change to the samples collection. /// @@ -183,34 +255,21 @@ namespace osu.Game.Rulesets.Objects.Drawables AddInternal(Samples); } + private void onSamplesChanged(object sender, NotifyCollectionChangedEventArgs e) => LoadSamples(); + + private void onNewResult(DrawableHitObject drawableHitObject, JudgementResult result) => OnNewResult?.Invoke(drawableHitObject, result); + + private void onRevertResult(DrawableHitObject drawableHitObject, JudgementResult result) => OnRevertResult?.Invoke(drawableHitObject, result); + + private void onApplyCustomUpdateState(DrawableHitObject drawableHitObject, ArmedState state) => ApplyCustomUpdateState?.Invoke(drawableHitObject, state); + private void onDefaultsApplied(HitObject hitObject) { - apply(hitObject); - updateState(state.Value, true); + FreeAfterUse(); + Apply(hitObject); DefaultsApplied?.Invoke(this); } - private void apply(HitObject hitObject) - { - if (nestedHitObjects.IsValueCreated) - { - nestedHitObjects.Value.Clear(); - ClearNestedHitObjects(); - } - - foreach (var h in hitObject.NestedHitObjects) - { - var drawableNested = CreateNestedHitObject(h) ?? throw new InvalidOperationException($"{nameof(CreateNestedHitObject)} returned null for {h.GetType().ReadableName()}."); - - drawableNested.OnNewResult += (d, r) => OnNewResult?.Invoke(d, r); - drawableNested.OnRevertResult += (d, r) => OnRevertResult?.Invoke(d, r); - drawableNested.ApplyCustomUpdateState += (d, j) => ApplyCustomUpdateState?.Invoke(d, j); - - nestedHitObjects.Value.Add(drawableNested); - AddNestedHitObject(drawableNested); - } - } - /// /// Invoked by the base to add nested s to the hierarchy. /// @@ -600,7 +659,9 @@ namespace osu.Game.Rulesets.Objects.Drawables protected override void Dispose(bool isDisposing) { base.Dispose(isDisposing); - HitObject.DefaultsApplied -= onDefaultsApplied; + + if (HitObject != null) + HitObject.DefaultsApplied -= onDefaultsApplied; } } diff --git a/osu.Game/Rulesets/Objects/HitObject.cs b/osu.Game/Rulesets/Objects/HitObject.cs index 826d411822..9ef3ff9c4a 100644 --- a/osu.Game/Rulesets/Objects/HitObject.cs +++ b/osu.Game/Rulesets/Objects/HitObject.cs @@ -12,6 +12,7 @@ using osu.Game.Audio; using osu.Game.Beatmaps; using osu.Game.Beatmaps.ControlPoints; using osu.Game.Rulesets.Judgements; +using osu.Game.Rulesets.Objects.Drawables; using osu.Game.Rulesets.Objects.Types; using osu.Game.Rulesets.Scoring; @@ -168,6 +169,11 @@ namespace osu.Game.Rulesets.Objects /// [NotNull] protected virtual HitWindows CreateHitWindows() => new HitWindows(); + + /// + /// The result this was judged with. Used internally for rewinding within . + /// + internal JudgementResult Result; } public static class HitObjectExtensions diff --git a/osu.Game/Rulesets/UI/HitObjectContainer.cs b/osu.Game/Rulesets/UI/HitObjectContainer.cs index 4cadfa9ad4..4666aa9211 100644 --- a/osu.Game/Rulesets/UI/HitObjectContainer.cs +++ b/osu.Game/Rulesets/UI/HitObjectContainer.cs @@ -16,7 +16,7 @@ namespace osu.Game.Rulesets.UI public IEnumerable Objects => InternalChildren.Cast().OrderBy(h => h.HitObject.StartTime); public IEnumerable AliveObjects => AliveInternalChildren.Cast().OrderBy(h => h.HitObject.StartTime); - private readonly Dictionary bindable, double timeAtAdd)> startTimeMap = new Dictionary, double)>(); + private readonly Dictionary startTimeMap = new Dictionary(); public HitObjectContainer() { @@ -25,10 +25,7 @@ namespace osu.Game.Rulesets.UI public virtual void Add(DrawableHitObject hitObject) { - // Added first for the comparer to remain ordered during AddInternal - startTimeMap[hitObject] = (hitObject.HitObject.StartTimeBindable.GetBoundCopy(), hitObject.HitObject.StartTime); - startTimeMap[hitObject].bindable.BindValueChanged(_ => onStartTimeChanged(hitObject)); - + bindStartTime(hitObject); AddInternal(hitObject); } @@ -37,54 +34,19 @@ namespace osu.Game.Rulesets.UI if (!RemoveInternal(hitObject)) return false; - // Removed last for the comparer to remain ordered during RemoveInternal - startTimeMap[hitObject].bindable.UnbindAll(); - startTimeMap.Remove(hitObject); + unbindStartTime(hitObject); return true; } - protected override void Dispose(bool isDisposing) - { - base.Dispose(isDisposing); - unbindStartTimeMap(); - } - public virtual void Clear(bool disposeChildren = true) { ClearInternal(disposeChildren); - unbindStartTimeMap(); - } - - private void unbindStartTimeMap() - { - foreach (var kvp in startTimeMap) - kvp.Value.bindable.UnbindAll(); - startTimeMap.Clear(); + unbindAllStartTimes(); } public int IndexOf(DrawableHitObject hitObject) => IndexOfInternal(hitObject); - private void onStartTimeChanged(DrawableHitObject hitObject) - { - if (!RemoveInternal(hitObject)) - return; - - // Update the stored time, preserving the existing bindable - startTimeMap[hitObject] = (startTimeMap[hitObject].bindable, hitObject.HitObject.StartTime); - AddInternal(hitObject); - } - - protected override int Compare(Drawable x, Drawable y) - { - if (!(x is DrawableHitObject xObj) || !(y is DrawableHitObject yObj)) - return base.Compare(x, y); - - // Put earlier hitobjects towards the end of the list, so they handle input first - int i = startTimeMap[yObj].timeAtAdd.CompareTo(startTimeMap[xObj].timeAtAdd); - return i == 0 ? CompareReverseChildID(x, y) : i; - } - protected override void OnChildLifetimeBoundaryCrossed(LifetimeBoundaryCrossedEvent e) { if (!(e.Child is DrawableHitObject hitObject)) @@ -96,5 +58,48 @@ namespace osu.Game.Rulesets.UI hitObject.OnKilled(); } } + + #region Comparator + StartTime tracking + + private void bindStartTime(DrawableHitObject hitObject) + { + var bindable = hitObject.StartTimeBindable.GetBoundCopy(); + bindable.BindValueChanged(_ => onStartTimeChanged(hitObject)); + + startTimeMap[hitObject] = bindable; + } + + private void unbindStartTime(DrawableHitObject hitObject) + { + startTimeMap[hitObject].UnbindAll(); + startTimeMap.Remove(hitObject); + } + + private void unbindAllStartTimes() + { + foreach (var kvp in startTimeMap) + kvp.Value.UnbindAll(); + startTimeMap.Clear(); + } + + private void onStartTimeChanged(DrawableHitObject hitObject) => SortInternal(); + + protected override int Compare(Drawable x, Drawable y) + { + if (!(x is DrawableHitObject xObj) || !(y is DrawableHitObject yObj)) + return base.Compare(x, y); + + // Put earlier hitobjects towards the end of the list, so they handle input first + int i = yObj.HitObject.StartTime.CompareTo(xObj.HitObject.StartTime); + return i == 0 ? CompareReverseChildID(x, y) : i; + } + + #endregion + + protected override void Dispose(bool isDisposing) + { + base.Dispose(isDisposing); + unbindAllStartTimes(); + } } } diff --git a/osu.Game/Skinning/SkinReloadableDrawable.cs b/osu.Game/Skinning/SkinReloadableDrawable.cs index d0531c46c5..cc9cbf7b59 100644 --- a/osu.Game/Skinning/SkinReloadableDrawable.cs +++ b/osu.Game/Skinning/SkinReloadableDrawable.cs @@ -3,7 +3,6 @@ using System; using osu.Framework.Allocation; -using osu.Framework.Graphics.Containers; using osu.Framework.Graphics.Pooling; namespace osu.Game.Skinning