Various code changes, fixes

osu.Game.Rulesets.Taiko.Tests/Skinning/TestSceneDrawableTaikoMascot.cs

@@ -6,9 +6,9 @@ using System.Collections.Generic;
 using System.Linq;
 using NUnit.Framework;
 using osu.Framework.Allocation;
-using osu.Framework.Graphics;
-using osu.Framework.Graphics.Containers;
+using osu.Framework.Bindables;
 using osu.Framework.Testing;
+using osu.Game.Beatmaps;
 using osu.Game.Beatmaps.ControlPoints;
 using osu.Game.Rulesets.Judgements;
 using osu.Game.Rulesets.Objects;
@@ -18,7 +18,6 @@ using osu.Game.Rulesets.Taiko.Objects;
 using osu.Game.Rulesets.Taiko.Objects.Drawables;
 using osu.Game.Rulesets.Taiko.UI;
 using osu.Game.Rulesets.UI.Scrolling;
-using osu.Game.Skinning;
 using osu.Game.Tests.Visual;
 
 namespace osu.Game.Rulesets.Taiko.Tests.Skinning
@@ -38,35 +37,28 @@ namespace osu.Game.Rulesets.Taiko.Tests.Skinning
             TimeRange = { Value = 5000 },
         };
 
+        [Cached(typeof(IBindable<WorkingBeatmap>))]
+        private Bindable<WorkingBeatmap> beatmap = new Bindable<WorkingBeatmap>();
+
         private readonly List<DrawableTaikoMascot> mascots = new List<DrawableTaikoMascot>();
-        private readonly List<SkinnableDrawable> skinnables = new List<SkinnableDrawable>();
         private readonly List<TaikoPlayfield> playfields = new List<TaikoPlayfield>();
+        private readonly List<DrawableTaikoRuleset> rulesets = new List<DrawableTaikoRuleset>();
 
         [Test]
         public void TestStateTextures()
         {
+            AddStep("Set beatmap", () => setBeatmap());
+
             AddStep("Create mascot (idle)", () =>
-            {
-                skinnables.Clear();
-                SetContents(() =>
-                {
-                    var skinnable = getMascot();
-                    skinnables.Add(skinnable);
-                    return skinnable;
-                });
-            });
-
-            AddUntilStep("Wait for SkinnableDrawable", () => skinnables.Any(d => d.Drawable is DrawableTaikoMascot));
-
-            AddStep("Collect mascots", () =>
             {
                 mascots.Clear();
 
-                foreach (var skinnable in skinnables)
+                SetContents(() =>
                 {
-                    if (skinnable.Drawable is DrawableTaikoMascot mascot)
-                        mascots.Add(mascot);
-                }
+                    var mascot = new TestDrawableTaikoMascot();
+                    mascots.Add(mascot);
+                    return mascot;
+                });
             });
 
             AddStep("Clear state", () => setState(TaikoMascotAnimationState.Clear));
@@ -76,59 +68,25 @@ namespace osu.Game.Rulesets.Taiko.Tests.Skinning
             AddStep("Fail state", () => setState(TaikoMascotAnimationState.Fail));
         }
 
