Merge pull request #29564 from peppy/skinnable-components-what-2

Remove requirement of `base` calls to ensure user skin container layouts are retrieved
ppy · Aug 23, 2024 · 60271fb · 60271fb
2 parents bd6943e + 48cfd77
commit 60271fb
Show file tree

Hide file tree

Showing 41 changed files with 209 additions and 223 deletions.
diff --git a/osu.Game.Rulesets.Catch.Tests/TestSceneCatchPlayerLegacySkin.cs b/osu.Game.Rulesets.Catch.Tests/TestSceneCatchPlayerLegacySkin.cs
@@ -23,7 +23,7 @@ public void TestLegacyHUDComboCounterNotExistent([Values] bool withModifiedSkin)
             if (withModifiedSkin)
             {
                 AddStep("change component scale", () => Player.ChildrenOfType<LegacyScoreCounter>().First().Scale = new Vector2(2f));
-                AddStep("update target", () => Player.ChildrenOfType<SkinComponentsContainer>().ForEach(LegacySkin.UpdateDrawableTarget));
+                AddStep("update target", () => Player.ChildrenOfType<SkinnableContainer>().ForEach(LegacySkin.UpdateDrawableTarget));
                 AddStep("exit player", () => Player.Exit());
                 CreateTest();
             }

diff --git a/osu.Game.Rulesets.Catch/CatchSkinComponentLookup.cs b/osu.Game.Rulesets.Catch/CatchSkinComponentLookup.cs
@@ -5,7 +5,7 @@
 
 namespace osu.Game.Rulesets.Catch
 {
-    public class CatchSkinComponentLookup : GameplaySkinComponentLookup<CatchSkinComponents>
+    public class CatchSkinComponentLookup : SkinComponentLookup<CatchSkinComponents>
     {
         public CatchSkinComponentLookup(CatchSkinComponents component)
             : base(component)

diff --git a/osu.Game.Rulesets.Catch/Skinning/Legacy/CatchLegacySkinTransformer.cs b/osu.Game.Rulesets.Catch/Skinning/Legacy/CatchLegacySkinTransformer.cs
@@ -30,23 +30,19 @@ public CatchLegacySkinTransformer(ISkin skin)
         {
             switch (lookup)
             {
-                case SkinComponentsContainerLookup containerLookup:
+                case GlobalSkinnableContainerLookup containerLookup:
                     // Only handle per ruleset defaults here.
                     if (containerLookup.Ruleset == null)
                         return base.GetDrawableComponent(lookup);
 
-                    // Skin has configuration.
-                    if (base.GetDrawableComponent(lookup) is UserConfiguredLayoutContainer d)
-                        return d;
-
                     // we don't have enough assets to display these components (this is especially the case on a "beatmap" skin).
                     if (!IsProvidingLegacyResources)
                         return null;
 
                     // Our own ruleset components default.
-                    switch (containerLookup.Target)
+                    switch (containerLookup.Lookup)
                     {
-                        case SkinComponentsContainerLookup.TargetArea.MainHUDComponents:
+                        case GlobalSkinnableContainers.MainHUDComponents:
                             // todo: remove CatchSkinComponents.CatchComboCounter and refactor LegacyCatchComboCounter to be added here instead.
                             return new DefaultSkinComponentsContainer(container =>
                             {

diff --git a/osu.Game.Rulesets.Mania/ManiaSkinComponentLookup.cs b/osu.Game.Rulesets.Mania/ManiaSkinComponentLookup.cs
@@ -5,7 +5,7 @@
 
 namespace osu.Game.Rulesets.Mania
 {
-    public class ManiaSkinComponentLookup : GameplaySkinComponentLookup<ManiaSkinComponents>
+    public class ManiaSkinComponentLookup : SkinComponentLookup<ManiaSkinComponents>
     {
         /// <summary>
         /// Creates a new <see cref="ManiaSkinComponentLookup"/>.

diff --git a/osu.Game.Rulesets.Mania/Skinning/Argon/ManiaArgonSkinTransformer.cs b/osu.Game.Rulesets.Mania/Skinning/Argon/ManiaArgonSkinTransformer.cs
@@ -28,18 +28,14 @@ public ManiaArgonSkinTransformer(ISkin skin, IBeatmap beatmap)
         {
             switch (lookup)
             {
-                case SkinComponentsContainerLookup containerLookup:
+                case GlobalSkinnableContainerLookup containerLookup:
                     // Only handle per ruleset defaults here.
                     if (containerLookup.Ruleset == null)
                         return base.GetDrawableComponent(lookup);
 
-                    // Skin has configuration.
-                    if (base.GetDrawableComponent(lookup) is UserConfiguredLayoutContainer d)
-                        return d;
-
-                    switch (containerLookup.Target)
+                    switch (containerLookup.Lookup)
                     {
-                        case SkinComponentsContainerLookup.TargetArea.MainHUDComponents:
+                        case GlobalSkinnableContainers.MainHUDComponents:
                             return new DefaultSkinComponentsContainer(container =>
                             {
                                 var combo = container.ChildrenOfType<ArgonManiaComboCounter>().FirstOrDefault();
@@ -59,7 +55,7 @@ public ManiaArgonSkinTransformer(ISkin skin, IBeatmap beatmap)
 
                     return null;
 
-                case GameplaySkinComponentLookup<HitResult> resultComponent:
+                case SkinComponentLookup<HitResult> resultComponent:
                     // This should eventually be moved to a skin setting, when supported.
                     if (Skin is ArgonProSkin && resultComponent.Component >= HitResult.Great)
                         return Drawable.Empty();

diff --git a/osu.Game.Rulesets.Mania/Skinning/Legacy/ManiaLegacySkinTransformer.cs b/osu.Game.Rulesets.Mania/Skinning/Legacy/ManiaLegacySkinTransformer.cs
@@ -80,22 +80,18 @@ public override Drawable GetDrawableComponent(ISkinComponentLookup lookup)
         {
             switch (lookup)
             {
-                case SkinComponentsContainerLookup containerLookup:
+                case GlobalSkinnableContainerLookup containerLookup:
                     // Modifications for global components.
                     if (containerLookup.Ruleset == null)
                         return base.GetDrawableComponent(lookup);
 
-                    // Skin has configuration.
-                    if (base.GetDrawableComponent(lookup) is UserConfiguredLayoutContainer d)
-                        return d;
-
                     // we don't have enough assets to display these components (this is especially the case on a "beatmap" skin).
                     if (!IsProvidingLegacyResources)
                         return null;
 
-                    switch (containerLookup.Target)
+                    switch (containerLookup.Lookup)
                     {
-                        case SkinComponentsContainerLookup.TargetArea.MainHUDComponents:
+                        case GlobalSkinnableContainers.MainHUDComponents:
                             return new DefaultSkinComponentsContainer(container =>
                             {
                                 var combo = container.ChildrenOfType<LegacyManiaComboCounter>().FirstOrDefault();
@@ -114,7 +110,7 @@ public override Drawable GetDrawableComponent(ISkinComponentLookup lookup)
 
                     return null;
 
-                case GameplaySkinComponentLookup<HitResult> resultComponent:
+                case SkinComponentLookup<HitResult> resultComponent:
                     return getResult(resultComponent.Component);
 
                 case ManiaSkinComponentLookup maniaComponent:

diff --git a/osu.Game.Rulesets.Osu/OsuSkinComponentLookup.cs b/osu.Game.Rulesets.Osu/OsuSkinComponentLookup.cs
@@ -5,7 +5,7 @@
 
 namespace osu.Game.Rulesets.Osu
 {
-    public class OsuSkinComponentLookup : GameplaySkinComponentLookup<OsuSkinComponents>
+    public class OsuSkinComponentLookup : SkinComponentLookup<OsuSkinComponents>
     {
         public OsuSkinComponentLookup(OsuSkinComponents component)
             : base(component)

diff --git a/osu.Game.Rulesets.Osu/Skinning/Argon/OsuArgonSkinTransformer.cs b/osu.Game.Rulesets.Osu/Skinning/Argon/OsuArgonSkinTransformer.cs
@@ -18,7 +18,7 @@ public OsuArgonSkinTransformer(ISkin skin)
         {
             switch (lookup)
             {
-                case GameplaySkinComponentLookup<HitResult> resultComponent:
+                case SkinComponentLookup<HitResult> resultComponent:
                     HitResult result = resultComponent.Component;
 
                     // This should eventually be moved to a skin setting, when supported.

diff --git a/osu.Game.Rulesets.Osu/Skinning/Default/OsuTrianglesSkinTransformer.cs b/osu.Game.Rulesets.Osu/Skinning/Default/OsuTrianglesSkinTransformer.cs
@@ -18,7 +18,7 @@ public OsuTrianglesSkinTransformer(ISkin skin)
         {
             switch (lookup)
             {
-                case GameplaySkinComponentLookup<HitResult> resultComponent:
+                case SkinComponentLookup<HitResult> resultComponent:
                     HitResult result = resultComponent.Component;
 
                     switch (result)

diff --git a/osu.Game.Rulesets.Osu/Skinning/Legacy/OsuLegacySkinTransformer.cs b/osu.Game.Rulesets.Osu/Skinning/Legacy/OsuLegacySkinTransformer.cs
@@ -44,23 +44,19 @@ public OsuLegacySkinTransformer(ISkin skin)
         {
             switch (lookup)
             {
-                case SkinComponentsContainerLookup containerLookup:
+                case GlobalSkinnableContainerLookup containerLookup:
                     // Only handle per ruleset defaults here.
                     if (containerLookup.Ruleset == null)
                         return base.GetDrawableComponent(lookup);
 
-                    // Skin has configuration.
-                    if (base.GetDrawableComponent(lookup) is UserConfiguredLayoutContainer d)
-                        return d;
-
                     // we don't have enough assets to display these components (this is especially the case on a "beatmap" skin).
                     if (!IsProvidingLegacyResources)
                         return null;
 
                     // Our own ruleset components default.
-                    switch (containerLookup.Target)
+                    switch (containerLookup.Lookup)
                     {
-                        case SkinComponentsContainerLookup.TargetArea.MainHUDComponents:
+                        case GlobalSkinnableContainers.MainHUDComponents:
                             return new DefaultSkinComponentsContainer(container =>
                             {
                                 var keyCounter = container.OfType<LegacyKeyCounterDisplay>().FirstOrDefault();

diff --git a/osu.Game.Rulesets.Taiko/Skinning/Argon/TaikoArgonSkinTransformer.cs b/osu.Game.Rulesets.Taiko/Skinning/Argon/TaikoArgonSkinTransformer.cs
@@ -18,7 +18,7 @@ public TaikoArgonSkinTransformer(ISkin skin)
         {
             switch (lookup)
             {
-                case GameplaySkinComponentLookup<HitResult> resultComponent:
+                case SkinComponentLookup<HitResult> resultComponent:
                     // This should eventually be moved to a skin setting, when supported.
                     if (Skin is ArgonProSkin && resultComponent.Component >= HitResult.Great)
                         return Drawable.Empty();

diff --git a/osu.Game.Rulesets.Taiko/Skinning/Legacy/TaikoLegacySkinTransformer.cs b/osu.Game.Rulesets.Taiko/Skinning/Legacy/TaikoLegacySkinTransformer.cs
@@ -29,7 +29,7 @@ public TaikoLegacySkinTransformer(ISkin skin)
 
         public override Drawable? GetDrawableComponent(ISkinComponentLookup lookup)
         {
-            if (lookup is GameplaySkinComponentLookup<HitResult>)
+            if (lookup is SkinComponentLookup<HitResult>)
             {
                 // if a taiko skin is providing explosion sprites, hide the judgements completely
                 if (hasExplosion.Value)

diff --git a/osu.Game.Rulesets.Taiko/TaikoSkinComponentLookup.cs b/osu.Game.Rulesets.Taiko/TaikoSkinComponentLookup.cs
@@ -5,7 +5,7 @@
 
 namespace osu.Game.Rulesets.Taiko
 {
-    public class TaikoSkinComponentLookup : GameplaySkinComponentLookup<TaikoSkinComponents>
+    public class TaikoSkinComponentLookup : SkinComponentLookup<TaikoSkinComponents>
     {
         public TaikoSkinComponentLookup(TaikoSkinComponents component)
             : base(component)

diff --git a/osu.Game.Tests/Skins/SkinDeserialisationTest.cs b/osu.Game.Tests/Skins/SkinDeserialisationTest.cs
@@ -107,7 +107,7 @@ public void TestDeserialiseModifiedDefault()
                 var skin = new TestSkin(new SkinInfo(), null, storage);
 
                 Assert.That(skin.LayoutInfos, Has.Count.EqualTo(2));
-                Assert.That(skin.LayoutInfos[SkinComponentsContainerLookup.TargetArea.MainHUDComponents].AllDrawables.ToArray(), Has.Length.EqualTo(9));
+                Assert.That(skin.LayoutInfos[GlobalSkinnableContainers.MainHUDComponents].AllDrawables.ToArray(), Has.Length.EqualTo(9));
             }
         }
 
@@ -120,8 +120,8 @@ public void TestDeserialiseModifiedArgon()
                 var skin = new TestSkin(new SkinInfo(), null, storage);
 
                 Assert.That(skin.LayoutInfos, Has.Count.EqualTo(2));
-                Assert.That(skin.LayoutInfos[SkinComponentsContainerLookup.TargetArea.MainHUDComponents].AllDrawables.ToArray(), Has.Length.EqualTo(10));
-                Assert.That(skin.LayoutInfos[SkinComponentsContainerLookup.TargetArea.MainHUDComponents].AllDrawables.Select(i => i.Type), Contains.Item(typeof(PlayerName)));
+                Assert.That(skin.LayoutInfos[GlobalSkinnableContainers.MainHUDComponents].AllDrawables.ToArray(), Has.Length.EqualTo(10));
+                Assert.That(skin.LayoutInfos[GlobalSkinnableContainers.MainHUDComponents].AllDrawables.Select(i => i.Type), Contains.Item(typeof(PlayerName)));
             }
         }
 
@@ -134,10 +134,10 @@ public void TestDeserialiseModifiedClassic()
                 var skin = new TestSkin(new SkinInfo(), null, storage);
 
                 Assert.That(skin.LayoutInfos, Has.Count.EqualTo(2));
-                Assert.That(skin.LayoutInfos[SkinComponentsContainerLookup.TargetArea.MainHUDComponents].AllDrawables.ToArray(), Has.Length.EqualTo(6));
-                Assert.That(skin.LayoutInfos[SkinComponentsContainerLookup.TargetArea.SongSelect].AllDrawables.ToArray(), Has.Length.EqualTo(1));
+                Assert.That(skin.LayoutInfos[GlobalSkinnableContainers.MainHUDComponents].AllDrawables.ToArray(), Has.Length.EqualTo(6));
+                Assert.That(skin.LayoutInfos[GlobalSkinnableContainers.SongSelect].AllDrawables.ToArray(), Has.Length.EqualTo(1));
 
-                var skinnableInfo = skin.LayoutInfos[SkinComponentsContainerLookup.TargetArea.SongSelect].AllDrawables.First();
+                var skinnableInfo = skin.LayoutInfos[GlobalSkinnableContainers.SongSelect].AllDrawables.First();
 
                 Assert.That(skinnableInfo.Type, Is.EqualTo(typeof(SkinnableSprite)));
                 Assert.That(skinnableInfo.Settings.First().Key, Is.EqualTo("sprite_name"));
@@ -148,10 +148,10 @@ public void TestDeserialiseModifiedClassic()
             using (var storage = new ZipArchiveReader(stream))
             {
                 var skin = new TestSkin(new SkinInfo(), null, storage);
-                Assert.That(skin.LayoutInfos[SkinComponentsContainerLookup.TargetArea.MainHUDComponents].AllDrawables.ToArray(), Has.Length.EqualTo(8));
-                Assert.That(skin.LayoutInfos[SkinComponentsContainerLookup.TargetArea.MainHUDComponents].AllDrawables.Select(i => i.Type), Contains.Item(typeof(UnstableRateCounter)));
-                Assert.That(skin.LayoutInfos[SkinComponentsContainerLookup.TargetArea.MainHUDComponents].AllDrawables.Select(i => i.Type), Contains.Item(typeof(ColourHitErrorMeter)));
-                Assert.That(skin.LayoutInfos[SkinComponentsContainerLookup.TargetArea.MainHUDComponents].AllDrawables.Select(i => i.Type), Contains.Item(typeof(LegacySongProgress)));
+                Assert.That(skin.LayoutInfos[GlobalSkinnableContainers.MainHUDComponents].AllDrawables.ToArray(), Has.Length.EqualTo(8));
+                Assert.That(skin.LayoutInfos[GlobalSkinnableContainers.MainHUDComponents].AllDrawables.Select(i => i.Type), Contains.Item(typeof(UnstableRateCounter)));
+                Assert.That(skin.LayoutInfos[GlobalSkinnableContainers.MainHUDComponents].AllDrawables.Select(i => i.Type), Contains.Item(typeof(ColourHitErrorMeter)));
+                Assert.That(skin.LayoutInfos[GlobalSkinnableContainers.MainHUDComponents].AllDrawables.Select(i => i.Type), Contains.Item(typeof(LegacySongProgress)));
             }
         }
 

diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneBeatmapSkinFallbacks.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneBeatmapSkinFallbacks.cs
@@ -37,8 +37,8 @@ public partial class TestSceneBeatmapSkinFallbacks : OsuPlayerTestScene
         public void TestEmptyLegacyBeatmapSkinFallsBack()
         {
             CreateSkinTest(TrianglesSkin.CreateInfo(), () => new LegacyBeatmapSkin(new BeatmapInfo(), null));
-            AddUntilStep("wait for hud load", () => Player.ChildrenOfType<SkinComponentsContainer>().All(c => c.ComponentsLoaded));
-            AddAssert("hud from default skin", () => AssertComponentsFromExpectedSource(SkinComponentsContainerLookup.TargetArea.MainHUDComponents, skinManager.CurrentSkin.Value));
+            AddUntilStep("wait for hud load", () => Player.ChildrenOfType<SkinnableContainer>().All(c => c.ComponentsLoaded));
+            AddAssert("hud from default skin", () => AssertComponentsFromExpectedSource(GlobalSkinnableContainers.MainHUDComponents, skinManager.CurrentSkin.Value));
         }
 
         protected void CreateSkinTest(SkinInfo gameCurrentSkin, Func<ISkin> getBeatmapSkin)
@@ -53,17 +53,17 @@ protected void CreateSkinTest(SkinInfo gameCurrentSkin, Func<ISkin> getBeatmapSk
             });
         }
 
-        protected bool AssertComponentsFromExpectedSource(SkinComponentsContainerLookup.TargetArea target, ISkin expectedSource)
+        protected bool AssertComponentsFromExpectedSource(GlobalSkinnableContainers target, ISkin expectedSource)
         {
-            var targetContainer = Player.ChildrenOfType<SkinComponentsContainer>().First(s => s.Lookup.Target == target);
+            var targetContainer = Player.ChildrenOfType<SkinnableContainer>().First(s => s.Lookup.Lookup == target);
             var actualComponentsContainer = targetContainer.ChildrenOfType<Container>().SingleOrDefault(c => c.Parent == targetContainer);
 
             if (actualComponentsContainer == null)
                 return false;
 
             var actualInfo = actualComponentsContainer.CreateSerialisedInfo();
 
-            var expectedComponentsContainer = expectedSource.GetDrawableComponent(new SkinComponentsContainerLookup(target)) as Container;
+            var expectedComponentsContainer = expectedSource.GetDrawableComponent(new GlobalSkinnableContainerLookup(target)) as Container;
             if (expectedComponentsContainer == null)
                 return false;
 

diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneHUDOverlay.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneHUDOverlay.cs
@@ -43,7 +43,7 @@ public partial class TestSceneHUDOverlay : OsuManualInputManagerTestScene
         private readonly IGameplayClock gameplayClock = new GameplayClockContainer(new TrackVirtual(60000), false, false);
 
         // best way to check without exposing.
-        private Drawable hideTarget => hudOverlay.ChildrenOfType<SkinComponentsContainer>().First();
+        private Drawable hideTarget => hudOverlay.ChildrenOfType<SkinnableContainer>().First();
         private Drawable keyCounterContent => hudOverlay.ChildrenOfType<KeyCounterDisplay>().First().ChildrenOfType<Drawable>().Skip(1).First();
 
         public TestSceneHUDOverlay()
@@ -242,8 +242,8 @@ public void TestHiddenHUDDoesntBlockComponentUpdates()
 
             createNew();
 
-            AddUntilStep("wait for components to be hidden", () => hudOverlay.ChildrenOfType<SkinComponentsContainer>().Single().Alpha == 0);
-            AddUntilStep("wait for hud load", () => hudOverlay.ChildrenOfType<SkinComponentsContainer>().All(c => c.ComponentsLoaded));
+            AddUntilStep("wait for components to be hidden", () => hudOverlay.ChildrenOfType<SkinnableContainer>().Single().Alpha == 0);
+            AddUntilStep("wait for hud load", () => hudOverlay.ChildrenOfType<SkinnableContainer>().All(c => c.ComponentsLoaded));
 
             AddStep("bind on update", () =>
             {
@@ -260,10 +260,10 @@ public void TestHiddenHUDDoesntBlockSkinnableComponentsLoad()
 
             createNew();
 
-            AddUntilStep("wait for components to be hidden", () => hudOverlay.ChildrenOfType<SkinComponentsContainer>().Single().Alpha == 0);
+            AddUntilStep("wait for components to be hidden", () => hudOverlay.ChildrenOfType<SkinnableContainer>().Single().Alpha == 0);
 
-            AddStep("reload components", () => hudOverlay.ChildrenOfType<SkinComponentsContainer>().Single().Reload());
-            AddUntilStep("skinnable components loaded", () => hudOverlay.ChildrenOfType<SkinComponentsContainer>().Single().ComponentsLoaded);
+            AddStep("reload components", () => hudOverlay.ChildrenOfType<SkinnableContainer>().Single().Reload());
+            AddUntilStep("skinnable components loaded", () => hudOverlay.ChildrenOfType<SkinnableContainer>().Single().ComponentsLoaded);
         }
 
         private void createNew(Action<HUDOverlay>? action = null)

diff --git a/osu.Game.Tests/Visual/Gameplay/TestScenePauseInputHandling.cs b/osu.Game.Tests/Visual/Gameplay/TestScenePauseInputHandling.cs
@@ -286,7 +286,7 @@ private void loadPlayer(Func<Ruleset> createRuleset)
             AddStep("set ruleset", () => currentRuleset = createRuleset());
             AddStep("load player", LoadPlayer);
             AddUntilStep("player loaded", () => Player.IsLoaded && Player.Alpha == 1);
-            AddUntilStep("wait for hud", () => Player.HUDOverlay.ChildrenOfType<SkinComponentsContainer>().All(s => s.ComponentsLoaded));
+            AddUntilStep("wait for hud", () => Player.HUDOverlay.ChildrenOfType<SkinnableContainer>().All(s => s.ComponentsLoaded));
 
             AddStep("seek to gameplay", () => Player.GameplayClockContainer.Seek(0));
             AddUntilStep("wait for seek to finish", () => Player.DrawableRuleset.FrameStableClock.CurrentTime, () => Is.EqualTo(0).Within(500));