From d74ac57092f3d1953345532010936d93f3beb052 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 13 Aug 2024 19:26:24 +0900 Subject: [PATCH 1/3] Never call `prepareDrawables` from unsafe context I can't mentally figure out *what* is causing the issue here, but in the case where `prepareDrawables` is called from `JudgementBody.OnSkinChanged` (only happens in a non-pooled scenario), things go very wrong. I think a smell test is enough for anyone to agree that the flow was very bad. Removing this call doesn't seem to cause any issues. `runAnimation` should always be called in `PrepareForUse` (both pooled and non-pooled scenarios) so things should still always be in a correct state. Closes #29398. --- osu.Game/Rulesets/Judgements/DrawableJudgement.cs | 3 --- 1 file changed, 3 deletions(-) diff --git a/osu.Game/Rulesets/Judgements/DrawableJudgement.cs b/osu.Game/Rulesets/Judgements/DrawableJudgement.cs index bdeadfd201..1b12bfc945 100644 --- a/osu.Game/Rulesets/Judgements/DrawableJudgement.cs +++ b/osu.Game/Rulesets/Judgements/DrawableJudgement.cs @@ -119,9 +119,6 @@ namespace osu.Game.Rulesets.Judgements private void runAnimation() { - // is a no-op if the drawables are already in a correct state. - prepareDrawables(); - // undo any transforms applies in ApplyMissAnimations/ApplyHitAnimations to get a sane initial state. ApplyTransformsAt(double.MinValue, true); ClearTransforms(true); From bb0c9e24974f2a8ea2b3c2954d2342f053a7da7f Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 13 Aug 2024 19:30:29 +0900 Subject: [PATCH 2/3] Add log output when judgements aren't being pooled --- osu.Game/Rulesets/Judgements/DrawableJudgement.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/osu.Game/Rulesets/Judgements/DrawableJudgement.cs b/osu.Game/Rulesets/Judgements/DrawableJudgement.cs index 1b12bfc945..0e04cd3eec 100644 --- a/osu.Game/Rulesets/Judgements/DrawableJudgement.cs +++ b/osu.Game/Rulesets/Judgements/DrawableJudgement.cs @@ -7,6 +7,7 @@ using osu.Framework.Allocation; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Framework.Graphics.Pooling; +using osu.Framework.Logging; using osu.Game.Rulesets.Objects; using osu.Game.Rulesets.Objects.Drawables; using osu.Game.Rulesets.Scoring; @@ -112,6 +113,9 @@ namespace osu.Game.Rulesets.Judgements { base.PrepareForUse(); + if (!IsInPool) + Logger.Log($"{nameof(DrawableJudgement)} for judgement type {Result} was not retrieved from a pool. Consider adding to a JudgementPooler."); + Debug.Assert(Result != null); runAnimation(); From 2221c4891f3fa7e9d0b005adefee982ab6091f2d Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 13 Aug 2024 21:03:00 +0900 Subject: [PATCH 3/3] Remove legacy non-pooled pathway to `DrawableJudgement` --- .../Skinning/TestSceneDrawableJudgement.cs | 16 +++++++++++----- .../UI/DrawableManiaJudgement.cs | 10 ---------- .../Rulesets/Judgements/DrawableJudgement.cs | 11 ----------- 3 files changed, 11 insertions(+), 26 deletions(-) diff --git a/osu.Game.Rulesets.Mania.Tests/Skinning/TestSceneDrawableJudgement.cs b/osu.Game.Rulesets.Mania.Tests/Skinning/TestSceneDrawableJudgement.cs index c993ba0e0a..b52919987f 100644 --- a/osu.Game.Rulesets.Mania.Tests/Skinning/TestSceneDrawableJudgement.cs +++ b/osu.Game.Rulesets.Mania.Tests/Skinning/TestSceneDrawableJudgement.cs @@ -28,14 +28,20 @@ namespace osu.Game.Rulesets.Mania.Tests.Skinning AddStep("Show " + result.GetDescription(), () => { SetContents(_ => - new DrawableManiaJudgement(new JudgementResult(new HitObject { StartTime = Time.Current }, new Judgement()) - { - Type = result - }, null) + { + var drawableManiaJudgement = new DrawableManiaJudgement { Anchor = Anchor.Centre, Origin = Anchor.Centre, - }); + }; + + drawableManiaJudgement.Apply(new JudgementResult(new HitObject { StartTime = Time.Current }, new Judgement()) + { + Type = result + }, null); + + return drawableManiaJudgement; + }); // for test purposes, undo the Y adjustment related to the `ScorePosition` legacy positioning config value // (see `LegacyManiaJudgementPiece.load()`). diff --git a/osu.Game.Rulesets.Mania/UI/DrawableManiaJudgement.cs b/osu.Game.Rulesets.Mania/UI/DrawableManiaJudgement.cs index 896dfb2b23..9f25a44e21 100644 --- a/osu.Game.Rulesets.Mania/UI/DrawableManiaJudgement.cs +++ b/osu.Game.Rulesets.Mania/UI/DrawableManiaJudgement.cs @@ -5,22 +5,12 @@ using osu.Framework.Graphics; using osu.Game.Rulesets.Judgements; -using osu.Game.Rulesets.Objects.Drawables; using osu.Game.Rulesets.Scoring; namespace osu.Game.Rulesets.Mania.UI { public partial class DrawableManiaJudgement : DrawableJudgement { - public DrawableManiaJudgement(JudgementResult result, DrawableHitObject judgedObject) - : base(result, judgedObject) - { - } - - public DrawableManiaJudgement() - { - } - protected override Drawable CreateDefaultJudgement(HitResult result) => new DefaultManiaJudgementPiece(result); private partial class DefaultManiaJudgementPiece : DefaultJudgementPiece diff --git a/osu.Game/Rulesets/Judgements/DrawableJudgement.cs b/osu.Game/Rulesets/Judgements/DrawableJudgement.cs index 0e04cd3eec..8c326ecf49 100644 --- a/osu.Game/Rulesets/Judgements/DrawableJudgement.cs +++ b/osu.Game/Rulesets/Judgements/DrawableJudgement.cs @@ -36,17 +36,6 @@ namespace osu.Game.Rulesets.Judgements private readonly Lazy proxiedAboveHitObjectsContent; public Drawable ProxiedAboveHitObjectsContent => proxiedAboveHitObjectsContent.Value; - /// - /// Creates a drawable which visualises a . - /// - /// The judgement to visualise. - /// The object which was judged. - public DrawableJudgement(JudgementResult result, DrawableHitObject judgedObject) - : this() - { - Apply(result, judgedObject); - } - public DrawableJudgement() { Size = new Vector2(judgement_size);