From 4fd89faaa459d26ed3467675ad391d10c08dc1da Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 27 May 2021 11:48:48 +0900 Subject: [PATCH 01/89] Fix default skin not having resources or providing samples --- osu.Game/Skinning/DefaultSkin.cs | 15 ++++++++++++++- osu.Game/Skinning/SkinManager.cs | 4 +++- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/osu.Game/Skinning/DefaultSkin.cs b/osu.Game/Skinning/DefaultSkin.cs index ba31816a07..a17a052b97 100644 --- a/osu.Game/Skinning/DefaultSkin.cs +++ b/osu.Game/Skinning/DefaultSkin.cs @@ -22,6 +22,8 @@ namespace osu.Game.Skinning { public class DefaultSkin : Skin { + private readonly IStorageResourceProvider resources; + public DefaultSkin(IStorageResourceProvider resources) : this(SkinInfo.Default, resources) { @@ -31,12 +33,23 @@ namespace osu.Game.Skinning public DefaultSkin(SkinInfo skin, IStorageResourceProvider resources) : base(skin, resources) { + this.resources = resources; Configuration = new DefaultSkinConfiguration(); } public override Texture GetTexture(string componentName, WrapMode wrapModeS, WrapMode wrapModeT) => null; - public override ISample GetSample(ISampleInfo sampleInfo) => null; + public override ISample GetSample(ISampleInfo sampleInfo) + { + foreach (var lookup in sampleInfo.LookupNames) + { + var sample = resources.AudioManager.Samples.Get(lookup); + if (sample != null) + return sample; + } + + return null; + } public override Drawable GetDrawableComponent(ISkinComponent component) { diff --git a/osu.Game/Skinning/SkinManager.cs b/osu.Game/Skinning/SkinManager.cs index 5793edda30..66c776d32d 100644 --- a/osu.Game/Skinning/SkinManager.cs +++ b/osu.Game/Skinning/SkinManager.cs @@ -39,7 +39,7 @@ namespace osu.Game.Skinning private readonly IResourceStore legacyDefaultResources; - public readonly Bindable CurrentSkin = new Bindable(new DefaultSkin(null)); + public readonly Bindable CurrentSkin = new Bindable(); public readonly Bindable CurrentSkinInfo = new Bindable(SkinInfo.Default) { Default = SkinInfo.Default }; public override IEnumerable HandledExtensions => new[] { ".osk" }; @@ -56,6 +56,8 @@ namespace osu.Game.Skinning this.legacyDefaultResources = legacyDefaultResources; + CurrentSkin.Value = new DefaultSkin(this); + CurrentSkinInfo.ValueChanged += skin => CurrentSkin.Value = GetSkin(skin.NewValue); CurrentSkin.ValueChanged += skin => { From 70a844ac10409380692e05343f21b2afee5f9229 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 27 May 2021 14:50:42 +0900 Subject: [PATCH 02/89] Remove `allowFallback` parameters completely --- .../UI/CatchComboDisplay.cs | 4 +-- osu.Game.Rulesets.Catch/UI/Catcher.cs | 4 +-- .../UI/Components/HitObjectArea.cs | 4 +-- .../Objects/Drawables/SkinnableLighting.cs | 4 +-- osu.Game.Rulesets.Osu/UI/Cursor/OsuCursor.cs | 2 +- .../Gameplay/TestSceneSkinnableDrawable.cs | 31 +++++++++---------- osu.Game/Skinning/PoolableSkinnableSample.cs | 13 ++------ osu.Game/Skinning/SkinManager.cs | 4 +-- osu.Game/Skinning/SkinReloadableDrawable.cs | 21 ++----------- osu.Game/Skinning/SkinnableDrawable.cs | 13 +++----- osu.Game/Skinning/SkinnableSprite.cs | 5 ++- osu.Game/Skinning/SkinnableSpriteText.cs | 8 ++--- osu.Game/Skinning/SkinnableTargetContainer.cs | 4 +-- .../Drawables/DrawableStoryboardSample.cs | 4 +-- 14 files changed, 45 insertions(+), 76 deletions(-) diff --git a/osu.Game.Rulesets.Catch/UI/CatchComboDisplay.cs b/osu.Game.Rulesets.Catch/UI/CatchComboDisplay.cs index 75feb21298..ad344ff2dd 100644 --- a/osu.Game.Rulesets.Catch/UI/CatchComboDisplay.cs +++ b/osu.Game.Rulesets.Catch/UI/CatchComboDisplay.cs @@ -25,9 +25,9 @@ namespace osu.Game.Rulesets.Catch.UI { } - protected override void SkinChanged(ISkinSource skin, bool allowFallback) + protected override void SkinChanged(ISkinSource skin) { - base.SkinChanged(skin, allowFallback); + base.SkinChanged(skin); ComboCounter?.UpdateCombo(currentCombo); } diff --git a/osu.Game.Rulesets.Catch/UI/Catcher.cs b/osu.Game.Rulesets.Catch/UI/Catcher.cs index 0d6a577d1e..b8c4d8f036 100644 --- a/osu.Game.Rulesets.Catch/UI/Catcher.cs +++ b/osu.Game.Rulesets.Catch/UI/Catcher.cs @@ -399,9 +399,9 @@ namespace osu.Game.Rulesets.Catch.UI private void updateTrailVisibility() => trails.DisplayTrail = Dashing || HyperDashing; - protected override void SkinChanged(ISkinSource skin, bool allowFallback) + protected override void SkinChanged(ISkinSource skin) { - base.SkinChanged(skin, allowFallback); + base.SkinChanged(skin); hyperDashColour = skin.GetConfig(CatchSkinColour.HyperDash)?.Value ?? diff --git a/osu.Game.Rulesets.Mania/UI/Components/HitObjectArea.cs b/osu.Game.Rulesets.Mania/UI/Components/HitObjectArea.cs index 8f7880dafa..b75b586ecf 100644 --- a/osu.Game.Rulesets.Mania/UI/Components/HitObjectArea.cs +++ b/osu.Game.Rulesets.Mania/UI/Components/HitObjectArea.cs @@ -33,9 +33,9 @@ namespace osu.Game.Rulesets.Mania.UI.Components Direction.BindValueChanged(onDirectionChanged, true); } - protected override void SkinChanged(ISkinSource skin, bool allowFallback) + protected override void SkinChanged(ISkinSource skin) { - base.SkinChanged(skin, allowFallback); + base.SkinChanged(skin); UpdateHitPosition(); } diff --git a/osu.Game.Rulesets.Osu/Objects/Drawables/SkinnableLighting.cs b/osu.Game.Rulesets.Osu/Objects/Drawables/SkinnableLighting.cs index 02dc770285..c72080c9e5 100644 --- a/osu.Game.Rulesets.Osu/Objects/Drawables/SkinnableLighting.cs +++ b/osu.Game.Rulesets.Osu/Objects/Drawables/SkinnableLighting.cs @@ -18,9 +18,9 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables { } - protected override void SkinChanged(ISkinSource skin, bool allowFallback) + protected override void SkinChanged(ISkinSource skin) { - base.SkinChanged(skin, allowFallback); + base.SkinChanged(skin); updateColour(); } diff --git a/osu.Game.Rulesets.Osu/UI/Cursor/OsuCursor.cs b/osu.Game.Rulesets.Osu/UI/Cursor/OsuCursor.cs index eea45c6c80..0e7d7cdcf3 100644 --- a/osu.Game.Rulesets.Osu/UI/Cursor/OsuCursor.cs +++ b/osu.Game.Rulesets.Osu/UI/Cursor/OsuCursor.cs @@ -31,7 +31,7 @@ namespace osu.Game.Rulesets.Osu.UI.Cursor Size = new Vector2(size); } - protected override void SkinChanged(ISkinSource skin, bool allowFallback) + protected override void SkinChanged(ISkinSource skin) { cursorExpand = skin.GetConfig(OsuSkinConfiguration.CursorExpand)?.Value ?? true; } diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneSkinnableDrawable.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneSkinnableDrawable.cs index 7a6e2f54c2..38da7f7104 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneSkinnableDrawable.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneSkinnableDrawable.cs @@ -42,9 +42,9 @@ namespace osu.Game.Tests.Visual.Gameplay Spacing = new Vector2(10), Children = new[] { - new ExposedSkinnableDrawable("default", _ => new DefaultBox(), _ => true), - new ExposedSkinnableDrawable("available", _ => new DefaultBox(), _ => true), - new ExposedSkinnableDrawable("available", _ => new DefaultBox(), _ => true, ConfineMode.NoScaling) + new ExposedSkinnableDrawable("default", _ => new DefaultBox()), + new ExposedSkinnableDrawable("available", _ => new DefaultBox()), + new ExposedSkinnableDrawable("available", _ => new DefaultBox(), ConfineMode.NoScaling) } }, }; @@ -73,9 +73,9 @@ namespace osu.Game.Tests.Visual.Gameplay Spacing = new Vector2(10), Children = new[] { - new ExposedSkinnableDrawable("default", _ => new DefaultBox(), _ => true), - new ExposedSkinnableDrawable("available", _ => new DefaultBox(), _ => true, ConfineMode.ScaleToFit), - new ExposedSkinnableDrawable("available", _ => new DefaultBox(), _ => true, ConfineMode.NoScaling) + new ExposedSkinnableDrawable("default", _ => new DefaultBox()), + new ExposedSkinnableDrawable("available", _ => new DefaultBox(), ConfineMode.ScaleToFit), + new ExposedSkinnableDrawable("available", _ => new DefaultBox(), ConfineMode.NoScaling) } }, }; @@ -100,7 +100,7 @@ namespace osu.Game.Tests.Visual.Gameplay Child = new SkinProvidingContainer(secondarySource) { RelativeSizeAxes = Axes.Both, - Child = consumer = new SkinConsumer("test", name => new NamedBox("Default Implementation"), source => true) + Child = consumer = new SkinConsumer("test", name => new NamedBox("Default Implementation")) } }; }); @@ -129,7 +129,7 @@ namespace osu.Game.Tests.Visual.Gameplay }; }); - AddStep("add permissive", () => target.Add(consumer = new SkinConsumer("test", name => new NamedBox("Default Implementation"), source => true))); + AddStep("add permissive", () => target.Add(consumer = new SkinConsumer("test", name => new NamedBox("Default Implementation")))); AddAssert("consumer using override source", () => consumer.Drawable is SecondarySourceBox); AddAssert("skinchanged only called once", () => consumer.SkinChangedCount == 1); } @@ -152,7 +152,7 @@ namespace osu.Game.Tests.Visual.Gameplay }; }); - AddStep("add permissive", () => target.Add(consumer = new SkinConsumer("test", name => new NamedBox("Default Implementation"), source => true))); + AddStep("add permissive", () => target.Add(consumer = new SkinConsumer("test", name => new NamedBox("Default Implementation")))); AddAssert("consumer using override source", () => consumer.Drawable is SecondarySourceBox); AddStep("disable", () => target.Disable()); AddAssert("consumer using base source", () => consumer.Drawable is BaseSourceBox); @@ -180,9 +180,8 @@ namespace osu.Game.Tests.Visual.Gameplay { public new Drawable Drawable => base.Drawable; - public ExposedSkinnableDrawable(string name, Func defaultImplementation, Func allowFallback = null, - ConfineMode confineMode = ConfineMode.ScaleToFit) - : base(new TestSkinComponent(name), defaultImplementation, allowFallback, confineMode) + public ExposedSkinnableDrawable(string name, Func defaultImplementation, ConfineMode confineMode = ConfineMode.ScaleToFit) + : base(new TestSkinComponent(name), defaultImplementation, confineMode) { } } @@ -250,14 +249,14 @@ namespace osu.Game.Tests.Visual.Gameplay public new Drawable Drawable => base.Drawable; public int SkinChangedCount { get; private set; } - public SkinConsumer(string name, Func defaultImplementation, Func allowFallback = null) - : base(new TestSkinComponent(name), defaultImplementation, allowFallback) + public SkinConsumer(string name, Func defaultImplementation) + : base(new TestSkinComponent(name), defaultImplementation) { } - protected override void SkinChanged(ISkinSource skin, bool allowFallback) + protected override void SkinChanged(ISkinSource skin) { - base.SkinChanged(skin, allowFallback); + base.SkinChanged(skin); SkinChangedCount++; } } diff --git a/osu.Game/Skinning/PoolableSkinnableSample.cs b/osu.Game/Skinning/PoolableSkinnableSample.cs index b04158a58f..33e8c137f4 100644 --- a/osu.Game/Skinning/PoolableSkinnableSample.cs +++ b/osu.Game/Skinning/PoolableSkinnableSample.cs @@ -70,9 +70,9 @@ namespace osu.Game.Skinning updateSample(); } - protected override void SkinChanged(ISkinSource skin, bool allowFallback) + protected override void SkinChanged(ISkinSource skin) { - base.SkinChanged(skin, allowFallback); + base.SkinChanged(skin); updateSample(); } @@ -88,15 +88,6 @@ namespace osu.Game.Skinning var sample = CurrentSkin.GetSample(sampleInfo); - if (sample == null && AllowDefaultFallback) - { - foreach (var lookup in sampleInfo.LookupNames) - { - if ((sample = sampleStore.Get(lookup)) != null) - break; - } - } - if (sample == null) return; diff --git a/osu.Game/Skinning/SkinManager.cs b/osu.Game/Skinning/SkinManager.cs index 66c776d32d..503c99d023 100644 --- a/osu.Game/Skinning/SkinManager.cs +++ b/osu.Game/Skinning/SkinManager.cs @@ -56,9 +56,9 @@ namespace osu.Game.Skinning this.legacyDefaultResources = legacyDefaultResources; - CurrentSkin.Value = new DefaultSkin(this); - CurrentSkinInfo.ValueChanged += skin => CurrentSkin.Value = GetSkin(skin.NewValue); + + CurrentSkin.Value = new DefaultSkin(this); CurrentSkin.ValueChanged += skin => { if (skin.NewValue.SkinInfo != CurrentSkinInfo.Value) diff --git a/osu.Game/Skinning/SkinReloadableDrawable.cs b/osu.Game/Skinning/SkinReloadableDrawable.cs index 50b4143375..dec546b82d 100644 --- a/osu.Game/Skinning/SkinReloadableDrawable.cs +++ b/osu.Game/Skinning/SkinReloadableDrawable.cs @@ -22,22 +22,6 @@ namespace osu.Game.Skinning /// protected ISkinSource CurrentSkin { get; private set; } - private readonly Func allowFallback; - - /// - /// Whether fallback to default skin should be allowed if the custom skin is missing this resource. - /// - protected bool AllowDefaultFallback => allowFallback == null || allowFallback.Invoke(CurrentSkin); - - /// - /// Create a new - /// - /// A conditional to decide whether to allow fallback to the default implementation if a skinned element is not present. - protected SkinReloadableDrawable(Func allowFallback = null) - { - this.allowFallback = allowFallback; - } - [BackgroundDependencyLoader] private void load(ISkinSource source) { @@ -58,7 +42,7 @@ namespace osu.Game.Skinning private void skinChanged() { - SkinChanged(CurrentSkin, AllowDefaultFallback); + SkinChanged(CurrentSkin); OnSkinChanged?.Invoke(); } @@ -66,8 +50,7 @@ namespace osu.Game.Skinning /// Called when a change is made to the skin. /// /// The new skin. - /// Whether fallback to default skin should be allowed if the custom skin is missing this resource. - protected virtual void SkinChanged(ISkinSource skin, bool allowFallback) + protected virtual void SkinChanged(ISkinSource skin) { } diff --git a/osu.Game/Skinning/SkinnableDrawable.cs b/osu.Game/Skinning/SkinnableDrawable.cs index fc2730ca44..72f64e2e12 100644 --- a/osu.Game/Skinning/SkinnableDrawable.cs +++ b/osu.Game/Skinning/SkinnableDrawable.cs @@ -40,17 +40,14 @@ namespace osu.Game.Skinning /// /// The namespace-complete resource name for this skinnable element. /// A function to create the default skin implementation of this element. - /// A conditional to decide whether to allow fallback to the default implementation if a skinned element is not present. /// How (if at all) the should be resize to fit within our own bounds. - public SkinnableDrawable(ISkinComponent component, Func defaultImplementation = null, Func allowFallback = null, - ConfineMode confineMode = ConfineMode.NoScaling) - : this(component, allowFallback, confineMode) + public SkinnableDrawable(ISkinComponent component, Func defaultImplementation = null, ConfineMode confineMode = ConfineMode.NoScaling) + : this(component, confineMode) { createDefault = defaultImplementation; } - protected SkinnableDrawable(ISkinComponent component, Func allowFallback = null, ConfineMode confineMode = ConfineMode.NoScaling) - : base(allowFallback) + protected SkinnableDrawable(ISkinComponent component, ConfineMode confineMode = ConfineMode.NoScaling) { this.component = component; this.confineMode = confineMode; @@ -76,13 +73,13 @@ namespace osu.Game.Skinning /// protected virtual bool ApplySizeRestrictionsToDefault => false; - protected override void SkinChanged(ISkinSource skin, bool allowFallback) + protected override void SkinChanged(ISkinSource skin) { Drawable = skin.GetDrawableComponent(component); isDefault = false; - if (Drawable == null && allowFallback) + if (Drawable == null) { Drawable = CreateDefault(component); isDefault = true; diff --git a/osu.Game/Skinning/SkinnableSprite.cs b/osu.Game/Skinning/SkinnableSprite.cs index 1340d1474c..56e576d081 100644 --- a/osu.Game/Skinning/SkinnableSprite.cs +++ b/osu.Game/Skinning/SkinnableSprite.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.Allocation; using osu.Framework.Graphics; using osu.Framework.Graphics.Sprites; @@ -19,8 +18,8 @@ namespace osu.Game.Skinning [Resolved] private TextureStore textures { get; set; } - public SkinnableSprite(string textureName, Func allowFallback = null, ConfineMode confineMode = ConfineMode.NoScaling) - : base(new SpriteComponent(textureName), allowFallback, confineMode) + public SkinnableSprite(string textureName, ConfineMode confineMode = ConfineMode.NoScaling) + : base(new SpriteComponent(textureName), confineMode) { } diff --git a/osu.Game/Skinning/SkinnableSpriteText.cs b/osu.Game/Skinning/SkinnableSpriteText.cs index 06461127b1..2bde3c4180 100644 --- a/osu.Game/Skinning/SkinnableSpriteText.cs +++ b/osu.Game/Skinning/SkinnableSpriteText.cs @@ -9,14 +9,14 @@ namespace osu.Game.Skinning { public class SkinnableSpriteText : SkinnableDrawable, IHasText { - public SkinnableSpriteText(ISkinComponent component, Func defaultImplementation, Func allowFallback = null, ConfineMode confineMode = ConfineMode.NoScaling) - : base(component, defaultImplementation, allowFallback, confineMode) + public SkinnableSpriteText(ISkinComponent component, Func defaultImplementation, ConfineMode confineMode = ConfineMode.NoScaling) + : base(component, defaultImplementation, confineMode) { } - protected override void SkinChanged(ISkinSource skin, bool allowFallback) + protected override void SkinChanged(ISkinSource skin) { - base.SkinChanged(skin, allowFallback); + base.SkinChanged(skin); if (Drawable is IHasText textDrawable) textDrawable.Text = Text; diff --git a/osu.Game/Skinning/SkinnableTargetContainer.cs b/osu.Game/Skinning/SkinnableTargetContainer.cs index d454e199dc..1338462dd6 100644 --- a/osu.Game/Skinning/SkinnableTargetContainer.cs +++ b/osu.Game/Skinning/SkinnableTargetContainer.cs @@ -73,9 +73,9 @@ namespace osu.Game.Skinning components.Remove(component); } - protected override void SkinChanged(ISkinSource skin, bool allowFallback) + protected override void SkinChanged(ISkinSource skin) { - base.SkinChanged(skin, allowFallback); + base.SkinChanged(skin); Reload(); } diff --git a/osu.Game/Storyboards/Drawables/DrawableStoryboardSample.cs b/osu.Game/Storyboards/Drawables/DrawableStoryboardSample.cs index fbdd27e762..672274a2ad 100644 --- a/osu.Game/Storyboards/Drawables/DrawableStoryboardSample.cs +++ b/osu.Game/Storyboards/Drawables/DrawableStoryboardSample.cs @@ -31,9 +31,9 @@ namespace osu.Game.Storyboards.Drawables [Resolved] private IBindable> mods { get; set; } - protected override void SkinChanged(ISkinSource skin, bool allowFallback) + protected override void SkinChanged(ISkinSource skin) { - base.SkinChanged(skin, allowFallback); + base.SkinChanged(skin); foreach (var mod in mods.Value.OfType()) { From b13b732e02e3efb0a7a58cf96b03e1bf7d9a0d05 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 27 May 2021 14:50:56 +0900 Subject: [PATCH 03/89] Remove incorrect `DefaultSkin` usage --- osu.Game/Beatmaps/WorkingBeatmap.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Beatmaps/WorkingBeatmap.cs b/osu.Game/Beatmaps/WorkingBeatmap.cs index 3576b149bf..73a167b7aa 100644 --- a/osu.Game/Beatmaps/WorkingBeatmap.cs +++ b/osu.Game/Beatmaps/WorkingBeatmap.cs @@ -324,7 +324,7 @@ namespace osu.Game.Beatmaps public bool SkinLoaded => skin.IsResultAvailable; public ISkin Skin => skin.Value; - protected virtual ISkin GetSkin() => new DefaultSkin(null); + protected virtual ISkin GetSkin() => null; private readonly RecyclableLazy skin; public abstract Stream GetStream(string storagePath); From c39ea857012ca12f6591b737cfb212cffdf71a4a Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 27 May 2021 15:39:35 +0900 Subject: [PATCH 04/89] Fix `TestSceneSkinnableSound` not doing DI correctly --- .../Visual/Gameplay/TestSceneSkinnableSound.cs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneSkinnableSound.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneSkinnableSound.cs index d792405eeb..0e3d22ff1d 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneSkinnableSound.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneSkinnableSound.cs @@ -29,14 +29,13 @@ namespace osu.Game.Tests.Visual.Gameplay { AddStep("setup hierarchy", () => { - Children = new Drawable[] + Child = skinSource = new TestSkinSourceContainer { - skinSource = new TestSkinSourceContainer - { - RelativeSizeAxes = Axes.Both, - Child = skinnableSound = new PausableSkinnableSound(new SampleInfo("Gameplay/normal-sliderslide")) - }, + RelativeSizeAxes = Axes.Both, }; + + // has to be added after the hierarchy above else the `ISkinSource` dependency won't be cached. + skinSource.Add(skinnableSound = new PausableSkinnableSound(new SampleInfo("Gameplay/normal-sliderslide"))); }); } From 4b27d43e26b3983bd7a60630f997f3317edecc26 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 31 May 2021 15:13:56 +0900 Subject: [PATCH 05/89] 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 06/89] 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 07/89] 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 08/89] 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 09/89] 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 10/89] 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 11/89] 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 12/89] 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 13/89] 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) From ff815cb4b4f461f55d6154682ccd5ce0213ce80d Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 1 Jun 2021 16:57:40 +0900 Subject: [PATCH 14/89] Fix incorrect xmldoc --- osu.Game/Skinning/ISkin.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Skinning/ISkin.cs b/osu.Game/Skinning/ISkin.cs index 1c3598abb4..09e79a5ff5 100644 --- a/osu.Game/Skinning/ISkin.cs +++ b/osu.Game/Skinning/ISkin.cs @@ -60,7 +60,7 @@ namespace osu.Game.Skinning IBindable GetConfig(TLookup lookup); /// - /// For the specified texture, find any potential skin that can fulfill the lookup. + /// Find the first (if any) 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. From df0a5689e462c31046a917ac3f44e57505becdc1 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 1 Jun 2021 17:13:13 +0900 Subject: [PATCH 15/89] Revert "Fix weird taiko logic failing for weird reasons that probably should not have been a thing" This reverts commit 69c4ccad05297772db8bc7cabc5dae759f789d92. --- osu.Game/Skinning/LegacySkin.cs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/osu.Game/Skinning/LegacySkin.cs b/osu.Game/Skinning/LegacySkin.cs index f3f3e67eef..92944a9c93 100644 --- a/osu.Game/Skinning/LegacySkin.cs +++ b/osu.Game/Skinning/LegacySkin.cs @@ -563,11 +563,7 @@ 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(); - - if (componentName.StartsWith("Gameplay/taiko/", StringComparison.Ordinal)) - yield return "taiko-" + lastPiece; - - yield return lastPiece; + yield return componentName.StartsWith("Gameplay/taiko/", StringComparison.Ordinal) ? "taiko-" + lastPiece : lastPiece; } protected override void Dispose(bool isDisposing) From 83bfd36498dcb30d2ad8e22184ab56504a9968d4 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 1 Jun 2021 17:56:12 +0900 Subject: [PATCH 16/89] Disable broken taiko hitsound fallback tests for now --- .../TestSceneTaikoHitObjectSamples.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game.Rulesets.Taiko.Tests/TestSceneTaikoHitObjectSamples.cs b/osu.Game.Rulesets.Taiko.Tests/TestSceneTaikoHitObjectSamples.cs index 221d715a35..7318a6d929 100644 --- a/osu.Game.Rulesets.Taiko.Tests/TestSceneTaikoHitObjectSamples.cs +++ b/osu.Game.Rulesets.Taiko.Tests/TestSceneTaikoHitObjectSamples.cs @@ -16,7 +16,7 @@ namespace osu.Game.Rulesets.Taiko.Tests [TestCase("taiko-normal-hitnormal")] [TestCase("normal-hitnormal")] - [TestCase("hitnormal")] + // [TestCase("hitnormal")] intentionally broken (will play classic default instead). public void TestDefaultCustomSampleFromBeatmap(string expectedSample) { SetupSkins(expectedSample, expectedSample); @@ -28,7 +28,7 @@ namespace osu.Game.Rulesets.Taiko.Tests [TestCase("taiko-normal-hitnormal")] [TestCase("normal-hitnormal")] - [TestCase("hitnormal")] + // [TestCase("hitnormal")] intentionally broken (will play classic default instead). public void TestDefaultCustomSampleFromUserSkinFallback(string expectedSample) { SetupSkins(string.Empty, expectedSample); From a837fc9e3ba99c315e3fd29c13692c7c159965c9 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 1 Jun 2021 18:00:24 +0900 Subject: [PATCH 17/89] Remove duplicated taiko fallback --- osu.Game/Skinning/LegacySkin.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/osu.Game/Skinning/LegacySkin.cs b/osu.Game/Skinning/LegacySkin.cs index 92944a9c93..98cc5c8fd8 100644 --- a/osu.Game/Skinning/LegacySkin.cs +++ b/osu.Game/Skinning/LegacySkin.cs @@ -562,8 +562,7 @@ namespace osu.Game.Skinning yield return componentName; // 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; + yield return componentName.Split('/').Last(); } protected override void Dispose(bool isDisposing) From ea4644be905f602ae2dd9ed068808f568b900363 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 1 Jun 2021 17:13:13 +0900 Subject: [PATCH 18/89] Revert "Fix weird taiko logic failing for weird reasons that probably should not have been a thing" This reverts commit 69c4ccad05297772db8bc7cabc5dae759f789d92. --- osu.Game/Skinning/LegacySkin.cs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/osu.Game/Skinning/LegacySkin.cs b/osu.Game/Skinning/LegacySkin.cs index f3f3e67eef..92944a9c93 100644 --- a/osu.Game/Skinning/LegacySkin.cs +++ b/osu.Game/Skinning/LegacySkin.cs @@ -563,11 +563,7 @@ 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(); - - if (componentName.StartsWith("Gameplay/taiko/", StringComparison.Ordinal)) - yield return "taiko-" + lastPiece; - - yield return lastPiece; + yield return componentName.StartsWith("Gameplay/taiko/", StringComparison.Ordinal) ? "taiko-" + lastPiece : lastPiece; } protected override void Dispose(bool isDisposing) From dd006401b3d1bcbe41f56b907178ba4d42a691ae Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 1 Jun 2021 17:56:12 +0900 Subject: [PATCH 19/89] Disable broken taiko hitsound fallback tests for now --- .../TestSceneTaikoHitObjectSamples.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game.Rulesets.Taiko.Tests/TestSceneTaikoHitObjectSamples.cs b/osu.Game.Rulesets.Taiko.Tests/TestSceneTaikoHitObjectSamples.cs index 221d715a35..7318a6d929 100644 --- a/osu.Game.Rulesets.Taiko.Tests/TestSceneTaikoHitObjectSamples.cs +++ b/osu.Game.Rulesets.Taiko.Tests/TestSceneTaikoHitObjectSamples.cs @@ -16,7 +16,7 @@ namespace osu.Game.Rulesets.Taiko.Tests [TestCase("taiko-normal-hitnormal")] [TestCase("normal-hitnormal")] - [TestCase("hitnormal")] + // [TestCase("hitnormal")] intentionally broken (will play classic default instead). public void TestDefaultCustomSampleFromBeatmap(string expectedSample) { SetupSkins(expectedSample, expectedSample); @@ -28,7 +28,7 @@ namespace osu.Game.Rulesets.Taiko.Tests [TestCase("taiko-normal-hitnormal")] [TestCase("normal-hitnormal")] - [TestCase("hitnormal")] + // [TestCase("hitnormal")] intentionally broken (will play classic default instead). public void TestDefaultCustomSampleFromUserSkinFallback(string expectedSample) { SetupSkins(string.Empty, expectedSample); From 3a6d081d82c20fc587a529783c1495b662d2d8c0 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 1 Jun 2021 18:00:24 +0900 Subject: [PATCH 20/89] Remove duplicated taiko fallback --- osu.Game/Skinning/LegacySkin.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/osu.Game/Skinning/LegacySkin.cs b/osu.Game/Skinning/LegacySkin.cs index 92944a9c93..98cc5c8fd8 100644 --- a/osu.Game/Skinning/LegacySkin.cs +++ b/osu.Game/Skinning/LegacySkin.cs @@ -562,8 +562,7 @@ namespace osu.Game.Skinning yield return componentName; // 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; + yield return componentName.Split('/').Last(); } protected override void Dispose(bool isDisposing) From 54338bdcc5666a60395ee478a817d4aa145a7a18 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 1 Jun 2021 18:29:15 +0900 Subject: [PATCH 21/89] Add test ensuring correct osu! ruleset sample lookups --- ...u-hitobject-beatmap-custom-sample-bank.osu | 10 ++++ .../TestSceneOsuHitObjectSamples.cs | 49 +++++++++++++++++++ 2 files changed, 59 insertions(+) create mode 100644 osu.Game.Rulesets.Osu.Tests/Resources/SampleLookups/osu-hitobject-beatmap-custom-sample-bank.osu create mode 100644 osu.Game.Rulesets.Osu.Tests/TestSceneOsuHitObjectSamples.cs diff --git a/osu.Game.Rulesets.Osu.Tests/Resources/SampleLookups/osu-hitobject-beatmap-custom-sample-bank.osu b/osu.Game.Rulesets.Osu.Tests/Resources/SampleLookups/osu-hitobject-beatmap-custom-sample-bank.osu new file mode 100644 index 0000000000..a84fc08bb8 --- /dev/null +++ b/osu.Game.Rulesets.Osu.Tests/Resources/SampleLookups/osu-hitobject-beatmap-custom-sample-bank.osu @@ -0,0 +1,10 @@ +osu file format v14 + +[General] +Mode: 0 + +[TimingPoints] +0,300,4,1,2,100,1,0 + +[HitObjects] +444,320,1000,5,0,0:0:0:0: diff --git a/osu.Game.Rulesets.Osu.Tests/TestSceneOsuHitObjectSamples.cs b/osu.Game.Rulesets.Osu.Tests/TestSceneOsuHitObjectSamples.cs new file mode 100644 index 0000000000..e8d98ce3b8 --- /dev/null +++ b/osu.Game.Rulesets.Osu.Tests/TestSceneOsuHitObjectSamples.cs @@ -0,0 +1,49 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System.Reflection; +using NUnit.Framework; +using osu.Framework.IO.Stores; +using osu.Game.Tests.Beatmaps; + +namespace osu.Game.Rulesets.Osu.Tests +{ + public class TestSceneOsuHitObjectSamples : HitObjectSampleTest + { + protected override Ruleset CreatePlayerRuleset() => new OsuRuleset(); + + protected override IResourceStore RulesetResources => new DllResourceStore(Assembly.GetAssembly(typeof(TestSceneOsuHitObjectSamples))); + + [TestCase("normal-hitnormal")] + [TestCase("hitnormal")] + public void TestDefaultCustomSampleFromBeatmap(string expectedSample) + { + SetupSkins(expectedSample, expectedSample); + + CreateTestWithBeatmap("osu-hitobject-beatmap-custom-sample-bank.osu"); + + AssertBeatmapLookup(expectedSample); + } + + [TestCase("normal-hitnormal")] + [TestCase("hitnormal")] + public void TestDefaultCustomSampleFromUserSkinFallback(string expectedSample) + { + SetupSkins(string.Empty, expectedSample); + + CreateTestWithBeatmap("osu-hitobject-beatmap-custom-sample-bank.osu"); + + AssertUserLookup(expectedSample); + } + + [TestCase("normal-hitnormal2")] + public void TestUserSkinLookupIgnoresSampleBank(string unwantedSample) + { + SetupSkins(string.Empty, unwantedSample); + + CreateTestWithBeatmap("osu-hitobject-beatmap-custom-sample-bank.osu"); + + AssertNoLookup(unwantedSample); + } + } +} From 2e2281c7d22345164d314464341138d3d544f250 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 1 Jun 2021 18:56:22 +0900 Subject: [PATCH 22/89] Revert disabling taiko sample tests and fix logic --- .../TestSceneTaikoHitObjectSamples.cs | 4 ++-- .../Legacy/TaikoLegacySkinTransformer.cs | 23 +++++++++++-------- osu.Game/Skinning/LegacySkin.cs | 2 ++ 3 files changed, 17 insertions(+), 12 deletions(-) diff --git a/osu.Game.Rulesets.Taiko.Tests/TestSceneTaikoHitObjectSamples.cs b/osu.Game.Rulesets.Taiko.Tests/TestSceneTaikoHitObjectSamples.cs index 7318a6d929..221d715a35 100644 --- a/osu.Game.Rulesets.Taiko.Tests/TestSceneTaikoHitObjectSamples.cs +++ b/osu.Game.Rulesets.Taiko.Tests/TestSceneTaikoHitObjectSamples.cs @@ -16,7 +16,7 @@ namespace osu.Game.Rulesets.Taiko.Tests [TestCase("taiko-normal-hitnormal")] [TestCase("normal-hitnormal")] - // [TestCase("hitnormal")] intentionally broken (will play classic default instead). + [TestCase("hitnormal")] public void TestDefaultCustomSampleFromBeatmap(string expectedSample) { SetupSkins(expectedSample, expectedSample); @@ -28,7 +28,7 @@ namespace osu.Game.Rulesets.Taiko.Tests [TestCase("taiko-normal-hitnormal")] [TestCase("normal-hitnormal")] - // [TestCase("hitnormal")] intentionally broken (will play classic default instead). + [TestCase("hitnormal")] public void TestDefaultCustomSampleFromUserSkinFallback(string expectedSample) { SetupSkins(string.Empty, expectedSample); diff --git a/osu.Game.Rulesets.Taiko/Skinning/Legacy/TaikoLegacySkinTransformer.cs b/osu.Game.Rulesets.Taiko/Skinning/Legacy/TaikoLegacySkinTransformer.cs index e0557c8617..7ce0f6b93b 100644 --- a/osu.Game.Rulesets.Taiko/Skinning/Legacy/TaikoLegacySkinTransformer.cs +++ b/osu.Game.Rulesets.Taiko/Skinning/Legacy/TaikoLegacySkinTransformer.cs @@ -152,32 +152,35 @@ namespace osu.Game.Rulesets.Taiko.Skinning.Legacy throw new ArgumentOutOfRangeException(nameof(component), $"Invalid component type: {component}"); } - public override ISample GetSample(ISampleInfo sampleInfo) => Source.GetSample(new LegacyTaikoSampleInfo(sampleInfo)); + public override ISample GetSample(ISampleInfo sampleInfo) + { + if (sampleInfo is HitSampleInfo hitSampleInfo) + return Source.GetSample(new LegacyTaikoSampleInfo(hitSampleInfo)); + + return base.GetSample(sampleInfo); + } public override IBindable GetConfig(TLookup lookup) => Source.GetConfig(lookup); - private class LegacyTaikoSampleInfo : ISampleInfo + private class LegacyTaikoSampleInfo : HitSampleInfo { - private readonly ISampleInfo source; + public LegacyTaikoSampleInfo(HitSampleInfo sampleInfo) + : base(sampleInfo.Name, sampleInfo.Bank, sampleInfo.Suffix, sampleInfo.Volume) - public LegacyTaikoSampleInfo(ISampleInfo source) { - this.source = source; } - public IEnumerable LookupNames + public override IEnumerable LookupNames { get { - foreach (var name in source.LookupNames) + foreach (var name in base.LookupNames) yield return name.Insert(name.LastIndexOf('/') + 1, "taiko-"); - foreach (var name in source.LookupNames) + foreach (var name in base.LookupNames) yield return name; } } - - public int Volume => source.Volume; } } } diff --git a/osu.Game/Skinning/LegacySkin.cs b/osu.Game/Skinning/LegacySkin.cs index 98cc5c8fd8..474ac1a794 100644 --- a/osu.Game/Skinning/LegacySkin.cs +++ b/osu.Game/Skinning/LegacySkin.cs @@ -519,7 +519,9 @@ namespace osu.Game.Skinning var sample = Samples?.Get(lookup); if (sample != null) + { return sample; + } } return legacyDefaultFallback?.GetSample(sampleInfo); From 37c8c63fc566c115aa3f974b6d23bb12f62caa05 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 4 Jun 2021 16:18:04 +0900 Subject: [PATCH 23/89] Ensure all frames in an animation are retrieved from the same skin --- osu.Game/Skinning/LegacySkinExtensions.cs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/osu.Game/Skinning/LegacySkinExtensions.cs b/osu.Game/Skinning/LegacySkinExtensions.cs index d8fb1fa664..6e8276c01e 100644 --- a/osu.Game/Skinning/LegacySkinExtensions.cs +++ b/osu.Game/Skinning/LegacySkinExtensions.cs @@ -54,9 +54,16 @@ namespace osu.Game.Skinning IEnumerable getTextures() { + ISkin lookupSource = null; + for (int i = 0; true; i++) { - if ((texture = source.GetTexture($"{componentName}{animationSeparator}{i}", wrapModeS, wrapModeT)) == null) + string frameName = $"{componentName}{animationSeparator}{i}"; + + // ensure all textures are retrieved from the same skin source. + lookupSource ??= source.FindProvider(s => s.GetTexture(frameName, wrapModeS, wrapModeT) != null); + + if ((texture = lookupSource?.GetTexture(frameName, wrapModeS, wrapModeT)) == null) break; yield return texture; From ae2165b3be51770a8fe8da1eaefb730e6a75e6de Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 1 Jun 2021 16:57:40 +0900 Subject: [PATCH 24/89] Fix incorrect xmldoc --- osu.Game/Skinning/ISkin.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Skinning/ISkin.cs b/osu.Game/Skinning/ISkin.cs index 1c3598abb4..09e79a5ff5 100644 --- a/osu.Game/Skinning/ISkin.cs +++ b/osu.Game/Skinning/ISkin.cs @@ -60,7 +60,7 @@ namespace osu.Game.Skinning IBindable GetConfig(TLookup lookup); /// - /// For the specified texture, find any potential skin that can fulfill the lookup. + /// Find the first (if any) 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. From 9f2a9608f2acbb7c4169bdc6b0c2e9cc7c4aaa84 Mon Sep 17 00:00:00 2001 From: Pasi4K5 Date: Fri, 4 Jun 2021 16:17:54 +0200 Subject: [PATCH 25/89] Rework slider positioning --- osu.Game.Rulesets.Osu/Mods/OsuModRandom.cs | 87 ++++++++++++---------- 1 file changed, 47 insertions(+), 40 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Mods/OsuModRandom.cs b/osu.Game.Rulesets.Osu/Mods/OsuModRandom.cs index 4dfadbb835..f8572cc28b 100644 --- a/osu.Game.Rulesets.Osu/Mods/OsuModRandom.cs +++ b/osu.Game.Rulesets.Osu/Mods/OsuModRandom.cs @@ -23,6 +23,8 @@ namespace osu.Game.Rulesets.Osu.Mods public override string Description => "It never gets boring!"; public override bool Ranked => false; + private const float slider_path_checking_rate = 10; + // The relative distance to the edge of the playfield before objects' positions should start to "turn around" and curve towards the middle. // The closer the hit objects draw to the border, the sharper the turn private const float playfield_edge_ratio = 0.375f; @@ -74,22 +76,8 @@ namespace osu.Game.Rulesets.Osu.Mods // update end position as it may have changed as a result of the position update. current.EndPositionRandomised = current.PositionRandomised; - switch (hitObject) - { - case Slider slider: - // Shift nested objects the same distance as the slider got shifted in the randomisation process - // so that moveSliderIntoPlayfield() can determine their relative distances to slider.Position and thus minMargin - shiftNestedObjects(slider, Vector2.Subtract(slider.Position, current.PositionOriginal)); - - var oldPos = new Vector2(slider.Position.X, slider.Position.Y); - - moveSliderIntoPlayfield(slider, current); - - // Shift them again to move them to their final position after the slider got moved into the playfield - shiftNestedObjects(slider, Vector2.Subtract(slider.Position, oldPos)); - - break; - } + if (hitObject is Slider slider) + moveSliderIntoPlayfield(slider, current); previous = current; } @@ -146,34 +134,53 @@ namespace osu.Game.Rulesets.Osu.Mods /// private void moveSliderIntoPlayfield(Slider slider, RandomObjectInfo currentObjectInfo) { - // Min. distances from the slider's position to the playfield border - var minMargin = new MarginPadding(); + var minMargin = getMinSliderMargin(slider); - foreach (var hitObject in slider.NestedHitObjects.Where(o => o is SliderTick || o is SliderEndCircle)) - { - if (!(hitObject is OsuHitObject osuHitObject)) - continue; - - var relativePos = Vector2.Subtract(osuHitObject.Position, slider.Position); - - minMargin.Left = Math.Max(minMargin.Left, -relativePos.X); - minMargin.Right = Math.Max(minMargin.Right, relativePos.X); - minMargin.Top = Math.Max(minMargin.Top, -relativePos.Y); - minMargin.Bottom = Math.Max(minMargin.Bottom, relativePos.Y); - } - - if (slider.Position.X < minMargin.Left) - slider.Position = new Vector2(minMargin.Left, slider.Position.Y); - else if (slider.Position.X + minMargin.Right > OsuPlayfield.BASE_SIZE.X) - slider.Position = new Vector2(OsuPlayfield.BASE_SIZE.X - minMargin.Right, slider.Position.Y); - - if (slider.Position.Y < minMargin.Top) - slider.Position = new Vector2(slider.Position.X, minMargin.Top); - else if (slider.Position.Y + minMargin.Bottom > OsuPlayfield.BASE_SIZE.Y) - slider.Position = new Vector2(slider.Position.X, OsuPlayfield.BASE_SIZE.Y - minMargin.Bottom); + slider.Position = new Vector2( + Math.Clamp(slider.Position.X, minMargin.Left, OsuPlayfield.BASE_SIZE.X - minMargin.Right), + Math.Clamp(slider.Position.Y, minMargin.Top, OsuPlayfield.BASE_SIZE.Y - minMargin.Bottom) + ); currentObjectInfo.PositionRandomised = slider.Position; currentObjectInfo.EndPositionRandomised = slider.EndPosition; + + shiftNestedObjects(slider, Vector2.Subtract(currentObjectInfo.PositionRandomised, currentObjectInfo.PositionOriginal)); + } + + /// + /// Calculates the min. distances from the 's position to the playfield border for the slider to be fully inside of the playfield. + /// + private MarginPadding getMinSliderMargin(Slider slider) + { + var minMargin = new MarginPadding(); + Vector2 pos; + + for (double j = 0; j <= 1; j += 1 / (slider_path_checking_rate / 1000 * (slider.EndTime - slider.StartTime))) + { + pos = slider.Path.PositionAt(j); + updateMargin(); + } + + var repeat = (SliderRepeat)slider.NestedHitObjects.FirstOrDefault(o => o is SliderRepeat); + + if (repeat != null) + { + pos = repeat.Position - slider.Position; + updateMargin(); + } + + pos = slider.Path.PositionAt(1); + updateMargin(); + + return minMargin; + + void updateMargin() + { + minMargin.Left = Math.Max(minMargin.Left, -pos.X); + minMargin.Right = Math.Max(minMargin.Right, pos.X); + minMargin.Top = Math.Max(minMargin.Top, -pos.Y); + minMargin.Bottom = Math.Max(minMargin.Bottom, pos.Y); + } } /// From a0a6f3ef81021df70ccccbc792d6b2253b55e4f9 Mon Sep 17 00:00:00 2001 From: Pasi4K5 Date: Fri, 4 Jun 2021 16:23:03 +0200 Subject: [PATCH 26/89] Replace `Vector2` methods with math operators --- osu.Game.Rulesets.Osu/Mods/OsuModRandom.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Mods/OsuModRandom.cs b/osu.Game.Rulesets.Osu/Mods/OsuModRandom.cs index f8572cc28b..3525eddd2c 100644 --- a/osu.Game.Rulesets.Osu/Mods/OsuModRandom.cs +++ b/osu.Game.Rulesets.Osu/Mods/OsuModRandom.cs @@ -32,7 +32,7 @@ namespace osu.Game.Rulesets.Osu.Mods private static readonly float border_distance_x = OsuPlayfield.BASE_SIZE.X * playfield_edge_ratio; private static readonly float border_distance_y = OsuPlayfield.BASE_SIZE.Y * playfield_edge_ratio; - private static readonly Vector2 playfield_middle = Vector2.Divide(OsuPlayfield.BASE_SIZE, 2); + private static readonly Vector2 playfield_middle = OsuPlayfield.BASE_SIZE / 2; private static readonly float playfield_diagonal = OsuPlayfield.BASE_SIZE.LengthFast; @@ -119,7 +119,7 @@ namespace osu.Game.Rulesets.Osu.Mods current.AngleRad = (float)Math.Atan2(posRelativeToPrev.Y, posRelativeToPrev.X); - var position = Vector2.Add(previous.EndPositionRandomised, posRelativeToPrev); + var position = previous.EndPositionRandomised + posRelativeToPrev; // Move hit objects back into the playfield if they are outside of it, // which would sometimes happen during big jumps otherwise. @@ -144,7 +144,7 @@ namespace osu.Game.Rulesets.Osu.Mods currentObjectInfo.PositionRandomised = slider.Position; currentObjectInfo.EndPositionRandomised = slider.EndPosition; - shiftNestedObjects(slider, Vector2.Subtract(currentObjectInfo.PositionRandomised, currentObjectInfo.PositionOriginal)); + shiftNestedObjects(slider, currentObjectInfo.PositionRandomised - currentObjectInfo.PositionOriginal); } /// @@ -195,7 +195,7 @@ namespace osu.Game.Rulesets.Osu.Mods if (!(hitObject is OsuHitObject osuHitObject)) continue; - osuHitObject.Position = Vector2.Add(osuHitObject.Position, shift); + osuHitObject.Position += shift; } } From 6357d1363c36b2ece628648ea98d93c9b232ec13 Mon Sep 17 00:00:00 2001 From: Pasi4K5 Date: Fri, 4 Jun 2021 16:26:40 +0200 Subject: [PATCH 27/89] Add comment for `slider_path_checking_rate` --- osu.Game.Rulesets.Osu/Mods/OsuModRandom.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/osu.Game.Rulesets.Osu/Mods/OsuModRandom.cs b/osu.Game.Rulesets.Osu/Mods/OsuModRandom.cs index 3525eddd2c..e5c48ca96e 100644 --- a/osu.Game.Rulesets.Osu/Mods/OsuModRandom.cs +++ b/osu.Game.Rulesets.Osu/Mods/OsuModRandom.cs @@ -23,6 +23,7 @@ namespace osu.Game.Rulesets.Osu.Mods public override string Description => "It never gets boring!"; public override bool Ranked => false; + // How often per second getMinSliderMargin() checks if the slider is outside of the playfield private const float slider_path_checking_rate = 10; // The relative distance to the edge of the playfield before objects' positions should start to "turn around" and curve towards the middle. From 32e41048ff0a482263f30a177ada14a8fe8925ae Mon Sep 17 00:00:00 2001 From: Pasi4K5 Date: Fri, 4 Jun 2021 16:50:27 +0200 Subject: [PATCH 28/89] Fix `System.ArgumentException` caused by sliders bigger than the playfield --- osu.Game.Rulesets.Osu/Mods/OsuModRandom.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/osu.Game.Rulesets.Osu/Mods/OsuModRandom.cs b/osu.Game.Rulesets.Osu/Mods/OsuModRandom.cs index e5c48ca96e..6181b2257e 100644 --- a/osu.Game.Rulesets.Osu/Mods/OsuModRandom.cs +++ b/osu.Game.Rulesets.Osu/Mods/OsuModRandom.cs @@ -173,6 +173,9 @@ namespace osu.Game.Rulesets.Osu.Mods pos = slider.Path.PositionAt(1); updateMargin(); + minMargin.Left = Math.Min(minMargin.Left, OsuPlayfield.BASE_SIZE.X - minMargin.Right); + minMargin.Top = Math.Min(minMargin.Top, OsuPlayfield.BASE_SIZE.Y - minMargin.Bottom); + return minMargin; void updateMargin() From b4f190c6ff51a4e1b0fbd3d2e44606d24bb7a847 Mon Sep 17 00:00:00 2001 From: Pasi4K5 Date: Fri, 4 Jun 2021 17:22:36 +0200 Subject: [PATCH 29/89] Rename iteration variable --- osu.Game.Rulesets.Osu/Mods/OsuModRandom.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Mods/OsuModRandom.cs b/osu.Game.Rulesets.Osu/Mods/OsuModRandom.cs index 6181b2257e..79f821a8ef 100644 --- a/osu.Game.Rulesets.Osu/Mods/OsuModRandom.cs +++ b/osu.Game.Rulesets.Osu/Mods/OsuModRandom.cs @@ -156,9 +156,9 @@ namespace osu.Game.Rulesets.Osu.Mods var minMargin = new MarginPadding(); Vector2 pos; - for (double j = 0; j <= 1; j += 1 / (slider_path_checking_rate / 1000 * (slider.EndTime - slider.StartTime))) + for (double i = 0; i <= 1; i += 1 / (slider_path_checking_rate / 1000 * (slider.EndTime - slider.StartTime))) { - pos = slider.Path.PositionAt(j); + pos = slider.Path.PositionAt(i); updateMargin(); } From f59263932a1a74984658bd61df832b1724190c6d Mon Sep 17 00:00:00 2001 From: Pasi4K5 Date: Sat, 5 Jun 2021 17:04:58 +0200 Subject: [PATCH 30/89] Use `SliderPath.GetPathToProgress` for getting the `SliderPath`'s positions --- osu.Game.Rulesets.Osu/Mods/OsuModRandom.cs | 25 +++++++--------------- 1 file changed, 8 insertions(+), 17 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Mods/OsuModRandom.cs b/osu.Game.Rulesets.Osu/Mods/OsuModRandom.cs index 79f821a8ef..2d61c64fcb 100644 --- a/osu.Game.Rulesets.Osu/Mods/OsuModRandom.cs +++ b/osu.Game.Rulesets.Osu/Mods/OsuModRandom.cs @@ -2,6 +2,7 @@ // See the LICENCE file in the repository root for full licence text. using System; +using System.Collections.Generic; using System.Linq; using osu.Framework.Graphics; using osu.Framework.Utils; @@ -23,9 +24,6 @@ namespace osu.Game.Rulesets.Osu.Mods public override string Description => "It never gets boring!"; public override bool Ranked => false; - // How often per second getMinSliderMargin() checks if the slider is outside of the playfield - private const float slider_path_checking_rate = 10; - // The relative distance to the edge of the playfield before objects' positions should start to "turn around" and curve towards the middle. // The closer the hit objects draw to the border, the sharper the turn private const float playfield_edge_ratio = 0.375f; @@ -154,31 +152,24 @@ namespace osu.Game.Rulesets.Osu.Mods private MarginPadding getMinSliderMargin(Slider slider) { var minMargin = new MarginPadding(); - Vector2 pos; - for (double i = 0; i <= 1; i += 1 / (slider_path_checking_rate / 1000 * (slider.EndTime - slider.StartTime))) - { - pos = slider.Path.PositionAt(i); - updateMargin(); - } + var pathPositions = new List(); + slider.Path.GetPathToProgress(pathPositions, 0, 1); + + foreach (var pos in pathPositions) + updateMargin(pos); var repeat = (SliderRepeat)slider.NestedHitObjects.FirstOrDefault(o => o is SliderRepeat); if (repeat != null) - { - pos = repeat.Position - slider.Position; - updateMargin(); - } - - pos = slider.Path.PositionAt(1); - updateMargin(); + updateMargin(repeat.Position - slider.Position); minMargin.Left = Math.Min(minMargin.Left, OsuPlayfield.BASE_SIZE.X - minMargin.Right); minMargin.Top = Math.Min(minMargin.Top, OsuPlayfield.BASE_SIZE.Y - minMargin.Bottom); return minMargin; - void updateMargin() + void updateMargin(Vector2 pos) { minMargin.Left = Math.Max(minMargin.Left, -pos.X); minMargin.Right = Math.Max(minMargin.Right, pos.X); From b214f2ae0e39a08e6763c8943d9e63991b996568 Mon Sep 17 00:00:00 2001 From: Pasi4K5 Date: Sat, 5 Jun 2021 17:13:08 +0200 Subject: [PATCH 31/89] Remove `repeat` and simplify `getMinSliderMargin` --- osu.Game.Rulesets.Osu/Mods/OsuModRandom.cs | 22 +++++++--------------- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Mods/OsuModRandom.cs b/osu.Game.Rulesets.Osu/Mods/OsuModRandom.cs index 2d61c64fcb..c282a919ea 100644 --- a/osu.Game.Rulesets.Osu/Mods/OsuModRandom.cs +++ b/osu.Game.Rulesets.Osu/Mods/OsuModRandom.cs @@ -151,31 +151,23 @@ namespace osu.Game.Rulesets.Osu.Mods /// private MarginPadding getMinSliderMargin(Slider slider) { - var minMargin = new MarginPadding(); - var pathPositions = new List(); slider.Path.GetPathToProgress(pathPositions, 0, 1); + var minMargin = new MarginPadding(); + foreach (var pos in pathPositions) - updateMargin(pos); - - var repeat = (SliderRepeat)slider.NestedHitObjects.FirstOrDefault(o => o is SliderRepeat); - - if (repeat != null) - updateMargin(repeat.Position - slider.Position); - - minMargin.Left = Math.Min(minMargin.Left, OsuPlayfield.BASE_SIZE.X - minMargin.Right); - minMargin.Top = Math.Min(minMargin.Top, OsuPlayfield.BASE_SIZE.Y - minMargin.Bottom); - - return minMargin; - - void updateMargin(Vector2 pos) { minMargin.Left = Math.Max(minMargin.Left, -pos.X); minMargin.Right = Math.Max(minMargin.Right, pos.X); minMargin.Top = Math.Max(minMargin.Top, -pos.Y); minMargin.Bottom = Math.Max(minMargin.Bottom, pos.Y); } + + minMargin.Left = Math.Min(minMargin.Left, OsuPlayfield.BASE_SIZE.X - minMargin.Right); + minMargin.Top = Math.Min(minMargin.Top, OsuPlayfield.BASE_SIZE.Y - minMargin.Bottom); + + return minMargin; } /// From 39f99bf785676c443f37248668eb9bf45006032e Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sun, 6 Jun 2021 11:08:54 +0900 Subject: [PATCH 32/89] Move `FindProvider` to `ISkinSource` --- osu.Game/Skinning/ISkin.cs | 9 ------- osu.Game/Skinning/ISkinSource.cs | 9 +++++++ osu.Game/Skinning/LegacySkin.cs | 27 +++++-------------- osu.Game/Skinning/Skin.cs | 2 -- osu.Game/Skinning/SkinManager.cs | 2 +- .../Beatmaps/LegacyBeatmapSkinColourTest.cs | 2 ++ 6 files changed, 18 insertions(+), 33 deletions(-) diff --git a/osu.Game/Skinning/ISkin.cs b/osu.Game/Skinning/ISkin.cs index 09e79a5ff5..73f7cf6d39 100644 --- a/osu.Game/Skinning/ISkin.cs +++ b/osu.Game/Skinning/ISkin.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 JetBrains.Annotations; using osu.Framework.Audio.Sample; using osu.Framework.Bindables; @@ -58,13 +57,5 @@ namespace osu.Game.Skinning /// A matching value boxed in an , or null if unavailable. [CanBeNull] IBindable GetConfig(TLookup lookup); - - /// - /// Find the first (if any) 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); } } diff --git a/osu.Game/Skinning/ISkinSource.cs b/osu.Game/Skinning/ISkinSource.cs index 337d2a87a4..c7ebe91d64 100644 --- a/osu.Game/Skinning/ISkinSource.cs +++ b/osu.Game/Skinning/ISkinSource.cs @@ -2,6 +2,7 @@ // See the LICENCE file in the repository root for full licence text. using System; +using JetBrains.Annotations; namespace osu.Game.Skinning { @@ -11,5 +12,13 @@ namespace osu.Game.Skinning public interface ISkinSource : ISkin { event Action SourceChanged; + + /// + /// Find the first (if any) 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); } } diff --git a/osu.Game/Skinning/LegacySkin.cs b/osu.Game/Skinning/LegacySkin.cs index d2d7cc4d86..8f1895883d 100644 --- a/osu.Game/Skinning/LegacySkin.cs +++ b/osu.Game/Skinning/LegacySkin.cs @@ -54,9 +54,6 @@ namespace osu.Game.Skinning private readonly Dictionary maniaConfigurations = new Dictionary(); - [CanBeNull] - private readonly DefaultLegacySkin legacyDefaultFallback; - [UsedImplicitly(ImplicitUseKindFlags.InstantiatedWithFixedConstructorSignature)] public LegacySkin(SkinInfo skin, IStorageResourceProvider resources) : this(skin, new LegacySkinResourceStore(skin, resources.Files), resources, "skin.ini") @@ -73,9 +70,6 @@ namespace osu.Game.Skinning protected LegacySkin(SkinInfo skin, [CanBeNull] IResourceStore storage, [CanBeNull] IStorageResourceProvider resources, string configurationFilename) : base(skin, resources) { - if (resources != null) - legacyDefaultFallback = CreateFallbackSkin(storage, resources); - using (var stream = storage?.GetStream(configurationFilename)) { if (stream != null) @@ -158,7 +152,7 @@ namespace osu.Game.Skinning return genericLookup(lookup); } - return legacyDefaultFallback?.GetConfig(lookup); + return null; } private IBindable lookupForMania(LegacyManiaSkinConfigurationLookup maniaLookup) @@ -335,7 +329,7 @@ namespace osu.Game.Skinning { } - return legacyDefaultFallback?.GetConfig(lookup); + return null; } public override Drawable GetDrawableComponent(ISkinComponent component) @@ -406,6 +400,7 @@ namespace osu.Game.Skinning return null; case GameplaySkinComponent resultComponent: + // TODO: this should be inside the judgement pieces. Func createDrawable = () => getJudgementAnimation(resultComponent.Component); // kind of wasteful that we throw this away, but should do for now. @@ -427,7 +422,7 @@ namespace osu.Game.Skinning if (animation != null) return animation; - return legacyDefaultFallback?.GetDrawableComponent(component); + return null; } private Texture getParticleTexture(HitResult result) @@ -487,7 +482,7 @@ namespace osu.Game.Skinning return texture; } - return legacyDefaultFallback?.GetTexture(componentName, wrapModeS, wrapModeT); + return null; } public override ISample GetSample(ISampleInfo sampleInfo) @@ -511,17 +506,7 @@ 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); + return null; } private IEnumerable getLegacyLookupNames(HitSampleInfo hitSample) diff --git a/osu.Game/Skinning/Skin.cs b/osu.Game/Skinning/Skin.cs index c12e9a64c2..b6cb8fc7a4 100644 --- a/osu.Game/Skinning/Skin.cs +++ b/osu.Game/Skinning/Skin.cs @@ -35,8 +35,6 @@ 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 fa4f657882..d373618232 100644 --- a/osu.Game/Skinning/SkinManager.cs +++ b/osu.Game/Skinning/SkinManager.cs @@ -212,7 +212,7 @@ namespace osu.Game.Skinning public IBindable GetConfig(TLookup lookup) => CurrentSkin.Value.GetConfig(lookup); - public ISkin FindProvider(Func lookupFunction) => CurrentSkin.Value.FindProvider(lookupFunction); + public ISkin FindProvider(Func lookupFunction) => lookupFunction(CurrentSkin.Value) ? CurrentSkin.Value : null; #region IResourceStorageProvider diff --git a/osu.Game/Tests/Beatmaps/LegacyBeatmapSkinColourTest.cs b/osu.Game/Tests/Beatmaps/LegacyBeatmapSkinColourTest.cs index 0a7fb1483d..2540b6d7da 100644 --- a/osu.Game/Tests/Beatmaps/LegacyBeatmapSkinColourTest.cs +++ b/osu.Game/Tests/Beatmaps/LegacyBeatmapSkinColourTest.cs @@ -158,6 +158,8 @@ namespace osu.Game.Tests.Beatmaps add { } remove { } } + + public ISkin FindProvider(Func lookupFunction) => null; } } } From b87a5956dd62de3fb77532073022ee6ac0a08e21 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sun, 6 Jun 2021 12:17:55 +0900 Subject: [PATCH 33/89] Add fallback logic to `SkinManager` --- osu.Game/Skinning/SkinManager.cs | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/osu.Game/Skinning/SkinManager.cs b/osu.Game/Skinning/SkinManager.cs index d373618232..9aa2d90064 100644 --- a/osu.Game/Skinning/SkinManager.cs +++ b/osu.Game/Skinning/SkinManager.cs @@ -204,16 +204,34 @@ namespace osu.Game.Skinning public event Action SourceChanged; - public Drawable GetDrawableComponent(ISkinComponent component) => CurrentSkin.Value.GetDrawableComponent(component); + public Drawable GetDrawableComponent(ISkinComponent component) => lookupWithFallback(s => s.GetDrawableComponent(component)); - public Texture GetTexture(string componentName, WrapMode wrapModeS, WrapMode wrapModeT) => CurrentSkin.Value.GetTexture(componentName, wrapModeS, wrapModeT); + public Texture GetTexture(string componentName, WrapMode wrapModeS, WrapMode wrapModeT) => lookupWithFallback(s => s.GetTexture(componentName, wrapModeS, wrapModeT)); - public ISample GetSample(ISampleInfo sampleInfo) => CurrentSkin.Value.GetSample(sampleInfo); + public ISample GetSample(ISampleInfo sampleInfo) => lookupWithFallback(s => s.GetSample(sampleInfo)); - public IBindable GetConfig(TLookup lookup) => CurrentSkin.Value.GetConfig(lookup); + public IBindable GetConfig(TLookup lookup) => lookupWithFallback(s => s.GetConfig(lookup)); public ISkin FindProvider(Func lookupFunction) => lookupFunction(CurrentSkin.Value) ? CurrentSkin.Value : null; + private Skin defaultLegacySkin; + + private T lookupWithFallback(Func func) + where T : class + { + var selectedSkin = func(CurrentSkin.Value); + + if (selectedSkin != null) + return selectedSkin; + + defaultLegacySkin ??= new DefaultLegacySkin(this); + + if (CurrentSkin.Value is LegacySkin) + return func(defaultLegacySkin); + + return null; + } + #region IResourceStorageProvider AudioManager IStorageResourceProvider.AudioManager => audio; From b904fa6615ad210afc94e874a3861572e9bb0499 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sun, 6 Jun 2021 12:37:42 +0900 Subject: [PATCH 34/89] Revert "Ensure all frames in an animation are retrieved from the same skin" This reverts commit 37c8c63fc566c115aa3f974b6d23bb12f62caa05. --- osu.Game/Skinning/LegacySkinExtensions.cs | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/osu.Game/Skinning/LegacySkinExtensions.cs b/osu.Game/Skinning/LegacySkinExtensions.cs index 6e8276c01e..d8fb1fa664 100644 --- a/osu.Game/Skinning/LegacySkinExtensions.cs +++ b/osu.Game/Skinning/LegacySkinExtensions.cs @@ -54,16 +54,9 @@ namespace osu.Game.Skinning IEnumerable getTextures() { - ISkin lookupSource = null; - for (int i = 0; true; i++) { - string frameName = $"{componentName}{animationSeparator}{i}"; - - // ensure all textures are retrieved from the same skin source. - lookupSource ??= source.FindProvider(s => s.GetTexture(frameName, wrapModeS, wrapModeT) != null); - - if ((texture = lookupSource?.GetTexture(frameName, wrapModeS, wrapModeT)) == null) + if ((texture = source.GetTexture($"{componentName}{animationSeparator}{i}", wrapModeS, wrapModeT)) == null) break; yield return texture; From 9ebafb1ec0dcde3756fb681d826d85a465775a95 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sun, 6 Jun 2021 22:26:27 +0900 Subject: [PATCH 35/89] Fix cursor trail logic --- .../TestSceneCursorTrail.cs | 18 ++++++++++++++---- .../Skinning/Legacy/LegacyCursor.cs | 6 ++++-- .../Skinning/Legacy/LegacyCursorTrail.cs | 8 +++++++- .../Legacy/OsuLegacySkinTransformer.cs | 12 ++++++++---- 4 files changed, 33 insertions(+), 11 deletions(-) diff --git a/osu.Game.Rulesets.Osu.Tests/TestSceneCursorTrail.cs b/osu.Game.Rulesets.Osu.Tests/TestSceneCursorTrail.cs index 9997660c2d..46274e779b 100644 --- a/osu.Game.Rulesets.Osu.Tests/TestSceneCursorTrail.cs +++ b/osu.Game.Rulesets.Osu.Tests/TestSceneCursorTrail.cs @@ -39,18 +39,28 @@ namespace osu.Game.Rulesets.Osu.Tests [Test] public void TestLegacySmoothCursorTrail() { - createTest(() => new LegacySkinContainer(false) + createTest(() => { - Child = new LegacyCursorTrail() + var skinContainer = new LegacySkinContainer(false); + var legacyCursorTrail = new LegacyCursorTrail(skinContainer); + + skinContainer.Child = legacyCursorTrail; + + return skinContainer; }); } [Test] public void TestLegacyDisjointCursorTrail() { - createTest(() => new LegacySkinContainer(true) + createTest(() => { - Child = new LegacyCursorTrail() + var skinContainer = new LegacySkinContainer(true); + var legacyCursorTrail = new LegacyCursorTrail(skinContainer); + + skinContainer.Child = legacyCursorTrail; + + return skinContainer; }); } diff --git a/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacyCursor.cs b/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacyCursor.cs index 7a8555d991..b2ffc171be 100644 --- a/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacyCursor.cs +++ b/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacyCursor.cs @@ -11,10 +11,12 @@ namespace osu.Game.Rulesets.Osu.Skinning.Legacy { public class LegacyCursor : OsuCursorSprite { + private readonly ISkin skin; private bool spin; - public LegacyCursor() + public LegacyCursor(ISkin skin) { + this.skin = skin; Size = new Vector2(50); Anchor = Anchor.Centre; @@ -22,7 +24,7 @@ namespace osu.Game.Rulesets.Osu.Skinning.Legacy } [BackgroundDependencyLoader] - private void load(ISkinSource skin) + private void load() { bool centre = skin.GetConfig(OsuSkinConfiguration.CursorCentre)?.Value ?? true; spin = skin.GetConfig(OsuSkinConfiguration.CursorRotate)?.Value ?? true; diff --git a/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacyCursorTrail.cs b/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacyCursorTrail.cs index 0025576325..f6fd3e36ab 100644 --- a/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacyCursorTrail.cs +++ b/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacyCursorTrail.cs @@ -14,14 +14,20 @@ namespace osu.Game.Rulesets.Osu.Skinning.Legacy { public class LegacyCursorTrail : CursorTrail { + private readonly ISkin skin; private const double disjoint_trail_time_separation = 1000 / 60.0; private bool disjointTrail; private double lastTrailTime; private IBindable cursorSize; + public LegacyCursorTrail(ISkin skin) + { + this.skin = skin; + } + [BackgroundDependencyLoader] - private void load(ISkinSource skin, OsuConfigManager config) + private void load(OsuConfigManager config) { Texture = skin.GetTexture("cursortrail"); disjointTrail = skin.GetTexture("cursormiddle") == null; diff --git a/osu.Game.Rulesets.Osu/Skinning/Legacy/OsuLegacySkinTransformer.cs b/osu.Game.Rulesets.Osu/Skinning/Legacy/OsuLegacySkinTransformer.cs index 33693748d9..e3f32fb76f 100644 --- a/osu.Game.Rulesets.Osu/Skinning/Legacy/OsuLegacySkinTransformer.cs +++ b/osu.Game.Rulesets.Osu/Skinning/Legacy/OsuLegacySkinTransformer.cs @@ -84,14 +84,18 @@ namespace osu.Game.Rulesets.Osu.Skinning.Legacy return null; case OsuSkinComponents.Cursor: - if (Source.GetTexture("cursor") != null) - return new LegacyCursor(); + var cursorProvider = Source.FindProvider(s => s.GetTexture("cursor") != null); + + if (cursorProvider != null) + return new LegacyCursor(cursorProvider); return null; case OsuSkinComponents.CursorTrail: - if (Source.GetTexture("cursortrail") != null) - return new LegacyCursorTrail(); + var trailProvider = Source.FindProvider(s => s.GetTexture("cursortrail") != null); + + if (trailProvider != null) + return new LegacyCursorTrail(trailProvider); return null; From b5f145cfa92e08b26c7d6fb976fbe011ce7167cb Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sun, 6 Jun 2021 23:01:37 +0900 Subject: [PATCH 36/89] Use null propagation for animation lookups --- osu.Game/Skinning/LegacySkin.cs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/osu.Game/Skinning/LegacySkin.cs b/osu.Game/Skinning/LegacySkin.cs index 8f1895883d..337acee9e8 100644 --- a/osu.Game/Skinning/LegacySkin.cs +++ b/osu.Game/Skinning/LegacySkin.cs @@ -417,12 +417,7 @@ namespace osu.Game.Skinning break; } - var animation = this.GetAnimation(component.LookupName, false, false); - - if (animation != null) - return animation; - - return null; + return this.GetAnimation(component.LookupName, false, false); } private Texture getParticleTexture(HitResult result) From e10dfab2e85190328a3e6621d54b4adab1e72f9a Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sun, 6 Jun 2021 23:23:35 +0900 Subject: [PATCH 37/89] Ensure scorebar marker lookup is performed on the source the background is retrieved from --- osu.Game/Skinning/LegacyHealthDisplay.cs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/osu.Game/Skinning/LegacyHealthDisplay.cs b/osu.Game/Skinning/LegacyHealthDisplay.cs index c601adc3a0..5a5c7f11ea 100644 --- a/osu.Game/Skinning/LegacyHealthDisplay.cs +++ b/osu.Game/Skinning/LegacyHealthDisplay.cs @@ -35,7 +35,10 @@ namespace osu.Game.Skinning { AutoSizeAxes = Axes.Both; - isNewStyle = getTexture(skin, "marker") != null; + var backgroundSource = skin.FindProvider(s => getTexture(s, "bg") != null); + + // the marker lookup to decide which display style must be performed on the source of the bg, which is the most common element. + isNewStyle = getTexture(backgroundSource, "marker") != null; // background implementation is the same for both versions. AddInternal(new Sprite { Texture = getTexture(skin, "bg") }); @@ -76,7 +79,7 @@ namespace osu.Game.Skinning protected override void Flash(JudgementResult result) => marker.Flash(result); - private static Texture getTexture(ISkinSource skin, string name) => skin.GetTexture($"scorebar-{name}"); + private static Texture getTexture(ISkin skin, string name) => skin?.GetTexture($"scorebar-{name}"); private static Color4 getFillColour(double hp) { From 277eb9fa6ed825c53ccefe87e641e186ff998bd3 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 7 Jun 2021 15:58:41 +0900 Subject: [PATCH 38/89] Fix slider repeat arrow not updating rotation immediately while paused in editor A bit of a local solution, but not sure there's a better way to handle this. Closes #13342. --- .../Objects/Drawables/DrawableSliderRepeat.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSliderRepeat.cs b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSliderRepeat.cs index b7458b5695..4a2a18ffd6 100644 --- a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSliderRepeat.cs +++ b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSliderRepeat.cs @@ -152,7 +152,8 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables while (Math.Abs(aimRotation - Arrow.Rotation) > 180) aimRotation += aimRotation < Arrow.Rotation ? 360 : -360; - if (!hasRotation) + // The clock may be paused in a scenario like the editor. + if (!hasRotation || !Clock.IsRunning) { Arrow.Rotation = aimRotation; hasRotation = true; From f677f9b5f44a36fd4dc880c55bd7c998fb5b98db Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 7 Jun 2021 17:22:30 +0900 Subject: [PATCH 39/89] Stop `BackgroundScreenDefault` from reloading beatmap background when already correct --- .../TestSceneBackgroundScreenDefault.cs | 59 ++++++++++ .../Backgrounds/BackgroundScreenDefault.cs | 103 ++++++++++-------- 2 files changed, 115 insertions(+), 47 deletions(-) create mode 100644 osu.Game.Tests/Visual/Background/TestSceneBackgroundScreenDefault.cs diff --git a/osu.Game.Tests/Visual/Background/TestSceneBackgroundScreenDefault.cs b/osu.Game.Tests/Visual/Background/TestSceneBackgroundScreenDefault.cs new file mode 100644 index 0000000000..ef50f866d5 --- /dev/null +++ b/osu.Game.Tests/Visual/Background/TestSceneBackgroundScreenDefault.cs @@ -0,0 +1,59 @@ +// 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.Screens; +using osu.Framework.Testing; +using osu.Game.Configuration; +using osu.Game.Graphics.Backgrounds; +using osu.Game.Online.API; +using osu.Game.Screens; +using osu.Game.Screens.Backgrounds; +using osu.Game.Users; + +namespace osu.Game.Tests.Visual.Background +{ + [TestFixture] + public class TestSceneBackgroundScreenDefault : OsuTestScene + { + private BackgroundScreenStack stack; + private BackgroundScreenDefault screen; + + private Graphics.Backgrounds.Background getCurrentBackground() => screen.ChildrenOfType().FirstOrDefault(); + + [Resolved] + private OsuConfigManager config { get; set; } + + [SetUpSteps] + public void SetUpSteps() + { + AddStep("create background stack", () => Child = stack = new BackgroundScreenStack()); + AddStep("push default screen", () => stack.Push(screen = new BackgroundScreenDefault(false))); + AddUntilStep("wait for screen to load", () => screen.IsCurrentScreen()); + } + + [Test] + public void TestBeatmapDoesntReloadOnNoChange() + { + BeatmapBackground last = null; + + setSourceMode(BackgroundSource.Beatmap); + setSupporter(true); + + AddUntilStep("wait for beatmap background to be loaded", () => (last = getCurrentBackground() as BeatmapBackground) != null); + AddAssert("next doesn't load new background", () => screen.Next() == false); + + // doesn't really need to be checked but might as well. + AddWaitStep("wait a bit", 5); + AddUntilStep("ensure same background instance", () => last == getCurrentBackground()); + } + + private void setSourceMode(BackgroundSource source) => + AddStep("set background mode to beatmap", () => config.SetValue(OsuSetting.MenuBackgroundSource, source)); + + private void setSupporter(bool isSupporter) => + AddStep("set supporter", () => ((DummyAPIAccess)API).LocalUser.Value = new User { IsSupporter = isSupporter }); + } +} diff --git a/osu.Game/Screens/Backgrounds/BackgroundScreenDefault.cs b/osu.Game/Screens/Backgrounds/BackgroundScreenDefault.cs index b02e7ddb0d..dc27514459 100644 --- a/osu.Game/Screens/Backgrounds/BackgroundScreenDefault.cs +++ b/osu.Game/Screens/Backgrounds/BackgroundScreenDefault.cs @@ -5,8 +5,8 @@ using System.Threading; using osu.Framework.Allocation; using osu.Framework.Bindables; using osu.Framework.Graphics; -using osu.Framework.Utils; using osu.Framework.Threading; +using osu.Framework.Utils; using osu.Game.Beatmaps; using osu.Game.Configuration; using osu.Game.Graphics.Backgrounds; @@ -53,14 +53,41 @@ namespace osu.Game.Screens.Backgrounds mode.ValueChanged += _ => Next(); beatmap.ValueChanged += _ => Next(); introSequence.ValueChanged += _ => Next(); - seasonalBackgroundLoader.SeasonalBackgroundChanged += Next; + seasonalBackgroundLoader.SeasonalBackgroundChanged += () => Next(); currentDisplay = RNG.Next(0, background_count); Next(); } - private void display(Background newBackground) + private ScheduledDelegate nextTask; + private CancellationTokenSource cancellationTokenSource; + + /// + /// Request loading the next background. + /// + /// Whether a new background was queued for load. May return false if the current background is still valid. + public bool Next() + { + var nextBackground = createBackground(); + + // in the case that the background hasn't changed, we want to avoid cancelling any tasks that could still be loading. + if (nextBackground == background) + return false; + + cancellationTokenSource?.Cancel(); + cancellationTokenSource = new CancellationTokenSource(); + + nextTask?.Cancel(); + nextTask = Scheduler.AddDelayed(() => + { + LoadComponentAsync(nextBackground, displayNext, cancellationTokenSource.Token); + }, 100); + + return true; + } + + private void displayNext(Background newBackground) { background?.FadeOut(800, Easing.InOutSine); background?.Expire(); @@ -69,68 +96,50 @@ namespace osu.Game.Screens.Backgrounds currentDisplay++; } - private ScheduledDelegate nextTask; - private CancellationTokenSource cancellationTokenSource; - - public void Next() - { - nextTask?.Cancel(); - cancellationTokenSource?.Cancel(); - cancellationTokenSource = new CancellationTokenSource(); - nextTask = Scheduler.AddDelayed(() => LoadComponentAsync(createBackground(), display, cancellationTokenSource.Token), 100); - } - private Background createBackground() { - Background newBackground; - string backgroundName; + // seasonal background loading gets highest priority. + Background newBackground = seasonalBackgroundLoader.LoadNextBackground(); - var seasonalBackground = seasonalBackgroundLoader.LoadNextBackground(); - - if (seasonalBackground != null) - { - seasonalBackground.Depth = currentDisplay; - return seasonalBackground; - } - - switch (introSequence.Value) - { - case IntroSequence.Welcome: - backgroundName = "Intro/Welcome/menu-background"; - break; - - default: - backgroundName = $@"Menu/menu-background-{currentDisplay % background_count + 1}"; - break; - } - - if (user.Value?.IsSupporter ?? false) + if (newBackground == null && user.Value?.IsSupporter == true) { switch (mode.Value) { case BackgroundSource.Beatmap: - newBackground = new BeatmapBackground(beatmap.Value, backgroundName); - break; - case BackgroundSource.BeatmapWithStoryboard: - newBackground = AllowStoryboardBackground - ? new BeatmapBackgroundWithStoryboard(beatmap.Value, backgroundName) - : new BeatmapBackground(beatmap.Value, backgroundName); - break; + { + // this method is called in many cases where the beatmap hasn't changed (ie. on screen transitions). + // if a background is already displayed for the requested beatmap, we don't want to load it again. + if ((background as BeatmapBackground)?.Beatmap == beatmap.Value) + return background; - default: - newBackground = new SkinnedBackground(skin.Value, backgroundName); + if (mode.Value == BackgroundSource.BeatmapWithStoryboard && AllowStoryboardBackground) + newBackground = new BeatmapBackgroundWithStoryboard(beatmap.Value, getBackgroundTextureName()); + + newBackground ??= new BeatmapBackground(beatmap.Value, getBackgroundTextureName()); break; + } } } - else - newBackground = new Background(backgroundName); + newBackground ??= new Background(getBackgroundTextureName()); newBackground.Depth = currentDisplay; return newBackground; } + private string getBackgroundTextureName() + { + switch (introSequence.Value) + { + case IntroSequence.Welcome: + return @"Intro/Welcome/menu-background"; + + default: + return $@"Menu/menu-background-{currentDisplay % background_count + 1}"; + } + } + private class SkinnedBackground : Background { private readonly Skin skin; From 59130be99cce706979294681a0aa2cd0f159c7c1 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 7 Jun 2021 17:32:04 +0900 Subject: [PATCH 40/89] Fix switching storyboard mode not triggering a reload --- .../Screens/Backgrounds/BackgroundScreenDefault.cs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/osu.Game/Screens/Backgrounds/BackgroundScreenDefault.cs b/osu.Game/Screens/Backgrounds/BackgroundScreenDefault.cs index dc27514459..6bcfaac907 100644 --- a/osu.Game/Screens/Backgrounds/BackgroundScreenDefault.cs +++ b/osu.Game/Screens/Backgrounds/BackgroundScreenDefault.cs @@ -108,15 +108,16 @@ namespace osu.Game.Screens.Backgrounds case BackgroundSource.Beatmap: case BackgroundSource.BeatmapWithStoryboard: { - // this method is called in many cases where the beatmap hasn't changed (ie. on screen transitions). - // if a background is already displayed for the requested beatmap, we don't want to load it again. - if ((background as BeatmapBackground)?.Beatmap == beatmap.Value) - return background; - if (mode.Value == BackgroundSource.BeatmapWithStoryboard && AllowStoryboardBackground) newBackground = new BeatmapBackgroundWithStoryboard(beatmap.Value, getBackgroundTextureName()); - newBackground ??= new BeatmapBackground(beatmap.Value, getBackgroundTextureName()); + + // this method is called in many cases where the beatmap hasn't changed (ie. on screen transitions). + // if a background is already displayed for the requested beatmap, we don't want to load it again. + if (background?.GetType() == newBackground.GetType() && + (background as BeatmapBackground)?.Beatmap == beatmap.Value) + return background; + break; } } From 729e05241f22e096682e550ae6ab502e32b58c2b Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 7 Jun 2021 17:32:10 +0900 Subject: [PATCH 41/89] Add more test coverage --- .../TestSceneBackgroundScreenDefault.cs | 33 ++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/osu.Game.Tests/Visual/Background/TestSceneBackgroundScreenDefault.cs b/osu.Game.Tests/Visual/Background/TestSceneBackgroundScreenDefault.cs index ef50f866d5..09fe9b3767 100644 --- a/osu.Game.Tests/Visual/Background/TestSceneBackgroundScreenDefault.cs +++ b/osu.Game.Tests/Visual/Background/TestSceneBackgroundScreenDefault.cs @@ -34,6 +34,33 @@ namespace osu.Game.Tests.Visual.Background AddUntilStep("wait for screen to load", () => screen.IsCurrentScreen()); } + [Test] + public void TestTogglingStoryboardSwitchesBackgroundType() + { + setSupporter(true); + + setSourceMode(BackgroundSource.Beatmap); + AddUntilStep("is beatmap background", () => getCurrentBackground() is BeatmapBackground); + + setSourceMode(BackgroundSource.BeatmapWithStoryboard); + AddUntilStep("is storyboard background", () => getCurrentBackground() is BeatmapBackgroundWithStoryboard); + } + + [Test] + public void TestTogglingSupporterTogglesBeatmapBackground() + { + setSourceMode(BackgroundSource.Beatmap); + + setSupporter(true); + AddUntilStep("is beatmap background", () => getCurrentBackground() is BeatmapBackground); + + setSupporter(false); + AddUntilStep("is default background", () => !(getCurrentBackground() is BeatmapBackground)); + + setSupporter(true); + AddUntilStep("is beatmap background", () => getCurrentBackground() is BeatmapBackground); + } + [Test] public void TestBeatmapDoesntReloadOnNoChange() { @@ -54,6 +81,10 @@ namespace osu.Game.Tests.Visual.Background AddStep("set background mode to beatmap", () => config.SetValue(OsuSetting.MenuBackgroundSource, source)); private void setSupporter(bool isSupporter) => - AddStep("set supporter", () => ((DummyAPIAccess)API).LocalUser.Value = new User { IsSupporter = isSupporter }); + AddStep($"set supporter {isSupporter}", () => ((DummyAPIAccess)API).LocalUser.Value = new User + { + IsSupporter = isSupporter, + Id = API.LocalUser.Value.Id + 1, + }); } } From a0fbf29b987da2eead9141b45bf0e7c4fd6c8d68 Mon Sep 17 00:00:00 2001 From: Susko3 <16479013+Susko3@users.noreply.github.com> Date: Mon, 7 Jun 2021 11:24:48 +0200 Subject: [PATCH 42/89] add `application/x-osu-archive` mime type to Android `IntentFilter`s --- osu.Android/OsuGameActivity.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/osu.Android/OsuGameActivity.cs b/osu.Android/OsuGameActivity.cs index cffcea22c2..063e02d349 100644 --- a/osu.Android/OsuGameActivity.cs +++ b/osu.Android/OsuGameActivity.cs @@ -20,7 +20,8 @@ namespace osu.Android [Activity(Theme = "@android:style/Theme.NoTitleBar", MainLauncher = true, ScreenOrientation = ScreenOrientation.FullUser, SupportsPictureInPicture = false, ConfigurationChanges = ConfigChanges.Orientation | ConfigChanges.ScreenSize, HardwareAccelerated = false, LaunchMode = LaunchMode.SingleInstance, Exported = true)] [IntentFilter(new[] { Intent.ActionView }, Categories = new[] { Intent.CategoryDefault }, DataScheme = "content", DataPathPattern = ".*\\\\.osz", DataHost = "*", DataMimeType = "*/*")] [IntentFilter(new[] { Intent.ActionView }, Categories = new[] { Intent.CategoryDefault }, DataScheme = "content", DataPathPattern = ".*\\\\.osk", DataHost = "*", DataMimeType = "*/*")] - [IntentFilter(new[] { Intent.ActionSend, Intent.ActionSendMultiple }, Categories = new[] { Intent.CategoryDefault }, DataMimeTypes = new[] { "application/zip", "application/octet-stream", "application/download", "application/x-zip", "application/x-zip-compressed" })] + [IntentFilter(new[] { Intent.ActionView }, Categories = new[] { Intent.CategoryDefault }, DataScheme = "content", DataMimeType = "application/x-osu-archive")] + [IntentFilter(new[] { Intent.ActionSend, Intent.ActionSendMultiple }, Categories = new[] { Intent.CategoryDefault }, DataMimeTypes = new[] { "application/zip", "application/octet-stream", "application/download", "application/x-zip", "application/x-zip-compressed", "application/x-osu-archive" })] [IntentFilter(new[] { Intent.ActionView }, Categories = new[] { Intent.CategoryBrowsable, Intent.CategoryDefault }, DataSchemes = new[] { "osu", "osump" })] public class OsuGameActivity : AndroidGameActivity { From 122a624b7fb68d8f5bb55ad30b96cd38be8e9a3b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Mon, 7 Jun 2021 12:15:37 +0200 Subject: [PATCH 43/89] Remove bogus `CatchHitWindows` `CatchHitWindows` were a vestige from the past, and were not actually used anywhere except for the hit error meter test, giving off an appearance that the hit error meter was working properly. `CatchHitObject` actually specifies empty hit windows. --- .../Scoring/CatchHitWindows.cs | 22 ------------------- .../Visual/Gameplay/TestSceneHitErrorMeter.cs | 6 ++--- 2 files changed, 2 insertions(+), 26 deletions(-) delete mode 100644 osu.Game.Rulesets.Catch/Scoring/CatchHitWindows.cs diff --git a/osu.Game.Rulesets.Catch/Scoring/CatchHitWindows.cs b/osu.Game.Rulesets.Catch/Scoring/CatchHitWindows.cs deleted file mode 100644 index 0a444d923e..0000000000 --- a/osu.Game.Rulesets.Catch/Scoring/CatchHitWindows.cs +++ /dev/null @@ -1,22 +0,0 @@ -// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. -// See the LICENCE file in the repository root for full licence text. - -using osu.Game.Rulesets.Scoring; - -namespace osu.Game.Rulesets.Catch.Scoring -{ - public class CatchHitWindows : HitWindows - { - public override bool IsHitResultAllowed(HitResult result) - { - switch (result) - { - case HitResult.Great: - case HitResult.Miss: - return true; - } - - return false; - } - } -} diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneHitErrorMeter.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneHitErrorMeter.cs index ab13095af4..acc7287b5a 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneHitErrorMeter.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneHitErrorMeter.cs @@ -11,7 +11,6 @@ using osu.Framework.Graphics.Containers; using osu.Framework.Threading; using osu.Framework.Utils; using osu.Game.Graphics.Sprites; -using osu.Game.Rulesets.Catch.Scoring; using osu.Game.Rulesets.Judgements; using osu.Game.Rulesets.Mania.Scoring; using osu.Game.Rulesets.Mods; @@ -84,10 +83,9 @@ namespace osu.Game.Tests.Visual.Gameplay } [Test] - public void TestCatch() + public void TestEmpty() { - AddStep("OD 1", () => recreateDisplay(new CatchHitWindows(), 1)); - AddStep("OD 10", () => recreateDisplay(new CatchHitWindows(), 10)); + AddStep("empty windows", () => recreateDisplay(HitWindows.Empty, 5)); } private void recreateDisplay(HitWindows hitWindows, float overallDifficulty) From 37d062c7cd736563d2760ac5a9f4b154a74c1c02 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Mon, 7 Jun 2021 12:25:48 +0200 Subject: [PATCH 44/89] Add failing assertions to hit error meter test --- .../Visual/Gameplay/TestSceneHitErrorMeter.cs | 18 ++++++++++++++++-- .../HUD/HitErrorMeters/BarHitErrorMeter.cs | 2 +- .../HUD/HitErrorMeters/ColourHitErrorMeter.cs | 2 +- 3 files changed, 18 insertions(+), 4 deletions(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneHitErrorMeter.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneHitErrorMeter.cs index acc7287b5a..d2e25d1f9f 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneHitErrorMeter.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneHitErrorMeter.cs @@ -4,10 +4,12 @@ using System; using System.Collections.Generic; using System.Diagnostics.CodeAnalysis; +using System.Linq; using NUnit.Framework; using osu.Framework.Allocation; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; +using osu.Framework.Testing; using osu.Framework.Threading; using osu.Framework.Utils; using osu.Game.Graphics.Sprites; @@ -86,6 +88,18 @@ namespace osu.Game.Tests.Visual.Gameplay public void TestEmpty() { AddStep("empty windows", () => recreateDisplay(HitWindows.Empty, 5)); + + AddStep("hit", () => newJudgement()); + AddAssert("no bars added", () => !this.ChildrenOfType().Any()); + AddAssert("circle added", () => + this.ChildrenOfType().All( + meter => meter.ChildrenOfType().Count() == 1)); + + AddStep("miss", () => newJudgement(50, HitResult.Miss)); + AddAssert("no bars added", () => !this.ChildrenOfType().Any()); + AddAssert("circle added", () => + this.ChildrenOfType().All( + meter => meter.ChildrenOfType().Count() == 2)); } private void recreateDisplay(HitWindows hitWindows, float overallDifficulty) @@ -152,12 +166,12 @@ namespace osu.Game.Tests.Visual.Gameplay }); } - private void newJudgement(double offset = 0) + private void newJudgement(double offset = 0, HitResult result = HitResult.Perfect) { scoreProcessor.ApplyResult(new JudgementResult(new HitCircle { HitWindows = drawableRuleset.HitWindows }, new Judgement()) { TimeOffset = offset == 0 ? RNG.Next(-150, 150) : offset, - Type = HitResult.Perfect, + Type = result, }); } diff --git a/osu.Game/Screens/Play/HUD/HitErrorMeters/BarHitErrorMeter.cs b/osu.Game/Screens/Play/HUD/HitErrorMeters/BarHitErrorMeter.cs index 5d0263772d..5a8a65acfd 100644 --- a/osu.Game/Screens/Play/HUD/HitErrorMeters/BarHitErrorMeter.cs +++ b/osu.Game/Screens/Play/HUD/HitErrorMeters/BarHitErrorMeter.cs @@ -244,7 +244,7 @@ namespace osu.Game.Screens.Play.HUD.HitErrorMeters private float getRelativeJudgementPosition(double value) => Math.Clamp((float)((value / maxHitWindow) + 1) / 2, 0, 1); - private class JudgementLine : CompositeDrawable + internal class JudgementLine : CompositeDrawable { private const int judgement_fade_duration = 5000; diff --git a/osu.Game/Screens/Play/HUD/HitErrorMeters/ColourHitErrorMeter.cs b/osu.Game/Screens/Play/HUD/HitErrorMeters/ColourHitErrorMeter.cs index e9ccbcdae2..86c0de8855 100644 --- a/osu.Game/Screens/Play/HUD/HitErrorMeters/ColourHitErrorMeter.cs +++ b/osu.Game/Screens/Play/HUD/HitErrorMeters/ColourHitErrorMeter.cs @@ -53,7 +53,7 @@ namespace osu.Game.Screens.Play.HUD.HitErrorMeters } } - private class HitErrorCircle : Container + internal class HitErrorCircle : Container { public bool IsRemoved { get; private set; } From 0531c2dcd9a5072f684541f039f8a1dfd1a1468e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Mon, 7 Jun 2021 12:31:24 +0200 Subject: [PATCH 45/89] Move empty window check to bar error meter It's not valid in the base `HitErrorMeter`, as the colour meter only displays colour for a given judgement, so it is still valid to add new items to it even if the hit window is 0, as misses are still possible. --- .../Play/HUD/HitErrorMeters/BarHitErrorMeter.cs | 2 +- .../Screens/Play/HUD/HitErrorMeters/HitErrorMeter.cs | 12 ++---------- 2 files changed, 3 insertions(+), 11 deletions(-) diff --git a/osu.Game/Screens/Play/HUD/HitErrorMeters/BarHitErrorMeter.cs b/osu.Game/Screens/Play/HUD/HitErrorMeters/BarHitErrorMeter.cs index 5a8a65acfd..0412085d1d 100644 --- a/osu.Game/Screens/Play/HUD/HitErrorMeters/BarHitErrorMeter.cs +++ b/osu.Game/Screens/Play/HUD/HitErrorMeters/BarHitErrorMeter.cs @@ -214,7 +214,7 @@ namespace osu.Game.Screens.Play.HUD.HitErrorMeters protected override void OnNewJudgement(JudgementResult judgement) { - if (!judgement.IsHit) + if (!judgement.IsHit || judgement.HitObject.HitWindows?.WindowFor(HitResult.Miss) == 0) return; if (judgementsContainer.Count > max_concurrent_judgements) diff --git a/osu.Game/Screens/Play/HUD/HitErrorMeters/HitErrorMeter.cs b/osu.Game/Screens/Play/HUD/HitErrorMeters/HitErrorMeter.cs index b0f9928b13..17a6e772fb 100644 --- a/osu.Game/Screens/Play/HUD/HitErrorMeters/HitErrorMeter.cs +++ b/osu.Game/Screens/Play/HUD/HitErrorMeters/HitErrorMeter.cs @@ -32,15 +32,7 @@ namespace osu.Game.Screens.Play.HUD.HitErrorMeters { base.LoadComplete(); - processor.NewJudgement += onNewJudgement; - } - - private void onNewJudgement(JudgementResult result) - { - if (result.HitObject.HitWindows?.WindowFor(HitResult.Miss) == 0) - return; - - OnNewJudgement(result); + processor.NewJudgement += OnNewJudgement; } protected abstract void OnNewJudgement(JudgementResult judgement); @@ -74,7 +66,7 @@ namespace osu.Game.Screens.Play.HUD.HitErrorMeters base.Dispose(isDisposing); if (processor != null) - processor.NewJudgement -= onNewJudgement; + processor.NewJudgement -= OnNewJudgement; } } } From 1b4771655aa694dabdfdb9a78e3d063e32d34b09 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Mon, 7 Jun 2021 13:13:24 +0200 Subject: [PATCH 46/89] Adjust test scene to avoid cross-test interference * Move steps from ctor to a separate basic test. * Wait for barrage to complete in basic test, as not doing so polluted state of other tests. * Reset score processor after every test. --- .../Visual/Gameplay/TestSceneHitErrorMeter.cs | 24 ++++++++++++++----- 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneHitErrorMeter.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneHitErrorMeter.cs index d2e25d1f9f..2a12577ad8 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneHitErrorMeter.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneHitErrorMeter.cs @@ -30,15 +30,22 @@ namespace osu.Game.Tests.Visual.Gameplay { public class TestSceneHitErrorMeter : OsuTestScene { - [Cached] - private ScoreProcessor scoreProcessor = new ScoreProcessor(); + [Cached(typeof(ScoreProcessor))] + private TestScoreProcessor scoreProcessor = new TestScoreProcessor(); [Cached(typeof(DrawableRuleset))] private TestDrawableRuleset drawableRuleset = new TestDrawableRuleset(); - public TestSceneHitErrorMeter() + [SetUpSteps] + public void SetUp() { - recreateDisplay(new OsuHitWindows(), 5); + AddStep("reset score processor", () => scoreProcessor.Reset()); + } + + [Test] + public void TestBasic() + { + AddStep("create display", () => recreateDisplay(new OsuHitWindows(), 5)); AddRepeatStep("New random judgement", () => newJudgement(), 40); @@ -46,12 +53,11 @@ namespace osu.Game.Tests.Visual.Gameplay AddRepeatStep("New max positive", () => newJudgement(drawableRuleset.HitWindows.WindowFor(HitResult.Meh)), 20); AddStep("New fixed judgement (50ms)", () => newJudgement(50)); + ScheduledDelegate del = null; AddStep("Judgement barrage", () => { int runCount = 0; - ScheduledDelegate del = null; - del = Scheduler.AddDelayed(() => { newJudgement(runCount++ / 10f); @@ -61,6 +67,7 @@ namespace osu.Game.Tests.Visual.Gameplay del?.Cancel(); }, 10, true); }); + AddUntilStep("wait for barrage", () => del.Cancelled); } [Test] @@ -211,5 +218,10 @@ namespace osu.Game.Tests.Visual.Gameplay public override void CancelResume() => throw new NotImplementedException(); } + + private class TestScoreProcessor : ScoreProcessor + { + public void Reset() => base.Reset(false); + } } } From 08701b5eab700ea2944cf358cd60616d8fe9c96a Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 7 Jun 2021 23:23:10 +0900 Subject: [PATCH 47/89] Ensure all lookups in `LegacyHealthDisplay` use the found provider Not actually needed to fix the remaining issue but does feel better --- osu.Game/Skinning/LegacyHealthDisplay.cs | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/osu.Game/Skinning/LegacyHealthDisplay.cs b/osu.Game/Skinning/LegacyHealthDisplay.cs index 5a5c7f11ea..9d3bafd0b1 100644 --- a/osu.Game/Skinning/LegacyHealthDisplay.cs +++ b/osu.Game/Skinning/LegacyHealthDisplay.cs @@ -20,9 +20,6 @@ namespace osu.Game.Skinning { private const double epic_cutoff = 0.5; - [Resolved] - private ISkinSource skin { get; set; } - private LegacyHealthPiece fill; private LegacyHealthPiece marker; @@ -31,14 +28,14 @@ namespace osu.Game.Skinning private bool isNewStyle; [BackgroundDependencyLoader] - private void load() + private void load(ISkinSource source) { AutoSizeAxes = Axes.Both; - var backgroundSource = skin.FindProvider(s => getTexture(s, "bg") != null); + var skin = source.FindProvider(s => getTexture(s, "bg") != null); // the marker lookup to decide which display style must be performed on the source of the bg, which is the most common element. - isNewStyle = getTexture(backgroundSource, "marker") != null; + isNewStyle = getTexture(skin, "marker") != null; // background implementation is the same for both versions. AddInternal(new Sprite { Texture = getTexture(skin, "bg") }); @@ -98,7 +95,7 @@ namespace osu.Game.Skinning private readonly Texture dangerTexture; private readonly Texture superDangerTexture; - public LegacyOldStyleMarker(ISkinSource skin) + public LegacyOldStyleMarker(ISkin skin) { normalTexture = getTexture(skin, "ki"); dangerTexture = getTexture(skin, "kidanger"); @@ -129,9 +126,9 @@ namespace osu.Game.Skinning public class LegacyNewStyleMarker : LegacyMarker { - private readonly ISkinSource skin; + private readonly ISkin skin; - public LegacyNewStyleMarker(ISkinSource skin) + public LegacyNewStyleMarker(ISkin skin) { this.skin = skin; } @@ -153,7 +150,7 @@ namespace osu.Game.Skinning internal class LegacyOldStyleFill : LegacyHealthPiece { - public LegacyOldStyleFill(ISkinSource skin) + public LegacyOldStyleFill(ISkin skin) { // required for sizing correctly.. var firstFrame = getTexture(skin, "colour-0"); @@ -176,7 +173,7 @@ namespace osu.Game.Skinning internal class LegacyNewStyleFill : LegacyHealthPiece { - public LegacyNewStyleFill(ISkinSource skin) + public LegacyNewStyleFill(ISkin skin) { InternalChild = new Sprite { From c0305343bc9312333957362d124c906f2febd82b Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 7 Jun 2021 23:23:44 +0900 Subject: [PATCH 48/89] Fix `FindProvider` incorrectly returning `LegacySkinTransformer` itself --- osu.Game/Skinning/LegacySkinTransformer.cs | 8 +++++++- osu.Game/Skinning/SkinProvidingContainer.cs | 12 ++++++++++-- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/osu.Game/Skinning/LegacySkinTransformer.cs b/osu.Game/Skinning/LegacySkinTransformer.cs index cace4acf6c..651fdddb1b 100644 --- a/osu.Game/Skinning/LegacySkinTransformer.cs +++ b/osu.Game/Skinning/LegacySkinTransformer.cs @@ -16,7 +16,7 @@ namespace osu.Game.Skinning /// /// Transformer used to handle support of legacy features for individual rulesets. /// - public abstract class LegacySkinTransformer : ISkin + public abstract class LegacySkinTransformer : ISkinSource { /// /// Source of the which is being transformed. @@ -50,5 +50,11 @@ namespace osu.Game.Skinning public abstract IBindable GetConfig(TLookup lookup); public ISkin FindProvider(Func lookupFunction) => Source.FindProvider(lookupFunction); + + public event Action SourceChanged + { + add { throw new NotSupportedException(); } + remove { } + } } } diff --git a/osu.Game/Skinning/SkinProvidingContainer.cs b/osu.Game/Skinning/SkinProvidingContainer.cs index 863b5f5a24..0e16cf43ee 100644 --- a/osu.Game/Skinning/SkinProvidingContainer.cs +++ b/osu.Game/Skinning/SkinProvidingContainer.cs @@ -46,8 +46,16 @@ namespace osu.Game.Skinning public ISkin FindProvider(Func lookupFunction) { - if (skin != null && lookupFunction(skin)) - return skin; + if (skin is ISkinSource source) + { + if (source.FindProvider(lookupFunction) is ISkin found) + return found; + } + else if (skin != null) + { + if (lookupFunction(skin)) + return skin; + } return fallbackSource?.FindProvider(lookupFunction); } From 6d56e02ddbe825599c3aaf1c09310c08858c8158 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 8 Jun 2021 00:02:57 +0900 Subject: [PATCH 49/89] Add back incorrectly reverted animation handling logic This reverts commit b904fa6615ad210afc94e874a3861572e9bb0499. --- osu.Game/Skinning/LegacySkinExtensions.cs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/osu.Game/Skinning/LegacySkinExtensions.cs b/osu.Game/Skinning/LegacySkinExtensions.cs index d8fb1fa664..051bcf4a8e 100644 --- a/osu.Game/Skinning/LegacySkinExtensions.cs +++ b/osu.Game/Skinning/LegacySkinExtensions.cs @@ -54,9 +54,16 @@ namespace osu.Game.Skinning IEnumerable getTextures() { + ISkin lookupSource = null; + for (int i = 0; true; i++) { - if ((texture = source.GetTexture($"{componentName}{animationSeparator}{i}", wrapModeS, wrapModeT)) == null) + string frameName = $"{componentName}{animationSeparator}{i}"; + + // ensure all textures are retrieved from the same skin source. + lookupSource ??= (source as ISkinSource)?.FindProvider(s => s.GetTexture(frameName, wrapModeS, wrapModeT) != null) ?? source; + + if ((texture = lookupSource?.GetTexture(frameName, wrapModeS, wrapModeT)) == null) break; yield return texture; From 273d66a0e0a76ca45e5188601bd03a34e9804011 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 8 Jun 2021 00:42:34 +0900 Subject: [PATCH 50/89] Fix `TaikoMascot` texture animation lookups --- osu.Game.Rulesets.Taiko/UI/TaikoMascotAnimation.cs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/osu.Game.Rulesets.Taiko/UI/TaikoMascotAnimation.cs b/osu.Game.Rulesets.Taiko/UI/TaikoMascotAnimation.cs index 9c76aea54c..3706acbe23 100644 --- a/osu.Game.Rulesets.Taiko/UI/TaikoMascotAnimation.cs +++ b/osu.Game.Rulesets.Taiko/UI/TaikoMascotAnimation.cs @@ -85,8 +85,12 @@ namespace osu.Game.Rulesets.Taiko.UI } [BackgroundDependencyLoader] - private void load(ISkinSource skin) + private void load(ISkinSource source) { + ISkin skin = source.FindProvider(s => getAnimationFrame(s, state, 0) != null); + + if (skin == null) return; + for (int frameIndex = 0; true; frameIndex++) { var texture = getAnimationFrame(skin, state, frameIndex); @@ -112,8 +116,12 @@ namespace osu.Game.Rulesets.Taiko.UI } [BackgroundDependencyLoader] - private void load(ISkinSource skin) + private void load(ISkinSource source) { + ISkin skin = source.FindProvider(s => getAnimationFrame(s, TaikoMascotAnimationState.Clear, 0) != null); + + if (skin == null) return; + foreach (var frameIndex in clear_animation_sequence) { var texture = getAnimationFrame(skin, TaikoMascotAnimationState.Clear, frameIndex); From e7e9197f03d2247fb2dfbb599401dff62a8160ec Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 8 Jun 2021 00:42:50 +0900 Subject: [PATCH 51/89] Fix `FindProvider` not correctly checking legacy default in `SkinManager` --- osu.Game/Skinning/SkinManager.cs | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/osu.Game/Skinning/SkinManager.cs b/osu.Game/Skinning/SkinManager.cs index 9aa2d90064..ea4e1232c3 100644 --- a/osu.Game/Skinning/SkinManager.cs +++ b/osu.Game/Skinning/SkinManager.cs @@ -48,6 +48,8 @@ namespace osu.Game.Skinning protected override string ImportFromStablePath => "Skins"; + private readonly Skin defaultLegacySkin; + public SkinManager(Storage storage, DatabaseContextFactory contextFactory, GameHost host, IResourceStore resources, AudioManager audio) : base(storage, contextFactory, new SkinStore(contextFactory, storage), host) { @@ -55,6 +57,8 @@ namespace osu.Game.Skinning this.host = host; this.resources = resources; + defaultLegacySkin = new DefaultLegacySkin(this); + CurrentSkinInfo.ValueChanged += skin => CurrentSkin.Value = GetSkin(skin.NewValue); CurrentSkin.ValueChanged += skin => { @@ -212,9 +216,16 @@ namespace osu.Game.Skinning public IBindable GetConfig(TLookup lookup) => lookupWithFallback(s => s.GetConfig(lookup)); - public ISkin FindProvider(Func lookupFunction) => lookupFunction(CurrentSkin.Value) ? CurrentSkin.Value : null; + public ISkin FindProvider(Func lookupFunction) + { + if (lookupFunction(CurrentSkin.Value)) + return CurrentSkin.Value; - private Skin defaultLegacySkin; + if (CurrentSkin.Value is LegacySkin && lookupFunction(defaultLegacySkin)) + return defaultLegacySkin; + + return null; + } private T lookupWithFallback(Func func) where T : class @@ -224,8 +235,6 @@ namespace osu.Game.Skinning if (selectedSkin != null) return selectedSkin; - defaultLegacySkin ??= new DefaultLegacySkin(this); - if (CurrentSkin.Value is LegacySkin) return func(defaultLegacySkin); From 2c1f22d7ae914e84a6263d4ae7231d2f0f2a3bb7 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 8 Jun 2021 01:17:20 +0900 Subject: [PATCH 52/89] Refactor animation lookup to properly handle skins providing non-animated resources --- osu.Game/Skinning/LegacySkinExtensions.cs | 27 ++++++++++++++--------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/osu.Game/Skinning/LegacySkinExtensions.cs b/osu.Game/Skinning/LegacySkinExtensions.cs index 051bcf4a8e..ec25268be4 100644 --- a/osu.Game/Skinning/LegacySkinExtensions.cs +++ b/osu.Game/Skinning/LegacySkinExtensions.cs @@ -27,6 +27,18 @@ namespace osu.Game.Skinning { Texture texture; + // find the first source which provides either the animated or non-animated version. + ISkin skin = (source as ISkinSource)?.FindProvider(s => + { + if (animatable && s.GetTexture(getFrameName(0)) != null) + return true; + + return s.GetTexture(componentName, wrapModeS, wrapModeT) != null; + }) ?? source; + + if (skin == null) + return null; + if (animatable) { var textures = getTextures().ToArray(); @@ -35,7 +47,7 @@ namespace osu.Game.Skinning { var animation = new SkinnableTextureAnimation(startAtCurrentTime) { - DefaultFrameLength = frameLength ?? getFrameLength(source, applyConfigFrameRate, textures), + DefaultFrameLength = frameLength ?? getFrameLength(skin, applyConfigFrameRate, textures), Loop = looping, }; @@ -47,28 +59,23 @@ namespace osu.Game.Skinning } // if an animation was not allowed or not found, fall back to a sprite retrieval. - if ((texture = source.GetTexture(componentName, wrapModeS, wrapModeT)) != null) + if ((texture = skin.GetTexture(componentName, wrapModeS, wrapModeT)) != null) return new Sprite { Texture = texture }; return null; IEnumerable getTextures() { - ISkin lookupSource = null; - for (int i = 0; true; i++) { - string frameName = $"{componentName}{animationSeparator}{i}"; - - // ensure all textures are retrieved from the same skin source. - lookupSource ??= (source as ISkinSource)?.FindProvider(s => s.GetTexture(frameName, wrapModeS, wrapModeT) != null) ?? source; - - if ((texture = lookupSource?.GetTexture(frameName, wrapModeS, wrapModeT)) == null) + if ((texture = skin.GetTexture(getFrameName(i), wrapModeS, wrapModeT)) == null) break; yield return texture; } } + + string getFrameName(int frameIndex) => $"{componentName}{animationSeparator}{frameIndex}"; } public static bool HasFont(this ISkin source, LegacyFont font) From 06840d78cc4df29d7b5f8a58c0b7cdb44f83a9ec Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 8 Jun 2021 12:04:29 +0900 Subject: [PATCH 53/89] Remove now unused method --- osu.Game/Skinning/DefaultLegacySkin.cs | 2 -- osu.Game/Skinning/LegacyBeatmapSkin.cs | 8 -------- osu.Game/Skinning/LegacySkin.cs | 3 --- 3 files changed, 13 deletions(-) diff --git a/osu.Game/Skinning/DefaultLegacySkin.cs b/osu.Game/Skinning/DefaultLegacySkin.cs index 1d17b5ce20..30192182f3 100644 --- a/osu.Game/Skinning/DefaultLegacySkin.cs +++ b/osu.Game/Skinning/DefaultLegacySkin.cs @@ -31,8 +31,6 @@ 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/LegacyBeatmapSkin.cs b/osu.Game/Skinning/LegacyBeatmapSkin.cs index 2374cb976b..caf37e5bc9 100644 --- a/osu.Game/Skinning/LegacyBeatmapSkin.cs +++ b/osu.Game/Skinning/LegacyBeatmapSkin.cs @@ -70,14 +70,6 @@ namespace osu.Game.Skinning return base.GetSample(sampleInfo); } - protected override DefaultLegacySkin CreateFallbackSkin(IResourceStore storage, IStorageResourceProvider resources) - { - // 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; - } - 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 337acee9e8..e255fbae81 100644 --- a/osu.Game/Skinning/LegacySkin.cs +++ b/osu.Game/Skinning/LegacySkin.cs @@ -109,9 +109,6 @@ namespace osu.Game.Skinning true) != null); } - [CanBeNull] - protected virtual DefaultLegacySkin CreateFallbackSkin(IResourceStore storage, IStorageResourceProvider resources) => new DefaultLegacySkin(resources); - public override IBindable GetConfig(TLookup lookup) { switch (lookup) From 88b87b98a850b89b4e6f52e47791ba5c60da9f00 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 8 Jun 2021 12:10:14 +0900 Subject: [PATCH 54/89] Fix slider ball layer sources --- osu.Game.Rulesets.Osu/Skinning/Legacy/LegacySliderBall.cs | 7 +++++-- .../Skinning/Legacy/OsuLegacySkinTransformer.cs | 7 +++++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacySliderBall.cs b/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacySliderBall.cs index 1a8c5ada1b..e4e1483665 100644 --- a/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacySliderBall.cs +++ b/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacySliderBall.cs @@ -14,18 +14,21 @@ namespace osu.Game.Rulesets.Osu.Skinning.Legacy { private readonly Drawable animationContent; + private readonly ISkin skin; + private Sprite layerNd; private Sprite layerSpec; - public LegacySliderBall(Drawable animationContent) + public LegacySliderBall(Drawable animationContent, ISkin skin) { this.animationContent = animationContent; + this.skin = skin; AutoSizeAxes = Axes.Both; } [BackgroundDependencyLoader] - private void load(ISkinSource skin) + private void load() { var ballColour = skin.GetConfig(OsuSkinColour.SliderBall)?.Value ?? Color4.White; diff --git a/osu.Game.Rulesets.Osu/Skinning/Legacy/OsuLegacySkinTransformer.cs b/osu.Game.Rulesets.Osu/Skinning/Legacy/OsuLegacySkinTransformer.cs index e3f32fb76f..3267b48ebf 100644 --- a/osu.Game.Rulesets.Osu/Skinning/Legacy/OsuLegacySkinTransformer.cs +++ b/osu.Game.Rulesets.Osu/Skinning/Legacy/OsuLegacySkinTransformer.cs @@ -49,13 +49,16 @@ namespace osu.Game.Rulesets.Osu.Skinning.Legacy return followCircle; case OsuSkinComponents.SliderBall: - var sliderBallContent = this.GetAnimation("sliderb", true, true, animationSeparator: ""); + // specular and nd layers must come from the same source as the ball texure. + var ballProvider = Source.FindProvider(s => s.GetTexture("sliderb") != null || s.GetTexture("sliderb0") != null); + + var sliderBallContent = ballProvider.GetAnimation("sliderb", true, true, animationSeparator: ""); // todo: slider ball has a custom frame delay based on velocity // Math.Max((150 / Velocity) * GameBase.SIXTY_FRAME_TIME, GameBase.SIXTY_FRAME_TIME); if (sliderBallContent != null) - return new LegacySliderBall(sliderBallContent); + return new LegacySliderBall(sliderBallContent, ballProvider); return null; From 27e3de3ea33a99abaf453a3eac542221be6c1e61 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 8 Jun 2021 12:12:14 +0900 Subject: [PATCH 55/89] Add TODO about beatmap skin fallback support --- osu.Game/Skinning/SkinManager.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/osu.Game/Skinning/SkinManager.cs b/osu.Game/Skinning/SkinManager.cs index ea4e1232c3..25cd909035 100644 --- a/osu.Game/Skinning/SkinManager.cs +++ b/osu.Game/Skinning/SkinManager.cs @@ -235,6 +235,9 @@ namespace osu.Game.Skinning if (selectedSkin != null) return selectedSkin; + // TODO: we also want to return a DefaultLegacySkin here if the current *beatmap* is providing any skinned elements. + // When attempting to address this, we may want to move the full DefaultLegacySkin fallback logic to within Player itself (to better allow + // for beatmap skin visibility). if (CurrentSkin.Value is LegacySkin) return func(defaultLegacySkin); From 7341e474f1405dde31bd1082cdf7d725d99c61ac Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Tue, 8 Jun 2021 14:25:39 +0900 Subject: [PATCH 56/89] Attempt to safeguard against collections database corruptions --- osu.Game/Collections/CollectionManager.cs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/osu.Game/Collections/CollectionManager.cs b/osu.Game/Collections/CollectionManager.cs index 086cc573d5..b53cc659f7 100644 --- a/osu.Game/Collections/CollectionManager.cs +++ b/osu.Game/Collections/CollectionManager.cs @@ -264,14 +264,18 @@ namespace osu.Game.Collections using (var sw = new SerializationWriter(storage.GetStream(database_name, FileAccess.Write))) { sw.Write(database_version); - sw.Write(Collections.Count); - foreach (var c in Collections) + var collectionsCopy = Collections.ToArray(); + sw.Write(collectionsCopy.Length); + + foreach (var c in collectionsCopy) { sw.Write(c.Name.Value); - sw.Write(c.Beatmaps.Count); - foreach (var b in c.Beatmaps) + var beatmapsCopy = c.Beatmaps.ToArray(); + sw.Write(beatmapsCopy.Length); + + foreach (var b in beatmapsCopy) sw.Write(b.MD5Hash); } } From f3f634e969607be08ba6e592d4b7454a2e53692f Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 8 Jun 2021 15:05:14 +0900 Subject: [PATCH 57/89] Clean up previous sample immediately on skin source change to avoid `Play` after disposal This seems to be the simplest way to avoid calls to `Play` after the underlying sample may have been disposed. As per the issue thread, a local workaround is acceptable here. Closes #13223. --- osu.Game/Skinning/PoolableSkinnableSample.cs | 44 +++++++++++++++++--- 1 file changed, 39 insertions(+), 5 deletions(-) diff --git a/osu.Game/Skinning/PoolableSkinnableSample.cs b/osu.Game/Skinning/PoolableSkinnableSample.cs index b04158a58f..7565417b7f 100644 --- a/osu.Game/Skinning/PoolableSkinnableSample.cs +++ b/osu.Game/Skinning/PoolableSkinnableSample.cs @@ -2,6 +2,7 @@ // See the LICENCE file in the repository root for full licence text. using System; +using System.Linq; using JetBrains.Annotations; using osu.Framework.Allocation; using osu.Framework.Audio; @@ -70,22 +71,48 @@ namespace osu.Game.Skinning updateSample(); } + protected override void LoadComplete() + { + base.LoadComplete(); + + CurrentSkin.SourceChanged += skinChangedImmediate; + } + + private void skinChangedImmediate() + { + // Clean up the previous sample immediately on a source change. + // This avoids a potential call to Play() of an already disposed sample (samples are disposed along with the skin, but SkinChanged is scheduled). + clearPreviousSamples(); + } + protected override void SkinChanged(ISkinSource skin, bool allowFallback) { base.SkinChanged(skin, allowFallback); updateSample(); } + /// + /// Whether this sample was playing before a skin source change. + /// + private bool wasPlaying; + + private void clearPreviousSamples() + { + // only run if the samples aren't already cleared. + // this ensures the "wasPlaying" state is stored correctly even if multiple clear calls are executed. + if (!sampleContainer.Any()) return; + + wasPlaying = Playing; + + sampleContainer.Clear(); + Sample = null; + } + private void updateSample() { if (sampleInfo == null) return; - bool wasPlaying = Playing; - - sampleContainer.Clear(); - Sample = null; - var sample = CurrentSkin.GetSample(sampleInfo); if (sample == null && AllowDefaultFallback) @@ -155,6 +182,13 @@ namespace osu.Game.Skinning } } + protected override void Dispose(bool isDisposing) + { + base.Dispose(isDisposing); + + CurrentSkin.SourceChanged -= skinChangedImmediate; + } + #region Re-expose AudioContainer public BindableNumber Volume => sampleContainer.Volume; From e388a896e87e6ad03701bedd286a751b9f965408 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 8 Jun 2021 16:02:25 +0900 Subject: [PATCH 58/89] Don't apply visibility increase to first object in osu!catch The goal of the visibility increase is to help in cases where timing is an issue (by showing the approach circle etc.). This doesn't need to apply to catch. @smoogipoo interested as to whether you agree with this one. Visually it looks better to me but it does change the behaviour for only osu!catch, so I'm not 100% confident on it. Closes #13367. --- osu.Game.Rulesets.Catch.Tests/TestSceneCatchModHidden.cs | 8 -------- osu.Game.Rulesets.Catch/Mods/CatchModHidden.cs | 3 +-- 2 files changed, 1 insertion(+), 10 deletions(-) diff --git a/osu.Game.Rulesets.Catch.Tests/TestSceneCatchModHidden.cs b/osu.Game.Rulesets.Catch.Tests/TestSceneCatchModHidden.cs index 1248409b2a..09362929d2 100644 --- a/osu.Game.Rulesets.Catch.Tests/TestSceneCatchModHidden.cs +++ b/osu.Game.Rulesets.Catch.Tests/TestSceneCatchModHidden.cs @@ -4,10 +4,8 @@ using System.Collections.Generic; using System.Linq; using NUnit.Framework; -using osu.Framework.Allocation; using osu.Framework.Testing; using osu.Game.Beatmaps; -using osu.Game.Configuration; using osu.Game.Rulesets.Catch.Mods; using osu.Game.Rulesets.Catch.Objects; using osu.Game.Rulesets.Catch.Objects.Drawables; @@ -21,12 +19,6 @@ namespace osu.Game.Rulesets.Catch.Tests { public class TestSceneCatchModHidden : ModTestScene { - [BackgroundDependencyLoader] - private void load() - { - LocalConfig.SetValue(OsuSetting.IncreaseFirstObjectVisibility, false); - } - [Test] public void TestJuiceStream() { diff --git a/osu.Game.Rulesets.Catch/Mods/CatchModHidden.cs b/osu.Game.Rulesets.Catch/Mods/CatchModHidden.cs index 7bad4c79cb..f9e106f097 100644 --- a/osu.Game.Rulesets.Catch/Mods/CatchModHidden.cs +++ b/osu.Game.Rulesets.Catch/Mods/CatchModHidden.cs @@ -29,8 +29,7 @@ namespace osu.Game.Rulesets.Catch.Mods } protected override void ApplyIncreasedVisibilityState(DrawableHitObject hitObject, ArmedState state) - { - } + => ApplyNormalVisibilityState(hitObject, state); protected override void ApplyNormalVisibilityState(DrawableHitObject hitObject, ArmedState state) { From 67135ce3dbaa2ea8a55abba016fd525d16f1ad2a Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Tue, 8 Jun 2021 16:15:17 +0900 Subject: [PATCH 59/89] Add null check --- osu.Game/Skinning/PoolableSkinnableSample.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/osu.Game/Skinning/PoolableSkinnableSample.cs b/osu.Game/Skinning/PoolableSkinnableSample.cs index 7565417b7f..9acc1f1a9d 100644 --- a/osu.Game/Skinning/PoolableSkinnableSample.cs +++ b/osu.Game/Skinning/PoolableSkinnableSample.cs @@ -186,7 +186,8 @@ namespace osu.Game.Skinning { base.Dispose(isDisposing); - CurrentSkin.SourceChanged -= skinChangedImmediate; + if (CurrentSkin != null) + CurrentSkin.SourceChanged -= skinChangedImmediate; } #region Re-expose AudioContainer From 89895f6ce40ab080b0d5cce81763b3a63310ecf3 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 8 Jun 2021 16:24:00 +0900 Subject: [PATCH 60/89] Update framework --- osu.Android.props | 2 +- osu.Game/osu.Game.csproj | 2 +- osu.iOS.props | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/osu.Android.props b/osu.Android.props index 1216b1772f..395470824f 100644 --- a/osu.Android.props +++ b/osu.Android.props @@ -52,6 +52,6 @@ - + diff --git a/osu.Game/osu.Game.csproj b/osu.Game/osu.Game.csproj index 21a890014a..9ecab1ee48 100644 --- a/osu.Game/osu.Game.csproj +++ b/osu.Game/osu.Game.csproj @@ -34,7 +34,7 @@ all runtime; build; native; contentfiles; analyzers; buildtransitive - + diff --git a/osu.iOS.props b/osu.iOS.props index bf080e4def..e66f125985 100644 --- a/osu.iOS.props +++ b/osu.iOS.props @@ -70,7 +70,7 @@ - + @@ -93,7 +93,7 @@ - + From 7fa0ac6ed720b32bcb3c52b05f3cf8352c69927a Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Tue, 8 Jun 2021 17:03:46 +0900 Subject: [PATCH 61/89] Fix possible nullref when exiting song select too fast --- .../Visual/Navigation/TestSceneScreenNavigation.cs | 8 ++++++++ osu.Game/Screens/Select/SongSelect.cs | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs b/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs index 0308d74aa4..3694d3afce 100644 --- a/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs +++ b/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs @@ -152,6 +152,14 @@ namespace osu.Game.Tests.Visual.Navigation AddUntilStep("wait for track", () => !Game.MusicController.CurrentTrack.IsDummyDevice && Game.MusicController.IsPlaying); } + [Test] + public void TestPushSongSelectAndPressBackButtonImmediately() + { + AddStep("push song select", () => Game.ScreenStack.Push(new TestPlaySongSelect())); + AddStep("press back button", () => Game.ChildrenOfType().First().Action()); + AddWaitStep("wait two frame", 2); + } + [Test] public void TestExitSongSelectWithClick() { diff --git a/osu.Game/Screens/Select/SongSelect.cs b/osu.Game/Screens/Select/SongSelect.cs index 270addc8e6..c697c64c3f 100644 --- a/osu.Game/Screens/Select/SongSelect.cs +++ b/osu.Game/Screens/Select/SongSelect.cs @@ -665,7 +665,7 @@ namespace osu.Game.Screens.Select public override bool OnBackButton() { - if (ModSelect.State.Value == Visibility.Visible) + if (ModSelect?.State.Value == Visibility.Visible) { ModSelect.Hide(); return true; From 490ab9e96a1fed83ed169d9084b095e6af1e9e96 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Tue, 8 Jun 2021 17:09:03 +0900 Subject: [PATCH 62/89] Fix typo --- osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs b/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs index 3694d3afce..6fc19b95b9 100644 --- a/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs +++ b/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs @@ -157,7 +157,7 @@ namespace osu.Game.Tests.Visual.Navigation { AddStep("push song select", () => Game.ScreenStack.Push(new TestPlaySongSelect())); AddStep("press back button", () => Game.ChildrenOfType().First().Action()); - AddWaitStep("wait two frame", 2); + AddWaitStep("wait two frames", 2); } [Test] From 860f1aebb385ab9bccdad07848a72a418f500914 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Tue, 8 Jun 2021 17:38:12 +0900 Subject: [PATCH 63/89] Only call OnBackButton() if the screen has finished loading --- osu.Game/OsuGame.cs | 5 +++-- osu.Game/Screens/IOsuScreen.cs | 3 +++ osu.Game/Screens/Select/SongSelect.cs | 2 +- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/osu.Game/OsuGame.cs b/osu.Game/OsuGame.cs index c51624341e..8d87baa363 100644 --- a/osu.Game/OsuGame.cs +++ b/osu.Game/OsuGame.cs @@ -651,9 +651,10 @@ namespace osu.Game Origin = Anchor.BottomLeft, Action = () => { - var currentScreen = ScreenStack.CurrentScreen as IOsuScreen; + if (!(ScreenStack.CurrentScreen is IOsuScreen currentScreen)) + return; - if (currentScreen?.AllowBackButton == true && !currentScreen.OnBackButton()) + if (!((Drawable)currentScreen).IsLoaded || currentScreen.AllowBackButton && !currentScreen.OnBackButton()) ScreenStack.Exit(); } }, diff --git a/osu.Game/Screens/IOsuScreen.cs b/osu.Game/Screens/IOsuScreen.cs index cc8778d9ae..0434135547 100644 --- a/osu.Game/Screens/IOsuScreen.cs +++ b/osu.Game/Screens/IOsuScreen.cs @@ -67,8 +67,11 @@ namespace osu.Game.Screens /// Invoked when the back button has been pressed to close any overlays before exiting this . /// /// + /// If this has not yet finished loading, the exit will occur immediately without this method being invoked. + /// /// Return true to block this from being exited after closing an overlay. /// Return false if this should continue exiting. + /// /// bool OnBackButton(); } diff --git a/osu.Game/Screens/Select/SongSelect.cs b/osu.Game/Screens/Select/SongSelect.cs index c697c64c3f..270addc8e6 100644 --- a/osu.Game/Screens/Select/SongSelect.cs +++ b/osu.Game/Screens/Select/SongSelect.cs @@ -665,7 +665,7 @@ namespace osu.Game.Screens.Select public override bool OnBackButton() { - if (ModSelect?.State.Value == Visibility.Visible) + if (ModSelect.State.Value == Visibility.Visible) { ModSelect.Hide(); return true; From ab9290772b46ac2087eeb9f9e5fdfaf848fa03a5 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Tue, 8 Jun 2021 17:54:54 +0900 Subject: [PATCH 64/89] Fix a similar case with online play sub-screens --- .../Navigation/TestSceneScreenNavigation.cs | 28 +++++++++++++++++++ .../Screens/OnlinePlay/OnlinePlayScreen.cs | 5 +++- 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs b/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs index 6fc19b95b9..52401d32e5 100644 --- a/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs +++ b/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs @@ -9,16 +9,19 @@ using osu.Framework.Screens; using osu.Framework.Testing; using osu.Game.Beatmaps; using osu.Game.Graphics.UserInterface; +using osu.Game.Online.Multiplayer; using osu.Game.Overlays; using osu.Game.Overlays.Mods; using osu.Game.Overlays.Toolbar; using osu.Game.Rulesets.Mods; using osu.Game.Rulesets.Osu.Mods; +using osu.Game.Screens.OnlinePlay.Components; using osu.Game.Screens.Play; using osu.Game.Screens.Ranking; using osu.Game.Screens.Select; using osu.Game.Screens.Select.Options; using osu.Game.Tests.Beatmaps.IO; +using osu.Game.Tests.Visual.Multiplayer; using osuTK; using osuTK.Input; @@ -306,6 +309,18 @@ namespace osu.Game.Tests.Visual.Navigation AddAssert("Toolbar is hidden", () => Game.Toolbar.State.Value == Visibility.Hidden); } + [Test] + public void TestPushMatchSubScreenAndPressBackButtonImmediately() + { + TestMultiplayer multiplayer = null; + + PushAndConfirm(() => multiplayer = new TestMultiplayer()); + + AddStep("open room", () => multiplayer.OpenNewRoom()); + AddStep("press back button", () => Game.ChildrenOfType().First().Action()); + AddWaitStep("wait two frames", 2); + } + private void pushEscape() => AddStep("Press escape", () => InputManager.Key(Key.Escape)); @@ -330,5 +345,18 @@ namespace osu.Game.Tests.Visual.Navigation protected override bool DisplayStableImportPrompt => false; } + + private class TestMultiplayer : Screens.OnlinePlay.Multiplayer.Multiplayer + { + [Cached(typeof(MultiplayerClient))] + public readonly TestMultiplayerClient Client; + + public TestMultiplayer() + { + Client = new TestMultiplayerClient((TestMultiplayerRoomManager)RoomManager); + } + + protected override RoomManager CreateRoomManager() => new TestMultiplayerRoomManager(); + } } } diff --git a/osu.Game/Screens/OnlinePlay/OnlinePlayScreen.cs b/osu.Game/Screens/OnlinePlay/OnlinePlayScreen.cs index 90e499c67f..e418d36d40 100644 --- a/osu.Game/Screens/OnlinePlay/OnlinePlayScreen.cs +++ b/osu.Game/Screens/OnlinePlay/OnlinePlayScreen.cs @@ -253,7 +253,10 @@ namespace osu.Game.Screens.OnlinePlay public override bool OnBackButton() { - if ((screenStack.CurrentScreen as IOnlinePlaySubScreen)?.OnBackButton() == true) + if (!(screenStack.CurrentScreen is IOnlinePlaySubScreen onlineSubScreen)) + return false; + + if (((Drawable)onlineSubScreen).IsLoaded && onlineSubScreen.AllowBackButton && onlineSubScreen.OnBackButton()) return true; if (screenStack.CurrentScreen != null && !(screenStack.CurrentScreen is LoungeSubScreen)) From 4d9fffc01bbcc2863386f0d32f38840cb0077c91 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 8 Jun 2021 17:58:57 +0900 Subject: [PATCH 65/89] Update score encoder version to be higher than any existing stable version --- osu.Game/Scoring/Legacy/LegacyScoreEncoder.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/osu.Game/Scoring/Legacy/LegacyScoreEncoder.cs b/osu.Game/Scoring/Legacy/LegacyScoreEncoder.cs index f8dd6953ad..cd77fbbdd8 100644 --- a/osu.Game/Scoring/Legacy/LegacyScoreEncoder.cs +++ b/osu.Game/Scoring/Legacy/LegacyScoreEncoder.cs @@ -15,7 +15,10 @@ namespace osu.Game.Scoring.Legacy { public class LegacyScoreEncoder { - public const int LATEST_VERSION = 128; + /// + /// Database version in stable-compatible YYYYMMDD format. + /// + public const int LATEST_VERSION = 30000000; private readonly Score score; private readonly IBeatmap beatmap; From 061e3d7f26c22df9da73b81a07cb9c75835a2281 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 8 Jun 2021 18:00:09 +0900 Subject: [PATCH 66/89] Move legacy `ScoreInfo` to be completely based on presence of classic mod --- .../Requests/Responses/APILegacyScoreInfo.cs | 8 +++++-- osu.Game/Scoring/Legacy/LegacyScoreDecoder.cs | 4 ++++ osu.Game/Scoring/ScoreInfo.cs | 23 +------------------ 3 files changed, 11 insertions(+), 24 deletions(-) diff --git a/osu.Game/Online/API/Requests/Responses/APILegacyScoreInfo.cs b/osu.Game/Online/API/Requests/Responses/APILegacyScoreInfo.cs index 3d3c07a5ad..1b394185fd 100644 --- a/osu.Game/Online/API/Requests/Responses/APILegacyScoreInfo.cs +++ b/osu.Game/Online/API/Requests/Responses/APILegacyScoreInfo.cs @@ -21,7 +21,12 @@ namespace osu.Game.Online.API.Requests.Responses { var ruleset = rulesets.GetRuleset(OnlineRulesetID); - var mods = Mods != null ? ruleset.CreateInstance().GetAllMods().Where(mod => Mods.Contains(mod.Acronym)).ToArray() : Array.Empty(); + var rulesetInstance = ruleset.CreateInstance(); + + var mods = Mods != null ? rulesetInstance.GetAllMods().Where(mod => Mods.Contains(mod.Acronym)).ToArray() : Array.Empty(); + + // all API scores provided by this class are considered to be legacy. + mods = mods.Append(rulesetInstance.GetAllMods().OfType().Single()).ToArray(); var scoreInfo = new ScoreInfo { @@ -38,7 +43,6 @@ namespace osu.Game.Online.API.Requests.Responses Rank = Rank, Ruleset = ruleset, Mods = mods, - IsLegacyScore = true }; if (Statistics != null) diff --git a/osu.Game/Scoring/Legacy/LegacyScoreDecoder.cs b/osu.Game/Scoring/Legacy/LegacyScoreDecoder.cs index 97cb5ca7ab..4b93530e45 100644 --- a/osu.Game/Scoring/Legacy/LegacyScoreDecoder.cs +++ b/osu.Game/Scoring/Legacy/LegacyScoreDecoder.cs @@ -65,6 +65,10 @@ namespace osu.Game.Scoring.Legacy scoreInfo.Mods = currentRuleset.ConvertFromLegacyMods((LegacyMods)sr.ReadInt32()).ToArray(); + // lazer replays get a really high version number. + if (version < 30000000) + scoreInfo.Mods = scoreInfo.Mods.Append(currentRuleset.GetAllMods().OfType().Single()).ToArray(); + currentBeatmap = workingBeatmap.GetPlayableBeatmap(currentRuleset.RulesetInfo, scoreInfo.Mods); scoreInfo.Beatmap = currentBeatmap.BeatmapInfo; diff --git a/osu.Game/Scoring/ScoreInfo.cs b/osu.Game/Scoring/ScoreInfo.cs index a6faaf6379..801175d90d 100644 --- a/osu.Game/Scoring/ScoreInfo.cs +++ b/osu.Game/Scoring/ScoreInfo.cs @@ -201,33 +201,12 @@ namespace osu.Game.Scoring [JsonProperty("position")] public int? Position { get; set; } - private bool isLegacyScore; - /// /// Whether this represents a legacy (osu!stable) score. /// [JsonIgnore] [NotMapped] - public bool IsLegacyScore - { - get - { - if (isLegacyScore) - return true; - - // The above check will catch legacy online scores that have an appropriate UserString + UserId. - // For non-online scores such as those imported in, a heuristic is used based on the following table: - // - // Mode | UserString | UserId - // --------------- | ---------- | --------- - // stable | | 1 - // lazer | | - // lazer (offline) | Guest | 1 - - return ID > 0 && UserID == 1 && UserString != "Guest"; - } - set => isLegacyScore = value; - } + public bool IsLegacyScore => mods.OfType().Any(); public IEnumerable GetStatisticsForDisplay() { From b287366c8bed5e6f51e98409c6eb3cc6a41150e4 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 8 Jun 2021 18:09:57 +0900 Subject: [PATCH 67/89] Remove forgotten classic mod addition --- osu.Game/Scoring/ScoreInfo.cs | 3 --- 1 file changed, 3 deletions(-) diff --git a/osu.Game/Scoring/ScoreInfo.cs b/osu.Game/Scoring/ScoreInfo.cs index 801175d90d..755dab17f9 100644 --- a/osu.Game/Scoring/ScoreInfo.cs +++ b/osu.Game/Scoring/ScoreInfo.cs @@ -76,9 +76,6 @@ namespace osu.Game.Scoring else if (localAPIMods != null) scoreMods = apiMods.Select(m => m.ToMod(rulesetInstance)).ToArray(); - if (IsLegacyScore) - scoreMods = scoreMods.Append(rulesetInstance.GetAllMods().OfType().Single()).ToArray(); - return scoreMods; } set From d31e3e8f1c5d1f41a620f8f9e582a5553d13c1b5 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Tue, 8 Jun 2021 18:23:03 +0900 Subject: [PATCH 68/89] Fix nullref --- osu.Game/Scoring/ScoreInfo.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Scoring/ScoreInfo.cs b/osu.Game/Scoring/ScoreInfo.cs index 755dab17f9..3944c1d3de 100644 --- a/osu.Game/Scoring/ScoreInfo.cs +++ b/osu.Game/Scoring/ScoreInfo.cs @@ -203,7 +203,7 @@ namespace osu.Game.Scoring /// [JsonIgnore] [NotMapped] - public bool IsLegacyScore => mods.OfType().Any(); + public bool IsLegacyScore => Mods.OfType().Any(); public IEnumerable GetStatisticsForDisplay() { From 4ee7721c51ae5300f80a71f0f58de993530c2211 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 8 Jun 2021 18:38:47 +0900 Subject: [PATCH 69/89] Extract first version out to constant --- osu.Game/Scoring/Legacy/LegacyScoreDecoder.cs | 2 +- osu.Game/Scoring/Legacy/LegacyScoreEncoder.cs | 8 +++++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/osu.Game/Scoring/Legacy/LegacyScoreDecoder.cs b/osu.Game/Scoring/Legacy/LegacyScoreDecoder.cs index 4b93530e45..2f17167297 100644 --- a/osu.Game/Scoring/Legacy/LegacyScoreDecoder.cs +++ b/osu.Game/Scoring/Legacy/LegacyScoreDecoder.cs @@ -66,7 +66,7 @@ namespace osu.Game.Scoring.Legacy scoreInfo.Mods = currentRuleset.ConvertFromLegacyMods((LegacyMods)sr.ReadInt32()).ToArray(); // lazer replays get a really high version number. - if (version < 30000000) + if (version < LegacyScoreEncoder.FIRST_LAZER_VERSION) scoreInfo.Mods = scoreInfo.Mods.Append(currentRuleset.GetAllMods().OfType().Single()).ToArray(); currentBeatmap = workingBeatmap.GetPlayableBeatmap(currentRuleset.RulesetInfo, scoreInfo.Mods); diff --git a/osu.Game/Scoring/Legacy/LegacyScoreEncoder.cs b/osu.Game/Scoring/Legacy/LegacyScoreEncoder.cs index cd77fbbdd8..288552879c 100644 --- a/osu.Game/Scoring/Legacy/LegacyScoreEncoder.cs +++ b/osu.Game/Scoring/Legacy/LegacyScoreEncoder.cs @@ -17,8 +17,14 @@ namespace osu.Game.Scoring.Legacy { /// /// Database version in stable-compatible YYYYMMDD format. + /// Should be incremented if any changes are made to the format/usage. /// - public const int LATEST_VERSION = 30000000; + public const int LATEST_VERSION = FIRST_LAZER_VERSION; + + /// + /// The first stable-compatible YYYYMMDD format version given to lazer usage of replays. + /// + public const int FIRST_LAZER_VERSION = 30000000; private readonly Score score; private readonly IBeatmap beatmap; From 0b9916b266a9be0eb888cb0484ea2d05216ed71d Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Tue, 8 Jun 2021 18:39:52 +0900 Subject: [PATCH 70/89] Add parens to declare operator precedence --- osu.Game/OsuGame.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/OsuGame.cs b/osu.Game/OsuGame.cs index 8d87baa363..2dca91cbf3 100644 --- a/osu.Game/OsuGame.cs +++ b/osu.Game/OsuGame.cs @@ -654,7 +654,7 @@ namespace osu.Game if (!(ScreenStack.CurrentScreen is IOsuScreen currentScreen)) return; - if (!((Drawable)currentScreen).IsLoaded || currentScreen.AllowBackButton && !currentScreen.OnBackButton()) + if (!((Drawable)currentScreen).IsLoaded || (currentScreen.AllowBackButton && !currentScreen.OnBackButton())) ScreenStack.Exit(); } }, From c3ea1b26e1b519bb876b13841e9557ced5ed2e5e Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Tue, 8 Jun 2021 22:48:07 +0900 Subject: [PATCH 71/89] Fix DT being doubled in multiplayer spectator --- .../OnlinePlay/Multiplayer/Spectate/MultiSpectatorScreen.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/MultiSpectatorScreen.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/MultiSpectatorScreen.cs index 277aa5d772..983daac909 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/MultiSpectatorScreen.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/MultiSpectatorScreen.cs @@ -22,6 +22,9 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Spectate // Isolates beatmap/ruleset to this screen. public override bool DisallowExternalBeatmapRulesetChanges => true; + // We are managing our own adjustments. For now, this happens inside the Player instances themselves. + public override bool AllowRateAdjustments => false; + /// /// Whether all spectating players have finished loading. /// From c8e14d771033917af1b42339af7bdba0647e3fb1 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Tue, 8 Jun 2021 23:09:18 +0900 Subject: [PATCH 72/89] Ignore non-scorable and bonus judgements --- .../Visual/Gameplay/TestSceneHitErrorMeter.cs | 28 +++++++++++++++++++ .../HUD/HitErrorMeters/BarHitErrorMeter.cs | 3 ++ .../HUD/HitErrorMeters/ColourHitErrorMeter.cs | 9 +++++- 3 files changed, 39 insertions(+), 1 deletion(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneHitErrorMeter.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneHitErrorMeter.cs index 2a12577ad8..7accaef818 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneHitErrorMeter.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneHitErrorMeter.cs @@ -109,6 +109,34 @@ namespace osu.Game.Tests.Visual.Gameplay meter => meter.ChildrenOfType().Count() == 2)); } + [Test] + public void TestBonus() + { + AddStep("OD 1", () => recreateDisplay(new OsuHitWindows(), 1)); + + AddStep("small bonus", () => newJudgement(result: HitResult.SmallBonus)); + AddAssert("no bars added", () => !this.ChildrenOfType().Any()); + AddAssert("no circle added", () => !this.ChildrenOfType().Any()); + + AddStep("large bonus", () => newJudgement(result: HitResult.LargeBonus)); + AddAssert("no bars added", () => !this.ChildrenOfType().Any()); + AddAssert("no circle added", () => !this.ChildrenOfType().Any()); + } + + [Test] + public void TestIgnore() + { + AddStep("OD 1", () => recreateDisplay(new OsuHitWindows(), 1)); + + AddStep("ignore hit", () => newJudgement(result: HitResult.IgnoreHit)); + AddAssert("no bars added", () => !this.ChildrenOfType().Any()); + AddAssert("no circle added", () => !this.ChildrenOfType().Any()); + + AddStep("ignore miss", () => newJudgement(result: HitResult.IgnoreMiss)); + AddAssert("no bars added", () => !this.ChildrenOfType().Any()); + AddAssert("no circle added", () => !this.ChildrenOfType().Any()); + } + private void recreateDisplay(HitWindows hitWindows, float overallDifficulty) { hitWindows?.SetDifficulty(overallDifficulty); diff --git a/osu.Game/Screens/Play/HUD/HitErrorMeters/BarHitErrorMeter.cs b/osu.Game/Screens/Play/HUD/HitErrorMeters/BarHitErrorMeter.cs index 0412085d1d..89f61785e8 100644 --- a/osu.Game/Screens/Play/HUD/HitErrorMeters/BarHitErrorMeter.cs +++ b/osu.Game/Screens/Play/HUD/HitErrorMeters/BarHitErrorMeter.cs @@ -217,6 +217,9 @@ namespace osu.Game.Screens.Play.HUD.HitErrorMeters if (!judgement.IsHit || judgement.HitObject.HitWindows?.WindowFor(HitResult.Miss) == 0) return; + if (!judgement.Type.IsScorable() || judgement.Type.IsBonus()) + return; + if (judgementsContainer.Count > max_concurrent_judgements) { const double quick_fade_time = 100; diff --git a/osu.Game/Screens/Play/HUD/HitErrorMeters/ColourHitErrorMeter.cs b/osu.Game/Screens/Play/HUD/HitErrorMeters/ColourHitErrorMeter.cs index 86c0de8855..dda2a6da95 100644 --- a/osu.Game/Screens/Play/HUD/HitErrorMeters/ColourHitErrorMeter.cs +++ b/osu.Game/Screens/Play/HUD/HitErrorMeters/ColourHitErrorMeter.cs @@ -7,6 +7,7 @@ using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Framework.Graphics.Shapes; using osu.Game.Rulesets.Judgements; +using osu.Game.Rulesets.Scoring; using osuTK; using osuTK.Graphics; @@ -24,7 +25,13 @@ namespace osu.Game.Screens.Play.HUD.HitErrorMeters InternalChild = judgementsFlow = new JudgementFlow(); } - protected override void OnNewJudgement(JudgementResult judgement) => judgementsFlow.Push(GetColourForHitResult(judgement.Type)); + protected override void OnNewJudgement(JudgementResult judgement) + { + if (!judgement.Type.IsScorable() || judgement.Type.IsBonus()) + return; + + judgementsFlow.Push(GetColourForHitResult(judgement.Type)); + } private class JudgementFlow : FillFlowContainer { From 00efed2c39d67d0d5f4c5b0d21509ebea2fef205 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Tue, 8 Jun 2021 23:10:21 +0900 Subject: [PATCH 73/89] Add colours for tick judgements --- osu.Game/Screens/Play/HUD/HitErrorMeters/HitErrorMeter.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/osu.Game/Screens/Play/HUD/HitErrorMeters/HitErrorMeter.cs b/osu.Game/Screens/Play/HUD/HitErrorMeters/HitErrorMeter.cs index 17a6e772fb..9844b9f10d 100644 --- a/osu.Game/Screens/Play/HUD/HitErrorMeters/HitErrorMeter.cs +++ b/osu.Game/Screens/Play/HUD/HitErrorMeters/HitErrorMeter.cs @@ -41,6 +41,8 @@ namespace osu.Game.Screens.Play.HUD.HitErrorMeters { switch (result) { + case HitResult.SmallTickMiss: + case HitResult.LargeTickMiss: case HitResult.Miss: return colours.Red; @@ -53,6 +55,8 @@ namespace osu.Game.Screens.Play.HUD.HitErrorMeters case HitResult.Good: return colours.GreenLight; + case HitResult.SmallTickHit: + case HitResult.LargeTickHit: case HitResult.Great: return colours.Blue; From 5bf4dd63588324b8b8cc409d2940cfe43223c698 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Tue, 8 Jun 2021 21:57:08 +0200 Subject: [PATCH 74/89] Move skin background to separate file --- .../Graphics/Backgrounds/SkinBackground.cs | 25 +++++++++++++++++++ .../Backgrounds/BackgroundScreenDefault.cs | 17 ------------- 2 files changed, 25 insertions(+), 17 deletions(-) create mode 100644 osu.Game/Graphics/Backgrounds/SkinBackground.cs diff --git a/osu.Game/Graphics/Backgrounds/SkinBackground.cs b/osu.Game/Graphics/Backgrounds/SkinBackground.cs new file mode 100644 index 0000000000..8be017dc91 --- /dev/null +++ b/osu.Game/Graphics/Backgrounds/SkinBackground.cs @@ -0,0 +1,25 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using osu.Framework.Allocation; +using osu.Game.Skinning; + +namespace osu.Game.Graphics.Backgrounds +{ + internal class SkinBackground : Background + { + private readonly Skin skin; + + public SkinBackground(Skin skin, string fallbackTextureName) + : base(fallbackTextureName) + { + this.skin = skin; + } + + [BackgroundDependencyLoader] + private void load() + { + Sprite.Texture = skin.GetTexture("menu-background") ?? Sprite.Texture; + } + } +} diff --git a/osu.Game/Screens/Backgrounds/BackgroundScreenDefault.cs b/osu.Game/Screens/Backgrounds/BackgroundScreenDefault.cs index 6bcfaac907..abfbb64f61 100644 --- a/osu.Game/Screens/Backgrounds/BackgroundScreenDefault.cs +++ b/osu.Game/Screens/Backgrounds/BackgroundScreenDefault.cs @@ -140,22 +140,5 @@ namespace osu.Game.Screens.Backgrounds return $@"Menu/menu-background-{currentDisplay % background_count + 1}"; } } - - private class SkinnedBackground : Background - { - private readonly Skin skin; - - public SkinnedBackground(Skin skin, string fallbackTextureName) - : base(fallbackTextureName) - { - this.skin = skin; - } - - [BackgroundDependencyLoader] - private void load() - { - Sprite.Texture = skin.GetTexture("menu-background") ?? Sprite.Texture; - } - } } } From d86ace4d11b10ce6af7ed61527007ea8c53c55dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Tue, 8 Jun 2021 21:58:44 +0200 Subject: [PATCH 75/89] Add test coverage for skin background source --- .../Visual/Background/TestSceneBackgroundScreenDefault.cs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/osu.Game.Tests/Visual/Background/TestSceneBackgroundScreenDefault.cs b/osu.Game.Tests/Visual/Background/TestSceneBackgroundScreenDefault.cs index 09fe9b3767..c574d89ad9 100644 --- a/osu.Game.Tests/Visual/Background/TestSceneBackgroundScreenDefault.cs +++ b/osu.Game.Tests/Visual/Background/TestSceneBackgroundScreenDefault.cs @@ -35,7 +35,7 @@ namespace osu.Game.Tests.Visual.Background } [Test] - public void TestTogglingStoryboardSwitchesBackgroundType() + public void TestBackgroundTypeSwitch() { setSupporter(true); @@ -44,6 +44,9 @@ namespace osu.Game.Tests.Visual.Background setSourceMode(BackgroundSource.BeatmapWithStoryboard); AddUntilStep("is storyboard background", () => getCurrentBackground() is BeatmapBackgroundWithStoryboard); + + setSourceMode(BackgroundSource.Skin); + AddUntilStep("is skin background", () => getCurrentBackground() is SkinBackground); } [Test] @@ -78,7 +81,7 @@ namespace osu.Game.Tests.Visual.Background } private void setSourceMode(BackgroundSource source) => - AddStep("set background mode to beatmap", () => config.SetValue(OsuSetting.MenuBackgroundSource, source)); + AddStep($"set background mode to {source}", () => config.SetValue(OsuSetting.MenuBackgroundSource, source)); private void setSupporter(bool isSupporter) => AddStep($"set supporter {isSupporter}", () => ((DummyAPIAccess)API).LocalUser.Value = new User From a98c302211d61d3139f0e981ebeb52e0c6680f64 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Tue, 8 Jun 2021 22:04:59 +0200 Subject: [PATCH 76/89] Bring back skin background source --- osu.Game/Screens/Backgrounds/BackgroundScreenDefault.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/osu.Game/Screens/Backgrounds/BackgroundScreenDefault.cs b/osu.Game/Screens/Backgrounds/BackgroundScreenDefault.cs index abfbb64f61..46574ff8c8 100644 --- a/osu.Game/Screens/Backgrounds/BackgroundScreenDefault.cs +++ b/osu.Game/Screens/Backgrounds/BackgroundScreenDefault.cs @@ -120,6 +120,10 @@ namespace osu.Game.Screens.Backgrounds break; } + + case BackgroundSource.Skin: + newBackground = new SkinBackground(skin.Value, getBackgroundTextureName()); + break; } } From f628ec25ef9b80b1b88f5d380f4322328bbdafd5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Tue, 8 Jun 2021 22:25:29 +0200 Subject: [PATCH 77/89] Add test coverage for keeping same background instance --- .../Background/TestSceneBackgroundScreenDefault.cs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/osu.Game.Tests/Visual/Background/TestSceneBackgroundScreenDefault.cs b/osu.Game.Tests/Visual/Background/TestSceneBackgroundScreenDefault.cs index c574d89ad9..135998695b 100644 --- a/osu.Game.Tests/Visual/Background/TestSceneBackgroundScreenDefault.cs +++ b/osu.Game.Tests/Visual/Background/TestSceneBackgroundScreenDefault.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.Linq; using NUnit.Framework; using osu.Framework.Allocation; @@ -64,15 +65,17 @@ namespace osu.Game.Tests.Visual.Background AddUntilStep("is beatmap background", () => getCurrentBackground() is BeatmapBackground); } - [Test] - public void TestBeatmapDoesntReloadOnNoChange() + [TestCase(BackgroundSource.Beatmap, typeof(BeatmapBackground))] + [TestCase(BackgroundSource.BeatmapWithStoryboard, typeof(BeatmapBackgroundWithStoryboard))] + [TestCase(BackgroundSource.Skin, typeof(SkinBackground))] + public void TestBackgroundDoesntReloadOnNoChange(BackgroundSource source, Type backgroundType) { - BeatmapBackground last = null; + Graphics.Backgrounds.Background last = null; - setSourceMode(BackgroundSource.Beatmap); + setSourceMode(source); setSupporter(true); - AddUntilStep("wait for beatmap background to be loaded", () => (last = getCurrentBackground() as BeatmapBackground) != null); + AddUntilStep("wait for beatmap background to be loaded", () => (last = getCurrentBackground())?.GetType() == backgroundType); AddAssert("next doesn't load new background", () => screen.Next() == false); // doesn't really need to be checked but might as well. From 97204b6f278408f97fe3ab2c5cec289445c6810a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Tue, 8 Jun 2021 22:26:15 +0200 Subject: [PATCH 78/89] Reduce unnecessary background changes via `IEquatable` implementation --- osu.Game/Graphics/Backgrounds/Background.cs | 12 +++++++++++- osu.Game/Graphics/Backgrounds/BeatmapBackground.cs | 9 +++++++++ osu.Game/Graphics/Backgrounds/SkinBackground.cs | 9 +++++++++ .../Screens/Backgrounds/BackgroundScreenDefault.cs | 11 +++++------ 4 files changed, 34 insertions(+), 7 deletions(-) diff --git a/osu.Game/Graphics/Backgrounds/Background.cs b/osu.Game/Graphics/Backgrounds/Background.cs index c90b1e0e98..cfc1eb1806 100644 --- a/osu.Game/Graphics/Backgrounds/Background.cs +++ b/osu.Game/Graphics/Backgrounds/Background.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.Allocation; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; @@ -14,7 +15,7 @@ namespace osu.Game.Graphics.Backgrounds /// /// A background which offers blurring via a on demand. /// - public class Background : CompositeDrawable + public class Background : CompositeDrawable, IEquatable { private const float blur_scale = 0.5f; @@ -71,5 +72,14 @@ namespace osu.Game.Graphics.Backgrounds bufferedContainer?.BlurTo(newBlurSigma * blur_scale, duration, easing); } + + public virtual bool Equals(Background other) + { + if (ReferenceEquals(null, other)) return false; + if (ReferenceEquals(this, other)) return true; + + return other.GetType() == GetType() + && other.textureName == textureName; + } } } diff --git a/osu.Game/Graphics/Backgrounds/BeatmapBackground.cs b/osu.Game/Graphics/Backgrounds/BeatmapBackground.cs index 058d2ed0f9..e0c15dd52a 100644 --- a/osu.Game/Graphics/Backgrounds/BeatmapBackground.cs +++ b/osu.Game/Graphics/Backgrounds/BeatmapBackground.cs @@ -24,5 +24,14 @@ namespace osu.Game.Graphics.Backgrounds { Sprite.Texture = Beatmap?.Background ?? textures.Get(fallbackTextureName); } + + public override bool Equals(Background other) + { + if (ReferenceEquals(null, other)) return false; + if (ReferenceEquals(this, other)) return true; + + return other.GetType() == GetType() + && ((BeatmapBackground)other).Beatmap == Beatmap; + } } } diff --git a/osu.Game/Graphics/Backgrounds/SkinBackground.cs b/osu.Game/Graphics/Backgrounds/SkinBackground.cs index 8be017dc91..9266e7b17b 100644 --- a/osu.Game/Graphics/Backgrounds/SkinBackground.cs +++ b/osu.Game/Graphics/Backgrounds/SkinBackground.cs @@ -21,5 +21,14 @@ namespace osu.Game.Graphics.Backgrounds { Sprite.Texture = skin.GetTexture("menu-background") ?? Sprite.Texture; } + + public override bool Equals(Background other) + { + if (ReferenceEquals(null, other)) return false; + if (ReferenceEquals(this, other)) return true; + + return other.GetType() == GetType() + && ((SkinBackground)other).skin.SkinInfo.Equals(skin.SkinInfo); + } } } diff --git a/osu.Game/Screens/Backgrounds/BackgroundScreenDefault.cs b/osu.Game/Screens/Backgrounds/BackgroundScreenDefault.cs index 46574ff8c8..81b15570d2 100644 --- a/osu.Game/Screens/Backgrounds/BackgroundScreenDefault.cs +++ b/osu.Game/Screens/Backgrounds/BackgroundScreenDefault.cs @@ -112,12 +112,6 @@ namespace osu.Game.Screens.Backgrounds newBackground = new BeatmapBackgroundWithStoryboard(beatmap.Value, getBackgroundTextureName()); newBackground ??= new BeatmapBackground(beatmap.Value, getBackgroundTextureName()); - // this method is called in many cases where the beatmap hasn't changed (ie. on screen transitions). - // if a background is already displayed for the requested beatmap, we don't want to load it again. - if (background?.GetType() == newBackground.GetType() && - (background as BeatmapBackground)?.Beatmap == beatmap.Value) - return background; - break; } @@ -127,6 +121,11 @@ namespace osu.Game.Screens.Backgrounds } } + // this method is called in many cases where the background might not necessarily need to change. + // if an equivalent background is currently being shown, we don't want to load it again. + if (newBackground?.Equals(background) == true) + return background; + newBackground ??= new Background(getBackgroundTextureName()); newBackground.Depth = currentDisplay; From 4707918c6afedf0cff2111dbbdfc2833aba75b11 Mon Sep 17 00:00:00 2001 From: ekrctb Date: Wed, 9 Jun 2021 10:53:52 +0900 Subject: [PATCH 79/89] Fix hit circle animation when a replay is rewound --- osu.Game.Rulesets.Osu/Skinning/Default/MainCirclePiece.cs | 2 ++ osu.Game.Rulesets.Osu/Skinning/Legacy/LegacyMainCirclePiece.cs | 2 ++ 2 files changed, 4 insertions(+) diff --git a/osu.Game.Rulesets.Osu/Skinning/Default/MainCirclePiece.cs b/osu.Game.Rulesets.Osu/Skinning/Default/MainCirclePiece.cs index b52dc749f0..fece3494e6 100644 --- a/osu.Game.Rulesets.Osu/Skinning/Default/MainCirclePiece.cs +++ b/osu.Game.Rulesets.Osu/Skinning/Default/MainCirclePiece.cs @@ -15,6 +15,8 @@ namespace osu.Game.Rulesets.Osu.Skinning.Default { public class MainCirclePiece : CompositeDrawable { + public override bool RemoveCompletedTransforms => false; + private readonly CirclePiece circle; private readonly RingPiece ring; private readonly FlashPiece flash; diff --git a/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacyMainCirclePiece.cs b/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacyMainCirclePiece.cs index 822dad8523..e5200ac248 100644 --- a/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacyMainCirclePiece.cs +++ b/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacyMainCirclePiece.cs @@ -20,6 +20,8 @@ namespace osu.Game.Rulesets.Osu.Skinning.Legacy { public class LegacyMainCirclePiece : CompositeDrawable { + public override bool RemoveCompletedTransforms => false; + private readonly string priorityLookup; private readonly bool hasNumber; From 555ab8fccd61f3aff1ee9237e9da873d02a9b500 Mon Sep 17 00:00:00 2001 From: ekrctb Date: Wed, 9 Jun 2021 12:31:30 +0900 Subject: [PATCH 80/89] Fix event not unregistered on dispose --- osu.Game.Rulesets.Osu/Skinning/Default/DefaultSpinner.cs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/osu.Game.Rulesets.Osu/Skinning/Default/DefaultSpinner.cs b/osu.Game.Rulesets.Osu/Skinning/Default/DefaultSpinner.cs index ae8c03dad1..df33bf52be 100644 --- a/osu.Game.Rulesets.Osu/Skinning/Default/DefaultSpinner.cs +++ b/osu.Game.Rulesets.Osu/Skinning/Default/DefaultSpinner.cs @@ -128,5 +128,13 @@ namespace osu.Game.Rulesets.Osu.Skinning.Default spmContainer.FadeIn(drawableSpinner.HitObject.TimeFadeIn); } } + + protected override void Dispose(bool isDisposing) + { + base.Dispose(isDisposing); + + if (drawableSpinner != null) + drawableSpinner.ApplyCustomUpdateState -= updateStateTransforms; + } } } From a87226ab10a241f52bb10129622d9e3420bc44fa Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 9 Jun 2021 14:08:27 +0900 Subject: [PATCH 81/89] Remove obsoleted `DrawableJudgement` methods Undated, but change was made on 2020-11-18. --- osu.Game/Rulesets/Judgements/DrawableJudgement.cs | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/osu.Game/Rulesets/Judgements/DrawableJudgement.cs b/osu.Game/Rulesets/Judgements/DrawableJudgement.cs index feeafb7151..8a57b4af91 100644 --- a/osu.Game/Rulesets/Judgements/DrawableJudgement.cs +++ b/osu.Game/Rulesets/Judgements/DrawableJudgement.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 System.Diagnostics; using JetBrains.Annotations; using osu.Framework.Allocation; @@ -32,18 +31,6 @@ namespace osu.Game.Rulesets.Judgements private readonly Container aboveHitObjectsContent; - /// - /// Duration of initial fade in. - /// - [Obsolete("Apply any animations manually via ApplyHitAnimations / ApplyMissAnimations. Defaults were moved inside skinned components.")] - protected virtual double FadeInDuration => 100; - - /// - /// Duration to wait until fade out begins. Defaults to . - /// - [Obsolete("Apply any animations manually via ApplyHitAnimations / ApplyMissAnimations. Defaults were moved inside skinned components.")] - protected virtual double FadeOutDelay => FadeInDuration; - /// /// Creates a drawable which visualises a . /// From f41e34ae2c840adadc16dffb04994b1da815a4ed Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 9 Jun 2021 14:09:28 +0900 Subject: [PATCH 82/89] Remove more obsoleted members --- osu.Game/Graphics/Backgrounds/Triangles.cs | 6 ---- osu.Game/Rulesets/Judgements/Judgement.cs | 9 ----- .../Objects/Drawables/DrawableHitObject.cs | 36 ++----------------- 3 files changed, 2 insertions(+), 49 deletions(-) diff --git a/osu.Game/Graphics/Backgrounds/Triangles.cs b/osu.Game/Graphics/Backgrounds/Triangles.cs index 67cee883c8..269360c492 100644 --- a/osu.Game/Graphics/Backgrounds/Triangles.cs +++ b/osu.Game/Graphics/Backgrounds/Triangles.cs @@ -57,12 +57,6 @@ namespace osu.Game.Graphics.Backgrounds } } - /// - /// Whether we want to expire triangles as they exit our draw area completely. - /// - [Obsolete("Unused.")] // Can be removed 20210518 - protected virtual bool ExpireOffScreenTriangles => true; - /// /// Whether we should create new triangles as others expire. /// diff --git a/osu.Game/Rulesets/Judgements/Judgement.cs b/osu.Game/Rulesets/Judgements/Judgement.cs index be69db5ca8..fd576e9b9f 100644 --- a/osu.Game/Rulesets/Judgements/Judgement.cs +++ b/osu.Game/Rulesets/Judgements/Judgement.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.Game.Rulesets.Objects; using osu.Game.Rulesets.Scoring; @@ -69,14 +68,6 @@ namespace osu.Game.Rulesets.Judgements /// public double MaxHealthIncrease => HealthIncreaseFor(MaxResult); - /// - /// Retrieves the numeric score representation of a . - /// - /// The to find the numeric score representation for. - /// The numeric score representation of . - [Obsolete("Has no effect. Use ToNumericResult(HitResult) (standardised across all rulesets).")] // Can be made non-virtual 20210328 - protected virtual int NumericResultFor(HitResult result) => ToNumericResult(result); - /// /// Retrieves the numeric score representation of a . /// diff --git a/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs b/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs index 6c688c1625..ef4fc1bc15 100644 --- a/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs +++ b/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs @@ -409,11 +409,6 @@ namespace osu.Game.Rulesets.Objects.Drawables using (BeginAbsoluteSequence(StateUpdateTime, true)) UpdateStartTimeStateTransforms(); -#pragma warning disable 618 - using (BeginAbsoluteSequence(StateUpdateTime + (Result?.TimeOffset ?? 0), true)) - UpdateStateTransforms(newState); -#pragma warning restore 618 - using (BeginAbsoluteSequence(HitStateUpdateTime, true)) UpdateHitStateTransforms(newState); @@ -447,7 +442,7 @@ namespace osu.Game.Rulesets.Objects.Drawables /// By default, this will fade in the object from zero with no duration. /// /// - /// This is called once before every . This is to ensure a good state in the case + /// This is called once before every . This is to ensure a good state in the case /// the was negative and potentially altered the pre-hit transforms. /// protected virtual void UpdateInitialTransforms() @@ -455,16 +450,6 @@ namespace osu.Game.Rulesets.Objects.Drawables this.FadeInFromZero(); } - /// - /// Apply transforms based on the current . Previous states are automatically cleared. - /// In the case of a non-idle , and if was not set during this call, will be invoked. - /// - /// The new armed state. - [Obsolete("Use UpdateStartTimeStateTransforms and UpdateHitStateTransforms instead")] // Can be removed 20210504 - protected virtual void UpdateStateTransforms(ArmedState state) - { - } - /// /// Apply passive transforms at the 's StartTime. /// This is called each time changes. @@ -520,23 +505,6 @@ namespace osu.Game.Rulesets.Objects.Drawables AccentColour.Value = combo.GetComboColour(comboColours); } - /// - /// Called to retrieve the combo colour. Automatically assigned to . - /// Defaults to using to decide on a colour. - /// - /// - /// This will only be called if the implements . - /// - /// A list of combo colours provided by the beatmap or skin. Can be null if not available. - [Obsolete("Unused. Implement IHasComboInformation and IHasComboInformation.GetComboColour() on the HitObject model instead.")] // Can be removed 20210527 - protected virtual Color4 GetComboColour(IReadOnlyList comboColours) - { - if (!(HitObject is IHasComboInformation combo)) - throw new InvalidOperationException($"{nameof(HitObject)} must implement {nameof(IHasComboInformation)}"); - - return comboColours?.Count > 0 ? comboColours[combo.ComboIndex % comboColours.Count] : Color4.White; - } - /// /// Called when a change is made to the skin. /// @@ -630,7 +598,7 @@ namespace osu.Game.Rulesets.Objects.Drawables /// /// The time at which state transforms should be applied that line up to 's StartTime. - /// This is used to offset calls to . + /// This is used to offset calls to . /// public double StateUpdateTime => HitObject.StartTime; From 62199a38a80716aa2617259b051f4d29996ab8d2 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 9 Jun 2021 14:11:50 +0900 Subject: [PATCH 83/89] Add one missing obsoletion removal date --- osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs b/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs index ef4fc1bc15..8818e4c14a 100644 --- a/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs +++ b/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs @@ -192,7 +192,7 @@ namespace osu.Game.Rulesets.Objects.Drawables /// /// Applies a hit object to be represented by this . /// - [Obsolete("Use either overload of Apply that takes a single argument of type HitObject or HitObjectLifetimeEntry")] + [Obsolete("Use either overload of Apply that takes a single argument of type HitObject or HitObjectLifetimeEntry")] // Can be removed 20211021. public void Apply([NotNull] HitObject hitObject, [CanBeNull] HitObjectLifetimeEntry lifetimeEntry) { if (lifetimeEntry != null) From b8df3fff9e26a4454240c2c562c6598d42cdd9bd Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 9 Jun 2021 14:20:01 +0900 Subject: [PATCH 84/89] Fix incorrect method referenced in xmldco Co-authored-by: ekrctb <32995012+ekrctb@users.noreply.github.com> --- osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs b/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs index 8818e4c14a..5fd2b2493e 100644 --- a/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs +++ b/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs @@ -598,7 +598,7 @@ namespace osu.Game.Rulesets.Objects.Drawables /// /// The time at which state transforms should be applied that line up to 's StartTime. - /// This is used to offset calls to . + /// This is used to offset calls to . /// public double StateUpdateTime => HitObject.StartTime; From 5487012060e239cbcfca3028adc71f5a03cec6c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 9 Jun 2021 07:48:16 +0200 Subject: [PATCH 85/89] Add test coverage for default skin background cycling --- .../TestSceneBackgroundScreenDefault.cs | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/osu.Game.Tests/Visual/Background/TestSceneBackgroundScreenDefault.cs b/osu.Game.Tests/Visual/Background/TestSceneBackgroundScreenDefault.cs index 135998695b..f7d42a2ee6 100644 --- a/osu.Game.Tests/Visual/Background/TestSceneBackgroundScreenDefault.cs +++ b/osu.Game.Tests/Visual/Background/TestSceneBackgroundScreenDefault.cs @@ -12,6 +12,7 @@ using osu.Game.Graphics.Backgrounds; using osu.Game.Online.API; using osu.Game.Screens; using osu.Game.Screens.Backgrounds; +using osu.Game.Skinning; using osu.Game.Users; namespace osu.Game.Tests.Visual.Background @@ -24,6 +25,9 @@ namespace osu.Game.Tests.Visual.Background private Graphics.Backgrounds.Background getCurrentBackground() => screen.ChildrenOfType().FirstOrDefault(); + [Resolved] + private SkinManager skins { get; set; } + [Resolved] private OsuConfigManager config { get; set; } @@ -47,6 +51,9 @@ namespace osu.Game.Tests.Visual.Background AddUntilStep("is storyboard background", () => getCurrentBackground() is BeatmapBackgroundWithStoryboard); setSourceMode(BackgroundSource.Skin); + AddUntilStep("is default background", () => getCurrentBackground().GetType() == typeof(Graphics.Backgrounds.Background)); + + setCustomSkin(); AddUntilStep("is skin background", () => getCurrentBackground() is SkinBackground); } @@ -74,6 +81,8 @@ namespace osu.Game.Tests.Visual.Background setSourceMode(source); setSupporter(true); + if (source == BackgroundSource.Skin) + setCustomSkin(); AddUntilStep("wait for beatmap background to be loaded", () => (last = getCurrentBackground())?.GetType() == backgroundType); AddAssert("next doesn't load new background", () => screen.Next() == false); @@ -83,6 +92,23 @@ namespace osu.Game.Tests.Visual.Background AddUntilStep("ensure same background instance", () => last == getCurrentBackground()); } + [Test] + public void TestBackgroundCyclingOnDefaultSkin([Values] bool supporter) + { + Graphics.Backgrounds.Background last = null; + + setSourceMode(BackgroundSource.Skin); + setSupporter(supporter); + setDefaultSkin(); + + AddUntilStep("wait for beatmap background to be loaded", () => (last = getCurrentBackground())?.GetType() == typeof(Graphics.Backgrounds.Background)); + AddAssert("next cycles background", () => screen.Next()); + + // doesn't really need to be checked but might as well. + AddWaitStep("wait a bit", 5); + AddUntilStep("ensure different background instance", () => last != getCurrentBackground()); + } + private void setSourceMode(BackgroundSource source) => AddStep($"set background mode to {source}", () => config.SetValue(OsuSetting.MenuBackgroundSource, source)); @@ -92,5 +118,16 @@ namespace osu.Game.Tests.Visual.Background IsSupporter = isSupporter, Id = API.LocalUser.Value.Id + 1, }); + + private void setCustomSkin() + { + // feign a skin switch. this doesn't do anything except force CurrentSkin to become a LegacySkin. + AddStep("set custom skin", () => skins.CurrentSkinInfo.Value = new SkinInfo { ID = 5 }); + } + + private void setDefaultSkin() => AddStep("set default skin", () => skins.CurrentSkinInfo.SetDefault()); + + [TearDownSteps] + public void TearDown() => setDefaultSkin(); } } From a801a9a14d7b52fbafb3f88a43180f53af53552a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 9 Jun 2021 07:59:47 +0200 Subject: [PATCH 86/89] Ensure background rotation on default skins --- osu.Game/Screens/Backgrounds/BackgroundScreenDefault.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/osu.Game/Screens/Backgrounds/BackgroundScreenDefault.cs b/osu.Game/Screens/Backgrounds/BackgroundScreenDefault.cs index 81b15570d2..f0c90cc409 100644 --- a/osu.Game/Screens/Backgrounds/BackgroundScreenDefault.cs +++ b/osu.Game/Screens/Backgrounds/BackgroundScreenDefault.cs @@ -116,6 +116,10 @@ namespace osu.Game.Screens.Backgrounds } case BackgroundSource.Skin: + // default skins should use the default background rotation, which won't be the case if a SkinBackground is created for them. + if (skin.Value is DefaultSkin || skin.Value is DefaultLegacySkin) + break; + newBackground = new SkinBackground(skin.Value, getBackgroundTextureName()); break; } From d248bbd4c8dff39fafc7c316f37033d6a563e490 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 9 Jun 2021 15:00:55 +0900 Subject: [PATCH 87/89] Use candidate skin for mania skin key lookup rather than `this` --- .../Skinning/Legacy/ManiaLegacySkinTransformer.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Rulesets.Mania/Skinning/Legacy/ManiaLegacySkinTransformer.cs b/osu.Game.Rulesets.Mania/Skinning/Legacy/ManiaLegacySkinTransformer.cs index 8aa0c85433..962a13ebea 100644 --- a/osu.Game.Rulesets.Mania/Skinning/Legacy/ManiaLegacySkinTransformer.cs +++ b/osu.Game.Rulesets.Mania/Skinning/Legacy/ManiaLegacySkinTransformer.cs @@ -71,7 +71,7 @@ namespace osu.Game.Rulesets.Mania.Skinning.Legacy { 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 + s.GetManiaSkinConfig(LegacyManiaSkinConfigurationLookups.KeyImage, 0)?.Value ?? "mania-key1", true, true) != null) != null); } From 610cdaea98bb1fb9720035cfdbdaa113c542bed8 Mon Sep 17 00:00:00 2001 From: ekrctb Date: Wed, 9 Jun 2021 16:14:55 +0900 Subject: [PATCH 88/89] Fix circle piece animation is sometimes not playing when a replay is rewound --- .../Skinning/Default/MainCirclePiece.cs | 21 +++++++++++-------- .../Skinning/Legacy/LegacyMainCirclePiece.cs | 17 ++++++++++----- 2 files changed, 24 insertions(+), 14 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Skinning/Default/MainCirclePiece.cs b/osu.Game.Rulesets.Osu/Skinning/Default/MainCirclePiece.cs index fece3494e6..d7ebe9333d 100644 --- a/osu.Game.Rulesets.Osu/Skinning/Default/MainCirclePiece.cs +++ b/osu.Game.Rulesets.Osu/Skinning/Default/MainCirclePiece.cs @@ -15,8 +15,6 @@ namespace osu.Game.Rulesets.Osu.Skinning.Default { public class MainCirclePiece : CompositeDrawable { - public override bool RemoveCompletedTransforms => false; - private readonly CirclePiece circle; private readonly RingPiece ring; private readonly FlashPiece flash; @@ -44,7 +42,6 @@ namespace osu.Game.Rulesets.Osu.Skinning.Default private readonly IBindable accentColour = new Bindable(); private readonly IBindable indexInCurrentCombo = new Bindable(); - private readonly IBindable armedState = new Bindable(); [Resolved] private DrawableHitObject drawableObject { get; set; } @@ -56,7 +53,6 @@ namespace osu.Game.Rulesets.Osu.Skinning.Default accentColour.BindTo(drawableObject.AccentColour); indexInCurrentCombo.BindTo(drawableOsuObject.IndexInCurrentComboBindable); - armedState.BindTo(drawableObject.State); } protected override void LoadComplete() @@ -72,19 +68,18 @@ namespace osu.Game.Rulesets.Osu.Skinning.Default indexInCurrentCombo.BindValueChanged(index => number.Text = (index.NewValue + 1).ToString(), true); - armedState.BindValueChanged(animate, true); + drawableObject.ApplyCustomUpdateState += updateStateTransforms; + updateStateTransforms(drawableObject, drawableObject.State.Value); } - private void animate(ValueChangedEvent state) + private void updateStateTransforms(DrawableHitObject drawableHitObject, ArmedState state) { - ClearTransforms(true); - using (BeginAbsoluteSequence(drawableObject.StateUpdateTime)) glow.FadeOut(400); using (BeginAbsoluteSequence(drawableObject.HitStateUpdateTime)) { - switch (state.NewValue) + switch (state) { case ArmedState.Hit: const double flash_in = 40; @@ -111,5 +106,13 @@ namespace osu.Game.Rulesets.Osu.Skinning.Default } } } + + protected override void Dispose(bool isDisposing) + { + base.Dispose(isDisposing); + + if (drawableObject != null) + drawableObject.ApplyCustomUpdateState -= updateStateTransforms; + } } } diff --git a/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacyMainCirclePiece.cs b/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacyMainCirclePiece.cs index e5200ac248..bb7a335efe 100644 --- a/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacyMainCirclePiece.cs +++ b/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacyMainCirclePiece.cs @@ -41,7 +41,6 @@ namespace osu.Game.Rulesets.Osu.Skinning.Legacy private readonly Bindable accentColour = new Bindable(); private readonly IBindable indexInCurrentCombo = new Bindable(); - private readonly IBindable armedState = new Bindable(); [Resolved] private DrawableHitObject drawableObject { get; set; } @@ -116,7 +115,6 @@ namespace osu.Game.Rulesets.Osu.Skinning.Legacy accentColour.BindTo(drawableObject.AccentColour); indexInCurrentCombo.BindTo(drawableOsuObject.IndexInCurrentComboBindable); - armedState.BindTo(drawableObject.State); Texture getTextureWithFallback(string name) { @@ -142,10 +140,11 @@ namespace osu.Game.Rulesets.Osu.Skinning.Legacy if (hasNumber) indexInCurrentCombo.BindValueChanged(index => hitCircleText.Text = (index.NewValue + 1).ToString(), true); - armedState.BindValueChanged(animate, true); + drawableObject.ApplyCustomUpdateState += updateStateTransforms; + updateStateTransforms(drawableObject, drawableObject.State.Value); } - private void animate(ValueChangedEvent state) + private void updateStateTransforms(DrawableHitObject drawableHitObject, ArmedState state) { const double legacy_fade_duration = 240; @@ -153,7 +152,7 @@ namespace osu.Game.Rulesets.Osu.Skinning.Legacy using (BeginAbsoluteSequence(drawableObject.HitStateUpdateTime)) { - switch (state.NewValue) + switch (state) { case ArmedState.Hit: circleSprites.FadeOut(legacy_fade_duration, Easing.Out); @@ -178,5 +177,13 @@ namespace osu.Game.Rulesets.Osu.Skinning.Legacy } } } + + protected override void Dispose(bool isDisposing) + { + base.Dispose(isDisposing); + + if (drawableObject != null) + drawableObject.ApplyCustomUpdateState -= updateStateTransforms; + } } } From 5418e895ae93b31f9515e7100a4a8136cd04756f Mon Sep 17 00:00:00 2001 From: ekrctb Date: Wed, 9 Jun 2021 16:50:13 +0900 Subject: [PATCH 89/89] Remove useless `ClearTransforms` The transforms are cleared by DHO before `ApplyCustomUpdateState` is invoked. --- osu.Game.Rulesets.Osu/Skinning/Legacy/LegacyMainCirclePiece.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacyMainCirclePiece.cs b/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacyMainCirclePiece.cs index bb7a335efe..7a210324d7 100644 --- a/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacyMainCirclePiece.cs +++ b/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacyMainCirclePiece.cs @@ -148,8 +148,6 @@ namespace osu.Game.Rulesets.Osu.Skinning.Legacy { const double legacy_fade_duration = 240; - ClearTransforms(true); - using (BeginAbsoluteSequence(drawableObject.HitStateUpdateTime)) { switch (state)