-        private void setState(TaikoMascotAnimationState state)
-        {
-            foreach (var mascot in mascots)
-            {
-                if (mascot == null)
-                    continue;
-
-                mascot.Dumb = true;
-                mascot.State = state;
-            }
-        }
-
-        private SkinnableDrawable getMascot() =>
-            new SkinnableDrawable(new TaikoSkinComponent(TaikoSkinComponents.TaikoDon), _ => new Container(), confineMode: ConfineMode.ScaleToFit)
-            {
-                RelativePositionAxes = Axes.Both
-            };
-
         [Test]
         public void TestPlayfield()
         {
-            AddStep("Create playfield", () =>
+            AddStep("Set beatmap", () => setBeatmap());
+
+            AddStep("Create ruleset", () =>
             {
-                playfields.Clear();
+                rulesets.Clear();
                 SetContents(() =>
                 {
-                    var playfield = new TaikoPlayfield(new ControlPointInfo())
-                    {
-                        Height = 0.4f,
-                        Anchor = Anchor.BottomLeft,
-                        Origin = Anchor.BottomLeft,
-                    };
-
-                    playfields.Add(playfield);
-
-                    return playfield;
+                    var ruleset = new TaikoRuleset();
+                    var drawableRuleset = new DrawableTaikoRuleset(ruleset, beatmap.Value.GetPlayableBeatmap(ruleset.RulesetInfo));
+                    rulesets.Add(drawableRuleset);
+                    return drawableRuleset;
                 });
             });
 
-            AddUntilStep("Wait for SkinnableDrawable", () => playfields.Any(p => p.ChildrenOfType<DrawableTaikoMascot>().Any()));
-
-            AddStep("Collect mascots", () =>
-            {
-                mascots.Clear();
-
-                foreach (var playfield in playfields)
-                {
-                    var mascot = playfield.ChildrenOfType<DrawableTaikoMascot>().SingleOrDefault();
-
-                    if (mascot != null)
-                        mascots.Add(mascot);
-                }
-            });
+            AddStep("Collect playfields", collectPlayfields);
+            AddStep("Collect mascots", collectMascots);
 
             AddStep("Create hit (miss)", () =>
             {
@@ -136,7 +94,7 @@ namespace osu.Game.Rulesets.Taiko.Tests.Skinning
                     addJudgement(playfield, HitResult.Miss);
             });
 
-            AddAssert("Check if state is fail", () => mascots.Where(d => d != null).All(d => d.PlayfieldState.Value == TaikoMascotAnimationState.Fail));
+            AddUntilStep("Wait for fail state", () => mascots.Where(d => d != null).All(d => d.State == TaikoMascotAnimationState.Fail));
 
             AddStep("Create hit (great)", () =>
             {
@@ -144,12 +102,111 @@ namespace osu.Game.Rulesets.Taiko.Tests.Skinning
                     addJudgement(playfield, HitResult.Great);
             });
 
-            AddAssert("Check if state is idle", () => mascots.Where(d => d != null).All(d => d.PlayfieldState.Value == TaikoMascotAnimationState.Idle));
+            AddUntilStep("Wait for idle state", () => mascots.Where(d => d != null).All(d => d.State == TaikoMascotAnimationState.Idle));
+        }
+
+        [Test]
+        public void TestKiai()
+        {
+            AddStep("Set beatmap", () => setBeatmap(true));
+
+            AddUntilStep("Wait for beatmap to be loaded", () => beatmap.Value.Track.IsLoaded);
+
+            AddStep("Create kiai ruleset", () =>
+            {
+                beatmap.Value.Track.Start();
+
+                rulesets.Clear();
+                SetContents(() =>
+                {
+                    var ruleset = new TaikoRuleset();
+                    var drawableRuleset = new DrawableTaikoRuleset(ruleset, beatmap.Value.GetPlayableBeatmap(ruleset.RulesetInfo));
+                    rulesets.Add(drawableRuleset);
+                    return drawableRuleset;
+                });
+            });
+
+            AddStep("Collect playfields", collectPlayfields);
+            AddStep("Collect mascots", collectMascots);
+
+            AddUntilStep("Wait for fail state", () => mascots.Where(d => d != null).All(d => d.State == TaikoMascotAnimationState.Fail));
+
+            AddStep("Create hit (great)", () =>
+            {
+                foreach (var playfield in playfields)
+                    addJudgement(playfield, HitResult.Great);
+            });
+
+            AddUntilStep("Wait for kiai state", () => mascots.Where(d => d != null).All(d => d.State == TaikoMascotAnimationState.Kiai));
+        }
+
+        private void setBeatmap(bool kiai = false)
+        {
+            var controlPointInfo = new ControlPointInfo();
+            controlPointInfo.Add(0, new TimingControlPoint { BeatLength = 90 });
+
+            if (kiai)
+                controlPointInfo.Add(0, new EffectControlPoint { KiaiMode = true });
+
+            beatmap.Value = CreateWorkingBeatmap(new Beatmap
+            {
+                HitObjects = new List<HitObject> { new Hit { Type = HitType.Centre } },
+                BeatmapInfo = new BeatmapInfo
+                {
+                    BaseDifficulty = new BeatmapDifficulty(),
+                    Metadata = new BeatmapMetadata
+                    {
+                        Artist = @"Unknown",
+                        Title = @"Sample Beatmap",
+                        AuthorString = @"Craftplacer",
+                    },
+                    Ruleset = new TaikoRuleset().RulesetInfo
+                },
+                ControlPointInfo = controlPointInfo
+            });
+        }
+
+        private void setState(TaikoMascotAnimationState state)
+        {
+            foreach (var mascot in mascots)
+                mascot?.ShowState(state);
+        }
+
+        private void collectPlayfields()
+        {
+            playfields.Clear();
+            foreach (var ruleset in rulesets) playfields.Add(ruleset.ChildrenOfType<TaikoPlayfield>().Single());
+        }
+
+        private void collectMascots()
+        {
+            mascots.Clear();
+
+            foreach (var playfield in playfields)
+            {
+                var mascot = playfield.ChildrenOfType<DrawableTaikoMascot>()
+                                      .SingleOrDefault();
+
+                if (mascot != null) mascots.Add(mascot);
+            }
         }
 
         private void addJudgement(TaikoPlayfield playfield, HitResult result)
         {
             playfield.OnNewResult(new DrawableRimHit(new Hit()), new JudgementResult(new HitObject(), new TaikoJudgement()) { Type = result });
         }
+
+        private class TestDrawableTaikoMascot : DrawableTaikoMascot
+        {
+            public TestDrawableTaikoMascot(TaikoMascotAnimationState startingState = TaikoMascotAnimationState.Idle)
+                : base(startingState)
+            {
+            }
+
+            protected override TaikoMascotAnimationState GetFinalAnimationState(EffectControlPoint effectPoint, TaikoMascotAnimationState playfieldState)
+            {
+                return State;
+            }
+        }
     }
 }

