From 4c25fe750f193f7e624ffce2185070d85267e73b Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Sat, 15 May 2021 14:32:15 +0300 Subject: [PATCH 01/43] Disallow beatmap skin to fall back to default HUD components This should become a more generalized `AllowDefaultSkinFallback` when default legacy skin fallback is supported. --- osu.Game/Skinning/LegacyBeatmapSkin.cs | 1 + osu.Game/Skinning/LegacySkin.cs | 7 +++++++ 2 files changed, 8 insertions(+) diff --git a/osu.Game/Skinning/LegacyBeatmapSkin.cs b/osu.Game/Skinning/LegacyBeatmapSkin.cs index 3ec205e897..1a298576f9 100644 --- a/osu.Game/Skinning/LegacyBeatmapSkin.cs +++ b/osu.Game/Skinning/LegacyBeatmapSkin.cs @@ -14,6 +14,7 @@ namespace osu.Game.Skinning public class LegacyBeatmapSkin : LegacySkin { protected override bool AllowManiaSkin => false; + protected override bool AllowDefaultHUDComponentsFallback => false; protected override bool UseCustomSampleBanks => true; public LegacyBeatmapSkin(BeatmapInfo beatmap, IResourceStore storage, IStorageResourceProvider resources) diff --git a/osu.Game/Skinning/LegacySkin.cs b/osu.Game/Skinning/LegacySkin.cs index a6f8f45c0f..e7edba1e13 100644 --- a/osu.Game/Skinning/LegacySkin.cs +++ b/osu.Game/Skinning/LegacySkin.cs @@ -38,6 +38,11 @@ namespace osu.Game.Skinning protected virtual bool AllowManiaSkin => hasKeyTexture.Value; + /// + /// Whether this skin will fall back to default HUD components if it has no fonts for use. + /// + protected virtual bool AllowDefaultHUDComponentsFallback => true; + /// /// Whether this skin can use samples with a custom bank (custom sample set in stable terminology). /// Added in order to match sample lookup logic from stable (in stable, only the beatmap skin could use samples with a custom sample bank). @@ -331,6 +336,8 @@ namespace osu.Game.Skinning switch (target.Target) { case SkinnableTarget.MainHUDComponents: + if (!this.HasFont(LegacyFont.Score) && !AllowDefaultHUDComponentsFallback) + return null; var skinnableTargetWrapper = new SkinnableTargetComponentsContainer(container => { From 102842bcf1c1919c75a7693771e6b3ce51d83e6f Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Sat, 15 May 2021 14:32:58 +0300 Subject: [PATCH 02/43] Expire legacy combo counters on catch ruleset --- .../Legacy/CatchLegacySkinTransformer.cs | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/osu.Game.Rulesets.Catch/Skinning/Legacy/CatchLegacySkinTransformer.cs b/osu.Game.Rulesets.Catch/Skinning/Legacy/CatchLegacySkinTransformer.cs index 1b48832ed6..6c477154ac 100644 --- a/osu.Game.Rulesets.Catch/Skinning/Legacy/CatchLegacySkinTransformer.cs +++ b/osu.Game.Rulesets.Catch/Skinning/Legacy/CatchLegacySkinTransformer.cs @@ -1,8 +1,10 @@ // 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 osu.Framework.Bindables; using osu.Framework.Graphics; +using osu.Game.Screens.Play.HUD; using osu.Game.Skinning; using osuTK.Graphics; @@ -22,14 +24,17 @@ namespace osu.Game.Rulesets.Catch.Skinning.Legacy public override Drawable GetDrawableComponent(ISkinComponent component) { - if (component is HUDSkinComponent hudComponent) + if (component is SkinnableTargetComponent targetComponent && targetComponent.Target == SkinnableTarget.MainHUDComponents) { - switch (hudComponent.Component) - { - case HUDSkinComponents.ComboCounter: - // catch may provide its own combo counter; hide the default. - return providesComboCounter ? Drawable.Empty() : null; - } + if (!providesComboCounter || !(Source.GetDrawableComponent(component) is SkinnableTargetComponentsContainer components)) + return null; + + // catch may provide its own combo counter; hide the default. + // todo: this should probably be done in an elegant way. + foreach (var legacyComboCounter in components.OfType()) + legacyComboCounter.Expire(); + + return components; } if (!(component is CatchSkinComponent catchSkinComponent)) From 243c8aa58557719f8ce8983dd848395e4f399d44 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Sat, 15 May 2021 18:02:38 +0300 Subject: [PATCH 03/43] Add test coverage --- .../TestSceneCatchPlayerLegacySkin.cs | 26 +++++++++++++++++++ .../Tests/Visual/LegacySkinPlayerTestScene.cs | 10 +++++-- 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/osu.Game.Rulesets.Catch.Tests/TestSceneCatchPlayerLegacySkin.cs b/osu.Game.Rulesets.Catch.Tests/TestSceneCatchPlayerLegacySkin.cs index 64695153b5..d49470079b 100644 --- a/osu.Game.Rulesets.Catch.Tests/TestSceneCatchPlayerLegacySkin.cs +++ b/osu.Game.Rulesets.Catch.Tests/TestSceneCatchPlayerLegacySkin.cs @@ -1,8 +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 System.Linq; using NUnit.Framework; +using osu.Framework.Allocation; +using osu.Framework.Extensions.IEnumerableExtensions; +using osu.Framework.Screens; +using osu.Framework.Testing; +using osu.Game.Screens.Play.HUD; +using osu.Game.Skinning; using osu.Game.Tests.Visual; +using osuTK; namespace osu.Game.Rulesets.Catch.Tests { @@ -10,5 +18,23 @@ namespace osu.Game.Rulesets.Catch.Tests public class TestSceneCatchPlayerLegacySkin : LegacySkinPlayerTestScene { protected override Ruleset CreatePlayerRuleset() => new CatchRuleset(); + + [Test] + [Ignore("HUD components broken, remove when fixed.")] + public void TestLegacyHUDComboCounterHidden([Values] bool withModifiedSkin) + { + if (withModifiedSkin) + { + AddStep("change component scale", () => Player.ChildrenOfType().First().Scale = new Vector2(2f)); + AddStep("update target", () => Player.ChildrenOfType().ForEach(LegacySkin.UpdateDrawableTarget)); + AddStep("exit player", () => Player.Exit()); + CreateTest(null); + } + + AddAssert("legacy HUD combo counter hidden", () => + { + return Player.ChildrenOfType().All(counter => !counter.IsPresent || !counter.IsAlive); + }); + } } } diff --git a/osu.Game/Tests/Visual/LegacySkinPlayerTestScene.cs b/osu.Game/Tests/Visual/LegacySkinPlayerTestScene.cs index c186525757..80f09e21f3 100644 --- a/osu.Game/Tests/Visual/LegacySkinPlayerTestScene.cs +++ b/osu.Game/Tests/Visual/LegacySkinPlayerTestScene.cs @@ -3,7 +3,11 @@ using NUnit.Framework; using osu.Framework.Allocation; +using osu.Framework.Audio; +using osu.Framework.Graphics.Textures; using osu.Framework.IO.Stores; +using osu.Framework.Platform; +using osu.Game.IO; using osu.Game.Rulesets; using osu.Game.Skinning; @@ -12,6 +16,8 @@ namespace osu.Game.Tests.Visual [TestFixture] public abstract class LegacySkinPlayerTestScene : PlayerTestScene { + protected LegacySkin LegacySkin { get; private set; } + private ISkinSource legacySkinSource; protected override TestPlayer CreatePlayer(Ruleset ruleset) => new SkinProvidingPlayer(legacySkinSource); @@ -19,8 +25,8 @@ namespace osu.Game.Tests.Visual [BackgroundDependencyLoader] private void load(OsuGameBase game, SkinManager skins) { - var legacySkin = new DefaultLegacySkin(new NamespacedResourceStore(game.Resources, "Skins/Legacy"), skins); - legacySkinSource = new SkinProvidingContainer(legacySkin); + LegacySkin = new DefaultLegacySkin(new NamespacedResourceStore(game.Resources, "Skins/Legacy"), skins); + legacySkinSource = new SkinProvidingContainer(LegacySkin); } public class SkinProvidingPlayer : TestPlayer From f00799cc643f34b0d76ef202c1af6c8efa74472d Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Sat, 15 May 2021 18:36:46 +0300 Subject: [PATCH 04/43] Remove unused using directive ...damn it --- .../TestSceneCatchPlayerLegacySkin.cs | 1 - osu.Game/Tests/Visual/LegacySkinPlayerTestScene.cs | 4 ---- 2 files changed, 5 deletions(-) diff --git a/osu.Game.Rulesets.Catch.Tests/TestSceneCatchPlayerLegacySkin.cs b/osu.Game.Rulesets.Catch.Tests/TestSceneCatchPlayerLegacySkin.cs index d49470079b..7f0cbc6943 100644 --- a/osu.Game.Rulesets.Catch.Tests/TestSceneCatchPlayerLegacySkin.cs +++ b/osu.Game.Rulesets.Catch.Tests/TestSceneCatchPlayerLegacySkin.cs @@ -3,7 +3,6 @@ using System.Linq; using NUnit.Framework; -using osu.Framework.Allocation; using osu.Framework.Extensions.IEnumerableExtensions; using osu.Framework.Screens; using osu.Framework.Testing; diff --git a/osu.Game/Tests/Visual/LegacySkinPlayerTestScene.cs b/osu.Game/Tests/Visual/LegacySkinPlayerTestScene.cs index 80f09e21f3..907a37f4b9 100644 --- a/osu.Game/Tests/Visual/LegacySkinPlayerTestScene.cs +++ b/osu.Game/Tests/Visual/LegacySkinPlayerTestScene.cs @@ -3,11 +3,7 @@ using NUnit.Framework; using osu.Framework.Allocation; -using osu.Framework.Audio; -using osu.Framework.Graphics.Textures; using osu.Framework.IO.Stores; -using osu.Framework.Platform; -using osu.Game.IO; using osu.Game.Rulesets; using osu.Game.Skinning; From df248ea41b76cb5425b37b45c2e270d8843115ee Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Mon, 17 May 2021 11:41:53 +0300 Subject: [PATCH 05/43] Improve code readability --- .../Legacy/CatchLegacySkinTransformer.cs | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/osu.Game.Rulesets.Catch/Skinning/Legacy/CatchLegacySkinTransformer.cs b/osu.Game.Rulesets.Catch/Skinning/Legacy/CatchLegacySkinTransformer.cs index 6c477154ac..db8c9a2e95 100644 --- a/osu.Game.Rulesets.Catch/Skinning/Legacy/CatchLegacySkinTransformer.cs +++ b/osu.Game.Rulesets.Catch/Skinning/Legacy/CatchLegacySkinTransformer.cs @@ -26,15 +26,20 @@ namespace osu.Game.Rulesets.Catch.Skinning.Legacy { if (component is SkinnableTargetComponent targetComponent && targetComponent.Target == SkinnableTarget.MainHUDComponents) { - if (!providesComboCounter || !(Source.GetDrawableComponent(component) is SkinnableTargetComponentsContainer components)) + if (!providesComboCounter) return null; - // catch may provide its own combo counter; hide the default. - // todo: this should probably be done in an elegant way. - foreach (var legacyComboCounter in components.OfType()) - legacyComboCounter.Expire(); + if (Source.GetDrawableComponent(component) is SkinnableTargetComponentsContainer components) + { + // catch may provide its own combo counter; hide the default. + // todo: this should probably be done in an elegant way. + foreach (var legacyComboCounter in components.OfType()) + legacyComboCounter.Expire(); - return components; + return components; + } + + return null; } if (!(component is CatchSkinComponent catchSkinComponent)) From f667ea3fd0045f5d365dfe20a90bd32936c630d8 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Mon, 17 May 2021 21:28:14 +0300 Subject: [PATCH 06/43] Replace `AllowDefaultHUDComponentsFallback` with a temporary override at `LegacyBeatmapSkin` --- osu.Game/Skinning/LegacyBeatmapSkin.cs | 15 ++++++++++++++- osu.Game/Skinning/LegacySkin.cs | 8 -------- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/osu.Game/Skinning/LegacyBeatmapSkin.cs b/osu.Game/Skinning/LegacyBeatmapSkin.cs index 1a298576f9..91b70395f4 100644 --- a/osu.Game/Skinning/LegacyBeatmapSkin.cs +++ b/osu.Game/Skinning/LegacyBeatmapSkin.cs @@ -3,6 +3,7 @@ using osu.Framework.Audio.Sample; using osu.Framework.Bindables; +using osu.Framework.Graphics; using osu.Framework.IO.Stores; using osu.Game.Audio; using osu.Game.Beatmaps; @@ -14,7 +15,6 @@ namespace osu.Game.Skinning public class LegacyBeatmapSkin : LegacySkin { protected override bool AllowManiaSkin => false; - protected override bool AllowDefaultHUDComponentsFallback => false; protected override bool UseCustomSampleBanks => true; public LegacyBeatmapSkin(BeatmapInfo beatmap, IResourceStore storage, IStorageResourceProvider resources) @@ -24,6 +24,19 @@ namespace osu.Game.Skinning Configuration.AllowDefaultComboColoursFallback = false; } + public override Drawable GetDrawableComponent(ISkinComponent component) + { + if (component is SkinnableTargetComponent targetComponent && targetComponent.Target == SkinnableTarget.MainHUDComponents) + { + // for now, if the beatmap skin doesn't skin the score font, fall back to current skin + // instead of potentially returning default lazer skin HUD components from here. + if (!this.HasFont(LegacyFont.Score)) + return null; + } + + return base.GetDrawableComponent(component); + } + public override IBindable GetConfig(TLookup lookup) { switch (lookup) diff --git a/osu.Game/Skinning/LegacySkin.cs b/osu.Game/Skinning/LegacySkin.cs index e7edba1e13..6b4f140c12 100644 --- a/osu.Game/Skinning/LegacySkin.cs +++ b/osu.Game/Skinning/LegacySkin.cs @@ -38,11 +38,6 @@ namespace osu.Game.Skinning protected virtual bool AllowManiaSkin => hasKeyTexture.Value; - /// - /// Whether this skin will fall back to default HUD components if it has no fonts for use. - /// - protected virtual bool AllowDefaultHUDComponentsFallback => true; - /// /// Whether this skin can use samples with a custom bank (custom sample set in stable terminology). /// Added in order to match sample lookup logic from stable (in stable, only the beatmap skin could use samples with a custom sample bank). @@ -336,9 +331,6 @@ namespace osu.Game.Skinning switch (target.Target) { case SkinnableTarget.MainHUDComponents: - if (!this.HasFont(LegacyFont.Score) && !AllowDefaultHUDComponentsFallback) - return null; - var skinnableTargetWrapper = new SkinnableTargetComponentsContainer(container => { var score = container.OfType().FirstOrDefault(); From ff419af5128a3e0f2d188e10fcb05aed712ac128 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Tue, 18 May 2021 09:08:56 +0300 Subject: [PATCH 07/43] Hide the combo counter content rather than full death --- .../TestSceneCatchPlayerLegacySkin.cs | 3 +- .../Legacy/CatchLegacySkinTransformer.cs | 4 +- .../Screens/Play/HUD/LegacyComboCounter.cs | 50 ++++++++++++------- 3 files changed, 36 insertions(+), 21 deletions(-) diff --git a/osu.Game.Rulesets.Catch.Tests/TestSceneCatchPlayerLegacySkin.cs b/osu.Game.Rulesets.Catch.Tests/TestSceneCatchPlayerLegacySkin.cs index 7f0cbc6943..4e56e4b4a7 100644 --- a/osu.Game.Rulesets.Catch.Tests/TestSceneCatchPlayerLegacySkin.cs +++ b/osu.Game.Rulesets.Catch.Tests/TestSceneCatchPlayerLegacySkin.cs @@ -4,6 +4,7 @@ using System.Linq; using NUnit.Framework; using osu.Framework.Extensions.IEnumerableExtensions; +using osu.Framework.Graphics.Containers; using osu.Framework.Screens; using osu.Framework.Testing; using osu.Game.Screens.Play.HUD; @@ -32,7 +33,7 @@ namespace osu.Game.Rulesets.Catch.Tests AddAssert("legacy HUD combo counter hidden", () => { - return Player.ChildrenOfType().All(counter => !counter.IsPresent || !counter.IsAlive); + return Player.ChildrenOfType().All(c => !c.ChildrenOfType().Single().IsPresent); }); } } diff --git a/osu.Game.Rulesets.Catch/Skinning/Legacy/CatchLegacySkinTransformer.cs b/osu.Game.Rulesets.Catch/Skinning/Legacy/CatchLegacySkinTransformer.cs index db8c9a2e95..d757f36cde 100644 --- a/osu.Game.Rulesets.Catch/Skinning/Legacy/CatchLegacySkinTransformer.cs +++ b/osu.Game.Rulesets.Catch/Skinning/Legacy/CatchLegacySkinTransformer.cs @@ -32,9 +32,9 @@ namespace osu.Game.Rulesets.Catch.Skinning.Legacy if (Source.GetDrawableComponent(component) is SkinnableTargetComponentsContainer components) { // catch may provide its own combo counter; hide the default. - // todo: this should probably be done in an elegant way. + // todo: this should be done in an elegant way per ruleset, defining which HUD skin components should be displayed. foreach (var legacyComboCounter in components.OfType()) - legacyComboCounter.Expire(); + legacyComboCounter.ContentVisible = false; return components; } diff --git a/osu.Game/Screens/Play/HUD/LegacyComboCounter.cs b/osu.Game/Screens/Play/HUD/LegacyComboCounter.cs index d64513d41e..ee00c71b0f 100644 --- a/osu.Game/Screens/Play/HUD/LegacyComboCounter.cs +++ b/osu.Game/Screens/Play/HUD/LegacyComboCounter.cs @@ -17,7 +17,7 @@ namespace osu.Game.Screens.Play.HUD /// public class LegacyComboCounter : CompositeDrawable, ISkinnableDrawable { - public Bindable Current { get; } = new BindableInt { MinValue = 0, }; + public Bindable Current { get; } = new BindableInt { MinValue = 0 }; private uint scheduledPopOutCurrentId; @@ -32,9 +32,9 @@ namespace osu.Game.Screens.Play.HUD /// private const double rolling_duration = 20; - private Drawable popOutCount; + private readonly Drawable popOutCount; - private Drawable displayedCountSpriteText; + private readonly Drawable displayedCountSpriteText; private int previousValue; @@ -45,6 +45,13 @@ namespace osu.Game.Screens.Play.HUD [Resolved] private ISkinSource skin { get; set; } + private readonly Container counterContainer; + + public bool ContentVisible + { + set => counterContainer.Alpha = value ? 1 : 0; + } + public LegacyComboCounter() { AutoSizeAxes = Axes.Both; @@ -55,6 +62,25 @@ namespace osu.Game.Screens.Play.HUD Margin = new MarginPadding(10); Scale = new Vector2(1.2f); + + InternalChild = counterContainer = new Container + { + AutoSizeAxes = Axes.Both, + AlwaysPresent = true, + Children = new[] + { + popOutCount = new LegacySpriteText(LegacyFont.Combo) + { + Alpha = 0, + Margin = new MarginPadding(0.05f), + Blending = BlendingParameters.Additive, + }, + displayedCountSpriteText = new LegacySpriteText(LegacyFont.Combo) + { + Alpha = 0, + }, + } + }; } /// @@ -82,20 +108,6 @@ namespace osu.Game.Screens.Play.HUD [BackgroundDependencyLoader] private void load(ScoreProcessor scoreProcessor) { - InternalChildren = new[] - { - popOutCount = new LegacySpriteText(LegacyFont.Combo) - { - Alpha = 0, - Margin = new MarginPadding(0.05f), - Blending = BlendingParameters.Additive, - }, - displayedCountSpriteText = new LegacySpriteText(LegacyFont.Combo) - { - Alpha = 0, - }, - }; - Current.BindTo(scoreProcessor.Combo); } @@ -105,10 +117,12 @@ namespace osu.Game.Screens.Play.HUD ((IHasText)displayedCountSpriteText).Text = formatCount(Current.Value); + counterContainer.Anchor = Anchor; + counterContainer.Origin = Origin; displayedCountSpriteText.Anchor = Anchor; displayedCountSpriteText.Origin = Origin; - popOutCount.Origin = Origin; popOutCount.Anchor = Anchor; + popOutCount.Origin = Origin; Current.BindValueChanged(combo => updateCount(combo.NewValue == 0), true); } From 265a89e5cc778e5c7054f566bdcaba16d61bf3a6 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Tue, 18 May 2021 09:45:32 +0300 Subject: [PATCH 08/43] Fix `LegacySkinPlayerTestScene` overriden by default beatmap skin --- .../Tests/Visual/LegacySkinPlayerTestScene.cs | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/osu.Game/Tests/Visual/LegacySkinPlayerTestScene.cs b/osu.Game/Tests/Visual/LegacySkinPlayerTestScene.cs index c186525757..27ddd77c03 100644 --- a/osu.Game/Tests/Visual/LegacySkinPlayerTestScene.cs +++ b/osu.Game/Tests/Visual/LegacySkinPlayerTestScene.cs @@ -1,11 +1,17 @@ // 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.Allocation; +using osu.Framework.Audio; using osu.Framework.IO.Stores; +using osu.Framework.Testing; +using osu.Framework.Timing; +using osu.Game.Beatmaps; using osu.Game.Rulesets; using osu.Game.Skinning; +using osu.Game.Storyboards; namespace osu.Game.Tests.Visual { @@ -16,6 +22,9 @@ namespace osu.Game.Tests.Visual protected override TestPlayer CreatePlayer(Ruleset ruleset) => new SkinProvidingPlayer(legacySkinSource); + protected override WorkingBeatmap CreateWorkingBeatmap(IBeatmap beatmap, Storyboard storyboard = null) + => new LegacySkinWorkingBeatmap(beatmap, storyboard, Clock, Audio); + [BackgroundDependencyLoader] private void load(OsuGameBase game, SkinManager skins) { @@ -23,6 +32,14 @@ namespace osu.Game.Tests.Visual legacySkinSource = new SkinProvidingContainer(legacySkin); } + public override void SetUpSteps() + { + base.SetUpSteps(); + + // check presence of a random legacy HUD component to ensure this is using legacy skin. + AddAssert("using legacy skin", () => this.ChildrenOfType().Any()); + } + public class SkinProvidingPlayer : TestPlayer { [Cached(typeof(ISkinSource))] @@ -33,5 +50,15 @@ namespace osu.Game.Tests.Visual this.skinSource = skinSource; } } + + private class LegacySkinWorkingBeatmap : ClockBackedTestWorkingBeatmap + { + public LegacySkinWorkingBeatmap(IBeatmap beatmap, Storyboard storyboard, IFrameBasedClock frameBasedClock, AudioManager audio) + : base(beatmap, storyboard, frameBasedClock, audio) + { + } + + protected override ISkin GetSkin() => new LegacyBeatmapSkin(BeatmapInfo, null, null); + } } } From 4e12a2734ccf28dae236d6c78385e79d69bf32d7 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Tue, 18 May 2021 09:51:58 +0300 Subject: [PATCH 09/43] Remove ignore attribute from now fixed test scene --- .../TestSceneCatchPlayerLegacySkin.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/osu.Game.Rulesets.Catch.Tests/TestSceneCatchPlayerLegacySkin.cs b/osu.Game.Rulesets.Catch.Tests/TestSceneCatchPlayerLegacySkin.cs index 4e56e4b4a7..b7cd6737b1 100644 --- a/osu.Game.Rulesets.Catch.Tests/TestSceneCatchPlayerLegacySkin.cs +++ b/osu.Game.Rulesets.Catch.Tests/TestSceneCatchPlayerLegacySkin.cs @@ -20,7 +20,6 @@ namespace osu.Game.Rulesets.Catch.Tests protected override Ruleset CreatePlayerRuleset() => new CatchRuleset(); [Test] - [Ignore("HUD components broken, remove when fixed.")] public void TestLegacyHUDComboCounterHidden([Values] bool withModifiedSkin) { if (withModifiedSkin) @@ -33,7 +32,7 @@ namespace osu.Game.Rulesets.Catch.Tests AddAssert("legacy HUD combo counter hidden", () => { - return Player.ChildrenOfType().All(c => !c.ChildrenOfType().Single().IsPresent); + return Player.ChildrenOfType().All(c => c.ChildrenOfType().Single().Alpha == 0f); }); } } From 076fcec3dff504fbab201aa82094b7f994a50e03 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Tue, 18 May 2021 17:45:15 +0300 Subject: [PATCH 10/43] Revert "Remove ignore attribute from now fixed test scene" This reverts commit 4e12a2734ccf28dae236d6c78385e79d69bf32d7. --- .../TestSceneCatchPlayerLegacySkin.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/osu.Game.Rulesets.Catch.Tests/TestSceneCatchPlayerLegacySkin.cs b/osu.Game.Rulesets.Catch.Tests/TestSceneCatchPlayerLegacySkin.cs index b7cd6737b1..4e56e4b4a7 100644 --- a/osu.Game.Rulesets.Catch.Tests/TestSceneCatchPlayerLegacySkin.cs +++ b/osu.Game.Rulesets.Catch.Tests/TestSceneCatchPlayerLegacySkin.cs @@ -20,6 +20,7 @@ namespace osu.Game.Rulesets.Catch.Tests protected override Ruleset CreatePlayerRuleset() => new CatchRuleset(); [Test] + [Ignore("HUD components broken, remove when fixed.")] public void TestLegacyHUDComboCounterHidden([Values] bool withModifiedSkin) { if (withModifiedSkin) @@ -32,7 +33,7 @@ namespace osu.Game.Rulesets.Catch.Tests AddAssert("legacy HUD combo counter hidden", () => { - return Player.ChildrenOfType().All(c => c.ChildrenOfType().Single().Alpha == 0f); + return Player.ChildrenOfType().All(c => !c.ChildrenOfType().Single().IsPresent); }); } } From e7d2f42149955a0fb623c39809f279b34f4a35a0 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Tue, 18 May 2021 17:46:15 +0300 Subject: [PATCH 11/43] Revert "Merge branch 'fix-legacy-skin-test' into catch-hide-combo-workaround" This reverts commit 380d004683ad658d9ecbc408f7844eaa4c2b43c8, reversing changes made to ff419af5128a3e0f2d188e10fcb05aed712ac128. --- .../TestSceneBeatmapMetadataDisplay.cs | 151 ------------------ osu.Game/Beatmaps/BeatmapDifficultyCache.cs | 4 +- .../Screens/Play/BeatmapMetadataDisplay.cs | 76 ++------- .../Tests/Visual/LegacySkinPlayerTestScene.cs | 27 ---- 4 files changed, 15 insertions(+), 243 deletions(-) delete mode 100644 osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapMetadataDisplay.cs diff --git a/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapMetadataDisplay.cs b/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapMetadataDisplay.cs deleted file mode 100644 index 271fbde5c3..0000000000 --- a/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapMetadataDisplay.cs +++ /dev/null @@ -1,151 +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 System.Threading; -using System.Threading.Tasks; -using NUnit.Framework; -using osu.Framework.Allocation; -using osu.Framework.Bindables; -using osu.Framework.Graphics; -using osu.Framework.Utils; -using osu.Game.Beatmaps; -using osu.Game.Rulesets; -using osu.Game.Rulesets.Mods; -using osu.Game.Screens.Menu; -using osu.Game.Screens.Play; -using osuTK; - -namespace osu.Game.Tests.Visual.SongSelect -{ - public class TestSceneBeatmapMetadataDisplay : OsuTestScene - { - private BeatmapMetadataDisplay display; - - [Resolved] - private BeatmapManager manager { get; set; } - - [Cached(typeof(BeatmapDifficultyCache))] - private readonly TestBeatmapDifficultyCache testDifficultyCache = new TestBeatmapDifficultyCache(); - - [Test] - public void TestLocal([Values("Beatmap", "Some long title and stuff")] - string title, - [Values("Trial", "Some1's very hardest difficulty")] - string version) - { - showMetadataForBeatmap(() => CreateWorkingBeatmap(new Beatmap - { - BeatmapInfo = - { - Metadata = new BeatmapMetadata - { - Title = title, - }, - Version = version, - StarDifficulty = RNG.NextDouble(0, 10), - } - })); - } - - [Test] - public void TestDelayedStarRating() - { - AddStep("block calculation", () => testDifficultyCache.BlockCalculation = true); - - showMetadataForBeatmap(() => CreateWorkingBeatmap(new Beatmap - { - BeatmapInfo = - { - Metadata = new BeatmapMetadata - { - Title = "Heavy beatmap", - }, - Version = "10k objects", - StarDifficulty = 99.99f, - } - })); - - AddStep("allow calculation", () => testDifficultyCache.BlockCalculation = false); - } - - [Test] - public void TestRandomFromDatabase() - { - showMetadataForBeatmap(() => - { - var allBeatmapSets = manager.GetAllUsableBeatmapSets(IncludedDetails.Minimal); - if (allBeatmapSets.Count == 0) - return manager.DefaultBeatmap; - - var randomBeatmapSet = allBeatmapSets[RNG.Next(0, allBeatmapSets.Count - 1)]; - var randomBeatmap = randomBeatmapSet.Beatmaps[RNG.Next(0, randomBeatmapSet.Beatmaps.Count - 1)]; - - return manager.GetWorkingBeatmap(randomBeatmap); - }); - } - - private void showMetadataForBeatmap(Func getBeatmap) - { - AddStep("setup display", () => - { - var randomMods = Ruleset.Value.CreateInstance().GetAllMods().OrderBy(_ => RNG.Next()).Take(5).ToList(); - - OsuLogo logo = new OsuLogo { Scale = new Vector2(0.15f) }; - - Remove(testDifficultyCache); - - Children = new Drawable[] - { - testDifficultyCache, - display = new BeatmapMetadataDisplay(getBeatmap(), new Bindable>(randomMods), logo) - { - Anchor = Anchor.Centre, - Origin = Anchor.Centre, - Alpha = 0f, - } - }; - - display.FadeIn(400, Easing.OutQuint); - }); - - AddWaitStep("wait a bit", 5); - - AddStep("finish loading", () => display.Loading = false); - } - - private class TestBeatmapDifficultyCache : BeatmapDifficultyCache - { - private TaskCompletionSource calculationBlocker; - - private bool blockCalculation; - - public bool BlockCalculation - { - get => blockCalculation; - set - { - if (value == blockCalculation) - return; - - blockCalculation = value; - - if (value) - calculationBlocker = new TaskCompletionSource(); - else - calculationBlocker?.SetResult(false); - } - } - - public override async Task GetDifficultyAsync(BeatmapInfo beatmapInfo, RulesetInfo rulesetInfo = null, IEnumerable mods = null, CancellationToken cancellationToken = default) - { - if (blockCalculation) - await calculationBlocker.Task; - - return await base.GetDifficultyAsync(beatmapInfo, rulesetInfo, mods, cancellationToken); - } - } - } -} diff --git a/osu.Game/Beatmaps/BeatmapDifficultyCache.cs b/osu.Game/Beatmaps/BeatmapDifficultyCache.cs index 6ed623d0c0..53d82c385d 100644 --- a/osu.Game/Beatmaps/BeatmapDifficultyCache.cs +++ b/osu.Game/Beatmaps/BeatmapDifficultyCache.cs @@ -103,8 +103,8 @@ namespace osu.Game.Beatmaps /// The s to get the difficulty with. /// An optional which stops computing the star difficulty. /// The . - public virtual Task GetDifficultyAsync([NotNull] BeatmapInfo beatmapInfo, [CanBeNull] RulesetInfo rulesetInfo = null, - [CanBeNull] IEnumerable mods = null, CancellationToken cancellationToken = default) + public Task GetDifficultyAsync([NotNull] BeatmapInfo beatmapInfo, [CanBeNull] RulesetInfo rulesetInfo = null, [CanBeNull] IEnumerable mods = null, + CancellationToken cancellationToken = default) { // In the case that the user hasn't given us a ruleset, use the beatmap's default ruleset. rulesetInfo ??= beatmapInfo.Ruleset; diff --git a/osu.Game/Screens/Play/BeatmapMetadataDisplay.cs b/osu.Game/Screens/Play/BeatmapMetadataDisplay.cs index fd1150650c..c56344a8fb 100644 --- a/osu.Game/Screens/Play/BeatmapMetadataDisplay.cs +++ b/osu.Game/Screens/Play/BeatmapMetadataDisplay.cs @@ -2,7 +2,6 @@ // See the LICENCE file in the repository root for full licence text. using System.Collections.Generic; -using System.Diagnostics; using osu.Framework.Allocation; using osu.Framework.Bindables; using osu.Framework.Graphics; @@ -15,7 +14,6 @@ using osu.Game.Graphics.Sprites; using osu.Game.Graphics.UserInterface; using osu.Game.Rulesets.Mods; using osu.Game.Screens.Play.HUD; -using osu.Game.Screens.Ranking.Expanded; using osuTK; namespace osu.Game.Screens.Play @@ -27,7 +25,7 @@ namespace osu.Game.Screens.Play { private readonly WorkingBeatmap beatmap; private readonly Bindable> mods; - private readonly Drawable logoFacade; + private readonly Drawable facade; private LoadingSpinner loading; public IBindable> Mods => mods; @@ -43,24 +41,19 @@ namespace osu.Game.Screens.Play } } - public BeatmapMetadataDisplay(WorkingBeatmap beatmap, Bindable> mods, Drawable logoFacade) + public BeatmapMetadataDisplay(WorkingBeatmap beatmap, Bindable> mods, Drawable facade) { this.beatmap = beatmap; - this.logoFacade = logoFacade; + this.facade = facade; this.mods = new Bindable>(); this.mods.BindTo(mods); } - private IBindable starDifficulty; - - private FillFlowContainer versionFlow; - private StarRatingDisplay starRatingDisplay; - [BackgroundDependencyLoader] - private void load(BeatmapDifficultyCache difficultyCache) + private void load() { - var metadata = beatmap.BeatmapInfo.Metadata; + var metadata = beatmap.BeatmapInfo?.Metadata ?? new BeatmapMetadata(); AutoSizeAxes = Axes.Both; Children = new Drawable[] @@ -73,7 +66,7 @@ namespace osu.Game.Screens.Play Direction = FillDirection.Vertical, Children = new[] { - logoFacade.With(d => + facade.With(d => { d.Anchor = Anchor.TopCentre; d.Origin = Anchor.TopCentre; @@ -114,30 +107,16 @@ namespace osu.Game.Screens.Play loading = new LoadingLayer(true) } }, - versionFlow = new FillFlowContainer + new OsuSpriteText { - AutoSizeAxes = Axes.Both, - Anchor = Anchor.TopCentre, + Text = beatmap?.BeatmapInfo?.Version, + Font = OsuFont.GetFont(size: 26, italics: true), Origin = Anchor.TopCentre, - Direction = FillDirection.Vertical, - Spacing = new Vector2(5f), - Margin = new MarginPadding { Bottom = 40 }, - Children = new Drawable[] + Anchor = Anchor.TopCentre, + Margin = new MarginPadding { - new OsuSpriteText - { - Text = beatmap?.BeatmapInfo?.Version, - Font = OsuFont.GetFont(size: 26, italics: true), - Anchor = Anchor.TopCentre, - Origin = Anchor.TopCentre, - }, - starRatingDisplay = new StarRatingDisplay(default) - { - Alpha = 0f, - Anchor = Anchor.TopCentre, - Origin = Anchor.TopCentre, - } - } + Bottom = 40 + }, }, new GridContainer { @@ -180,38 +159,9 @@ namespace osu.Game.Screens.Play } }; - starDifficulty = difficultyCache.GetBindableDifficulty(beatmap.BeatmapInfo); - Loading = true; } - protected override void LoadComplete() - { - base.LoadComplete(); - - if (starDifficulty.Value != null) - { - starRatingDisplay.Current.Value = starDifficulty.Value.Value; - starRatingDisplay.Show(); - } - else - { - starRatingDisplay.Hide(); - - starDifficulty.ValueChanged += d => - { - Debug.Assert(d.NewValue != null); - - starRatingDisplay.Current.Value = d.NewValue.Value; - - versionFlow.AutoSizeDuration = 300; - versionFlow.AutoSizeEasing = Easing.OutQuint; - - starRatingDisplay.FadeIn(300, Easing.InQuint); - }; - } - } - private class MetadataLineLabel : OsuSpriteText { public MetadataLineLabel(string text) diff --git a/osu.Game/Tests/Visual/LegacySkinPlayerTestScene.cs b/osu.Game/Tests/Visual/LegacySkinPlayerTestScene.cs index 25c1ae26f9..907a37f4b9 100644 --- a/osu.Game/Tests/Visual/LegacySkinPlayerTestScene.cs +++ b/osu.Game/Tests/Visual/LegacySkinPlayerTestScene.cs @@ -1,17 +1,11 @@ // 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.Allocation; -using osu.Framework.Audio; using osu.Framework.IO.Stores; -using osu.Framework.Testing; -using osu.Framework.Timing; -using osu.Game.Beatmaps; using osu.Game.Rulesets; using osu.Game.Skinning; -using osu.Game.Storyboards; namespace osu.Game.Tests.Visual { @@ -24,9 +18,6 @@ namespace osu.Game.Tests.Visual protected override TestPlayer CreatePlayer(Ruleset ruleset) => new SkinProvidingPlayer(legacySkinSource); - protected override WorkingBeatmap CreateWorkingBeatmap(IBeatmap beatmap, Storyboard storyboard = null) - => new LegacySkinWorkingBeatmap(beatmap, storyboard, Clock, Audio); - [BackgroundDependencyLoader] private void load(OsuGameBase game, SkinManager skins) { @@ -34,14 +25,6 @@ namespace osu.Game.Tests.Visual legacySkinSource = new SkinProvidingContainer(LegacySkin); } - public override void SetUpSteps() - { - base.SetUpSteps(); - - // check presence of a random legacy HUD component to ensure this is using legacy skin. - AddAssert("using legacy skin", () => this.ChildrenOfType().Any()); - } - public class SkinProvidingPlayer : TestPlayer { [Cached(typeof(ISkinSource))] @@ -52,15 +35,5 @@ namespace osu.Game.Tests.Visual this.skinSource = skinSource; } } - - private class LegacySkinWorkingBeatmap : ClockBackedTestWorkingBeatmap - { - public LegacySkinWorkingBeatmap(IBeatmap beatmap, Storyboard storyboard, IFrameBasedClock frameBasedClock, AudioManager audio) - : base(beatmap, storyboard, frameBasedClock, audio) - { - } - - protected override ISkin GetSkin() => new LegacyBeatmapSkin(BeatmapInfo, null, null); - } } } From 013fe4928fe2177802a99bcd1715d9d31077021e Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Wed, 19 May 2021 08:48:21 +0300 Subject: [PATCH 12/43] Unrevert irrelevant changes --- .../TestSceneBeatmapMetadataDisplay.cs | 151 ++++++++++++++++++ osu.Game/Beatmaps/BeatmapDifficultyCache.cs | 4 +- .../Screens/Play/BeatmapMetadataDisplay.cs | 76 +++++++-- 3 files changed, 216 insertions(+), 15 deletions(-) create mode 100644 osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapMetadataDisplay.cs diff --git a/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapMetadataDisplay.cs b/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapMetadataDisplay.cs new file mode 100644 index 0000000000..271fbde5c3 --- /dev/null +++ b/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapMetadataDisplay.cs @@ -0,0 +1,151 @@ +// 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.Threading; +using System.Threading.Tasks; +using NUnit.Framework; +using osu.Framework.Allocation; +using osu.Framework.Bindables; +using osu.Framework.Graphics; +using osu.Framework.Utils; +using osu.Game.Beatmaps; +using osu.Game.Rulesets; +using osu.Game.Rulesets.Mods; +using osu.Game.Screens.Menu; +using osu.Game.Screens.Play; +using osuTK; + +namespace osu.Game.Tests.Visual.SongSelect +{ + public class TestSceneBeatmapMetadataDisplay : OsuTestScene + { + private BeatmapMetadataDisplay display; + + [Resolved] + private BeatmapManager manager { get; set; } + + [Cached(typeof(BeatmapDifficultyCache))] + private readonly TestBeatmapDifficultyCache testDifficultyCache = new TestBeatmapDifficultyCache(); + + [Test] + public void TestLocal([Values("Beatmap", "Some long title and stuff")] + string title, + [Values("Trial", "Some1's very hardest difficulty")] + string version) + { + showMetadataForBeatmap(() => CreateWorkingBeatmap(new Beatmap + { + BeatmapInfo = + { + Metadata = new BeatmapMetadata + { + Title = title, + }, + Version = version, + StarDifficulty = RNG.NextDouble(0, 10), + } + })); + } + + [Test] + public void TestDelayedStarRating() + { + AddStep("block calculation", () => testDifficultyCache.BlockCalculation = true); + + showMetadataForBeatmap(() => CreateWorkingBeatmap(new Beatmap + { + BeatmapInfo = + { + Metadata = new BeatmapMetadata + { + Title = "Heavy beatmap", + }, + Version = "10k objects", + StarDifficulty = 99.99f, + } + })); + + AddStep("allow calculation", () => testDifficultyCache.BlockCalculation = false); + } + + [Test] + public void TestRandomFromDatabase() + { + showMetadataForBeatmap(() => + { + var allBeatmapSets = manager.GetAllUsableBeatmapSets(IncludedDetails.Minimal); + if (allBeatmapSets.Count == 0) + return manager.DefaultBeatmap; + + var randomBeatmapSet = allBeatmapSets[RNG.Next(0, allBeatmapSets.Count - 1)]; + var randomBeatmap = randomBeatmapSet.Beatmaps[RNG.Next(0, randomBeatmapSet.Beatmaps.Count - 1)]; + + return manager.GetWorkingBeatmap(randomBeatmap); + }); + } + + private void showMetadataForBeatmap(Func getBeatmap) + { + AddStep("setup display", () => + { + var randomMods = Ruleset.Value.CreateInstance().GetAllMods().OrderBy(_ => RNG.Next()).Take(5).ToList(); + + OsuLogo logo = new OsuLogo { Scale = new Vector2(0.15f) }; + + Remove(testDifficultyCache); + + Children = new Drawable[] + { + testDifficultyCache, + display = new BeatmapMetadataDisplay(getBeatmap(), new Bindable>(randomMods), logo) + { + Anchor = Anchor.Centre, + Origin = Anchor.Centre, + Alpha = 0f, + } + }; + + display.FadeIn(400, Easing.OutQuint); + }); + + AddWaitStep("wait a bit", 5); + + AddStep("finish loading", () => display.Loading = false); + } + + private class TestBeatmapDifficultyCache : BeatmapDifficultyCache + { + private TaskCompletionSource calculationBlocker; + + private bool blockCalculation; + + public bool BlockCalculation + { + get => blockCalculation; + set + { + if (value == blockCalculation) + return; + + blockCalculation = value; + + if (value) + calculationBlocker = new TaskCompletionSource(); + else + calculationBlocker?.SetResult(false); + } + } + + public override async Task GetDifficultyAsync(BeatmapInfo beatmapInfo, RulesetInfo rulesetInfo = null, IEnumerable mods = null, CancellationToken cancellationToken = default) + { + if (blockCalculation) + await calculationBlocker.Task; + + return await base.GetDifficultyAsync(beatmapInfo, rulesetInfo, mods, cancellationToken); + } + } + } +} diff --git a/osu.Game/Beatmaps/BeatmapDifficultyCache.cs b/osu.Game/Beatmaps/BeatmapDifficultyCache.cs index 53d82c385d..6ed623d0c0 100644 --- a/osu.Game/Beatmaps/BeatmapDifficultyCache.cs +++ b/osu.Game/Beatmaps/BeatmapDifficultyCache.cs @@ -103,8 +103,8 @@ namespace osu.Game.Beatmaps /// The s to get the difficulty with. /// An optional which stops computing the star difficulty. /// The . - public Task GetDifficultyAsync([NotNull] BeatmapInfo beatmapInfo, [CanBeNull] RulesetInfo rulesetInfo = null, [CanBeNull] IEnumerable mods = null, - CancellationToken cancellationToken = default) + public virtual Task GetDifficultyAsync([NotNull] BeatmapInfo beatmapInfo, [CanBeNull] RulesetInfo rulesetInfo = null, + [CanBeNull] IEnumerable mods = null, CancellationToken cancellationToken = default) { // In the case that the user hasn't given us a ruleset, use the beatmap's default ruleset. rulesetInfo ??= beatmapInfo.Ruleset; diff --git a/osu.Game/Screens/Play/BeatmapMetadataDisplay.cs b/osu.Game/Screens/Play/BeatmapMetadataDisplay.cs index c56344a8fb..fd1150650c 100644 --- a/osu.Game/Screens/Play/BeatmapMetadataDisplay.cs +++ b/osu.Game/Screens/Play/BeatmapMetadataDisplay.cs @@ -2,6 +2,7 @@ // See the LICENCE file in the repository root for full licence text. using System.Collections.Generic; +using System.Diagnostics; using osu.Framework.Allocation; using osu.Framework.Bindables; using osu.Framework.Graphics; @@ -14,6 +15,7 @@ using osu.Game.Graphics.Sprites; using osu.Game.Graphics.UserInterface; using osu.Game.Rulesets.Mods; using osu.Game.Screens.Play.HUD; +using osu.Game.Screens.Ranking.Expanded; using osuTK; namespace osu.Game.Screens.Play @@ -25,7 +27,7 @@ namespace osu.Game.Screens.Play { private readonly WorkingBeatmap beatmap; private readonly Bindable> mods; - private readonly Drawable facade; + private readonly Drawable logoFacade; private LoadingSpinner loading; public IBindable> Mods => mods; @@ -41,19 +43,24 @@ namespace osu.Game.Screens.Play } } - public BeatmapMetadataDisplay(WorkingBeatmap beatmap, Bindable> mods, Drawable facade) + public BeatmapMetadataDisplay(WorkingBeatmap beatmap, Bindable> mods, Drawable logoFacade) { this.beatmap = beatmap; - this.facade = facade; + this.logoFacade = logoFacade; this.mods = new Bindable>(); this.mods.BindTo(mods); } + private IBindable starDifficulty; + + private FillFlowContainer versionFlow; + private StarRatingDisplay starRatingDisplay; + [BackgroundDependencyLoader] - private void load() + private void load(BeatmapDifficultyCache difficultyCache) { - var metadata = beatmap.BeatmapInfo?.Metadata ?? new BeatmapMetadata(); + var metadata = beatmap.BeatmapInfo.Metadata; AutoSizeAxes = Axes.Both; Children = new Drawable[] @@ -66,7 +73,7 @@ namespace osu.Game.Screens.Play Direction = FillDirection.Vertical, Children = new[] { - facade.With(d => + logoFacade.With(d => { d.Anchor = Anchor.TopCentre; d.Origin = Anchor.TopCentre; @@ -107,16 +114,30 @@ namespace osu.Game.Screens.Play loading = new LoadingLayer(true) } }, - new OsuSpriteText + versionFlow = new FillFlowContainer { - Text = beatmap?.BeatmapInfo?.Version, - Font = OsuFont.GetFont(size: 26, italics: true), - Origin = Anchor.TopCentre, + AutoSizeAxes = Axes.Both, Anchor = Anchor.TopCentre, - Margin = new MarginPadding + Origin = Anchor.TopCentre, + Direction = FillDirection.Vertical, + Spacing = new Vector2(5f), + Margin = new MarginPadding { Bottom = 40 }, + Children = new Drawable[] { - Bottom = 40 - }, + new OsuSpriteText + { + Text = beatmap?.BeatmapInfo?.Version, + Font = OsuFont.GetFont(size: 26, italics: true), + Anchor = Anchor.TopCentre, + Origin = Anchor.TopCentre, + }, + starRatingDisplay = new StarRatingDisplay(default) + { + Alpha = 0f, + Anchor = Anchor.TopCentre, + Origin = Anchor.TopCentre, + } + } }, new GridContainer { @@ -159,9 +180,38 @@ namespace osu.Game.Screens.Play } }; + starDifficulty = difficultyCache.GetBindableDifficulty(beatmap.BeatmapInfo); + Loading = true; } + protected override void LoadComplete() + { + base.LoadComplete(); + + if (starDifficulty.Value != null) + { + starRatingDisplay.Current.Value = starDifficulty.Value.Value; + starRatingDisplay.Show(); + } + else + { + starRatingDisplay.Hide(); + + starDifficulty.ValueChanged += d => + { + Debug.Assert(d.NewValue != null); + + starRatingDisplay.Current.Value = d.NewValue.Value; + + versionFlow.AutoSizeDuration = 300; + versionFlow.AutoSizeEasing = Easing.OutQuint; + + starRatingDisplay.FadeIn(300, Easing.InQuint); + }; + } + } + private class MetadataLineLabel : OsuSpriteText { public MetadataLineLabel(string text) From a730349629c5b3f39a0107980a39bbb5161125cf Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Wed, 19 May 2021 23:18:54 +0300 Subject: [PATCH 13/43] Remove the ignore attribute once again --- osu.Game.Rulesets.Catch.Tests/TestSceneCatchPlayerLegacySkin.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/osu.Game.Rulesets.Catch.Tests/TestSceneCatchPlayerLegacySkin.cs b/osu.Game.Rulesets.Catch.Tests/TestSceneCatchPlayerLegacySkin.cs index 4e56e4b4a7..79394c1f19 100644 --- a/osu.Game.Rulesets.Catch.Tests/TestSceneCatchPlayerLegacySkin.cs +++ b/osu.Game.Rulesets.Catch.Tests/TestSceneCatchPlayerLegacySkin.cs @@ -20,7 +20,6 @@ namespace osu.Game.Rulesets.Catch.Tests protected override Ruleset CreatePlayerRuleset() => new CatchRuleset(); [Test] - [Ignore("HUD components broken, remove when fixed.")] public void TestLegacyHUDComboCounterHidden([Values] bool withModifiedSkin) { if (withModifiedSkin) From e20c25b0d95b782126d1d49d5c47ce89ecc1a7f3 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Wed, 19 May 2021 23:32:16 +0300 Subject: [PATCH 14/43] Fix miswritten test assertion --- osu.Game.Rulesets.Catch.Tests/TestSceneCatchPlayerLegacySkin.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Rulesets.Catch.Tests/TestSceneCatchPlayerLegacySkin.cs b/osu.Game.Rulesets.Catch.Tests/TestSceneCatchPlayerLegacySkin.cs index 79394c1f19..b7cd6737b1 100644 --- a/osu.Game.Rulesets.Catch.Tests/TestSceneCatchPlayerLegacySkin.cs +++ b/osu.Game.Rulesets.Catch.Tests/TestSceneCatchPlayerLegacySkin.cs @@ -32,7 +32,7 @@ namespace osu.Game.Rulesets.Catch.Tests AddAssert("legacy HUD combo counter hidden", () => { - return Player.ChildrenOfType().All(c => !c.ChildrenOfType().Single().IsPresent); + return Player.ChildrenOfType().All(c => c.ChildrenOfType().Single().Alpha == 0f); }); } } From a4d52a7f529c1693aace27ed772d2e9805f236a6 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Wed, 19 May 2021 23:37:22 +0300 Subject: [PATCH 15/43] =?UTF-8?q?Use=20switch=E2=80=94case=20instead?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../Legacy/CatchLegacySkinTransformer.cs | 26 ++++++++++++------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/osu.Game.Rulesets.Catch/Skinning/Legacy/CatchLegacySkinTransformer.cs b/osu.Game.Rulesets.Catch/Skinning/Legacy/CatchLegacySkinTransformer.cs index d757f36cde..46a9cc6f70 100644 --- a/osu.Game.Rulesets.Catch/Skinning/Legacy/CatchLegacySkinTransformer.cs +++ b/osu.Game.Rulesets.Catch/Skinning/Legacy/CatchLegacySkinTransformer.cs @@ -24,19 +24,25 @@ namespace osu.Game.Rulesets.Catch.Skinning.Legacy public override Drawable GetDrawableComponent(ISkinComponent component) { - if (component is SkinnableTargetComponent targetComponent && targetComponent.Target == SkinnableTarget.MainHUDComponents) + if (component is SkinnableTargetComponent targetComponent) { - if (!providesComboCounter) - return null; - - if (Source.GetDrawableComponent(component) is SkinnableTargetComponentsContainer components) + switch (targetComponent.Target) { - // catch may provide its own combo counter; hide the default. - // todo: this should be done in an elegant way per ruleset, defining which HUD skin components should be displayed. - foreach (var legacyComboCounter in components.OfType()) - legacyComboCounter.ContentVisible = false; + case SkinnableTarget.MainHUDComponents: + if (!providesComboCounter) + break; - return components; + if (Source.GetDrawableComponent(component) is SkinnableTargetComponentsContainer components) + { + // catch may provide its own combo counter; hide the default. + // todo: this should be done in an elegant way per ruleset, defining which HUD skin components should be displayed. + foreach (var legacyComboCounter in components.OfType()) + legacyComboCounter.ContentVisible = false; + + return components; + } + + break; } return null; From d8efcc0793313d0e09d5f566467805021cb645eb Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Wed, 19 May 2021 23:44:53 +0300 Subject: [PATCH 16/43] Remove drive-by change --- osu.Game/Tests/Visual/LegacySkinPlayerTestScene.cs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/osu.Game/Tests/Visual/LegacySkinPlayerTestScene.cs b/osu.Game/Tests/Visual/LegacySkinPlayerTestScene.cs index 907a37f4b9..c186525757 100644 --- a/osu.Game/Tests/Visual/LegacySkinPlayerTestScene.cs +++ b/osu.Game/Tests/Visual/LegacySkinPlayerTestScene.cs @@ -12,8 +12,6 @@ namespace osu.Game.Tests.Visual [TestFixture] public abstract class LegacySkinPlayerTestScene : PlayerTestScene { - protected LegacySkin LegacySkin { get; private set; } - private ISkinSource legacySkinSource; protected override TestPlayer CreatePlayer(Ruleset ruleset) => new SkinProvidingPlayer(legacySkinSource); @@ -21,8 +19,8 @@ namespace osu.Game.Tests.Visual [BackgroundDependencyLoader] private void load(OsuGameBase game, SkinManager skins) { - LegacySkin = new DefaultLegacySkin(new NamespacedResourceStore(game.Resources, "Skins/Legacy"), skins); - legacySkinSource = new SkinProvidingContainer(LegacySkin); + var legacySkin = new DefaultLegacySkin(new NamespacedResourceStore(game.Resources, "Skins/Legacy"), skins); + legacySkinSource = new SkinProvidingContainer(legacySkin); } public class SkinProvidingPlayer : TestPlayer From 3d99b89633b622b93a59423f72660d2af57eec01 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Thu, 20 May 2021 00:03:10 +0300 Subject: [PATCH 17/43] Add back actually needed change *no comment* --- osu.Game/Tests/Visual/LegacySkinPlayerTestScene.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/osu.Game/Tests/Visual/LegacySkinPlayerTestScene.cs b/osu.Game/Tests/Visual/LegacySkinPlayerTestScene.cs index c186525757..907a37f4b9 100644 --- a/osu.Game/Tests/Visual/LegacySkinPlayerTestScene.cs +++ b/osu.Game/Tests/Visual/LegacySkinPlayerTestScene.cs @@ -12,6 +12,8 @@ namespace osu.Game.Tests.Visual [TestFixture] public abstract class LegacySkinPlayerTestScene : PlayerTestScene { + protected LegacySkin LegacySkin { get; private set; } + private ISkinSource legacySkinSource; protected override TestPlayer CreatePlayer(Ruleset ruleset) => new SkinProvidingPlayer(legacySkinSource); @@ -19,8 +21,8 @@ namespace osu.Game.Tests.Visual [BackgroundDependencyLoader] private void load(OsuGameBase game, SkinManager skins) { - var legacySkin = new DefaultLegacySkin(new NamespacedResourceStore(game.Resources, "Skins/Legacy"), skins); - legacySkinSource = new SkinProvidingContainer(legacySkin); + LegacySkin = new DefaultLegacySkin(new NamespacedResourceStore(game.Resources, "Skins/Legacy"), skins); + legacySkinSource = new SkinProvidingContainer(LegacySkin); } public class SkinProvidingPlayer : TestPlayer From b349ff8693096103d14a94411b8f25cb647044d6 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 28 May 2021 14:33:04 +0900 Subject: [PATCH 18/43] Revert "Add temporary accounting for tests with null files" This reverts commit e52c0a34f82a6b93c740b4c2d3d068bc3c52196a. --- osu.Game/Beatmaps/BeatmapManager.cs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/osu.Game/Beatmaps/BeatmapManager.cs b/osu.Game/Beatmaps/BeatmapManager.cs index 2e2bc96c21..f70d29861f 100644 --- a/osu.Game/Beatmaps/BeatmapManager.cs +++ b/osu.Game/Beatmaps/BeatmapManager.cs @@ -293,11 +293,7 @@ namespace osu.Game.Beatmaps if (beatmapInfo?.BeatmapSet == null || beatmapInfo == DefaultBeatmap?.BeatmapInfo) return DefaultBeatmap; - // files may be null in some tests. - if (beatmapInfo.BeatmapSet?.Files == null) - return DefaultBeatmap; - - if (beatmapInfo.BeatmapSet.Files.Count == 0) + if (beatmapInfo.BeatmapSet.Files == null) { var info = beatmapInfo; beatmapInfo = QueryBeatmap(b => b.ID == info.ID); From 41733af0ed8eefd3e7206115680ad45588d2e652 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 28 May 2021 14:33:05 +0900 Subject: [PATCH 19/43] Revert "Revert "Make `BeatmapSetInfo.Files` non-nullable"" This reverts commit 9c4f39e9681644d84b60828ee8ce613b6d81a89f. --- osu.Game/Beatmaps/BeatmapManager.cs | 22 ++++++++++------------ osu.Game/Beatmaps/BeatmapSetInfo.cs | 10 ++++++---- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/osu.Game/Beatmaps/BeatmapManager.cs b/osu.Game/Beatmaps/BeatmapManager.cs index f70d29861f..f459657a0e 100644 --- a/osu.Game/Beatmaps/BeatmapManager.cs +++ b/osu.Game/Beatmaps/BeatmapManager.cs @@ -3,7 +3,6 @@ using System; using System.Collections.Generic; -using System.Diagnostics; using System.IO; using System.Linq; using System.Linq.Expressions; @@ -245,8 +244,6 @@ namespace osu.Game.Beatmaps { var setInfo = info.BeatmapSet; - Debug.Assert(setInfo.Files != null); - using (var stream = new MemoryStream()) { using (var sw = new StreamWriter(stream, Encoding.UTF8, 1024, true)) @@ -287,20 +284,21 @@ namespace osu.Game.Beatmaps /// A instance correlating to the provided . public WorkingBeatmap GetWorkingBeatmap(BeatmapInfo beatmapInfo, WorkingBeatmap previous = null) { - if (beatmapInfo?.ID > 0 && previous != null && previous.BeatmapInfo?.ID == beatmapInfo.ID) - return previous; - - if (beatmapInfo?.BeatmapSet == null || beatmapInfo == DefaultBeatmap?.BeatmapInfo) + if (beatmapInfo?.BeatmapSet == null) return DefaultBeatmap; - if (beatmapInfo.BeatmapSet.Files == null) + if (beatmapInfo == DefaultBeatmap?.BeatmapInfo) + return DefaultBeatmap; + + // if there are no files, presume the full beatmap info has not yet been fetched from the database. + if (beatmapInfo.BeatmapSet.Files.Count == 0) { - var info = beatmapInfo; - beatmapInfo = QueryBeatmap(b => b.ID == info.ID); + int lookupId = beatmapInfo.ID; + beatmapInfo = QueryBeatmap(b => b.ID == lookupId); } - if (beatmapInfo == null) - return DefaultBeatmap; + if (beatmapInfo.ID > 0 && previous != null && previous.BeatmapInfo?.ID == beatmapInfo.ID) + return previous; lock (workingCache) { diff --git a/osu.Game/Beatmaps/BeatmapSetInfo.cs b/osu.Game/Beatmaps/BeatmapSetInfo.cs index 1ce42535a0..3b1ff4ced0 100644 --- a/osu.Game/Beatmaps/BeatmapSetInfo.cs +++ b/osu.Game/Beatmaps/BeatmapSetInfo.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Generic; using System.ComponentModel.DataAnnotations.Schema; using System.Linq; +using JetBrains.Annotations; using osu.Framework.Testing; using osu.Game.Database; @@ -31,6 +32,9 @@ namespace osu.Game.Beatmaps public List Beatmaps { get; set; } + [NotNull] + public List Files { get; set; } = new List(); + [NotMapped] public BeatmapSetOnlineInfo OnlineInfo { get; set; } @@ -57,16 +61,14 @@ namespace osu.Game.Beatmaps public string Hash { get; set; } - public string StoryboardFile => Files?.Find(f => f.Filename.EndsWith(".osb", StringComparison.OrdinalIgnoreCase))?.Filename; + public string StoryboardFile => Files.Find(f => f.Filename.EndsWith(".osb", StringComparison.OrdinalIgnoreCase))?.Filename; /// /// Returns the storage path for the file in this beatmapset with the given filename, if any exists, otherwise null. /// The path returned is relative to the user file storage. /// /// The name of the file to get the storage path of. - public string GetPathForFile(string filename) => Files?.SingleOrDefault(f => string.Equals(f.Filename, filename, StringComparison.OrdinalIgnoreCase))?.FileInfo.StoragePath; - - public List Files { get; set; } + public string GetPathForFile(string filename) => Files.SingleOrDefault(f => string.Equals(f.Filename, filename, StringComparison.OrdinalIgnoreCase))?.FileInfo.StoragePath; public override string ToString() => Metadata?.ToString() ?? base.ToString(); From 581a86b91a231e5ed12d49bc1267d840fb58232b Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 28 May 2021 14:33:06 +0900 Subject: [PATCH 20/43] Revert "Revert "Fix editor tests failing due to empty files being specified"" This reverts commit 1af684c4b2cb41e45f473e90285f5ac5cc192d12. --- .../Editing/TestSceneEditorBeatmapCreation.cs | 2 + osu.Game/Beatmaps/BeatmapManager.cs | 4 +- osu.Game/Tests/Beatmaps/TestBeatmap.cs | 1 - osu.Game/Tests/Visual/EditorTestScene.cs | 43 ++++++++++++++++++- 4 files changed, 45 insertions(+), 5 deletions(-) diff --git a/osu.Game.Tests/Visual/Editing/TestSceneEditorBeatmapCreation.cs b/osu.Game.Tests/Visual/Editing/TestSceneEditorBeatmapCreation.cs index 7584c74c71..dd5e01adbb 100644 --- a/osu.Game.Tests/Visual/Editing/TestSceneEditorBeatmapCreation.cs +++ b/osu.Game.Tests/Visual/Editing/TestSceneEditorBeatmapCreation.cs @@ -24,6 +24,8 @@ namespace osu.Game.Tests.Visual.Editing protected override bool EditorComponentsReady => Editor.ChildrenOfType().SingleOrDefault()?.IsLoaded == true; + protected override bool IsolateSavingFromDatabase => false; + [Resolved] private BeatmapManager beatmapManager { get; set; } diff --git a/osu.Game/Beatmaps/BeatmapManager.cs b/osu.Game/Beatmaps/BeatmapManager.cs index f459657a0e..18fbd1f5c2 100644 --- a/osu.Game/Beatmaps/BeatmapManager.cs +++ b/osu.Game/Beatmaps/BeatmapManager.cs @@ -240,7 +240,7 @@ namespace osu.Game.Beatmaps /// The to save the content against. The file referenced by will be replaced. /// The content to write. /// The beatmap content to write, null if to be omitted. - public void Save(BeatmapInfo info, IBeatmap beatmapContent, ISkin beatmapSkin = null) + public virtual void Save(BeatmapInfo info, IBeatmap beatmapContent, ISkin beatmapSkin = null) { var setInfo = info.BeatmapSet; @@ -282,7 +282,7 @@ namespace osu.Game.Beatmaps /// The beatmap to lookup. /// The currently loaded . Allows for optimisation where elements are shared with the new beatmap. May be returned if beatmapInfo requested matches /// A instance correlating to the provided . - public WorkingBeatmap GetWorkingBeatmap(BeatmapInfo beatmapInfo, WorkingBeatmap previous = null) + public virtual WorkingBeatmap GetWorkingBeatmap(BeatmapInfo beatmapInfo, WorkingBeatmap previous = null) { if (beatmapInfo?.BeatmapSet == null) return DefaultBeatmap; diff --git a/osu.Game/Tests/Beatmaps/TestBeatmap.cs b/osu.Game/Tests/Beatmaps/TestBeatmap.cs index fa6dc5647d..2717146c99 100644 --- a/osu.Game/Tests/Beatmaps/TestBeatmap.cs +++ b/osu.Game/Tests/Beatmaps/TestBeatmap.cs @@ -29,7 +29,6 @@ namespace osu.Game.Tests.Beatmaps BeatmapInfo.Ruleset = ruleset; BeatmapInfo.RulesetID = ruleset.ID ?? 0; BeatmapInfo.BeatmapSet.Metadata = BeatmapInfo.Metadata; - BeatmapInfo.BeatmapSet.Files = new List(); BeatmapInfo.BeatmapSet.Beatmaps = new List { BeatmapInfo }; BeatmapInfo.Length = 75000; BeatmapInfo.OnlineInfo = new BeatmapOnlineInfo(); diff --git a/osu.Game/Tests/Visual/EditorTestScene.cs b/osu.Game/Tests/Visual/EditorTestScene.cs index a9ee8e2668..de7fdd0cf8 100644 --- a/osu.Game/Tests/Visual/EditorTestScene.cs +++ b/osu.Game/Tests/Visual/EditorTestScene.cs @@ -4,11 +4,18 @@ using System.Linq; using JetBrains.Annotations; using osu.Framework.Allocation; +using osu.Framework.Audio; +using osu.Framework.Platform; using osu.Framework.Testing; +using osu.Game.Beatmaps; +using osu.Game.Database; +using osu.Game.IO.Archives; +using osu.Game.Online.API; using osu.Game.Rulesets; using osu.Game.Rulesets.Edit; using osu.Game.Screens.Edit; using osu.Game.Screens.Edit.Compose.Components.Timeline; +using osu.Game.Skinning; namespace osu.Game.Tests.Visual { @@ -20,10 +27,20 @@ namespace osu.Game.Tests.Visual protected EditorClock EditorClock { get; private set; } + /// + /// Whether any saves performed by the editor should be isolate (and not persist) to the underlying . + /// + protected virtual bool IsolateSavingFromDatabase => true; + [BackgroundDependencyLoader] - private void load() + private void load(GameHost host, AudioManager audio, RulesetStore rulesets) { - Beatmap.Value = CreateWorkingBeatmap(Ruleset.Value); + var working = CreateWorkingBeatmap(Ruleset.Value); + + Beatmap.Value = working; + + if (IsolateSavingFromDatabase) + Dependencies.CacheAs(new TestBeatmapManager(LocalStorage, ContextFactory, rulesets, null, audio, host, Beatmap.Default, working)); } protected virtual bool EditorComponentsReady => Editor.ChildrenOfType().FirstOrDefault()?.IsLoaded == true @@ -65,5 +82,27 @@ namespace osu.Game.Tests.Visual public new bool HasUnsavedChanges => base.HasUnsavedChanges; } + + private class TestBeatmapManager : BeatmapManager + { + private readonly WorkingBeatmap testBeatmap; + + public TestBeatmapManager(Storage storage, IDatabaseContextFactory contextFactory, RulesetStore rulesets, IAPIProvider api, [NotNull] AudioManager audioManager, GameHost host, WorkingBeatmap defaultBeatmap, WorkingBeatmap testBeatmap) + : base(storage, contextFactory, rulesets, api, audioManager, host, defaultBeatmap, false) + { + this.testBeatmap = testBeatmap; + } + + protected override string ComputeHash(BeatmapSetInfo item, ArchiveReader reader = null) + => string.Empty; + + public override WorkingBeatmap GetWorkingBeatmap(BeatmapInfo beatmapInfo, WorkingBeatmap previous = null) + => testBeatmap; + + public override void Save(BeatmapInfo info, IBeatmap beatmapContent, ISkin beatmapSkin = null) + { + // don't actually care about saving for this context. + } + } } } From 631d217f788a7d6808acd4337bdf018d3320539e Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 28 May 2021 15:42:35 +0900 Subject: [PATCH 21/43] Remove no longer necessary conditional access --- osu.Game/Storyboards/Drawables/DrawableStoryboardVideo.cs | 2 +- osu.Game/Storyboards/Storyboard.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game/Storyboards/Drawables/DrawableStoryboardVideo.cs b/osu.Game/Storyboards/Drawables/DrawableStoryboardVideo.cs index 4ea582ca4a..d746ff5ae5 100644 --- a/osu.Game/Storyboards/Drawables/DrawableStoryboardVideo.cs +++ b/osu.Game/Storyboards/Drawables/DrawableStoryboardVideo.cs @@ -29,7 +29,7 @@ namespace osu.Game.Storyboards.Drawables [BackgroundDependencyLoader(true)] private void load(IBindable beatmap, TextureStore textureStore) { - var path = beatmap.Value.BeatmapSetInfo?.Files?.Find(f => f.Filename.Equals(Video.Path, StringComparison.OrdinalIgnoreCase))?.FileInfo.StoragePath; + var path = beatmap.Value.BeatmapSetInfo?.Files.Find(f => f.Filename.Equals(Video.Path, StringComparison.OrdinalIgnoreCase))?.FileInfo.StoragePath; if (path == null) return; diff --git a/osu.Game/Storyboards/Storyboard.cs b/osu.Game/Storyboards/Storyboard.cs index 3486c1d66a..38e0e4e38c 100644 --- a/osu.Game/Storyboards/Storyboard.cs +++ b/osu.Game/Storyboards/Storyboard.cs @@ -91,7 +91,7 @@ namespace osu.Game.Storyboards public Drawable CreateSpriteFromResourcePath(string path, TextureStore textureStore) { Drawable drawable = null; - var storyboardPath = BeatmapInfo.BeatmapSet?.Files?.Find(f => f.Filename.Equals(path, StringComparison.OrdinalIgnoreCase))?.FileInfo.StoragePath; + var storyboardPath = BeatmapInfo.BeatmapSet?.Files.Find(f => f.Filename.Equals(path, StringComparison.OrdinalIgnoreCase))?.FileInfo.StoragePath; if (storyboardPath != null) drawable = new Sprite { Texture = textureStore.Get(storyboardPath) }; From 60b781701fb03854a19c6f3b9f7ebaac5249234b Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Sun, 30 May 2021 14:20:03 +0300 Subject: [PATCH 22/43] Rewrite catch combo counter hide logic --- .../Skinning/Legacy/CatchLegacySkinTransformer.cs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/osu.Game.Rulesets.Catch/Skinning/Legacy/CatchLegacySkinTransformer.cs b/osu.Game.Rulesets.Catch/Skinning/Legacy/CatchLegacySkinTransformer.cs index 05580f6d06..802de55b7b 100644 --- a/osu.Game.Rulesets.Catch/Skinning/Legacy/CatchLegacySkinTransformer.cs +++ b/osu.Game.Rulesets.Catch/Skinning/Legacy/CatchLegacySkinTransformer.cs @@ -29,20 +29,17 @@ namespace osu.Game.Rulesets.Catch.Skinning.Legacy switch (targetComponent.Target) { case SkinnableTarget.MainHUDComponents: - if (!providesComboCounter) - break; + var components = Source.GetDrawableComponent(component) as SkinnableTargetComponentsContainer; - if (Source.GetDrawableComponent(component) is SkinnableTargetComponentsContainer components) + if (providesComboCounter && components != null) { // catch may provide its own combo counter; hide the default. // todo: this should be done in an elegant way per ruleset, defining which HUD skin components should be displayed. foreach (var legacyComboCounter in components.OfType()) legacyComboCounter.ContentVisible = false; - - return components; } - break; + return components; } } From d12e93bfc67598007192899b440726396b8e202e Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Mon, 31 May 2021 00:02:55 +0300 Subject: [PATCH 23/43] Add skin traget resetting on setup/teardown steps --- .../Tests/Visual/LegacySkinPlayerTestScene.cs | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/osu.Game/Tests/Visual/LegacySkinPlayerTestScene.cs b/osu.Game/Tests/Visual/LegacySkinPlayerTestScene.cs index 907a37f4b9..f78d05c6e5 100644 --- a/osu.Game/Tests/Visual/LegacySkinPlayerTestScene.cs +++ b/osu.Game/Tests/Visual/LegacySkinPlayerTestScene.cs @@ -3,7 +3,9 @@ using NUnit.Framework; using osu.Framework.Allocation; +using osu.Framework.Extensions.IEnumerableExtensions; using osu.Framework.IO.Stores; +using osu.Framework.Testing; using osu.Game.Rulesets; using osu.Game.Skinning; @@ -25,6 +27,29 @@ namespace osu.Game.Tests.Visual legacySkinSource = new SkinProvidingContainer(LegacySkin); } + [SetUpSteps] + public override void SetUpSteps() + { + base.SetUpSteps(); + addResetTargetsStep(); + } + + [TearDownSteps] + public override void TearDownSteps() + { + addResetTargetsStep(); + base.TearDownSteps(); + } + + private void addResetTargetsStep() + { + AddStep("reset targets", () => this.ChildrenOfType().ForEach(t => + { + LegacySkin.ResetDrawableTarget(t); + t.Reload(); + })); + } + public class SkinProvidingPlayer : TestPlayer { [Cached(typeof(ISkinSource))] From 3fbd4e276d632b2763858a8efa031a0d43b57d71 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Mon, 31 May 2021 00:07:29 +0300 Subject: [PATCH 24/43] Add simple xmldoc --- osu.Game/Screens/Play/HUD/LegacyComboCounter.cs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/osu.Game/Screens/Play/HUD/LegacyComboCounter.cs b/osu.Game/Screens/Play/HUD/LegacyComboCounter.cs index ee00c71b0f..112b36616c 100644 --- a/osu.Game/Screens/Play/HUD/LegacyComboCounter.cs +++ b/osu.Game/Screens/Play/HUD/LegacyComboCounter.cs @@ -47,6 +47,13 @@ namespace osu.Game.Screens.Play.HUD private readonly Container counterContainer; + /// + /// Changes the visibility state of the combo counter internally without affecting its . + /// + /// + /// This is temporarily done for rulesets that provide their own combo counter and don't want the HUD one to be visible, + /// without potentially affecting the user's selected skin. + /// public bool ContentVisible { set => counterContainer.Alpha = value ? 1 : 0; From fb111e23d86b02c5c8cc42c843fa706b5fe67481 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Mon, 31 May 2021 07:24:59 +0300 Subject: [PATCH 25/43] Remove "temporarily" --- osu.Game/Screens/Play/HUD/LegacyComboCounter.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Screens/Play/HUD/LegacyComboCounter.cs b/osu.Game/Screens/Play/HUD/LegacyComboCounter.cs index 112b36616c..6c2752569d 100644 --- a/osu.Game/Screens/Play/HUD/LegacyComboCounter.cs +++ b/osu.Game/Screens/Play/HUD/LegacyComboCounter.cs @@ -51,7 +51,7 @@ namespace osu.Game.Screens.Play.HUD /// Changes the visibility state of the combo counter internally without affecting its . /// /// - /// This is temporarily done for rulesets that provide their own combo counter and don't want the HUD one to be visible, + /// This is used for rulesets that provide their own combo counter and don't want the HUD one to be visible, /// without potentially affecting the user's selected skin. /// public bool ContentVisible From 06bd696cc21eb4e20667875093f790468cc474e6 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 31 May 2021 14:11:57 +0900 Subject: [PATCH 26/43] Remove `previous` consumption logic in `GetWorkingBeatmap` This should not be required since the introduction of `workingCache`, which does the same thing in a more global way. --- osu.Game/Beatmaps/BeatmapManager.cs | 6 +----- osu.Game/Overlays/MusicController.cs | 4 ++-- osu.Game/Screens/Select/SongSelect.cs | 7 ++++--- osu.Game/Tests/Visual/EditorTestScene.cs | 2 +- 4 files changed, 8 insertions(+), 11 deletions(-) diff --git a/osu.Game/Beatmaps/BeatmapManager.cs b/osu.Game/Beatmaps/BeatmapManager.cs index b2cd0c9392..5a26f5b66c 100644 --- a/osu.Game/Beatmaps/BeatmapManager.cs +++ b/osu.Game/Beatmaps/BeatmapManager.cs @@ -280,9 +280,8 @@ namespace osu.Game.Beatmaps /// Retrieve a instance for the provided /// /// The beatmap to lookup. - /// The currently loaded . Allows for optimisation where elements are shared with the new beatmap. May be returned if beatmapInfo requested matches /// A instance correlating to the provided . - public virtual WorkingBeatmap GetWorkingBeatmap(BeatmapInfo beatmapInfo, WorkingBeatmap previous = null) + public virtual WorkingBeatmap GetWorkingBeatmap(BeatmapInfo beatmapInfo) { if (beatmapInfo?.BeatmapSet == null) return DefaultBeatmap; @@ -294,9 +293,6 @@ namespace osu.Game.Beatmaps beatmapInfo = QueryBeatmap(b => b.ID == lookupId); } - if (beatmapInfo.ID > 0 && previous != null && previous.BeatmapInfo?.ID == beatmapInfo.ID) - return previous; - lock (workingCache) { var working = workingCache.FirstOrDefault(w => w.BeatmapInfo?.ID == beatmapInfo.ID); diff --git a/osu.Game/Overlays/MusicController.cs b/osu.Game/Overlays/MusicController.cs index 3a9a6261ba..a15f80ca21 100644 --- a/osu.Game/Overlays/MusicController.cs +++ b/osu.Game/Overlays/MusicController.cs @@ -252,7 +252,7 @@ namespace osu.Game.Overlays if (playable != null) { - changeBeatmap(beatmaps.GetWorkingBeatmap(playable.Beatmaps.First(), beatmap.Value)); + changeBeatmap(beatmaps.GetWorkingBeatmap(playable.Beatmaps.First())); restartTrack(); return PreviousTrackResult.Previous; } @@ -283,7 +283,7 @@ namespace osu.Game.Overlays if (playable != null) { - changeBeatmap(beatmaps.GetWorkingBeatmap(playable.Beatmaps.First(), beatmap.Value)); + changeBeatmap(beatmaps.GetWorkingBeatmap(playable.Beatmaps.First())); restartTrack(); return true; } diff --git a/osu.Game/Screens/Select/SongSelect.cs b/osu.Game/Screens/Select/SongSelect.cs index 74e10037ab..270addc8e6 100644 --- a/osu.Game/Screens/Select/SongSelect.cs +++ b/osu.Game/Screens/Select/SongSelect.cs @@ -505,12 +505,13 @@ namespace osu.Game.Screens.Select { Logger.Log($"beatmap changed from \"{Beatmap.Value.BeatmapInfo}\" to \"{beatmap}\""); - WorkingBeatmap previous = Beatmap.Value; - Beatmap.Value = beatmaps.GetWorkingBeatmap(beatmap, previous); + int? lastSetID = Beatmap.Value?.BeatmapInfo.BeatmapSetInfoID; + + Beatmap.Value = beatmaps.GetWorkingBeatmap(beatmap); if (beatmap != null) { - if (beatmap.BeatmapSetInfoID == previous?.BeatmapInfo.BeatmapSetInfoID) + if (beatmap.BeatmapSetInfoID == lastSetID) sampleChangeDifficulty.Play(); else sampleChangeBeatmap.Play(); diff --git a/osu.Game/Tests/Visual/EditorTestScene.cs b/osu.Game/Tests/Visual/EditorTestScene.cs index de7fdd0cf8..ca331088ce 100644 --- a/osu.Game/Tests/Visual/EditorTestScene.cs +++ b/osu.Game/Tests/Visual/EditorTestScene.cs @@ -96,7 +96,7 @@ namespace osu.Game.Tests.Visual protected override string ComputeHash(BeatmapSetInfo item, ArchiveReader reader = null) => string.Empty; - public override WorkingBeatmap GetWorkingBeatmap(BeatmapInfo beatmapInfo, WorkingBeatmap previous = null) + public override WorkingBeatmap GetWorkingBeatmap(BeatmapInfo beatmapInfo) => testBeatmap; public override void Save(BeatmapInfo info, IBeatmap beatmapContent, ISkin beatmapSkin = null) From 5925beaf21ffbebddee2239cc51110491ac07cda Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 31 May 2021 14:24:46 +0900 Subject: [PATCH 27/43] Fix bindable lease failure in editor beatmap creation tests --- .../Visual/Editing/TestSceneEditorBeatmapCreation.cs | 8 ++++++-- osu.Game/Tests/Visual/EditorTestScene.cs | 7 ++++++- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/osu.Game.Tests/Visual/Editing/TestSceneEditorBeatmapCreation.cs b/osu.Game.Tests/Visual/Editing/TestSceneEditorBeatmapCreation.cs index 7584c74c71..49bc01aecc 100644 --- a/osu.Game.Tests/Visual/Editing/TestSceneEditorBeatmapCreation.cs +++ b/osu.Game.Tests/Visual/Editing/TestSceneEditorBeatmapCreation.cs @@ -29,8 +29,6 @@ namespace osu.Game.Tests.Visual.Editing public override void SetUpSteps() { - AddStep("set dummy", () => Beatmap.Value = new DummyWorkingBeatmap(Audio, null)); - base.SetUpSteps(); // if we save a beatmap with a hash collision, things fall over. @@ -38,6 +36,12 @@ namespace osu.Game.Tests.Visual.Editing AddStep("make new beatmap unique", () => EditorBeatmap.Metadata.Title = Guid.NewGuid().ToString()); } + protected override void LoadEditor() + { + Beatmap.Value = new DummyWorkingBeatmap(Audio, null); + base.LoadEditor(); + } + [Test] public void TestCreateNewBeatmap() { diff --git a/osu.Game/Tests/Visual/EditorTestScene.cs b/osu.Game/Tests/Visual/EditorTestScene.cs index a9ee8e2668..80e1fdcce5 100644 --- a/osu.Game/Tests/Visual/EditorTestScene.cs +++ b/osu.Game/Tests/Visual/EditorTestScene.cs @@ -33,12 +33,17 @@ namespace osu.Game.Tests.Visual { base.SetUpSteps(); - AddStep("load editor", () => LoadScreen(Editor = CreateEditor())); + AddStep("load editor", LoadEditor); AddUntilStep("wait for editor to load", () => EditorComponentsReady); AddStep("get beatmap", () => EditorBeatmap = Editor.ChildrenOfType().Single()); AddStep("get clock", () => EditorClock = Editor.ChildrenOfType().Single()); } + protected virtual void LoadEditor() + { + LoadScreen(Editor = CreateEditor()); + } + /// /// Creates the ruleset for providing a corresponding beatmap to load the editor on. /// From 4e186b0cf55360f9b5f9442c49844e42b0cd50e0 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Mon, 31 May 2021 09:24:26 +0300 Subject: [PATCH 28/43] `ContentVisible` -> `HiddenByRulesetImplementation` --- .../Skinning/Legacy/CatchLegacySkinTransformer.cs | 2 +- osu.Game/Screens/Play/HUD/LegacyComboCounter.cs | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/osu.Game.Rulesets.Catch/Skinning/Legacy/CatchLegacySkinTransformer.cs b/osu.Game.Rulesets.Catch/Skinning/Legacy/CatchLegacySkinTransformer.cs index 802de55b7b..8c9e602cd4 100644 --- a/osu.Game.Rulesets.Catch/Skinning/Legacy/CatchLegacySkinTransformer.cs +++ b/osu.Game.Rulesets.Catch/Skinning/Legacy/CatchLegacySkinTransformer.cs @@ -36,7 +36,7 @@ namespace osu.Game.Rulesets.Catch.Skinning.Legacy // catch may provide its own combo counter; hide the default. // todo: this should be done in an elegant way per ruleset, defining which HUD skin components should be displayed. foreach (var legacyComboCounter in components.OfType()) - legacyComboCounter.ContentVisible = false; + legacyComboCounter.HiddenByRulesetImplementation = false; } return components; diff --git a/osu.Game/Screens/Play/HUD/LegacyComboCounter.cs b/osu.Game/Screens/Play/HUD/LegacyComboCounter.cs index 6c2752569d..1737634e31 100644 --- a/osu.Game/Screens/Play/HUD/LegacyComboCounter.cs +++ b/osu.Game/Screens/Play/HUD/LegacyComboCounter.cs @@ -48,13 +48,13 @@ namespace osu.Game.Screens.Play.HUD private readonly Container counterContainer; /// - /// Changes the visibility state of the combo counter internally without affecting its . + /// Hides the combo counter internally without affecting its . /// /// - /// This is used for rulesets that provide their own combo counter and don't want the HUD one to be visible, + /// This is used for rulesets that provide their own combo counter and don't want this HUD one to be visible, /// without potentially affecting the user's selected skin. /// - public bool ContentVisible + public bool HiddenByRulesetImplementation { set => counterContainer.Alpha = value ? 1 : 0; } From 675fe3744692a8a525c8e2769051bbd5f3561fcd Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 31 May 2021 18:24:42 +0900 Subject: [PATCH 29/43] Change check order around to ensure re-fetches which return no results don't nullref --- osu.Game/Beatmaps/BeatmapManager.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/osu.Game/Beatmaps/BeatmapManager.cs b/osu.Game/Beatmaps/BeatmapManager.cs index 5a26f5b66c..518ef2cc19 100644 --- a/osu.Game/Beatmaps/BeatmapManager.cs +++ b/osu.Game/Beatmaps/BeatmapManager.cs @@ -283,16 +283,16 @@ namespace osu.Game.Beatmaps /// A instance correlating to the provided . public virtual WorkingBeatmap GetWorkingBeatmap(BeatmapInfo beatmapInfo) { - if (beatmapInfo?.BeatmapSet == null) - return DefaultBeatmap; - // if there are no files, presume the full beatmap info has not yet been fetched from the database. - if (beatmapInfo.BeatmapSet.Files.Count == 0) + if (beatmapInfo?.BeatmapSet?.Files.Count == 0) { int lookupId = beatmapInfo.ID; beatmapInfo = QueryBeatmap(b => b.ID == lookupId); } + if (beatmapInfo?.BeatmapSet == null) + return DefaultBeatmap; + lock (workingCache) { var working = workingCache.FirstOrDefault(w => w.BeatmapInfo?.ID == beatmapInfo.ID); From b16d10bd95af2d67c37adeeacfe6ca527abe52bf Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 31 May 2021 18:37:32 +0900 Subject: [PATCH 30/43] Provide game-wide resources via `IStorageResourceProvider` --- .../TestSceneManiaHitObjectSamples.cs | 2 +- .../TestSceneTaikoHitObjectSamples.cs | 2 +- osu.Game.Tests/Gameplay/TestSceneHitObjectSamples.cs | 2 +- osu.Game.Tests/Gameplay/TestSceneStoryboardSamples.cs | 1 + .../TestSceneOnlinePlayBeatmapAvailabilityTracker.cs | 7 ++++--- .../Visual/Background/TestSceneUserDimBackgrounds.cs | 2 +- .../Collections/TestSceneManageCollectionsDialog.cs | 2 +- .../Visual/Multiplayer/TestSceneDrawableRoomPlaylist.cs | 2 +- osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayer.cs | 2 +- .../Multiplayer/TestSceneMultiplayerMatchSongSelect.cs | 2 +- .../Multiplayer/TestSceneMultiplayerMatchSubScreen.cs | 2 +- .../Visual/Multiplayer/TestSceneMultiplayerReadyButton.cs | 2 +- .../Multiplayer/TestSceneMultiplayerSpectateButton.cs | 2 +- .../Visual/Multiplayer/TestScenePlaylistsSongSelect.cs | 2 +- .../Visual/Playlists/TestScenePlaylistsRoomSubScreen.cs | 2 +- .../Visual/SongSelect/TestSceneFilterControl.cs | 2 +- .../Visual/SongSelect/TestScenePlaySongSelect.cs | 2 +- .../Visual/UserInterface/TestSceneDeleteLocalScore.cs | 2 +- osu.Game/Beatmaps/BeatmapManager.cs | 5 ++++- osu.Game/IO/IStorageResourceProvider.cs | 5 +++++ osu.Game/OsuGameBase.cs | 4 ++-- osu.Game/Skinning/SkinManager.cs | 8 ++++++-- osu.Game/Tests/Beatmaps/HitObjectSampleTest.cs | 4 +++- osu.Game/Tests/Visual/OsuTestScene.cs | 5 +++++ osu.Game/Tests/Visual/SkinnableTestScene.cs | 1 + 25 files changed, 47 insertions(+), 25 deletions(-) diff --git a/osu.Game.Rulesets.Mania.Tests/TestSceneManiaHitObjectSamples.cs b/osu.Game.Rulesets.Mania.Tests/TestSceneManiaHitObjectSamples.cs index 0d726e1a50..eedabd1adb 100644 --- a/osu.Game.Rulesets.Mania.Tests/TestSceneManiaHitObjectSamples.cs +++ b/osu.Game.Rulesets.Mania.Tests/TestSceneManiaHitObjectSamples.cs @@ -11,7 +11,7 @@ namespace osu.Game.Rulesets.Mania.Tests public class TestSceneManiaHitObjectSamples : HitObjectSampleTest { protected override Ruleset CreatePlayerRuleset() => new ManiaRuleset(); - protected override IResourceStore Resources => new DllResourceStore(Assembly.GetAssembly(typeof(TestSceneManiaHitObjectSamples))); + public override IResourceStore Resources => new DllResourceStore(Assembly.GetAssembly(typeof(TestSceneManiaHitObjectSamples))); /// /// Tests that when a normal sample bank is used, the normal hitsound will be looked up. diff --git a/osu.Game.Rulesets.Taiko.Tests/TestSceneTaikoHitObjectSamples.cs b/osu.Game.Rulesets.Taiko.Tests/TestSceneTaikoHitObjectSamples.cs index 7089ea6619..1258784a14 100644 --- a/osu.Game.Rulesets.Taiko.Tests/TestSceneTaikoHitObjectSamples.cs +++ b/osu.Game.Rulesets.Taiko.Tests/TestSceneTaikoHitObjectSamples.cs @@ -12,7 +12,7 @@ namespace osu.Game.Rulesets.Taiko.Tests { protected override Ruleset CreatePlayerRuleset() => new TaikoRuleset(); - protected override IResourceStore Resources => new DllResourceStore(Assembly.GetAssembly(typeof(TestSceneTaikoHitObjectSamples))); + public override IResourceStore Resources => new DllResourceStore(Assembly.GetAssembly(typeof(TestSceneTaikoHitObjectSamples))); [TestCase("taiko-normal-hitnormal")] [TestCase("normal-hitnormal")] diff --git a/osu.Game.Tests/Gameplay/TestSceneHitObjectSamples.cs b/osu.Game.Tests/Gameplay/TestSceneHitObjectSamples.cs index 64eaafbe75..afd4365c45 100644 --- a/osu.Game.Tests/Gameplay/TestSceneHitObjectSamples.cs +++ b/osu.Game.Tests/Gameplay/TestSceneHitObjectSamples.cs @@ -14,7 +14,7 @@ namespace osu.Game.Tests.Gameplay public class TestSceneHitObjectSamples : HitObjectSampleTest { protected override Ruleset CreatePlayerRuleset() => new OsuRuleset(); - protected override IResourceStore Resources => TestResources.GetStore(); + public override IResourceStore Resources => TestResources.GetStore(); /// /// Tests that a hitobject which provides no custom sample set retrieves samples from the user skin. diff --git a/osu.Game.Tests/Gameplay/TestSceneStoryboardSamples.cs b/osu.Game.Tests/Gameplay/TestSceneStoryboardSamples.cs index bbab9ae94d..aed28f5f84 100644 --- a/osu.Game.Tests/Gameplay/TestSceneStoryboardSamples.cs +++ b/osu.Game.Tests/Gameplay/TestSceneStoryboardSamples.cs @@ -219,6 +219,7 @@ namespace osu.Game.Tests.Gameplay public AudioManager AudioManager => Audio; public IResourceStore Files => null; + public new IResourceStore Resources => base.Resources; public IResourceStore CreateTextureLoaderStore(IResourceStore underlyingStore) => null; #endregion diff --git a/osu.Game.Tests/Online/TestSceneOnlinePlayBeatmapAvailabilityTracker.cs b/osu.Game.Tests/Online/TestSceneOnlinePlayBeatmapAvailabilityTracker.cs index 8dab570e30..42848ffc0c 100644 --- a/osu.Game.Tests/Online/TestSceneOnlinePlayBeatmapAvailabilityTracker.cs +++ b/osu.Game.Tests/Online/TestSceneOnlinePlayBeatmapAvailabilityTracker.cs @@ -12,6 +12,7 @@ using osu.Framework.Allocation; using osu.Framework.Audio; using osu.Framework.Bindables; using osu.Framework.Extensions; +using osu.Framework.IO.Stores; using osu.Framework.Platform; using osu.Framework.Testing; using osu.Game.Beatmaps; @@ -44,7 +45,7 @@ namespace osu.Game.Tests.Online private void load(AudioManager audio, GameHost host) { Dependencies.Cache(rulesets = new RulesetStore(ContextFactory)); - Dependencies.CacheAs(beatmaps = new TestBeatmapManager(LocalStorage, ContextFactory, rulesets, API, audio, host, Beatmap.Default)); + Dependencies.CacheAs(beatmaps = new TestBeatmapManager(LocalStorage, ContextFactory, rulesets, API, audio, Resources, host, Beatmap.Default)); } [SetUp] @@ -160,8 +161,8 @@ namespace osu.Game.Tests.Online protected override ArchiveDownloadRequest CreateDownloadRequest(BeatmapSetInfo set, bool minimiseDownloadSize) => new TestDownloadRequest(set); - public TestBeatmapManager(Storage storage, IDatabaseContextFactory contextFactory, RulesetStore rulesets, IAPIProvider api, [NotNull] AudioManager audioManager, GameHost host = null, WorkingBeatmap defaultBeatmap = null, bool performOnlineLookups = false) - : base(storage, contextFactory, rulesets, api, audioManager, host, defaultBeatmap, performOnlineLookups) + public TestBeatmapManager(Storage storage, IDatabaseContextFactory contextFactory, RulesetStore rulesets, IAPIProvider api, [NotNull] AudioManager audioManager, IResourceStore resources, GameHost host = null, WorkingBeatmap defaultBeatmap = null, bool performOnlineLookups = false) + : base(storage, contextFactory, rulesets, api, audioManager, resources, host, defaultBeatmap, performOnlineLookups) { } diff --git a/osu.Game.Tests/Visual/Background/TestSceneUserDimBackgrounds.cs b/osu.Game.Tests/Visual/Background/TestSceneUserDimBackgrounds.cs index f89988cd1a..1670d86545 100644 --- a/osu.Game.Tests/Visual/Background/TestSceneUserDimBackgrounds.cs +++ b/osu.Game.Tests/Visual/Background/TestSceneUserDimBackgrounds.cs @@ -48,7 +48,7 @@ namespace osu.Game.Tests.Visual.Background private void load(GameHost host, AudioManager audio) { Dependencies.Cache(rulesets = new RulesetStore(ContextFactory)); - Dependencies.Cache(manager = new BeatmapManager(LocalStorage, ContextFactory, rulesets, null, audio, host, Beatmap.Default)); + Dependencies.Cache(manager = new BeatmapManager(LocalStorage, ContextFactory, rulesets, null, audio, Resources, host, Beatmap.Default)); Dependencies.Cache(new OsuConfigManager(LocalStorage)); manager.Import(TestResources.GetQuickTestBeatmapForImport()).Wait(); diff --git a/osu.Game.Tests/Visual/Collections/TestSceneManageCollectionsDialog.cs b/osu.Game.Tests/Visual/Collections/TestSceneManageCollectionsDialog.cs index eca857f9e5..28218ea220 100644 --- a/osu.Game.Tests/Visual/Collections/TestSceneManageCollectionsDialog.cs +++ b/osu.Game.Tests/Visual/Collections/TestSceneManageCollectionsDialog.cs @@ -36,7 +36,7 @@ namespace osu.Game.Tests.Visual.Collections private void load(GameHost host) { Dependencies.Cache(rulesets = new RulesetStore(ContextFactory)); - Dependencies.Cache(beatmapManager = new BeatmapManager(LocalStorage, ContextFactory, rulesets, null, Audio, host, Beatmap.Default)); + Dependencies.Cache(beatmapManager = new BeatmapManager(LocalStorage, ContextFactory, rulesets, null, Audio, Resources, host, Beatmap.Default)); beatmapManager.Import(TestResources.GetQuickTestBeatmapForImport()).Wait(); diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneDrawableRoomPlaylist.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneDrawableRoomPlaylist.cs index 960aad10c6..dfb78a235b 100644 --- a/osu.Game.Tests/Visual/Multiplayer/TestSceneDrawableRoomPlaylist.cs +++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneDrawableRoomPlaylist.cs @@ -37,7 +37,7 @@ namespace osu.Game.Tests.Visual.Multiplayer private void load(GameHost host, AudioManager audio) { Dependencies.Cache(rulesets = new RulesetStore(ContextFactory)); - Dependencies.Cache(manager = new BeatmapManager(LocalStorage, ContextFactory, rulesets, null, audio, host, Beatmap.Default)); + Dependencies.Cache(manager = new BeatmapManager(LocalStorage, ContextFactory, rulesets, null, audio, Resources, host, Beatmap.Default)); manager.Import(new TestBeatmap(new OsuRuleset().RulesetInfo).BeatmapInfo.BeatmapSet).Wait(); } diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayer.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayer.cs index 424efb255b..c5a6723508 100644 --- a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayer.cs +++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayer.cs @@ -43,7 +43,7 @@ namespace osu.Game.Tests.Visual.Multiplayer private void load(GameHost host, AudioManager audio) { Dependencies.Cache(rulesets = new RulesetStore(ContextFactory)); - Dependencies.Cache(beatmaps = new BeatmapManager(LocalStorage, ContextFactory, rulesets, null, audio, host, Beatmap.Default)); + Dependencies.Cache(beatmaps = new BeatmapManager(LocalStorage, ContextFactory, rulesets, null, audio, Resources, host, Beatmap.Default)); } [SetUp] diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerMatchSongSelect.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerMatchSongSelect.cs index faa5d9e6fc..5b059c06f5 100644 --- a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerMatchSongSelect.cs +++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerMatchSongSelect.cs @@ -42,7 +42,7 @@ namespace osu.Game.Tests.Visual.Multiplayer private void load(GameHost host, AudioManager audio) { Dependencies.Cache(rulesets = new RulesetStore(ContextFactory)); - Dependencies.Cache(manager = new BeatmapManager(LocalStorage, ContextFactory, rulesets, null, audio, host, Beatmap.Default)); + Dependencies.Cache(manager = new BeatmapManager(LocalStorage, ContextFactory, rulesets, null, audio, Resources, host, Beatmap.Default)); beatmaps = new List(); diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerMatchSubScreen.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerMatchSubScreen.cs index f611d5fecf..e8ebc0c426 100644 --- a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerMatchSubScreen.cs +++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerMatchSubScreen.cs @@ -40,7 +40,7 @@ namespace osu.Game.Tests.Visual.Multiplayer private void load(GameHost host, AudioManager audio) { Dependencies.Cache(rulesets = new RulesetStore(ContextFactory)); - Dependencies.Cache(beatmaps = new BeatmapManager(LocalStorage, ContextFactory, rulesets, null, audio, host, Beatmap.Default)); + Dependencies.Cache(beatmaps = new BeatmapManager(LocalStorage, ContextFactory, rulesets, null, audio, Resources, host, Beatmap.Default)); beatmaps.Import(TestResources.GetQuickTestBeatmapForImport()).Wait(); importedSet = beatmaps.GetAllUsableBeatmapSetsEnumerable(IncludedDetails.All).First(); diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerReadyButton.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerReadyButton.cs index dfb4306e67..929cd6ca80 100644 --- a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerReadyButton.cs +++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerReadyButton.cs @@ -41,7 +41,7 @@ namespace osu.Game.Tests.Visual.Multiplayer private void load(GameHost host, AudioManager audio) { Dependencies.Cache(rulesets = new RulesetStore(ContextFactory)); - Dependencies.Cache(beatmaps = new BeatmapManager(LocalStorage, ContextFactory, rulesets, null, audio, host, Beatmap.Default)); + Dependencies.Cache(beatmaps = new BeatmapManager(LocalStorage, ContextFactory, rulesets, null, audio, Resources, host, Beatmap.Default)); beatmaps.Import(TestResources.GetQuickTestBeatmapForImport()).Wait(); Add(beatmapTracker = new OnlinePlayBeatmapAvailabilityTracker diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerSpectateButton.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerSpectateButton.cs index e59b342176..d00404102c 100644 --- a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerSpectateButton.cs +++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerSpectateButton.cs @@ -59,7 +59,7 @@ namespace osu.Game.Tests.Visual.Multiplayer private void load(GameHost host, AudioManager audio) { Dependencies.Cache(rulesets = new RulesetStore(ContextFactory)); - Dependencies.Cache(beatmaps = new BeatmapManager(LocalStorage, ContextFactory, rulesets, null, audio, host, Beatmap.Default)); + Dependencies.Cache(beatmaps = new BeatmapManager(LocalStorage, ContextFactory, rulesets, null, audio, Resources, host, Beatmap.Default)); var beatmapTracker = new OnlinePlayBeatmapAvailabilityTracker { SelectedItem = { BindTarget = selectedItem } }; base.Content.Add(beatmapTracker); diff --git a/osu.Game.Tests/Visual/Multiplayer/TestScenePlaylistsSongSelect.cs b/osu.Game.Tests/Visual/Multiplayer/TestScenePlaylistsSongSelect.cs index 7d83ba569d..d95a95ebe5 100644 --- a/osu.Game.Tests/Visual/Multiplayer/TestScenePlaylistsSongSelect.cs +++ b/osu.Game.Tests/Visual/Multiplayer/TestScenePlaylistsSongSelect.cs @@ -38,7 +38,7 @@ namespace osu.Game.Tests.Visual.Multiplayer private void load(GameHost host, AudioManager audio) { Dependencies.Cache(rulesets = new RulesetStore(ContextFactory)); - Dependencies.Cache(manager = new BeatmapManager(LocalStorage, ContextFactory, rulesets, null, audio, host, Beatmap.Default)); + Dependencies.Cache(manager = new BeatmapManager(LocalStorage, ContextFactory, rulesets, null, audio, Resources, host, Beatmap.Default)); var beatmaps = new List(); diff --git a/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsRoomSubScreen.cs b/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsRoomSubScreen.cs index 264004b6c3..a08a91314b 100644 --- a/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsRoomSubScreen.cs +++ b/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsRoomSubScreen.cs @@ -38,7 +38,7 @@ namespace osu.Game.Tests.Visual.Playlists private void load(GameHost host, AudioManager audio) { Dependencies.Cache(rulesets = new RulesetStore(ContextFactory)); - Dependencies.Cache(manager = new BeatmapManager(LocalStorage, ContextFactory, rulesets, null, audio, host, Beatmap.Default)); + Dependencies.Cache(manager = new BeatmapManager(LocalStorage, ContextFactory, rulesets, null, audio, Resources, host, Beatmap.Default)); manager.Import(new TestBeatmap(new OsuRuleset().RulesetInfo).BeatmapInfo.BeatmapSet).Wait(); diff --git a/osu.Game.Tests/Visual/SongSelect/TestSceneFilterControl.cs b/osu.Game.Tests/Visual/SongSelect/TestSceneFilterControl.cs index c13bdf0955..a5b90e6655 100644 --- a/osu.Game.Tests/Visual/SongSelect/TestSceneFilterControl.cs +++ b/osu.Game.Tests/Visual/SongSelect/TestSceneFilterControl.cs @@ -36,7 +36,7 @@ namespace osu.Game.Tests.Visual.SongSelect private void load(GameHost host) { Dependencies.Cache(rulesets = new RulesetStore(ContextFactory)); - Dependencies.Cache(beatmapManager = new BeatmapManager(LocalStorage, ContextFactory, rulesets, null, Audio, host, Beatmap.Default)); + Dependencies.Cache(beatmapManager = new BeatmapManager(LocalStorage, ContextFactory, rulesets, null, Audio, Resources, host, Beatmap.Default)); beatmapManager.Import(TestResources.GetQuickTestBeatmapForImport()).Wait(); diff --git a/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs b/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs index 2eb6d3f80e..102e5ee425 100644 --- a/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs +++ b/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs @@ -47,7 +47,7 @@ namespace osu.Game.Tests.Visual.SongSelect private void load(GameHost host, AudioManager audio) { Dependencies.Cache(rulesets = new RulesetStore(ContextFactory)); - Dependencies.Cache(manager = new BeatmapManager(LocalStorage, ContextFactory, rulesets, null, audio, host, defaultBeatmap = Beatmap.Default)); + Dependencies.Cache(manager = new BeatmapManager(LocalStorage, ContextFactory, rulesets, null, audio, Resources, host, defaultBeatmap = Beatmap.Default)); Dependencies.Cache(music = new MusicController()); diff --git a/osu.Game.Tests/Visual/UserInterface/TestSceneDeleteLocalScore.cs b/osu.Game.Tests/Visual/UserInterface/TestSceneDeleteLocalScore.cs index 97f3b2954d..3f9e0048dd 100644 --- a/osu.Game.Tests/Visual/UserInterface/TestSceneDeleteLocalScore.cs +++ b/osu.Game.Tests/Visual/UserInterface/TestSceneDeleteLocalScore.cs @@ -81,7 +81,7 @@ namespace osu.Game.Tests.Visual.UserInterface var dependencies = new DependencyContainer(base.CreateChildDependencies(parent)); dependencies.Cache(rulesetStore = new RulesetStore(ContextFactory)); - dependencies.Cache(beatmapManager = new BeatmapManager(LocalStorage, ContextFactory, rulesetStore, null, dependencies.Get(), dependencies.Get(), Beatmap.Default)); + dependencies.Cache(beatmapManager = new BeatmapManager(LocalStorage, ContextFactory, rulesetStore, null, dependencies.Get(), Resources, dependencies.Get(), Beatmap.Default)); dependencies.Cache(scoreManager = new ScoreManager(rulesetStore, () => beatmapManager, LocalStorage, null, ContextFactory)); beatmap = beatmapManager.Import(new ImportTask(TestResources.GetQuickTestBeatmapForImport())).Result.Beatmaps[0]; diff --git a/osu.Game/Beatmaps/BeatmapManager.cs b/osu.Game/Beatmaps/BeatmapManager.cs index e7f6bb3c3a..829327d37b 100644 --- a/osu.Game/Beatmaps/BeatmapManager.cs +++ b/osu.Game/Beatmaps/BeatmapManager.cs @@ -73,6 +73,7 @@ namespace osu.Game.Beatmaps private readonly RulesetStore rulesets; private readonly BeatmapStore beatmaps; private readonly AudioManager audioManager; + private readonly IResourceStore resources; private readonly LargeTextureStore largeTextureStore; private readonly ITrackStore trackStore; @@ -82,12 +83,13 @@ namespace osu.Game.Beatmaps [CanBeNull] private readonly BeatmapOnlineLookupQueue onlineLookupQueue; - public BeatmapManager(Storage storage, IDatabaseContextFactory contextFactory, RulesetStore rulesets, IAPIProvider api, [NotNull] AudioManager audioManager, GameHost host = null, + public BeatmapManager(Storage storage, IDatabaseContextFactory contextFactory, RulesetStore rulesets, IAPIProvider api, [NotNull] AudioManager audioManager, IResourceStore resources, GameHost host = null, WorkingBeatmap defaultBeatmap = null, bool performOnlineLookups = false) : base(storage, contextFactory, api, new BeatmapStore(contextFactory), host) { this.rulesets = rulesets; this.audioManager = audioManager; + this.resources = resources; this.host = host; DefaultBeatmap = defaultBeatmap; @@ -511,6 +513,7 @@ namespace osu.Game.Beatmaps ITrackStore IBeatmapResourceProvider.Tracks => trackStore; AudioManager IStorageResourceProvider.AudioManager => audioManager; IResourceStore IStorageResourceProvider.Files => Files.Store; + IResourceStore IStorageResourceProvider.Resources => resources; IResourceStore IStorageResourceProvider.CreateTextureLoaderStore(IResourceStore underlyingStore) => host?.CreateTextureLoaderStore(underlyingStore); #endregion diff --git a/osu.Game/IO/IStorageResourceProvider.cs b/osu.Game/IO/IStorageResourceProvider.cs index cbd1039807..e4c97e18fa 100644 --- a/osu.Game/IO/IStorageResourceProvider.cs +++ b/osu.Game/IO/IStorageResourceProvider.cs @@ -19,6 +19,11 @@ namespace osu.Game.IO /// IResourceStore Files { get; } + /// + /// Access game-wide resources. + /// + IResourceStore Resources { get; } + /// /// Create a texture loader store based on an underlying data store. /// diff --git a/osu.Game/OsuGameBase.cs b/osu.Game/OsuGameBase.cs index 918f231a19..7935815f38 100644 --- a/osu.Game/OsuGameBase.cs +++ b/osu.Game/OsuGameBase.cs @@ -209,7 +209,7 @@ namespace osu.Game runMigrations(); - dependencies.Cache(SkinManager = new SkinManager(Storage, contextFactory, Host, Audio, new NamespacedResourceStore(Resources, "Skins/Legacy"))); + dependencies.Cache(SkinManager = new SkinManager(Storage, contextFactory, Host, Resources, Audio)); dependencies.CacheAs(SkinManager); // needs to be done here rather than inside SkinManager to ensure thread safety of CurrentSkinInfo. @@ -242,7 +242,7 @@ namespace osu.Game // ordering is important here to ensure foreign keys rules are not broken in ModelStore.Cleanup() dependencies.Cache(ScoreManager = new ScoreManager(RulesetStore, () => BeatmapManager, Storage, API, contextFactory, Host, () => difficultyCache, LocalConfig)); - dependencies.Cache(BeatmapManager = new BeatmapManager(Storage, contextFactory, RulesetStore, API, Audio, Host, defaultBeatmap, true)); + dependencies.Cache(BeatmapManager = new BeatmapManager(Storage, contextFactory, RulesetStore, API, Audio, Resources, Host, defaultBeatmap, true)); // this should likely be moved to ArchiveModelManager when another case appers where it is necessary // to have inter-dependent model managers. this could be obtained with an IHasForeign interface to diff --git a/osu.Game/Skinning/SkinManager.cs b/osu.Game/Skinning/SkinManager.cs index 5793edda30..034b920c1a 100644 --- a/osu.Game/Skinning/SkinManager.cs +++ b/osu.Game/Skinning/SkinManager.cs @@ -37,6 +37,8 @@ namespace osu.Game.Skinning private readonly GameHost host; + private readonly IResourceStore resources; + private readonly IResourceStore legacyDefaultResources; public readonly Bindable CurrentSkin = new Bindable(new DefaultSkin(null)); @@ -48,13 +50,14 @@ namespace osu.Game.Skinning protected override string ImportFromStablePath => "Skins"; - public SkinManager(Storage storage, DatabaseContextFactory contextFactory, GameHost host, AudioManager audio, IResourceStore legacyDefaultResources) + public SkinManager(Storage storage, DatabaseContextFactory contextFactory, GameHost host, IResourceStore resources, AudioManager audio) : base(storage, contextFactory, new SkinStore(contextFactory, storage), host) { this.audio = audio; this.host = host; + this.resources = resources; - this.legacyDefaultResources = legacyDefaultResources; + legacyDefaultResources = new NamespacedResourceStore(resources, "Skins/Legacy"); CurrentSkinInfo.ValueChanged += skin => CurrentSkin.Value = GetSkin(skin.NewValue); CurrentSkin.ValueChanged += skin => @@ -216,6 +219,7 @@ namespace osu.Game.Skinning #region IResourceStorageProvider AudioManager IStorageResourceProvider.AudioManager => audio; + IResourceStore IStorageResourceProvider.Resources => resources; IResourceStore IStorageResourceProvider.Files => Files.Store; IResourceStore IStorageResourceProvider.CreateTextureLoaderStore(IResourceStore underlyingStore) => host.CreateTextureLoaderStore(underlyingStore); diff --git a/osu.Game/Tests/Beatmaps/HitObjectSampleTest.cs b/osu.Game/Tests/Beatmaps/HitObjectSampleTest.cs index 62814d4ed4..038ab334e8 100644 --- a/osu.Game/Tests/Beatmaps/HitObjectSampleTest.cs +++ b/osu.Game/Tests/Beatmaps/HitObjectSampleTest.cs @@ -28,7 +28,8 @@ namespace osu.Game.Tests.Beatmaps [HeadlessTest] public abstract class HitObjectSampleTest : PlayerTestScene, IStorageResourceProvider { - protected abstract IResourceStore Resources { get; } + protected abstract IResourceStore RulesetResources { get; } + protected LegacySkin Skin { get; private set; } [Resolved] @@ -127,6 +128,7 @@ namespace osu.Game.Tests.Beatmaps public AudioManager AudioManager => Audio; public IResourceStore Files => userSkinResourceStore; + public new IResourceStore Resources => base.Resources; public IResourceStore CreateTextureLoaderStore(IResourceStore underlyingStore) => null; #endregion diff --git a/osu.Game/Tests/Visual/OsuTestScene.cs b/osu.Game/Tests/Visual/OsuTestScene.cs index 198d22fedd..be9a015ab2 100644 --- a/osu.Game/Tests/Visual/OsuTestScene.cs +++ b/osu.Game/Tests/Visual/OsuTestScene.cs @@ -14,6 +14,7 @@ using osu.Framework.Audio.Track; using osu.Framework.Bindables; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; +using osu.Framework.IO.Stores; using osu.Framework.Platform; using osu.Framework.Testing; using osu.Framework.Timing; @@ -49,6 +50,8 @@ namespace osu.Game.Tests.Visual private Lazy contextFactory; + protected IResourceStore Resources; + protected IAPIProvider API { get @@ -81,6 +84,8 @@ namespace osu.Game.Tests.Visual if (!UseFreshStoragePerRun) isolatedHostStorage = (parent.Get() as HeadlessGameHost)?.Storage; + Resources = parent.Get().Resources; + contextFactory = new Lazy(() => { var factory = new DatabaseContextFactory(LocalStorage); diff --git a/osu.Game/Tests/Visual/SkinnableTestScene.cs b/osu.Game/Tests/Visual/SkinnableTestScene.cs index 3d2c68c2ad..729c12187f 100644 --- a/osu.Game/Tests/Visual/SkinnableTestScene.cs +++ b/osu.Game/Tests/Visual/SkinnableTestScene.cs @@ -156,6 +156,7 @@ namespace osu.Game.Tests.Visual public AudioManager AudioManager => Audio; public IResourceStore Files => null; + public new IResourceStore Resources => base.Resources; public IResourceStore CreateTextureLoaderStore(IResourceStore underlyingStore) => host.CreateTextureLoaderStore(underlyingStore); #endregion From 65709ec7d7853657128db2613dda60ad5c80dd05 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 31 May 2021 18:58:40 +0900 Subject: [PATCH 31/43] Move leagcy resource store construction local to `DefaultLegacySkin` --- osu.Game/Skinning/DefaultLegacySkin.cs | 8 ++++---- osu.Game/Skinning/SkinInfo.cs | 6 +----- osu.Game/Skinning/SkinManager.cs | 6 +----- osu.Game/Tests/Visual/LegacySkinPlayerTestScene.cs | 5 ++--- osu.Game/Tests/Visual/SkinnableTestScene.cs | 4 ++-- 5 files changed, 10 insertions(+), 19 deletions(-) diff --git a/osu.Game/Skinning/DefaultLegacySkin.cs b/osu.Game/Skinning/DefaultLegacySkin.cs index f30130b1fb..30192182f3 100644 --- a/osu.Game/Skinning/DefaultLegacySkin.cs +++ b/osu.Game/Skinning/DefaultLegacySkin.cs @@ -11,14 +11,14 @@ namespace osu.Game.Skinning { public class DefaultLegacySkin : LegacySkin { - public DefaultLegacySkin(IResourceStore storage, IStorageResourceProvider resources) - : this(Info, storage, resources) + public DefaultLegacySkin(IStorageResourceProvider resources) + : this(Info, resources) { } [UsedImplicitly(ImplicitUseKindFlags.InstantiatedWithFixedConstructorSignature)] - public DefaultLegacySkin(SkinInfo skin, IResourceStore storage, IStorageResourceProvider resources) - : base(skin, storage, resources, string.Empty) + public DefaultLegacySkin(SkinInfo skin, IStorageResourceProvider resources) + : base(skin, new NamespacedResourceStore(resources.Resources, "Skins/Legacy"), resources, string.Empty) { Configuration.CustomColours["SliderBall"] = new Color4(2, 170, 255, 255); Configuration.AddComboColours( diff --git a/osu.Game/Skinning/SkinInfo.cs b/osu.Game/Skinning/SkinInfo.cs index 55760876e3..e30bc16d8b 100644 --- a/osu.Game/Skinning/SkinInfo.cs +++ b/osu.Game/Skinning/SkinInfo.cs @@ -4,7 +4,6 @@ using System; using System.Collections.Generic; using osu.Framework.Extensions.ObjectExtensions; -using osu.Framework.IO.Stores; using osu.Game.Configuration; using osu.Game.Database; using osu.Game.Extensions; @@ -28,16 +27,13 @@ namespace osu.Game.Skinning public string InstantiationInfo { get; set; } - public virtual Skin CreateInstance(IResourceStore legacyDefaultResources, IStorageResourceProvider resources) + public virtual Skin CreateInstance(IStorageResourceProvider resources) { var type = string.IsNullOrEmpty(InstantiationInfo) // handle the case of skins imported before InstantiationInfo was added. ? typeof(LegacySkin) : Type.GetType(InstantiationInfo).AsNonNull(); - if (type == typeof(DefaultLegacySkin)) - return (Skin)Activator.CreateInstance(type, this, legacyDefaultResources, resources); - return (Skin)Activator.CreateInstance(type, this, resources); } diff --git a/osu.Game/Skinning/SkinManager.cs b/osu.Game/Skinning/SkinManager.cs index 034b920c1a..079c537066 100644 --- a/osu.Game/Skinning/SkinManager.cs +++ b/osu.Game/Skinning/SkinManager.cs @@ -39,8 +39,6 @@ namespace osu.Game.Skinning private readonly IResourceStore resources; - private readonly IResourceStore legacyDefaultResources; - public readonly Bindable CurrentSkin = new Bindable(new DefaultSkin(null)); public readonly Bindable CurrentSkinInfo = new Bindable(SkinInfo.Default) { Default = SkinInfo.Default }; @@ -57,8 +55,6 @@ namespace osu.Game.Skinning this.host = host; this.resources = resources; - legacyDefaultResources = new NamespacedResourceStore(resources, "Skins/Legacy"); - CurrentSkinInfo.ValueChanged += skin => CurrentSkin.Value = GetSkin(skin.NewValue); CurrentSkin.ValueChanged += skin => { @@ -155,7 +151,7 @@ namespace osu.Game.Skinning /// /// The skin to lookup. /// A instance correlating to the provided . - public Skin GetSkin(SkinInfo skinInfo) => skinInfo.CreateInstance(legacyDefaultResources, this); + public Skin GetSkin(SkinInfo skinInfo) => skinInfo.CreateInstance(this); /// /// Ensure that the current skin is in a state it can accept user modifications. diff --git a/osu.Game/Tests/Visual/LegacySkinPlayerTestScene.cs b/osu.Game/Tests/Visual/LegacySkinPlayerTestScene.cs index c186525757..e3d7a21ab0 100644 --- a/osu.Game/Tests/Visual/LegacySkinPlayerTestScene.cs +++ b/osu.Game/Tests/Visual/LegacySkinPlayerTestScene.cs @@ -3,7 +3,6 @@ using NUnit.Framework; using osu.Framework.Allocation; -using osu.Framework.IO.Stores; using osu.Game.Rulesets; using osu.Game.Skinning; @@ -17,9 +16,9 @@ namespace osu.Game.Tests.Visual protected override TestPlayer CreatePlayer(Ruleset ruleset) => new SkinProvidingPlayer(legacySkinSource); [BackgroundDependencyLoader] - private void load(OsuGameBase game, SkinManager skins) + private void load(SkinManager skins) { - var legacySkin = new DefaultLegacySkin(new NamespacedResourceStore(game.Resources, "Skins/Legacy"), skins); + var legacySkin = new DefaultLegacySkin(skins); legacySkinSource = new SkinProvidingContainer(legacySkin); } diff --git a/osu.Game/Tests/Visual/SkinnableTestScene.cs b/osu.Game/Tests/Visual/SkinnableTestScene.cs index 729c12187f..e801412fc5 100644 --- a/osu.Game/Tests/Visual/SkinnableTestScene.cs +++ b/osu.Game/Tests/Visual/SkinnableTestScene.cs @@ -40,12 +40,12 @@ namespace osu.Game.Tests.Visual } [BackgroundDependencyLoader] - private void load(AudioManager audio, SkinManager skinManager, OsuGameBase game) + private void load(AudioManager audio, SkinManager skinManager) { var dllStore = new DllResourceStore(DynamicCompilationOriginal.GetType().Assembly); metricsSkin = new TestLegacySkin(new SkinInfo { Name = "metrics-skin" }, new NamespacedResourceStore(dllStore, "Resources/metrics_skin"), this, true); - defaultSkin = new DefaultLegacySkin(new NamespacedResourceStore(game.Resources, "Skins/Legacy"), this); + defaultSkin = new DefaultLegacySkin(this); specialSkin = new TestLegacySkin(new SkinInfo { Name = "special-skin" }, new NamespacedResourceStore(dllStore, "Resources/special_skin"), this, true); oldSkin = new TestLegacySkin(new SkinInfo { Name = "old-skin" }, new NamespacedResourceStore(dllStore, "Resources/old_skin"), this, true); } From ebfc24a499eb56e17b8ee12f008b7b4526931dad Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 31 May 2021 18:55:47 +0900 Subject: [PATCH 32/43] Rename conflicting resources --- .../TestSceneManiaHitObjectSamples.cs | 2 +- .../TestSceneTaikoHitObjectSamples.cs | 2 +- osu.Game.Tests/Gameplay/TestSceneHitObjectSamples.cs | 2 +- osu.Game/Tests/Beatmaps/HitObjectSampleTest.cs | 3 +-- 4 files changed, 4 insertions(+), 5 deletions(-) diff --git a/osu.Game.Rulesets.Mania.Tests/TestSceneManiaHitObjectSamples.cs b/osu.Game.Rulesets.Mania.Tests/TestSceneManiaHitObjectSamples.cs index eedabd1adb..ea57e51d1c 100644 --- a/osu.Game.Rulesets.Mania.Tests/TestSceneManiaHitObjectSamples.cs +++ b/osu.Game.Rulesets.Mania.Tests/TestSceneManiaHitObjectSamples.cs @@ -11,7 +11,7 @@ namespace osu.Game.Rulesets.Mania.Tests public class TestSceneManiaHitObjectSamples : HitObjectSampleTest { protected override Ruleset CreatePlayerRuleset() => new ManiaRuleset(); - public override IResourceStore Resources => new DllResourceStore(Assembly.GetAssembly(typeof(TestSceneManiaHitObjectSamples))); + protected override IResourceStore RulesetResources => new DllResourceStore(Assembly.GetAssembly(typeof(TestSceneManiaHitObjectSamples))); /// /// Tests that when a normal sample bank is used, the normal hitsound will be looked up. diff --git a/osu.Game.Rulesets.Taiko.Tests/TestSceneTaikoHitObjectSamples.cs b/osu.Game.Rulesets.Taiko.Tests/TestSceneTaikoHitObjectSamples.cs index 1258784a14..221d715a35 100644 --- a/osu.Game.Rulesets.Taiko.Tests/TestSceneTaikoHitObjectSamples.cs +++ b/osu.Game.Rulesets.Taiko.Tests/TestSceneTaikoHitObjectSamples.cs @@ -12,7 +12,7 @@ namespace osu.Game.Rulesets.Taiko.Tests { protected override Ruleset CreatePlayerRuleset() => new TaikoRuleset(); - public override IResourceStore Resources => new DllResourceStore(Assembly.GetAssembly(typeof(TestSceneTaikoHitObjectSamples))); + protected override IResourceStore RulesetResources => new DllResourceStore(Assembly.GetAssembly(typeof(TestSceneTaikoHitObjectSamples))); [TestCase("taiko-normal-hitnormal")] [TestCase("normal-hitnormal")] diff --git a/osu.Game.Tests/Gameplay/TestSceneHitObjectSamples.cs b/osu.Game.Tests/Gameplay/TestSceneHitObjectSamples.cs index afd4365c45..fc420e22a1 100644 --- a/osu.Game.Tests/Gameplay/TestSceneHitObjectSamples.cs +++ b/osu.Game.Tests/Gameplay/TestSceneHitObjectSamples.cs @@ -14,7 +14,7 @@ namespace osu.Game.Tests.Gameplay public class TestSceneHitObjectSamples : HitObjectSampleTest { protected override Ruleset CreatePlayerRuleset() => new OsuRuleset(); - public override IResourceStore Resources => TestResources.GetStore(); + protected override IResourceStore RulesetResources => TestResources.GetStore(); /// /// Tests that a hitobject which provides no custom sample set retrieves samples from the user skin. diff --git a/osu.Game/Tests/Beatmaps/HitObjectSampleTest.cs b/osu.Game/Tests/Beatmaps/HitObjectSampleTest.cs index 038ab334e8..7ee6c519b7 100644 --- a/osu.Game/Tests/Beatmaps/HitObjectSampleTest.cs +++ b/osu.Game/Tests/Beatmaps/HitObjectSampleTest.cs @@ -29,7 +29,6 @@ namespace osu.Game.Tests.Beatmaps public abstract class HitObjectSampleTest : PlayerTestScene, IStorageResourceProvider { protected abstract IResourceStore RulesetResources { get; } - protected LegacySkin Skin { get; private set; } [Resolved] @@ -76,7 +75,7 @@ namespace osu.Game.Tests.Beatmaps AddStep($"load {filename}", () => { - using (var reader = new LineBufferedReader(Resources.GetStream($"Resources/SampleLookups/{filename}"))) + using (var reader = new LineBufferedReader(RulesetResources.GetStream($"Resources/SampleLookups/{filename}"))) currentTestBeatmap = Decoder.GetDecoder(reader).Decode(reader); // populate ruleset for beatmap converters that require it to be present. From e78391db7a298ec4e0d84516832dfe8c51e9f5a6 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 31 May 2021 19:57:31 +0900 Subject: [PATCH 33/43] Fix usage of DI before it's ready in combo colour tests --- .../TestSceneLegacyBeatmapSkin.cs | 14 +++++++------- .../TestSceneLegacyBeatmapSkin.cs | 10 +++++----- .../Tests/Beatmaps/LegacyBeatmapSkinColourTest.cs | 12 ++++++++---- 3 files changed, 20 insertions(+), 16 deletions(-) diff --git a/osu.Game.Rulesets.Catch.Tests/TestSceneLegacyBeatmapSkin.cs b/osu.Game.Rulesets.Catch.Tests/TestSceneLegacyBeatmapSkin.cs index eea83ef7c1..28ff02cb1f 100644 --- a/osu.Game.Rulesets.Catch.Tests/TestSceneLegacyBeatmapSkin.cs +++ b/osu.Game.Rulesets.Catch.Tests/TestSceneLegacyBeatmapSkin.cs @@ -34,7 +34,7 @@ namespace osu.Game.Rulesets.Catch.Tests [TestCase(false, false)] public override void TestBeatmapComboColours(bool userHasCustomColours, bool useBeatmapSkin) { - TestBeatmap = new CatchCustomSkinWorkingBeatmap(audio, true); + PrepareBeatmap(() => new CatchCustomSkinWorkingBeatmap(audio, true)); base.TestBeatmapComboColours(userHasCustomColours, useBeatmapSkin); AddAssert("is beatmap skin colours", () => TestPlayer.UsableComboColours.SequenceEqual(TestBeatmapSkin.Colours)); } @@ -43,7 +43,7 @@ namespace osu.Game.Rulesets.Catch.Tests [TestCase(false)] public override void TestBeatmapComboColoursOverride(bool useBeatmapSkin) { - TestBeatmap = new CatchCustomSkinWorkingBeatmap(audio, true); + PrepareBeatmap(() => new CatchCustomSkinWorkingBeatmap(audio, true)); base.TestBeatmapComboColoursOverride(useBeatmapSkin); AddAssert("is user custom skin colours", () => TestPlayer.UsableComboColours.SequenceEqual(TestSkin.Colours)); } @@ -52,7 +52,7 @@ namespace osu.Game.Rulesets.Catch.Tests [TestCase(false)] public override void TestBeatmapComboColoursOverrideWithDefaultColours(bool useBeatmapSkin) { - TestBeatmap = new CatchCustomSkinWorkingBeatmap(audio, true); + PrepareBeatmap(() => new CatchCustomSkinWorkingBeatmap(audio, true)); base.TestBeatmapComboColoursOverrideWithDefaultColours(useBeatmapSkin); AddAssert("is default user skin colours", () => TestPlayer.UsableComboColours.SequenceEqual(SkinConfiguration.DefaultComboColours)); } @@ -63,7 +63,7 @@ namespace osu.Game.Rulesets.Catch.Tests [TestCase(false, false)] public override void TestBeatmapNoComboColours(bool useBeatmapSkin, bool useBeatmapColour) { - TestBeatmap = new CatchCustomSkinWorkingBeatmap(audio, false); + PrepareBeatmap(() => new CatchCustomSkinWorkingBeatmap(audio, false)); base.TestBeatmapNoComboColours(useBeatmapSkin, useBeatmapColour); AddAssert("is default user skin colours", () => TestPlayer.UsableComboColours.SequenceEqual(SkinConfiguration.DefaultComboColours)); } @@ -74,7 +74,7 @@ namespace osu.Game.Rulesets.Catch.Tests [TestCase(false, false)] public override void TestBeatmapNoComboColoursSkinOverride(bool useBeatmapSkin, bool useBeatmapColour) { - TestBeatmap = new CatchCustomSkinWorkingBeatmap(audio, false); + PrepareBeatmap(() => new CatchCustomSkinWorkingBeatmap(audio, false)); base.TestBeatmapNoComboColoursSkinOverride(useBeatmapSkin, useBeatmapColour); AddAssert("is custom user skin colours", () => TestPlayer.UsableComboColours.SequenceEqual(TestSkin.Colours)); } @@ -83,7 +83,7 @@ namespace osu.Game.Rulesets.Catch.Tests [TestCase(false)] public void TestBeatmapHyperDashColours(bool useBeatmapSkin) { - TestBeatmap = new CatchCustomSkinWorkingBeatmap(audio, true); + PrepareBeatmap(() => new CatchCustomSkinWorkingBeatmap(audio, true)); ConfigureTest(useBeatmapSkin, true, true); AddAssert("is custom hyper dash colours", () => ((CatchExposedPlayer)TestPlayer).UsableHyperDashColour == TestBeatmapSkin.HYPER_DASH_COLOUR); AddAssert("is custom hyper dash after image colours", () => ((CatchExposedPlayer)TestPlayer).UsableHyperDashAfterImageColour == TestBeatmapSkin.HYPER_DASH_AFTER_IMAGE_COLOUR); @@ -94,7 +94,7 @@ namespace osu.Game.Rulesets.Catch.Tests [TestCase(false)] public void TestBeatmapHyperDashColoursOverride(bool useBeatmapSkin) { - TestBeatmap = new CatchCustomSkinWorkingBeatmap(audio, true); + PrepareBeatmap(() => new CatchCustomSkinWorkingBeatmap(audio, true)); ConfigureTest(useBeatmapSkin, false, true); AddAssert("is custom hyper dash colours", () => ((CatchExposedPlayer)TestPlayer).UsableHyperDashColour == TestSkin.HYPER_DASH_COLOUR); AddAssert("is custom hyper dash after image colours", () => ((CatchExposedPlayer)TestPlayer).UsableHyperDashAfterImageColour == TestSkin.HYPER_DASH_AFTER_IMAGE_COLOUR); diff --git a/osu.Game.Rulesets.Osu.Tests/TestSceneLegacyBeatmapSkin.cs b/osu.Game.Rulesets.Osu.Tests/TestSceneLegacyBeatmapSkin.cs index c26419b0e8..66f919f91f 100644 --- a/osu.Game.Rulesets.Osu.Tests/TestSceneLegacyBeatmapSkin.cs +++ b/osu.Game.Rulesets.Osu.Tests/TestSceneLegacyBeatmapSkin.cs @@ -32,7 +32,7 @@ namespace osu.Game.Rulesets.Osu.Tests [TestCase(false, false)] public override void TestBeatmapComboColours(bool userHasCustomColours, bool useBeatmapSkin) { - TestBeatmap = new OsuCustomSkinWorkingBeatmap(audio, true); + PrepareBeatmap(() => new OsuCustomSkinWorkingBeatmap(audio, true)); base.TestBeatmapComboColours(userHasCustomColours, useBeatmapSkin); AddAssert("is beatmap skin colours", () => TestPlayer.UsableComboColours.SequenceEqual(TestBeatmapSkin.Colours)); } @@ -41,7 +41,7 @@ namespace osu.Game.Rulesets.Osu.Tests [TestCase(false)] public override void TestBeatmapComboColoursOverride(bool useBeatmapSkin) { - TestBeatmap = new OsuCustomSkinWorkingBeatmap(audio, true); + PrepareBeatmap(() => new OsuCustomSkinWorkingBeatmap(audio, true)); base.TestBeatmapComboColoursOverride(useBeatmapSkin); AddAssert("is user custom skin colours", () => TestPlayer.UsableComboColours.SequenceEqual(TestSkin.Colours)); } @@ -50,7 +50,7 @@ namespace osu.Game.Rulesets.Osu.Tests [TestCase(false)] public override void TestBeatmapComboColoursOverrideWithDefaultColours(bool useBeatmapSkin) { - TestBeatmap = new OsuCustomSkinWorkingBeatmap(audio, true); + PrepareBeatmap(() => new OsuCustomSkinWorkingBeatmap(audio, true)); base.TestBeatmapComboColoursOverrideWithDefaultColours(useBeatmapSkin); AddAssert("is default user skin colours", () => TestPlayer.UsableComboColours.SequenceEqual(SkinConfiguration.DefaultComboColours)); } @@ -61,7 +61,7 @@ namespace osu.Game.Rulesets.Osu.Tests [TestCase(false, false)] public override void TestBeatmapNoComboColours(bool useBeatmapSkin, bool useBeatmapColour) { - TestBeatmap = new OsuCustomSkinWorkingBeatmap(audio, false); + PrepareBeatmap(() => new OsuCustomSkinWorkingBeatmap(audio, false)); base.TestBeatmapNoComboColours(useBeatmapSkin, useBeatmapColour); AddAssert("is default user skin colours", () => TestPlayer.UsableComboColours.SequenceEqual(SkinConfiguration.DefaultComboColours)); } @@ -72,7 +72,7 @@ namespace osu.Game.Rulesets.Osu.Tests [TestCase(false, false)] public override void TestBeatmapNoComboColoursSkinOverride(bool useBeatmapSkin, bool useBeatmapColour) { - TestBeatmap = new OsuCustomSkinWorkingBeatmap(audio, false); + PrepareBeatmap(() => new OsuCustomSkinWorkingBeatmap(audio, false)); base.TestBeatmapNoComboColoursSkinOverride(useBeatmapSkin, useBeatmapColour); AddAssert("is custom user skin colours", () => TestPlayer.UsableComboColours.SequenceEqual(TestSkin.Colours)); } diff --git a/osu.Game/Tests/Beatmaps/LegacyBeatmapSkinColourTest.cs b/osu.Game/Tests/Beatmaps/LegacyBeatmapSkinColourTest.cs index 051ede30b7..708963c760 100644 --- a/osu.Game/Tests/Beatmaps/LegacyBeatmapSkinColourTest.cs +++ b/osu.Game/Tests/Beatmaps/LegacyBeatmapSkinColourTest.cs @@ -21,8 +21,10 @@ namespace osu.Game.Tests.Beatmaps { protected readonly Bindable BeatmapSkins = new Bindable(); protected readonly Bindable BeatmapColours = new Bindable(); + protected ExposedPlayer TestPlayer; - protected WorkingBeatmap TestBeatmap; + + private WorkingBeatmap testBeatmap; public virtual void TestBeatmapComboColours(bool userHasCustomColours, bool useBeatmapSkin) => ConfigureTest(useBeatmapSkin, true, userHasCustomColours); @@ -34,10 +36,12 @@ namespace osu.Game.Tests.Beatmaps public virtual void TestBeatmapNoComboColoursSkinOverride(bool useBeatmapSkin, bool useBeatmapColour) => ConfigureTest(useBeatmapSkin, useBeatmapColour, true); - protected virtual void ConfigureTest(bool useBeatmapSkin, bool useBeatmapColours, bool userHasCustomColours) + protected void PrepareBeatmap(Func createBeatmap) => AddStep("prepare beatmap", () => testBeatmap = createBeatmap()); + + protected void ConfigureTest(bool useBeatmapSkin, bool useBeatmapColours, bool userHasCustomColours) { configureSettings(useBeatmapSkin, useBeatmapColours); - AddStep($"load {(((CustomSkinWorkingBeatmap)TestBeatmap).HasColours ? "coloured " : "")} beatmap", () => TestPlayer = LoadBeatmap(userHasCustomColours)); + AddStep("load beatmap", () => TestPlayer = LoadBeatmap(userHasCustomColours)); AddUntilStep("wait for player load", () => TestPlayer.IsLoaded); } @@ -57,7 +61,7 @@ namespace osu.Game.Tests.Beatmaps { ExposedPlayer player; - Beatmap.Value = TestBeatmap; + Beatmap.Value = testBeatmap; LoadScreen(player = CreateTestPlayer(userHasCustomColours)); From f60e9cb08588760c1355bc9b41a1fc012e308080 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 31 May 2021 20:00:47 +0900 Subject: [PATCH 34/43] Remove weird override logic in `TestCase` methods --- .../TestSceneLegacyBeatmapSkin.cs | 20 +++++++++---------- .../TestSceneLegacyBeatmapSkin.cs | 20 +++++++++---------- .../Beatmaps/LegacyBeatmapSkinColourTest.cs | 10 ---------- 3 files changed, 20 insertions(+), 30 deletions(-) diff --git a/osu.Game.Rulesets.Catch.Tests/TestSceneLegacyBeatmapSkin.cs b/osu.Game.Rulesets.Catch.Tests/TestSceneLegacyBeatmapSkin.cs index 28ff02cb1f..bc3daca16f 100644 --- a/osu.Game.Rulesets.Catch.Tests/TestSceneLegacyBeatmapSkin.cs +++ b/osu.Game.Rulesets.Catch.Tests/TestSceneLegacyBeatmapSkin.cs @@ -32,28 +32,28 @@ namespace osu.Game.Rulesets.Catch.Tests [TestCase(true, false)] [TestCase(false, true)] [TestCase(false, false)] - public override void TestBeatmapComboColours(bool userHasCustomColours, bool useBeatmapSkin) + public void TestBeatmapComboColours(bool userHasCustomColours, bool useBeatmapSkin) { PrepareBeatmap(() => new CatchCustomSkinWorkingBeatmap(audio, true)); - base.TestBeatmapComboColours(userHasCustomColours, useBeatmapSkin); + ConfigureTest(useBeatmapSkin, true, userHasCustomColours); AddAssert("is beatmap skin colours", () => TestPlayer.UsableComboColours.SequenceEqual(TestBeatmapSkin.Colours)); } [TestCase(true)] [TestCase(false)] - public override void TestBeatmapComboColoursOverride(bool useBeatmapSkin) + public void TestBeatmapComboColoursOverride(bool useBeatmapSkin) { PrepareBeatmap(() => new CatchCustomSkinWorkingBeatmap(audio, true)); - base.TestBeatmapComboColoursOverride(useBeatmapSkin); + ConfigureTest(useBeatmapSkin, false, true); AddAssert("is user custom skin colours", () => TestPlayer.UsableComboColours.SequenceEqual(TestSkin.Colours)); } [TestCase(true)] [TestCase(false)] - public override void TestBeatmapComboColoursOverrideWithDefaultColours(bool useBeatmapSkin) + public void TestBeatmapComboColoursOverrideWithDefaultColours(bool useBeatmapSkin) { PrepareBeatmap(() => new CatchCustomSkinWorkingBeatmap(audio, true)); - base.TestBeatmapComboColoursOverrideWithDefaultColours(useBeatmapSkin); + ConfigureTest(useBeatmapSkin, false, false); AddAssert("is default user skin colours", () => TestPlayer.UsableComboColours.SequenceEqual(SkinConfiguration.DefaultComboColours)); } @@ -61,10 +61,10 @@ namespace osu.Game.Rulesets.Catch.Tests [TestCase(false, true)] [TestCase(true, false)] [TestCase(false, false)] - public override void TestBeatmapNoComboColours(bool useBeatmapSkin, bool useBeatmapColour) + public void TestBeatmapNoComboColours(bool useBeatmapSkin, bool useBeatmapColour) { PrepareBeatmap(() => new CatchCustomSkinWorkingBeatmap(audio, false)); - base.TestBeatmapNoComboColours(useBeatmapSkin, useBeatmapColour); + ConfigureTest(useBeatmapSkin, useBeatmapColour, false); AddAssert("is default user skin colours", () => TestPlayer.UsableComboColours.SequenceEqual(SkinConfiguration.DefaultComboColours)); } @@ -72,10 +72,10 @@ namespace osu.Game.Rulesets.Catch.Tests [TestCase(false, true)] [TestCase(true, false)] [TestCase(false, false)] - public override void TestBeatmapNoComboColoursSkinOverride(bool useBeatmapSkin, bool useBeatmapColour) + public void TestBeatmapNoComboColoursSkinOverride(bool useBeatmapSkin, bool useBeatmapColour) { PrepareBeatmap(() => new CatchCustomSkinWorkingBeatmap(audio, false)); - base.TestBeatmapNoComboColoursSkinOverride(useBeatmapSkin, useBeatmapColour); + ConfigureTest(useBeatmapSkin, useBeatmapColour, true); AddAssert("is custom user skin colours", () => TestPlayer.UsableComboColours.SequenceEqual(TestSkin.Colours)); } diff --git a/osu.Game.Rulesets.Osu.Tests/TestSceneLegacyBeatmapSkin.cs b/osu.Game.Rulesets.Osu.Tests/TestSceneLegacyBeatmapSkin.cs index 66f919f91f..56307861f1 100644 --- a/osu.Game.Rulesets.Osu.Tests/TestSceneLegacyBeatmapSkin.cs +++ b/osu.Game.Rulesets.Osu.Tests/TestSceneLegacyBeatmapSkin.cs @@ -30,28 +30,28 @@ namespace osu.Game.Rulesets.Osu.Tests [TestCase(true, false)] [TestCase(false, true)] [TestCase(false, false)] - public override void TestBeatmapComboColours(bool userHasCustomColours, bool useBeatmapSkin) + public void TestBeatmapComboColours(bool userHasCustomColours, bool useBeatmapSkin) { PrepareBeatmap(() => new OsuCustomSkinWorkingBeatmap(audio, true)); - base.TestBeatmapComboColours(userHasCustomColours, useBeatmapSkin); + ConfigureTest(useBeatmapSkin, true, userHasCustomColours); AddAssert("is beatmap skin colours", () => TestPlayer.UsableComboColours.SequenceEqual(TestBeatmapSkin.Colours)); } [TestCase(true)] [TestCase(false)] - public override void TestBeatmapComboColoursOverride(bool useBeatmapSkin) + public void TestBeatmapComboColoursOverride(bool useBeatmapSkin) { PrepareBeatmap(() => new OsuCustomSkinWorkingBeatmap(audio, true)); - base.TestBeatmapComboColoursOverride(useBeatmapSkin); + ConfigureTest(useBeatmapSkin, false, true); AddAssert("is user custom skin colours", () => TestPlayer.UsableComboColours.SequenceEqual(TestSkin.Colours)); } [TestCase(true)] [TestCase(false)] - public override void TestBeatmapComboColoursOverrideWithDefaultColours(bool useBeatmapSkin) + public void TestBeatmapComboColoursOverrideWithDefaultColours(bool useBeatmapSkin) { PrepareBeatmap(() => new OsuCustomSkinWorkingBeatmap(audio, true)); - base.TestBeatmapComboColoursOverrideWithDefaultColours(useBeatmapSkin); + ConfigureTest(useBeatmapSkin, false, false); AddAssert("is default user skin colours", () => TestPlayer.UsableComboColours.SequenceEqual(SkinConfiguration.DefaultComboColours)); } @@ -59,10 +59,10 @@ namespace osu.Game.Rulesets.Osu.Tests [TestCase(false, true)] [TestCase(true, false)] [TestCase(false, false)] - public override void TestBeatmapNoComboColours(bool useBeatmapSkin, bool useBeatmapColour) + public void TestBeatmapNoComboColours(bool useBeatmapSkin, bool useBeatmapColour) { PrepareBeatmap(() => new OsuCustomSkinWorkingBeatmap(audio, false)); - base.TestBeatmapNoComboColours(useBeatmapSkin, useBeatmapColour); + ConfigureTest(useBeatmapSkin, useBeatmapColour, false); AddAssert("is default user skin colours", () => TestPlayer.UsableComboColours.SequenceEqual(SkinConfiguration.DefaultComboColours)); } @@ -70,10 +70,10 @@ namespace osu.Game.Rulesets.Osu.Tests [TestCase(false, true)] [TestCase(true, false)] [TestCase(false, false)] - public override void TestBeatmapNoComboColoursSkinOverride(bool useBeatmapSkin, bool useBeatmapColour) + public void TestBeatmapNoComboColoursSkinOverride(bool useBeatmapSkin, bool useBeatmapColour) { PrepareBeatmap(() => new OsuCustomSkinWorkingBeatmap(audio, false)); - base.TestBeatmapNoComboColoursSkinOverride(useBeatmapSkin, useBeatmapColour); + ConfigureTest(useBeatmapSkin, useBeatmapColour, true); AddAssert("is custom user skin colours", () => TestPlayer.UsableComboColours.SequenceEqual(TestSkin.Colours)); } diff --git a/osu.Game/Tests/Beatmaps/LegacyBeatmapSkinColourTest.cs b/osu.Game/Tests/Beatmaps/LegacyBeatmapSkinColourTest.cs index 708963c760..0a7fb1483d 100644 --- a/osu.Game/Tests/Beatmaps/LegacyBeatmapSkinColourTest.cs +++ b/osu.Game/Tests/Beatmaps/LegacyBeatmapSkinColourTest.cs @@ -26,16 +26,6 @@ namespace osu.Game.Tests.Beatmaps private WorkingBeatmap testBeatmap; - public virtual void TestBeatmapComboColours(bool userHasCustomColours, bool useBeatmapSkin) => ConfigureTest(useBeatmapSkin, true, userHasCustomColours); - - public virtual void TestBeatmapComboColoursOverride(bool useBeatmapSkin) => ConfigureTest(useBeatmapSkin, false, true); - - public virtual void TestBeatmapComboColoursOverrideWithDefaultColours(bool useBeatmapSkin) => ConfigureTest(useBeatmapSkin, false, false); - - public virtual void TestBeatmapNoComboColours(bool useBeatmapSkin, bool useBeatmapColour) => ConfigureTest(useBeatmapSkin, useBeatmapColour, false); - - public virtual void TestBeatmapNoComboColoursSkinOverride(bool useBeatmapSkin, bool useBeatmapColour) => ConfigureTest(useBeatmapSkin, useBeatmapColour, true); - protected void PrepareBeatmap(Func createBeatmap) => AddStep("prepare beatmap", () => testBeatmap = createBeatmap()); protected void ConfigureTest(bool useBeatmapSkin, bool useBeatmapColours, bool userHasCustomColours) From 4b27d43e26b3983bd7a60630f997f3317edecc26 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 31 May 2021 15:13:56 +0900 Subject: [PATCH 35/43] Add new parameter for default fallback logic in `LegacySkin` --- osu.Game/Skinning/LegacyBeatmapSkin.cs | 2 +- osu.Game/Skinning/LegacySkin.cs | 21 ++++++++++++++++++--- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/osu.Game/Skinning/LegacyBeatmapSkin.cs b/osu.Game/Skinning/LegacyBeatmapSkin.cs index caf37e5bc9..6085eb1c37 100644 --- a/osu.Game/Skinning/LegacyBeatmapSkin.cs +++ b/osu.Game/Skinning/LegacyBeatmapSkin.cs @@ -18,7 +18,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(createSkinInfo(beatmap), new LegacySkinResourceStore(beatmap.BeatmapSet, storage), resources, beatmap.Path, fallbackToDefault: false) { // Disallow default colours fallback on beatmap skins to allow using parent skin combo colours. (via SkinProvidingContainer) Configuration.AllowDefaultComboColoursFallback = false; diff --git a/osu.Game/Skinning/LegacySkin.cs b/osu.Game/Skinning/LegacySkin.cs index d3474caac9..908ed37b6a 100644 --- a/osu.Game/Skinning/LegacySkin.cs +++ b/osu.Game/Skinning/LegacySkin.cs @@ -26,6 +26,8 @@ namespace osu.Game.Skinning { public class LegacySkin : Skin { + private readonly bool fallbackToDefault; + [CanBeNull] protected TextureStore Textures; @@ -54,16 +56,29 @@ namespace osu.Game.Skinning private readonly Dictionary maniaConfigurations = new Dictionary(); + private readonly DefaultLegacySkin legacyDefaultFallback; + [UsedImplicitly(ImplicitUseKindFlags.InstantiatedWithFixedConstructorSignature)] public LegacySkin(SkinInfo skin, IStorageResourceProvider resources) - : this(skin, new LegacySkinResourceStore(skin, resources.Files), resources, "skin.ini") + : this(skin, new LegacySkinResourceStore(skin, resources.Files), resources, "skin.ini", true) { } - protected LegacySkin(SkinInfo skin, [CanBeNull] IResourceStore storage, [CanBeNull] IStorageResourceProvider resources, string filename) + /// + /// Construct a new legacy skin instance. + /// + /// The model for this skin. + /// A storage for looking up files within this skin using user-facing filenames. + /// Access to raw game resources. + /// The user-facing filename of the configuration file to be parsed. Can accept an .osu or skin.ini file. + /// Whether lookups should fallback to the implementations if not provided locally. + protected LegacySkin(SkinInfo skin, [CanBeNull] IResourceStore storage, [CanBeNull] IStorageResourceProvider resources, string configurationFilename, bool fallbackToDefault = false) : base(skin, resources) { - using (var stream = storage?.GetStream(filename)) + this.fallbackToDefault = fallbackToDefault; + legacyDefaultFallback = new DefaultLegacySkin(storage, resources); + + using (var stream = storage?.GetStream(configurationFilename)) { if (stream != null) { From 1d30791ab0b8d747eea64b7ea4c5696c2945e1e5 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 31 May 2021 15:27:14 +0900 Subject: [PATCH 36/43] Add potential pathway for legacy lookups --- osu.Game/Skinning/LegacySkin.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/osu.Game/Skinning/LegacySkin.cs b/osu.Game/Skinning/LegacySkin.cs index 908ed37b6a..02d9c32281 100644 --- a/osu.Game/Skinning/LegacySkin.cs +++ b/osu.Game/Skinning/LegacySkin.cs @@ -142,7 +142,7 @@ namespace osu.Game.Skinning case LegacyManiaSkinConfigurationLookup maniaLookup: if (!AllowManiaSkin) - return null; + break; var result = lookupForMania(maniaLookup); if (result != null) @@ -157,7 +157,7 @@ namespace osu.Game.Skinning return genericLookup(lookup); } - return null; + return fallbackToDefault ? legacyDefaultFallback.GetConfig(lookup) : null; } private IBindable lookupForMania(LegacyManiaSkinConfigurationLookup maniaLookup) @@ -334,7 +334,7 @@ namespace osu.Game.Skinning { } - return null; + return fallbackToDefault ? legacyDefaultFallback.GetConfig(lookup) : null; } public override Drawable GetDrawableComponent(ISkinComponent component) @@ -516,7 +516,7 @@ namespace osu.Game.Skinning return sample; } - return null; + return fallbackToDefault ? legacyDefaultFallback.GetSample(sampleInfo) : null; } private IEnumerable getLegacyLookupNames(HitSampleInfo hitSample) From 88ed95e012ec2ea9609f8959285a5135c43ef6bb Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 31 May 2021 17:04:38 +0900 Subject: [PATCH 37/43] Add `FindProvider` lookup function --- .../Skinning/Legacy/ManiaLegacySkinTransformer.cs | 6 +++--- .../Skinning/Legacy/OsuLegacySkinTransformer.cs | 2 +- osu.Game/Skinning/ISkin.cs | 8 ++++++++ osu.Game/Skinning/LegacySkin.cs | 11 +++++++++++ osu.Game/Skinning/SkinProvidingContainer.cs | 8 ++++++++ 5 files changed, 31 insertions(+), 4 deletions(-) diff --git a/osu.Game.Rulesets.Mania/Skinning/Legacy/ManiaLegacySkinTransformer.cs b/osu.Game.Rulesets.Mania/Skinning/Legacy/ManiaLegacySkinTransformer.cs index 261b8b1fad..c137e2f7cb 100644 --- a/osu.Game.Rulesets.Mania/Skinning/Legacy/ManiaLegacySkinTransformer.cs +++ b/osu.Game.Rulesets.Mania/Skinning/Legacy/ManiaLegacySkinTransformer.cs @@ -69,10 +69,10 @@ namespace osu.Game.Rulesets.Mania.Skinning.Legacy private void sourceChanged() { - isLegacySkin = new Lazy(() => Source.GetConfig(LegacySkinConfiguration.LegacySetting.Version) != null); - hasKeyTexture = new Lazy(() => Source.GetAnimation( + isLegacySkin = new Lazy(() => Source.FindProvider(s => s.GetConfig(LegacySkinConfiguration.LegacySetting.Version) != null) != null); + hasKeyTexture = new Lazy(() => Source.FindProvider(s => s.GetAnimation( this.GetManiaSkinConfig(LegacyManiaSkinConfigurationLookups.KeyImage, 0)?.Value - ?? "mania-key1", true, true) != null); + ?? "mania-key1", true, true) != null) != null); } public override Drawable GetDrawableComponent(ISkinComponent component) diff --git a/osu.Game.Rulesets.Osu/Skinning/Legacy/OsuLegacySkinTransformer.cs b/osu.Game.Rulesets.Osu/Skinning/Legacy/OsuLegacySkinTransformer.cs index ffd4f78400..33dc59f30a 100644 --- a/osu.Game.Rulesets.Osu/Skinning/Legacy/OsuLegacySkinTransformer.cs +++ b/osu.Game.Rulesets.Osu/Skinning/Legacy/OsuLegacySkinTransformer.cs @@ -29,7 +29,7 @@ namespace osu.Game.Rulesets.Osu.Skinning.Legacy private void sourceChanged() { - hasHitCircle = new Lazy(() => Source.GetTexture("hitcircle") != null); + hasHitCircle = new Lazy(Source.FindProvider(s => s.GetTexture("hitcircle") != null) != null); } public override Drawable GetDrawableComponent(ISkinComponent component) diff --git a/osu.Game/Skinning/ISkin.cs b/osu.Game/Skinning/ISkin.cs index 73f7cf6d39..df346556fd 100644 --- a/osu.Game/Skinning/ISkin.cs +++ b/osu.Game/Skinning/ISkin.cs @@ -57,5 +57,13 @@ namespace osu.Game.Skinning /// A matching value boxed in an , or null if unavailable. [CanBeNull] IBindable GetConfig(TLookup lookup); + + /// + /// For the specified texture, find any potential skin that can fulfill the lookup. + /// This should be used for cases where subsequent lookups (for related components) need to occur on the same skin. + /// + /// The skin to be used for subsequent lookups, or null if none is available. + [CanBeNull] + ISkin FindProvider(Func lookupFunction) => lookupFunction(this) ? this : null; } } diff --git a/osu.Game/Skinning/LegacySkin.cs b/osu.Game/Skinning/LegacySkin.cs index 02d9c32281..5ce2d74c99 100644 --- a/osu.Game/Skinning/LegacySkin.cs +++ b/osu.Game/Skinning/LegacySkin.cs @@ -556,5 +556,16 @@ namespace osu.Game.Skinning Textures?.Dispose(); Samples?.Dispose(); } + + ISkin ISkin.FindProvider(Func lookupFunction) + { + if (lookupFunction(this)) + return this; + + if (!fallbackToDefault) + return null; + + return (legacyDefaultFallback as ISkin)?.FindProvider(lookupFunction); + } } } diff --git a/osu.Game/Skinning/SkinProvidingContainer.cs b/osu.Game/Skinning/SkinProvidingContainer.cs index cf22b2e820..c183cd62df 100644 --- a/osu.Game/Skinning/SkinProvidingContainer.cs +++ b/osu.Game/Skinning/SkinProvidingContainer.cs @@ -41,6 +41,14 @@ namespace osu.Game.Skinning RelativeSizeAxes = Axes.Both; } + public ISkin FindProvider(Func lookupFunction) + { + if (lookupFunction(skin)) + return skin; + + return fallbackSource.FindProvider(lookupFunction); + } + public Drawable GetDrawableComponent(ISkinComponent component) { Drawable sourceDrawable; From 8e489754cc4595a3bf95ec34b6024431af7b15f0 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 31 May 2021 17:09:30 +0900 Subject: [PATCH 38/43] Add ability for `LegacySkin`s to customise the fallback provider --- osu.Game/Skinning/DefaultLegacySkin.cs | 2 ++ osu.Game/Skinning/LegacySkin.cs | 26 ++++++++++++++++++-------- 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/osu.Game/Skinning/DefaultLegacySkin.cs b/osu.Game/Skinning/DefaultLegacySkin.cs index 30192182f3..1d17b5ce20 100644 --- a/osu.Game/Skinning/DefaultLegacySkin.cs +++ b/osu.Game/Skinning/DefaultLegacySkin.cs @@ -31,6 +31,8 @@ namespace osu.Game.Skinning Configuration.LegacyVersion = 2.7m; } + protected override DefaultLegacySkin CreateFallbackSkin(IResourceStore storage, IStorageResourceProvider resources) => null; + public static SkinInfo Info { get; } = new SkinInfo { ID = SkinInfo.CLASSIC_SKIN, // this is temporary until database storage is decided upon. diff --git a/osu.Game/Skinning/LegacySkin.cs b/osu.Game/Skinning/LegacySkin.cs index 5ce2d74c99..d249f63901 100644 --- a/osu.Game/Skinning/LegacySkin.cs +++ b/osu.Game/Skinning/LegacySkin.cs @@ -24,7 +24,7 @@ using osuTK.Graphics; namespace osu.Game.Skinning { - public class LegacySkin : Skin + public class LegacySkin : Skin, ISkin { private readonly bool fallbackToDefault; @@ -56,6 +56,7 @@ namespace osu.Game.Skinning private readonly Dictionary maniaConfigurations = new Dictionary(); + [CanBeNull] private readonly DefaultLegacySkin legacyDefaultFallback; [UsedImplicitly(ImplicitUseKindFlags.InstantiatedWithFixedConstructorSignature)] @@ -71,12 +72,12 @@ namespace osu.Game.Skinning /// A storage for looking up files within this skin using user-facing filenames. /// Access to raw game resources. /// The user-facing filename of the configuration file to be parsed. Can accept an .osu or skin.ini file. - /// Whether lookups should fallback to the implementations if not provided locally. + /// Whether lookups via fallback to the implementations if not provided locally. protected LegacySkin(SkinInfo skin, [CanBeNull] IResourceStore storage, [CanBeNull] IStorageResourceProvider resources, string configurationFilename, bool fallbackToDefault = false) : base(skin, resources) { this.fallbackToDefault = fallbackToDefault; - legacyDefaultFallback = new DefaultLegacySkin(storage, resources); + legacyDefaultFallback = CreateFallbackSkin(storage, resources); using (var stream = storage?.GetStream(configurationFilename)) { @@ -117,6 +118,10 @@ namespace osu.Game.Skinning true) != null); } + [CanBeNull] + protected virtual DefaultLegacySkin CreateFallbackSkin(IResourceStore storage, IStorageResourceProvider resources) => + new DefaultLegacySkin(storage, resources); + public override IBindable GetConfig(TLookup lookup) { switch (lookup) @@ -157,7 +162,7 @@ namespace osu.Game.Skinning return genericLookup(lookup); } - return fallbackToDefault ? legacyDefaultFallback.GetConfig(lookup) : null; + return legacyDefaultFallback?.GetConfig(lookup); } private IBindable lookupForMania(LegacyManiaSkinConfigurationLookup maniaLookup) @@ -334,7 +339,7 @@ namespace osu.Game.Skinning { } - return fallbackToDefault ? legacyDefaultFallback.GetConfig(lookup) : null; + return legacyDefaultFallback?.GetConfig(lookup); } public override Drawable GetDrawableComponent(ISkinComponent component) @@ -434,7 +439,12 @@ namespace osu.Game.Skinning break; } - return this.GetAnimation(component.LookupName, false, false); + var animation = this.GetAnimation(component.LookupName, false, false); + + if (animation != null) + return animation; + + return legacyDefaultFallback?.GetDrawableComponent(component); } private Texture getParticleTexture(HitResult result) @@ -494,7 +504,7 @@ namespace osu.Game.Skinning return texture; } - return null; + return legacyDefaultFallback?.GetTexture(componentName, wrapModeS, wrapModeT); } public override ISample GetSample(ISampleInfo sampleInfo) @@ -516,7 +526,7 @@ namespace osu.Game.Skinning return sample; } - return fallbackToDefault ? legacyDefaultFallback.GetSample(sampleInfo) : null; + return legacyDefaultFallback?.GetSample(sampleInfo); } private IEnumerable getLegacyLookupNames(HitSampleInfo hitSample) From 3ff9f9c89de524d0ecbffafcc1c13fbe02223ce7 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 31 May 2021 17:25:21 +0900 Subject: [PATCH 39/43] Make `FindProvider` non-default --- .../TestSceneCursorTrail.cs | 2 ++ .../TestSceneGameplayCursor.cs | 1 + .../TestSceneSkinFallbacks.cs | 1 + .../TestSceneHitObjectAccentColour.cs | 2 ++ .../Skinning/LegacySkinAnimationTest.cs | 1 + .../Skins/TestSceneSkinConfigurationLookup.cs | 3 ++ .../Gameplay/TestSceneSkinnableDrawable.cs | 6 ++++ .../Gameplay/TestSceneSkinnableSound.cs | 1 + osu.Game/Skinning/ISkin.cs | 3 +- osu.Game/Skinning/LegacyBeatmapSkin.cs | 12 ++++++- osu.Game/Skinning/LegacySkin.cs | 31 ++++++++----------- osu.Game/Skinning/LegacySkinTransformer.cs | 3 ++ osu.Game/Skinning/Skin.cs | 2 ++ osu.Game/Skinning/SkinManager.cs | 2 ++ 14 files changed, 50 insertions(+), 20 deletions(-) diff --git a/osu.Game.Rulesets.Osu.Tests/TestSceneCursorTrail.cs b/osu.Game.Rulesets.Osu.Tests/TestSceneCursorTrail.cs index 0ba97fac54..9997660c2d 100644 --- a/osu.Game.Rulesets.Osu.Tests/TestSceneCursorTrail.cs +++ b/osu.Game.Rulesets.Osu.Tests/TestSceneCursorTrail.cs @@ -102,6 +102,8 @@ namespace osu.Game.Rulesets.Osu.Tests public IBindable GetConfig(TLookup lookup) => null; + public ISkin FindProvider(Func lookupFunction) => null; + public event Action SourceChanged { add { } diff --git a/osu.Game.Rulesets.Osu.Tests/TestSceneGameplayCursor.cs b/osu.Game.Rulesets.Osu.Tests/TestSceneGameplayCursor.cs index 9a77292aff..76111342f0 100644 --- a/osu.Game.Rulesets.Osu.Tests/TestSceneGameplayCursor.cs +++ b/osu.Game.Rulesets.Osu.Tests/TestSceneGameplayCursor.cs @@ -113,6 +113,7 @@ namespace osu.Game.Rulesets.Osu.Tests public Drawable GetDrawableComponent(ISkinComponent component) => null; public Texture GetTexture(string componentName, WrapMode wrapModeS, WrapMode wrapModeT) => null; public ISample GetSample(ISampleInfo sampleInfo) => null; + public ISkin FindProvider(Func lookupFunction) => null; public IBindable GetConfig(TLookup lookup) { diff --git a/osu.Game.Rulesets.Osu.Tests/TestSceneSkinFallbacks.cs b/osu.Game.Rulesets.Osu.Tests/TestSceneSkinFallbacks.cs index 6c6f05c5c5..fd523fffcb 100644 --- a/osu.Game.Rulesets.Osu.Tests/TestSceneSkinFallbacks.cs +++ b/osu.Game.Rulesets.Osu.Tests/TestSceneSkinFallbacks.cs @@ -166,6 +166,7 @@ namespace osu.Game.Rulesets.Osu.Tests public TValue GetValue(Func query) where TConfiguration : SkinConfiguration => default; public IBindable GetConfig(TLookup lookup) => null; + public ISkin FindProvider(Func lookupFunction) => null; public event Action SourceChanged; diff --git a/osu.Game.Tests/Gameplay/TestSceneHitObjectAccentColour.cs b/osu.Game.Tests/Gameplay/TestSceneHitObjectAccentColour.cs index 883791c35c..76e5437305 100644 --- a/osu.Game.Tests/Gameplay/TestSceneHitObjectAccentColour.cs +++ b/osu.Game.Tests/Gameplay/TestSceneHitObjectAccentColour.cs @@ -123,6 +123,8 @@ namespace osu.Game.Tests.Gameplay public ISample GetSample(ISampleInfo sampleInfo) => throw new NotImplementedException(); + public ISkin FindProvider(Func lookupFunction) => null; + public IBindable GetConfig(TLookup lookup) { switch (lookup) diff --git a/osu.Game.Tests/NonVisual/Skinning/LegacySkinAnimationTest.cs b/osu.Game.Tests/NonVisual/Skinning/LegacySkinAnimationTest.cs index b08a228de3..e45b8f7dc5 100644 --- a/osu.Game.Tests/NonVisual/Skinning/LegacySkinAnimationTest.cs +++ b/osu.Game.Tests/NonVisual/Skinning/LegacySkinAnimationTest.cs @@ -61,6 +61,7 @@ namespace osu.Game.Tests.NonVisual.Skinning public Drawable GetDrawableComponent(ISkinComponent component) => throw new NotSupportedException(); public ISample GetSample(ISampleInfo sampleInfo) => throw new NotSupportedException(); public IBindable GetConfig(TLookup lookup) => throw new NotSupportedException(); + public ISkin FindProvider(Func lookupFunction) => null; } private class TestAnimationTimeReference : IAnimationTimeReference diff --git a/osu.Game.Tests/Skins/TestSceneSkinConfigurationLookup.cs b/osu.Game.Tests/Skins/TestSceneSkinConfigurationLookup.cs index 732a3f3f42..c15d804a19 100644 --- a/osu.Game.Tests/Skins/TestSceneSkinConfigurationLookup.cs +++ b/osu.Game.Tests/Skins/TestSceneSkinConfigurationLookup.cs @@ -1,6 +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; using System.Collections.Generic; using System.IO; using System.Linq; @@ -222,6 +223,8 @@ namespace osu.Game.Tests.Skins public ISample GetSample(ISampleInfo sampleInfo) => skin.GetSample(sampleInfo); public IBindable GetConfig(TLookup lookup) => skin.GetConfig(lookup); + + public ISkin FindProvider(Func lookupFunction) => skin.FindProvider(lookupFunction); } } } diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneSkinnableDrawable.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneSkinnableDrawable.cs index 7a6e2f54c2..bb45d568de 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneSkinnableDrawable.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneSkinnableDrawable.cs @@ -301,6 +301,8 @@ namespace osu.Game.Tests.Visual.Gameplay public ISample GetSample(ISampleInfo sampleInfo) => throw new NotImplementedException(); public IBindable GetConfig(TLookup lookup) => throw new NotImplementedException(); + + public ISkin FindProvider(Func lookupFunction) => throw new NotImplementedException(); } private class SecondarySource : ISkin @@ -312,6 +314,8 @@ namespace osu.Game.Tests.Visual.Gameplay public ISample GetSample(ISampleInfo sampleInfo) => throw new NotImplementedException(); public IBindable GetConfig(TLookup lookup) => throw new NotImplementedException(); + + public ISkin FindProvider(Func lookupFunction) => throw new NotImplementedException(); } [Cached(typeof(ISkinSource))] @@ -325,6 +329,8 @@ namespace osu.Game.Tests.Visual.Gameplay public IBindable GetConfig(TLookup lookup) => throw new NotImplementedException(); + public ISkin FindProvider(Func lookupFunction) => throw new NotImplementedException(); + public event Action SourceChanged { add { } diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneSkinnableSound.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneSkinnableSound.cs index d792405eeb..d69395fbaa 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneSkinnableSound.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneSkinnableSound.cs @@ -147,6 +147,7 @@ namespace osu.Game.Tests.Visual.Gameplay public Texture GetTexture(string componentName, WrapMode wrapModeS, WrapMode wrapModeT) => source?.GetTexture(componentName, wrapModeS, wrapModeT); public ISample GetSample(ISampleInfo sampleInfo) => source?.GetSample(sampleInfo); public IBindable GetConfig(TLookup lookup) => source?.GetConfig(lookup); + public ISkin FindProvider(Func lookupFunction) => source?.FindProvider(lookupFunction); public void TriggerSourceChanged() { diff --git a/osu.Game/Skinning/ISkin.cs b/osu.Game/Skinning/ISkin.cs index df346556fd..1c3598abb4 100644 --- a/osu.Game/Skinning/ISkin.cs +++ b/osu.Game/Skinning/ISkin.cs @@ -1,6 +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; using JetBrains.Annotations; using osu.Framework.Audio.Sample; using osu.Framework.Bindables; @@ -64,6 +65,6 @@ namespace osu.Game.Skinning /// /// The skin to be used for subsequent lookups, or null if none is available. [CanBeNull] - ISkin FindProvider(Func lookupFunction) => lookupFunction(this) ? this : null; + ISkin FindProvider(Func lookupFunction); } } diff --git a/osu.Game/Skinning/LegacyBeatmapSkin.cs b/osu.Game/Skinning/LegacyBeatmapSkin.cs index 6085eb1c37..0d6608f579 100644 --- a/osu.Game/Skinning/LegacyBeatmapSkin.cs +++ b/osu.Game/Skinning/LegacyBeatmapSkin.cs @@ -1,6 +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; using osu.Framework.Audio.Sample; using osu.Framework.Bindables; using osu.Framework.Graphics; @@ -18,7 +19,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, fallbackToDefault: false) + : 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; @@ -70,6 +71,15 @@ namespace osu.Game.Skinning return base.GetSample(sampleInfo); } + public override ISkin FindProvider(Func lookupFunction) + { + if (lookupFunction(this)) + return this; + + // beatmap skins don't do lookups on the default skin. this allows fallback to user / game default skins. + return null; + } + private static SkinInfo createSkinInfo(BeatmapInfo beatmap) => new SkinInfo { Name = beatmap.ToString(), Creator = beatmap.Metadata?.AuthorString }; } diff --git a/osu.Game/Skinning/LegacySkin.cs b/osu.Game/Skinning/LegacySkin.cs index d249f63901..856e795dc6 100644 --- a/osu.Game/Skinning/LegacySkin.cs +++ b/osu.Game/Skinning/LegacySkin.cs @@ -24,10 +24,8 @@ using osuTK.Graphics; namespace osu.Game.Skinning { - public class LegacySkin : Skin, ISkin + public class LegacySkin : Skin { - private readonly bool fallbackToDefault; - [CanBeNull] protected TextureStore Textures; @@ -61,7 +59,7 @@ namespace osu.Game.Skinning [UsedImplicitly(ImplicitUseKindFlags.InstantiatedWithFixedConstructorSignature)] public LegacySkin(SkinInfo skin, IStorageResourceProvider resources) - : this(skin, new LegacySkinResourceStore(skin, resources.Files), resources, "skin.ini", true) + : this(skin, new LegacySkinResourceStore(skin, resources.Files), resources, "skin.ini") { } @@ -72,11 +70,9 @@ namespace osu.Game.Skinning /// A storage for looking up files within this skin using user-facing filenames. /// Access to raw game resources. /// The user-facing filename of the configuration file to be parsed. Can accept an .osu or skin.ini file. - /// Whether lookups via fallback to the implementations if not provided locally. - protected LegacySkin(SkinInfo skin, [CanBeNull] IResourceStore storage, [CanBeNull] IStorageResourceProvider resources, string configurationFilename, bool fallbackToDefault = false) + protected LegacySkin(SkinInfo skin, [CanBeNull] IResourceStore storage, [CanBeNull] IStorageResourceProvider resources, string configurationFilename) : base(skin, resources) { - this.fallbackToDefault = fallbackToDefault; legacyDefaultFallback = CreateFallbackSkin(storage, resources); using (var stream = storage?.GetStream(configurationFilename)) @@ -529,6 +525,16 @@ namespace osu.Game.Skinning return legacyDefaultFallback?.GetSample(sampleInfo); } + public override ISkin FindProvider(Func lookupFunction) + { + var source = base.FindProvider(lookupFunction); + + if (source != null) + return source; + + return legacyDefaultFallback?.FindProvider(lookupFunction); + } + private IEnumerable getLegacyLookupNames(HitSampleInfo hitSample) { var lookupNames = hitSample.LookupNames.SelectMany(getFallbackNames); @@ -566,16 +572,5 @@ namespace osu.Game.Skinning Textures?.Dispose(); Samples?.Dispose(); } - - ISkin ISkin.FindProvider(Func lookupFunction) - { - if (lookupFunction(this)) - return this; - - if (!fallbackToDefault) - return null; - - return (legacyDefaultFallback as ISkin)?.FindProvider(lookupFunction); - } } } diff --git a/osu.Game/Skinning/LegacySkinTransformer.cs b/osu.Game/Skinning/LegacySkinTransformer.cs index ae8faf1a3b..cace4acf6c 100644 --- a/osu.Game/Skinning/LegacySkinTransformer.cs +++ b/osu.Game/Skinning/LegacySkinTransformer.cs @@ -1,6 +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; using osu.Framework.Audio.Sample; using osu.Framework.Bindables; using osu.Framework.Graphics; @@ -47,5 +48,7 @@ namespace osu.Game.Skinning } public abstract IBindable GetConfig(TLookup lookup); + + public ISkin FindProvider(Func lookupFunction) => Source.FindProvider(lookupFunction); } } diff --git a/osu.Game/Skinning/Skin.cs b/osu.Game/Skinning/Skin.cs index b6cb8fc7a4..c12e9a64c2 100644 --- a/osu.Game/Skinning/Skin.cs +++ b/osu.Game/Skinning/Skin.cs @@ -35,6 +35,8 @@ namespace osu.Game.Skinning public abstract IBindable GetConfig(TLookup lookup); + public virtual ISkin FindProvider(Func lookupFunction) => lookupFunction(this) ? this : null; + protected Skin(SkinInfo skin, IStorageResourceProvider resources) { SkinInfo = skin; diff --git a/osu.Game/Skinning/SkinManager.cs b/osu.Game/Skinning/SkinManager.cs index 079c537066..fa4f657882 100644 --- a/osu.Game/Skinning/SkinManager.cs +++ b/osu.Game/Skinning/SkinManager.cs @@ -212,6 +212,8 @@ namespace osu.Game.Skinning public IBindable GetConfig(TLookup lookup) => CurrentSkin.Value.GetConfig(lookup); + public ISkin FindProvider(Func lookupFunction) => CurrentSkin.Value.FindProvider(lookupFunction); + #region IResourceStorageProvider AudioManager IStorageResourceProvider.AudioManager => audio; From 282c5a917786cc99d08a178356ed971f039fbb35 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 31 May 2021 18:00:06 +0900 Subject: [PATCH 40/43] Fix potential nullref in `SkinProvidingContainer` --- osu.Game/Skinning/LegacySkin.cs | 6 +++--- osu.Game/Skinning/SkinProvidingContainer.cs | 9 ++++++--- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/osu.Game/Skinning/LegacySkin.cs b/osu.Game/Skinning/LegacySkin.cs index 856e795dc6..92944a9c93 100644 --- a/osu.Game/Skinning/LegacySkin.cs +++ b/osu.Game/Skinning/LegacySkin.cs @@ -73,7 +73,8 @@ namespace osu.Game.Skinning protected LegacySkin(SkinInfo skin, [CanBeNull] IResourceStore storage, [CanBeNull] IStorageResourceProvider resources, string configurationFilename) : base(skin, resources) { - legacyDefaultFallback = CreateFallbackSkin(storage, resources); + if (resources != null) + legacyDefaultFallback = CreateFallbackSkin(storage, resources); using (var stream = storage?.GetStream(configurationFilename)) { @@ -115,8 +116,7 @@ namespace osu.Game.Skinning } [CanBeNull] - protected virtual DefaultLegacySkin CreateFallbackSkin(IResourceStore storage, IStorageResourceProvider resources) => - new DefaultLegacySkin(storage, resources); + protected virtual DefaultLegacySkin CreateFallbackSkin(IResourceStore storage, IStorageResourceProvider resources) => new DefaultLegacySkin(resources); public override IBindable GetConfig(TLookup lookup) { diff --git a/osu.Game/Skinning/SkinProvidingContainer.cs b/osu.Game/Skinning/SkinProvidingContainer.cs index c183cd62df..863b5f5a24 100644 --- a/osu.Game/Skinning/SkinProvidingContainer.cs +++ b/osu.Game/Skinning/SkinProvidingContainer.cs @@ -2,6 +2,7 @@ // See the LICENCE file in the repository root for full licence text. using System; +using JetBrains.Annotations; using osu.Framework.Allocation; using osu.Framework.Audio.Sample; using osu.Framework.Bindables; @@ -20,8 +21,10 @@ namespace osu.Game.Skinning { public event Action SourceChanged; + [CanBeNull] private readonly ISkin skin; + [CanBeNull] private ISkinSource fallbackSource; protected virtual bool AllowDrawableLookup(ISkinComponent component) => true; @@ -43,10 +46,10 @@ namespace osu.Game.Skinning public ISkin FindProvider(Func lookupFunction) { - if (lookupFunction(skin)) + if (skin != null && lookupFunction(skin)) return skin; - return fallbackSource.FindProvider(lookupFunction); + return fallbackSource?.FindProvider(lookupFunction); } public Drawable GetDrawableComponent(ISkinComponent component) @@ -93,7 +96,7 @@ namespace osu.Game.Skinning { if (canUseSkinLookup) { - var bindable = skin.GetConfig(lookup); + var bindable = skin?.GetConfig(lookup); if (bindable != null) return bindable; } From 1161378b6bec0fd430a3734a0d5e852fec41298e Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 31 May 2021 20:15:26 +0900 Subject: [PATCH 41/43] Fix incorrect fallback logic in `LegacyBeatmapSkin` --- osu.Game/Skinning/LegacyBeatmapSkin.cs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/osu.Game/Skinning/LegacyBeatmapSkin.cs b/osu.Game/Skinning/LegacyBeatmapSkin.cs index 0d6608f579..2374cb976b 100644 --- a/osu.Game/Skinning/LegacyBeatmapSkin.cs +++ b/osu.Game/Skinning/LegacyBeatmapSkin.cs @@ -1,7 +1,6 @@ // 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 osu.Framework.Audio.Sample; using osu.Framework.Bindables; using osu.Framework.Graphics; @@ -71,12 +70,11 @@ namespace osu.Game.Skinning return base.GetSample(sampleInfo); } - public override ISkin FindProvider(Func lookupFunction) + protected override DefaultLegacySkin CreateFallbackSkin(IResourceStore storage, IStorageResourceProvider resources) { - if (lookupFunction(this)) - return this; - - // beatmap skins don't do lookups on the default skin. this allows fallback to user / game default skins. + // for simplicity, beatmap skins don't do lookups on the default skin. + // this will mean that fallback always occurs to the user (then default) skin. + // this may not offer perfect behaviour, but helps keep things simple. return null; } From 33577cbad5fe2f3a262877879ffdd9dcafc7753b Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 31 May 2021 20:43:01 +0900 Subject: [PATCH 42/43] Fix multiple issues with default lookups --- .../Skinning/Legacy/ManiaLegacySkinTransformer.cs | 4 ++-- .../Skinning/Legacy/OsuLegacySkinTransformer.cs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/osu.Game.Rulesets.Mania/Skinning/Legacy/ManiaLegacySkinTransformer.cs b/osu.Game.Rulesets.Mania/Skinning/Legacy/ManiaLegacySkinTransformer.cs index c137e2f7cb..8aa0c85433 100644 --- a/osu.Game.Rulesets.Mania/Skinning/Legacy/ManiaLegacySkinTransformer.cs +++ b/osu.Game.Rulesets.Mania/Skinning/Legacy/ManiaLegacySkinTransformer.cs @@ -69,8 +69,8 @@ namespace osu.Game.Rulesets.Mania.Skinning.Legacy private void sourceChanged() { - isLegacySkin = new Lazy(() => Source.FindProvider(s => s.GetConfig(LegacySkinConfiguration.LegacySetting.Version) != null) != null); - hasKeyTexture = new Lazy(() => Source.FindProvider(s => s.GetAnimation( + isLegacySkin = new Lazy(() => FindProvider(s => s.GetConfig(LegacySkinConfiguration.LegacySetting.Version) != null) != null); + hasKeyTexture = new Lazy(() => FindProvider(s => s.GetAnimation( this.GetManiaSkinConfig(LegacyManiaSkinConfigurationLookups.KeyImage, 0)?.Value ?? "mania-key1", true, true) != null) != null); } diff --git a/osu.Game.Rulesets.Osu/Skinning/Legacy/OsuLegacySkinTransformer.cs b/osu.Game.Rulesets.Osu/Skinning/Legacy/OsuLegacySkinTransformer.cs index 33dc59f30a..33693748d9 100644 --- a/osu.Game.Rulesets.Osu/Skinning/Legacy/OsuLegacySkinTransformer.cs +++ b/osu.Game.Rulesets.Osu/Skinning/Legacy/OsuLegacySkinTransformer.cs @@ -29,7 +29,7 @@ namespace osu.Game.Rulesets.Osu.Skinning.Legacy private void sourceChanged() { - hasHitCircle = new Lazy(Source.FindProvider(s => s.GetTexture("hitcircle") != null) != null); + hasHitCircle = new Lazy(() => FindProvider(s => s.GetTexture("hitcircle") != null) != null); } public override Drawable GetDrawableComponent(ISkinComponent component) From 69c4ccad05297772db8bc7cabc5dae759f789d92 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 31 May 2021 21:27:11 +0900 Subject: [PATCH 43/43] Fix weird taiko logic failing for weird reasons that probably should not have been a thing --- osu.Game/Skinning/LegacySkin.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/osu.Game/Skinning/LegacySkin.cs b/osu.Game/Skinning/LegacySkin.cs index 92944a9c93..f3f3e67eef 100644 --- a/osu.Game/Skinning/LegacySkin.cs +++ b/osu.Game/Skinning/LegacySkin.cs @@ -563,7 +563,11 @@ namespace osu.Game.Skinning // Fall back to using the last piece for components coming from lazer (e.g. "Gameplay/osu/approachcircle" -> "approachcircle"). string lastPiece = componentName.Split('/').Last(); - yield return componentName.StartsWith("Gameplay/taiko/", StringComparison.Ordinal) ? "taiko-" + lastPiece : lastPiece; + + if (componentName.StartsWith("Gameplay/taiko/", StringComparison.Ordinal)) + yield return "taiko-" + lastPiece; + + yield return lastPiece; } protected override void Dispose(bool isDisposing)