From 9806d94b745348e7c44af36cc93b76401234d225 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Sat, 15 May 2021 17:56:19 +0300 Subject: [PATCH 01/19] Move beatmap skin info creation to static method at `IBeatmapSkin` --- osu.Game/Skinning/BeatmapSkinExtensions.cs | 16 ++++++++++++++++ osu.Game/Skinning/LegacyBeatmapSkin.cs | 5 +---- 2 files changed, 17 insertions(+), 4 deletions(-) create mode 100644 osu.Game/Skinning/BeatmapSkinExtensions.cs diff --git a/osu.Game/Skinning/BeatmapSkinExtensions.cs b/osu.Game/Skinning/BeatmapSkinExtensions.cs new file mode 100644 index 0000000000..18ef09c392 --- /dev/null +++ b/osu.Game/Skinning/BeatmapSkinExtensions.cs @@ -0,0 +1,16 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using osu.Game.Beatmaps; + +namespace osu.Game.Skinning +{ + public static class BeatmapSkinExtensions + { + public static SkinInfo CreateSkinInfo(BeatmapInfo beatmap) => new SkinInfo + { + Name = beatmap.ToString(), + Creator = beatmap.Metadata?.AuthorString, + }; + } +} diff --git a/osu.Game/Skinning/LegacyBeatmapSkin.cs b/osu.Game/Skinning/LegacyBeatmapSkin.cs index 3ec205e897..5ee436e8bb 100644 --- a/osu.Game/Skinning/LegacyBeatmapSkin.cs +++ b/osu.Game/Skinning/LegacyBeatmapSkin.cs @@ -17,7 +17,7 @@ namespace osu.Game.Skinning protected override bool UseCustomSampleBanks => true; public LegacyBeatmapSkin(BeatmapInfo beatmap, IResourceStore storage, IStorageResourceProvider resources) - : base(createSkinInfo(beatmap), new LegacySkinResourceStore(beatmap.BeatmapSet, storage), resources, beatmap.Path) + : base(BeatmapSkinExtensions.CreateSkinInfo(beatmap), new LegacySkinResourceStore(beatmap.BeatmapSet, storage), resources, beatmap.Path) { // Disallow default colours fallback on beatmap skins to allow using parent skin combo colours. (via SkinProvidingContainer) Configuration.AllowDefaultComboColoursFallback = false; @@ -49,8 +49,5 @@ namespace osu.Game.Skinning return base.GetSample(sampleInfo); } - - private static SkinInfo createSkinInfo(BeatmapInfo beatmap) => - new SkinInfo { Name = beatmap.ToString(), Creator = beatmap.Metadata.Author.ToString() }; } } From 825d61e22c92646d3166f65d7a44ae631b8fb343 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Wed, 19 May 2021 12:06:09 +0300 Subject: [PATCH 02/19] Separate default beatmap skins from `DefaultSkin` --- osu.Game/Beatmaps/WorkingBeatmap.cs | 2 +- osu.Game/Skinning/BeatmapSkin.cs | 32 +++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) create mode 100644 osu.Game/Skinning/BeatmapSkin.cs diff --git a/osu.Game/Beatmaps/WorkingBeatmap.cs b/osu.Game/Beatmaps/WorkingBeatmap.cs index 3576b149bf..ead8572c54 100644 --- a/osu.Game/Beatmaps/WorkingBeatmap.cs +++ b/osu.Game/Beatmaps/WorkingBeatmap.cs @@ -324,7 +324,7 @@ namespace osu.Game.Beatmaps public bool SkinLoaded => skin.IsResultAvailable; public ISkin Skin => skin.Value; - protected virtual ISkin GetSkin() => new DefaultSkin(null); + protected virtual ISkin GetSkin() => new BeatmapSkin(BeatmapInfo); private readonly RecyclableLazy skin; public abstract Stream GetStream(string storagePath); diff --git a/osu.Game/Skinning/BeatmapSkin.cs b/osu.Game/Skinning/BeatmapSkin.cs new file mode 100644 index 0000000000..14b845faeb --- /dev/null +++ b/osu.Game/Skinning/BeatmapSkin.cs @@ -0,0 +1,32 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using osu.Framework.Audio.Sample; +using osu.Framework.Bindables; +using osu.Framework.Graphics.OpenGL.Textures; +using osu.Framework.Graphics.Textures; +using osu.Game.Audio; +using osu.Game.Beatmaps; + +namespace osu.Game.Skinning +{ + /// + /// An empty implementation of a beatmap skin, serves as a temporary default for s. + /// + /// + /// This should be removed once becomes instantiable or a new skin type for osu!lazer beatmaps is defined. + /// + public class BeatmapSkin : Skin + { + public BeatmapSkin(BeatmapInfo beatmap) + : base(BeatmapSkinExtensions.CreateSkinInfo(beatmap), null) + { + } + + public override Texture GetTexture(string componentName, WrapMode wrapModeS, WrapMode wrapModeT) => null; + + public override IBindable GetConfig(TLookup lookup) => null; + + public override ISample GetSample(ISampleInfo sampleInfo) => null; + } +} From ec89a149dd752d242e1675a4462c8af4e360eb3e Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Wed, 19 May 2021 13:36:10 +0300 Subject: [PATCH 03/19] Add failing test case --- .../TestSceneCatchPlayerLegacySkin.cs | 40 ++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/osu.Game.Rulesets.Catch.Tests/TestSceneCatchPlayerLegacySkin.cs b/osu.Game.Rulesets.Catch.Tests/TestSceneCatchPlayerLegacySkin.cs index 64695153b5..a0b6b6dbe1 100644 --- a/osu.Game.Rulesets.Catch.Tests/TestSceneCatchPlayerLegacySkin.cs +++ b/osu.Game.Rulesets.Catch.Tests/TestSceneCatchPlayerLegacySkin.cs @@ -1,7 +1,13 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. +using System.Linq; using NUnit.Framework; +using osu.Framework.Graphics; +using osu.Framework.Testing; +using osu.Game.Beatmaps; +using osu.Game.Rulesets.Catch.Skinning.Legacy; +using osu.Game.Skinning; using osu.Game.Tests.Visual; namespace osu.Game.Rulesets.Catch.Tests @@ -9,6 +15,38 @@ namespace osu.Game.Rulesets.Catch.Tests [TestFixture] public class TestSceneCatchPlayerLegacySkin : LegacySkinPlayerTestScene { - protected override Ruleset CreatePlayerRuleset() => new CatchRuleset(); + [Test] + public void TestUsingLegacySkin() + { + // check for the existence of a random legacy component to ensure using legacy skin. + // this should exist in LegacySkinPlayerTestScene but the weird transformer logic below needs to be "fixed" or otherwise first. + AddAssert("using legacy skin", () => this.ChildrenOfType().Any()); + } + + protected override Ruleset CreatePlayerRuleset() => new TestCatchRuleset(); + + private class TestCatchRuleset : CatchRuleset + { + public override ISkin CreateLegacySkinProvider(ISkinSource source, IBeatmap beatmap) => new TestCatchLegacySkinTransformer(source); + } + + private class TestCatchLegacySkinTransformer : CatchLegacySkinTransformer + { + public TestCatchLegacySkinTransformer(ISkinSource source) + : base(source) + { + } + + public override Drawable GetDrawableComponent(ISkinComponent component) + { + var drawable = base.GetDrawableComponent(component); + if (drawable != null) + return drawable; + + // it shouldn't really matter whether to return null or return this, + // but returning null skips over the beatmap skin, so this needs to exist to test things properly. + return Source.GetDrawableComponent(component); + } + } } } From 434377aa52300a5af7698733722f462196434840 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Wed, 19 May 2021 21:35:06 +0300 Subject: [PATCH 04/19] Revert "Add failing test case" This reverts commit ec89a149dd752d242e1675a4462c8af4e360eb3e. --- .../TestSceneCatchPlayerLegacySkin.cs | 40 +------------------ 1 file changed, 1 insertion(+), 39 deletions(-) diff --git a/osu.Game.Rulesets.Catch.Tests/TestSceneCatchPlayerLegacySkin.cs b/osu.Game.Rulesets.Catch.Tests/TestSceneCatchPlayerLegacySkin.cs index a0b6b6dbe1..64695153b5 100644 --- a/osu.Game.Rulesets.Catch.Tests/TestSceneCatchPlayerLegacySkin.cs +++ b/osu.Game.Rulesets.Catch.Tests/TestSceneCatchPlayerLegacySkin.cs @@ -1,13 +1,7 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -using System.Linq; using NUnit.Framework; -using osu.Framework.Graphics; -using osu.Framework.Testing; -using osu.Game.Beatmaps; -using osu.Game.Rulesets.Catch.Skinning.Legacy; -using osu.Game.Skinning; using osu.Game.Tests.Visual; namespace osu.Game.Rulesets.Catch.Tests @@ -15,38 +9,6 @@ namespace osu.Game.Rulesets.Catch.Tests [TestFixture] public class TestSceneCatchPlayerLegacySkin : LegacySkinPlayerTestScene { - [Test] - public void TestUsingLegacySkin() - { - // check for the existence of a random legacy component to ensure using legacy skin. - // this should exist in LegacySkinPlayerTestScene but the weird transformer logic below needs to be "fixed" or otherwise first. - AddAssert("using legacy skin", () => this.ChildrenOfType().Any()); - } - - protected override Ruleset CreatePlayerRuleset() => new TestCatchRuleset(); - - private class TestCatchRuleset : CatchRuleset - { - public override ISkin CreateLegacySkinProvider(ISkinSource source, IBeatmap beatmap) => new TestCatchLegacySkinTransformer(source); - } - - private class TestCatchLegacySkinTransformer : CatchLegacySkinTransformer - { - public TestCatchLegacySkinTransformer(ISkinSource source) - : base(source) - { - } - - public override Drawable GetDrawableComponent(ISkinComponent component) - { - var drawable = base.GetDrawableComponent(component); - if (drawable != null) - return drawable; - - // it shouldn't really matter whether to return null or return this, - // but returning null skips over the beatmap skin, so this needs to exist to test things properly. - return Source.GetDrawableComponent(component); - } - } + protected override Ruleset CreatePlayerRuleset() => new CatchRuleset(); } } From 6fc06a10a15037bb4890fc2d35ffc46d74bf8df5 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Wed, 19 May 2021 21:52:29 +0300 Subject: [PATCH 05/19] Add extensible test scene for beatmap skins fallback instead --- .../Gameplay/TestSceneBeatmapSkinFallbacks.cs | 129 ++++++++++++++++++ 1 file changed, 129 insertions(+) create mode 100644 osu.Game.Tests/Visual/Gameplay/TestSceneBeatmapSkinFallbacks.cs diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneBeatmapSkinFallbacks.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneBeatmapSkinFallbacks.cs new file mode 100644 index 0000000000..760fc5a139 --- /dev/null +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneBeatmapSkinFallbacks.cs @@ -0,0 +1,129 @@ +// 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.Linq; +using NUnit.Framework; +using osu.Framework.Allocation; +using osu.Framework.Audio; +using osu.Framework.Graphics; +using osu.Framework.Lists; +using osu.Framework.Testing; +using osu.Framework.Timing; +using osu.Framework.Utils; +using osu.Game.Beatmaps; +using osu.Game.Extensions; +using osu.Game.Rulesets; +using osu.Game.Rulesets.Osu; +using osu.Game.Rulesets.Osu.Skinning.Legacy; +using osu.Game.Rulesets.Scoring; +using osu.Game.Screens.Play.HUD; +using osu.Game.Skinning; +using osu.Game.Storyboards; +using osu.Game.Tests.Beatmaps; + +namespace osu.Game.Tests.Visual.Gameplay +{ + public class TestSceneBeatmapSkinFallbacks : OsuPlayerTestScene + { + private ISkin currentBeatmapSkin; + + [Resolved] + private SkinManager skinManager { get; set; } + + [Cached] + private ScoreProcessor scoreProcessor = new ScoreProcessor(); + + [Cached(typeof(HealthProcessor))] + private HealthProcessor healthProcessor = new DrainingHealthProcessor(0); + + protected override bool HasCustomSteps => true; + + [Test] + public void TestEmptyDefaultBeatmapSkinFallsBack() + { + CreateSkinTest(DefaultLegacySkin.Info, () => new TestWorkingBeatmap(CreateBeatmap(new OsuRuleset().RulesetInfo)).Skin); + AssertComponentsFromExpectedSource(SkinnableTarget.MainHUDComponents, () => skinManager.CurrentSkin.Value); + } + + protected void CreateSkinTest(SkinInfo gameCurrentSkin, Func beatmapSkin) + { + CreateTest(() => + { + AddStep("setup skins", () => + { + skinManager.CurrentSkinInfo.Value = gameCurrentSkin; + currentBeatmapSkin = beatmapSkin(); + }); + }); + } + + protected void AssertComponentsFromExpectedSource(SkinnableTarget target, Func expectedSource) + { + AddAssert($"{target} from {expectedSource.GetType().Name}", () => + { + var expectedComponentsContainer = (SkinnableTargetComponentsContainer)expectedSource().GetDrawableComponent(new SkinnableTargetComponent(target)); + + Add(expectedComponentsContainer); + expectedComponentsContainer?.UpdateSubTree(); + var expectedInfo = expectedComponentsContainer?.CreateSkinnableInfo(); + Remove(expectedComponentsContainer); + + var actualInfo = Player.ChildrenOfType().First(s => s.Target == target) + .ChildrenOfType().Single().CreateSkinnableInfo(); + + return almostEqual(actualInfo, expectedInfo, 2f); + }); + + static bool almostEqual(SkinnableInfo info, SkinnableInfo other, float positionTolerance) => + other != null + && info.Anchor == other.Anchor + && info.Origin == other.Origin + && Precision.AlmostEquals(info.Position, other.Position, positionTolerance) + && info.Children.SequenceEqual(other.Children, new FuncEqualityComparer((s1, s2) => almostEqual(s1, s2, positionTolerance))); + } + + protected override WorkingBeatmap CreateWorkingBeatmap(IBeatmap beatmap, Storyboard storyboard = null) + => new CustomSkinWorkingBeatmap(beatmap, storyboard, Clock, Audio, currentBeatmapSkin); + + protected override Ruleset CreatePlayerRuleset() => new TestOsuRuleset(); + + private class CustomSkinWorkingBeatmap : ClockBackedTestWorkingBeatmap + { + private readonly ISkin beatmapSkin; + + public CustomSkinWorkingBeatmap(IBeatmap beatmap, Storyboard storyboard, IFrameBasedClock referenceClock, AudioManager audio, ISkin beatmapSkin) + : base(beatmap, storyboard, referenceClock, audio) + { + this.beatmapSkin = beatmapSkin; + } + + protected override ISkin GetSkin() => beatmapSkin; + } + + private class TestOsuRuleset : OsuRuleset + { + public override ISkin CreateLegacySkinProvider(ISkinSource source, IBeatmap beatmap) => new TestOsuLegacySkinTransformer(source); + + private class TestOsuLegacySkinTransformer : OsuLegacySkinTransformer + { + public TestOsuLegacySkinTransformer(ISkinSource source) + : base(source) + { + } + + public override Drawable GetDrawableComponent(ISkinComponent component) + { + var drawable = base.GetDrawableComponent(component); + if (drawable != null) + return drawable; + + // this isn't really supposed to make a difference from returning null, + // but it appears it does, returning null will skip over falling back to beatmap skin, + // while calling Source.GetDrawableComponent() doesn't. + return Source.GetDrawableComponent(component); + } + } + } + } +} From 71da9600009ce891be550e07be5cafda3b9ec065 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Wed, 19 May 2021 22:53:21 +0300 Subject: [PATCH 06/19] Extract assert step addition out of assertion method --- .../Gameplay/TestSceneBeatmapSkinFallbacks.cs | 27 +++++++++---------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneBeatmapSkinFallbacks.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneBeatmapSkinFallbacks.cs index 760fc5a139..1d0caa86e2 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneBeatmapSkinFallbacks.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneBeatmapSkinFallbacks.cs @@ -43,37 +43,34 @@ namespace osu.Game.Tests.Visual.Gameplay public void TestEmptyDefaultBeatmapSkinFallsBack() { CreateSkinTest(DefaultLegacySkin.Info, () => new TestWorkingBeatmap(CreateBeatmap(new OsuRuleset().RulesetInfo)).Skin); - AssertComponentsFromExpectedSource(SkinnableTarget.MainHUDComponents, () => skinManager.CurrentSkin.Value); + AddAssert("hud from default legacy skin", () => AssertComponentsFromExpectedSource(SkinnableTarget.MainHUDComponents, skinManager.CurrentSkin.Value)); } - protected void CreateSkinTest(SkinInfo gameCurrentSkin, Func beatmapSkin) + protected void CreateSkinTest(SkinInfo gameCurrentSkin, Func getBeatmapSkin) { CreateTest(() => { AddStep("setup skins", () => { skinManager.CurrentSkinInfo.Value = gameCurrentSkin; - currentBeatmapSkin = beatmapSkin(); + currentBeatmapSkin = getBeatmapSkin(); }); }); } - protected void AssertComponentsFromExpectedSource(SkinnableTarget target, Func expectedSource) + protected bool AssertComponentsFromExpectedSource(SkinnableTarget target, ISkin expectedSource) { - AddAssert($"{target} from {expectedSource.GetType().Name}", () => - { - var expectedComponentsContainer = (SkinnableTargetComponentsContainer)expectedSource().GetDrawableComponent(new SkinnableTargetComponent(target)); + var expectedComponentsContainer = (SkinnableTargetComponentsContainer)expectedSource.GetDrawableComponent(new SkinnableTargetComponent(target)); - Add(expectedComponentsContainer); - expectedComponentsContainer?.UpdateSubTree(); - var expectedInfo = expectedComponentsContainer?.CreateSkinnableInfo(); - Remove(expectedComponentsContainer); + Add(expectedComponentsContainer); + expectedComponentsContainer?.UpdateSubTree(); + var expectedInfo = expectedComponentsContainer?.CreateSkinnableInfo(); + Remove(expectedComponentsContainer); - var actualInfo = Player.ChildrenOfType().First(s => s.Target == target) - .ChildrenOfType().Single().CreateSkinnableInfo(); + var actualInfo = Player.ChildrenOfType().First(s => s.Target == target) + .ChildrenOfType().Single().CreateSkinnableInfo(); - return almostEqual(actualInfo, expectedInfo, 2f); - }); + return almostEqual(actualInfo, expectedInfo, 2f); static bool almostEqual(SkinnableInfo info, SkinnableInfo other, float positionTolerance) => other != null From 262a27610c94dbaae44925e88534166b34318fd9 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Thu, 20 May 2021 20:47:40 +0300 Subject: [PATCH 07/19] Improve components assertion logic --- .../Gameplay/TestSceneBeatmapSkinFallbacks.cs | 39 ++++++++++++++----- 1 file changed, 29 insertions(+), 10 deletions(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneBeatmapSkinFallbacks.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneBeatmapSkinFallbacks.cs index 1d0caa86e2..dd77a71ca5 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneBeatmapSkinFallbacks.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneBeatmapSkinFallbacks.cs @@ -7,7 +7,9 @@ using NUnit.Framework; using osu.Framework.Allocation; using osu.Framework.Audio; using osu.Framework.Graphics; +using osu.Framework.Graphics.Containers; using osu.Framework.Lists; +using osu.Framework.Logging; using osu.Framework.Testing; using osu.Framework.Timing; using osu.Framework.Utils; @@ -60,24 +62,41 @@ namespace osu.Game.Tests.Visual.Gameplay protected bool AssertComponentsFromExpectedSource(SkinnableTarget target, ISkin expectedSource) { + var actualComponentsContainer = Player.ChildrenOfType().First(s => s.Target == target) + .ChildrenOfType().SingleOrDefault(); + + if (actualComponentsContainer == null) + return false; + + var actualInfo = actualComponentsContainer.CreateSkinnableInfo(); + var expectedComponentsContainer = (SkinnableTargetComponentsContainer)expectedSource.GetDrawableComponent(new SkinnableTargetComponent(target)); + if (expectedComponentsContainer == null) + return false; - Add(expectedComponentsContainer); - expectedComponentsContainer?.UpdateSubTree(); + var expectedComponentsAdjustmentContainer = new Container + { + Position = actualComponentsContainer.Parent.ToSpaceOfOtherDrawable(actualComponentsContainer.DrawPosition, Content), + Size = actualComponentsContainer.DrawSize, + Child = expectedComponentsContainer, + }; + + Add(expectedComponentsAdjustmentContainer); + expectedComponentsAdjustmentContainer?.UpdateSubTree(); var expectedInfo = expectedComponentsContainer?.CreateSkinnableInfo(); - Remove(expectedComponentsContainer); + Remove(expectedComponentsAdjustmentContainer); - var actualInfo = Player.ChildrenOfType().First(s => s.Target == target) - .ChildrenOfType().Single().CreateSkinnableInfo(); + return almostEqual(actualInfo, expectedInfo); - return almostEqual(actualInfo, expectedInfo, 2f); - - static bool almostEqual(SkinnableInfo info, SkinnableInfo other, float positionTolerance) => + static bool almostEqual(SkinnableInfo info, SkinnableInfo other) => other != null + && info.Type == other.Type && info.Anchor == other.Anchor && info.Origin == other.Origin - && Precision.AlmostEquals(info.Position, other.Position, positionTolerance) - && info.Children.SequenceEqual(other.Children, new FuncEqualityComparer((s1, s2) => almostEqual(s1, s2, positionTolerance))); + && Precision.AlmostEquals(info.Position, other.Position) + && Precision.AlmostEquals(info.Scale, other.Scale) + && Precision.AlmostEquals(info.Rotation, other.Rotation) + && info.Children.SequenceEqual(other.Children, new FuncEqualityComparer(almostEqual)); } protected override WorkingBeatmap CreateWorkingBeatmap(IBeatmap beatmap, Storyboard storyboard = null) From eaae9a1b678ae64b3c680c7ff1d172205b122396 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Thu, 20 May 2021 21:08:31 +0300 Subject: [PATCH 08/19] Remove unrequired null conditional --- .../Visual/Gameplay/TestSceneBeatmapSkinFallbacks.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneBeatmapSkinFallbacks.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneBeatmapSkinFallbacks.cs index dd77a71ca5..ecdb6fa4af 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneBeatmapSkinFallbacks.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneBeatmapSkinFallbacks.cs @@ -82,8 +82,8 @@ namespace osu.Game.Tests.Visual.Gameplay }; Add(expectedComponentsAdjustmentContainer); - expectedComponentsAdjustmentContainer?.UpdateSubTree(); - var expectedInfo = expectedComponentsContainer?.CreateSkinnableInfo(); + expectedComponentsAdjustmentContainer.UpdateSubTree(); + var expectedInfo = expectedComponentsContainer.CreateSkinnableInfo(); Remove(expectedComponentsAdjustmentContainer); return almostEqual(actualInfo, expectedInfo); From c0dfe379655d9e68496d0de3a6182a764c1f904f Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Thu, 20 May 2021 21:08:36 +0300 Subject: [PATCH 09/19] Remove unused using directive --- osu.Game.Tests/Visual/Gameplay/TestSceneBeatmapSkinFallbacks.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneBeatmapSkinFallbacks.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneBeatmapSkinFallbacks.cs index ecdb6fa4af..22959c082a 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneBeatmapSkinFallbacks.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneBeatmapSkinFallbacks.cs @@ -9,7 +9,6 @@ using osu.Framework.Audio; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Framework.Lists; -using osu.Framework.Logging; using osu.Framework.Testing; using osu.Framework.Timing; using osu.Framework.Utils; From 318e5fc60bf5455653e8e2ae9846360b0b531542 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Fri, 21 May 2021 20:14:26 +0300 Subject: [PATCH 10/19] Mark `WorkingBeatmap.GetSkin()` as abstract --- osu.Game/Beatmaps/WorkingBeatmap.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Beatmaps/WorkingBeatmap.cs b/osu.Game/Beatmaps/WorkingBeatmap.cs index ead8572c54..0a55678fb7 100644 --- a/osu.Game/Beatmaps/WorkingBeatmap.cs +++ b/osu.Game/Beatmaps/WorkingBeatmap.cs @@ -324,7 +324,7 @@ namespace osu.Game.Beatmaps public bool SkinLoaded => skin.IsResultAvailable; public ISkin Skin => skin.Value; - protected virtual ISkin GetSkin() => new BeatmapSkin(BeatmapInfo); + protected abstract ISkin GetSkin(); private readonly RecyclableLazy skin; public abstract Stream GetStream(string storagePath); From 04e75d8f2b19ab70dd32a49922dc1157e304fb2b Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Fri, 21 May 2021 20:16:51 +0300 Subject: [PATCH 11/19] Return empty skin for `GetSkin()` in `TestWorkingBeatmap` --- osu.Game/Tests/Beatmaps/TestWorkingBeatmap.cs | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/osu.Game/Tests/Beatmaps/TestWorkingBeatmap.cs b/osu.Game/Tests/Beatmaps/TestWorkingBeatmap.cs index 852006bc9b..2e4a9fa28f 100644 --- a/osu.Game/Tests/Beatmaps/TestWorkingBeatmap.cs +++ b/osu.Game/Tests/Beatmaps/TestWorkingBeatmap.cs @@ -3,9 +3,15 @@ using System.IO; using osu.Framework.Audio; +using osu.Framework.Audio.Sample; using osu.Framework.Audio.Track; +using osu.Framework.Bindables; +using osu.Framework.Graphics; +using osu.Framework.Graphics.OpenGL.Textures; using osu.Framework.Graphics.Textures; +using osu.Game.Audio; using osu.Game.Beatmaps; +using osu.Game.Skinning; using osu.Game.Storyboards; namespace osu.Game.Tests.Beatmaps @@ -36,10 +42,23 @@ namespace osu.Game.Tests.Beatmaps protected override Storyboard GetStoryboard() => storyboard ?? base.GetStoryboard(); + protected override ISkin GetSkin() => new EmptySkin(); + public override Stream GetStream(string storagePath) => null; protected override Texture GetBackground() => null; protected override Track GetBeatmapTrack() => null; + + private class EmptySkin : ISkin + { + public Drawable GetDrawableComponent(ISkinComponent component) => null; + + public Texture GetTexture(string componentName, WrapMode wrapModeS, WrapMode wrapModeT) => null; + + public ISample GetSample(ISampleInfo sampleInfo) => null; + + public IBindable GetConfig(TLookup lookup) => null; + } } } From 4f6de6fdc66a0e18aad8f2640dbaa848ea8c8888 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Fri, 21 May 2021 20:21:00 +0300 Subject: [PATCH 12/19] Implement `GetSkin()` for other working beatmaps --- osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapEncoderTest.cs | 2 ++ osu.Game.Tests/WaveformTestBeatmap.cs | 3 +++ osu.Game/Beatmaps/BeatmapManager.cs | 1 + osu.Game/Beatmaps/DummyWorkingBeatmap.cs | 3 +++ osu.Game/Screens/Edit/LegacyEditorBeatmapPatcher.cs | 3 +++ osu.Game/Tests/Beatmaps/BeatmapConversionTest.cs | 3 +++ 6 files changed, 15 insertions(+) diff --git a/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapEncoderTest.cs b/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapEncoderTest.cs index a18f82fe4a..059432eeaf 100644 --- a/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapEncoderTest.cs +++ b/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapEncoderTest.cs @@ -169,6 +169,8 @@ namespace osu.Game.Tests.Beatmaps.Formats protected override Track GetBeatmapTrack() => throw new NotImplementedException(); + protected override ISkin GetSkin() => throw new NotImplementedException(); + public override Stream GetStream(string storagePath) => throw new NotImplementedException(); } } diff --git a/osu.Game.Tests/WaveformTestBeatmap.cs b/osu.Game.Tests/WaveformTestBeatmap.cs index cbed28641c..5477e4a0f8 100644 --- a/osu.Game.Tests/WaveformTestBeatmap.cs +++ b/osu.Game.Tests/WaveformTestBeatmap.cs @@ -11,6 +11,7 @@ using osu.Game.Beatmaps; using osu.Game.IO.Archives; using osu.Game.Rulesets; using osu.Game.Rulesets.Osu; +using osu.Game.Skinning; using osu.Game.Tests.Beatmaps; using osu.Game.Tests.Resources; @@ -52,6 +53,8 @@ namespace osu.Game.Tests protected override Waveform GetWaveform() => new Waveform(trackStore.GetStream(firstAudioFile)); + protected override ISkin GetSkin() => null; + public override Stream GetStream(string storagePath) => null; protected override Track GetBeatmapTrack() => trackStore.Get(firstAudioFile); diff --git a/osu.Game/Beatmaps/BeatmapManager.cs b/osu.Game/Beatmaps/BeatmapManager.cs index 5e975de77c..46e3a4f6d7 100644 --- a/osu.Game/Beatmaps/BeatmapManager.cs +++ b/osu.Game/Beatmaps/BeatmapManager.cs @@ -526,6 +526,7 @@ namespace osu.Game.Beatmaps protected override IBeatmap GetBeatmap() => beatmap; protected override Texture GetBackground() => null; protected override Track GetBeatmapTrack() => null; + protected override ISkin GetSkin() => null; public override Stream GetStream(string storagePath) => null; } } diff --git a/osu.Game/Beatmaps/DummyWorkingBeatmap.cs b/osu.Game/Beatmaps/DummyWorkingBeatmap.cs index 6922d1c286..ea7f45e53f 100644 --- a/osu.Game/Beatmaps/DummyWorkingBeatmap.cs +++ b/osu.Game/Beatmaps/DummyWorkingBeatmap.cs @@ -15,6 +15,7 @@ using osu.Game.Rulesets.Difficulty; using osu.Game.Rulesets.Mods; using osu.Game.Rulesets.Objects; using osu.Game.Rulesets.UI; +using osu.Game.Skinning; namespace osu.Game.Beatmaps { @@ -49,6 +50,8 @@ namespace osu.Game.Beatmaps protected override Track GetBeatmapTrack() => GetVirtualTrack(); + protected override ISkin GetSkin() => null; + public override Stream GetStream(string storagePath) => null; private class DummyRulesetInfo : RulesetInfo diff --git a/osu.Game/Screens/Edit/LegacyEditorBeatmapPatcher.cs b/osu.Game/Screens/Edit/LegacyEditorBeatmapPatcher.cs index 66784fda54..6f92db98ee 100644 --- a/osu.Game/Screens/Edit/LegacyEditorBeatmapPatcher.cs +++ b/osu.Game/Screens/Edit/LegacyEditorBeatmapPatcher.cs @@ -11,6 +11,7 @@ using osu.Framework.Audio.Track; using osu.Framework.Graphics.Textures; using osu.Game.Beatmaps; using osu.Game.IO; +using osu.Game.Skinning; using Decoder = osu.Game.Beatmaps.Formats.Decoder; namespace osu.Game.Screens.Edit @@ -117,6 +118,8 @@ namespace osu.Game.Screens.Edit protected override Track GetBeatmapTrack() => throw new NotImplementedException(); + protected override ISkin GetSkin() => throw new NotImplementedException(); + public override Stream GetStream(string storagePath) => throw new NotImplementedException(); } } diff --git a/osu.Game/Tests/Beatmaps/BeatmapConversionTest.cs b/osu.Game/Tests/Beatmaps/BeatmapConversionTest.cs index a97f6defe9..4935f7fc13 100644 --- a/osu.Game/Tests/Beatmaps/BeatmapConversionTest.cs +++ b/osu.Game/Tests/Beatmaps/BeatmapConversionTest.cs @@ -17,6 +17,7 @@ using osu.Game.IO; using osu.Game.Rulesets; using osu.Game.Rulesets.Mods; using osu.Game.Rulesets.Objects; +using osu.Game.Skinning; namespace osu.Game.Tests.Beatmaps { @@ -216,6 +217,8 @@ namespace osu.Game.Tests.Beatmaps protected override Track GetBeatmapTrack() => throw new NotImplementedException(); + protected override ISkin GetSkin() => throw new NotImplementedException(); + public override Stream GetStream(string storagePath) => throw new NotImplementedException(); protected override IBeatmapConverter CreateBeatmapConverter(IBeatmap beatmap, Ruleset ruleset) From afb33f1641a924b04bd28f3cf52af74c7d730af1 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Fri, 21 May 2021 20:21:50 +0300 Subject: [PATCH 13/19] Remove no longer necessary test case --- .../Visual/Gameplay/TestSceneBeatmapSkinFallbacks.cs | 9 --------- 1 file changed, 9 deletions(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneBeatmapSkinFallbacks.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneBeatmapSkinFallbacks.cs index 22959c082a..bece80903f 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneBeatmapSkinFallbacks.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneBeatmapSkinFallbacks.cs @@ -3,7 +3,6 @@ using System; using System.Linq; -using NUnit.Framework; using osu.Framework.Allocation; using osu.Framework.Audio; using osu.Framework.Graphics; @@ -21,7 +20,6 @@ using osu.Game.Rulesets.Scoring; using osu.Game.Screens.Play.HUD; using osu.Game.Skinning; using osu.Game.Storyboards; -using osu.Game.Tests.Beatmaps; namespace osu.Game.Tests.Visual.Gameplay { @@ -40,13 +38,6 @@ namespace osu.Game.Tests.Visual.Gameplay protected override bool HasCustomSteps => true; - [Test] - public void TestEmptyDefaultBeatmapSkinFallsBack() - { - CreateSkinTest(DefaultLegacySkin.Info, () => new TestWorkingBeatmap(CreateBeatmap(new OsuRuleset().RulesetInfo)).Skin); - AddAssert("hud from default legacy skin", () => AssertComponentsFromExpectedSource(SkinnableTarget.MainHUDComponents, skinManager.CurrentSkin.Value)); - } - protected void CreateSkinTest(SkinInfo gameCurrentSkin, Func getBeatmapSkin) { CreateTest(() => From 7792efb154f271e528f4ed0df0ddf966123d5208 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Mon, 24 May 2021 12:28:06 +0300 Subject: [PATCH 14/19] Remove no longer used `BeatmapSkin` --- osu.Game/Skinning/BeatmapSkin.cs | 32 -------------------------------- 1 file changed, 32 deletions(-) delete mode 100644 osu.Game/Skinning/BeatmapSkin.cs diff --git a/osu.Game/Skinning/BeatmapSkin.cs b/osu.Game/Skinning/BeatmapSkin.cs deleted file mode 100644 index 14b845faeb..0000000000 --- a/osu.Game/Skinning/BeatmapSkin.cs +++ /dev/null @@ -1,32 +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 osu.Framework.Audio.Sample; -using osu.Framework.Bindables; -using osu.Framework.Graphics.OpenGL.Textures; -using osu.Framework.Graphics.Textures; -using osu.Game.Audio; -using osu.Game.Beatmaps; - -namespace osu.Game.Skinning -{ - /// - /// An empty implementation of a beatmap skin, serves as a temporary default for s. - /// - /// - /// This should be removed once becomes instantiable or a new skin type for osu!lazer beatmaps is defined. - /// - public class BeatmapSkin : Skin - { - public BeatmapSkin(BeatmapInfo beatmap) - : base(BeatmapSkinExtensions.CreateSkinInfo(beatmap), null) - { - } - - public override Texture GetTexture(string componentName, WrapMode wrapModeS, WrapMode wrapModeT) => null; - - public override IBindable GetConfig(TLookup lookup) => null; - - public override ISample GetSample(ISampleInfo sampleInfo) => null; - } -} From 3585e2900ed8a488d6bb6fb69fddfd6305c37e32 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Mon, 24 May 2021 12:28:30 +0300 Subject: [PATCH 15/19] Replace unnecessary empty skin implementation with null --- osu.Game/Tests/Beatmaps/TestWorkingBeatmap.cs | 18 +----------------- 1 file changed, 1 insertion(+), 17 deletions(-) diff --git a/osu.Game/Tests/Beatmaps/TestWorkingBeatmap.cs b/osu.Game/Tests/Beatmaps/TestWorkingBeatmap.cs index 2e4a9fa28f..bfce59c7de 100644 --- a/osu.Game/Tests/Beatmaps/TestWorkingBeatmap.cs +++ b/osu.Game/Tests/Beatmaps/TestWorkingBeatmap.cs @@ -3,13 +3,8 @@ using System.IO; using osu.Framework.Audio; -using osu.Framework.Audio.Sample; using osu.Framework.Audio.Track; -using osu.Framework.Bindables; -using osu.Framework.Graphics; -using osu.Framework.Graphics.OpenGL.Textures; using osu.Framework.Graphics.Textures; -using osu.Game.Audio; using osu.Game.Beatmaps; using osu.Game.Skinning; using osu.Game.Storyboards; @@ -42,23 +37,12 @@ namespace osu.Game.Tests.Beatmaps protected override Storyboard GetStoryboard() => storyboard ?? base.GetStoryboard(); - protected override ISkin GetSkin() => new EmptySkin(); + protected override ISkin GetSkin() => null; public override Stream GetStream(string storagePath) => null; protected override Texture GetBackground() => null; protected override Track GetBeatmapTrack() => null; - - private class EmptySkin : ISkin - { - public Drawable GetDrawableComponent(ISkinComponent component) => null; - - public Texture GetTexture(string componentName, WrapMode wrapModeS, WrapMode wrapModeT) => null; - - public ISample GetSample(ISampleInfo sampleInfo) => null; - - public IBindable GetConfig(TLookup lookup) => null; - } } } From 0c2d3ae0e757b486b4e28180780387a8addcdf7e Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Thu, 27 May 2021 05:08:49 +0300 Subject: [PATCH 16/19] Revert "Move beatmap skin info creation to static method at `IBeatmapSkin`" This reverts commit 9806d94b745348e7c44af36cc93b76401234d225. --- osu.Game/Skinning/BeatmapSkinExtensions.cs | 16 ---------------- osu.Game/Skinning/LegacyBeatmapSkin.cs | 5 ++++- 2 files changed, 4 insertions(+), 17 deletions(-) delete mode 100644 osu.Game/Skinning/BeatmapSkinExtensions.cs diff --git a/osu.Game/Skinning/BeatmapSkinExtensions.cs b/osu.Game/Skinning/BeatmapSkinExtensions.cs deleted file mode 100644 index 18ef09c392..0000000000 --- a/osu.Game/Skinning/BeatmapSkinExtensions.cs +++ /dev/null @@ -1,16 +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 osu.Game.Beatmaps; - -namespace osu.Game.Skinning -{ - public static class BeatmapSkinExtensions - { - public static SkinInfo CreateSkinInfo(BeatmapInfo beatmap) => new SkinInfo - { - Name = beatmap.ToString(), - Creator = beatmap.Metadata?.AuthorString, - }; - } -} diff --git a/osu.Game/Skinning/LegacyBeatmapSkin.cs b/osu.Game/Skinning/LegacyBeatmapSkin.cs index 5ee436e8bb..3ec205e897 100644 --- a/osu.Game/Skinning/LegacyBeatmapSkin.cs +++ b/osu.Game/Skinning/LegacyBeatmapSkin.cs @@ -17,7 +17,7 @@ namespace osu.Game.Skinning protected override bool UseCustomSampleBanks => true; public LegacyBeatmapSkin(BeatmapInfo beatmap, IResourceStore storage, IStorageResourceProvider resources) - : base(BeatmapSkinExtensions.CreateSkinInfo(beatmap), new LegacySkinResourceStore(beatmap.BeatmapSet, storage), resources, beatmap.Path) + : base(createSkinInfo(beatmap), new LegacySkinResourceStore(beatmap.BeatmapSet, storage), resources, beatmap.Path) { // Disallow default colours fallback on beatmap skins to allow using parent skin combo colours. (via SkinProvidingContainer) Configuration.AllowDefaultComboColoursFallback = false; @@ -49,5 +49,8 @@ namespace osu.Game.Skinning return base.GetSample(sampleInfo); } + + private static SkinInfo createSkinInfo(BeatmapInfo beatmap) => + new SkinInfo { Name = beatmap.ToString(), Creator = beatmap.Metadata.Author.ToString() }; } } From 5e8893b20336249b6139b22d2269d00b25e243ec Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 28 May 2021 10:42:13 +0900 Subject: [PATCH 17/19] Update framework --- osu.Android.props | 2 +- osu.Game/osu.Game.csproj | 2 +- osu.iOS.props | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/osu.Android.props b/osu.Android.props index b3842a528d..bcd5f9bd9a 100644 --- a/osu.Android.props +++ b/osu.Android.props @@ -52,6 +52,6 @@ - + diff --git a/osu.Game/osu.Game.csproj b/osu.Game/osu.Game.csproj index fa2945db6a..5f0f3028f0 100644 --- a/osu.Game/osu.Game.csproj +++ b/osu.Game/osu.Game.csproj @@ -33,7 +33,7 @@ all runtime; build; native; contentfiles; analyzers; buildtransitive - + diff --git a/osu.iOS.props b/osu.iOS.props index e35b1b5c42..59b026e0ad 100644 --- a/osu.iOS.props +++ b/osu.iOS.props @@ -70,7 +70,7 @@ - + @@ -93,7 +93,7 @@ - + From 2439de1c3dd3ce3ff085d41c6c1aa95e2cf1d1e2 Mon Sep 17 00:00:00 2001 From: Susko3 <16479013+Susko3@users.noreply.github.com> Date: Fri, 28 May 2021 18:40:58 +0200 Subject: [PATCH 18/19] fix capitalzation of osu!catch HitResults --- osu.Game.Rulesets.Catch/CatchRuleset.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/osu.Game.Rulesets.Catch/CatchRuleset.cs b/osu.Game.Rulesets.Catch/CatchRuleset.cs index e3c457693e..23ce444560 100644 --- a/osu.Game.Rulesets.Catch/CatchRuleset.cs +++ b/osu.Game.Rulesets.Catch/CatchRuleset.cs @@ -161,13 +161,13 @@ namespace osu.Game.Rulesets.Catch switch (result) { case HitResult.LargeTickHit: - return "large droplet"; + return "Large droplet"; case HitResult.SmallTickHit: - return "small droplet"; + return "Small droplet"; case HitResult.LargeBonus: - return "banana"; + return "Banana"; } return base.GetDisplayNameForHitResult(result); From 951fc5ef6ecff428a20b7b52c8f26cab9c170fe9 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sat, 29 May 2021 15:39:12 +0900 Subject: [PATCH 19/19] Remove misleading comment and misplaced bug fix This shouldn't be fixed in a test scene; the underlying issue should be fixed in actual game code. --- .../Gameplay/TestSceneBeatmapSkinFallbacks.cs | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneBeatmapSkinFallbacks.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneBeatmapSkinFallbacks.cs index bece80903f..d6bedffaa8 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneBeatmapSkinFallbacks.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneBeatmapSkinFallbacks.cs @@ -5,7 +5,6 @@ using System; using System.Linq; using osu.Framework.Allocation; using osu.Framework.Audio; -using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Framework.Lists; using osu.Framework.Testing; @@ -117,18 +116,6 @@ namespace osu.Game.Tests.Visual.Gameplay : base(source) { } - - public override Drawable GetDrawableComponent(ISkinComponent component) - { - var drawable = base.GetDrawableComponent(component); - if (drawable != null) - return drawable; - - // this isn't really supposed to make a difference from returning null, - // but it appears it does, returning null will skip over falling back to beatmap skin, - // while calling Source.GetDrawableComponent() doesn't. - return Source.GetDrawableComponent(component); - } } } }