From 65db64e13e615de506c07d9c71816c294e414361 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Tue, 7 Apr 2020 22:41:20 +0200 Subject: [PATCH 1/3] Add failing test cases --- .../Skinning/LegacySkinTextureFallbackTest.cs | 109 ++++++++++++++++++ 1 file changed, 109 insertions(+) create mode 100644 osu.Game.Tests/NonVisual/Skinning/LegacySkinTextureFallbackTest.cs diff --git a/osu.Game.Tests/NonVisual/Skinning/LegacySkinTextureFallbackTest.cs b/osu.Game.Tests/NonVisual/Skinning/LegacySkinTextureFallbackTest.cs new file mode 100644 index 0000000000..867af9c1b8 --- /dev/null +++ b/osu.Game.Tests/NonVisual/Skinning/LegacySkinTextureFallbackTest.cs @@ -0,0 +1,109 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System.Collections.Generic; +using System.Linq; +using NUnit.Framework; +using osu.Framework.Graphics.Textures; +using osu.Game.Skinning; + +namespace osu.Game.Tests.NonVisual.Skinning +{ + [TestFixture] + public sealed class LegacySkinTextureFallbackTest + { + private static object[][] fallbackTestCases = + { + new object[] + { + // textures in store + new[] { "Gameplay/osu/followpoint@2x", "Gameplay/osu/followpoint" }, + // requested component + "Gameplay/osu/followpoint", + // returned texture name & scale + "Gameplay/osu/followpoint@2x", 2 + }, + new object[] + { + new[] { "Gameplay/osu/followpoint@2x" }, + "Gameplay/osu/followpoint", + "Gameplay/osu/followpoint@2x", 2 + }, + new object[] + { + new[] { "Gameplay/osu/followpoint" }, + "Gameplay/osu/followpoint", + "Gameplay/osu/followpoint", 1 + }, + new object[] + { + new[] { "Gameplay/osu/followpoint", "followpoint@2x" }, + "Gameplay/osu/followpoint", + "Gameplay/osu/followpoint", 1 + }, + new object[] + { + new[] { "followpoint@2x", "followpoint" }, + "Gameplay/osu/followpoint", + "followpoint@2x", 2 + }, + new object[] + { + new[] { "followpoint@2x" }, + "Gameplay/osu/followpoint", + "followpoint@2x", 2 + }, + new object[] + { + new[] { "followpoint" }, + "Gameplay/osu/followpoint", + "followpoint", 1 + }, + }; + + [TestCaseSource(nameof(fallbackTestCases))] + public void TestFallbackOrder(string[] filesInStore, string requestedComponent, string expectedTexture, float expectedScale) + { + var textureStore = new TestTextureStore(filesInStore); + var legacySkin = new TestLegacySkin(textureStore); + + var texture = legacySkin.GetTexture(requestedComponent); + + Assert.IsNotNull(texture); + Assert.AreEqual(textureStore.Textures[expectedTexture], texture); + Assert.AreEqual(expectedScale, texture.ScaleAdjust); + } + + [Test] + public void TestReturnNullOnFallbackFailure() + { + var textureStore = new TestTextureStore("sliderb", "hit100"); + var legacySkin = new TestLegacySkin(textureStore); + + var texture = legacySkin.GetTexture("Gameplay/osu/followpoint"); + + Assert.IsNull(texture); + } + + private class TestLegacySkin : LegacySkin + { + public TestLegacySkin(TextureStore textureStore) + : base(new SkinInfo(), null, null, string.Empty) + { + Textures = textureStore; + } + } + + private class TestTextureStore : TextureStore + { + public readonly Dictionary Textures; + + public TestTextureStore(params string[] fileNames) + { + Textures = fileNames.ToDictionary(fileName => fileName, fileName => new Texture(1, 1)); + } + + public override Texture Get(string name) => Textures.GetValueOrDefault(name); + } + } +} From f5f0b94944af1f1455859420aba1ed4f2fed083c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Tue, 7 Apr 2020 22:50:25 +0200 Subject: [PATCH 2/3] Fix incorrect fallback logic The recently-modified skin texture fallback logic was very subtly incorrect. If at the end of the first loop no texture was found, it would be checked for null to avoid setting scale adjust on a null texture, but then returned anyway, bypassing the fallback logic for subsequent possible paths entirely. Invert the check and explicitly continue to the next fallback path if neither a 2x, nor 1x texture with the given name is found in the store. --- osu.Game/Skinning/LegacySkin.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/osu.Game/Skinning/LegacySkin.cs b/osu.Game/Skinning/LegacySkin.cs index 60eb3d8e51..ea1cc203d7 100644 --- a/osu.Game/Skinning/LegacySkin.cs +++ b/osu.Game/Skinning/LegacySkin.cs @@ -293,9 +293,10 @@ namespace osu.Game.Skinning texture = Textures?.Get(name); } - if (texture != null) - texture.ScaleAdjust = ratio; + if (texture == null) + continue; + texture.ScaleAdjust = ratio; return texture; } From 737a3b608a925aff03cbe91214a2f8f3ecc2df7d Mon Sep 17 00:00:00 2001 From: Alchyr Date: Tue, 7 Apr 2020 17:34:18 -0700 Subject: [PATCH 3/3] Correct spelling --- osu.Game/Screens/Menu/MainMenu.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Screens/Menu/MainMenu.cs b/osu.Game/Screens/Menu/MainMenu.cs index dcee5e83b7..174eadfe26 100644 --- a/osu.Game/Screens/Menu/MainMenu.cs +++ b/osu.Game/Screens/Menu/MainMenu.cs @@ -294,7 +294,7 @@ namespace osu.Game.Screens.Menu { new PopupDialogOkButton { - Text = @"Good bye", + Text = @"Goodbye", Action = confirm }, new PopupDialogCancelButton