Merge branch 'legacy-skin-default-fallback' into fix-skin-sample-lookup

osu.Game.Rulesets.Osu/Skinning/Legacy/LegacySliderBall.cs

@@ -14,18 +14,21 @@ namespace osu.Game.Rulesets.Osu.Skinning.Legacy
     {
         private readonly Drawable animationContent;
 
+        private readonly ISkin skin;
+
         private Sprite layerNd;
         private Sprite layerSpec;
 
-        public LegacySliderBall(Drawable animationContent)
+        public LegacySliderBall(Drawable animationContent, ISkin skin)
         {
             this.animationContent = animationContent;
+            this.skin = skin;
 
             AutoSizeAxes = Axes.Both;
         }
 
         [BackgroundDependencyLoader]
-        private void load(ISkinSource skin)
+        private void load()
         {
             var ballColour = skin.GetConfig<OsuSkinColour, Color4>(OsuSkinColour.SliderBall)?.Value ?? Color4.White;
 

osu.Game.Rulesets.Osu/Skinning/Legacy/OsuLegacySkinTransformer.cs

@@ -49,13 +49,16 @@ namespace osu.Game.Rulesets.Osu.Skinning.Legacy
                         return followCircle;
 
                     case OsuSkinComponents.SliderBall:
-                        var sliderBallContent = this.GetAnimation("sliderb", true, true, animationSeparator: "");
+                        // specular and nd layers must come from the same source as the ball texure.
+                        var ballProvider = Source.FindProvider(s => s.GetTexture("sliderb") != null || s.GetTexture("sliderb0") != null);
+
+                        var sliderBallContent = ballProvider.GetAnimation("sliderb", true, true, animationSeparator: "");
 
                         // todo: slider ball has a custom frame delay based on velocity
                         // Math.Max((150 / Velocity) * GameBase.SIXTY_FRAME_TIME, GameBase.SIXTY_FRAME_TIME);
 
                         if (sliderBallContent != null)
-                            return new LegacySliderBall(sliderBallContent);
+                            return new LegacySliderBall(sliderBallContent, ballProvider);
 
                         return null;
 

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

@@ -85,8 +85,12 @@ namespace osu.Game.Rulesets.Taiko.UI
             }
 
             [BackgroundDependencyLoader]
-            private void load(ISkinSource skin)
+            private void load(ISkinSource source)
             {
+                ISkin skin = source.FindProvider(s => getAnimationFrame(s, state, 0) != null);
+
+                if (skin == null) return;
+
                 for (int frameIndex = 0; true; frameIndex++)
                 {
                     var texture = getAnimationFrame(skin, state, frameIndex);
@@ -112,8 +116,12 @@ namespace osu.Game.Rulesets.Taiko.UI
             }
 
             [BackgroundDependencyLoader]
-            private void load(ISkinSource skin)
+            private void load(ISkinSource source)
             {
+                ISkin skin = source.FindProvider(s => getAnimationFrame(s, TaikoMascotAnimationState.Clear, 0) != null);
+
+                if (skin == null) return;
+
                 foreach (var frameIndex in clear_animation_sequence)
                 {
                     var texture = getAnimationFrame(skin, TaikoMascotAnimationState.Clear, frameIndex);

osu.Game/Skinning/DefaultLegacySkin.cs

@@ -31,8 +31,6 @@ namespace osu.Game.Skinning
             Configuration.LegacyVersion = 2.7m;
         }
 
-        protected override DefaultLegacySkin CreateFallbackSkin(IResourceStore<byte[]> storage, IStorageResourceProvider resources) => null;
-
         public static SkinInfo Info { get; } = new SkinInfo
         {
             ID = SkinInfo.CLASSIC_SKIN, // this is temporary until database storage is decided upon.

osu.Game/Skinning/LegacyBeatmapSkin.cs

@@ -70,14 +70,6 @@ namespace osu.Game.Skinning
             return base.GetSample(sampleInfo);
         }
 
-        protected override DefaultLegacySkin CreateFallbackSkin(IResourceStore<byte[]> storage, IStorageResourceProvider resources)
-        {
-            // for simplicity, beatmap skins don't do lookups on the default skin.
-            // this will mean that fallback always occurs to the user (then default) skin.
-            // this may not offer perfect behaviour, but helps keep things simple.
-            return null;
-        }
-
         private static SkinInfo createSkinInfo(BeatmapInfo beatmap) =>
             new SkinInfo { Name = beatmap.ToString(), Creator = beatmap.Metadata?.AuthorString };
     }

osu.Game/Skinning/LegacyHealthDisplay.cs

@@ -20,9 +20,6 @@ namespace osu.Game.Skinning
     {
         private const double epic_cutoff = 0.5;
 
-        [Resolved]
-        private ISkinSource skin { get; set; }
-
         private LegacyHealthPiece fill;
         private LegacyHealthPiece marker;
 
@@ -31,14 +28,14 @@ namespace osu.Game.Skinning
         private bool isNewStyle;
 
         [BackgroundDependencyLoader]
-        private void load()
+        private void load(ISkinSource source)
         {
             AutoSizeAxes = Axes.Both;
 
-            var backgroundSource = skin.FindProvider(s => getTexture(s, "bg") != null);
+            var skin = source.FindProvider(s => getTexture(s, "bg") != null);
 
             // the marker lookup to decide which display style must be performed on the source of the bg, which is the most common element.
-            isNewStyle = getTexture(backgroundSource, "marker") != null;
+            isNewStyle = getTexture(skin, "marker") != null;
 
             // background implementation is the same for both versions.
             AddInternal(new Sprite { Texture = getTexture(skin, "bg") });
@@ -98,7 +95,7 @@ namespace osu.Game.Skinning
             private readonly Texture dangerTexture;
             private readonly Texture superDangerTexture;
 
-            public LegacyOldStyleMarker(ISkinSource skin)
+            public LegacyOldStyleMarker(ISkin skin)
             {
                 normalTexture = getTexture(skin, "ki");
                 dangerTexture = getTexture(skin, "kidanger");
@@ -129,9 +126,9 @@ namespace osu.Game.Skinning
 
         public class LegacyNewStyleMarker : LegacyMarker
         {
-            private readonly ISkinSource skin;
+            private readonly ISkin skin;
 
-            public LegacyNewStyleMarker(ISkinSource skin)
+            public LegacyNewStyleMarker(ISkin skin)
             {
                 this.skin = skin;
             }
@@ -153,7 +150,7 @@ namespace osu.Game.Skinning
 
         internal class LegacyOldStyleFill : LegacyHealthPiece
         {
-            public LegacyOldStyleFill(ISkinSource skin)
+            public LegacyOldStyleFill(ISkin skin)
             {
                 // required for sizing correctly..
                 var firstFrame = getTexture(skin, "colour-0");
@@ -176,7 +173,7 @@ namespace osu.Game.Skinning
 
         internal class LegacyNewStyleFill : LegacyHealthPiece
         {
-            public LegacyNewStyleFill(ISkinSource skin)
+            public LegacyNewStyleFill(ISkin skin)
             {
                 InternalChild = new Sprite
                 {

osu.Game/Skinning/LegacySkin.cs

@@ -109,9 +109,6 @@ namespace osu.Game.Skinning
                 true) != null);
         }
 
-        [CanBeNull]
-        protected virtual DefaultLegacySkin CreateFallbackSkin(IResourceStore<byte[]> storage, IStorageResourceProvider resources) => new DefaultLegacySkin(resources);
-
         public override IBindable<TValue> GetConfig<TLookup, TValue>(TLookup lookup)
         {
             switch (lookup)

osu.Game/Skinning/LegacySkinExtensions.cs

@@ -27,6 +27,18 @@ namespace osu.Game.Skinning
         {
             Texture texture;
 
+            // find the first source which provides either the animated or non-animated version.
+            ISkin skin = (source as ISkinSource)?.FindProvider(s =>
+            {
+                if (animatable && s.GetTexture(getFrameName(0)) != null)
+                    return true;
+
+                return s.GetTexture(componentName, wrapModeS, wrapModeT) != null;
+            }) ?? source;
+
+            if (skin == null)
+                return null;
+
             if (animatable)
             {
                 var textures = getTextures().ToArray();
@@ -35,7 +47,7 @@ namespace osu.Game.Skinning
                 {
                     var animation = new SkinnableTextureAnimation(startAtCurrentTime)
                     {
-                        DefaultFrameLength = frameLength ?? getFrameLength(source, applyConfigFrameRate, textures),
+                        DefaultFrameLength = frameLength ?? getFrameLength(skin, applyConfigFrameRate, textures),
                         Loop = looping,
                     };
 
@@ -47,7 +59,7 @@ namespace osu.Game.Skinning
             }
 
             // if an animation was not allowed or not found, fall back to a sprite retrieval.
-            if ((texture = source.GetTexture(componentName, wrapModeS, wrapModeT)) != null)
+            if ((texture = skin.GetTexture(componentName, wrapModeS, wrapModeT)) != null)
                 return new Sprite { Texture = texture };
 
             return null;
@@ -56,12 +68,14 @@ namespace osu.Game.Skinning
             {
                 for (int i = 0; true; i++)
                 {
-                    if ((texture = source.GetTexture($"{componentName}{animationSeparator}{i}", wrapModeS, wrapModeT)) == null)
+                    if ((texture = skin.GetTexture(getFrameName(i), wrapModeS, wrapModeT)) == null)
                         break;
 
                     yield return texture;
                 }
             }
+
+            string getFrameName(int frameIndex) => $"{componentName}{animationSeparator}{frameIndex}";
         }
 
         public static bool HasFont(this ISkin source, LegacyFont font)

osu.Game/Skinning/LegacySkinTransformer.cs

@@ -16,7 +16,7 @@ namespace osu.Game.Skinning
     /// <summary>
     /// Transformer used to handle support of legacy features for individual rulesets.
     /// </summary>
-    public abstract class LegacySkinTransformer : ISkin
+    public abstract class LegacySkinTransformer : ISkinSource
     {
         /// <summary>
         /// Source of the <see cref="ISkin"/> which is being transformed.
@@ -50,5 +50,11 @@ namespace osu.Game.Skinning
         public abstract IBindable<TValue> GetConfig<TLookup, TValue>(TLookup lookup);
 
         public ISkin FindProvider(Func<ISkin, bool> lookupFunction) => Source.FindProvider(lookupFunction);
+
+        public event Action SourceChanged
+        {
+            add { throw new NotSupportedException(); }
+            remove { }
+        }
     }
 }

osu.Game/Skinning/SkinManager.cs

@@ -48,6 +48,8 @@ namespace osu.Game.Skinning
 
         protected override string ImportFromStablePath => "Skins";
 
+        private readonly Skin defaultLegacySkin;
+
         public SkinManager(Storage storage, DatabaseContextFactory contextFactory, GameHost host, IResourceStore<byte[]> resources, AudioManager audio)
             : base(storage, contextFactory, new SkinStore(contextFactory, storage), host)
         {
@@ -55,6 +57,8 @@ namespace osu.Game.Skinning
             this.host = host;
             this.resources = resources;
 
+            defaultLegacySkin = new DefaultLegacySkin(this);
+
             CurrentSkinInfo.ValueChanged += skin => CurrentSkin.Value = GetSkin(skin.NewValue);
 
             CurrentSkin.Value = new DefaultSkin(this);
@@ -214,9 +218,16 @@ namespace osu.Game.Skinning
 
         public IBindable<TValue> GetConfig<TLookup, TValue>(TLookup lookup) => lookupWithFallback(s => s.GetConfig<TLookup, TValue>(lookup));
 
-        public ISkin FindProvider(Func<ISkin, bool> lookupFunction) => lookupFunction(CurrentSkin.Value) ? CurrentSkin.Value : null;
+        public ISkin FindProvider(Func<ISkin, bool> lookupFunction)
+        {
+            if (lookupFunction(CurrentSkin.Value))
+                return CurrentSkin.Value;
 
-        private Skin defaultLegacySkin;
+            if (CurrentSkin.Value is LegacySkin && lookupFunction(defaultLegacySkin))
+                return defaultLegacySkin;
+
+            return null;
+        }
 
         private T lookupWithFallback<T>(Func<ISkin, T> func)
             where T : class
@@ -226,8 +237,9 @@ namespace osu.Game.Skinning
             if (selectedSkin != null)
                 return selectedSkin;
 
-            defaultLegacySkin ??= new DefaultLegacySkin(this);
+            // TODO: we also want to return a DefaultLegacySkin here if the current *beatmap* is providing any skinned elements.
-
+            // When attempting to address this, we may want to move the full DefaultLegacySkin fallback logic to within Player itself (to better allow
+            // for beatmap skin visibility).
             if (CurrentSkin.Value is LegacySkin)
                 return func(defaultLegacySkin);
 

osu.Game/Skinning/SkinProvidingContainer.cs

@@ -46,8 +46,16 @@ namespace osu.Game.Skinning
 
         public ISkin FindProvider(Func<ISkin, bool> lookupFunction)
         {
-            if (skin != null && lookupFunction(skin))
+            if (skin is ISkinSource source)
-                return skin;
+            {
+                if (source.FindProvider(lookupFunction) is ISkin found)
+                    return found;
+            }
+            else if (skin != null)
+            {
+                if (lookupFunction(skin))
+                    return skin;
+            }
 
             return fallbackSource?.FindProvider(lookupFunction);
         }