From cfd8d05fde5cb5cf0b45ff9284ae1182305e5c27 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Mon, 23 Oct 2023 21:26:25 +0200 Subject: [PATCH 1/3] Fix spinner ticks not playing samples correctly sometimes Noticed this during work on https://github.com/ppy/osu/pull/25185. In some circumstances, it seemed that spinner bonus ticks (and mostly them specifically) would not always play with the correct volume. Hours of debugging later pointed at a trace at `DrawableAudioWrapper.refreshLayoutFromParent()` not firing sometimes. Initially I thought it to be some sort of framework bug, but after preparing a diff and running final checks, I noticed that sometimes the sample was being played *by a `PoolableSkinnableSample` that wasn't loaded*. And determining why *that* is the case turned out with this diff. As it happens, spinner ticks get assigned a start time proportionally, i.e. the 1st of 10 ticks is placed at 10% of the duration, the 2nd at 20%, and so on. The start time generally shouldn't matter, because the spinner is manually judging the ticks. *However*, the ticks *still* receive a lifetime start / end in the same way normal objects do, which means that in some cases they can *not be alive* when they're hit, which means that the `DrawableAudioWrapper` flow *hasn't had a chance to run*, and rightly so. To fix, ensure that all spinner ticks are alive throughout the entirety of the spinner's duration. --- .../Objects/Drawables/DrawableSpinnerTick.cs | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSpinnerTick.cs b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSpinnerTick.cs index 34253e3d4f2b..20fe7172adf5 100644 --- a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSpinnerTick.cs +++ b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSpinnerTick.cs @@ -4,6 +4,8 @@ #nullable disable using osu.Framework.Graphics; +using osu.Game.Rulesets.Objects; +using osu.Game.Rulesets.Objects.Drawables; namespace osu.Game.Rulesets.Osu.Objects.Drawables { @@ -25,6 +27,26 @@ public DrawableSpinnerTick(SpinnerTick spinnerTick) Origin = Anchor.Centre; } + protected override void OnApply() + { + base.OnApply(); + + // the tick can be theoretically judged at any point in the spinner's duration, + // so it must be alive throughout the spinner's entire lifetime. + // this mostly matters for correct sample playback. + LifetimeStart = DrawableSpinner.HitObject.StartTime; + } + + protected override void UpdateHitStateTransforms(ArmedState state) + { + base.UpdateHitStateTransforms(state); + + // the tick can be theoretically judged at any point in the spinner's duration, + // so it must be alive throughout the spinner's entire lifetime (or until hit, whichever applies). + // this mostly matters for correct sample playback. + LifetimeEnd = (Result?.IsHit == true ? Result.TimeAbsolute : DrawableSpinner.HitObject.GetEndTime()) + (Samples?.Length ?? 0); + } + /// /// Apply a judgement result. /// From a0c8158033fcb59459bd10e7ceaba11310241ce5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Tue, 24 Oct 2023 10:42:03 +0200 Subject: [PATCH 2/3] Remove unnecessary `LifetimeEnd` specification --- .../Objects/Drawables/DrawableSpinnerTick.cs | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSpinnerTick.cs b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSpinnerTick.cs index 20fe7172adf5..b2de4da01b76 100644 --- a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSpinnerTick.cs +++ b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSpinnerTick.cs @@ -5,7 +5,6 @@ using osu.Framework.Graphics; using osu.Game.Rulesets.Objects; -using osu.Game.Rulesets.Objects.Drawables; namespace osu.Game.Rulesets.Osu.Objects.Drawables { @@ -37,16 +36,6 @@ protected override void OnApply() LifetimeStart = DrawableSpinner.HitObject.StartTime; } - protected override void UpdateHitStateTransforms(ArmedState state) - { - base.UpdateHitStateTransforms(state); - - // the tick can be theoretically judged at any point in the spinner's duration, - // so it must be alive throughout the spinner's entire lifetime (or until hit, whichever applies). - // this mostly matters for correct sample playback. - LifetimeEnd = (Result?.IsHit == true ? Result.TimeAbsolute : DrawableSpinner.HitObject.GetEndTime()) + (Samples?.Length ?? 0); - } - /// /// Apply a judgement result. /// From 68397f0e816f131dda8f20b6959298403d7a87b1 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 24 Oct 2023 18:01:24 +0900 Subject: [PATCH 3/3] Remove unused using --- osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSpinnerTick.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSpinnerTick.cs b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSpinnerTick.cs index b2de4da01b76..a5785dd1f6a2 100644 --- a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSpinnerTick.cs +++ b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSpinnerTick.cs @@ -4,7 +4,6 @@ #nullable disable using osu.Framework.Graphics; -using osu.Game.Rulesets.Objects; namespace osu.Game.Rulesets.Osu.Objects.Drawables {