osu.Game.Rulesets.Taiko/UI/DrawableTaikoMascot.cs

@@ -1,6 +1,7 @@
 // Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
 // See the LICENCE file in the repository root for full licence text.
 
+using System;
 using osu.Framework.Allocation;
 using osu.Framework.Audio.Track;
 using osu.Framework.Bindables;
@@ -11,68 +12,29 @@ using osu.Game.Graphics.Containers;
 
 namespace osu.Game.Rulesets.Taiko.UI
 {
-    public sealed class DrawableTaikoMascot : BeatSyncedContainer
+    public class DrawableTaikoMascot : BeatSyncedContainer
     {
-        private static TaikoMascotTextureAnimation idleDrawable, clearDrawable, kiaiDrawable, failDrawable;
+        private TaikoMascotTextureAnimation idleDrawable, clearDrawable, kiaiDrawable, failDrawable;
         private EffectControlPoint lastEffectControlPoint;
-        private TaikoMascotAnimationState state;
 
         public Bindable<TaikoMascotAnimationState> PlayfieldState;
 
-        /// <summary>
-        /// Determines if there should be no "state logic", intended for testing.
-        /// </summary>
-        public bool Dumb { get; set; }
-
-        public TaikoMascotAnimationState State
-        {
-            get => state;
-            set
-            {
-                state = value;
-
-                foreach (var child in InternalChildren)
-                    child.Hide();
-
-                var drawable = getStateDrawable(State);
-
-                drawable?.Show();
-            }
-        }
+        public TaikoMascotAnimationState State { get; private set; }
 
         public DrawableTaikoMascot(TaikoMascotAnimationState startingState = TaikoMascotAnimationState.Idle)
         {
             RelativeSizeAxes = Axes.Both;
+
             PlayfieldState = new Bindable<TaikoMascotAnimationState>();
             PlayfieldState.BindValueChanged(b =>
             {
                 if (lastEffectControlPoint != null)
-                    State = getFinalAnimationState(lastEffectControlPoint, b.NewValue);
+                    ShowState(GetFinalAnimationState(lastEffectControlPoint, b.NewValue));
             });
 
             State = startingState;
         }
 
-        private TaikoMascotTextureAnimation getStateDrawable(TaikoMascotAnimationState state)
-        {
-            return state switch
-            {
-                TaikoMascotAnimationState.Idle => idleDrawable,
-                TaikoMascotAnimationState.Clear => clearDrawable,
-                TaikoMascotAnimationState.Kiai => kiaiDrawable,
-                TaikoMascotAnimationState.Fail => failDrawable,
-                _ => null
-            };
-        }
-
-        private TaikoMascotAnimationState getFinalAnimationState(EffectControlPoint effectPoint, TaikoMascotAnimationState playfieldState)
-        {
-            if (playfieldState == TaikoMascotAnimationState.Fail)
-                return playfieldState;
-
-            return effectPoint.KiaiMode ? TaikoMascotAnimationState.Kiai : TaikoMascotAnimationState.Idle;
-        }
-
         [BackgroundDependencyLoader]
         private void load(TextureStore textures)
         {
@@ -84,21 +46,60 @@ namespace osu.Game.Rulesets.Taiko.UI
                 failDrawable = new TaikoMascotTextureAnimation(TaikoMascotAnimationState.Fail),
             };
 
-            // making sure we have the correct sprite set
+            ShowState(State);
+        }
+
+        public void ShowState(TaikoMascotAnimationState state)
+        {
+            foreach (var child in InternalChildren)
+                child.Hide();
+
             State = state;
+
+            var drawable = getStateDrawable(State);
+            drawable.Show();
+        }
+
+        private TaikoMascotTextureAnimation getStateDrawable(TaikoMascotAnimationState state)
+        {
+            switch (state)
+            {
+                case TaikoMascotAnimationState.Idle:
+                    return idleDrawable;
+
+                case TaikoMascotAnimationState.Clear:
+                    return clearDrawable;
+
+                case TaikoMascotAnimationState.Kiai:
+                    return kiaiDrawable;
+
+                case TaikoMascotAnimationState.Fail:
+                    return failDrawable;
+
+                default:
+                    throw new ArgumentException($"There's no case for animation state ${state} available", nameof(state));
+            }
+        }
+
+        protected virtual TaikoMascotAnimationState GetFinalAnimationState(EffectControlPoint effectPoint, TaikoMascotAnimationState playfieldState)
+        {
+            if (playfieldState == TaikoMascotAnimationState.Fail)
+                return playfieldState;
+
+            return effectPoint.KiaiMode ? TaikoMascotAnimationState.Kiai : TaikoMascotAnimationState.Idle;
         }
 
         protected override void OnNewBeat(int beatIndex, TimingControlPoint timingPoint, EffectControlPoint effectPoint, TrackAmplitudes amplitudes)
         {
             base.OnNewBeat(beatIndex, timingPoint, effectPoint, amplitudes);
 
-            if (!Dumb)
-                State = getFinalAnimationState(lastEffectControlPoint = effectPoint, PlayfieldState.Value);
+            var state = GetFinalAnimationState(lastEffectControlPoint = effectPoint, PlayfieldState.Value);
+            ShowState(state);
 
-            if (State == TaikoMascotAnimationState.Clear)
+            if (state == TaikoMascotAnimationState.Clear)
                 return;
 
-            var drawable = getStateDrawable(State);
+            var drawable = getStateDrawable(state);
             drawable.Move();
         }
     }

