From 3a16483214bb564dbf75f5bd61f6c7623e0783b5 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 23 Mar 2022 15:10:26 +0900 Subject: [PATCH 01/19] Add prioritised user lookups for default skin This allows user resources to be consumed before falling back to the game bundled assets. --- osu.Game/Skinning/DefaultSkin.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game/Skinning/DefaultSkin.cs b/osu.Game/Skinning/DefaultSkin.cs index 7c6d138f4c..43ada59bcb 100644 --- a/osu.Game/Skinning/DefaultSkin.cs +++ b/osu.Game/Skinning/DefaultSkin.cs @@ -46,13 +46,13 @@ namespace osu.Game.Skinning this.resources = resources; } - public override Texture GetTexture(string componentName, WrapMode wrapModeS, WrapMode wrapModeT) => null; + public override Texture GetTexture(string componentName, WrapMode wrapModeS, WrapMode wrapModeT) => Textures?.Get(componentName, wrapModeS, wrapModeT); public override ISample GetSample(ISampleInfo sampleInfo) { foreach (string lookup in sampleInfo.LookupNames) { - var sample = resources.AudioManager.Samples.Get(lookup); + var sample = Samples?.Get(lookup) ?? resources.AudioManager.Samples.Get(lookup); if (sample != null) return sample; } From fca9faac9b87eb9c24d86e2d52764619bbe2ddf0 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 23 Mar 2022 15:11:35 +0900 Subject: [PATCH 02/19] Add `SkinnableSprite` for arbitrary sprite additions --- .../Skinning/Components/SkinnableSprite.cs | 49 ++++++++++++++++ osu.Game/Skinning/Editor/SkinEditor.cs | 56 +++++++++++++++++-- osu.Game/Skinning/SkinManager.cs | 44 ++++++++++++++- osu.Game/Skinning/SkinnableSprite.cs | 4 +- 4 files changed, 147 insertions(+), 6 deletions(-) create mode 100644 osu.Game/Skinning/Components/SkinnableSprite.cs diff --git a/osu.Game/Skinning/Components/SkinnableSprite.cs b/osu.Game/Skinning/Components/SkinnableSprite.cs new file mode 100644 index 0000000000..292bbe2321 --- /dev/null +++ b/osu.Game/Skinning/Components/SkinnableSprite.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 JetBrains.Annotations; +using osu.Framework.Allocation; +using osu.Framework.Bindables; +using osu.Framework.Graphics; +using osu.Framework.Graphics.Containers; +using osu.Framework.Graphics.Sprites; +using osu.Game.Configuration; + +namespace osu.Game.Skinning.Components +{ + /// + /// Intended to be a test bed for skinning. May be removed at some point in the future. + /// + [UsedImplicitly] + public class SkinSprite : CompositeDrawable, ISkinnableDrawable + { + public bool UsesFixedAnchor { get; set; } + + [SettingSource("Sprite name", "The filename of the sprite")] + public Bindable SpriteName { get; } = new Bindable(string.Empty); + + [Resolved] + private ISkinSource source { get; set; } + + public SkinSprite() + { + AutoSizeAxes = Axes.Both; + } + + protected override void LoadComplete() + { + base.LoadComplete(); + + SpriteName.BindValueChanged(spriteName => + { + InternalChildren = new Drawable[] + { + new Sprite + { + Texture = source.GetTexture(SpriteName.Value), + } + }; + }, true); + } + } +} diff --git a/osu.Game/Skinning/Editor/SkinEditor.cs b/osu.Game/Skinning/Editor/SkinEditor.cs index 7bf4e94662..7701fafbfc 100644 --- a/osu.Game/Skinning/Editor/SkinEditor.cs +++ b/osu.Game/Skinning/Editor/SkinEditor.cs @@ -3,7 +3,9 @@ using System; using System.Collections.Generic; +using System.IO; using System.Linq; +using System.Threading.Tasks; using osu.Framework.Allocation; using osu.Framework.Bindables; using osu.Framework.Graphics; @@ -11,6 +13,7 @@ using osu.Framework.Graphics.Containers; using osu.Framework.Graphics.UserInterface; using osu.Framework.Input.Events; using osu.Framework.Testing; +using osu.Game.Database; using osu.Game.Graphics; using osu.Game.Graphics.Containers; using osu.Game.Graphics.Cursor; @@ -18,11 +21,12 @@ using osu.Game.Graphics.UserInterface; using osu.Game.Overlays; using osu.Game.Screens.Edit.Components; using osu.Game.Screens.Edit.Components.Menus; +using osu.Game.Skinning.Components; namespace osu.Game.Skinning.Editor { [Cached(typeof(SkinEditor))] - public class SkinEditor : VisibilityContainer + public class SkinEditor : VisibilityContainer, ICanAcceptFiles { public const double TRANSITION_DURATION = 500; @@ -36,6 +40,9 @@ namespace osu.Game.Skinning.Editor private Bindable currentSkin; + [Resolved(canBeNull: true)] + private OsuGame game { get; set; } + [Resolved] private SkinManager skins { get; set; } @@ -171,6 +178,8 @@ namespace osu.Game.Skinning.Editor Show(); + game?.RegisterImportHandler(this); + // as long as the skin editor is loaded, let's make sure we can modify the current skin. currentSkin = skins.CurrentSkin.GetBoundCopy(); @@ -186,6 +195,13 @@ namespace osu.Game.Skinning.Editor SelectedComponents.BindCollectionChanged((_, __) => Scheduler.AddOnce(populateSettings), true); } + protected override void Dispose(bool isDisposing) + { + base.Dispose(isDisposing); + + game?.UnregisterImportHandler(this); + } + public void UpdateTargetScreen(Drawable targetScreen) { this.targetScreen = targetScreen; @@ -229,15 +245,20 @@ namespace osu.Game.Skinning.Editor } private void placeComponent(Type type) + { + if (!(Activator.CreateInstance(type) is ISkinnableDrawable component)) + throw new InvalidOperationException($"Attempted to instantiate a component for placement which was not an {typeof(ISkinnableDrawable)}."); + + placeComponent(component); + } + + private void placeComponent(ISkinnableDrawable component) { var targetContainer = getFirstTarget(); if (targetContainer == null) return; - if (!(Activator.CreateInstance(type) is ISkinnableDrawable component)) - throw new InvalidOperationException($"Attempted to instantiate a component for placement which was not an {typeof(ISkinnableDrawable)}."); - var drawableComponent = (Drawable)component; // give newly added components a sane starting location. @@ -313,5 +334,32 @@ namespace osu.Game.Skinning.Editor foreach (var item in items) availableTargets.FirstOrDefault(t => t.Components.Contains(item))?.Remove(item); } + + public Task Import(params string[] paths) + { + Schedule(() => + { + var file = new FileInfo(paths.First()); + + // import to skin + currentSkin.Value.SkinInfo.PerformWrite(skinInfo => + { + using (var contents = file.OpenRead()) + skins.AddFile(skinInfo, contents, file.Name); + }); + + // place component + placeComponent(new SkinSprite + { + SpriteName = { Value = Path.GetFileNameWithoutExtension(file.Name) } + }); + }); + + return Task.CompletedTask; + } + + public Task Import(params ImportTask[] tasks) => throw new NotImplementedException(); + + public IEnumerable HandledExtensions => new[] { ".jpg", ".jpeg", ".png" }; } } diff --git a/osu.Game/Skinning/SkinManager.cs b/osu.Game/Skinning/SkinManager.cs index bad559d9fe..5333b58625 100644 --- a/osu.Game/Skinning/SkinManager.cs +++ b/osu.Game/Skinning/SkinManager.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.IO; using System.Linq; using System.Linq.Expressions; using System.Threading; @@ -23,6 +24,7 @@ using osu.Game.Audio; using osu.Game.Database; using osu.Game.IO; using osu.Game.IO.Archives; +using osu.Game.Models; using osu.Game.Overlays.Notifications; namespace osu.Game.Skinning @@ -35,7 +37,7 @@ namespace osu.Game.Skinning /// For gameplay components, see which adds extra legacy and toggle logic that may affect the lookup process. /// [ExcludeFromDynamicCompile] - public class SkinManager : ISkinSource, IStorageResourceProvider, IModelImporter + public class SkinManager : ISkinSource, IStorageResourceProvider, IModelImporter, IModelManager, IModelFileManager { private readonly AudioManager audio; @@ -306,5 +308,45 @@ namespace osu.Game.Skinning } #endregion + + public bool Delete(SkinInfo item) + { + return skinModelManager.Delete(item); + } + + public void Delete(List items, bool silent = false) + { + skinModelManager.Delete(items, silent); + } + + public void Undelete(List items, bool silent = false) + { + skinModelManager.Undelete(items, silent); + } + + public void Undelete(SkinInfo item) + { + skinModelManager.Undelete(item); + } + + public bool IsAvailableLocally(SkinInfo model) + { + return skinModelManager.IsAvailableLocally(model); + } + + public void ReplaceFile(SkinInfo model, RealmNamedFileUsage file, Stream contents) + { + skinModelManager.ReplaceFile(model, file, contents); + } + + public void DeleteFile(SkinInfo model, RealmNamedFileUsage file) + { + skinModelManager.DeleteFile(model, file); + } + + public void AddFile(SkinInfo model, Stream contents, string filename) + { + skinModelManager.AddFile(model, contents, filename); + } } } diff --git a/osu.Game/Skinning/SkinnableSprite.cs b/osu.Game/Skinning/SkinnableSprite.cs index 56e576d081..38803bd8e3 100644 --- a/osu.Game/Skinning/SkinnableSprite.cs +++ b/osu.Game/Skinning/SkinnableSprite.cs @@ -11,7 +11,7 @@ namespace osu.Game.Skinning /// /// A skinnable element which uses a stable sprite and can therefore share implementation logic. /// - public class SkinnableSprite : SkinnableDrawable + public class SkinnableSprite : SkinnableDrawable, ISkinnableDrawable { protected override bool ApplySizeRestrictionsToDefault => true; @@ -42,5 +42,7 @@ namespace osu.Game.Skinning public string LookupName { get; } } + + public bool UsesFixedAnchor { get; set; } } } From 66f5eae530cac55975b46655825d2b159e73976d Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 24 Mar 2022 19:32:34 +0900 Subject: [PATCH 03/19] Hook up a dropdown to show all available sprites for the current skin --- .../Configuration/SettingSourceAttribute.cs | 1 + osu.Game/Overlays/Settings/SettingsItem.cs | 6 ++++++ .../Skinning/Components/SkinnableSprite.cs | 19 ++++++++++++++++++- osu.Game/Skinning/Editor/SkinEditor.cs | 2 +- 4 files changed, 26 insertions(+), 2 deletions(-) diff --git a/osu.Game/Configuration/SettingSourceAttribute.cs b/osu.Game/Configuration/SettingSourceAttribute.cs index 4111a67b24..8c84707b88 100644 --- a/osu.Game/Configuration/SettingSourceAttribute.cs +++ b/osu.Game/Configuration/SettingSourceAttribute.cs @@ -88,6 +88,7 @@ namespace osu.Game.Configuration throw new InvalidOperationException($"{nameof(SettingSourceAttribute)} had an unsupported custom control type ({controlType.ReadableName()})"); var control = (Drawable)Activator.CreateInstance(controlType); + controlType.GetProperty(nameof(SettingsItem.Source))?.SetValue(control, obj); controlType.GetProperty(nameof(SettingsItem.LabelText))?.SetValue(control, attr.Label); controlType.GetProperty(nameof(SettingsItem.TooltipText))?.SetValue(control, attr.Description); controlType.GetProperty(nameof(SettingsItem.Current))?.SetValue(control, value); diff --git a/osu.Game/Overlays/Settings/SettingsItem.cs b/osu.Game/Overlays/Settings/SettingsItem.cs index e709be1343..6ac5351270 100644 --- a/osu.Game/Overlays/Settings/SettingsItem.cs +++ b/osu.Game/Overlays/Settings/SettingsItem.cs @@ -11,6 +11,7 @@ using osu.Framework.Graphics.Cursor; using osu.Framework.Graphics.Sprites; using osu.Framework.Graphics.UserInterface; using osu.Framework.Localisation; +using osu.Game.Configuration; using osu.Game.Graphics; using osu.Game.Graphics.Sprites; using osu.Game.Graphics.Containers; @@ -24,6 +25,11 @@ namespace osu.Game.Overlays.Settings protected Drawable Control { get; } + /// + /// The source component if this was created via . + /// + public Drawable Source { get; internal set; } + private IHasCurrentValue controlWithCurrent => Control as IHasCurrentValue; protected override Container Content => FlowContent; diff --git a/osu.Game/Skinning/Components/SkinnableSprite.cs b/osu.Game/Skinning/Components/SkinnableSprite.cs index 292bbe2321..aa23e428d1 100644 --- a/osu.Game/Skinning/Components/SkinnableSprite.cs +++ b/osu.Game/Skinning/Components/SkinnableSprite.cs @@ -1,6 +1,8 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. +using System.Collections.Generic; +using System.Linq; using JetBrains.Annotations; using osu.Framework.Allocation; using osu.Framework.Bindables; @@ -8,6 +10,7 @@ using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Framework.Graphics.Sprites; using osu.Game.Configuration; +using osu.Game.Overlays.Settings; namespace osu.Game.Skinning.Components { @@ -19,12 +22,14 @@ namespace osu.Game.Skinning.Components { public bool UsesFixedAnchor { get; set; } - [SettingSource("Sprite name", "The filename of the sprite")] + [SettingSource("Sprite name", "The filename of the sprite", SettingControlType = typeof(SpriteSelectorControl))] public Bindable SpriteName { get; } = new Bindable(string.Empty); [Resolved] private ISkinSource source { get; set; } + public IEnumerable AvailableFiles => (source.AllSources.First() as Skin)?.SkinInfo.PerformRead(s => s.Files.Select(f => f.Filename)); + public SkinSprite() { AutoSizeAxes = Axes.Both; @@ -45,5 +50,17 @@ namespace osu.Game.Skinning.Components }; }, true); } + + public class SpriteSelectorControl : SettingsDropdown + { + public SkinSprite Source { get; set; } + + protected override void LoadComplete() + { + base.LoadComplete(); + + Items = Source.AvailableFiles; + } + } } } diff --git a/osu.Game/Skinning/Editor/SkinEditor.cs b/osu.Game/Skinning/Editor/SkinEditor.cs index 7701fafbfc..392cb2f32b 100644 --- a/osu.Game/Skinning/Editor/SkinEditor.cs +++ b/osu.Game/Skinning/Editor/SkinEditor.cs @@ -351,7 +351,7 @@ namespace osu.Game.Skinning.Editor // place component placeComponent(new SkinSprite { - SpriteName = { Value = Path.GetFileNameWithoutExtension(file.Name) } + SpriteName = { Value = file.Name } }); }); From 9c3dad9fbf72f63cbb79fdc8f2ebc687a2d66f69 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 24 Mar 2022 19:09:17 +0900 Subject: [PATCH 04/19] Add proof of concept flow to ensure `RealmBackedResourceStore` is invalidated on realm file changes I'm not at all happy with this, but it does work so let's go with it for now. --- osu.Game/Skinning/LegacyBeatmapSkin.cs | 5 +- osu.Game/Skinning/RealmBackedResourceStore.cs | 57 ++++++++++++------- osu.Game/Skinning/Skin.cs | 8 ++- osu.Game/Skinning/SkinManager.cs | 21 ++++++- 4 files changed, 67 insertions(+), 24 deletions(-) diff --git a/osu.Game/Skinning/LegacyBeatmapSkin.cs b/osu.Game/Skinning/LegacyBeatmapSkin.cs index 16a05f4197..70f5b35d00 100644 --- a/osu.Game/Skinning/LegacyBeatmapSkin.cs +++ b/osu.Game/Skinning/LegacyBeatmapSkin.cs @@ -11,6 +11,7 @@ using osu.Framework.IO.Stores; using osu.Game.Audio; using osu.Game.Beatmaps; using osu.Game.Beatmaps.Formats; +using osu.Game.Database; using osu.Game.IO; using osu.Game.Rulesets.Objects.Legacy; using osu.Game.Rulesets.Objects.Types; @@ -37,11 +38,11 @@ namespace osu.Game.Skinning private static IResourceStore createRealmBackedStore(BeatmapInfo beatmapInfo, IStorageResourceProvider? resources) { - if (resources == null) + if (resources == null || beatmapInfo.BeatmapSet == null) // should only ever be used in tests. return new ResourceStore(); - return new RealmBackedResourceStore(beatmapInfo.BeatmapSet, resources.Files, new[] { @"ogg" }); + return new RealmBackedResourceStore(beatmapInfo.BeatmapSet.ToLive(resources.RealmAccess), resources.Files, resources.RealmAccess); } public override Drawable? GetDrawableComponent(ISkinComponent component) diff --git a/osu.Game/Skinning/RealmBackedResourceStore.cs b/osu.Game/Skinning/RealmBackedResourceStore.cs index fc9036727f..115d563575 100644 --- a/osu.Game/Skinning/RealmBackedResourceStore.cs +++ b/osu.Game/Skinning/RealmBackedResourceStore.cs @@ -1,51 +1,68 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. +#nullable enable + +using System; using System.Collections.Generic; +using System.Diagnostics; +using System.Threading; using osu.Framework.Extensions; using osu.Framework.IO.Stores; using osu.Game.Database; using osu.Game.Extensions; +using Realms; namespace osu.Game.Skinning { - public class RealmBackedResourceStore : ResourceStore + public class RealmBackedResourceStore : ResourceStore + where T : RealmObject, IHasRealmFiles, IHasGuidPrimaryKey { - private readonly Dictionary fileToStoragePathMapping = new Dictionary(); + private Lazy> fileToStoragePathMapping; - public RealmBackedResourceStore(IHasRealmFiles source, IResourceStore underlyingStore, string[] extensions = null) + private readonly Live liveSource; + + public RealmBackedResourceStore(Live source, IResourceStore underlyingStore, RealmAccess? realm) : base(underlyingStore) { - // Must be initialised before the file cache. - if (extensions != null) - { - foreach (string extension in extensions) - AddExtension(extension); - } + liveSource = source; - initialiseFileCache(source); + invalidateCache(); + Debug.Assert(fileToStoragePathMapping != null); } - private void initialiseFileCache(IHasRealmFiles source) - { - fileToStoragePathMapping.Clear(); - foreach (var f in source.Files) - fileToStoragePathMapping[f.Filename.ToLowerInvariant()] = f.File.GetStoragePath(); - } + public void Invalidate() => invalidateCache(); protected override IEnumerable GetFilenames(string name) { foreach (string filename in base.GetFilenames(name)) { - string path = getPathForFile(filename.ToStandardisedPath()); + string? path = getPathForFile(filename.ToStandardisedPath()); if (path != null) yield return path; } } - private string getPathForFile(string filename) => - fileToStoragePathMapping.TryGetValue(filename.ToLower(), out string path) ? path : null; + private string? getPathForFile(string filename) + { + if (fileToStoragePathMapping.Value.TryGetValue(filename.ToLowerInvariant(), out string path)) + return path; - public override IEnumerable GetAvailableResources() => fileToStoragePathMapping.Keys; + return null; + } + + private void invalidateCache() => fileToStoragePathMapping = new Lazy>(initialiseFileCache, LazyThreadSafetyMode.ExecutionAndPublication); + + private Dictionary initialiseFileCache() => liveSource.PerformRead(source => + { + var dictionary = new Dictionary(); + dictionary.Clear(); + foreach (var f in source.Files) + dictionary[f.Filename.ToLowerInvariant()] = f.File.GetStoragePath(); + + return dictionary; + }); + + public override IEnumerable GetAvailableResources() => fileToStoragePathMapping.Value.Keys; } } diff --git a/osu.Game/Skinning/Skin.cs b/osu.Game/Skinning/Skin.cs index 2f01bb7301..fb9914cd9e 100644 --- a/osu.Game/Skinning/Skin.cs +++ b/osu.Game/Skinning/Skin.cs @@ -54,6 +54,10 @@ namespace osu.Game.Skinning where TLookup : notnull where TValue : notnull; + public void InvalidateCaches() => realmBackedStorage?.Invalidate(); + + private readonly RealmBackedResourceStore realmBackedStorage; + /// /// Construct a new skin. /// @@ -67,7 +71,9 @@ namespace osu.Game.Skinning { SkinInfo = skin.ToLive(resources.RealmAccess); - storage ??= new RealmBackedResourceStore(skin, resources.Files, new[] { @"ogg" }); + storage ??= realmBackedStorage = new RealmBackedResourceStore(SkinInfo, resources.Files, resources.RealmAccess); + + (storage as ResourceStore)?.AddExtension("ogg"); var samples = resources.AudioManager?.GetSampleStore(storage); if (samples != null) diff --git a/osu.Game/Skinning/SkinManager.cs b/osu.Game/Skinning/SkinManager.cs index 5333b58625..bafb088f68 100644 --- a/osu.Game/Skinning/SkinManager.cs +++ b/osu.Game/Skinning/SkinManager.cs @@ -16,6 +16,7 @@ using osu.Framework.Graphics; using osu.Framework.Graphics.OpenGL.Textures; using osu.Framework.Graphics.Textures; using osu.Framework.IO.Stores; +using osu.Framework.Logging; using osu.Framework.Platform; using osu.Framework.Testing; using osu.Framework.Threading; @@ -26,6 +27,7 @@ using osu.Game.IO; using osu.Game.IO.Archives; using osu.Game.Models; using osu.Game.Overlays.Notifications; +using Realms; namespace osu.Game.Skinning { @@ -59,6 +61,8 @@ namespace osu.Game.Skinning private readonly IResourceStore userFiles; + private IDisposable currentSkinSubscription; + /// /// The default skin. /// @@ -97,7 +101,16 @@ namespace osu.Game.Skinning } }); - CurrentSkinInfo.ValueChanged += skin => CurrentSkin.Value = skin.NewValue.PerformRead(GetSkin); + CurrentSkinInfo.ValueChanged += skin => + { + CurrentSkin.Value = skin.NewValue.PerformRead(GetSkin); + + scheduler.Add(() => + { + currentSkinSubscription?.Dispose(); + currentSkinSubscription = realm.RegisterForNotifications(r => r.All().Where(s => s.ID == skin.NewValue.ID), realmSkinChanged); + }); + }; CurrentSkin.Value = DefaultSkin; CurrentSkin.ValueChanged += skin => @@ -109,6 +122,12 @@ namespace osu.Game.Skinning }; } + private void realmSkinChanged(IRealmCollection sender, ChangeSet changes, Exception error) where T : RealmObjectBase + { + Logger.Log("Detected a skin change"); + CurrentSkin.Value.InvalidateCaches(); + } + public void SelectRandomSkin() { realm.Run(r => From 762de3cc9780e76e744ea4e3c5d532e16117e4fd Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 24 Mar 2022 22:53:49 +0900 Subject: [PATCH 05/19] Replace invalidation logic with local realm notification subscription --- osu.Game/Skinning/RealmBackedResourceStore.cs | 12 +++++++++++- osu.Game/Skinning/Skin.cs | 4 ++-- osu.Game/Skinning/SkinManager.cs | 16 ---------------- 3 files changed, 13 insertions(+), 19 deletions(-) diff --git a/osu.Game/Skinning/RealmBackedResourceStore.cs b/osu.Game/Skinning/RealmBackedResourceStore.cs index 115d563575..e727a7e59a 100644 --- a/osu.Game/Skinning/RealmBackedResourceStore.cs +++ b/osu.Game/Skinning/RealmBackedResourceStore.cs @@ -6,6 +6,7 @@ using System; using System.Collections.Generic; using System.Diagnostics; +using System.Linq; using System.Threading; using osu.Framework.Extensions; using osu.Framework.IO.Stores; @@ -21,6 +22,7 @@ namespace osu.Game.Skinning private Lazy> fileToStoragePathMapping; private readonly Live liveSource; + private readonly IDisposable? realmSubscription; public RealmBackedResourceStore(Live source, IResourceStore underlyingStore, RealmAccess? realm) : base(underlyingStore) @@ -29,9 +31,17 @@ namespace osu.Game.Skinning invalidateCache(); Debug.Assert(fileToStoragePathMapping != null); + + realmSubscription = realm?.RegisterForNotifications(r => r.All().Where(s => s.ID == source.ID), skinChanged); } - public void Invalidate() => invalidateCache(); + protected override void Dispose(bool disposing) + { + base.Dispose(disposing); + realmSubscription?.Dispose(); + } + + private void skinChanged(IRealmCollection sender, ChangeSet changes, Exception error) => invalidateCache(); protected override IEnumerable GetFilenames(string name) { diff --git a/osu.Game/Skinning/Skin.cs b/osu.Game/Skinning/Skin.cs index fb9914cd9e..4cd1d952db 100644 --- a/osu.Game/Skinning/Skin.cs +++ b/osu.Game/Skinning/Skin.cs @@ -54,8 +54,6 @@ namespace osu.Game.Skinning where TLookup : notnull where TValue : notnull; - public void InvalidateCaches() => realmBackedStorage?.Invalidate(); - private readonly RealmBackedResourceStore realmBackedStorage; /// @@ -206,6 +204,8 @@ namespace osu.Game.Skinning Textures?.Dispose(); Samples?.Dispose(); + + realmBackedStorage?.Dispose(); } #endregion diff --git a/osu.Game/Skinning/SkinManager.cs b/osu.Game/Skinning/SkinManager.cs index bafb088f68..5e85f9e4ca 100644 --- a/osu.Game/Skinning/SkinManager.cs +++ b/osu.Game/Skinning/SkinManager.cs @@ -16,7 +16,6 @@ using osu.Framework.Graphics; using osu.Framework.Graphics.OpenGL.Textures; using osu.Framework.Graphics.Textures; using osu.Framework.IO.Stores; -using osu.Framework.Logging; using osu.Framework.Platform; using osu.Framework.Testing; using osu.Framework.Threading; @@ -27,7 +26,6 @@ using osu.Game.IO; using osu.Game.IO.Archives; using osu.Game.Models; using osu.Game.Overlays.Notifications; -using Realms; namespace osu.Game.Skinning { @@ -61,8 +59,6 @@ namespace osu.Game.Skinning private readonly IResourceStore userFiles; - private IDisposable currentSkinSubscription; - /// /// The default skin. /// @@ -104,12 +100,6 @@ namespace osu.Game.Skinning CurrentSkinInfo.ValueChanged += skin => { CurrentSkin.Value = skin.NewValue.PerformRead(GetSkin); - - scheduler.Add(() => - { - currentSkinSubscription?.Dispose(); - currentSkinSubscription = realm.RegisterForNotifications(r => r.All().Where(s => s.ID == skin.NewValue.ID), realmSkinChanged); - }); }; CurrentSkin.Value = DefaultSkin; @@ -122,12 +112,6 @@ namespace osu.Game.Skinning }; } - private void realmSkinChanged(IRealmCollection sender, ChangeSet changes, Exception error) where T : RealmObjectBase - { - Logger.Log("Detected a skin change"); - CurrentSkin.Value.InvalidateCaches(); - } - public void SelectRandomSkin() { realm.Run(r => From d1be229d74806794f8f136f3919885f915c118fc Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 25 Mar 2022 17:31:03 +0900 Subject: [PATCH 06/19] Combine `SkinSprite` into `SkinnableSprite` --- .../Skinning/Components/SkinnableSprite.cs | 66 ------------------- osu.Game/Skinning/DefaultSkin.cs | 4 ++ osu.Game/Skinning/Editor/SkinEditor.cs | 5 +- osu.Game/Skinning/Skin.cs | 2 +- osu.Game/Skinning/SkinnableDrawable.cs | 8 +-- osu.Game/Skinning/SkinnableSprite.cs | 47 ++++++++++++- 6 files changed, 55 insertions(+), 77 deletions(-) delete mode 100644 osu.Game/Skinning/Components/SkinnableSprite.cs diff --git a/osu.Game/Skinning/Components/SkinnableSprite.cs b/osu.Game/Skinning/Components/SkinnableSprite.cs deleted file mode 100644 index aa23e428d1..0000000000 --- a/osu.Game/Skinning/Components/SkinnableSprite.cs +++ /dev/null @@ -1,66 +0,0 @@ -// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. -// See the LICENCE file in the repository root for full licence text. - -using System.Collections.Generic; -using System.Linq; -using JetBrains.Annotations; -using osu.Framework.Allocation; -using osu.Framework.Bindables; -using osu.Framework.Graphics; -using osu.Framework.Graphics.Containers; -using osu.Framework.Graphics.Sprites; -using osu.Game.Configuration; -using osu.Game.Overlays.Settings; - -namespace osu.Game.Skinning.Components -{ - /// - /// Intended to be a test bed for skinning. May be removed at some point in the future. - /// - [UsedImplicitly] - public class SkinSprite : CompositeDrawable, ISkinnableDrawable - { - public bool UsesFixedAnchor { get; set; } - - [SettingSource("Sprite name", "The filename of the sprite", SettingControlType = typeof(SpriteSelectorControl))] - public Bindable SpriteName { get; } = new Bindable(string.Empty); - - [Resolved] - private ISkinSource source { get; set; } - - public IEnumerable AvailableFiles => (source.AllSources.First() as Skin)?.SkinInfo.PerformRead(s => s.Files.Select(f => f.Filename)); - - public SkinSprite() - { - AutoSizeAxes = Axes.Both; - } - - protected override void LoadComplete() - { - base.LoadComplete(); - - SpriteName.BindValueChanged(spriteName => - { - InternalChildren = new Drawable[] - { - new Sprite - { - Texture = source.GetTexture(SpriteName.Value), - } - }; - }, true); - } - - public class SpriteSelectorControl : SettingsDropdown - { - public SkinSprite Source { get; set; } - - protected override void LoadComplete() - { - base.LoadComplete(); - - Items = Source.AvailableFiles; - } - } - } -} diff --git a/osu.Game/Skinning/DefaultSkin.cs b/osu.Game/Skinning/DefaultSkin.cs index 43ada59bcb..c645b0fae4 100644 --- a/osu.Game/Skinning/DefaultSkin.cs +++ b/osu.Game/Skinning/DefaultSkin.cs @@ -8,6 +8,7 @@ using osu.Framework.Audio.Sample; using osu.Framework.Bindables; using osu.Framework.Graphics; using osu.Framework.Graphics.OpenGL.Textures; +using osu.Framework.Graphics.Sprites; using osu.Framework.Graphics.Textures; using osu.Game.Audio; using osu.Game.Beatmaps.Formats; @@ -157,6 +158,9 @@ namespace osu.Game.Skinning break; } + if (GetTexture(component.LookupName) is Texture t) + return new Sprite { Texture = t }; + return null; } diff --git a/osu.Game/Skinning/Editor/SkinEditor.cs b/osu.Game/Skinning/Editor/SkinEditor.cs index 392cb2f32b..484faebdc0 100644 --- a/osu.Game/Skinning/Editor/SkinEditor.cs +++ b/osu.Game/Skinning/Editor/SkinEditor.cs @@ -21,7 +21,6 @@ using osu.Game.Graphics.UserInterface; using osu.Game.Overlays; using osu.Game.Screens.Edit.Components; using osu.Game.Screens.Edit.Components.Menus; -using osu.Game.Skinning.Components; namespace osu.Game.Skinning.Editor { @@ -349,9 +348,9 @@ namespace osu.Game.Skinning.Editor }); // place component - placeComponent(new SkinSprite + placeComponent(new SkinnableSprite { - SpriteName = { Value = file.Name } + SpriteName = { Value = Path.GetFileNameWithoutExtension(file.Name) } }); }); diff --git a/osu.Game/Skinning/Skin.cs b/osu.Game/Skinning/Skin.cs index 4cd1d952db..f2d095f880 100644 --- a/osu.Game/Skinning/Skin.cs +++ b/osu.Game/Skinning/Skin.cs @@ -54,7 +54,7 @@ namespace osu.Game.Skinning where TLookup : notnull where TValue : notnull; - private readonly RealmBackedResourceStore realmBackedStorage; + private readonly RealmBackedResourceStore? realmBackedStorage; /// /// Construct a new skin. diff --git a/osu.Game/Skinning/SkinnableDrawable.cs b/osu.Game/Skinning/SkinnableDrawable.cs index 72f64e2e12..45409694b5 100644 --- a/osu.Game/Skinning/SkinnableDrawable.cs +++ b/osu.Game/Skinning/SkinnableDrawable.cs @@ -31,7 +31,7 @@ namespace osu.Game.Skinning set => base.AutoSizeAxes = value; } - private readonly ISkinComponent component; + protected readonly ISkinComponent Component; private readonly ConfineMode confineMode; @@ -49,7 +49,7 @@ namespace osu.Game.Skinning protected SkinnableDrawable(ISkinComponent component, ConfineMode confineMode = ConfineMode.NoScaling) { - this.component = component; + Component = component; this.confineMode = confineMode; RelativeSizeAxes = Axes.Both; @@ -75,13 +75,13 @@ namespace osu.Game.Skinning protected override void SkinChanged(ISkinSource skin) { - Drawable = skin.GetDrawableComponent(component); + Drawable = skin.GetDrawableComponent(Component); isDefault = false; if (Drawable == null) { - Drawable = CreateDefault(component); + Drawable = CreateDefault(Component); isDefault = true; } diff --git a/osu.Game/Skinning/SkinnableSprite.cs b/osu.Game/Skinning/SkinnableSprite.cs index 38803bd8e3..aa3001fe45 100644 --- a/osu.Game/Skinning/SkinnableSprite.cs +++ b/osu.Game/Skinning/SkinnableSprite.cs @@ -1,10 +1,16 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. +using System.Collections.Generic; +using System.IO; +using System.Linq; using osu.Framework.Allocation; +using osu.Framework.Bindables; using osu.Framework.Graphics; using osu.Framework.Graphics.Sprites; using osu.Framework.Graphics.Textures; +using osu.Game.Configuration; +using osu.Game.Overlays.Settings; namespace osu.Game.Skinning { @@ -18,9 +24,32 @@ namespace osu.Game.Skinning [Resolved] private TextureStore textures { get; set; } + [SettingSource("Sprite name", "The filename of the sprite", SettingControlType = typeof(SpriteSelectorControl))] + public Bindable SpriteName { get; } = new Bindable(string.Empty); + + [Resolved] + private ISkinSource source { get; set; } + + public IEnumerable AvailableFiles => (source.AllSources.First() as Skin)?.SkinInfo.PerformRead(s => s.Files.Select(f => Path.GetFileNameWithoutExtension(f.Filename)).Distinct()); + public SkinnableSprite(string textureName, ConfineMode confineMode = ConfineMode.NoScaling) : base(new SpriteComponent(textureName), confineMode) { + SpriteName.Value = textureName; + } + + public SkinnableSprite() + : base(new SpriteComponent(string.Empty), ConfineMode.NoScaling) + { + RelativeSizeAxes = Axes.None; + AutoSizeAxes = Axes.Both; + + SpriteName.BindValueChanged(name => + { + ((SpriteComponent)Component).LookupName = name.NewValue ?? string.Empty; + if (IsLoaded) + SkinChanged(CurrentSkin); + }); } protected override Drawable CreateDefault(ISkinComponent component) @@ -33,16 +62,28 @@ namespace osu.Game.Skinning return new Sprite { Texture = texture }; } + public bool UsesFixedAnchor { get; set; } + private class SpriteComponent : ISkinComponent { + public string LookupName { get; set; } + public SpriteComponent(string textureName) { LookupName = textureName; } - - public string LookupName { get; } } - public bool UsesFixedAnchor { get; set; } + public class SpriteSelectorControl : SettingsDropdown + { + public SkinnableSprite Source { get; set; } + + protected override void LoadComplete() + { + base.LoadComplete(); + + Items = Source.AvailableFiles; + } + } } } From 52eeaffce333df069e325c9b4bc69b0ed00bc4d3 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 25 Mar 2022 17:36:02 +0900 Subject: [PATCH 07/19] Limit lookup resources to images --- osu.Game/Skinning/SkinnableSprite.cs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/osu.Game/Skinning/SkinnableSprite.cs b/osu.Game/Skinning/SkinnableSprite.cs index aa3001fe45..f36ae89e25 100644 --- a/osu.Game/Skinning/SkinnableSprite.cs +++ b/osu.Game/Skinning/SkinnableSprite.cs @@ -1,8 +1,8 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. +using System; using System.Collections.Generic; -using System.IO; using System.Linq; using osu.Framework.Allocation; using osu.Framework.Bindables; @@ -30,7 +30,12 @@ namespace osu.Game.Skinning [Resolved] private ISkinSource source { get; set; } - public IEnumerable AvailableFiles => (source.AllSources.First() as Skin)?.SkinInfo.PerformRead(s => s.Files.Select(f => Path.GetFileNameWithoutExtension(f.Filename)).Distinct()); + public IEnumerable AvailableFiles => (source.AllSources.First() as Skin)?.SkinInfo.PerformRead(s => s.Files + .Where(f => + f.Filename.EndsWith(".png", StringComparison.Ordinal) + || f.Filename.EndsWith(".jpg", StringComparison.Ordinal) + ) + .Select(f => f.Filename).Distinct()); public SkinnableSprite(string textureName, ConfineMode confineMode = ConfineMode.NoScaling) : base(new SpriteComponent(textureName), confineMode) From 2b7105ac4fc0ab4663a5e7d3fd72c836028a4714 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 1 Apr 2022 14:46:14 +0900 Subject: [PATCH 08/19] Add a default sprite representation to allow better placeholder display in skin editor toolbox --- osu.Game/Skinning/SkinnableSprite.cs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/osu.Game/Skinning/SkinnableSprite.cs b/osu.Game/Skinning/SkinnableSprite.cs index f36ae89e25..0005045c00 100644 --- a/osu.Game/Skinning/SkinnableSprite.cs +++ b/osu.Game/Skinning/SkinnableSprite.cs @@ -11,6 +11,7 @@ using osu.Framework.Graphics.Sprites; using osu.Framework.Graphics.Textures; using osu.Game.Configuration; using osu.Game.Overlays.Settings; +using osuTK; namespace osu.Game.Skinning { @@ -62,7 +63,13 @@ namespace osu.Game.Skinning var texture = textures.Get(component.LookupName); if (texture == null) - return null; + { + return new SpriteIcon + { + Size = new Vector2(100), + Icon = FontAwesome.Solid.QuestionCircle + }; + } return new Sprite { Texture = texture }; } @@ -87,7 +94,8 @@ namespace osu.Game.Skinning { base.LoadComplete(); - Items = Source.AvailableFiles; + if (Source.AvailableFiles.Any()) + Items = Source.AvailableFiles; } } } From 314ad63c6eee361142d0129bab387301bf442231 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 1 Apr 2022 15:14:53 +0900 Subject: [PATCH 09/19] Simplify available file lookup and include file extension --- osu.Game/Skinning/Editor/SkinEditor.cs | 2 +- osu.Game/Skinning/SkinnableSprite.cs | 24 ++++++++++++------------ 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/osu.Game/Skinning/Editor/SkinEditor.cs b/osu.Game/Skinning/Editor/SkinEditor.cs index 484faebdc0..d36806a1b3 100644 --- a/osu.Game/Skinning/Editor/SkinEditor.cs +++ b/osu.Game/Skinning/Editor/SkinEditor.cs @@ -350,7 +350,7 @@ namespace osu.Game.Skinning.Editor // place component placeComponent(new SkinnableSprite { - SpriteName = { Value = Path.GetFileNameWithoutExtension(file.Name) } + SpriteName = { Value = file.Name } }); }); diff --git a/osu.Game/Skinning/SkinnableSprite.cs b/osu.Game/Skinning/SkinnableSprite.cs index 0005045c00..87490b4397 100644 --- a/osu.Game/Skinning/SkinnableSprite.cs +++ b/osu.Game/Skinning/SkinnableSprite.cs @@ -2,7 +2,6 @@ // See the LICENCE file in the repository root for full licence text. using System; -using System.Collections.Generic; using System.Linq; using osu.Framework.Allocation; using osu.Framework.Bindables; @@ -31,13 +30,6 @@ namespace osu.Game.Skinning [Resolved] private ISkinSource source { get; set; } - public IEnumerable AvailableFiles => (source.AllSources.First() as Skin)?.SkinInfo.PerformRead(s => s.Files - .Where(f => - f.Filename.EndsWith(".png", StringComparison.Ordinal) - || f.Filename.EndsWith(".jpg", StringComparison.Ordinal) - ) - .Select(f => f.Filename).Distinct()); - public SkinnableSprite(string textureName, ConfineMode confineMode = ConfineMode.NoScaling) : base(new SpriteComponent(textureName), confineMode) { @@ -88,14 +80,22 @@ namespace osu.Game.Skinning public class SpriteSelectorControl : SettingsDropdown { - public SkinnableSprite Source { get; set; } - protected override void LoadComplete() { base.LoadComplete(); - if (Source.AvailableFiles.Any()) - Items = Source.AvailableFiles; + // Round-about way of getting the user's skin to find available resources. + // In the future we'll probably want to allow access to resources from the fallbacks, or potentially other skins + // but that requires further thought. + var highestPrioritySkin = ((SkinnableSprite)Source).source.AllSources.First() as Skin; + + string[] availableFiles = highestPrioritySkin?.SkinInfo.PerformRead(s => s.Files + .Where(f => f.Filename.EndsWith(".png", StringComparison.Ordinal) + || f.Filename.EndsWith(".jpg", StringComparison.Ordinal)) + .Select(f => f.Filename).Distinct()).ToArray(); + + if (availableFiles?.Length > 0) + Items = availableFiles; } } } From bfd3406f5f69149a251112ce7f3d94ee81fecce0 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 1 Apr 2022 15:49:05 +0900 Subject: [PATCH 10/19] Ensure that file is imported and caches are invalidated before placing new sprites --- osu.Game/Skinning/Editor/SkinEditor.cs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/osu.Game/Skinning/Editor/SkinEditor.cs b/osu.Game/Skinning/Editor/SkinEditor.cs index d36806a1b3..df0bb7a70c 100644 --- a/osu.Game/Skinning/Editor/SkinEditor.cs +++ b/osu.Game/Skinning/Editor/SkinEditor.cs @@ -48,6 +48,9 @@ namespace osu.Game.Skinning.Editor [Resolved] private OsuColour colours { get; set; } + [Resolved] + private RealmAccess realm { get; set; } + [Resolved(canBeNull: true)] private SkinEditorOverlay skinEditorOverlay { get; set; } @@ -347,6 +350,11 @@ namespace osu.Game.Skinning.Editor skins.AddFile(skinInfo, contents, file.Name); }); + // Even though we are 100% on an update thread, we need to wait for realm callbacks to fire (to correctly invalidate caches in RealmBackedResourceStore). + // See https://github.com/realm/realm-dotnet/discussions/2634#discussioncomment-2483573 for further discussion. + // This is the best we can do for now. + realm.Run(r => r.Refresh()); + // place component placeComponent(new SkinnableSprite { From 6afed5e865ff278970db643719f1377c8cb08660 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 1 Apr 2022 16:00:51 +0900 Subject: [PATCH 11/19] Fix new `SettingsItem` attribute not playing well with non-`Drawable`s --- osu.Game/Configuration/SettingSourceAttribute.cs | 2 +- osu.Game/Overlays/Settings/SettingsItem.cs | 2 +- osu.Game/Skinning/SkinnableSprite.cs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/osu.Game/Configuration/SettingSourceAttribute.cs b/osu.Game/Configuration/SettingSourceAttribute.cs index 8c84707b88..89f0e73f4f 100644 --- a/osu.Game/Configuration/SettingSourceAttribute.cs +++ b/osu.Game/Configuration/SettingSourceAttribute.cs @@ -88,7 +88,7 @@ namespace osu.Game.Configuration throw new InvalidOperationException($"{nameof(SettingSourceAttribute)} had an unsupported custom control type ({controlType.ReadableName()})"); var control = (Drawable)Activator.CreateInstance(controlType); - controlType.GetProperty(nameof(SettingsItem.Source))?.SetValue(control, obj); + controlType.GetProperty(nameof(SettingsItem.SettingSourceObject))?.SetValue(control, obj); controlType.GetProperty(nameof(SettingsItem.LabelText))?.SetValue(control, attr.Label); controlType.GetProperty(nameof(SettingsItem.TooltipText))?.SetValue(control, attr.Description); controlType.GetProperty(nameof(SettingsItem.Current))?.SetValue(control, value); diff --git a/osu.Game/Overlays/Settings/SettingsItem.cs b/osu.Game/Overlays/Settings/SettingsItem.cs index 6ac5351270..098090bf78 100644 --- a/osu.Game/Overlays/Settings/SettingsItem.cs +++ b/osu.Game/Overlays/Settings/SettingsItem.cs @@ -28,7 +28,7 @@ namespace osu.Game.Overlays.Settings /// /// The source component if this was created via . /// - public Drawable Source { get; internal set; } + public object SettingSourceObject { get; internal set; } private IHasCurrentValue controlWithCurrent => Control as IHasCurrentValue; diff --git a/osu.Game/Skinning/SkinnableSprite.cs b/osu.Game/Skinning/SkinnableSprite.cs index 87490b4397..c6cc4c1bdd 100644 --- a/osu.Game/Skinning/SkinnableSprite.cs +++ b/osu.Game/Skinning/SkinnableSprite.cs @@ -87,7 +87,7 @@ namespace osu.Game.Skinning // Round-about way of getting the user's skin to find available resources. // In the future we'll probably want to allow access to resources from the fallbacks, or potentially other skins // but that requires further thought. - var highestPrioritySkin = ((SkinnableSprite)Source).source.AllSources.First() as Skin; + var highestPrioritySkin = ((SkinnableSprite)SettingSourceObject).source.AllSources.First() as Skin; string[] availableFiles = highestPrioritySkin?.SkinInfo.PerformRead(s => s.Files .Where(f => f.Filename.EndsWith(".png", StringComparison.Ordinal) From 09e15f5496a8ecee0f0c77f3743a3b664a150da9 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 4 Apr 2022 20:27:03 +0900 Subject: [PATCH 12/19] Remove nullable on `RealmBackedResourceStore` realm parameter --- osu.Game/Skinning/RealmBackedResourceStore.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game/Skinning/RealmBackedResourceStore.cs b/osu.Game/Skinning/RealmBackedResourceStore.cs index e727a7e59a..c81e976a67 100644 --- a/osu.Game/Skinning/RealmBackedResourceStore.cs +++ b/osu.Game/Skinning/RealmBackedResourceStore.cs @@ -24,7 +24,7 @@ namespace osu.Game.Skinning private readonly Live liveSource; private readonly IDisposable? realmSubscription; - public RealmBackedResourceStore(Live source, IResourceStore underlyingStore, RealmAccess? realm) + public RealmBackedResourceStore(Live source, IResourceStore underlyingStore, RealmAccess realm) : base(underlyingStore) { liveSource = source; @@ -32,7 +32,7 @@ namespace osu.Game.Skinning invalidateCache(); Debug.Assert(fileToStoragePathMapping != null); - realmSubscription = realm?.RegisterForNotifications(r => r.All().Where(s => s.ID == source.ID), skinChanged); + realmSubscription = realm.RegisterForNotifications(r => r.All().Where(s => s.ID == source.ID), skinChanged); } protected override void Dispose(bool disposing) From 300feadf6a9adba8120db8dc3e50fcef5c33c99d Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 4 Apr 2022 20:27:46 +0900 Subject: [PATCH 13/19] Update `SkinnableSprite` to match more broad usage --- osu.Game/Skinning/SkinnableSprite.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Skinning/SkinnableSprite.cs b/osu.Game/Skinning/SkinnableSprite.cs index c6cc4c1bdd..e7c62302b1 100644 --- a/osu.Game/Skinning/SkinnableSprite.cs +++ b/osu.Game/Skinning/SkinnableSprite.cs @@ -15,7 +15,7 @@ using osuTK; namespace osu.Game.Skinning { /// - /// A skinnable element which uses a stable sprite and can therefore share implementation logic. + /// A skinnable element which uses a single texture backing. /// public class SkinnableSprite : SkinnableDrawable, ISkinnableDrawable { From dac5dfde8f8cee98be41887a5b4c94444efe3953 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 4 Apr 2022 20:28:43 +0900 Subject: [PATCH 14/19] Remove unnecessary `LazyThreadSafetyMode` specification --- osu.Game/Skinning/RealmBackedResourceStore.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Skinning/RealmBackedResourceStore.cs b/osu.Game/Skinning/RealmBackedResourceStore.cs index c81e976a67..0353b8a64d 100644 --- a/osu.Game/Skinning/RealmBackedResourceStore.cs +++ b/osu.Game/Skinning/RealmBackedResourceStore.cs @@ -61,7 +61,7 @@ namespace osu.Game.Skinning return null; } - private void invalidateCache() => fileToStoragePathMapping = new Lazy>(initialiseFileCache, LazyThreadSafetyMode.ExecutionAndPublication); + private void invalidateCache() => fileToStoragePathMapping = new Lazy>(initialiseFileCache); private Dictionary initialiseFileCache() => liveSource.PerformRead(source => { From de30a42558b7ab5d0e03cae2ed80d574cb694732 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 4 Apr 2022 20:30:14 +0900 Subject: [PATCH 15/19] Add `region` for import methods and move `Dispose` to end of time --- osu.Game/Skinning/Editor/SkinEditor.cs | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/osu.Game/Skinning/Editor/SkinEditor.cs b/osu.Game/Skinning/Editor/SkinEditor.cs index df0bb7a70c..607a881d28 100644 --- a/osu.Game/Skinning/Editor/SkinEditor.cs +++ b/osu.Game/Skinning/Editor/SkinEditor.cs @@ -197,13 +197,6 @@ namespace osu.Game.Skinning.Editor SelectedComponents.BindCollectionChanged((_, __) => Scheduler.AddOnce(populateSettings), true); } - protected override void Dispose(bool isDisposing) - { - base.Dispose(isDisposing); - - game?.UnregisterImportHandler(this); - } - public void UpdateTargetScreen(Drawable targetScreen) { this.targetScreen = targetScreen; @@ -337,6 +330,8 @@ namespace osu.Game.Skinning.Editor availableTargets.FirstOrDefault(t => t.Components.Contains(item))?.Remove(item); } + #region Drag & drop import handling + public Task Import(params string[] paths) { Schedule(() => @@ -368,5 +363,14 @@ namespace osu.Game.Skinning.Editor public Task Import(params ImportTask[] tasks) => throw new NotImplementedException(); public IEnumerable HandledExtensions => new[] { ".jpg", ".jpeg", ".png" }; + + #endregion + + protected override void Dispose(bool isDisposing) + { + base.Dispose(isDisposing); + + game?.UnregisterImportHandler(this); + } } } From 8185020f128dcda5d5a1feb236b02e7a3fbd51d7 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 4 Apr 2022 20:35:48 +0900 Subject: [PATCH 16/19] Improve the visual of the missing sprite sprite --- osu.Game/Skinning/RealmBackedResourceStore.cs | 1 - osu.Game/Skinning/SkinnableSprite.cs | 33 +++++++++++++++---- 2 files changed, 26 insertions(+), 8 deletions(-) diff --git a/osu.Game/Skinning/RealmBackedResourceStore.cs b/osu.Game/Skinning/RealmBackedResourceStore.cs index 0353b8a64d..0057132044 100644 --- a/osu.Game/Skinning/RealmBackedResourceStore.cs +++ b/osu.Game/Skinning/RealmBackedResourceStore.cs @@ -7,7 +7,6 @@ using System; using System.Collections.Generic; using System.Diagnostics; using System.Linq; -using System.Threading; using osu.Framework.Extensions; using osu.Framework.IO.Stores; using osu.Game.Database; diff --git a/osu.Game/Skinning/SkinnableSprite.cs b/osu.Game/Skinning/SkinnableSprite.cs index e7c62302b1..c5f110f908 100644 --- a/osu.Game/Skinning/SkinnableSprite.cs +++ b/osu.Game/Skinning/SkinnableSprite.cs @@ -6,9 +6,11 @@ using System.Linq; using osu.Framework.Allocation; using osu.Framework.Bindables; using osu.Framework.Graphics; +using osu.Framework.Graphics.Containers; using osu.Framework.Graphics.Sprites; using osu.Framework.Graphics.Textures; using osu.Game.Configuration; +using osu.Game.Graphics.Sprites; using osu.Game.Overlays.Settings; using osuTK; @@ -55,13 +57,7 @@ namespace osu.Game.Skinning var texture = textures.Get(component.LookupName); if (texture == null) - { - return new SpriteIcon - { - Size = new Vector2(100), - Icon = FontAwesome.Solid.QuestionCircle - }; - } + return new SpriteNotFound(component.LookupName); return new Sprite { Texture = texture }; } @@ -98,5 +94,28 @@ namespace osu.Game.Skinning Items = availableFiles; } } + + public class SpriteNotFound : CompositeDrawable + { + public SpriteNotFound(string lookup) + { + AutoSizeAxes = Axes.Both; + + InternalChildren = new Drawable[] + { + new SpriteIcon + { + Size = new Vector2(50), + Icon = FontAwesome.Solid.QuestionCircle + }, + new OsuSpriteText + { + Position = new Vector2(25, 50), + Text = $"missing: {lookup}", + Origin = Anchor.TopCentre, + } + }; + } + } } } From 5f358a04e98ba580ce6fa7b7c32ee23a35d8cba3 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 4 Apr 2022 20:40:19 +0900 Subject: [PATCH 17/19] Return a valid "lighting" response from `DefaultSkin` This is temporary to allow the new sprite lookup flow to potentially be merged before hit lighting skinnability is addressed. --- osu.Game/Skinning/DefaultSkin.cs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/osu.Game/Skinning/DefaultSkin.cs b/osu.Game/Skinning/DefaultSkin.cs index c645b0fae4..119b0ec9ad 100644 --- a/osu.Game/Skinning/DefaultSkin.cs +++ b/osu.Game/Skinning/DefaultSkin.cs @@ -158,6 +158,13 @@ namespace osu.Game.Skinning break; } + switch (component.LookupName) + { + // Temporary until default skin has a valid hit lighting. + case @"lighting": + return Drawable.Empty(); + } + if (GetTexture(component.LookupName) is Texture t) return new Sprite { Texture = t }; From f73062a0d6c141290c93f227db7dd63173175324 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Mon, 4 Apr 2022 22:22:55 +0200 Subject: [PATCH 18/19] Revert "Remove nullable on `RealmBackedResourceStore` realm parameter" This reverts commit 09e15f5496a8ecee0f0c77f3743a3b664a150da9. --- osu.Game/Skinning/RealmBackedResourceStore.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game/Skinning/RealmBackedResourceStore.cs b/osu.Game/Skinning/RealmBackedResourceStore.cs index 0057132044..7fa24284ee 100644 --- a/osu.Game/Skinning/RealmBackedResourceStore.cs +++ b/osu.Game/Skinning/RealmBackedResourceStore.cs @@ -23,7 +23,7 @@ namespace osu.Game.Skinning private readonly Live liveSource; private readonly IDisposable? realmSubscription; - public RealmBackedResourceStore(Live source, IResourceStore underlyingStore, RealmAccess realm) + public RealmBackedResourceStore(Live source, IResourceStore underlyingStore, RealmAccess? realm) : base(underlyingStore) { liveSource = source; @@ -31,7 +31,7 @@ namespace osu.Game.Skinning invalidateCache(); Debug.Assert(fileToStoragePathMapping != null); - realmSubscription = realm.RegisterForNotifications(r => r.All().Where(s => s.ID == source.ID), skinChanged); + realmSubscription = realm?.RegisterForNotifications(r => r.All().Where(s => s.ID == source.ID), skinChanged); } protected override void Dispose(bool disposing) From 2ec15a1ebe83c3e18bbde06f9f41c29db7780797 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Tue, 5 Apr 2022 16:47:15 +0900 Subject: [PATCH 19/19] Fix lookup through transformers --- osu.Game/Skinning/LegacySkinTransformer.cs | 2 +- osu.Game/Skinning/SkinnableSprite.cs | 23 +++++++++++++++++++++- 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/osu.Game/Skinning/LegacySkinTransformer.cs b/osu.Game/Skinning/LegacySkinTransformer.cs index 97084f34e0..9481fc7182 100644 --- a/osu.Game/Skinning/LegacySkinTransformer.cs +++ b/osu.Game/Skinning/LegacySkinTransformer.cs @@ -23,7 +23,7 @@ namespace osu.Game.Skinning /// The which is being transformed. /// [NotNull] - protected ISkin Skin { get; } + protected internal ISkin Skin { get; } protected LegacySkinTransformer([NotNull] ISkin skin) { diff --git a/osu.Game/Skinning/SkinnableSprite.cs b/osu.Game/Skinning/SkinnableSprite.cs index c5f110f908..4b4d7fe2c6 100644 --- a/osu.Game/Skinning/SkinnableSprite.cs +++ b/osu.Game/Skinning/SkinnableSprite.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.Allocation; using osu.Framework.Bindables; @@ -83,7 +84,7 @@ namespace osu.Game.Skinning // Round-about way of getting the user's skin to find available resources. // In the future we'll probably want to allow access to resources from the fallbacks, or potentially other skins // but that requires further thought. - var highestPrioritySkin = ((SkinnableSprite)SettingSourceObject).source.AllSources.First() as Skin; + var highestPrioritySkin = getHighestPriorityUserSkin(((SkinnableSprite)SettingSourceObject).source.AllSources) as Skin; string[] availableFiles = highestPrioritySkin?.SkinInfo.PerformRead(s => s.Files .Where(f => f.Filename.EndsWith(".png", StringComparison.Ordinal) @@ -92,6 +93,26 @@ namespace osu.Game.Skinning if (availableFiles?.Length > 0) Items = availableFiles; + + static ISkin getHighestPriorityUserSkin(IEnumerable skins) + { + foreach (var skin in skins) + { + if (skin is LegacySkinTransformer transformer && isUserSkin(transformer.Skin)) + return transformer.Skin; + + if (isUserSkin(skin)) + return skin; + } + + return null; + } + + // Temporarily used to exclude undesirable ISkin implementations + static bool isUserSkin(ISkin skin) + => skin.GetType() == typeof(DefaultSkin) + || skin.GetType() == typeof(DefaultLegacySkin) + || skin.GetType() == typeof(LegacySkin); } }