Merge pull request #9695 from bdach/hit-lighting-pooling

osu.Game.Rulesets.Osu.Tests/TestSceneDrawableJudgement.cs

@@ -4,62 +4,109 @@
 using System;
 using System.Collections.Generic;
 using System.Linq;
+using NUnit.Framework;
+using osu.Framework.Allocation;
 using osu.Framework.Extensions;
 using osu.Framework.Graphics;
 using osu.Framework.Graphics.Containers;
 using osu.Framework.Graphics.Pooling;
+using osu.Framework.Testing;
+using osu.Game.Configuration;
 using osu.Game.Rulesets.Judgements;
 using osu.Game.Rulesets.Objects;
 using osu.Game.Rulesets.Osu.Objects.Drawables;
 using osu.Game.Rulesets.Scoring;
+using osu.Game.Skinning;
 
 namespace osu.Game.Rulesets.Osu.Tests
 {
     public class TestSceneDrawableJudgement : OsuSkinnableTestScene
     {
+        [Resolved]
+        private OsuConfigManager config { get; set; }
+
+        private readonly List<DrawablePool<TestDrawableOsuJudgement>> pools;
+
         public TestSceneDrawableJudgement()
         {
-            var pools = new List<DrawablePool<DrawableOsuJudgement>>();
+            pools = new List<DrawablePool<TestDrawableOsuJudgement>>();
 
             foreach (HitResult result in Enum.GetValues(typeof(HitResult)).OfType<HitResult>().Skip(1))
+                showResult(result);
+        }
+
+        [Test]
+        public void TestHitLightingDisabled()
+        {
+            AddStep("hit lighting disabled", () => config.Set(OsuSetting.HitLighting, false));
+
+            showResult(HitResult.Great);
+
+            AddUntilStep("judgements shown", () => this.ChildrenOfType<TestDrawableOsuJudgement>().Any());
+            AddAssert("judgement body immediately visible",
+                () => this.ChildrenOfType<TestDrawableOsuJudgement>().All(judgement => judgement.JudgementBody.Alpha == 1));
+            AddAssert("hit lighting hidden",
+                () => this.ChildrenOfType<TestDrawableOsuJudgement>().All(judgement => judgement.Lighting.Alpha == 0));
+        }
+
+        [Test]
+        public void TestHitLightingEnabled()
+        {
+            AddStep("hit lighting enabled", () => config.Set(OsuSetting.HitLighting, true));
+
+            showResult(HitResult.Great);
+
+            AddUntilStep("judgements shown", () => this.ChildrenOfType<TestDrawableOsuJudgement>().Any());
+            AddAssert("judgement body not immediately visible",
+                () => this.ChildrenOfType<TestDrawableOsuJudgement>().All(judgement => judgement.JudgementBody.Alpha > 0 && judgement.JudgementBody.Alpha < 1));
+            AddAssert("hit lighting shown",
+                () => this.ChildrenOfType<TestDrawableOsuJudgement>().All(judgement => judgement.Lighting.Alpha > 0));
+        }
+
+        private void showResult(HitResult result)
+        {
+            AddStep("Show " + result.GetDescription(), () =>
             {
-                AddStep("Show " + result.GetDescription(), () =>
+                int poolIndex = 0;
+
+                SetContents(() =>
                 {
-                    int poolIndex = 0;
+                    DrawablePool<TestDrawableOsuJudgement> pool;
 
-                    SetContents(() =>
+                    if (poolIndex >= pools.Count)
+                        pools.Add(pool = new DrawablePool<TestDrawableOsuJudgement>(1));
+                    else
                     {
-                        DrawablePool<DrawableOsuJudgement> pool;
+                        pool = pools[poolIndex];
 
-                        if (poolIndex >= pools.Count)
+                        // We need to make sure neither the pool nor the judgement get disposed when new content is set, and they both share the same parent.
-                            pools.Add(pool = new DrawablePool<DrawableOsuJudgement>(1));
+                        ((Container)pool.Parent).Clear(false);
-                        else
+                    }
+
+                    var container = new Container
+                    {
+                        RelativeSizeAxes = Axes.Both,
+                        Children = new Drawable[]
                         {
-                            pool = pools[poolIndex];
+                            pool,
-
+                            pool.Get(j => j.Apply(new JudgementResult(new HitObject(), new Judgement()) { Type = result }, null)).With(j =>
-                            // We need to make sure neither the pool nor the judgement get disposed when new content is set, and they both share the same parent.
-                            ((Container)pool.Parent).Clear(false);
-                        }
-
-                        var container = new Container
-                        {
-                            RelativeSizeAxes = Axes.Both,
-                            Children = new Drawable[]
                             {
-                                pool,
+                                j.Anchor = Anchor.Centre;
-                                pool.Get(j => j.Apply(new JudgementResult(new HitObject(), new Judgement()) { Type = result }, null)).With(j =>
+                                j.Origin = Anchor.Centre;
-                                {
+                            })
-                                    j.Anchor = Anchor.Centre;
+                        }
-                                    j.Origin = Anchor.Centre;
+                    };
-                                })
-                            }
-                        };
 
-                        poolIndex++;
+                    poolIndex++;
-                        return container;
+                    return container;
-                    });
                 });
-            }
+            });
+        }
+
+        private class TestDrawableOsuJudgement : DrawableOsuJudgement
+        {
+            public new SkinnableSprite Lighting => base.Lighting;
+            public new Container JudgementBody => base.JudgementBody;
         }
     }
 }