osu.Game.Rulesets.Taiko/UI/TaikoMascotTextureAnimation.cs

@@ -1,6 +1,7 @@
 // Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
 // See the LICENCE file in the repository root for full licence text.
 
+using System;
 using osu.Framework.Allocation;
 using osu.Framework.Graphics;
 using osu.Framework.Graphics.Animations;
@@ -39,7 +40,7 @@ namespace osu.Game.Rulesets.Taiko.UI
             {
                 foreach (var textureIndex in clear_animation_sequence)
                 {
-                    var textureName = _getStateTextureName(textureIndex);
+                    var textureName = getStateTextureName(textureIndex);
                     Texture texture = skin.GetTexture(textureName);
 
                     if (texture == null)
@@ -52,7 +53,7 @@ namespace osu.Game.Rulesets.Taiko.UI
             {
                 for (int i = 0; true; i++)
                 {
-                    var textureName = _getStateTextureName(i);
+                    var textureName = getStateTextureName(i);
                     Texture texture = skin.GetTexture(textureName);
 
                     if (texture == null)
@@ -63,10 +64,13 @@ namespace osu.Game.Rulesets.Taiko.UI
             }
         }
 
-        /// <summary>Advances the current frame by one.</summary>
+        /// <summary>
+        /// Advances the current frame by one.
+        /// </summary>
         public void Move()
         {
-            if (FrameCount == 0) // Frames are apparently broken
+            // Check whether there are frames before causing a crash.
+            if (FrameCount == 0)
                 return;
 
             if (FrameCount <= currentFrame)
@@ -77,18 +81,27 @@ namespace osu.Game.Rulesets.Taiko.UI
             currentFrame += 1;
         }
 
-        private string _getStateTextureName(int i) => $"pippidon{_getStateString(State)}{i}";
+        private string getStateTextureName(int i) => $"pippidon{getStateString(State)}{i}";
 
-        private string _getStateString(TaikoMascotAnimationState state)
+        private string getStateString(TaikoMascotAnimationState state)
         {
-            return state switch
+            switch (state)
             {
-                TaikoMascotAnimationState.Clear => "clear",
-                TaikoMascotAnimationState.Fail => "fail",
-                TaikoMascotAnimationState.Idle => "idle",
-                TaikoMascotAnimationState.Kiai => "kiai",
-                _ => null
-            };
+                case TaikoMascotAnimationState.Clear:
+                    return "clear";
+
+                case TaikoMascotAnimationState.Fail:
+                    return "fail";
+
+                case TaikoMascotAnimationState.Idle:
+                    return "idle";
+
+                case TaikoMascotAnimationState.Kiai:
+                    return "kiai";
+
+                default:
+                    throw new ArgumentException($"There's no case for animation state ${state} available", nameof(state));
+            }
         }
     }
 }

osu.Game.Rulesets.Taiko/UI/TaikoPlayfield.cs

@@ -282,15 +282,15 @@ namespace osu.Game.Rulesets.Taiko.UI
                 mascot.PlayfieldState.Value = isFailing ? TaikoMascotAnimationState.Fail : TaikoMascotAnimationState.Idle;
             }
         }
-    }
 
-    internal class ProxyContainer : LifetimeManagementContainer
-    {
-        public new MarginPadding Padding
+        private class ProxyContainer : LifetimeManagementContainer
         {
-            set => base.Padding = value;
-        }
+            public new MarginPadding Padding
+            {
+                set => base.Padding = value;
+            }
 
-        public void Add(Drawable proxy) => AddInternal(proxy);
+            public void Add(Drawable proxy) => AddInternal(proxy);
+        }
     }
 }