From 5afdc3ff66f39eb34bc8359e715f6719da8c038a Mon Sep 17 00:00:00 2001 From: ekrctb Date: Mon, 19 Apr 2021 19:56:17 +0900 Subject: [PATCH 01/11] Make DHO application logic clearer with Entry/HitObject separation --- .../Objects/Drawables/DrawableHitObject.cs | 57 ++++++++++++++----- 1 file changed, 42 insertions(+), 15 deletions(-) diff --git a/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs b/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs index 669e4cecbe..dfeb87de88 100644 --- a/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs +++ b/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs @@ -204,23 +204,27 @@ namespace osu.Game.Rulesets.Objects.Drawables /// The controlling the lifetime of . public void Apply([NotNull] HitObject hitObject, [CanBeNull] HitObjectLifetimeEntry lifetimeEntry) { - free(); - - HitObject = hitObject ?? throw new InvalidOperationException($"Cannot apply a null {nameof(HitObject)}."); - - this.lifetimeEntry = lifetimeEntry; + if (hitObject == null) + throw new ArgumentNullException($"Cannot apply a null {nameof(HitObject)}."); if (lifetimeEntry != null) { - // Transfer lifetime from the entry. - LifetimeStart = lifetimeEntry.LifetimeStart; - LifetimeEnd = lifetimeEntry.LifetimeEnd; - - // Copy any existing result from the entry (required for rewind / judgement revert). - Result = lifetimeEntry.Result; + applyEntry(lifetimeEntry); } else - LifetimeStart = HitObject.StartTime - InitialLifetimeOffset; + { + applyHitObject(hitObject); + + // Set default lifetime for a non-pooled DHO + LifetimeStart = hitObject.StartTime - InitialLifetimeOffset; + } + } + + private void applyHitObject([NotNull] HitObject hitObject) + { + freeHitObject(); + + HitObject = hitObject; // Ensure this DHO has a result. Result ??= CreateResult(HitObject.CreateJudgement()) @@ -281,10 +285,23 @@ namespace osu.Game.Rulesets.Objects.Drawables hasHitObjectApplied = true; } + private void applyEntry([NotNull] HitObjectLifetimeEntry entry) + { + freeEntry(); + + setLifetime(entry.LifetimeStart, entry.LifetimeEnd); + lifetimeEntry = entry; + + // Copy any existing result from the entry (required for rewind / judgement revert). + Result = entry.Result; + + applyHitObject(entry.HitObject); + } + /// /// Removes the currently applied /// - private void free() + private void freeHitObject() { if (!hasHitObjectApplied) return; @@ -322,13 +339,23 @@ namespace osu.Game.Rulesets.Objects.Drawables HitObject = null; ParentHitObject = null; Result = null; - lifetimeEntry = null; clearExistingStateTransforms(); hasHitObjectApplied = false; } + private void freeEntry() + { + freeHitObject(); + + if (lifetimeEntry == null) return; + + lifetimeEntry = null; + + setLifetime(double.MaxValue, double.MaxValue); + } + protected sealed override void FreeAfterUse() { base.FreeAfterUse(); @@ -337,7 +364,7 @@ namespace osu.Game.Rulesets.Objects.Drawables if (!IsInPool) return; - free(); + freeEntry(); } /// From 2c487ddb70bf4078422fba8e772e5e564e582d36 Mon Sep 17 00:00:00 2001 From: ekrctb Date: Mon, 19 Apr 2021 20:37:06 +0900 Subject: [PATCH 02/11] Create synthetic LifetimeEntry for a DHO when not supplied Now, a DHO is always associated with a HitObjectLifetimeEntry while used. Result is always stored in the entry, and not in the DHO. --- .../Objects/Drawables/DrawableHitObject.cs | 87 ++++++++----------- .../Objects/UnmanagedHitObjectEntry.cs | 20 +++++ 2 files changed, 55 insertions(+), 52 deletions(-) create mode 100644 osu.Game/Rulesets/Objects/UnmanagedHitObjectEntry.cs diff --git a/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs b/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs index dfeb87de88..9b132ea932 100644 --- a/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs +++ b/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs @@ -39,7 +39,12 @@ namespace osu.Game.Rulesets.Objects.Drawables /// /// The currently represented by this . /// - public HitObject HitObject { get; private set; } + public HitObject HitObject => lifetimeEntry?.HitObject ?? initialHitObject; + + /// + /// The given in the constructor that will be applied when loaded. + /// + private HitObject initialHitObject; /// /// The parenting , if any. @@ -108,7 +113,7 @@ namespace osu.Game.Rulesets.Objects.Drawables /// /// The scoring result of this . /// - public JudgementResult Result { get; private set; } + public JudgementResult Result => lifetimeEntry?.Result; /// /// The relative X position of this hit object for sample playback balance adjustment. @@ -140,11 +145,6 @@ namespace osu.Game.Rulesets.Objects.Drawables /// public IBindable State => state; - /// - /// Whether is currently applied. - /// - private bool hasHitObjectApplied; - /// /// The controlling the lifetime of the currently-attached . /// @@ -168,7 +168,7 @@ namespace osu.Game.Rulesets.Objects.Drawables /// protected DrawableHitObject([CanBeNull] HitObject initialHitObject = null) { - HitObject = initialHitObject; + this.initialHitObject = initialHitObject; } [BackgroundDependencyLoader] @@ -184,8 +184,11 @@ namespace osu.Game.Rulesets.Objects.Drawables { base.LoadAsyncComplete(); - if (HitObject != null) - Apply(HitObject, lifetimeEntry); + if (initialHitObject != null) + { + Apply(initialHitObject, null); + initialHitObject = null; + } } protected override void LoadComplete() @@ -209,26 +212,36 @@ namespace osu.Game.Rulesets.Objects.Drawables if (lifetimeEntry != null) { - applyEntry(lifetimeEntry); + if (lifetimeEntry.HitObject != hitObject) + throw new InvalidOperationException($"{nameof(HitObjectLifetimeEntry)} has different {nameof(HitObject)} from the specified one."); + + apply(lifetimeEntry); } else { - applyHitObject(hitObject); + var unmanagedEntry = new UnmanagedHitObjectEntry(hitObject, this); + apply(unmanagedEntry); // Set default lifetime for a non-pooled DHO LifetimeStart = hitObject.StartTime - InitialLifetimeOffset; } } - private void applyHitObject([NotNull] HitObject hitObject) + /// + /// Applies a new to be represented by this . + /// + private void apply([NotNull] HitObjectLifetimeEntry entry) { - freeHitObject(); + free(); - HitObject = hitObject; + lifetimeEntry = entry; + + LifetimeStart = entry.LifetimeStart; + LifetimeEnd = entry.LifetimeEnd; // Ensure this DHO has a result. - Result ??= CreateResult(HitObject.CreateJudgement()) - ?? throw new InvalidOperationException($"{GetType().ReadableName()} must provide a {nameof(JudgementResult)} through {nameof(CreateResult)}."); + entry.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) @@ -281,30 +294,14 @@ namespace osu.Game.Rulesets.Objects.Drawables else updateState(ArmedState.Idle, true); } - - hasHitObjectApplied = true; - } - - private void applyEntry([NotNull] HitObjectLifetimeEntry entry) - { - freeEntry(); - - setLifetime(entry.LifetimeStart, entry.LifetimeEnd); - lifetimeEntry = entry; - - // Copy any existing result from the entry (required for rewind / judgement revert). - Result = entry.Result; - - applyHitObject(entry.HitObject); } /// - /// Removes the currently applied + /// Removes the currently applied /// - private void freeHitObject() + private void free() { - if (!hasHitObjectApplied) - return; + if (lifetimeEntry == null) return; StartTimeBindable.UnbindFrom(HitObject.StartTimeBindable); if (HitObject is IHasComboInformation combo) @@ -336,24 +333,10 @@ namespace osu.Game.Rulesets.Objects.Drawables OnFree(); - HitObject = null; ParentHitObject = null; - Result = null; - - clearExistingStateTransforms(); - - hasHitObjectApplied = false; - } - - private void freeEntry() - { - freeHitObject(); - - if (lifetimeEntry == null) return; - lifetimeEntry = null; - setLifetime(double.MaxValue, double.MaxValue); + clearExistingStateTransforms(); } protected sealed override void FreeAfterUse() @@ -364,7 +347,7 @@ namespace osu.Game.Rulesets.Objects.Drawables if (!IsInPool) return; - freeEntry(); + free(); } /// diff --git a/osu.Game/Rulesets/Objects/UnmanagedHitObjectEntry.cs b/osu.Game/Rulesets/Objects/UnmanagedHitObjectEntry.cs new file mode 100644 index 0000000000..507cad15d3 --- /dev/null +++ b/osu.Game/Rulesets/Objects/UnmanagedHitObjectEntry.cs @@ -0,0 +1,20 @@ +// Copyright (c) ppy Pty Ltd . 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 +{ + internal class UnmanagedHitObjectEntry : HitObjectLifetimeEntry + { + public readonly DrawableHitObject DrawableHitObject; + + public UnmanagedHitObjectEntry(HitObject hitObject, DrawableHitObject drawableHitObject) + : base(hitObject) + { + DrawableHitObject = drawableHitObject; + LifetimeStart = DrawableHitObject.LifetimeStart; + LifetimeEnd = DrawableHitObject.LifetimeEnd; + } + } +} From c1b4aaaa03334995f4a96f223b3ce446a8ffb0dc Mon Sep 17 00:00:00 2001 From: ekrctb Date: Tue, 20 Apr 2021 08:38:02 +0900 Subject: [PATCH 03/11] Add doc comment --- osu.Game/Rulesets/Objects/UnmanagedHitObjectEntry.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/osu.Game/Rulesets/Objects/UnmanagedHitObjectEntry.cs b/osu.Game/Rulesets/Objects/UnmanagedHitObjectEntry.cs index 507cad15d3..86fa59f589 100644 --- a/osu.Game/Rulesets/Objects/UnmanagedHitObjectEntry.cs +++ b/osu.Game/Rulesets/Objects/UnmanagedHitObjectEntry.cs @@ -5,6 +5,10 @@ using osu.Game.Rulesets.Objects.Drawables; namespace osu.Game.Rulesets.Objects { + /// + /// Created for a when only is given + /// to make sure a is always associated with a . + /// internal class UnmanagedHitObjectEntry : HitObjectLifetimeEntry { public readonly DrawableHitObject DrawableHitObject; From 1bc63a4c611a99c9652743a9ba94eb6acc288e0a Mon Sep 17 00:00:00 2001 From: ekrctb Date: Tue, 20 Apr 2021 09:17:13 +0900 Subject: [PATCH 04/11] Now, DHO.lifetimeEntry can be non-null even it is not fully applied --- .../Objects/Drawables/DrawableHitObject.cs | 46 ++++++++----------- .../Objects/UnmanagedHitObjectEntry.cs | 7 +-- 2 files changed, 20 insertions(+), 33 deletions(-) diff --git a/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs b/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs index 9b132ea932..16f5ed9d17 100644 --- a/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs +++ b/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs @@ -39,12 +39,7 @@ namespace osu.Game.Rulesets.Objects.Drawables /// /// The currently represented by this . /// - public HitObject HitObject => lifetimeEntry?.HitObject ?? initialHitObject; - - /// - /// The given in the constructor that will be applied when loaded. - /// - private HitObject initialHitObject; + public HitObject HitObject => lifetimeEntry?.HitObject; /// /// The parenting , if any. @@ -145,9 +140,15 @@ 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; @@ -168,7 +169,8 @@ namespace osu.Game.Rulesets.Objects.Drawables /// protected DrawableHitObject([CanBeNull] HitObject initialHitObject = null) { - this.initialHitObject = initialHitObject; + if (initialHitObject != null) + lifetimeEntry = new UnmanagedHitObjectEntry(initialHitObject); } [BackgroundDependencyLoader] @@ -184,11 +186,8 @@ namespace osu.Game.Rulesets.Objects.Drawables { base.LoadAsyncComplete(); - if (initialHitObject != null) - { - Apply(initialHitObject, null); - initialHitObject = null; - } + if (lifetimeEntry != null && !hasEntryApplied) + apply(lifetimeEntry); } protected override void LoadComplete() @@ -210,21 +209,10 @@ namespace osu.Game.Rulesets.Objects.Drawables if (hitObject == null) throw new ArgumentNullException($"Cannot apply a null {nameof(HitObject)}."); - if (lifetimeEntry != null) - { - if (lifetimeEntry.HitObject != hitObject) - throw new InvalidOperationException($"{nameof(HitObjectLifetimeEntry)} has different {nameof(HitObject)} from the specified one."); + if (lifetimeEntry != null && lifetimeEntry.HitObject != hitObject) + throw new InvalidOperationException($"{nameof(HitObjectLifetimeEntry)} has different {nameof(HitObject)} from the specified one."); - apply(lifetimeEntry); - } - else - { - var unmanagedEntry = new UnmanagedHitObjectEntry(hitObject, this); - apply(unmanagedEntry); - - // Set default lifetime for a non-pooled DHO - LifetimeStart = hitObject.StartTime - InitialLifetimeOffset; - } + apply(lifetimeEntry ?? new UnmanagedHitObjectEntry(hitObject)); } /// @@ -294,6 +282,8 @@ namespace osu.Game.Rulesets.Objects.Drawables else updateState(ArmedState.Idle, true); } + + hasEntryApplied = true; } /// @@ -301,7 +291,7 @@ namespace osu.Game.Rulesets.Objects.Drawables /// private void free() { - if (lifetimeEntry == null) return; + if (!hasEntryApplied) return; StartTimeBindable.UnbindFrom(HitObject.StartTimeBindable); if (HitObject is IHasComboInformation combo) @@ -337,6 +327,8 @@ namespace osu.Game.Rulesets.Objects.Drawables lifetimeEntry = null; clearExistingStateTransforms(); + + hasEntryApplied = false; } protected sealed override void FreeAfterUse() diff --git a/osu.Game/Rulesets/Objects/UnmanagedHitObjectEntry.cs b/osu.Game/Rulesets/Objects/UnmanagedHitObjectEntry.cs index 86fa59f589..3cfa3c4d3a 100644 --- a/osu.Game/Rulesets/Objects/UnmanagedHitObjectEntry.cs +++ b/osu.Game/Rulesets/Objects/UnmanagedHitObjectEntry.cs @@ -11,14 +11,9 @@ namespace osu.Game.Rulesets.Objects /// internal class UnmanagedHitObjectEntry : HitObjectLifetimeEntry { - public readonly DrawableHitObject DrawableHitObject; - - public UnmanagedHitObjectEntry(HitObject hitObject, DrawableHitObject drawableHitObject) + public UnmanagedHitObjectEntry(HitObject hitObject) : base(hitObject) { - DrawableHitObject = drawableHitObject; - LifetimeStart = DrawableHitObject.LifetimeStart; - LifetimeEnd = DrawableHitObject.LifetimeEnd; } } } From 8a8b9084efa04747e8147d7602fc277359c18b43 Mon Sep 17 00:00:00 2001 From: ekrctb Date: Tue, 20 Apr 2021 10:11:36 +0900 Subject: [PATCH 05/11] Make single-argument overloead of DHO.Apply public --- .../Objects/Drawables/DrawableHitObject.cs | 46 ++++++++++++------- 1 file changed, 30 insertions(+), 16 deletions(-) diff --git a/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs b/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs index 16f5ed9d17..59088cffda 100644 --- a/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs +++ b/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; @@ -165,7 +166,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) { @@ -187,7 +188,7 @@ namespace osu.Game.Rulesets.Objects.Drawables base.LoadAsyncComplete(); if (lifetimeEntry != null && !hasEntryApplied) - apply(lifetimeEntry); + Apply(lifetimeEntry); } protected override void LoadComplete() @@ -200,36 +201,47 @@ namespace osu.Game.Rulesets.Objects.Drawables } /// - /// Applies a new to be represented by this . + /// Applies a hit object to be represented by this . /// - /// The to apply. - /// The controlling the lifetime of . + /// This overload is semi-deprecated. Use either or . public void Apply([NotNull] HitObject hitObject, [CanBeNull] HitObjectLifetimeEntry lifetimeEntry) + { + if (lifetimeEntry != null && lifetimeEntry.HitObject != hitObject) + throw new InvalidOperationException($"{nameof(HitObjectLifetimeEntry)} has different {nameof(HitObject)} from the specified one."); + + if (lifetimeEntry != null) + Apply(lifetimeEntry); + else + Apply(hitObject); + } + + /// + /// Applies a new to be represented by this . + /// A new is automatically created and applied to this . + /// + public void Apply([NotNull] HitObject hitObject) { if (hitObject == null) throw new ArgumentNullException($"Cannot apply a null {nameof(HitObject)}."); - if (lifetimeEntry != null && lifetimeEntry.HitObject != hitObject) - throw new InvalidOperationException($"{nameof(HitObjectLifetimeEntry)} has different {nameof(HitObject)} from the specified one."); - - apply(lifetimeEntry ?? new UnmanagedHitObjectEntry(hitObject)); + Apply(new UnmanagedHitObjectEntry(hitObject)); } /// /// Applies a new to be represented by this . /// - private void apply([NotNull] HitObjectLifetimeEntry entry) + public void Apply([NotNull] HitObjectLifetimeEntry newEntry) { free(); - lifetimeEntry = entry; + lifetimeEntry = newEntry; - LifetimeStart = entry.LifetimeStart; - LifetimeEnd = entry.LifetimeEnd; + LifetimeStart = lifetimeEntry.LifetimeStart; + LifetimeEnd = lifetimeEntry.LifetimeEnd; // Ensure this DHO has a result. - entry.Result ??= CreateResult(HitObject.CreateJudgement()) - ?? throw new InvalidOperationException($"{GetType().ReadableName()} must provide a {nameof(JudgementResult)} through {nameof(CreateResult)}."); + lifetimeEntry.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) @@ -387,7 +399,9 @@ namespace osu.Game.Rulesets.Objects.Drawables private void onDefaultsApplied(HitObject hitObject) { - Apply(hitObject, lifetimeEntry); + Debug.Assert(lifetimeEntry != null); + Apply(lifetimeEntry); + DefaultsApplied?.Invoke(this); } From c6ee4e900e25201389ba194b768832e45322941f Mon Sep 17 00:00:00 2001 From: ekrctb Date: Tue, 20 Apr 2021 15:18:36 +0900 Subject: [PATCH 06/11] Ensure a non-null hitobject entry has a non-null Result --- .../Objects/Drawables/DrawableHitObject.cs | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs b/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs index 59088cffda..d798eba4e1 100644 --- a/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs +++ b/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs @@ -171,7 +171,10 @@ namespace osu.Game.Rulesets.Objects.Drawables protected DrawableHitObject([CanBeNull] HitObject initialHitObject = null) { if (initialHitObject != null) + { lifetimeEntry = new UnmanagedHitObjectEntry(initialHitObject); + ensureEntryHasResult(); + } } [BackgroundDependencyLoader] @@ -239,13 +242,7 @@ namespace osu.Game.Rulesets.Objects.Drawables LifetimeStart = lifetimeEntry.LifetimeStart; LifetimeEnd = lifetimeEntry.LifetimeEnd; - // Ensure this DHO has a result. - lifetimeEntry.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) { @@ -799,6 +796,13 @@ namespace osu.Game.Rulesets.Objects.Drawables /// The that provides the scoring information. 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); From e80c3c317a34b989dc18a9d904d39da724c828f3 Mon Sep 17 00:00:00 2001 From: ekrctb Date: Wed, 21 Apr 2021 09:14:22 +0900 Subject: [PATCH 07/11] Rename UnmanagedHitObjectEntry -> SyntheticHitObjectEntry "Unmanaged" was confusing because its lifetime is still managed by the HitObjectContainer. --- osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs | 4 ++-- ...{UnmanagedHitObjectEntry.cs => SyntheticHitObjectEntry.cs} | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) rename osu.Game/Rulesets/Objects/{UnmanagedHitObjectEntry.cs => SyntheticHitObjectEntry.cs} (81%) diff --git a/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs b/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs index d798eba4e1..ca1c601c1d 100644 --- a/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs +++ b/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs @@ -172,7 +172,7 @@ namespace osu.Game.Rulesets.Objects.Drawables { if (initialHitObject != null) { - lifetimeEntry = new UnmanagedHitObjectEntry(initialHitObject); + lifetimeEntry = new SyntheticHitObjectEntry(initialHitObject); ensureEntryHasResult(); } } @@ -227,7 +227,7 @@ namespace osu.Game.Rulesets.Objects.Drawables if (hitObject == null) throw new ArgumentNullException($"Cannot apply a null {nameof(HitObject)}."); - Apply(new UnmanagedHitObjectEntry(hitObject)); + Apply(new SyntheticHitObjectEntry(hitObject)); } /// diff --git a/osu.Game/Rulesets/Objects/UnmanagedHitObjectEntry.cs b/osu.Game/Rulesets/Objects/SyntheticHitObjectEntry.cs similarity index 81% rename from osu.Game/Rulesets/Objects/UnmanagedHitObjectEntry.cs rename to osu.Game/Rulesets/Objects/SyntheticHitObjectEntry.cs index 3cfa3c4d3a..76f9eaf25a 100644 --- a/osu.Game/Rulesets/Objects/UnmanagedHitObjectEntry.cs +++ b/osu.Game/Rulesets/Objects/SyntheticHitObjectEntry.cs @@ -9,9 +9,9 @@ namespace osu.Game.Rulesets.Objects /// Created for a when only is given /// to make sure a is always associated with a . /// - internal class UnmanagedHitObjectEntry : HitObjectLifetimeEntry + internal class SyntheticHitObjectEntry : HitObjectLifetimeEntry { - public UnmanagedHitObjectEntry(HitObject hitObject) + public SyntheticHitObjectEntry(HitObject hitObject) : base(hitObject) { } From 67fcfd9dbc243a8ffe976410729efeb94c06336d Mon Sep 17 00:00:00 2001 From: ekrctb Date: Wed, 21 Apr 2021 09:48:16 +0900 Subject: [PATCH 08/11] Fix wrong InitialLifetimeOffset is used for a non-pooled DHO. HitObjectLifetimeEntry's InitialLifetimeOffset is different from DrawableHitObject's InitialLifetimeOffset. --- osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs | 4 ++-- osu.Game/Rulesets/Objects/HitObjectLifetimeEntry.cs | 9 +++++++-- osu.Game/Rulesets/Objects/SyntheticHitObjectEntry.cs | 6 ++++-- 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs b/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs index ca1c601c1d..4eea058163 100644 --- a/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs +++ b/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs @@ -172,7 +172,7 @@ namespace osu.Game.Rulesets.Objects.Drawables { if (initialHitObject != null) { - lifetimeEntry = new SyntheticHitObjectEntry(initialHitObject); + lifetimeEntry = new SyntheticHitObjectEntry(initialHitObject, initialHitObject.StartTime - InitialLifetimeOffset); ensureEntryHasResult(); } } @@ -227,7 +227,7 @@ namespace osu.Game.Rulesets.Objects.Drawables if (hitObject == null) throw new ArgumentNullException($"Cannot apply a null {nameof(HitObject)}."); - Apply(new SyntheticHitObjectEntry(hitObject)); + Apply(new SyntheticHitObjectEntry(hitObject, hitObject.StartTime - InitialLifetimeOffset)); } /// diff --git a/osu.Game/Rulesets/Objects/HitObjectLifetimeEntry.cs b/osu.Game/Rulesets/Objects/HitObjectLifetimeEntry.cs index 1954d7e6d2..d1d459a8f6 100644 --- a/osu.Game/Rulesets/Objects/HitObjectLifetimeEntry.cs +++ b/osu.Game/Rulesets/Objects/HitObjectLifetimeEntry.cs @@ -30,12 +30,17 @@ namespace osu.Game.Rulesets.Objects /// Creates a new . /// /// The to store the lifetime of. - public HitObjectLifetimeEntry(HitObject hitObject) + /// The . + /// The . + public HitObjectLifetimeEntry(HitObject hitObject, double lifetimeStart = double.MinValue, double lifetimeEnd = double.MaxValue) { HitObject = hitObject; startTimeBindable.BindTo(HitObject.StartTimeBindable); - startTimeBindable.BindValueChanged(onStartTimeChanged, true); + // Only set initial lifetime if it is not provided + startTimeBindable.BindValueChanged(onStartTimeChanged, lifetimeStart == double.MinValue); + + setLifetime(lifetimeStart, lifetimeEnd); } // The lifetime start, as set by the hitobject. diff --git a/osu.Game/Rulesets/Objects/SyntheticHitObjectEntry.cs b/osu.Game/Rulesets/Objects/SyntheticHitObjectEntry.cs index 76f9eaf25a..c064f3ff10 100644 --- a/osu.Game/Rulesets/Objects/SyntheticHitObjectEntry.cs +++ b/osu.Game/Rulesets/Objects/SyntheticHitObjectEntry.cs @@ -1,6 +1,8 @@ // 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 osu.Game.Rulesets.Objects.Drawables; namespace osu.Game.Rulesets.Objects @@ -11,8 +13,8 @@ namespace osu.Game.Rulesets.Objects /// internal class SyntheticHitObjectEntry : HitObjectLifetimeEntry { - public SyntheticHitObjectEntry(HitObject hitObject) - : base(hitObject) + public SyntheticHitObjectEntry(HitObject hitObject, double initialLifetimeStart) + : base(hitObject, initialLifetimeStart) { } } From 44ff08cce4675e40492364633a1917608f7c908b Mon Sep 17 00:00:00 2001 From: ekrctb Date: Wed, 21 Apr 2021 10:02:50 +0900 Subject: [PATCH 09/11] Revert "Fix wrong InitialLifetimeOffset is used for a non-pooled DHO." This reverts commit 67fcfd9d --- osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs | 4 ++-- osu.Game/Rulesets/Objects/HitObjectLifetimeEntry.cs | 9 ++------- osu.Game/Rulesets/Objects/SyntheticHitObjectEntry.cs | 6 ++---- 3 files changed, 6 insertions(+), 13 deletions(-) diff --git a/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs b/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs index 4eea058163..ca1c601c1d 100644 --- a/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs +++ b/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs @@ -172,7 +172,7 @@ namespace osu.Game.Rulesets.Objects.Drawables { if (initialHitObject != null) { - lifetimeEntry = new SyntheticHitObjectEntry(initialHitObject, initialHitObject.StartTime - InitialLifetimeOffset); + lifetimeEntry = new SyntheticHitObjectEntry(initialHitObject); ensureEntryHasResult(); } } @@ -227,7 +227,7 @@ namespace osu.Game.Rulesets.Objects.Drawables if (hitObject == null) throw new ArgumentNullException($"Cannot apply a null {nameof(HitObject)}."); - Apply(new SyntheticHitObjectEntry(hitObject, hitObject.StartTime - InitialLifetimeOffset)); + Apply(new SyntheticHitObjectEntry(hitObject)); } /// diff --git a/osu.Game/Rulesets/Objects/HitObjectLifetimeEntry.cs b/osu.Game/Rulesets/Objects/HitObjectLifetimeEntry.cs index d1d459a8f6..1954d7e6d2 100644 --- a/osu.Game/Rulesets/Objects/HitObjectLifetimeEntry.cs +++ b/osu.Game/Rulesets/Objects/HitObjectLifetimeEntry.cs @@ -30,17 +30,12 @@ namespace osu.Game.Rulesets.Objects /// Creates a new . /// /// The to store the lifetime of. - /// The . - /// The . - public HitObjectLifetimeEntry(HitObject hitObject, double lifetimeStart = double.MinValue, double lifetimeEnd = double.MaxValue) + public HitObjectLifetimeEntry(HitObject hitObject) { HitObject = hitObject; startTimeBindable.BindTo(HitObject.StartTimeBindable); - // Only set initial lifetime if it is not provided - startTimeBindable.BindValueChanged(onStartTimeChanged, lifetimeStart == double.MinValue); - - setLifetime(lifetimeStart, lifetimeEnd); + startTimeBindable.BindValueChanged(onStartTimeChanged, true); } // The lifetime start, as set by the hitobject. diff --git a/osu.Game/Rulesets/Objects/SyntheticHitObjectEntry.cs b/osu.Game/Rulesets/Objects/SyntheticHitObjectEntry.cs index c064f3ff10..76f9eaf25a 100644 --- a/osu.Game/Rulesets/Objects/SyntheticHitObjectEntry.cs +++ b/osu.Game/Rulesets/Objects/SyntheticHitObjectEntry.cs @@ -1,8 +1,6 @@ // 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 osu.Game.Rulesets.Objects.Drawables; namespace osu.Game.Rulesets.Objects @@ -13,8 +11,8 @@ namespace osu.Game.Rulesets.Objects /// internal class SyntheticHitObjectEntry : HitObjectLifetimeEntry { - public SyntheticHitObjectEntry(HitObject hitObject, double initialLifetimeStart) - : base(hitObject, initialLifetimeStart) + public SyntheticHitObjectEntry(HitObject hitObject) + : base(hitObject) { } } From 73d3da168769bf884ec876cdf888aa6f118a8dcb Mon Sep 17 00:00:00 2001 From: ekrctb Date: Wed, 21 Apr 2021 11:32:01 +0900 Subject: [PATCH 10/11] Fix wrong InitialLifetimeOffset is used for a non-pooled DHO. HitObjectLifetimeEntry's InitialLifetimeOffset is different from DrawableHitObject's InitialLifetimeOffset. --- osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs b/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs index ca1c601c1d..d0fa7eb22f 100644 --- a/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs +++ b/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs @@ -239,6 +239,11 @@ namespace osu.Game.Rulesets.Objects.Drawables 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; + LifetimeStart = lifetimeEntry.LifetimeStart; LifetimeEnd = lifetimeEntry.LifetimeEnd; From 3fbeadf31847cd87aef622e7ddba08cbb253f9ec Mon Sep 17 00:00:00 2001 From: ekrctb Date: Wed, 21 Apr 2021 14:32:37 +0900 Subject: [PATCH 11/11] Deprecate old overload of Apply --- osu.Game.Rulesets.Osu.Tests/TestSceneHitCircleApplication.cs | 2 +- osu.Game.Rulesets.Osu.Tests/TestSceneSliderApplication.cs | 2 +- osu.Game.Rulesets.Osu.Tests/TestSceneSpinnerApplication.cs | 2 +- osu.Game.Rulesets.Taiko.Tests/TestSceneBarLineApplication.cs | 4 ++-- .../TestSceneDrumRollApplication.cs | 4 ++-- osu.Game.Rulesets.Taiko.Tests/TestSceneHitApplication.cs | 4 ++-- osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs | 5 +---- osu.Game/Rulesets/UI/Playfield.cs | 2 +- 8 files changed, 11 insertions(+), 14 deletions(-) diff --git a/osu.Game.Rulesets.Osu.Tests/TestSceneHitCircleApplication.cs b/osu.Game.Rulesets.Osu.Tests/TestSceneHitCircleApplication.cs index 5fc1082743..8b3fead366 100644 --- a/osu.Game.Rulesets.Osu.Tests/TestSceneHitCircleApplication.cs +++ b/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) diff --git a/osu.Game.Rulesets.Osu.Tests/TestSceneSliderApplication.cs b/osu.Game.Rulesets.Osu.Tests/TestSceneSliderApplication.cs index aac6db60fe..e698766aac 100644 --- a/osu.Game.Rulesets.Osu.Tests/TestSceneSliderApplication.cs +++ b/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] diff --git a/osu.Game.Rulesets.Osu.Tests/TestSceneSpinnerApplication.cs b/osu.Game.Rulesets.Osu.Tests/TestSceneSpinnerApplication.cs index d7fbc7ac48..8c97c02049 100644 --- a/osu.Game.Rulesets.Osu.Tests/TestSceneSpinnerApplication.cs +++ b/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); } diff --git a/osu.Game.Rulesets.Taiko.Tests/TestSceneBarLineApplication.cs b/osu.Game.Rulesets.Taiko.Tests/TestSceneBarLineApplication.cs index a970965141..f33c738b04 100644 --- a/osu.Game.Rulesets.Taiko.Tests/TestSceneBarLineApplication.cs +++ b/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); } } diff --git a/osu.Game.Rulesets.Taiko.Tests/TestSceneDrumRollApplication.cs b/osu.Game.Rulesets.Taiko.Tests/TestSceneDrumRollApplication.cs index 54450e27db..c389a05566 100644 --- a/osu.Game.Rulesets.Taiko.Tests/TestSceneDrumRollApplication.cs +++ b/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); } diff --git a/osu.Game.Rulesets.Taiko.Tests/TestSceneHitApplication.cs b/osu.Game.Rulesets.Taiko.Tests/TestSceneHitApplication.cs index 52fd440857..c2f251fcb6 100644 --- a/osu.Game.Rulesets.Taiko.Tests/TestSceneHitApplication.cs +++ b/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); } diff --git a/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs b/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs index d0fa7eb22f..ba2b8423d0 100644 --- a/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs +++ b/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs @@ -206,12 +206,9 @@ namespace osu.Game.Rulesets.Objects.Drawables /// /// Applies a hit object to be represented by this . /// - /// This overload is semi-deprecated. Use either or . + [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 && lifetimeEntry.HitObject != hitObject) - throw new InvalidOperationException($"{nameof(HitObjectLifetimeEntry)} has different {nameof(HitObject)} from the specified one."); - if (lifetimeEntry != null) Apply(lifetimeEntry); else diff --git a/osu.Game/Rulesets/UI/Playfield.cs b/osu.Game/Rulesets/UI/Playfield.cs index d55005363c..17d3cf01a4 100644 --- a/osu.Game/Rulesets/UI/Playfield.cs +++ b/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); }); }