From 74c961bcffcf7030ccae4320e3fb951346a734d4 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 18 Jul 2019 12:10:28 +0900 Subject: [PATCH 1/4] Add more flexible skin element confine modes --- .../Drawables/Connections/FollowPoint.cs | 2 +- .../Objects/Drawables/DrawableRepeatPoint.cs | 2 +- .../Objects/Drawables/DrawableSliderTick.cs | 4 +- .../Objects/Drawables/Pieces/NumberPiece.cs | 2 +- osu.Game.Rulesets.Osu/UI/Cursor/OsuCursor.cs | 2 +- .../Gameplay/TestSceneSkinReloadable.cs | 4 +- .../Rulesets/Judgements/DrawableJudgement.cs | 2 +- osu.Game/Skinning/SkinnableDrawable.cs | 44 +++++++++++++------ osu.Game/Skinning/SkinnableSprite.cs | 4 +- osu.Game/Skinning/SkinnableSpriteText.cs | 4 +- 10 files changed, 43 insertions(+), 27 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Objects/Drawables/Connections/FollowPoint.cs b/osu.Game.Rulesets.Osu/Objects/Drawables/Connections/FollowPoint.cs index aacf3ee08d..a2a23e9ff7 100644 --- a/osu.Game.Rulesets.Osu/Objects/Drawables/Connections/FollowPoint.cs +++ b/osu.Game.Rulesets.Osu/Objects/Drawables/Connections/FollowPoint.cs @@ -41,7 +41,7 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables.Connections Anchor = Anchor.Centre, Alpha = 0.5f, } - }, restrictSize: false); + }, confineMode: ConfineMode.NoScaling); } } } diff --git a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableRepeatPoint.cs b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableRepeatPoint.cs index cce6dfe106..cf6f05a604 100644 --- a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableRepeatPoint.cs +++ b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableRepeatPoint.cs @@ -37,7 +37,7 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables { RelativeSizeAxes = Axes.Both, Icon = FontAwesome.Solid.ChevronRight - }, restrictSize: false) + }) }; } diff --git a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSliderTick.cs b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSliderTick.cs index 72b648bfd0..01ef1eb3e0 100644 --- a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSliderTick.cs +++ b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSliderTick.cs @@ -23,7 +23,7 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables public DrawableSliderTick(SliderTick sliderTick) : base(sliderTick) { - Size = new Vector2(16) * sliderTick.Scale; + Size = new Vector2(16 * sliderTick.Scale); Origin = Anchor.Centre; InternalChildren = new Drawable[] @@ -44,7 +44,7 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables Colour = AccentColour, Alpha = 0.3f, } - }, restrictSize: false) + }) }; } diff --git a/osu.Game.Rulesets.Osu/Objects/Drawables/Pieces/NumberPiece.cs b/osu.Game.Rulesets.Osu/Objects/Drawables/Pieces/NumberPiece.cs index 84034d3ee9..e8dc63abca 100644 --- a/osu.Game.Rulesets.Osu/Objects/Drawables/Pieces/NumberPiece.cs +++ b/osu.Game.Rulesets.Osu/Objects/Drawables/Pieces/NumberPiece.cs @@ -46,7 +46,7 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables.Pieces { Font = OsuFont.Numeric.With(size: 40), UseFullGlyphHeight = false, - }, restrictSize: false) + }, confineMode: ConfineMode.NoScaling) { Text = @"1" } diff --git a/osu.Game.Rulesets.Osu/UI/Cursor/OsuCursor.cs b/osu.Game.Rulesets.Osu/UI/Cursor/OsuCursor.cs index 27546fa424..b3492a2b31 100644 --- a/osu.Game.Rulesets.Osu/UI/Cursor/OsuCursor.cs +++ b/osu.Game.Rulesets.Osu/UI/Cursor/OsuCursor.cs @@ -101,7 +101,7 @@ namespace osu.Game.Rulesets.Osu.UI.Cursor }, }, } - }, restrictSize: false) + }) { Origin = Anchor.Centre, Anchor = Anchor.Centre, diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneSkinReloadable.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneSkinReloadable.cs index c7a0df6e9f..af03ddc65c 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneSkinReloadable.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneSkinReloadable.cs @@ -92,8 +92,8 @@ namespace osu.Game.Tests.Visual.Gameplay public new Drawable Drawable => base.Drawable; public int SkinChangedCount { get; private set; } - public SkinConsumer(string name, Func defaultImplementation, Func allowFallback = null, bool restrictSize = true) - : base(name, defaultImplementation, allowFallback, restrictSize) + public SkinConsumer(string name, Func defaultImplementation, Func allowFallback = null) + : base(name, defaultImplementation, allowFallback) { } diff --git a/osu.Game/Rulesets/Judgements/DrawableJudgement.cs b/osu.Game/Rulesets/Judgements/DrawableJudgement.cs index 2150726a42..61c2644c6f 100644 --- a/osu.Game/Rulesets/Judgements/DrawableJudgement.cs +++ b/osu.Game/Rulesets/Judgements/DrawableJudgement.cs @@ -67,7 +67,7 @@ namespace osu.Game.Rulesets.Judgements Font = OsuFont.Numeric.With(size: 12), Colour = judgementColour(Result.Type), Scale = new Vector2(0.85f, 1), - }, restrictSize: false) + }) }; } diff --git a/osu.Game/Skinning/SkinnableDrawable.cs b/osu.Game/Skinning/SkinnableDrawable.cs index 995cb15136..a30bdbd2dc 100644 --- a/osu.Game/Skinning/SkinnableDrawable.cs +++ b/osu.Game/Skinning/SkinnableDrawable.cs @@ -9,8 +9,8 @@ namespace osu.Game.Skinning { public class SkinnableDrawable : SkinnableDrawable { - public SkinnableDrawable(string name, Func defaultImplementation, Func allowFallback = null, bool restrictSize = true) - : base(name, defaultImplementation, allowFallback, restrictSize) + public SkinnableDrawable(string name, Func defaultImplementation, Func allowFallback = null, ConfineMode confineMode = ConfineMode.ScaleDownToFit) + : base(name, defaultImplementation, allowFallback, confineMode) { } } @@ -29,7 +29,7 @@ namespace osu.Game.Skinning private readonly string componentName; - private readonly bool restrictSize; + private readonly ConfineMode confineMode; /// /// Create a new skinnable drawable. @@ -37,18 +37,18 @@ namespace osu.Game.Skinning /// The namespace-complete resource name for this skinnable element. /// A function to create the default skin implementation of this element. /// A conditional to decide whether to allow fallback to the default implementation if a skinned element is not present. - /// Whether a user-skin drawable should be limited to the size of our parent. - public SkinnableDrawable(string name, Func defaultImplementation, Func allowFallback = null, bool restrictSize = true) - : this(name, allowFallback, restrictSize) + /// How (if at all) the should be resize to fit within our own bounds. + public SkinnableDrawable(string name, Func defaultImplementation, Func allowFallback = null, ConfineMode confineMode = ConfineMode.ScaleDownToFit) + : this(name, allowFallback, confineMode) { createDefault = defaultImplementation; } - protected SkinnableDrawable(string name, Func allowFallback = null, bool restrictSize = true) + protected SkinnableDrawable(string name, Func allowFallback = null, ConfineMode confineMode = ConfineMode.ScaleDownToFit) : base(allowFallback) { componentName = name; - this.restrictSize = restrictSize; + this.confineMode = confineMode; RelativeSizeAxes = Axes.Both; } @@ -58,7 +58,7 @@ namespace osu.Game.Skinning protected virtual T CreateDefault(string name) => createDefault(name); /// - /// Whether to apply size restrictions (specified via ) to the default implementation. + /// Whether to apply size restrictions (specified via ) to the default implementation. /// protected virtual bool ApplySizeRestrictionsToDefault => false; @@ -76,12 +76,18 @@ namespace osu.Game.Skinning if (Drawable != null) { - if (restrictSize && (!isDefault || ApplySizeRestrictionsToDefault)) + if (confineMode != ConfineMode.NoScaling && (!isDefault || ApplySizeRestrictionsToDefault)) { - Drawable.RelativeSizeAxes = Axes.Both; - Drawable.Size = Vector2.One; - Drawable.Scale = Vector2.One; - Drawable.FillMode = FillMode.Fit; + bool applyScaling = confineMode == ConfineMode.ScaleToFit || + (confineMode == ConfineMode.ScaleDownToFit && (Drawable.DrawSize.X > DrawSize.X || Drawable.DrawSize.Y > DrawSize.Y)); + + if (applyScaling) + { + Drawable.RelativeSizeAxes = Axes.Both; + Drawable.Size = Vector2.One; + Drawable.Scale = Vector2.One; + Drawable.FillMode = FillMode.Fit; + } } Drawable.Origin = Anchor.Centre; @@ -93,4 +99,14 @@ namespace osu.Game.Skinning ClearInternal(); } } + + public enum ConfineMode + { + /// + /// Don't apply any scaling. This allows the user element to be of any size, exceeding specified bounds. + /// + NoScaling, + ScaleDownToFit, + ScaleToFit, + } } diff --git a/osu.Game/Skinning/SkinnableSprite.cs b/osu.Game/Skinning/SkinnableSprite.cs index ceb1ed0f70..1716ec71de 100644 --- a/osu.Game/Skinning/SkinnableSprite.cs +++ b/osu.Game/Skinning/SkinnableSprite.cs @@ -18,8 +18,8 @@ namespace osu.Game.Skinning [Resolved] private TextureStore textures { get; set; } - public SkinnableSprite(string name, Func allowFallback = null, bool restrictSize = true) - : base(name, allowFallback, restrictSize) + public SkinnableSprite(string name, Func allowFallback = null, ConfineMode confineMode = ConfineMode.ScaleDownToFit) + : base(name, allowFallback, confineMode) { } diff --git a/osu.Game/Skinning/SkinnableSpriteText.cs b/osu.Game/Skinning/SkinnableSpriteText.cs index 36e646d743..d12a6f74f7 100644 --- a/osu.Game/Skinning/SkinnableSpriteText.cs +++ b/osu.Game/Skinning/SkinnableSpriteText.cs @@ -8,8 +8,8 @@ namespace osu.Game.Skinning { public class SkinnableSpriteText : SkinnableDrawable, IHasText { - public SkinnableSpriteText(string name, Func defaultImplementation, Func allowFallback = null, bool restrictSize = true) - : base(name, defaultImplementation, allowFallback, restrictSize) + public SkinnableSpriteText(string name, Func defaultImplementation, Func allowFallback = null, ConfineMode confineMode = ConfineMode.ScaleDownToFit) + : base(name, defaultImplementation, allowFallback, confineMode) { } From 9d091c96b81d2bf461890137cc32d87daa7b4972 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 18 Jul 2019 17:52:50 +0900 Subject: [PATCH 2/4] Use cache to ensure correct DrawSize when deciding scaling --- osu.Game/Skinning/SkinnableDrawable.cs | 32 +++++++++++++++++++------- 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/osu.Game/Skinning/SkinnableDrawable.cs b/osu.Game/Skinning/SkinnableDrawable.cs index a30bdbd2dc..5d1ca98bfd 100644 --- a/osu.Game/Skinning/SkinnableDrawable.cs +++ b/osu.Game/Skinning/SkinnableDrawable.cs @@ -2,6 +2,7 @@ // See the LICENCE file in the repository root for full licence text. using System; +using osu.Framework.Caching; using osu.Framework.Graphics; using osuTK; @@ -55,6 +56,10 @@ namespace osu.Game.Skinning private readonly Func createDefault; + private readonly Cached scaling = new Cached(); + + private bool isDefault; + protected virtual T CreateDefault(string name) => createDefault(name); /// @@ -66,7 +71,7 @@ namespace osu.Game.Skinning { Drawable = skin.GetDrawableComponent(componentName); - bool isDefault = false; + isDefault = false; if (Drawable == null && allowFallback) { @@ -76,7 +81,23 @@ namespace osu.Game.Skinning if (Drawable != null) { - if (confineMode != ConfineMode.NoScaling && (!isDefault || ApplySizeRestrictionsToDefault)) + scaling.Invalidate(); + Drawable.Origin = Anchor.Centre; + Drawable.Anchor = Anchor.Centre; + + InternalChild = Drawable; + } + else + ClearInternal(); + } + + protected override void Update() + { + base.Update(); + + if (!scaling.IsValid) + { + if (Drawable != null && confineMode != ConfineMode.NoScaling && (!isDefault || ApplySizeRestrictionsToDefault)) { bool applyScaling = confineMode == ConfineMode.ScaleToFit || (confineMode == ConfineMode.ScaleDownToFit && (Drawable.DrawSize.X > DrawSize.X || Drawable.DrawSize.Y > DrawSize.Y)); @@ -90,13 +111,8 @@ namespace osu.Game.Skinning } } - Drawable.Origin = Anchor.Centre; - Drawable.Anchor = Anchor.Centre; - - InternalChild = Drawable; + scaling.Validate(); } - else - ClearInternal(); } } From eca63980d2ed7cda53a848af781eb495960a11c0 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 18 Jul 2019 17:53:01 +0900 Subject: [PATCH 3/4] Add comprehensive scaling mode tests --- .../Gameplay/TestSceneSkinReloadable.cs | 145 --------- .../Gameplay/TestSceneSkinnableDrawable.cs | 283 ++++++++++++++++++ 2 files changed, 283 insertions(+), 145 deletions(-) delete mode 100644 osu.Game.Tests/Visual/Gameplay/TestSceneSkinReloadable.cs create mode 100644 osu.Game.Tests/Visual/Gameplay/TestSceneSkinnableDrawable.cs diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneSkinReloadable.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneSkinReloadable.cs deleted file mode 100644 index af03ddc65c..0000000000 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneSkinReloadable.cs +++ /dev/null @@ -1,145 +0,0 @@ -// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. -// See the LICENCE file in the repository root for full licence text. - -using System; -using NUnit.Framework; -using osu.Framework.Audio.Sample; -using osu.Framework.Graphics; -using osu.Framework.Graphics.Containers; -using osu.Framework.Graphics.Shapes; -using osu.Framework.Graphics.Textures; -using osu.Game.Graphics; -using osu.Game.Graphics.Sprites; -using osu.Game.Skinning; -using osuTK.Graphics; - -namespace osu.Game.Tests.Visual.Gameplay -{ - public class TestSceneSkinReloadable : OsuTestScene - { - [Test] - public void TestInitialLoad() - { - var secondarySource = new SecondarySource(); - SkinConsumer consumer = null; - - AddStep("setup layout", () => - { - Child = new SkinSourceContainer - { - RelativeSizeAxes = Axes.Both, - Child = new LocalSkinOverrideContainer(secondarySource) - { - RelativeSizeAxes = Axes.Both, - Child = consumer = new SkinConsumer("test", name => new NamedBox("Default Implementation"), source => true) - } - }; - }); - - AddAssert("consumer using override source", () => consumer.Drawable is SecondarySourceBox); - AddAssert("skinchanged only called once", () => consumer.SkinChangedCount == 1); - } - - [Test] - public void TestOverride() - { - var secondarySource = new SecondarySource(); - - SkinConsumer consumer = null; - Container target = null; - - AddStep("setup layout", () => - { - Child = new SkinSourceContainer - { - RelativeSizeAxes = Axes.Both, - Child = target = new LocalSkinOverrideContainer(secondarySource) - { - RelativeSizeAxes = Axes.Both, - } - }; - }); - - AddStep("add permissive", () => target.Add(consumer = new SkinConsumer("test", name => new NamedBox("Default Implementation"), source => true))); - AddAssert("consumer using override source", () => consumer.Drawable is SecondarySourceBox); - AddAssert("skinchanged only called once", () => consumer.SkinChangedCount == 1); - } - - private class NamedBox : Container - { - public NamedBox(string name) - { - Children = new Drawable[] - { - new Box - { - Colour = Color4.Black, - RelativeSizeAxes = Axes.Both, - }, - new OsuSpriteText - { - Font = OsuFont.Default.With(size: 40), - Anchor = Anchor.Centre, - Origin = Anchor.Centre, - Text = name - } - }; - } - } - - private class SkinConsumer : SkinnableDrawable - { - public new Drawable Drawable => base.Drawable; - public int SkinChangedCount { get; private set; } - - public SkinConsumer(string name, Func defaultImplementation, Func allowFallback = null) - : base(name, defaultImplementation, allowFallback) - { - } - - protected override void SkinChanged(ISkinSource skin, bool allowFallback) - { - base.SkinChanged(skin, allowFallback); - SkinChangedCount++; - } - } - - private class BaseSourceBox : NamedBox - { - public BaseSourceBox() - : base("Base Source") - { - } - } - - private class SecondarySourceBox : NamedBox - { - public SecondarySourceBox() - : base("Secondary Source") - { - } - } - - private class SecondarySource : ISkin - { - public Drawable GetDrawableComponent(string componentName) => new SecondarySourceBox(); - - public Texture GetTexture(string componentName) => throw new NotImplementedException(); - - public SampleChannel GetSample(string sampleName) => throw new NotImplementedException(); - - public TValue GetValue(Func query) where TConfiguration : SkinConfiguration => throw new NotImplementedException(); - } - - private class SkinSourceContainer : Container, ISkin - { - public Drawable GetDrawableComponent(string componentName) => new BaseSourceBox(); - - public Texture GetTexture(string componentName) => throw new NotImplementedException(); - - public SampleChannel GetSample(string sampleName) => throw new NotImplementedException(); - - public TValue GetValue(Func query) where TConfiguration : SkinConfiguration => throw new NotImplementedException(); - } - } -} diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneSkinnableDrawable.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneSkinnableDrawable.cs new file mode 100644 index 0000000000..0b5978e3eb --- /dev/null +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneSkinnableDrawable.cs @@ -0,0 +1,283 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System; +using System.Globalization; +using System.Linq; +using NUnit.Framework; +using osu.Framework.Audio.Sample; +using osu.Framework.Graphics; +using osu.Framework.Graphics.Containers; +using osu.Framework.Graphics.Shapes; +using osu.Framework.Graphics.Textures; +using osu.Game.Graphics; +using osu.Game.Graphics.Sprites; +using osu.Game.Skinning; +using osuTK; +using osuTK.Graphics; + +namespace osu.Game.Tests.Visual.Gameplay +{ + public class TestSceneSkinnableDrawable : OsuTestScene + { + [Test] + public void TestConfineScaleDown() + { + FillFlowContainer fill = null; + + AddStep("setup layout larger source", () => + { + Child = new LocalSkinOverrideContainer(new SizedSource(50)) + { + RelativeSizeAxes = Axes.Both, + Child = fill = new FillFlowContainer + { + Size = new Vector2(30), + Anchor = Anchor.Centre, + Origin = Anchor.Centre, + Spacing = new Vector2(10), + Children = new[] + { + new ExposedSkinnableDrawable("default", _ => new DefaultBox(), _ => true), + new ExposedSkinnableDrawable("available", _ => new DefaultBox(), _ => true), + new ExposedSkinnableDrawable("available", _ => new DefaultBox(), _ => true, ConfineMode.ScaleToFit), + new ExposedSkinnableDrawable("available", _ => new DefaultBox(), _ => true, ConfineMode.NoScaling) + } + }, + }; + }); + + AddAssert("check sizes", () => fill.Children.Select(c => c.Drawable.DrawWidth).SequenceEqual(new float[] { 30, 30, 30, 50 })); + AddStep("adjust scale", () => fill.Scale = new Vector2(2)); + AddAssert("check sizes unchanged by scale", () => fill.Children.Select(c => c.Drawable.DrawWidth).SequenceEqual(new float[] { 30, 30, 30, 50 })); + } + + [Test] + public void TestConfineScaleUp() + { + FillFlowContainer fill = null; + + AddStep("setup layout larger source", () => + { + Child = new LocalSkinOverrideContainer(new SizedSource(30)) + { + RelativeSizeAxes = Axes.Both, + Child = fill = new FillFlowContainer + { + Size = new Vector2(50), + Anchor = Anchor.Centre, + Origin = Anchor.Centre, + Spacing = new Vector2(10), + Children = new[] + { + new ExposedSkinnableDrawable("default", _ => new DefaultBox(), _ => true), + new ExposedSkinnableDrawable("available", _ => new DefaultBox(), _ => true), + new ExposedSkinnableDrawable("available", _ => new DefaultBox(), _ => true, ConfineMode.ScaleToFit), + new ExposedSkinnableDrawable("available", _ => new DefaultBox(), _ => true, ConfineMode.NoScaling) + } + }, + }; + }); + + AddAssert("check sizes", () => fill.Children.Select(c => c.Drawable.DrawWidth).SequenceEqual(new float[] { 50, 30, 50, 30 })); + AddStep("adjust scale", () => fill.Scale = new Vector2(2)); + AddAssert("check sizes unchanged by scale", () => fill.Children.Select(c => c.Drawable.DrawWidth).SequenceEqual(new float[] { 50, 30, 50, 30 })); + } + + [Test] + public void TestInitialLoad() + { + var secondarySource = new SecondarySource(); + SkinConsumer consumer = null; + + AddStep("setup layout", () => + { + Child = new SkinSourceContainer + { + RelativeSizeAxes = Axes.Both, + Child = new LocalSkinOverrideContainer(secondarySource) + { + RelativeSizeAxes = Axes.Both, + Child = consumer = new SkinConsumer("test", name => new NamedBox("Default Implementation"), source => true) + } + }; + }); + + AddAssert("consumer using override source", () => consumer.Drawable is SecondarySourceBox); + AddAssert("skinchanged only called once", () => consumer.SkinChangedCount == 1); + } + + [Test] + public void TestOverride() + { + var secondarySource = new SecondarySource(); + + SkinConsumer consumer = null; + Container target = null; + + AddStep("setup layout", () => + { + Child = new SkinSourceContainer + { + RelativeSizeAxes = Axes.Both, + Child = target = new LocalSkinOverrideContainer(secondarySource) + { + RelativeSizeAxes = Axes.Both, + } + }; + }); + + AddStep("add permissive", () => target.Add(consumer = new SkinConsumer("test", name => new NamedBox("Default Implementation"), source => true))); + AddAssert("consumer using override source", () => consumer.Drawable is SecondarySourceBox); + AddAssert("skinchanged only called once", () => consumer.SkinChangedCount == 1); + } + + private class ExposedSkinnableDrawable : SkinnableDrawable + { + public new Drawable Drawable => base.Drawable; + + public ExposedSkinnableDrawable(string name, Func defaultImplementation, Func allowFallback = null, ConfineMode confineMode = ConfineMode.ScaleDownToFit) + : base(name, defaultImplementation, allowFallback, confineMode) + { + } + } + + private class DefaultBox : DrawWidthBox + { + public DefaultBox() + { + RelativeSizeAxes = Axes.Both; + } + } + + private class DrawWidthBox : Container + { + private readonly OsuSpriteText text; + + public DrawWidthBox() + { + Children = new Drawable[] + { + new Box + { + Colour = Color4.Gray, + RelativeSizeAxes = Axes.Both, + }, + text = new OsuSpriteText + { + Anchor = Anchor.Centre, + Origin = Anchor.Centre, + } + }; + } + + protected override void UpdateAfterChildren() + { + base.UpdateAfterChildren(); + text.Text = DrawWidth.ToString(CultureInfo.InvariantCulture); + } + } + + private class NamedBox : Container + { + public NamedBox(string name) + { + Children = new Drawable[] + { + new Box + { + Colour = Color4.Black, + RelativeSizeAxes = Axes.Both, + }, + new OsuSpriteText + { + Font = OsuFont.Default.With(size: 40), + Anchor = Anchor.Centre, + Origin = Anchor.Centre, + Text = name + } + }; + } + } + + private class SkinConsumer : SkinnableDrawable + { + public new Drawable Drawable => base.Drawable; + public int SkinChangedCount { get; private set; } + + public SkinConsumer(string name, Func defaultImplementation, Func allowFallback = null) + : base(name, defaultImplementation, allowFallback) + { + } + + protected override void SkinChanged(ISkinSource skin, bool allowFallback) + { + base.SkinChanged(skin, allowFallback); + SkinChangedCount++; + } + } + + private class BaseSourceBox : NamedBox + { + public BaseSourceBox() + : base("Base Source") + { + } + } + + private class SecondarySourceBox : NamedBox + { + public SecondarySourceBox() + : base("Secondary Source") + { + } + } + + private class SizedSource : ISkin + { + private readonly float size; + + public SizedSource(float size) + { + this.size = size; + } + + public Drawable GetDrawableComponent(string componentName) => + componentName == "available" + ? new DrawWidthBox + { + Colour = Color4.Yellow, + Size = new Vector2(size) + } + : null; + + public Texture GetTexture(string componentName) => throw new NotImplementedException(); + + public SampleChannel GetSample(string sampleName) => throw new NotImplementedException(); + + public TValue GetValue(Func query) where TConfiguration : SkinConfiguration => throw new NotImplementedException(); + } + + private class SecondarySource : ISkin + { + public Drawable GetDrawableComponent(string componentName) => new SecondarySourceBox(); + + public Texture GetTexture(string componentName) => throw new NotImplementedException(); + + public SampleChannel GetSample(string sampleName) => throw new NotImplementedException(); + + public TValue GetValue(Func query) where TConfiguration : SkinConfiguration => throw new NotImplementedException(); + } + + private class SkinSourceContainer : Container, ISkin + { + public Drawable GetDrawableComponent(string componentName) => new BaseSourceBox(); + + public Texture GetTexture(string componentName) => throw new NotImplementedException(); + + public SampleChannel GetSample(string sampleName) => throw new NotImplementedException(); + + public TValue GetValue(Func query) where TConfiguration : SkinConfiguration => throw new NotImplementedException(); + } + } +} From 36c557c752be5bdecb37b9e761c6ad4551e47c12 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 18 Jul 2019 18:39:33 +0900 Subject: [PATCH 4/4] Add null check and simplify scaling conditional logic via switch --- osu.Game/Skinning/SkinnableDrawable.cs | 33 ++++++++++++++++---------- 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/osu.Game/Skinning/SkinnableDrawable.cs b/osu.Game/Skinning/SkinnableDrawable.cs index 5d1ca98bfd..eb0508b568 100644 --- a/osu.Game/Skinning/SkinnableDrawable.cs +++ b/osu.Game/Skinning/SkinnableDrawable.cs @@ -84,7 +84,6 @@ namespace osu.Game.Skinning scaling.Invalidate(); Drawable.Origin = Anchor.Centre; Drawable.Anchor = Anchor.Centre; - InternalChild = Drawable; } else @@ -97,21 +96,31 @@ namespace osu.Game.Skinning if (!scaling.IsValid) { - if (Drawable != null && confineMode != ConfineMode.NoScaling && (!isDefault || ApplySizeRestrictionsToDefault)) + try { - bool applyScaling = confineMode == ConfineMode.ScaleToFit || - (confineMode == ConfineMode.ScaleDownToFit && (Drawable.DrawSize.X > DrawSize.X || Drawable.DrawSize.Y > DrawSize.Y)); + if (Drawable == null || (isDefault && !ApplySizeRestrictionsToDefault)) return; - if (applyScaling) + switch (confineMode) { - Drawable.RelativeSizeAxes = Axes.Both; - Drawable.Size = Vector2.One; - Drawable.Scale = Vector2.One; - Drawable.FillMode = FillMode.Fit; - } - } + case ConfineMode.NoScaling: + return; - scaling.Validate(); + case ConfineMode.ScaleDownToFit: + if (Drawable.DrawSize.X <= DrawSize.X && Drawable.DrawSize.Y <= DrawSize.Y) + return; + + break; + } + + Drawable.RelativeSizeAxes = Axes.Both; + Drawable.Size = Vector2.One; + Drawable.Scale = Vector2.One; + Drawable.FillMode = FillMode.Fit; + } + finally + { + scaling.Validate(); + } } } }