From 9e16359f18a7e90049a1dacb80bad9ff2fe8d130 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Fri, 11 Jun 2021 12:29:28 +0300 Subject: [PATCH] Refactor disallowing in `SkinProvidingContainer` to become per source Fixes `FindProvider` becoming completely broken, because of no way to perform the checks on one skin source. --- osu.Game/Skinning/SkinProvidingContainer.cs | 188 ++++++++++---------- 1 file changed, 96 insertions(+), 92 deletions(-) diff --git a/osu.Game/Skinning/SkinProvidingContainer.cs b/osu.Game/Skinning/SkinProvidingContainer.cs index ab33a66265..c9bb3a6ec4 100644 --- a/osu.Game/Skinning/SkinProvidingContainer.cs +++ b/osu.Game/Skinning/SkinProvidingContainer.cs @@ -2,6 +2,7 @@ // See the LICENCE file in the repository root for full licence text. using System; +using System.Collections.Generic; using System.Collections.Specialized; using System.Linq; using JetBrains.Annotations; @@ -28,11 +29,15 @@ namespace osu.Game.Skinning /// protected readonly BindableList SkinSources = new BindableList(); + /// + /// A dictionary mapping each from the + /// to one that performs the "allow lookup" checks before proceeding with a lookup. + /// + private readonly Dictionary disableableSkinSources = new Dictionary(); + [CanBeNull] private ISkinSource fallbackSource; - private readonly NoFallbackProxy noFallbackLookupProxy; - protected virtual bool AllowDrawableLookup(ISkinComponent component) => true; protected virtual bool AllowTextureLookup(string componentName) => true; @@ -60,31 +65,49 @@ namespace osu.Game.Skinning { RelativeSizeAxes = Axes.Both; - noFallbackLookupProxy = new NoFallbackProxy(this); - SkinSources.BindCollectionChanged(((_, args) => { switch (args.Action) { case NotifyCollectionChangedAction.Add: - foreach (var source in args.NewItems.Cast().OfType()) - source.SourceChanged += OnSourceChanged; + foreach (var skin in args.NewItems.Cast()) + { + disableableSkinSources.Add(skin, new DisableableSkinSource(skin, this)); + + if (skin is ISkinSource source) + source.SourceChanged += OnSourceChanged; + } break; case NotifyCollectionChangedAction.Reset: case NotifyCollectionChangedAction.Remove: - foreach (var source in args.OldItems.Cast().OfType()) - source.SourceChanged -= OnSourceChanged; + foreach (var skin in args.OldItems.Cast()) + { + disableableSkinSources.Remove(skin); + + if (skin is ISkinSource source) + source.SourceChanged -= OnSourceChanged; + } break; case NotifyCollectionChangedAction.Replace: - foreach (var source in args.OldItems.Cast().OfType()) - source.SourceChanged -= OnSourceChanged; + foreach (var skin in args.OldItems.Cast()) + { + disableableSkinSources.Remove(skin); - foreach (var source in args.NewItems.Cast().OfType()) - source.SourceChanged += OnSourceChanged; + if (skin is ISkinSource source) + source.SourceChanged -= OnSourceChanged; + } + + foreach (var skin in args.NewItems.Cast()) + { + disableableSkinSources.Add(skin, new DisableableSkinSource(skin, this)); + + if (skin is ISkinSource source) + source.SourceChanged += OnSourceChanged; + } break; } @@ -95,8 +118,7 @@ namespace osu.Game.Skinning { foreach (var skin in SkinSources) { - // a proxy must be used here to correctly pass through the "Allow" checks without implicitly falling back to the fallbackSource. - if (lookupFunction(noFallbackLookupProxy)) + if (lookupFunction(disableableSkinSources[skin])) return skin; } @@ -104,94 +126,50 @@ namespace osu.Game.Skinning } public Drawable GetDrawableComponent(ISkinComponent component) - => GetDrawableComponent(component, true); - - public Drawable GetDrawableComponent(ISkinComponent component, bool fallback) { - if (AllowDrawableLookup(component)) + foreach (var skin in SkinSources) { - foreach (var skin in SkinSources) - { - Drawable sourceDrawable; - if ((sourceDrawable = skin?.GetDrawableComponent(component)) != null) - return sourceDrawable; - } + Drawable sourceDrawable; + if ((sourceDrawable = disableableSkinSources[skin]?.GetDrawableComponent(component)) != null) + return sourceDrawable; } - if (!fallback) - return null; - return fallbackSource?.GetDrawableComponent(component); } public Texture GetTexture(string componentName, WrapMode wrapModeS, WrapMode wrapModeT) - => GetTexture(componentName, wrapModeS, wrapModeT, true); - - public Texture GetTexture(string componentName, WrapMode wrapModeS, WrapMode wrapModeT, bool fallback) { - if (AllowTextureLookup(componentName)) + foreach (var skin in SkinSources) { - foreach (var skin in SkinSources) - { - Texture sourceTexture; - if ((sourceTexture = skin?.GetTexture(componentName, wrapModeS, wrapModeT)) != null) - return sourceTexture; - } + Texture sourceTexture; + if ((sourceTexture = disableableSkinSources[skin]?.GetTexture(componentName, wrapModeS, wrapModeT)) != null) + return sourceTexture; } - if (!fallback) - return null; - return fallbackSource?.GetTexture(componentName, wrapModeS, wrapModeT); } public ISample GetSample(ISampleInfo sampleInfo) - => GetSample(sampleInfo, true); - - public ISample GetSample(ISampleInfo sampleInfo, bool fallback) { - if (AllowSampleLookup(sampleInfo)) + foreach (var skin in SkinSources) { - foreach (var skin in SkinSources) - { - ISample sourceSample; - if ((sourceSample = skin?.GetSample(sampleInfo)) != null) - return sourceSample; - } + ISample sourceSample; + if ((sourceSample = disableableSkinSources[skin]?.GetSample(sampleInfo)) != null) + return sourceSample; } - if (!fallback) - return null; - return fallbackSource?.GetSample(sampleInfo); } public IBindable GetConfig(TLookup lookup) - => GetConfig(lookup, true); - - public IBindable GetConfig(TLookup lookup, bool fallback) { - if (lookup is GlobalSkinColours || lookup is SkinCustomColourLookup) - return lookupWithFallback(lookup, AllowColourLookup, fallback); - - return lookupWithFallback(lookup, AllowConfigurationLookup, fallback); - } - - private IBindable lookupWithFallback(TLookup lookup, bool canUseSkinLookup, bool canUseFallback) - { - if (canUseSkinLookup) + foreach (var skin in SkinSources) { - foreach (var skin in SkinSources) - { - IBindable bindable; - if ((bindable = skin?.GetConfig(lookup)) != null) - return bindable; - } + IBindable bindable; + if ((bindable = disableableSkinSources[skin]?.GetConfig(lookup)) != null) + return bindable; } - if (!canUseFallback) - return null; - return fallbackSource?.GetConfig(lookup); } @@ -224,35 +202,61 @@ namespace osu.Game.Skinning source.SourceChanged -= OnSourceChanged; } - private class NoFallbackProxy : ISkinSource + private class DisableableSkinSource : ISkin { + private readonly ISkin skin; private readonly SkinProvidingContainer provider; - public NoFallbackProxy(SkinProvidingContainer provider) + public DisableableSkinSource(ISkin skin, SkinProvidingContainer provider) { + this.skin = skin; this.provider = provider; } public Drawable GetDrawableComponent(ISkinComponent component) - => provider.GetDrawableComponent(component, false); - - public Texture GetTexture(string componentName, WrapMode wrapModeS, WrapMode wrapModeT) - => provider.GetTexture(componentName, wrapModeS, wrapModeT, false); - - public ISample GetSample(ISampleInfo sampleInfo) - => provider.GetSample(sampleInfo, false); - - public IBindable GetConfig(TLookup lookup) - => provider.GetConfig(lookup, false); - - public event Action SourceChanged { - add => provider.SourceChanged += value; - remove => provider.SourceChanged -= value; + if (provider.AllowDrawableLookup(component)) + return skin.GetDrawableComponent(component); + + return null; } - public ISkin FindProvider(Func lookupFunction) => - provider.FindProvider(lookupFunction); + public Texture GetTexture(string componentName, WrapMode wrapModeS, WrapMode wrapModeT) + { + if (provider.AllowTextureLookup(componentName)) + return skin.GetTexture(componentName, wrapModeS, wrapModeT); + + return null; + } + + public ISample GetSample(ISampleInfo sampleInfo) + { + if (provider.AllowSampleLookup(sampleInfo)) + return skin.GetSample(sampleInfo); + + return null; + } + + public IBindable GetConfig(TLookup lookup) + { + switch (lookup) + { + case GlobalSkinColours _: + case SkinCustomColourLookup _: + if (provider.AllowColourLookup) + return skin.GetConfig(lookup); + + break; + + default: + if (provider.AllowConfigurationLookup) + return skin.GetConfig(lookup); + + break; + } + + return null; + } } } }