osu.Game.Rulesets.Osu/Objects/Drawables/DrawableOsuJudgement.cs

@@ -16,9 +16,13 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables
 {
     public class DrawableOsuJudgement : DrawableJudgement
     {
-        private SkinnableSprite lighting;
+        protected SkinnableSprite Lighting;
+
         private Bindable<Color4> lightingColour;
 
+        [Resolved]
+        private OsuConfigManager config { get; set; }
+
         public DrawableOsuJudgement(JudgementResult result, DrawableHitObject judgedObject)
             : base(result, judgedObject)
         {
@@ -29,18 +33,16 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables
         }
 
         [BackgroundDependencyLoader]
-        private void load(OsuConfigManager config)
+        private void load()
         {
-            if (config.Get<bool>(OsuSetting.HitLighting))
+            AddInternal(Lighting = new SkinnableSprite("lighting")
             {
-                AddInternal(lighting = new SkinnableSprite("lighting")
+                Anchor = Anchor.Centre,
-                {
+                Origin = Anchor.Centre,
-                    Anchor = Anchor.Centre,
+                Blending = BlendingParameters.Additive,
-                    Origin = Anchor.Centre,
+                Depth = float.MaxValue,
-                    Blending = BlendingParameters.Additive,
+                Alpha = 0
-                    Depth = float.MaxValue
+            });
-                });
-            }
         }
 
         public override void Apply(JudgementResult result, DrawableHitObject judgedObject)
@@ -60,33 +62,39 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables
 
             lightingColour?.UnbindAll();
 
-            if (lighting != null)
+            Lighting.ResetAnimation();
-            {
-                lighting.ResetAnimation();
 
-                if (JudgedObject != null)
+            if (JudgedObject != null)
-                {
+            {
-                    lightingColour = JudgedObject.AccentColour.GetBoundCopy();
+                lightingColour = JudgedObject.AccentColour.GetBoundCopy();
-                    lightingColour.BindValueChanged(colour => lighting.Colour = Result.Type == HitResult.Miss ? Color4.Transparent : colour.NewValue, true);
+                lightingColour.BindValueChanged(colour => Lighting.Colour = Result.Type == HitResult.Miss ? Color4.Transparent : colour.NewValue, true);
-                }
+            }
-                else
+            else
-                {
+            {
-                    lighting.Colour = Color4.White;
+                Lighting.Colour = Color4.White;
-                }
             }
         }
 
-        protected override double FadeOutDelay => lighting == null ? base.FadeOutDelay : 1400;
+        private double fadeOutDelay;
+        protected override double FadeOutDelay => fadeOutDelay;
 
         protected override void ApplyHitAnimations()
         {
-            if (lighting != null)
+            bool hitLightingEnabled = config.Get<bool>(OsuSetting.HitLighting);
+
+            if (hitLightingEnabled)
             {
                 JudgementBody.FadeIn().Delay(FadeInDuration).FadeOut(400);
 
-                lighting.ScaleTo(0.8f).ScaleTo(1.2f, 600, Easing.Out);
+                Lighting.ScaleTo(0.8f).ScaleTo(1.2f, 600, Easing.Out);
-                lighting.FadeIn(200).Then().Delay(200).FadeOut(1000);
+                Lighting.FadeIn(200).Then().Delay(200).FadeOut(1000);
             }
+            else
+            {
+                JudgementBody.Alpha = 1;
+            }
+
+            fadeOutDelay = hitLightingEnabled ? 1400 : base.FadeOutDelay;
 
             JudgementText?.TransformSpacingTo(Vector2.Zero).Then().TransformSpacingTo(new Vector2(14, 0), 1800, Easing.OutQuint);
             base.ApplyHitAnimations();

osu.Game/Rulesets/Judgements/DrawableJudgement.cs

@@ -130,7 +130,11 @@ namespace osu.Game.Rulesets.Judgements
             if (type == currentDrawableType)
                 return;
 
-            InternalChild = JudgementBody = new Container
+            // sub-classes might have added their own children that would be removed here if .InternalChild was used.
+            if (JudgementBody != null)
+                RemoveInternal(JudgementBody);
+
+            AddInternal(JudgementBody = new Container
             {
                 Anchor = Anchor.Centre,
                 Origin = Anchor.Centre,
@@ -142,7 +146,7 @@ namespace osu.Game.Rulesets.Judgements
                     Colour = colours.ForHitResult(type),
                     Scale = new Vector2(0.85f, 1),
                 }, confineMode: ConfineMode.NoScaling)
-            };
+            });
 
             currentDrawableType = type;
         }