From 8dca69e3f9b346517659230f9350f711a8153cd0 Mon Sep 17 00:00:00 2001 From: Salman Alshamrani Date: Sun, 24 Nov 2024 23:10:23 -0500 Subject: [PATCH 01/12] Add osu!mania specifications for health bar display --- .../Legacy/ManiaLegacySkinTransformer.cs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/osu.Game.Rulesets.Mania/Skinning/Legacy/ManiaLegacySkinTransformer.cs b/osu.Game.Rulesets.Mania/Skinning/Legacy/ManiaLegacySkinTransformer.cs index cb42b2b62a..9517dad9c9 100644 --- a/osu.Game.Rulesets.Mania/Skinning/Legacy/ManiaLegacySkinTransformer.cs +++ b/osu.Game.Rulesets.Mania/Skinning/Legacy/ManiaLegacySkinTransformer.cs @@ -9,6 +9,7 @@ using System.Linq; using osu.Framework.Audio.Sample; using osu.Framework.Bindables; using osu.Framework.Graphics; +using osu.Framework.Graphics.Containers; using osu.Framework.Testing; using osu.Game.Audio; using osu.Game.Beatmaps; @@ -16,6 +17,7 @@ using osu.Game.Rulesets.Mania.Beatmaps; using osu.Game.Rulesets.Objects.Legacy; using osu.Game.Rulesets.Scoring; using osu.Game.Skinning; +using osuTK; namespace osu.Game.Rulesets.Mania.Skinning.Legacy { @@ -106,6 +108,20 @@ namespace osu.Game.Rulesets.Mania.Skinning.Legacy { new LegacyManiaComboCounter(), }; + + case GlobalSkinnableContainers.Playfield: + return new Container + { + RelativeSizeAxes = Axes.Both, + Child = new LegacyHealthDisplay + { + Rotation = -90f, + Anchor = Anchor.BottomRight, + Origin = Anchor.TopLeft, + X = 1, + Scale = new Vector2(0.7f), + }, + }; } return null; From f1b56869042ef2cd3ac8174c7d8a7c252247b9a2 Mon Sep 17 00:00:00 2001 From: Salman Alshamrani Date: Mon, 25 Nov 2024 00:01:48 -0500 Subject: [PATCH 02/12] Refactor skin migration to allow mutating multiple layouts at once --- osu.Game/Skinning/Skin.cs | 48 +++++++++++++++++++++++---------------- 1 file changed, 29 insertions(+), 19 deletions(-) diff --git a/osu.Game/Skinning/Skin.cs b/osu.Game/Skinning/Skin.cs index e93a10d50b..1afbf1d5e9 100644 --- a/osu.Game/Skinning/Skin.cs +++ b/osu.Game/Skinning/Skin.cs @@ -148,6 +148,28 @@ namespace osu.Game.Skinning Logger.Error(ex, "Failed to load skin configuration."); } } + + if (layoutInfos.Values.Any()) + { + int version = layoutInfos.Values.Min(l => l.Version); + + for (int i = version + 1; i <= SkinLayoutInfo.LATEST_VERSION; i++) + applyMigration(i); + + foreach (var layout in layoutInfos.Values) + { + layout.Version = SkinLayoutInfo.LATEST_VERSION; + + foreach (var kvp in layout.DrawableInfo.ToArray()) + { + foreach (var di in kvp.Value) + { + if (!isValidDrawable(di)) + layout.DrawableInfo[kvp.Key] = kvp.Value.Where(i => i.Type != di.Type).ToArray(); + } + } + } + } } protected virtual IResourceStore CreateTextureLoaderStore(IStorageResourceProvider resources, IResourceStore storage) @@ -244,20 +266,6 @@ namespace osu.Game.Skinning Logger.Log($"Ferrying {deserializedContent.Count()} components in {target} to global section of new {nameof(SkinLayoutInfo)} format"); } - for (int i = layout.Version + 1; i <= SkinLayoutInfo.LATEST_VERSION; i++) - applyMigration(layout, target, i); - - layout.Version = SkinLayoutInfo.LATEST_VERSION; - - foreach (var kvp in layout.DrawableInfo.ToArray()) - { - foreach (var di in kvp.Value) - { - if (!isValidDrawable(di)) - layout.DrawableInfo[kvp.Key] = kvp.Value.Where(i => i.Type != di.Type).ToArray(); - } - } - return layout; } @@ -275,7 +283,7 @@ namespace osu.Game.Skinning return true; } - private void applyMigration(SkinLayoutInfo layout, GlobalSkinnableContainers target, int version) + private void applyMigration(int version) { switch (version) { @@ -283,8 +291,10 @@ namespace osu.Game.Skinning { // Combo counters were moved out of the global HUD components into per-ruleset. // This is to allow some rulesets to customise further (ie. mania and catch moving the combo to within their play area). - if (target != GlobalSkinnableContainers.MainHUDComponents || - !layout.TryGetDrawableInfo(null, out var globalHUDComponents) || + if (!layoutInfos.TryGetValue(GlobalSkinnableContainers.MainHUDComponents, out var hudLayout)) + break; + + if (!hudLayout.TryGetDrawableInfo(null, out var globalHUDComponents) || resources == null) break; @@ -293,13 +303,13 @@ namespace osu.Game.Skinning c.Type.Name == nameof(DefaultComboCounter) || c.Type.Name == nameof(ArgonComboCounter)).ToArray(); - layout.Update(null, globalHUDComponents.Except(comboCounters).ToArray()); + hudLayout.Update(null, globalHUDComponents.Except(comboCounters).ToArray()); resources.RealmAccess.Run(r => { foreach (var ruleset in r.All()) { - layout.Update(ruleset, layout.TryGetDrawableInfo(ruleset, out var rulesetHUDComponents) + hudLayout.Update(ruleset, hudLayout.TryGetDrawableInfo(ruleset, out var rulesetHUDComponents) ? rulesetHUDComponents.Concat(comboCounters).ToArray() : comboCounters); } From 76f79ce083a6adfee7fd2e23dbb5be2ffb4ac947 Mon Sep 17 00:00:00 2001 From: Salman Alshamrani Date: Mon, 25 Nov 2024 00:02:27 -0500 Subject: [PATCH 03/12] Migrate legacy health bar display to per-ruleset target --- osu.Game/Skinning/LegacySkin.cs | 4 +-- osu.Game/Skinning/Skin.cs | 52 +++++++++++++++++++++++++++++ osu.Game/Skinning/SkinLayoutInfo.cs | 3 +- 3 files changed, 56 insertions(+), 3 deletions(-) diff --git a/osu.Game/Skinning/LegacySkin.cs b/osu.Game/Skinning/LegacySkin.cs index 6faadfba9b..da7c95a46f 100644 --- a/osu.Game/Skinning/LegacySkin.cs +++ b/osu.Game/Skinning/LegacySkin.cs @@ -376,7 +376,8 @@ namespace osu.Game.Skinning } }) { - new LegacyDefaultComboCounter() + new LegacyDefaultComboCounter(), + new LegacyHealthDisplay(), }; } @@ -415,7 +416,6 @@ namespace osu.Game.Skinning new LegacyScoreCounter(), new LegacyAccuracyCounter(), new LegacySongProgress(), - new LegacyHealthDisplay(), new BarHitErrorMeter(), } }; diff --git a/osu.Game/Skinning/Skin.cs b/osu.Game/Skinning/Skin.cs index 1afbf1d5e9..e22b00f2b0 100644 --- a/osu.Game/Skinning/Skin.cs +++ b/osu.Game/Skinning/Skin.cs @@ -23,6 +23,7 @@ using osu.Game.Database; using osu.Game.IO; using osu.Game.Rulesets; using osu.Game.Screens.Play.HUD; +using osuTK; namespace osu.Game.Skinning { @@ -317,6 +318,57 @@ namespace osu.Game.Skinning break; } + + case 2: + { + // Legacy health bar is moved from the global HUD components target into per-ruleset. + // On osu!mania, the health bar is moved from the HUD to Playfield target for stage-based positioning. + if (resources == null) + break; + + var hudLayout = layoutInfos.GetValueOrDefault(GlobalSkinnableContainers.MainHUDComponents); + var playfieldLayout = layoutInfos.GetValueOrDefault(GlobalSkinnableContainers.Playfield); + + if (hudLayout == null || !hudLayout.TryGetDrawableInfo(null, out var globalHUDComponents)) + globalHUDComponents = Array.Empty(); + + var legacyHealthBars = globalHUDComponents.Where(h => h.Type.Name == nameof(LegacyHealthDisplay)).ToArray(); + hudLayout?.Update(null, globalHUDComponents.Except(legacyHealthBars).ToArray()); + + resources.RealmAccess.Run(r => + { + foreach (var ruleset in r.All()) + { + if (ruleset.ShortName == @"mania") + { + // should avoid adding legacy health bar to non-legacy skins (unless explicitly added by user). + if (!legacyHealthBars.Any()) + break; + + var legacyManiaHealthDisplay = new LegacyHealthDisplay + { + Rotation = -90f, + Anchor = Anchor.BottomRight, + Origin = Anchor.TopLeft, + X = 1, + Scale = new Vector2(0.7f), + }.CreateSerialisedInfo(); + + playfieldLayout?.Update(ruleset, playfieldLayout.TryGetDrawableInfo(ruleset, out var maniaPlayfieldComponents) + ? maniaPlayfieldComponents.Append(legacyManiaHealthDisplay).ToArray() + : new[] { legacyManiaHealthDisplay }); + } + else + { + hudLayout?.Update(ruleset, hudLayout.TryGetDrawableInfo(ruleset, out var rulesetHUDComponents) + ? rulesetHUDComponents.Concat(legacyHealthBars).ToArray() + : legacyHealthBars); + } + } + }); + + break; + } } } diff --git a/osu.Game/Skinning/SkinLayoutInfo.cs b/osu.Game/Skinning/SkinLayoutInfo.cs index bf6c693621..4eee1cac5a 100644 --- a/osu.Game/Skinning/SkinLayoutInfo.cs +++ b/osu.Game/Skinning/SkinLayoutInfo.cs @@ -26,9 +26,10 @@ namespace osu.Game.Skinning /// /// 0: Initial version of all skin layouts. /// 1: Moves existing combo counters from global to per-ruleset HUD targets. + /// 2: Moves existing legacy health bars from global to per-ruleset HUD targets, and to playfield target on mania. /// /// - public const int LATEST_VERSION = 1; + public const int LATEST_VERSION = 2; [JsonProperty(DefaultValueHandling = DefaultValueHandling.Populate)] public int Version = LATEST_VERSION; From 7df7727591201bcbf86e21a51c6e8351b11cfe1f Mon Sep 17 00:00:00 2001 From: Salman Alshamrani Date: Mon, 25 Nov 2024 20:35:11 -0500 Subject: [PATCH 04/12] Revert "Migrate legacy health bar display to per-ruleset target" This reverts commit 76f79ce083a6adfee7fd2e23dbb5be2ffb4ac947. --- osu.Game/Skinning/LegacySkin.cs | 4 +-- osu.Game/Skinning/Skin.cs | 52 ----------------------------- osu.Game/Skinning/SkinLayoutInfo.cs | 3 +- 3 files changed, 3 insertions(+), 56 deletions(-) diff --git a/osu.Game/Skinning/LegacySkin.cs b/osu.Game/Skinning/LegacySkin.cs index da7c95a46f..6faadfba9b 100644 --- a/osu.Game/Skinning/LegacySkin.cs +++ b/osu.Game/Skinning/LegacySkin.cs @@ -376,8 +376,7 @@ namespace osu.Game.Skinning } }) { - new LegacyDefaultComboCounter(), - new LegacyHealthDisplay(), + new LegacyDefaultComboCounter() }; } @@ -416,6 +415,7 @@ namespace osu.Game.Skinning new LegacyScoreCounter(), new LegacyAccuracyCounter(), new LegacySongProgress(), + new LegacyHealthDisplay(), new BarHitErrorMeter(), } }; diff --git a/osu.Game/Skinning/Skin.cs b/osu.Game/Skinning/Skin.cs index e22b00f2b0..1afbf1d5e9 100644 --- a/osu.Game/Skinning/Skin.cs +++ b/osu.Game/Skinning/Skin.cs @@ -23,7 +23,6 @@ using osu.Game.Database; using osu.Game.IO; using osu.Game.Rulesets; using osu.Game.Screens.Play.HUD; -using osuTK; namespace osu.Game.Skinning { @@ -318,57 +317,6 @@ namespace osu.Game.Skinning break; } - - case 2: - { - // Legacy health bar is moved from the global HUD components target into per-ruleset. - // On osu!mania, the health bar is moved from the HUD to Playfield target for stage-based positioning. - if (resources == null) - break; - - var hudLayout = layoutInfos.GetValueOrDefault(GlobalSkinnableContainers.MainHUDComponents); - var playfieldLayout = layoutInfos.GetValueOrDefault(GlobalSkinnableContainers.Playfield); - - if (hudLayout == null || !hudLayout.TryGetDrawableInfo(null, out var globalHUDComponents)) - globalHUDComponents = Array.Empty(); - - var legacyHealthBars = globalHUDComponents.Where(h => h.Type.Name == nameof(LegacyHealthDisplay)).ToArray(); - hudLayout?.Update(null, globalHUDComponents.Except(legacyHealthBars).ToArray()); - - resources.RealmAccess.Run(r => - { - foreach (var ruleset in r.All()) - { - if (ruleset.ShortName == @"mania") - { - // should avoid adding legacy health bar to non-legacy skins (unless explicitly added by user). - if (!legacyHealthBars.Any()) - break; - - var legacyManiaHealthDisplay = new LegacyHealthDisplay - { - Rotation = -90f, - Anchor = Anchor.BottomRight, - Origin = Anchor.TopLeft, - X = 1, - Scale = new Vector2(0.7f), - }.CreateSerialisedInfo(); - - playfieldLayout?.Update(ruleset, playfieldLayout.TryGetDrawableInfo(ruleset, out var maniaPlayfieldComponents) - ? maniaPlayfieldComponents.Append(legacyManiaHealthDisplay).ToArray() - : new[] { legacyManiaHealthDisplay }); - } - else - { - hudLayout?.Update(ruleset, hudLayout.TryGetDrawableInfo(ruleset, out var rulesetHUDComponents) - ? rulesetHUDComponents.Concat(legacyHealthBars).ToArray() - : legacyHealthBars); - } - } - }); - - break; - } } } diff --git a/osu.Game/Skinning/SkinLayoutInfo.cs b/osu.Game/Skinning/SkinLayoutInfo.cs index 4eee1cac5a..bf6c693621 100644 --- a/osu.Game/Skinning/SkinLayoutInfo.cs +++ b/osu.Game/Skinning/SkinLayoutInfo.cs @@ -26,10 +26,9 @@ namespace osu.Game.Skinning /// /// 0: Initial version of all skin layouts. /// 1: Moves existing combo counters from global to per-ruleset HUD targets. - /// 2: Moves existing legacy health bars from global to per-ruleset HUD targets, and to playfield target on mania. /// /// - public const int LATEST_VERSION = 2; + public const int LATEST_VERSION = 1; [JsonProperty(DefaultValueHandling = DefaultValueHandling.Populate)] public int Version = LATEST_VERSION; From 83ecfbd15503dd7de3c18a438acb28b617044645 Mon Sep 17 00:00:00 2001 From: Salman Alshamrani Date: Mon, 25 Nov 2024 20:35:11 -0500 Subject: [PATCH 05/12] Revert "Refactor skin migration to allow mutating multiple layouts at once" This reverts commit f1b56869042ef2cd3ac8174c7d8a7c252247b9a2. --- osu.Game/Skinning/Skin.cs | 48 ++++++++++++++++----------------------- 1 file changed, 19 insertions(+), 29 deletions(-) diff --git a/osu.Game/Skinning/Skin.cs b/osu.Game/Skinning/Skin.cs index 1afbf1d5e9..e93a10d50b 100644 --- a/osu.Game/Skinning/Skin.cs +++ b/osu.Game/Skinning/Skin.cs @@ -148,28 +148,6 @@ namespace osu.Game.Skinning Logger.Error(ex, "Failed to load skin configuration."); } } - - if (layoutInfos.Values.Any()) - { - int version = layoutInfos.Values.Min(l => l.Version); - - for (int i = version + 1; i <= SkinLayoutInfo.LATEST_VERSION; i++) - applyMigration(i); - - foreach (var layout in layoutInfos.Values) - { - layout.Version = SkinLayoutInfo.LATEST_VERSION; - - foreach (var kvp in layout.DrawableInfo.ToArray()) - { - foreach (var di in kvp.Value) - { - if (!isValidDrawable(di)) - layout.DrawableInfo[kvp.Key] = kvp.Value.Where(i => i.Type != di.Type).ToArray(); - } - } - } - } } protected virtual IResourceStore CreateTextureLoaderStore(IStorageResourceProvider resources, IResourceStore storage) @@ -266,6 +244,20 @@ namespace osu.Game.Skinning Logger.Log($"Ferrying {deserializedContent.Count()} components in {target} to global section of new {nameof(SkinLayoutInfo)} format"); } + for (int i = layout.Version + 1; i <= SkinLayoutInfo.LATEST_VERSION; i++) + applyMigration(layout, target, i); + + layout.Version = SkinLayoutInfo.LATEST_VERSION; + + foreach (var kvp in layout.DrawableInfo.ToArray()) + { + foreach (var di in kvp.Value) + { + if (!isValidDrawable(di)) + layout.DrawableInfo[kvp.Key] = kvp.Value.Where(i => i.Type != di.Type).ToArray(); + } + } + return layout; } @@ -283,7 +275,7 @@ namespace osu.Game.Skinning return true; } - private void applyMigration(int version) + private void applyMigration(SkinLayoutInfo layout, GlobalSkinnableContainers target, int version) { switch (version) { @@ -291,10 +283,8 @@ namespace osu.Game.Skinning { // Combo counters were moved out of the global HUD components into per-ruleset. // This is to allow some rulesets to customise further (ie. mania and catch moving the combo to within their play area). - if (!layoutInfos.TryGetValue(GlobalSkinnableContainers.MainHUDComponents, out var hudLayout)) - break; - - if (!hudLayout.TryGetDrawableInfo(null, out var globalHUDComponents) || + if (target != GlobalSkinnableContainers.MainHUDComponents || + !layout.TryGetDrawableInfo(null, out var globalHUDComponents) || resources == null) break; @@ -303,13 +293,13 @@ namespace osu.Game.Skinning c.Type.Name == nameof(DefaultComboCounter) || c.Type.Name == nameof(ArgonComboCounter)).ToArray(); - hudLayout.Update(null, globalHUDComponents.Except(comboCounters).ToArray()); + layout.Update(null, globalHUDComponents.Except(comboCounters).ToArray()); resources.RealmAccess.Run(r => { foreach (var ruleset in r.All()) { - hudLayout.Update(ruleset, hudLayout.TryGetDrawableInfo(ruleset, out var rulesetHUDComponents) + layout.Update(ruleset, layout.TryGetDrawableInfo(ruleset, out var rulesetHUDComponents) ? rulesetHUDComponents.Concat(comboCounters).ToArray() : comboCounters); } From c2215b10cfbcd5e5458e5a1fab72da1b4f40ac7e Mon Sep 17 00:00:00 2001 From: Salman Alshamrani Date: Mon, 25 Nov 2024 22:02:11 -0500 Subject: [PATCH 06/12] Add extensive test scene for skin migrations This is different from `SkinDeserialisationTest` in that layouts can be written programmatically with as much ease, allowing to test migration logic with different scenarios without running the game and exporting skins and attaching them to tests. --- .../Visual/Skinning/TestSceneSkinMigration.cs | 225 ++++++++++++++++++ osu.Game/Skinning/SkinInfo.cs | 2 +- 2 files changed, 226 insertions(+), 1 deletion(-) create mode 100644 osu.Game.Tests/Visual/Skinning/TestSceneSkinMigration.cs diff --git a/osu.Game.Tests/Visual/Skinning/TestSceneSkinMigration.cs b/osu.Game.Tests/Visual/Skinning/TestSceneSkinMigration.cs new file mode 100644 index 0000000000..934abca248 --- /dev/null +++ b/osu.Game.Tests/Visual/Skinning/TestSceneSkinMigration.cs @@ -0,0 +1,225 @@ +// 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.Collections.Generic; +using System.Linq; +using System.Text; +using Newtonsoft.Json; +using NUnit.Framework; +using osu.Framework.Allocation; +using osu.Framework.Audio; +using osu.Framework.Extensions; +using osu.Framework.Graphics; +using osu.Framework.Graphics.Rendering; +using osu.Framework.Graphics.Rendering.Dummy; +using osu.Framework.Graphics.Textures; +using osu.Framework.IO.Stores; +using osu.Game.Database; +using osu.Game.Extensions; +using osu.Game.IO; +using osu.Game.Models; +using osu.Game.Rulesets; +using osu.Game.Skinning; +using osu.Game.Skinning.Components; + +namespace osu.Game.Tests.Visual.Skinning +{ + public partial class TestSceneSkinMigration : OsuTestScene + { + [Resolved] + private RulesetStore rulesets { get; set; } = null!; + + [Test] + public void TestEmptyConfiguration() + { + LegacySkin skin = null!; + + AddStep("load skin with empty configuration", () => skin = loadSkin()); + AddAssert("skin has no configuration", () => !skin.LayoutInfos.Any()); + } + + [Test] + public void TestSomeConfiguration() + { + LegacySkin skin = null!; + + AddStep("load skin with some configuration", () => + { + skin = loadSkin(new Dictionary + { + { GlobalSkinnableContainers.MainHUDComponents, createLayout(SkinLayoutInfo.LATEST_VERSION, [nameof(LegacyHealthDisplay)]) }, + }); + }); + + AddAssert("skin has correct configuration", () => + { + return skin.LayoutInfos.Single().Value.DrawableInfo["global"].Select(d => d.Type.Name).SequenceEqual([nameof(BigBlackBox), nameof(LegacyHealthDisplay)]) && + skin.LayoutInfos.Single().Value.DrawableInfo.Where(d => d.Key != @"global") + .All(d => d.Value.Single().Type.Name == nameof(BigBlackBox)); + }); + } + + #region Version 1 + + [Test] + public void TestMigration_1() + { + LegacySkin skin = null!; + + AddStep("load skin", () => + { + skin = loadSkin(new Dictionary + { + { + GlobalSkinnableContainers.MainHUDComponents, + createLayout(0, [nameof(LegacyDefaultComboCounter)]) + }, + }); + }); + + AddAssert("combo counter removed from global", () => + { + var dict = skin.LayoutInfos[GlobalSkinnableContainers.MainHUDComponents].DrawableInfo; + return dict.Single(kvp => kvp.Key == @"global").Value.Single().Type.Name == nameof(BigBlackBox); + }); + AddAssert("combo counter moved to each ruleset", () => + { + var dict = skin.LayoutInfos[GlobalSkinnableContainers.MainHUDComponents].DrawableInfo; + return dict.Where(kvp => kvp.Key != @"global").All(kvp => kvp.Value.Select(d => d.Type.Name).SequenceEqual([nameof(BigBlackBox), nameof(LegacyDefaultComboCounter)])); + }); + } + + [Test] + public void TestMigration_1_NoComboCounter() + { + LegacySkin skin = null!; + + AddStep("load skin", () => + { + skin = loadSkin(new Dictionary + { + { + GlobalSkinnableContainers.MainHUDComponents, + createLayout(0) + }, + }); + }); + + AddAssert("nothing removed from global", () => + { + var dict = skin.LayoutInfos[GlobalSkinnableContainers.MainHUDComponents].DrawableInfo; + return dict.Single(kvp => kvp.Key == @"global").Value.Single().Type.Name == nameof(BigBlackBox); + }); + AddAssert("no combo counter added", () => + { + var dict = skin.LayoutInfos[GlobalSkinnableContainers.MainHUDComponents].DrawableInfo; + return dict.Where(kvp => kvp.Key != @"global").All(kvp => kvp.Value.Select(d => d.Type.Name).SequenceEqual([nameof(BigBlackBox)])); + }); + } + + #endregion + + private SkinLayoutInfo createLayout(int version, string[]? globalComponents = null, string? ruleset = null, string[]? rulesetComponents = null) + { + var info = new SkinLayoutInfo + { + Version = version, + DrawableInfo = + { + { "global", globalComponents?.Select(c => resolveComponent(c).CreateSerialisedInfo()).ToArray() ?? Array.Empty() }, + } + }; + + if (ruleset != null && rulesetComponents != null) + info.DrawableInfo.Add(ruleset, rulesetComponents.Select(c => resolveComponent(c).CreateSerialisedInfo()).ToArray()); + + // add random drawable to ensure nothing is incorrectly discarded + foreach (string key in rulesets.AvailableRulesets.Select(r => r.ShortName).Prepend(@"global")) + { + if (!info.DrawableInfo.TryGetValue(key, out var drawables)) + info.DrawableInfo.Add(key, drawables = Array.Empty()); + + info.DrawableInfo[key] = drawables.Prepend(new BigBlackBox().CreateSerialisedInfo()).ToArray(); + } + + return info; + } + + private Drawable resolveComponent(string name, string? ruleset = null) + { + var drawables = SerialisedDrawableInfo.GetAllAvailableDrawables(); + + if (ruleset != null) + drawables = drawables.Concat(SerialisedDrawableInfo.GetAllAvailableDrawables(rulesets.GetRuleset(ruleset))).ToArray(); + + return (Drawable)Activator.CreateInstance(drawables.Single(d => d.Name == name))!; + } + + private T loadSkin(IDictionary? layout = null) + where T : Skin + { + var info = new TestSkinInfo(typeof(T).GetInvariantInstantiationInfo(), layout); + return (T)info.CreateInstance(new TestStorageResourceProvider(layout, info.Files, Realm)); + } + + private class TestSkinInfo : SkinInfo + { + public override IList Files { get; } = new List(); + + public TestSkinInfo(string instantiationInfo, IDictionary? layout) + : base("test skin", "me", instantiationInfo) + { + if (layout != null) + { + foreach (var kvp in layout) + Files.Add(new RealmNamedFileUsage(new RealmFile { Hash = Guid.NewGuid().ToString().ComputeMD5Hash() }, kvp.Key + ".json")); + } + } + } + + private class TestStorageResourceProvider : IStorageResourceProvider + { + public IRenderer Renderer { get; } = new DummyRenderer(); + public IResourceStore Resources { get; } = new ResourceStore(); + public IResourceStore? CreateTextureLoaderStore(IResourceStore underlyingStore) => null; + + public AudioManager? AudioManager => null; + + public IResourceStore Files { get; } + public RealmAccess RealmAccess { get; } + + public TestStorageResourceProvider(IDictionary? layout, IList files, RealmAccess realm) + { + Files = new TestResourceStore(layout, files); + RealmAccess = realm; + } + + private class TestResourceStore : ResourceStore + { + private readonly IDictionary? layout; + private readonly IList files; + + public TestResourceStore(IDictionary? layout, IList files) + { + this.layout = layout; + this.files = files; + } + + public override byte[] Get(string name) + { + string? filename = files.SingleOrDefault(f => f.File.GetStoragePath() == name)?.Filename; + if (filename == null || layout == null) + return base.Get(name); + + if (!Enum.TryParse(filename.Replace(@".json", string.Empty), out var type) || + !layout.TryGetValue(type, out var info)) + return base.Get(name); + + string json = JsonConvert.SerializeObject(info, new JsonSerializerSettings { Formatting = Formatting.Indented }); + return Encoding.UTF8.GetBytes(json); + } + } + } + } +} diff --git a/osu.Game/Skinning/SkinInfo.cs b/osu.Game/Skinning/SkinInfo.cs index 9763d3b57e..c715f263c4 100644 --- a/osu.Game/Skinning/SkinInfo.cs +++ b/osu.Game/Skinning/SkinInfo.cs @@ -58,7 +58,7 @@ namespace osu.Game.Skinning return (Skin)Activator.CreateInstance(type, this, resources)!; } - public IList Files { get; } = null!; + public virtual IList Files { get; } = null!; public bool DeletePending { get; set; } From fcbfbd02fd7ebf2e81101ffbc129aba51ca53426 Mon Sep 17 00:00:00 2001 From: Salman Alshamrani Date: Mon, 25 Nov 2024 23:24:19 -0500 Subject: [PATCH 07/12] Add migration logic for legacy health displays --- .../Visual/Skinning/TestSceneSkinMigration.cs | 197 +++++++++++++++++- osu.Game/Skinning/LegacySkin.cs | 4 +- osu.Game/Skinning/Skin.cs | 64 +++++- osu.Game/Skinning/SkinInfo.cs | 8 +- osu.Game/Skinning/SkinLayoutInfo.cs | 3 +- 5 files changed, 259 insertions(+), 17 deletions(-) diff --git a/osu.Game.Tests/Visual/Skinning/TestSceneSkinMigration.cs b/osu.Game.Tests/Visual/Skinning/TestSceneSkinMigration.cs index 934abca248..e44b0dbedd 100644 --- a/osu.Game.Tests/Visual/Skinning/TestSceneSkinMigration.cs +++ b/osu.Game.Tests/Visual/Skinning/TestSceneSkinMigration.cs @@ -44,7 +44,7 @@ namespace osu.Game.Tests.Visual.Skinning { LegacySkin skin = null!; - AddStep("load skin with some configuration", () => + AddStep("load skin", () => { skin = loadSkin(new Dictionary { @@ -120,16 +120,195 @@ namespace osu.Game.Tests.Visual.Skinning #endregion + #region Version 2 + + [Test] + public void TestMigration_2() + { + LegacySkin skin = null!; + + AddStep("load skin", () => + { + skin = loadSkin(new Dictionary + { + { + GlobalSkinnableContainers.MainHUDComponents, + createLayout(1, [nameof(LegacyHealthDisplay)]) + }, + { + GlobalSkinnableContainers.Playfield, + createLayout(1) + } + }); + }); + + // HUD + AddAssert("health display removed from global HUD", () => + { + var dict = skin.LayoutInfos[GlobalSkinnableContainers.MainHUDComponents].DrawableInfo; + return dict.Single(kvp => kvp.Key == @"global").Value.Single().Type.Name == nameof(BigBlackBox); + }); + AddAssert("health display moved to each ruleset except mania", () => + { + var dict = skin.LayoutInfos[GlobalSkinnableContainers.MainHUDComponents].DrawableInfo.ToArray(); + dict = dict.Where(kvp => kvp.Key != @"global" && kvp.Key != @"mania").ToArray(); + + return dict.All(kvp => kvp.Value.Select(d => d.Type.Name).SequenceEqual([nameof(BigBlackBox), nameof(LegacyHealthDisplay)])) && + dict.All(kvp => kvp.Value.Single(d => d.Type.Name == nameof(LegacyHealthDisplay)).Rotation == 0f); + }); + AddAssert("no health display in mania HUD", () => + { + var dict = skin.LayoutInfos[GlobalSkinnableContainers.MainHUDComponents].DrawableInfo; + return dict.Single(kvp => kvp.Key == @"mania").Value.Single().Type.Name == nameof(BigBlackBox); + }); + + // Playfield + AddAssert("health display in mania moved to playfield", () => + { + var dict = skin.LayoutInfos[GlobalSkinnableContainers.Playfield].DrawableInfo; + return dict.Single(kvp => kvp.Key == @"mania").Value.Select(d => d.Type.Name).SequenceEqual([nameof(BigBlackBox), nameof(LegacyHealthDisplay)]) && + dict.Single(kvp => kvp.Key == @"mania").Value.Single(d => d.Type.Name == nameof(LegacyHealthDisplay)).Rotation == -90f; + }); + AddAssert("rest is unaffected", () => + { + var dict = skin.LayoutInfos[GlobalSkinnableContainers.Playfield].DrawableInfo; + return dict.Where(kvp => kvp.Key != @"mania").All(kvp => kvp.Value.Single().Type.Name == nameof(BigBlackBox)); + }); + } + + [Test] + public void TestMigration_2_NonLegacySkin() + { + ArgonSkin skin = null!; + + AddStep("load argon skin", () => + { + skin = loadSkin(new Dictionary + { + { + GlobalSkinnableContainers.MainHUDComponents, + createLayout(1, [nameof(LegacyHealthDisplay)]) + }, + { + GlobalSkinnableContainers.Playfield, + createLayout(1) + } + }); + }); + + // One may argue that if a LegacyHealthDisplay exists in a non-legacy skin, + // then it should be swapped with the mania variant similar to legacy skins. + // This is not simple to achieve as we have to be aware of the presence of + // the health display in the HUD layout while migrating the Playfield layout, + // which is impossible with the current structure of skin layout migration. + // Instead, don't touch any non-legacy skin and call it a day. + + // HUD + AddAssert("health display still in global HUD", () => + { + var dict = skin.LayoutInfos[GlobalSkinnableContainers.MainHUDComponents].DrawableInfo; + + return dict.Single(kvp => kvp.Key == @"global").Value.Select(d => d.Type.Name).SequenceEqual([nameof(BigBlackBox), nameof(LegacyHealthDisplay)]) && + dict.Single(kvp => kvp.Key == @"global").Value.Single(d => d.Type.Name == nameof(LegacyHealthDisplay)).Rotation == 0f; + }); + AddAssert("ruleset HUDs unaffected", () => + { + var dict = skin.LayoutInfos[GlobalSkinnableContainers.MainHUDComponents].DrawableInfo.ToArray(); + return dict.Where(kvp => kvp.Key != @"global").All(kvp => kvp.Value.Single().Type.Name == nameof(BigBlackBox)); + }); + + // Playfield + AddAssert("playfield unaffected", () => + { + var dict = skin.LayoutInfos[GlobalSkinnableContainers.Playfield].DrawableInfo.ToArray(); + return dict.All(kvp => kvp.Value.Single().Type.Name == nameof(BigBlackBox)); + }); + } + + [Test] + public void TestMigration_2_NoHUD() + { + LegacySkin skin = null!; + + AddStep("load skin", () => + { + skin = loadSkin(new Dictionary + { + { + GlobalSkinnableContainers.Playfield, + createLayout(1) + }, + }); + }); + + // In this case, we must add a health display to the Playfield target, + // otherwise on mania the user will not see a health display anymore. + + // HUD + AddAssert("HUD not configured", () => !skin.LayoutInfos.ContainsKey(GlobalSkinnableContainers.MainHUDComponents)); + + // Playfield + AddAssert("health display in mania moved to playfield", () => + { + var dict = skin.LayoutInfos[GlobalSkinnableContainers.Playfield].DrawableInfo; + return dict.Single(kvp => kvp.Key == @"mania").Value.Select(d => d.Type.Name).SequenceEqual([nameof(BigBlackBox), nameof(LegacyHealthDisplay)]) && + dict.Single(kvp => kvp.Key == @"mania").Value.Single(d => d.Type.Name == nameof(LegacyHealthDisplay)).Rotation == -90f; + }); + AddAssert("rest is unaffected", () => + { + var dict = skin.LayoutInfos[GlobalSkinnableContainers.Playfield].DrawableInfo; + return dict.Where(kvp => kvp.Key != @"mania").All(d => d.Value.Single().Type.Name == nameof(BigBlackBox)); + }); + } + + [Test] + public void TestMigration_2_NoPlayfield() + { + LegacySkin skin = null!; + + AddStep("load skin", () => + { + skin = loadSkin(new Dictionary + { + { + GlobalSkinnableContainers.MainHUDComponents, + createLayout(1, [nameof(LegacyHealthDisplay)]) + } + }); + }); + + // HUD + AddAssert("health display removed from global HUD", () => + { + var dict = skin.LayoutInfos[GlobalSkinnableContainers.MainHUDComponents].DrawableInfo; + return dict.Single(kvp => kvp.Key == @"global").Value.Single().Type.Name == nameof(BigBlackBox); + }); + AddAssert("health display moved to each ruleset except mania", () => + { + var dict = skin.LayoutInfos[GlobalSkinnableContainers.MainHUDComponents].DrawableInfo.ToArray(); + dict = dict.Where(kvp => kvp.Key != @"global" && kvp.Key != @"mania").ToArray(); + + return dict.All(kvp => kvp.Value.Select(d => d.Type.Name).SequenceEqual([nameof(BigBlackBox), nameof(LegacyHealthDisplay)])) && + dict.All(kvp => kvp.Value.Single(d => d.Type.Name == nameof(LegacyHealthDisplay)).Rotation == 0f); + }); + AddAssert("no health display in mania HUD", () => + { + var dict = skin.LayoutInfos[GlobalSkinnableContainers.MainHUDComponents].DrawableInfo; + return dict.Single(kvp => kvp.Key == @"mania").Value.Single().Type.Name == nameof(BigBlackBox); + }); + + // Playfield + AddAssert("playfield not configured", () => !skin.LayoutInfos.ContainsKey(GlobalSkinnableContainers.Playfield)); + } + + #endregion + private SkinLayoutInfo createLayout(int version, string[]? globalComponents = null, string? ruleset = null, string[]? rulesetComponents = null) { - var info = new SkinLayoutInfo - { - Version = version, - DrawableInfo = - { - { "global", globalComponents?.Select(c => resolveComponent(c).CreateSerialisedInfo()).ToArray() ?? Array.Empty() }, - } - }; + var info = new SkinLayoutInfo { Version = version }; + + if (globalComponents != null) + info.DrawableInfo.Add(@"global", globalComponents.Select(c => resolveComponent(c).CreateSerialisedInfo()).ToArray()); if (ruleset != null && rulesetComponents != null) info.DrawableInfo.Add(ruleset, rulesetComponents.Select(c => resolveComponent(c).CreateSerialisedInfo()).ToArray()); diff --git a/osu.Game/Skinning/LegacySkin.cs b/osu.Game/Skinning/LegacySkin.cs index 6faadfba9b..da7c95a46f 100644 --- a/osu.Game/Skinning/LegacySkin.cs +++ b/osu.Game/Skinning/LegacySkin.cs @@ -376,7 +376,8 @@ namespace osu.Game.Skinning } }) { - new LegacyDefaultComboCounter() + new LegacyDefaultComboCounter(), + new LegacyHealthDisplay(), }; } @@ -415,7 +416,6 @@ namespace osu.Game.Skinning new LegacyScoreCounter(), new LegacyAccuracyCounter(), new LegacySongProgress(), - new LegacyHealthDisplay(), new BarHitErrorMeter(), } }; diff --git a/osu.Game/Skinning/Skin.cs b/osu.Game/Skinning/Skin.cs index e93a10d50b..630c314934 100644 --- a/osu.Game/Skinning/Skin.cs +++ b/osu.Game/Skinning/Skin.cs @@ -23,6 +23,7 @@ using osu.Game.Database; using osu.Game.IO; using osu.Game.Rulesets; using osu.Game.Screens.Play.HUD; +using osuTK; namespace osu.Game.Skinning { @@ -277,6 +278,10 @@ namespace osu.Game.Skinning private void applyMigration(SkinLayoutInfo layout, GlobalSkinnableContainers target, int version) { + Debug.Assert(resources != null); + + bool isLegacySkin = SkinInfo.PerformRead(s => s.GetInstanceType().IsAssignableTo(typeof(LegacySkin))); + switch (version) { case 1: @@ -284,8 +289,7 @@ namespace osu.Game.Skinning // Combo counters were moved out of the global HUD components into per-ruleset. // This is to allow some rulesets to customise further (ie. mania and catch moving the combo to within their play area). if (target != GlobalSkinnableContainers.MainHUDComponents || - !layout.TryGetDrawableInfo(null, out var globalHUDComponents) || - resources == null) + !layout.TryGetDrawableInfo(null, out var globalHUDComponents)) break; var comboCounters = globalHUDComponents.Where(c => @@ -307,6 +311,62 @@ namespace osu.Game.Skinning break; } + + case 2: + { + // Health displays are moved out of the global HUD components and into per-ruleset, + // except for osu!mania, wherein the health display is moved to the Playfield target. + switch (target) + { + case GlobalSkinnableContainers.MainHUDComponents: + if (!isLegacySkin || !layout.TryGetDrawableInfo(null, out var globalHUDComponents)) + break; + + var healthDisplays = globalHUDComponents.Where(c => c.Type.Name == nameof(LegacyHealthDisplay)).ToArray(); + layout.Update(null, globalHUDComponents.Except(healthDisplays).ToArray()); + + resources.RealmAccess.Run(r => + { + foreach (var ruleset in r.All()) + { + // for mania, the health display is moved from MainHUDComponents to Playfield. + if (ruleset.ShortName == @"mania") + continue; + + layout.Update(ruleset, layout.TryGetDrawableInfo(ruleset, out var rulesetHUDComponents) + ? rulesetHUDComponents.Concat(healthDisplays).ToArray() + : healthDisplays); + } + }); + + break; + + case GlobalSkinnableContainers.Playfield: + if (!isLegacySkin) + break; + + resources.RealmAccess.Run(r => + { + var maniaRuleset = r.Find(@"mania"); + + if (!layout.TryGetDrawableInfo(maniaRuleset, out var maniaPlayfieldComponents)) + maniaPlayfieldComponents = Array.Empty(); + + layout.Update(maniaRuleset, maniaPlayfieldComponents.Append(new LegacyHealthDisplay + { + Rotation = -90f, + Anchor = Anchor.BottomRight, + Origin = Anchor.TopLeft, + X = 1, + Scale = new Vector2(0.7f), + }.CreateSerialisedInfo()).ToArray()); + }); + + break; + } + + break; + } } } diff --git a/osu.Game/Skinning/SkinInfo.cs b/osu.Game/Skinning/SkinInfo.cs index c715f263c4..ac172e4060 100644 --- a/osu.Game/Skinning/SkinInfo.cs +++ b/osu.Game/Skinning/SkinInfo.cs @@ -39,7 +39,7 @@ namespace osu.Game.Skinning public bool Protected { get; set; } - public virtual Skin CreateInstance(IStorageResourceProvider resources) + public Type GetInstanceType() { var type = string.IsNullOrEmpty(InstantiationInfo) // handle the case of skins imported before InstantiationInfo was added. @@ -52,12 +52,14 @@ namespace osu.Game.Skinning // for user modified skins. This aims to amicably handle that. // If we ever add more default skins in the future this will need some kind of proper migration rather than // a single fallback. - return new TrianglesSkin(this, resources); + return typeof(TrianglesSkin); } - return (Skin)Activator.CreateInstance(type, this, resources)!; + return type; } + public virtual Skin CreateInstance(IStorageResourceProvider resources) => (Skin)Activator.CreateInstance(GetInstanceType(), this, resources)!; + public virtual IList Files { get; } = null!; public bool DeletePending { get; set; } diff --git a/osu.Game/Skinning/SkinLayoutInfo.cs b/osu.Game/Skinning/SkinLayoutInfo.cs index bf6c693621..4eee1cac5a 100644 --- a/osu.Game/Skinning/SkinLayoutInfo.cs +++ b/osu.Game/Skinning/SkinLayoutInfo.cs @@ -26,9 +26,10 @@ namespace osu.Game.Skinning /// /// 0: Initial version of all skin layouts. /// 1: Moves existing combo counters from global to per-ruleset HUD targets. + /// 2: Moves existing legacy health bars from global to per-ruleset HUD targets, and to playfield target on mania. /// /// - public const int LATEST_VERSION = 1; + public const int LATEST_VERSION = 2; [JsonProperty(DefaultValueHandling = DefaultValueHandling.Populate)] public int Version = LATEST_VERSION; From 5ab9074f5dbd637eac803cf87f1b64aeb2083c48 Mon Sep 17 00:00:00 2001 From: Salman Alshamrani Date: Mon, 25 Nov 2024 23:43:46 -0500 Subject: [PATCH 08/12] Move test and mark headless --- .../{Visual/Skinning => Skins}/TestSceneSkinMigration.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) rename osu.Game.Tests/{Visual/Skinning => Skins}/TestSceneSkinMigration.cs (99%) diff --git a/osu.Game.Tests/Visual/Skinning/TestSceneSkinMigration.cs b/osu.Game.Tests/Skins/TestSceneSkinMigration.cs similarity index 99% rename from osu.Game.Tests/Visual/Skinning/TestSceneSkinMigration.cs rename to osu.Game.Tests/Skins/TestSceneSkinMigration.cs index e44b0dbedd..b376938e22 100644 --- a/osu.Game.Tests/Visual/Skinning/TestSceneSkinMigration.cs +++ b/osu.Game.Tests/Skins/TestSceneSkinMigration.cs @@ -15,6 +15,7 @@ using osu.Framework.Graphics.Rendering; using osu.Framework.Graphics.Rendering.Dummy; using osu.Framework.Graphics.Textures; using osu.Framework.IO.Stores; +using osu.Framework.Testing; using osu.Game.Database; using osu.Game.Extensions; using osu.Game.IO; @@ -22,9 +23,11 @@ using osu.Game.Models; using osu.Game.Rulesets; using osu.Game.Skinning; using osu.Game.Skinning.Components; +using osu.Game.Tests.Visual; -namespace osu.Game.Tests.Visual.Skinning +namespace osu.Game.Tests.Skins { + [HeadlessTest] public partial class TestSceneSkinMigration : OsuTestScene { [Resolved] From 0e920a61da1281245729c779068a0ed82e31a340 Mon Sep 17 00:00:00 2001 From: Salman Alshamrani Date: Mon, 25 Nov 2024 23:49:55 -0500 Subject: [PATCH 09/12] Use type equality --- osu.Game/Skinning/Skin.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/osu.Game/Skinning/Skin.cs b/osu.Game/Skinning/Skin.cs index 630c314934..19b06da432 100644 --- a/osu.Game/Skinning/Skin.cs +++ b/osu.Game/Skinning/Skin.cs @@ -293,9 +293,9 @@ namespace osu.Game.Skinning break; var comboCounters = globalHUDComponents.Where(c => - c.Type.Name == nameof(LegacyDefaultComboCounter) || - c.Type.Name == nameof(DefaultComboCounter) || - c.Type.Name == nameof(ArgonComboCounter)).ToArray(); + c.Type == typeof(LegacyDefaultComboCounter) || + c.Type == typeof(DefaultComboCounter) || + c.Type == typeof(ArgonComboCounter)).ToArray(); layout.Update(null, globalHUDComponents.Except(comboCounters).ToArray()); @@ -322,7 +322,7 @@ namespace osu.Game.Skinning if (!isLegacySkin || !layout.TryGetDrawableInfo(null, out var globalHUDComponents)) break; - var healthDisplays = globalHUDComponents.Where(c => c.Type.Name == nameof(LegacyHealthDisplay)).ToArray(); + var healthDisplays = globalHUDComponents.Where(c => c.Type == typeof(LegacyHealthDisplay)).ToArray(); layout.Update(null, globalHUDComponents.Except(healthDisplays).ToArray()); resources.RealmAccess.Run(r => From 01f3a2dd142d9906ed2603ae520ff5980c84af54 Mon Sep 17 00:00:00 2001 From: Salman Alshamrani Date: Wed, 27 Nov 2024 06:12:49 -0500 Subject: [PATCH 10/12] Move non-legacy conditional reasoning to migration code --- osu.Game.Tests/Skins/TestSceneSkinMigration.cs | 7 ------- osu.Game/Skinning/Skin.cs | 8 ++++++++ 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/osu.Game.Tests/Skins/TestSceneSkinMigration.cs b/osu.Game.Tests/Skins/TestSceneSkinMigration.cs index b376938e22..baee5ec2f4 100644 --- a/osu.Game.Tests/Skins/TestSceneSkinMigration.cs +++ b/osu.Game.Tests/Skins/TestSceneSkinMigration.cs @@ -199,13 +199,6 @@ namespace osu.Game.Tests.Skins }); }); - // One may argue that if a LegacyHealthDisplay exists in a non-legacy skin, - // then it should be swapped with the mania variant similar to legacy skins. - // This is not simple to achieve as we have to be aware of the presence of - // the health display in the HUD layout while migrating the Playfield layout, - // which is impossible with the current structure of skin layout migration. - // Instead, don't touch any non-legacy skin and call it a day. - // HUD AddAssert("health display still in global HUD", () => { diff --git a/osu.Game/Skinning/Skin.cs b/osu.Game/Skinning/Skin.cs index 19b06da432..5f046959af 100644 --- a/osu.Game/Skinning/Skin.cs +++ b/osu.Game/Skinning/Skin.cs @@ -343,7 +343,15 @@ namespace osu.Game.Skinning case GlobalSkinnableContainers.Playfield: if (!isLegacySkin) + { + // One may argue that if a LegacyHealthDisplay exists in a non-legacy skin, + // then it should be swapped with the mania variant similar to legacy skins. + // This is not simple to achieve as we have to be aware of the presence of + // the health display in the HUD layout while migrating the Playfield layout, + // which is impossible with the current structure of skin layout migration. + // Instead, don't touch any non-legacy skin and call it a day. break; + } resources.RealmAccess.Run(r => { From 15a4726d68e53304bb847dc05636601394a3e1d1 Mon Sep 17 00:00:00 2001 From: Salman Alshamrani Date: Thu, 28 Nov 2024 23:26:57 -0500 Subject: [PATCH 11/12] Refactor skin deserialisation tests to better support skin migration --- .../Skins/SkinDeserialisationTest.cs | 185 ----------------- .../Skins/TestSceneSkinDeserialisation.cs | 194 ++++++++++++++++++ 2 files changed, 194 insertions(+), 185 deletions(-) delete mode 100644 osu.Game.Tests/Skins/SkinDeserialisationTest.cs create mode 100644 osu.Game.Tests/Skins/TestSceneSkinDeserialisation.cs diff --git a/osu.Game.Tests/Skins/SkinDeserialisationTest.cs b/osu.Game.Tests/Skins/SkinDeserialisationTest.cs deleted file mode 100644 index 7372557161..0000000000 --- a/osu.Game.Tests/Skins/SkinDeserialisationTest.cs +++ /dev/null @@ -1,185 +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 System.Collections.Generic; -using System.Linq; -using NUnit.Framework; -using osu.Framework.Audio.Sample; -using osu.Framework.Bindables; -using osu.Framework.Graphics.Textures; -using osu.Framework.IO.Stores; -using osu.Game.Audio; -using osu.Game.IO; -using osu.Game.IO.Archives; -using osu.Game.Screens.Menu; -using osu.Game.Screens.Play.HUD; -using osu.Game.Screens.Play.HUD.HitErrorMeters; -using osu.Game.Skinning; -using osu.Game.Skinning.Components; -using osu.Game.Tests.Resources; - -namespace osu.Game.Tests.Skins -{ - /// - /// Test that the main components (which are serialised based on namespace/class name) - /// remain compatible with any changes. - /// - /// - /// If this test breaks, check any naming or class structure changes. - /// Migration rules may need to be added to . - /// - [TestFixture] - public class SkinDeserialisationTest - { - private static readonly string[] available_skins = - { - // Covers song progress before namespace changes, and most other components. - "Archives/modified-default-20220723.osk", - "Archives/modified-classic-20220723.osk", - // Covers legacy song progress, UR counter, colour hit error metre. - "Archives/modified-classic-20220801.osk", - // Covers clicks/s counter - "Archives/modified-default-20220818.osk", - // Covers longest combo counter - "Archives/modified-default-20221012.osk", - // Covers Argon variant of song progress bar - "Archives/modified-argon-20221024.osk", - // Covers TextElement and BeatmapInfoDrawable - "Archives/modified-default-20221102.osk", - // Covers BPM counter. - "Archives/modified-default-20221205.osk", - // Covers judgement counter. - "Archives/modified-default-20230117.osk", - // Covers player avatar and flag. - "Archives/modified-argon-20230305.osk", - // Covers key counters - "Archives/modified-argon-pro-20230618.osk", - // Covers "Argon" health display - "Archives/modified-argon-pro-20231001.osk", - // Covers player name text component. - "Archives/modified-argon-20231106.osk", - // Covers "Argon" accuracy/score/combo counters, and wedges - "Archives/modified-argon-20231108.osk", - // Covers "Argon" performance points counter - "Archives/modified-argon-20240305.osk", - // Covers default rank display - "Archives/modified-default-20230809.osk", - // Covers legacy rank display - "Archives/modified-classic-20230809.osk", - // Covers legacy key counter - "Archives/modified-classic-20240724.osk" - }; - - /// - /// If this test fails, new test resources should be added to include new components. - /// - [Test] - public void TestSkinnableComponentsCoveredByDeserialisationTests() - { - HashSet instantiatedTypes = new HashSet(); - - foreach (string oskFile in available_skins) - { - using (var stream = TestResources.OpenResource(oskFile)) - using (var storage = new ZipArchiveReader(stream)) - { - var skin = new TestSkin(new SkinInfo(), null, storage); - - foreach (var target in skin.LayoutInfos) - { - foreach (var info in target.Value.AllDrawables) - instantiatedTypes.Add(info.Type); - } - } - } - - var editableTypes = SerialisedDrawableInfo.GetAllAvailableDrawables().Where(t => (Activator.CreateInstance(t) as ISerialisableDrawable)?.IsEditable == true); - - Assert.That(instantiatedTypes, Is.EquivalentTo(editableTypes)); - } - - [Test] - public void TestDeserialiseModifiedDefault() - { - using (var stream = TestResources.OpenResource("Archives/modified-default-20220723.osk")) - using (var storage = new ZipArchiveReader(stream)) - { - var skin = new TestSkin(new SkinInfo(), null, storage); - - Assert.That(skin.LayoutInfos, Has.Count.EqualTo(2)); - Assert.That(skin.LayoutInfos[GlobalSkinnableContainers.MainHUDComponents].AllDrawables.ToArray(), Has.Length.EqualTo(9)); - } - } - - [Test] - public void TestDeserialiseModifiedArgon() - { - using (var stream = TestResources.OpenResource("Archives/modified-argon-20231106.osk")) - using (var storage = new ZipArchiveReader(stream)) - { - var skin = new TestSkin(new SkinInfo(), null, storage); - - Assert.That(skin.LayoutInfos, Has.Count.EqualTo(2)); - Assert.That(skin.LayoutInfos[GlobalSkinnableContainers.MainHUDComponents].AllDrawables.ToArray(), Has.Length.EqualTo(10)); - Assert.That(skin.LayoutInfos[GlobalSkinnableContainers.MainHUDComponents].AllDrawables.Select(i => i.Type), Contains.Item(typeof(PlayerName))); - } - } - - [Test] - public void TestDeserialiseInvalidDrawables() - { - using (var stream = TestResources.OpenResource("Archives/argon-invalid-drawable.osk")) - using (var storage = new ZipArchiveReader(stream)) - { - var skin = new TestSkin(new SkinInfo(), null, storage); - - Assert.That(skin.LayoutInfos.Any(kvp => kvp.Value.AllDrawables.Any(d => d.Type == typeof(StarFountain))), Is.False); - } - } - - [Test] - public void TestDeserialiseModifiedClassic() - { - using (var stream = TestResources.OpenResource("Archives/modified-classic-20220723.osk")) - using (var storage = new ZipArchiveReader(stream)) - { - var skin = new TestSkin(new SkinInfo(), null, storage); - - Assert.That(skin.LayoutInfos, Has.Count.EqualTo(2)); - Assert.That(skin.LayoutInfos[GlobalSkinnableContainers.MainHUDComponents].AllDrawables.ToArray(), Has.Length.EqualTo(6)); - Assert.That(skin.LayoutInfos[GlobalSkinnableContainers.SongSelect].AllDrawables.ToArray(), Has.Length.EqualTo(1)); - - var skinnableInfo = skin.LayoutInfos[GlobalSkinnableContainers.SongSelect].AllDrawables.First(); - - Assert.That(skinnableInfo.Type, Is.EqualTo(typeof(SkinnableSprite))); - Assert.That(skinnableInfo.Settings.First().Key, Is.EqualTo("sprite_name")); - Assert.That(skinnableInfo.Settings.First().Value, Is.EqualTo("ppy_logo-2.png")); - } - - using (var stream = TestResources.OpenResource("Archives/modified-classic-20220801.osk")) - using (var storage = new ZipArchiveReader(stream)) - { - var skin = new TestSkin(new SkinInfo(), null, storage); - Assert.That(skin.LayoutInfos[GlobalSkinnableContainers.MainHUDComponents].AllDrawables.ToArray(), Has.Length.EqualTo(8)); - Assert.That(skin.LayoutInfos[GlobalSkinnableContainers.MainHUDComponents].AllDrawables.Select(i => i.Type), Contains.Item(typeof(UnstableRateCounter))); - Assert.That(skin.LayoutInfos[GlobalSkinnableContainers.MainHUDComponents].AllDrawables.Select(i => i.Type), Contains.Item(typeof(ColourHitErrorMeter))); - Assert.That(skin.LayoutInfos[GlobalSkinnableContainers.MainHUDComponents].AllDrawables.Select(i => i.Type), Contains.Item(typeof(LegacySongProgress))); - } - } - - private class TestSkin : Skin - { - public TestSkin(SkinInfo skin, IStorageResourceProvider? resources, IResourceStore? fallbackStore = null, string configurationFilename = "skin.ini") - : base(skin, resources, fallbackStore, configurationFilename) - { - } - - public override Texture GetTexture(string componentName, WrapMode wrapModeS, WrapMode wrapModeT) => throw new NotImplementedException(); - - public override IBindable GetConfig(TLookup lookup) => throw new NotImplementedException(); - - public override ISample GetSample(ISampleInfo sampleInfo) => throw new NotImplementedException(); - } - } -} diff --git a/osu.Game.Tests/Skins/TestSceneSkinDeserialisation.cs b/osu.Game.Tests/Skins/TestSceneSkinDeserialisation.cs new file mode 100644 index 0000000000..595180c27c --- /dev/null +++ b/osu.Game.Tests/Skins/TestSceneSkinDeserialisation.cs @@ -0,0 +1,194 @@ +// 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.Collections.Generic; +using System.IO; +using System.Linq; +using NUnit.Framework; +using osu.Framework.Allocation; +using osu.Framework.Extensions; +using osu.Game.Database; +using osu.Game.Screens.Menu; +using osu.Game.Screens.Play.HUD; +using osu.Game.Screens.Play.HUD.HitErrorMeters; +using osu.Game.Skinning; +using osu.Game.Skinning.Components; +using osu.Game.Tests.Resources; +using osu.Game.Tests.Visual; + +namespace osu.Game.Tests.Skins +{ + /// + /// Test that the main components (which are serialised based on namespace/class name) + /// remain compatible with any changes. + /// + /// + /// If this test breaks, check any naming or class structure changes. + /// Migration rules may need to be added to . + /// + public partial class TestSceneSkinDeserialisation : OsuTestScene + { + private static readonly string[] available_skins = + { + // Covers song progress before namespace changes, and most other components. + "Archives/modified-default-20220723.osk", + "Archives/modified-classic-20220723.osk", + // Covers legacy song progress, UR counter, colour hit error metre. + "Archives/modified-classic-20220801.osk", + // Covers clicks/s counter + "Archives/modified-default-20220818.osk", + // Covers longest combo counter + "Archives/modified-default-20221012.osk", + // Covers Argon variant of song progress bar + "Archives/modified-argon-20221024.osk", + // Covers TextElement and BeatmapInfoDrawable + "Archives/modified-default-20221102.osk", + // Covers BPM counter. + "Archives/modified-default-20221205.osk", + // Covers judgement counter. + "Archives/modified-default-20230117.osk", + // Covers player avatar and flag. + "Archives/modified-argon-20230305.osk", + // Covers key counters + "Archives/modified-argon-pro-20230618.osk", + // Covers "Argon" health display + "Archives/modified-argon-pro-20231001.osk", + // Covers player name text component. + "Archives/modified-argon-20231106.osk", + // Covers "Argon" accuracy/score/combo counters, and wedges + "Archives/modified-argon-20231108.osk", + // Covers "Argon" performance points counter + "Archives/modified-argon-20240305.osk", + // Covers default rank display + "Archives/modified-default-20230809.osk", + // Covers legacy rank display + "Archives/modified-classic-20230809.osk", + // Covers legacy key counter + "Archives/modified-classic-20240724.osk" + }; + + [Resolved] + private SkinManager skins { get; set; } = null!; + + /// + /// If this test fails, new test resources should be added to include new components. + /// + [Test] + public void TestSkinnableComponentsCoveredByDeserialisationTests() + { + HashSet instantiatedTypes = new HashSet(); + + AddStep("load skin", () => + { + foreach (string oskFile in available_skins) + { + var skin = importSkinFromArchives(oskFile); + + foreach (var target in skin.LayoutInfos) + { + foreach (var info in target.Value.AllDrawables) + instantiatedTypes.Add(info.Type); + } + } + }); + + var existingTypes = SerialisedDrawableInfo.GetAllAvailableDrawables().Where(t => (Activator.CreateInstance(t) as ISerialisableDrawable)?.IsEditable == true); + + AddAssert("all types available", () => instantiatedTypes, () => Is.EquivalentTo(existingTypes)); + } + + [Test] + public void TestDeserialiseModifiedDefault() + { + Skin skin = null!; + + AddStep("load skin", () => skin = importSkinFromArchives("Archives/modified-default-20220723.osk")); + + AddAssert("layouts count = 2", () => skin.LayoutInfos, () => Has.Count.EqualTo(2)); + AddAssert("hud count = 12", + () => skin.LayoutInfos[GlobalSkinnableContainers.MainHUDComponents].AllDrawables.ToArray(), + () => Has.Length.EqualTo(12)); + } + + [Test] + public void TestDeserialiseModifiedArgon() + { + Skin skin = null!; + + AddStep("load skin", () => skin = importSkinFromArchives("Archives/modified-argon-20231106.osk")); + + AddAssert("layouts count = 2", () => skin.LayoutInfos, () => Has.Count.EqualTo(2)); + AddAssert("hud count = 13", + () => skin.LayoutInfos[GlobalSkinnableContainers.MainHUDComponents].AllDrawables.ToArray(), + () => Has.Length.EqualTo(13)); + + AddAssert("hud contains player name", + () => skin.LayoutInfos[GlobalSkinnableContainers.MainHUDComponents].AllDrawables.Select(i => i.Type), + () => Does.Contain(typeof(PlayerName))); + } + + [Test] + public void TestDeserialiseInvalidDrawables() + { + Skin skin = null!; + + AddStep("load skin", () => skin = importSkinFromArchives("Archives/argon-invalid-drawable.osk")); + + AddAssert("skin does not contain star fountain", + () => skin.LayoutInfos.SelectMany(kvp => kvp.Value.AllDrawables).Select(d => d.Type), + () => Does.Not.Contain(typeof(StarFountain))); + } + + [Test] + public void TestDeserialiseModifiedClassic() + { + Skin skin = null!; + + AddStep("load skin", () => skin = importSkinFromArchives("Archives/modified-classic-20220723.osk")); + + AddAssert("layouts count = 2", () => skin.LayoutInfos, () => Has.Count.EqualTo(2)); + AddAssert("hud count = 11", + () => skin.LayoutInfos[GlobalSkinnableContainers.MainHUDComponents].AllDrawables.ToArray(), + () => Has.Length.EqualTo(11)); + + AddAssert("song select count = 1", + () => skin.LayoutInfos[GlobalSkinnableContainers.SongSelect].AllDrawables.ToArray(), + () => Has.Length.EqualTo(1)); + + AddAssert("song select component correct", () => + { + var skinnableInfo = skin.LayoutInfos[GlobalSkinnableContainers.SongSelect].AllDrawables.First(); + + Assert.That(skinnableInfo.Type, Is.EqualTo(typeof(SkinnableSprite))); + Assert.That(skinnableInfo.Settings.First().Key, Is.EqualTo("sprite_name")); + Assert.That(skinnableInfo.Settings.First().Value, Is.EqualTo("ppy_logo-2.png")); + return true; + }); + + AddStep("load another skin", () => skin = importSkinFromArchives("Archives/modified-classic-20220801.osk")); + + AddAssert("hud count = 13", + () => skin.LayoutInfos[GlobalSkinnableContainers.MainHUDComponents].AllDrawables.ToArray(), + () => Has.Length.EqualTo(13)); + + AddAssert("hud contains ur counter", + () => skin.LayoutInfos[GlobalSkinnableContainers.MainHUDComponents].AllDrawables.Select(i => i.Type), + () => Does.Contain(typeof(UnstableRateCounter))); + + AddAssert("hud contains colour hit error", + () => skin.LayoutInfos[GlobalSkinnableContainers.MainHUDComponents].AllDrawables.Select(i => i.Type), + () => Does.Contain(typeof(ColourHitErrorMeter))); + + AddAssert("hud contains legacy song progress", + () => skin.LayoutInfos[GlobalSkinnableContainers.MainHUDComponents].AllDrawables.Select(i => i.Type), + () => Does.Contain(typeof(LegacySongProgress))); + } + + private Skin importSkinFromArchives(string filename) + { + var imported = skins.Import(new ImportTask(TestResources.OpenResource(filename), Path.GetFileNameWithoutExtension(filename))).GetResultSafely(); + return imported.PerformRead(skinInfo => skins.GetSkin(skinInfo)); + } + } +} From c12cb2582e2ff15b015d21e95b384623c00237b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Mon, 2 Dec 2024 10:03:19 +0100 Subject: [PATCH 12/12] Document & reduce visibility of `GetInstanceType()` --- osu.Game/Skinning/SkinInfo.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/osu.Game/Skinning/SkinInfo.cs b/osu.Game/Skinning/SkinInfo.cs index ac172e4060..571deca8aa 100644 --- a/osu.Game/Skinning/SkinInfo.cs +++ b/osu.Game/Skinning/SkinInfo.cs @@ -39,7 +39,10 @@ namespace osu.Game.Skinning public bool Protected { get; set; } - public Type GetInstanceType() + /// + /// Returns the specific subtype of that will be constructed on calling . + /// + internal Type GetInstanceType() { var type = string.IsNullOrEmpty(InstantiationInfo) // handle the case of skins imported before InstantiationInfo was added.