From 76d4f86ca3616bc00b9fb62b62244496ab8c5661 Mon Sep 17 00:00:00 2001 From: goodtrailer Date: Fri, 1 Jul 2022 18:21:03 -0700 Subject: [PATCH 1/6] Make legacy slider ball fade out instantly --- .../Skinning/Legacy/LegacySliderBall.cs | 30 ++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacySliderBall.cs b/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacySliderBall.cs index 07e60c82d0..f3be52b227 100644 --- a/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacySliderBall.cs +++ b/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacySliderBall.cs @@ -7,6 +7,8 @@ using osu.Framework.Allocation; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Framework.Graphics.Sprites; +using osu.Game.Rulesets.Objects.Drawables; +using osu.Game.Rulesets.Osu.Objects.Drawables; using osu.Game.Skinning; using osuTK.Graphics; @@ -18,6 +20,8 @@ namespace osu.Game.Rulesets.Osu.Skinning.Legacy private readonly ISkin skin; + private DrawableSlider slider; + private Sprite layerNd; private Sprite layerSpec; @@ -30,8 +34,10 @@ namespace osu.Game.Rulesets.Osu.Skinning.Legacy } [BackgroundDependencyLoader] - private void load() + private void load(DrawableHitObject dho) { + slider = (DrawableSlider)dho; + var ballColour = skin.GetConfig(OsuSkinColour.SliderBall)?.Value ?? Color4.White; InternalChildren = new[] @@ -56,6 +62,8 @@ namespace osu.Game.Rulesets.Osu.Skinning.Legacy Blending = BlendingParameters.Additive, }, }; + + slider.ApplyCustomUpdateState += updateStateTransforms; } protected override void UpdateAfterChildren() @@ -68,5 +76,25 @@ namespace osu.Game.Rulesets.Osu.Skinning.Legacy layerNd.Rotation = -appliedRotation; layerSpec.Rotation = -appliedRotation; } + + private void updateStateTransforms(DrawableHitObject obj, ArmedState _) + { + if (obj is not DrawableSlider) + return; + + using (BeginAbsoluteSequence(slider.StateUpdateTime)) + this.FadeIn(); + + using (BeginAbsoluteSequence(slider.HitStateUpdateTime)) + this.FadeOut(); + } + + protected override void Dispose(bool isDisposing) + { + base.Dispose(isDisposing); + + if (slider != null) + slider.ApplyCustomUpdateState -= updateStateTransforms; + } } } From 2e3ff2c7e0a86b1d9e03859fa6f83bc057d9d962 Mon Sep 17 00:00:00 2001 From: goodtrailer Date: Fri, 1 Jul 2022 21:12:36 -0700 Subject: [PATCH 2/6] Prefer [Resolved] and LoadComplete --- .../Skinning/Legacy/LegacySliderBall.cs | 35 ++++++++++--------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacySliderBall.cs b/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacySliderBall.cs index f3be52b227..fdf7f18bb9 100644 --- a/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacySliderBall.cs +++ b/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacySliderBall.cs @@ -1,14 +1,12 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -#nullable disable - using osu.Framework.Allocation; +using osu.Framework.Extensions.ObjectExtensions; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Framework.Graphics.Sprites; using osu.Game.Rulesets.Objects.Drawables; -using osu.Game.Rulesets.Osu.Objects.Drawables; using osu.Game.Skinning; using osuTK.Graphics; @@ -20,10 +18,11 @@ namespace osu.Game.Rulesets.Osu.Skinning.Legacy private readonly ISkin skin; - private DrawableSlider slider; + [Resolved(canBeNull: true)] + private DrawableHitObject? drawableObject { get; set; } - private Sprite layerNd; - private Sprite layerSpec; + private Sprite layerNd = null!; + private Sprite layerSpec = null!; public LegacySliderBall(Drawable animationContent, ISkin skin) { @@ -36,8 +35,6 @@ namespace osu.Game.Rulesets.Osu.Skinning.Legacy [BackgroundDependencyLoader] private void load(DrawableHitObject dho) { - slider = (DrawableSlider)dho; - var ballColour = skin.GetConfig(OsuSkinColour.SliderBall)?.Value ?? Color4.White; InternalChildren = new[] @@ -62,8 +59,17 @@ namespace osu.Game.Rulesets.Osu.Skinning.Legacy Blending = BlendingParameters.Additive, }, }; + } - slider.ApplyCustomUpdateState += updateStateTransforms; + protected override void LoadComplete() + { + base.LoadComplete(); + + if (drawableObject != null) + { + drawableObject.ApplyCustomUpdateState += updateStateTransforms; + updateStateTransforms(drawableObject, drawableObject.State.Value); + } } protected override void UpdateAfterChildren() @@ -79,13 +85,10 @@ namespace osu.Game.Rulesets.Osu.Skinning.Legacy private void updateStateTransforms(DrawableHitObject obj, ArmedState _) { - if (obj is not DrawableSlider) - return; - - using (BeginAbsoluteSequence(slider.StateUpdateTime)) + using (BeginAbsoluteSequence(drawableObject.AsNonNull().StateUpdateTime)) this.FadeIn(); - using (BeginAbsoluteSequence(slider.HitStateUpdateTime)) + using (BeginAbsoluteSequence(drawableObject.AsNonNull().HitStateUpdateTime)) this.FadeOut(); } @@ -93,8 +96,8 @@ namespace osu.Game.Rulesets.Osu.Skinning.Legacy { base.Dispose(isDisposing); - if (slider != null) - slider.ApplyCustomUpdateState -= updateStateTransforms; + if (drawableObject != null) + drawableObject.ApplyCustomUpdateState -= updateStateTransforms; } } } From 6d6ffd35d01c9ffb8d40943fa50776ef3974a2be Mon Sep 17 00:00:00 2001 From: goodtrailer Date: Fri, 1 Jul 2022 21:14:53 -0700 Subject: [PATCH 3/6] Remove unused parameter --- osu.Game.Rulesets.Osu/Skinning/Legacy/LegacySliderBall.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacySliderBall.cs b/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacySliderBall.cs index fdf7f18bb9..57422e3709 100644 --- a/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacySliderBall.cs +++ b/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacySliderBall.cs @@ -33,7 +33,7 @@ namespace osu.Game.Rulesets.Osu.Skinning.Legacy } [BackgroundDependencyLoader] - private void load(DrawableHitObject dho) + private void load() { var ballColour = skin.GetConfig(OsuSkinColour.SliderBall)?.Value ?? Color4.White; From 471b64bd20619ff1124193cde8bc9621bce4d476 Mon Sep 17 00:00:00 2001 From: goodtrailer Date: Fri, 1 Jul 2022 21:19:54 -0700 Subject: [PATCH 4/6] Reinstate early return with comment Whoops, didn't read the reply until after I pushed... --- osu.Game.Rulesets.Osu/Skinning/Legacy/LegacySliderBall.cs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacySliderBall.cs b/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacySliderBall.cs index 57422e3709..e9fa5cbe43 100644 --- a/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacySliderBall.cs +++ b/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacySliderBall.cs @@ -7,6 +7,7 @@ using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Framework.Graphics.Sprites; using osu.Game.Rulesets.Objects.Drawables; +using osu.Game.Rulesets.Osu.Objects.Drawables; using osu.Game.Skinning; using osuTK.Graphics; @@ -85,6 +86,12 @@ namespace osu.Game.Rulesets.Osu.Skinning.Legacy private void updateStateTransforms(DrawableHitObject obj, ArmedState _) { + // Gets called by slider ticks, tails, etc., leading to duplicated + // animations which in this case have no visual impact (due to + // instant fade) but may negatively affect performance + if (obj is not DrawableSlider) + return; + using (BeginAbsoluteSequence(drawableObject.AsNonNull().StateUpdateTime)) this.FadeIn(); From f2141715b8c6f22eba91df5f98d1f1eb91017c35 Mon Sep 17 00:00:00 2001 From: goodtrailer Date: Fri, 1 Jul 2022 21:22:48 -0700 Subject: [PATCH 5/6] Replace AsNonNull() with Assert() --- osu.Game.Rulesets.Osu/Skinning/Legacy/LegacySliderBall.cs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacySliderBall.cs b/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacySliderBall.cs index e9fa5cbe43..d0745450da 100644 --- a/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacySliderBall.cs +++ b/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacySliderBall.cs @@ -1,8 +1,8 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. +using System.Diagnostics; using osu.Framework.Allocation; -using osu.Framework.Extensions.ObjectExtensions; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Framework.Graphics.Sprites; @@ -92,10 +92,12 @@ namespace osu.Game.Rulesets.Osu.Skinning.Legacy if (obj is not DrawableSlider) return; - using (BeginAbsoluteSequence(drawableObject.AsNonNull().StateUpdateTime)) + Debug.Assert(drawableObject != null); + + using (BeginAbsoluteSequence(drawableObject.StateUpdateTime)) this.FadeIn(); - using (BeginAbsoluteSequence(drawableObject.AsNonNull().HitStateUpdateTime)) + using (BeginAbsoluteSequence(drawableObject.HitStateUpdateTime)) this.FadeOut(); } From 9a5431d93f2688a01249729619d9154af84f2f9e Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sat, 2 Jul 2022 13:36:41 +0900 Subject: [PATCH 6/6] Minor renaming / refactoring to use local parameter rather than DI'd object --- .../Skinning/Legacy/LegacySliderBall.cs | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacySliderBall.cs b/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacySliderBall.cs index d0745450da..414879f42d 100644 --- a/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacySliderBall.cs +++ b/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacySliderBall.cs @@ -1,7 +1,6 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -using System.Diagnostics; using osu.Framework.Allocation; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; @@ -20,7 +19,7 @@ namespace osu.Game.Rulesets.Osu.Skinning.Legacy private readonly ISkin skin; [Resolved(canBeNull: true)] - private DrawableHitObject? drawableObject { get; set; } + private DrawableHitObject? parentObject { get; set; } private Sprite layerNd = null!; private Sprite layerSpec = null!; @@ -66,10 +65,10 @@ namespace osu.Game.Rulesets.Osu.Skinning.Legacy { base.LoadComplete(); - if (drawableObject != null) + if (parentObject != null) { - drawableObject.ApplyCustomUpdateState += updateStateTransforms; - updateStateTransforms(drawableObject, drawableObject.State.Value); + parentObject.ApplyCustomUpdateState += updateStateTransforms; + updateStateTransforms(parentObject, parentObject.State.Value); } } @@ -84,16 +83,14 @@ namespace osu.Game.Rulesets.Osu.Skinning.Legacy layerSpec.Rotation = -appliedRotation; } - private void updateStateTransforms(DrawableHitObject obj, ArmedState _) + private void updateStateTransforms(DrawableHitObject drawableObject, ArmedState _) { // Gets called by slider ticks, tails, etc., leading to duplicated // animations which in this case have no visual impact (due to // instant fade) but may negatively affect performance - if (obj is not DrawableSlider) + if (drawableObject is not DrawableSlider) return; - Debug.Assert(drawableObject != null); - using (BeginAbsoluteSequence(drawableObject.StateUpdateTime)) this.FadeIn(); @@ -105,8 +102,8 @@ namespace osu.Game.Rulesets.Osu.Skinning.Legacy { base.Dispose(isDisposing); - if (drawableObject != null) - drawableObject.ApplyCustomUpdateState -= updateStateTransforms; + if (parentObject != null) + parentObject.ApplyCustomUpdateState -= updateStateTransforms; } } }