From 8f3eb9a422c25472c35f08cc6c82f98881eadf84 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 23 Sep 2020 17:57:57 +0900 Subject: [PATCH 1/5] Fix taiko sample selection not updating when changing strong/rim type --- .../Objects/Drawables/DrawableHit.cs | 32 +++++++++++++++---- .../Drawables/DrawableTaikoHitObject.cs | 28 +++++++++++++++- 2 files changed, 53 insertions(+), 7 deletions(-) diff --git a/osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableHit.cs b/osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableHit.cs index 92ae7e0fd3..f4234445d6 100644 --- a/osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableHit.cs +++ b/osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableHit.cs @@ -42,6 +42,8 @@ namespace osu.Game.Rulesets.Taiko.Objects.Drawables : base(hit) { FillMode = FillMode.Fit; + + updateActionsFromType(); } [BackgroundDependencyLoader] @@ -50,21 +52,39 @@ namespace osu.Game.Rulesets.Taiko.Objects.Drawables type = HitObject.TypeBindable.GetBoundCopy(); type.BindValueChanged(_ => { - updateType(); + updateActionsFromType(); + + // will overwrite samples, should only be called on change. + updateSamplesFromTypeChange(); + RecreatePieces(); }); - - updateType(); } - private void updateType() + private void updateSamplesFromTypeChange() + { + var rimSamples = HitObject.Samples.Where(s => s.Name == HitSampleInfo.HIT_CLAP || s.Name == HitSampleInfo.HIT_WHISTLE).ToArray(); + + bool isRimType = HitObject.Type == HitType.Rim; + + if (isRimType != rimSamples.Any()) + { + if (isRimType) + HitObject.Samples.Add(new HitSampleInfo { Name = HitSampleInfo.HIT_CLAP }); + else + { + foreach (var sample in rimSamples) + HitObject.Samples.Remove(sample); + } + } + } + + private void updateActionsFromType() { HitActions = HitObject.Type == HitType.Centre ? new[] { TaikoAction.LeftCentre, TaikoAction.RightCentre } : new[] { TaikoAction.LeftRim, TaikoAction.RightRim }; - - RecreatePieces(); } protected override SkinnableDrawable CreateMainPiece() => HitObject.Type == HitType.Centre diff --git a/osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableTaikoHitObject.cs b/osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableTaikoHitObject.cs index 929cf8a937..0474de8453 100644 --- a/osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableTaikoHitObject.cs +++ b/osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableTaikoHitObject.cs @@ -141,7 +141,31 @@ namespace osu.Game.Rulesets.Taiko.Objects.Drawables private void load() { isStrong = HitObject.IsStrongBindable.GetBoundCopy(); - isStrong.BindValueChanged(_ => RecreatePieces(), true); + isStrong.BindValueChanged(_ => + { + // will overwrite samples, should only be called on change. + updateSamplesFromStrong(); + + RecreatePieces(); + }); + + RecreatePieces(); + } + + private void updateSamplesFromStrong() + { + var strongSamples = HitObject.Samples.Where(s => s.Name == HitSampleInfo.HIT_FINISH).ToArray(); + + if (isStrong.Value != strongSamples.Any()) + { + if (isStrong.Value) + HitObject.Samples.Add(new HitSampleInfo { Name = HitSampleInfo.HIT_FINISH }); + else + { + foreach (var sample in strongSamples) + HitObject.Samples.Remove(sample); + } + } } protected virtual void RecreatePieces() @@ -150,6 +174,8 @@ namespace osu.Game.Rulesets.Taiko.Objects.Drawables MainPiece?.Expire(); Content.Add(MainPiece = CreateMainPiece()); + + LoadSamples(); } protected override void AddNestedHitObject(DrawableHitObject hitObject) From 9a0e5ac154242ee6fd1764582899e6c1c48114b3 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 23 Sep 2020 18:09:40 +0900 Subject: [PATCH 2/5] Handle type/strength changes from samples changes --- .../Objects/Drawables/DrawableSlider.cs | 4 +-- .../Objects/Drawables/DrawableSpinner.cs | 4 +-- .../Objects/Drawables/DrawableHit.cs | 16 ++++++++-- .../Drawables/DrawableTaikoHitObject.cs | 30 ++++++++++++------- .../Objects/Drawables/DrawableHitObject.cs | 10 +++++-- 5 files changed, 43 insertions(+), 21 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSlider.cs b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSlider.cs index 07f40f763b..fc3bb76ae1 100644 --- a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSlider.cs +++ b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSlider.cs @@ -89,9 +89,9 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables private SkinnableSound slidingSample; - protected override void LoadSamples() + protected override void LoadSamples(bool changed) { - base.LoadSamples(); + base.LoadSamples(changed); slidingSample?.Expire(); slidingSample = null; diff --git a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSpinner.cs b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSpinner.cs index a57bb466c7..e78c886beb 100644 --- a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSpinner.cs +++ b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSpinner.cs @@ -88,9 +88,9 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables private const float spinning_sample_initial_frequency = 1.0f; private const float spinning_sample_modulated_base_frequency = 0.5f; - protected override void LoadSamples() + protected override void LoadSamples(bool changed) { - base.LoadSamples(); + base.LoadSamples(changed); spinningSample?.Expire(); spinningSample = null; diff --git a/osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableHit.cs b/osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableHit.cs index f4234445d6..7608514817 100644 --- a/osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableHit.cs +++ b/osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableHit.cs @@ -61,19 +61,29 @@ namespace osu.Game.Rulesets.Taiko.Objects.Drawables }); } + private HitSampleInfo[] rimSamples => HitObject.Samples.Where(s => s.Name == HitSampleInfo.HIT_CLAP || s.Name == HitSampleInfo.HIT_WHISTLE).ToArray(); + + protected override void LoadSamples(bool changed) + { + base.LoadSamples(changed); + + if (changed) + type.Value = rimSamples.Any() ? HitType.Rim : HitType.Centre; + } + private void updateSamplesFromTypeChange() { - var rimSamples = HitObject.Samples.Where(s => s.Name == HitSampleInfo.HIT_CLAP || s.Name == HitSampleInfo.HIT_WHISTLE).ToArray(); + var samples = rimSamples; bool isRimType = HitObject.Type == HitType.Rim; - if (isRimType != rimSamples.Any()) + if (isRimType != samples.Any()) { if (isRimType) HitObject.Samples.Add(new HitSampleInfo { Name = HitSampleInfo.HIT_CLAP }); else { - foreach (var sample in rimSamples) + foreach (var sample in samples) HitObject.Samples.Remove(sample); } } diff --git a/osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableTaikoHitObject.cs b/osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableTaikoHitObject.cs index 0474de8453..4b1cd80967 100644 --- a/osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableTaikoHitObject.cs +++ b/osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableTaikoHitObject.cs @@ -1,19 +1,19 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -using osu.Framework.Graphics; -using osu.Framework.Input.Bindings; -using osu.Game.Rulesets.Objects.Drawables; -using osuTK; -using System.Linq; -using osu.Game.Audio; using System.Collections.Generic; +using System.Linq; using osu.Framework.Allocation; using osu.Framework.Bindables; +using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Framework.Graphics.Primitives; +using osu.Framework.Input.Bindings; +using osu.Game.Audio; using osu.Game.Rulesets.Objects; +using osu.Game.Rulesets.Objects.Drawables; using osu.Game.Skinning; +using osuTK; namespace osu.Game.Rulesets.Taiko.Objects.Drawables { @@ -152,17 +152,27 @@ namespace osu.Game.Rulesets.Taiko.Objects.Drawables RecreatePieces(); } + private HitSampleInfo[] strongSamples => HitObject.Samples.Where(s => s.Name == HitSampleInfo.HIT_FINISH).ToArray(); + + protected override void LoadSamples(bool changed) + { + base.LoadSamples(changed); + + if (changed) + isStrong.Value = strongSamples.Any(); + } + private void updateSamplesFromStrong() { - var strongSamples = HitObject.Samples.Where(s => s.Name == HitSampleInfo.HIT_FINISH).ToArray(); + var samples = strongSamples; - if (isStrong.Value != strongSamples.Any()) + if (isStrong.Value != samples.Any()) { if (isStrong.Value) HitObject.Samples.Add(new HitSampleInfo { Name = HitSampleInfo.HIT_FINISH }); else { - foreach (var sample in strongSamples) + foreach (var sample in samples) HitObject.Samples.Remove(sample); } } @@ -174,8 +184,6 @@ namespace osu.Game.Rulesets.Taiko.Objects.Drawables MainPiece?.Expire(); Content.Add(MainPiece = CreateMainPiece()); - - LoadSamples(); } protected override void AddNestedHitObject(DrawableHitObject hitObject) diff --git a/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs b/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs index 581617b567..4521556182 100644 --- a/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs +++ b/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs @@ -126,7 +126,7 @@ namespace osu.Game.Rulesets.Objects.Drawables if (Result == null) throw new InvalidOperationException($"{GetType().ReadableName()} must provide a {nameof(JudgementResult)} through {nameof(CreateResult)}."); - LoadSamples(); + LoadSamples(false); } protected override void LoadAsyncComplete() @@ -145,7 +145,7 @@ namespace osu.Game.Rulesets.Objects.Drawables } samplesBindable = HitObject.SamplesBindable.GetBoundCopy(); - samplesBindable.CollectionChanged += (_, __) => LoadSamples(); + samplesBindable.CollectionChanged += (_, __) => LoadSamples(true); apply(HitObject); } @@ -157,7 +157,11 @@ namespace osu.Game.Rulesets.Objects.Drawables updateState(ArmedState.Idle, true); } - protected virtual void LoadSamples() + /// + /// Called to perform sample-related logic. + /// + /// True if triggered from a post-load change to samples. + protected virtual void LoadSamples(bool changed) { if (Samples != null) { From fee379b4b9b997bd53e73e76fe626a2bd71fff93 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 23 Sep 2020 18:12:07 +0900 Subject: [PATCH 3/5] Reword xmldoc for legibility --- osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs b/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs index 4521556182..08235e4ff8 100644 --- a/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs +++ b/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs @@ -158,9 +158,9 @@ namespace osu.Game.Rulesets.Objects.Drawables } /// - /// Called to perform sample-related logic. + /// Invoked by the base to populate samples, once on initial load and potentially again on any change to the samples collection. /// - /// True if triggered from a post-load change to samples. + /// True if triggered from a change to the samples collection. protected virtual void LoadSamples(bool changed) { if (Samples != null) From 156edf24c2736b28d134b9675e64aab08fd1f708 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 24 Sep 2020 13:22:14 +0900 Subject: [PATCH 4/5] Change properties to methods and improve naming --- .../Objects/Drawables/DrawableHit.cs | 10 +++++----- .../Objects/Drawables/DrawableTaikoHitObject.cs | 10 +++++----- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableHit.cs b/osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableHit.cs index 7608514817..7cf77885cf 100644 --- a/osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableHit.cs +++ b/osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableHit.cs @@ -61,29 +61,29 @@ namespace osu.Game.Rulesets.Taiko.Objects.Drawables }); } - private HitSampleInfo[] rimSamples => HitObject.Samples.Where(s => s.Name == HitSampleInfo.HIT_CLAP || s.Name == HitSampleInfo.HIT_WHISTLE).ToArray(); + private HitSampleInfo[] getRimSamples() => HitObject.Samples.Where(s => s.Name == HitSampleInfo.HIT_CLAP || s.Name == HitSampleInfo.HIT_WHISTLE).ToArray(); protected override void LoadSamples(bool changed) { base.LoadSamples(changed); if (changed) - type.Value = rimSamples.Any() ? HitType.Rim : HitType.Centre; + type.Value = getRimSamples().Any() ? HitType.Rim : HitType.Centre; } private void updateSamplesFromTypeChange() { - var samples = rimSamples; + var rimSamples = getRimSamples(); bool isRimType = HitObject.Type == HitType.Rim; - if (isRimType != samples.Any()) + if (isRimType != rimSamples.Any()) { if (isRimType) HitObject.Samples.Add(new HitSampleInfo { Name = HitSampleInfo.HIT_CLAP }); else { - foreach (var sample in samples) + foreach (var sample in rimSamples) HitObject.Samples.Remove(sample); } } diff --git a/osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableTaikoHitObject.cs b/osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableTaikoHitObject.cs index 4b1cd80967..f3790e65af 100644 --- a/osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableTaikoHitObject.cs +++ b/osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableTaikoHitObject.cs @@ -152,27 +152,27 @@ namespace osu.Game.Rulesets.Taiko.Objects.Drawables RecreatePieces(); } - private HitSampleInfo[] strongSamples => HitObject.Samples.Where(s => s.Name == HitSampleInfo.HIT_FINISH).ToArray(); + private HitSampleInfo[] getStrongSamples() => HitObject.Samples.Where(s => s.Name == HitSampleInfo.HIT_FINISH).ToArray(); protected override void LoadSamples(bool changed) { base.LoadSamples(changed); if (changed) - isStrong.Value = strongSamples.Any(); + isStrong.Value = getStrongSamples().Any(); } private void updateSamplesFromStrong() { - var samples = strongSamples; + var strongSamples = getStrongSamples(); - if (isStrong.Value != samples.Any()) + if (isStrong.Value != strongSamples.Any()) { if (isStrong.Value) HitObject.Samples.Add(new HitSampleInfo { Name = HitSampleInfo.HIT_FINISH }); else { - foreach (var sample in samples) + foreach (var sample in strongSamples) HitObject.Samples.Remove(sample); } } From 33fad27ec21705999cb6ad123d6c588fe60ff802 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 24 Sep 2020 13:28:29 +0900 Subject: [PATCH 5/5] Avoid API change to DrawableHitObject --- .../Objects/Drawables/DrawableSlider.cs | 4 ++-- .../Objects/Drawables/DrawableSpinner.cs | 4 ++-- .../Objects/Drawables/DrawableHit.cs | 11 +++++------ .../Objects/Drawables/DrawableTaikoHitObject.cs | 11 +++++------ .../Rulesets/Objects/Drawables/DrawableHitObject.cs | 7 +++---- 5 files changed, 17 insertions(+), 20 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSlider.cs b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSlider.cs index fc3bb76ae1..07f40f763b 100644 --- a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSlider.cs +++ b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSlider.cs @@ -89,9 +89,9 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables private SkinnableSound slidingSample; - protected override void LoadSamples(bool changed) + protected override void LoadSamples() { - base.LoadSamples(changed); + base.LoadSamples(); slidingSample?.Expire(); slidingSample = null; diff --git a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSpinner.cs b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSpinner.cs index e78c886beb..a57bb466c7 100644 --- a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSpinner.cs +++ b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSpinner.cs @@ -88,9 +88,9 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables private const float spinning_sample_initial_frequency = 1.0f; private const float spinning_sample_modulated_base_frequency = 0.5f; - protected override void LoadSamples(bool changed) + protected override void LoadSamples() { - base.LoadSamples(changed); + base.LoadSamples(); spinningSample?.Expire(); spinningSample = null; diff --git a/osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableHit.cs b/osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableHit.cs index 7cf77885cf..3a6eaa83db 100644 --- a/osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableHit.cs +++ b/osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableHit.cs @@ -36,11 +36,12 @@ namespace osu.Game.Rulesets.Taiko.Objects.Drawables private bool pressHandledThisFrame; - private Bindable type; + private readonly Bindable type; public DrawableHit(Hit hit) : base(hit) { + type = HitObject.TypeBindable.GetBoundCopy(); FillMode = FillMode.Fit; updateActionsFromType(); @@ -49,7 +50,6 @@ namespace osu.Game.Rulesets.Taiko.Objects.Drawables [BackgroundDependencyLoader] private void load() { - type = HitObject.TypeBindable.GetBoundCopy(); type.BindValueChanged(_ => { updateActionsFromType(); @@ -63,12 +63,11 @@ namespace osu.Game.Rulesets.Taiko.Objects.Drawables private HitSampleInfo[] getRimSamples() => HitObject.Samples.Where(s => s.Name == HitSampleInfo.HIT_CLAP || s.Name == HitSampleInfo.HIT_WHISTLE).ToArray(); - protected override void LoadSamples(bool changed) + protected override void LoadSamples() { - base.LoadSamples(changed); + base.LoadSamples(); - if (changed) - type.Value = getRimSamples().Any() ? HitType.Rim : HitType.Centre; + type.Value = getRimSamples().Any() ? HitType.Rim : HitType.Centre; } private void updateSamplesFromTypeChange() diff --git a/osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableTaikoHitObject.cs b/osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableTaikoHitObject.cs index f3790e65af..9cd23383c4 100644 --- a/osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableTaikoHitObject.cs +++ b/osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableTaikoHitObject.cs @@ -120,7 +120,7 @@ namespace osu.Game.Rulesets.Taiko.Objects.Drawables protected Vector2 BaseSize; protected SkinnableDrawable MainPiece; - private Bindable isStrong; + private readonly Bindable isStrong; private readonly Container strongHitContainer; @@ -128,6 +128,7 @@ namespace osu.Game.Rulesets.Taiko.Objects.Drawables : base(hitObject) { HitObject = hitObject; + isStrong = HitObject.IsStrongBindable.GetBoundCopy(); Anchor = Anchor.CentreLeft; Origin = Anchor.Custom; @@ -140,7 +141,6 @@ namespace osu.Game.Rulesets.Taiko.Objects.Drawables [BackgroundDependencyLoader] private void load() { - isStrong = HitObject.IsStrongBindable.GetBoundCopy(); isStrong.BindValueChanged(_ => { // will overwrite samples, should only be called on change. @@ -154,12 +154,11 @@ namespace osu.Game.Rulesets.Taiko.Objects.Drawables private HitSampleInfo[] getStrongSamples() => HitObject.Samples.Where(s => s.Name == HitSampleInfo.HIT_FINISH).ToArray(); - protected override void LoadSamples(bool changed) + protected override void LoadSamples() { - base.LoadSamples(changed); + base.LoadSamples(); - if (changed) - isStrong.Value = getStrongSamples().Any(); + isStrong.Value = getStrongSamples().Any(); } private void updateSamplesFromStrong() diff --git a/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs b/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs index 08235e4ff8..28d3a39096 100644 --- a/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs +++ b/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs @@ -126,7 +126,7 @@ namespace osu.Game.Rulesets.Objects.Drawables if (Result == null) throw new InvalidOperationException($"{GetType().ReadableName()} must provide a {nameof(JudgementResult)} through {nameof(CreateResult)}."); - LoadSamples(false); + LoadSamples(); } protected override void LoadAsyncComplete() @@ -145,7 +145,7 @@ namespace osu.Game.Rulesets.Objects.Drawables } samplesBindable = HitObject.SamplesBindable.GetBoundCopy(); - samplesBindable.CollectionChanged += (_, __) => LoadSamples(true); + samplesBindable.CollectionChanged += (_, __) => LoadSamples(); apply(HitObject); } @@ -160,8 +160,7 @@ namespace osu.Game.Rulesets.Objects.Drawables /// /// Invoked by the base to populate samples, once on initial load and potentially again on any change to the samples collection. /// - /// True if triggered from a change to the samples collection. - protected virtual void LoadSamples(bool changed) + protected virtual void LoadSamples() { if (Samples != null) {