From 1dd5e1ca89d78190cea96f56d0d05ee691ac7908 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 26 Nov 2021 14:43:35 +0900 Subject: [PATCH 01/34] Remove `RealmLive` context re-fetch optimisation for now --- osu.Game/Database/RealmLive.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/osu.Game/Database/RealmLive.cs b/osu.Game/Database/RealmLive.cs index 5ee40f5b4d..45b598ed92 100644 --- a/osu.Game/Database/RealmLive.cs +++ b/osu.Game/Database/RealmLive.cs @@ -112,7 +112,10 @@ namespace osu.Game.Database } } - private bool originalDataValid => !IsManaged || (isCorrectThread && data.IsValid); + // TODO: Revisit adding these conditionals back as an optimisation: || (isCorrectThread && data.IsValid); + // They have temporarily been removed due to an oversight involving .AsQueryable, see https://github.com/realm/realm-dotnet/discussions/2734. + // This means we are fetching a new context every `PerformRead` or `PerformWrite`, even when on the correct thread. + private bool originalDataValid => !IsManaged; // this matches realm's internal thread validation (see https://github.com/realm/realm-dotnet/blob/903b4d0b304f887e37e2d905384fb572a6496e70/Realm/Realm/Native/SynchronizationContextScheduler.cs#L72) private bool isCorrectThread From 348d1d0be910ccacf482360150c6b77634af560b Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 26 Nov 2021 21:33:11 +0900 Subject: [PATCH 02/34] Don't dispose fetched realm instance when using `RealmLive.Value` See https://github.com/realm/realm-dotnet/discussions/2734#discussioncomment-1705038 for reasoning. --- osu.Game/Database/RealmLive.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/osu.Game/Database/RealmLive.cs b/osu.Game/Database/RealmLive.cs index 45b598ed92..6d5410d1cd 100644 --- a/osu.Game/Database/RealmLive.cs +++ b/osu.Game/Database/RealmLive.cs @@ -100,10 +100,11 @@ namespace osu.Game.Database if (originalDataValid) return data; - T retrieved; + if (!isCorrectThread) + throw new InvalidOperationException($"Can't use {nameof(Value)} unless on the same thread the original data was fetched from."); - using (var realm = Realm.GetInstance(data.Realm.Config)) - retrieved = realm.Find(ID); + var realm = Realm.GetInstance(data.Realm.Config); + var retrieved = realm.Find(ID); if (!retrieved.IsValid) throw new InvalidOperationException("Attempted to access value without an open context"); From 4c2b0dd2d1168df00120016d02af29861eb9052a Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 29 Nov 2021 15:14:27 +0900 Subject: [PATCH 03/34] Update `RealmLive` tests in line with modified behaviour --- osu.Game.Tests/Database/RealmLiveTests.cs | 68 ++++++++++++++--------- 1 file changed, 42 insertions(+), 26 deletions(-) diff --git a/osu.Game.Tests/Database/RealmLiveTests.cs b/osu.Game.Tests/Database/RealmLiveTests.cs index 16e2c0fc6a..baedf44c1a 100644 --- a/osu.Game.Tests/Database/RealmLiveTests.cs +++ b/osu.Game.Tests/Database/RealmLiveTests.cs @@ -82,19 +82,29 @@ namespace osu.Game.Tests.Database Task.Factory.StartNew(() => { - Assert.DoesNotThrow(() => + // TODO: The commented code is the behaviour we hope to obtain, but is temporarily disabled. + // See https://github.com/ppy/osu/pull/15851 + using (realmFactory.CreateContext()) { - using (realmFactory.CreateContext()) + Assert.Throws(() => { - var resolved = liveBeatmap.Value; + var __ = liveBeatmap.Value; + }); + } - Assert.IsTrue(resolved.Realm.IsClosed); - Assert.IsTrue(resolved.IsValid); - - // can access properties without a crash. - Assert.IsFalse(resolved.Hidden); - } - }); + // Assert.DoesNotThrow(() => + // { + // using (realmFactory.CreateContext()) + // { + // var resolved = liveBeatmap.Value; + // + // Assert.IsTrue(resolved.Realm.IsClosed); + // Assert.IsTrue(resolved.IsValid); + // + // // can access properties without a crash. + // Assert.IsFalse(resolved.Hidden); + // } + // }); }, TaskCreationOptions.LongRunning | TaskCreationOptions.HideScheduler).Wait(); }); } @@ -215,23 +225,29 @@ namespace osu.Game.Tests.Database Assert.AreEqual(0, updateThreadContext.All().Count()); Assert.AreEqual(0, changesTriggered); - var resolved = liveBeatmap.Value; - - // retrieval causes an implicit refresh. even changes that aren't related to the retrieval are fired at this point. - Assert.AreEqual(2, updateThreadContext.All().Count()); - Assert.AreEqual(1, changesTriggered); - - // even though the realm that this instance was resolved for was closed, it's still valid. - Assert.IsTrue(resolved.Realm.IsClosed); - Assert.IsTrue(resolved.IsValid); - - // can access properties without a crash. - Assert.IsFalse(resolved.Hidden); - - updateThreadContext.Write(r => + // TODO: Originally the following was using `liveBeatmap.Value`. This has been temporarily disabled. + // See https://github.com/ppy/osu/pull/15851 + liveBeatmap.PerformRead(resolved => { - // can use with the main context. - r.Remove(resolved); + // retrieval causes an implicit refresh. even changes that aren't related to the retrieval are fired at this point. + // ReSharper disable once AccessToDisposedClosure + Assert.AreEqual(2, updateThreadContext.All().Count()); + Assert.AreEqual(1, changesTriggered); + + // TODO: as above, temporarily disabled as it doesn't make sense inside a `PerformRead`. + // // even though the realm that this instance was resolved for was closed, it's still valid. + // Assert.IsTrue(resolved.Realm.IsClosed); + // Assert.IsTrue(resolved.IsValid); + + // can access properties without a crash. + Assert.IsFalse(resolved.Hidden); + + // ReSharper disable once AccessToDisposedClosure + updateThreadContext.Write(r => + { + // can use with the main context. + r.Remove(resolved); + }); }); } From 0a961fd9d8d2da027f88904e575ec952d5535b7e Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 23 Nov 2021 13:24:43 +0900 Subject: [PATCH 04/34] Replace usages of `IHasFiles` with `IHasRealmFiles` --- osu.Game/Skinning/DefaultLegacySkin.cs | 2 +- osu.Game/Skinning/LegacyBeatmapSkin.cs | 2 +- osu.Game/Skinning/LegacySkin.cs | 2 +- osu.Game/Skinning/LegacySkinResourceStore.cs | 9 ++++----- 4 files changed, 7 insertions(+), 8 deletions(-) diff --git a/osu.Game/Skinning/DefaultLegacySkin.cs b/osu.Game/Skinning/DefaultLegacySkin.cs index cd6dbd9ddd..95e9a19c99 100644 --- a/osu.Game/Skinning/DefaultLegacySkin.cs +++ b/osu.Game/Skinning/DefaultLegacySkin.cs @@ -25,7 +25,7 @@ namespace osu.Game.Skinning resources, // A default legacy skin may still have a skin.ini if it is modified by the user. // We must specify the stream directly as we are redirecting storage to the osu-resources location for other files. - new LegacySkinResourceStore(skin, resources.Files).GetStream("skin.ini") + new LegacySkinResourceStore(skin, resources.Files).GetStream("skin.ini") ) { Configuration.CustomColours["SliderBall"] = new Color4(2, 170, 255, 255); diff --git a/osu.Game/Skinning/LegacyBeatmapSkin.cs b/osu.Game/Skinning/LegacyBeatmapSkin.cs index 8abef6800d..0599334b46 100644 --- a/osu.Game/Skinning/LegacyBeatmapSkin.cs +++ b/osu.Game/Skinning/LegacyBeatmapSkin.cs @@ -21,7 +21,7 @@ namespace osu.Game.Skinning protected override bool UseCustomSampleBanks => true; public LegacyBeatmapSkin(BeatmapInfo beatmapInfo, IResourceStore storage, IStorageResourceProvider resources) - : base(createSkinInfo(beatmapInfo), new LegacySkinResourceStore(beatmapInfo.BeatmapSet, storage), resources, beatmapInfo.Path) + : base(createSkinInfo(beatmapInfo), new LegacySkinResourceStore(beatmapInfo.BeatmapSet, storage), resources, beatmapInfo.Path) { // 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 0e7ae95169..64f920de85 100644 --- a/osu.Game/Skinning/LegacySkin.cs +++ b/osu.Game/Skinning/LegacySkin.cs @@ -51,7 +51,7 @@ namespace osu.Game.Skinning [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") { } diff --git a/osu.Game/Skinning/LegacySkinResourceStore.cs b/osu.Game/Skinning/LegacySkinResourceStore.cs index c4418baeff..71efc949b8 100644 --- a/osu.Game/Skinning/LegacySkinResourceStore.cs +++ b/osu.Game/Skinning/LegacySkinResourceStore.cs @@ -11,12 +11,11 @@ using osu.Game.Extensions; namespace osu.Game.Skinning { - public class LegacySkinResourceStore : ResourceStore - where T : INamedFileInfo + public class LegacySkinResourceStore : ResourceStore { - private readonly IHasFiles source; + private readonly IHasRealmFiles source; - public LegacySkinResourceStore(IHasFiles source, IResourceStore underlyingStore) + public LegacySkinResourceStore(IHasRealmFiles source, IResourceStore underlyingStore) : base(underlyingStore) { this.source = source; @@ -33,7 +32,7 @@ namespace osu.Game.Skinning } private string getPathForFile(string filename) => - source.Files.Find(f => string.Equals(f.Filename, filename, StringComparison.OrdinalIgnoreCase))?.FileInfo.GetStoragePath(); + source.Files.FirstOrDefault(f => string.Equals(f.Filename, filename, StringComparison.OrdinalIgnoreCase))?.FileInfo.GetStoragePath(); public override IEnumerable GetAvailableResources() => source.Files.Select(f => f.Filename); } From 5f067172b41e243aa79d8d25bac024b551212ce6 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 23 Nov 2021 14:54:57 +0900 Subject: [PATCH 05/34] Add model class for realm skin --- osu.Game/Models/RealmSkin.cs | 67 ++++++++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) create mode 100644 osu.Game/Models/RealmSkin.cs diff --git a/osu.Game/Models/RealmSkin.cs b/osu.Game/Models/RealmSkin.cs new file mode 100644 index 0000000000..2ae1328974 --- /dev/null +++ b/osu.Game/Models/RealmSkin.cs @@ -0,0 +1,67 @@ +// 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 osu.Framework.Extensions.ObjectExtensions; +using osu.Framework.Testing; +using osu.Game.Database; +using osu.Game.Extensions; +using osu.Game.IO; +using osu.Game.Skinning; +using Realms; + +#nullable enable + +namespace osu.Game.Models +{ + [ExcludeFromDynamicCompile] + [MapTo("Skin")] + public class RealmSkin : RealmObject, IHasRealmFiles, IEquatable, IHasGuidPrimaryKey, ISoftDelete + { + public Guid ID { get; set; } + + public string Name { get; set; } = string.Empty; + + public string Creator { get; set; } = string.Empty; + + public string Hash { get; set; } = string.Empty; + + public string InstantiationInfo { get; set; } = string.Empty; + + public virtual Skin CreateInstance(IStorageResourceProvider resources) + { + var type = string.IsNullOrEmpty(InstantiationInfo) + // handle the case of skins imported before InstantiationInfo was added. + ? typeof(LegacySkin) + : Type.GetType(InstantiationInfo).AsNonNull(); + + return (Skin)Activator.CreateInstance(type, this, resources); + } + + public IList Files { get; } = null!; + + public bool DeletePending { get; set; } + + public static RealmSkin Default { get; } = new RealmSkin + { + Name = "osu! (triangles)", + Creator = "team osu!", + InstantiationInfo = typeof(DefaultSkin).GetInvariantInstantiationInfo() + }; + + public bool Equals(RealmSkin? other) + { + if (ReferenceEquals(this, other)) return true; + if (other == null) return false; + + return ID == other.ID; + } + + public override string ToString() + { + string author = string.IsNullOrEmpty(Creator) ? string.Empty : $"({Creator})"; + return $"{Name} {author}".Trim(); + } + } +} From e283379f0ebaa703bf613a9108604a98c162bd91 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 23 Nov 2021 15:31:07 +0900 Subject: [PATCH 06/34] Replace EF `SkinInfo` with realm implementation --- osu.Game/Database/OsuDbContext.cs | 6 +-- .../RealmSkin.cs => Skinning/EFSkinInfo.cs} | 40 ++++++++----------- osu.Game/Skinning/SkinInfo.cs | 37 +++++++++++------ osu.Game/Skinning/SkinStore.cs | 2 +- 4 files changed, 46 insertions(+), 39 deletions(-) rename osu.Game/{Models/RealmSkin.cs => Skinning/EFSkinInfo.cs} (54%) diff --git a/osu.Game/Database/OsuDbContext.cs b/osu.Game/Database/OsuDbContext.cs index d8d2cb8981..26f287da26 100644 --- a/osu.Game/Database/OsuDbContext.cs +++ b/osu.Game/Database/OsuDbContext.cs @@ -25,7 +25,7 @@ namespace osu.Game.Database public DbSet BeatmapSetInfo { get; set; } public DbSet FileInfo { get; set; } public DbSet RulesetInfo { get; set; } - public DbSet SkinInfo { get; set; } + public DbSet SkinInfo { get; set; } public DbSet ScoreInfo { get; set; } // migrated to realm @@ -133,8 +133,8 @@ namespace osu.Game.Database modelBuilder.Entity().HasIndex(b => b.DeletePending); modelBuilder.Entity().HasIndex(b => b.Hash).IsUnique(); - modelBuilder.Entity().HasIndex(b => b.Hash).IsUnique(); - modelBuilder.Entity().HasIndex(b => b.DeletePending); + modelBuilder.Entity().HasIndex(b => b.Hash).IsUnique(); + modelBuilder.Entity().HasIndex(b => b.DeletePending); modelBuilder.Entity().HasIndex(b => new { b.RulesetID, b.Variant }); diff --git a/osu.Game/Models/RealmSkin.cs b/osu.Game/Skinning/EFSkinInfo.cs similarity index 54% rename from osu.Game/Models/RealmSkin.cs rename to osu.Game/Skinning/EFSkinInfo.cs index 2ae1328974..8bd734d4ff 100644 --- a/osu.Game/Models/RealmSkin.cs +++ b/osu.Game/Skinning/EFSkinInfo.cs @@ -1,33 +1,32 @@ -// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// 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.ComponentModel.DataAnnotations.Schema; using osu.Framework.Extensions.ObjectExtensions; -using osu.Framework.Testing; using osu.Game.Database; using osu.Game.Extensions; using osu.Game.IO; -using osu.Game.Skinning; -using Realms; -#nullable enable - -namespace osu.Game.Models +namespace osu.Game.Skinning { - [ExcludeFromDynamicCompile] - [MapTo("Skin")] - public class RealmSkin : RealmObject, IHasRealmFiles, IEquatable, IHasGuidPrimaryKey, ISoftDelete + [Table(@"SkinInfo")] + public class EFSkinInfo : IHasFiles, IEquatable, IHasPrimaryKey, ISoftDelete { - public Guid ID { get; set; } + internal const int DEFAULT_SKIN = 0; + internal const int CLASSIC_SKIN = -1; + internal const int RANDOM_SKIN = -2; + + public int ID { get; set; } public string Name { get; set; } = string.Empty; public string Creator { get; set; } = string.Empty; - public string Hash { get; set; } = string.Empty; + public string Hash { get; set; } - public string InstantiationInfo { get; set; } = string.Empty; + public string InstantiationInfo { get; set; } public virtual Skin CreateInstance(IStorageResourceProvider resources) { @@ -39,28 +38,23 @@ namespace osu.Game.Models return (Skin)Activator.CreateInstance(type, this, resources); } - public IList Files { get; } = null!; + public List Files { get; set; } = new List(); public bool DeletePending { get; set; } - public static RealmSkin Default { get; } = new RealmSkin + public static EFSkinInfo Default { get; } = new EFSkinInfo { + ID = DEFAULT_SKIN, Name = "osu! (triangles)", Creator = "team osu!", InstantiationInfo = typeof(DefaultSkin).GetInvariantInstantiationInfo() }; - public bool Equals(RealmSkin? other) - { - if (ReferenceEquals(this, other)) return true; - if (other == null) return false; - - return ID == other.ID; - } + public bool Equals(EFSkinInfo other) => other != null && ID == other.ID; public override string ToString() { - string author = string.IsNullOrEmpty(Creator) ? string.Empty : $"({Creator})"; + string author = Creator == null ? string.Empty : $"({Creator})"; return $"{Name} {author}".Trim(); } } diff --git a/osu.Game/Skinning/SkinInfo.cs b/osu.Game/Skinning/SkinInfo.cs index 5d2d51a9b0..a83e7f5b1e 100644 --- a/osu.Game/Skinning/SkinInfo.cs +++ b/osu.Game/Skinning/SkinInfo.cs @@ -1,30 +1,38 @@ -// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// 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 osu.Framework.Extensions.ObjectExtensions; +using osu.Framework.Testing; using osu.Game.Database; using osu.Game.Extensions; using osu.Game.IO; +using osu.Game.Models; +using Realms; + +#nullable enable namespace osu.Game.Skinning { - public class SkinInfo : IHasFiles, IEquatable, IHasPrimaryKey, ISoftDelete, IHasNamedFiles + [ExcludeFromDynamicCompile] + [MapTo("Skin")] + public class SkinInfo : RealmObject, IHasRealmFiles, IEquatable, IHasGuidPrimaryKey, ISoftDelete, IHasNamedFiles { - internal const int DEFAULT_SKIN = 0; - internal const int CLASSIC_SKIN = -1; - internal const int RANDOM_SKIN = -2; + internal static readonly Guid DEFAULT_SKIN = new Guid("2991CFD8-2140-469A-BCB9-2EC23FBCE4AD"); + internal static readonly Guid CLASSIC_SKIN = new Guid("81F02CD3-EEC6-4865-AC23-FAE26A386187"); + internal static readonly Guid RANDOM_SKIN = new Guid("D39DFEFB-477C-4372-B1EA-2BCEA5FB8908"); - public int ID { get; set; } + [PrimaryKey] + public Guid ID { get; set; } = Guid.NewGuid(); public string Name { get; set; } = string.Empty; public string Creator { get; set; } = string.Empty; - public string Hash { get; set; } + public string Hash { get; set; } = string.Empty; - public string InstantiationInfo { get; set; } + public string InstantiationInfo { get; set; } = string.Empty; public virtual Skin CreateInstance(IStorageResourceProvider resources) { @@ -36,23 +44,28 @@ namespace osu.Game.Skinning return (Skin)Activator.CreateInstance(type, this, resources); } - public List Files { get; } = new List(); + public IList Files { get; } = null!; public bool DeletePending { get; set; } public static SkinInfo Default { get; } = new SkinInfo { - ID = DEFAULT_SKIN, Name = "osu! (triangles)", Creator = "team osu!", InstantiationInfo = typeof(DefaultSkin).GetInvariantInstantiationInfo() }; - public bool Equals(SkinInfo other) => other != null && ID == other.ID; + public bool Equals(SkinInfo? other) + { + if (ReferenceEquals(this, other)) return true; + if (other == null) return false; + + return ID == other.ID; + } public override string ToString() { - string author = Creator == null ? string.Empty : $"({Creator})"; + string author = string.IsNullOrEmpty(Creator) ? string.Empty : $"({Creator})"; return $"{Name} {author}".Trim(); } diff --git a/osu.Game/Skinning/SkinStore.cs b/osu.Game/Skinning/SkinStore.cs index 31cadb0a24..922d146259 100644 --- a/osu.Game/Skinning/SkinStore.cs +++ b/osu.Game/Skinning/SkinStore.cs @@ -6,7 +6,7 @@ using osu.Game.Database; namespace osu.Game.Skinning { - public class SkinStore : MutableDatabaseBackedStoreWithFileIncludes + public class SkinStore : MutableDatabaseBackedStoreWithFileIncludes { public SkinStore(DatabaseContextFactory contextFactory, Storage storage = null) : base(contextFactory, storage) From 3db5646fa846e5c6d2550915e626d1fda5c0ed66 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 23 Nov 2021 16:04:55 +0900 Subject: [PATCH 07/34] Create Guid constants for system skins (and store skin choice to configuration as guid) --- osu.Game/Configuration/OsuConfigManager.cs | 11 +++++--- osu.Game/OsuGame.cs | 26 ++++++++----------- .../Overlays/Settings/Sections/SkinSection.cs | 20 +++++++------- osu.Game/Skinning/SkinInfo.cs | 1 + 4 files changed, 29 insertions(+), 29 deletions(-) diff --git a/osu.Game/Configuration/OsuConfigManager.cs b/osu.Game/Configuration/OsuConfigManager.cs index 84da3f666d..ade817e652 100644 --- a/osu.Game/Configuration/OsuConfigManager.cs +++ b/osu.Game/Configuration/OsuConfigManager.cs @@ -27,7 +27,7 @@ namespace osu.Game.Configuration { // UI/selection defaults SetDefault(OsuSetting.Ruleset, string.Empty); - SetDefault(OsuSetting.Skin, 0, -1, int.MaxValue); + SetDefault(OsuSetting.Skin, string.Empty); SetDefault(OsuSetting.BeatmapDetailTab, PlayBeatmapDetailArea.TabType.Details); SetDefault(OsuSetting.BeatmapDetailModsFilter, false); @@ -210,9 +210,12 @@ namespace osu.Game.Configuration value: scalingMode.GetLocalisableDescription() ) ), - new TrackedSetting(OsuSetting.Skin, skin => + new TrackedSetting(OsuSetting.Skin, skin => { - string skinName = LookupSkinName(skin) ?? string.Empty; + string skinName = string.Empty; + + if (Guid.TryParse(skin, out var id)) + skinName = LookupSkinName(id) ?? string.Empty; return new SettingDescription( rawValue: skinName, @@ -233,7 +236,7 @@ namespace osu.Game.Configuration }; } - public Func LookupSkinName { private get; set; } + public Func LookupSkinName { private get; set; } public Func LookupKeyBindings { get; set; } } diff --git a/osu.Game/OsuGame.cs b/osu.Game/OsuGame.cs index 99b67976e3..c294efa647 100644 --- a/osu.Game/OsuGame.cs +++ b/osu.Game/OsuGame.cs @@ -161,7 +161,7 @@ namespace osu.Game private Bindable uiScale; - private Bindable configSkin; + private Bindable configSkin; private readonly string[] args; @@ -243,27 +243,23 @@ namespace osu.Game Ruleset.ValueChanged += r => configRuleset.Value = r.NewValue.ShortName; // bind config int to database SkinInfo - configSkin = LocalConfig.GetBindable(OsuSetting.Skin); - SkinManager.CurrentSkinInfo.ValueChanged += skin => configSkin.Value = skin.NewValue.ID; + configSkin = LocalConfig.GetBindable(OsuSetting.Skin); + SkinManager.CurrentSkinInfo.ValueChanged += skin => configSkin.Value = skin.NewValue.ID.ToString(); configSkin.ValueChanged += skinId => { - var skinInfo = SkinManager.Query(s => s.ID == skinId.NewValue); + // TODO: migrate the user skin selection to the new ID format. + SkinInfo skinInfo = null; + + if (Guid.TryParse(skinId.NewValue, out var guid)) + skinInfo = SkinManager.Query(s => s.ID == guid); if (skinInfo == null) { - switch (skinId.NewValue) - { - case -1: - skinInfo = DefaultLegacySkin.Info; - break; - - default: - skinInfo = SkinInfo.Default; - break; - } + if (guid == SkinInfo.CLASSIC_SKIN) + skinInfo = DefaultLegacySkin.Info; } - SkinManager.CurrentSkinInfo.Value = skinInfo; + SkinManager.CurrentSkinInfo.Value = skinInfo ?? SkinInfo.Default; }; configSkin.TriggerChange(); diff --git a/osu.Game/Overlays/Settings/Sections/SkinSection.cs b/osu.Game/Overlays/Settings/Sections/SkinSection.cs index 0eb65b4b0f..168214bbde 100644 --- a/osu.Game/Overlays/Settings/Sections/SkinSection.cs +++ b/osu.Game/Overlays/Settings/Sections/SkinSection.cs @@ -33,7 +33,7 @@ namespace osu.Game.Overlays.Settings.Sections }; private readonly Bindable dropdownBindable = new Bindable { Default = SkinInfo.Default }; - private readonly Bindable configBindable = new Bindable(); + private readonly Bindable configBindable = new Bindable(); private static readonly SkinInfo random_skin_info = new SkinInfo { @@ -47,7 +47,7 @@ namespace osu.Game.Overlays.Settings.Sections { get { - int index = skinItems.FindIndex(s => s.ID > 0); + int index = skinItems.FindIndex(s => s.ID == SkinInfo.CLASSIC_SKIN); if (index < 0) index = skinItems.Count; @@ -84,33 +84,33 @@ namespace osu.Game.Overlays.Settings.Sections updateItems(); // Todo: This should not be necessary when OsuConfigManager is databased - if (skinDropdown.Items.All(s => s.ID != configBindable.Value)) - configBindable.Value = 0; + if (!Guid.TryParse(configBindable.Value, out var configId) || skinDropdown.Items.All(s => s.ID != configId)) + configBindable.Value = string.Empty; configBindable.BindValueChanged(id => Scheduler.AddOnce(updateSelectedSkinFromConfig), true); dropdownBindable.BindValueChanged(skin => { - if (skin.NewValue == random_skin_info) + if (skin.NewValue.Equals(random_skin_info)) { skins.SelectRandomSkin(); return; } - configBindable.Value = skin.NewValue.ID; + configBindable.Value = skin.NewValue.ID.ToString(); }); } private void updateSelectedSkinFromConfig() { - int id = configBindable.Value; + if (!Guid.TryParse(configBindable.Value, out var configId)) return; - var skin = skinDropdown.Items.FirstOrDefault(s => s.ID == id); + var skin = skinDropdown.Items.FirstOrDefault(s => s.ID == configId); if (skin == null) { // there may be a thread race condition where an item is selected that hasn't yet been added to the dropdown. // to avoid adding complexity, let's just ensure the item is added so we can perform the selection. - skin = skins.Query(s => s.ID == id); + skin = skins.Query(s => s.ID == configId); addItem(skin); } @@ -181,7 +181,7 @@ namespace osu.Game.Overlays.Settings.Sections Action = export; currentSkin = skins.CurrentSkin.GetBoundCopy(); - currentSkin.BindValueChanged(skin => Enabled.Value = skin.NewValue.SkinInfo.ID > 0, true); + currentSkin.BindValueChanged(skin => Enabled.Value = skin.NewValue.SkinInfo.IsManaged, true); } private void export() diff --git a/osu.Game/Skinning/SkinInfo.cs b/osu.Game/Skinning/SkinInfo.cs index a83e7f5b1e..c09d45c227 100644 --- a/osu.Game/Skinning/SkinInfo.cs +++ b/osu.Game/Skinning/SkinInfo.cs @@ -50,6 +50,7 @@ namespace osu.Game.Skinning public static SkinInfo Default { get; } = new SkinInfo { + ID = DEFAULT_SKIN, Name = "osu! (triangles)", Creator = "team osu!", InstantiationInfo = typeof(DefaultSkin).GetInvariantInstantiationInfo() From 2a9c7c00c8a261c63bb1e8860066940fbad7a2a3 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 23 Nov 2021 16:05:28 +0900 Subject: [PATCH 08/34] Update tests and file access code --- osu.Game.Tests/Skins/IO/ImportSkinTest.cs | 4 ++-- .../Visual/Background/TestSceneBackgroundScreenDefault.cs | 2 +- osu.Game/Skinning/Skin.cs | 4 ++-- osu.Game/Tests/Beatmaps/HitObjectSampleTest.cs | 7 ++----- 4 files changed, 7 insertions(+), 10 deletions(-) diff --git a/osu.Game.Tests/Skins/IO/ImportSkinTest.cs b/osu.Game.Tests/Skins/IO/ImportSkinTest.cs index ecc9c92025..64d7359480 100644 --- a/osu.Game.Tests/Skins/IO/ImportSkinTest.cs +++ b/osu.Game.Tests/Skins/IO/ImportSkinTest.cs @@ -181,14 +181,14 @@ namespace osu.Game.Tests.Skins.IO { Assert.That(import2.ID, Is.Not.EqualTo(import1.ID)); Assert.That(import2.Hash, Is.Not.EqualTo(import1.Hash)); - Assert.That(import2.Files.Select(f => f.FileInfoID), Is.Not.EquivalentTo(import1.Files.Select(f => f.FileInfoID))); + Assert.That(import2.Files.First(), Is.Not.EqualTo(import1.Files.First())); } private void assertImportedOnce(SkinInfo import1, SkinInfo import2) { Assert.That(import2.ID, Is.EqualTo(import1.ID)); Assert.That(import2.Hash, Is.EqualTo(import1.Hash)); - Assert.That(import2.Files.Select(f => f.FileInfoID), Is.EquivalentTo(import1.Files.Select(f => f.FileInfoID))); + Assert.That(import2.Files.First(), Is.EqualTo(import1.Files.First())); } private MemoryStream createEmptyOsk() diff --git a/osu.Game.Tests/Visual/Background/TestSceneBackgroundScreenDefault.cs b/osu.Game.Tests/Visual/Background/TestSceneBackgroundScreenDefault.cs index ec16578b71..a56b69a024 100644 --- a/osu.Game.Tests/Visual/Background/TestSceneBackgroundScreenDefault.cs +++ b/osu.Game.Tests/Visual/Background/TestSceneBackgroundScreenDefault.cs @@ -122,7 +122,7 @@ namespace osu.Game.Tests.Visual.Background 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 }); + AddStep("set custom skin", () => skins.CurrentSkinInfo.Value = new SkinInfo()); } private void setDefaultSkin() => AddStep("set default skin", () => skins.CurrentSkinInfo.SetDefault()); diff --git a/osu.Game/Skinning/Skin.cs b/osu.Game/Skinning/Skin.cs index 10526b69af..2f16270fb7 100644 --- a/osu.Game/Skinning/Skin.cs +++ b/osu.Game/Skinning/Skin.cs @@ -64,7 +64,7 @@ namespace osu.Game.Skinning if (fileInfo == null) continue; - byte[] bytes = resources?.Files.Get(fileInfo.FileInfo.GetStoragePath()); + byte[] bytes = resources?.Files.Get(fileInfo.File.GetStoragePath()); if (bytes == null) continue; @@ -94,7 +94,7 @@ namespace osu.Game.Skinning private Stream getConfigurationStream() { - string path = SkinInfo.Files.SingleOrDefault(f => f.Filename.Equals(@"skin.ini", StringComparison.OrdinalIgnoreCase))?.FileInfo.GetStoragePath(); + string path = SkinInfo.Files.SingleOrDefault(f => f.Filename.Equals(@"skin.ini", StringComparison.OrdinalIgnoreCase))?.File.GetStoragePath(); if (string.IsNullOrEmpty(path)) return null; diff --git a/osu.Game/Tests/Beatmaps/HitObjectSampleTest.cs b/osu.Game/Tests/Beatmaps/HitObjectSampleTest.cs index 9d92f5c5fc..31a2071249 100644 --- a/osu.Game/Tests/Beatmaps/HitObjectSampleTest.cs +++ b/osu.Game/Tests/Beatmaps/HitObjectSampleTest.cs @@ -15,6 +15,7 @@ using osu.Framework.Timing; using osu.Game.Beatmaps; using osu.Game.Beatmaps.Formats; using osu.Game.IO; +using osu.Game.Models; using osu.Game.Rulesets; using osu.Game.Rulesets.Objects; using osu.Game.Screens.Ranking; @@ -88,11 +89,7 @@ namespace osu.Game.Tests.Beatmaps AddStep("setup skins", () => { userSkinInfo.Files.Clear(); - userSkinInfo.Files.Add(new SkinFileInfo - { - Filename = userFile, - FileInfo = new IO.FileInfo { Hash = userFile } - }); + userSkinInfo.Files.Add(new RealmNamedFileUsage(new RealmFile { Hash = userFile }, userFile)); beatmapInfo.BeatmapSet.Files.Clear(); beatmapInfo.BeatmapSet.Files.Add(new BeatmapSetFileInfo From 23146d59d1b1ee9464e52723113ec86d798130f5 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 29 Nov 2021 17:57:17 +0900 Subject: [PATCH 09/34] Use `ILive` for current skin --- .../Editor/TestSceneManiaComposeScreen.cs | 5 ++- osu.Game.Tests/Skins/IO/ImportSkinTest.cs | 44 ++++++++++++------- .../Skins/TestSceneSkinResources.cs | 2 +- .../TestSceneBackgroundScreenDefault.cs | 3 +- .../Gameplay/TestSceneBeatmapSkinFallbacks.cs | 3 +- osu.Game/OsuGame.cs | 6 +-- osu.Game/Skinning/SkinManager.cs | 2 +- osu.Game/Tests/Visual/SkinnableTestScene.cs | 2 +- 8 files changed, 40 insertions(+), 27 deletions(-) diff --git a/osu.Game.Rulesets.Mania.Tests/Editor/TestSceneManiaComposeScreen.cs b/osu.Game.Rulesets.Mania.Tests/Editor/TestSceneManiaComposeScreen.cs index 24d2a786a0..a7e76e44b5 100644 --- a/osu.Game.Rulesets.Mania.Tests/Editor/TestSceneManiaComposeScreen.cs +++ b/osu.Game.Rulesets.Mania.Tests/Editor/TestSceneManiaComposeScreen.cs @@ -8,6 +8,7 @@ using osu.Framework.Allocation; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Framework.Testing; +using osu.Game.Database; using osu.Game.Rulesets.Edit; using osu.Game.Rulesets.Mania.Beatmaps; using osu.Game.Screens.Edit; @@ -55,13 +56,13 @@ namespace osu.Game.Rulesets.Mania.Tests.Editor [Test] public void TestDefaultSkin() { - AddStep("set default skin", () => skins.CurrentSkinInfo.Value = SkinInfo.Default); + AddStep("set default skin", () => skins.CurrentSkinInfo.Value = SkinInfo.Default.ToLive()); } [Test] public void TestLegacySkin() { - AddStep("set legacy skin", () => skins.CurrentSkinInfo.Value = DefaultLegacySkin.Info); + AddStep("set legacy skin", () => skins.CurrentSkinInfo.Value = DefaultLegacySkin.Info.ToLive()); } } } diff --git a/osu.Game.Tests/Skins/IO/ImportSkinTest.cs b/osu.Game.Tests/Skins/IO/ImportSkinTest.cs index 64d7359480..9b7e169745 100644 --- a/osu.Game.Tests/Skins/IO/ImportSkinTest.cs +++ b/osu.Game.Tests/Skins/IO/ImportSkinTest.cs @@ -9,6 +9,7 @@ using System.Threading.Tasks; using NUnit.Framework; using osu.Framework.Allocation; using osu.Framework.Platform; +using osu.Game.Database; using osu.Game.IO; using osu.Game.IO.Archives; using osu.Game.Skinning; @@ -165,30 +166,39 @@ namespace osu.Game.Tests.Skins.IO #endregion - private void assertCorrectMetadata(SkinInfo import1, string name, string creator, OsuGameBase osu) + private void assertCorrectMetadata(ILive import1, string name, string creator, OsuGameBase osu) { - Assert.That(import1.Name, Is.EqualTo(name)); - Assert.That(import1.Creator, Is.EqualTo(creator)); + import1.PerformRead(i => + { + Assert.That(i.Name, Is.EqualTo(name)); + Assert.That(i.Creator, Is.EqualTo(creator)); - // for extra safety let's reconstruct the skin, reading from the skin.ini. - var instance = import1.CreateInstance((IStorageResourceProvider)osu.Dependencies.Get(typeof(SkinManager))); + // for extra safety let's reconstruct the skin, reading from the skin.ini. + var instance = i.CreateInstance((IStorageResourceProvider)osu.Dependencies.Get(typeof(SkinManager))); - Assert.That(instance.Configuration.SkinInfo.Name, Is.EqualTo(name)); - Assert.That(instance.Configuration.SkinInfo.Creator, Is.EqualTo(creator)); + Assert.That(instance.Configuration.SkinInfo.Name, Is.EqualTo(name)); + Assert.That(instance.Configuration.SkinInfo.Creator, Is.EqualTo(creator)); + }); } - private void assertImportedBoth(SkinInfo import1, SkinInfo import2) + private void assertImportedBoth(ILive import1, ILive import2) { - Assert.That(import2.ID, Is.Not.EqualTo(import1.ID)); - Assert.That(import2.Hash, Is.Not.EqualTo(import1.Hash)); - Assert.That(import2.Files.First(), Is.Not.EqualTo(import1.Files.First())); + import1.PerformRead(i1 => import2.PerformRead(i2 => + { + Assert.That(i2.ID, Is.Not.EqualTo(i1.ID)); + Assert.That(i2.Hash, Is.Not.EqualTo(i1.Hash)); + Assert.That(i2.Files.First(), Is.Not.EqualTo(i1.Files.First())); + })); } - private void assertImportedOnce(SkinInfo import1, SkinInfo import2) + private void assertImportedOnce(ILive import1, ILive import2) { - Assert.That(import2.ID, Is.EqualTo(import1.ID)); - Assert.That(import2.Hash, Is.EqualTo(import1.Hash)); - Assert.That(import2.Files.First(), Is.EqualTo(import1.Files.First())); + import1.PerformRead(i1 => import2.PerformRead(i2 => + { + Assert.That(i2.ID, Is.EqualTo(i1.ID)); + Assert.That(i2.Hash, Is.EqualTo(i1.Hash)); + Assert.That(i2.Files.First(), Is.EqualTo(i1.Files.First())); + })); } private MemoryStream createEmptyOsk() @@ -255,10 +265,10 @@ namespace osu.Game.Tests.Skins.IO } } - private async Task loadSkinIntoOsu(OsuGameBase osu, ArchiveReader archive = null) + private async Task> loadSkinIntoOsu(OsuGameBase osu, ArchiveReader archive = null) { var skinManager = osu.Dependencies.Get(); - return (await skinManager.Import(archive)).Value; + return (await skinManager.Import(archive)); } } } diff --git a/osu.Game.Tests/Skins/TestSceneSkinResources.cs b/osu.Game.Tests/Skins/TestSceneSkinResources.cs index 10f1ab31df..09535b76e3 100644 --- a/osu.Game.Tests/Skins/TestSceneSkinResources.cs +++ b/osu.Game.Tests/Skins/TestSceneSkinResources.cs @@ -24,7 +24,7 @@ namespace osu.Game.Tests.Skins private void load() { var imported = skins.Import(new ZipArchiveReader(TestResources.OpenResource("Archives/ogg-skin.osk"))).Result; - skin = skins.GetSkin(imported.Value); + skin = imported.PerformRead(skinInfo => skins.GetSkin(skinInfo)); } [Test] diff --git a/osu.Game.Tests/Visual/Background/TestSceneBackgroundScreenDefault.cs b/osu.Game.Tests/Visual/Background/TestSceneBackgroundScreenDefault.cs index a56b69a024..bdd1b92c8d 100644 --- a/osu.Game.Tests/Visual/Background/TestSceneBackgroundScreenDefault.cs +++ b/osu.Game.Tests/Visual/Background/TestSceneBackgroundScreenDefault.cs @@ -8,6 +8,7 @@ using osu.Framework.Allocation; using osu.Framework.Screens; using osu.Framework.Testing; using osu.Game.Configuration; +using osu.Game.Database; using osu.Game.Graphics.Backgrounds; using osu.Game.Online.API; using osu.Game.Online.API.Requests.Responses; @@ -122,7 +123,7 @@ namespace osu.Game.Tests.Visual.Background 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()); + AddStep("set custom skin", () => skins.CurrentSkinInfo.Value = new SkinInfo().ToLive()); } private void setDefaultSkin() => AddStep("set default skin", () => skins.CurrentSkinInfo.SetDefault()); diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneBeatmapSkinFallbacks.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneBeatmapSkinFallbacks.cs index 7398527f57..c174a3edc2 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneBeatmapSkinFallbacks.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneBeatmapSkinFallbacks.cs @@ -12,6 +12,7 @@ using osu.Framework.Testing; using osu.Framework.Timing; using osu.Framework.Utils; using osu.Game.Beatmaps; +using osu.Game.Database; using osu.Game.Extensions; using osu.Game.Rulesets; using osu.Game.Rulesets.Osu; @@ -52,7 +53,7 @@ namespace osu.Game.Tests.Visual.Gameplay { AddStep("setup skins", () => { - skinManager.CurrentSkinInfo.Value = gameCurrentSkin; + skinManager.CurrentSkinInfo.Value = gameCurrentSkin.ToLive(); currentBeatmapSkin = getBeatmapSkin(); }); }); diff --git a/osu.Game/OsuGame.cs b/osu.Game/OsuGame.cs index c294efa647..1ffbb4cb85 100644 --- a/osu.Game/OsuGame.cs +++ b/osu.Game/OsuGame.cs @@ -248,7 +248,7 @@ namespace osu.Game configSkin.ValueChanged += skinId => { // TODO: migrate the user skin selection to the new ID format. - SkinInfo skinInfo = null; + ILive skinInfo = null; if (Guid.TryParse(skinId.NewValue, out var guid)) skinInfo = SkinManager.Query(s => s.ID == guid); @@ -256,10 +256,10 @@ namespace osu.Game if (skinInfo == null) { if (guid == SkinInfo.CLASSIC_SKIN) - skinInfo = DefaultLegacySkin.Info; + skinInfo = DefaultLegacySkin.Info.ToLive(); } - SkinManager.CurrentSkinInfo.Value = skinInfo ?? SkinInfo.Default; + SkinManager.CurrentSkinInfo.Value = skinInfo ?? SkinInfo.Default.ToLive(); }; configSkin.TriggerChange(); diff --git a/osu.Game/Skinning/SkinManager.cs b/osu.Game/Skinning/SkinManager.cs index 26ff4457af..24ec454276 100644 --- a/osu.Game/Skinning/SkinManager.cs +++ b/osu.Game/Skinning/SkinManager.cs @@ -46,7 +46,7 @@ namespace osu.Game.Skinning private readonly IResourceStore resources; public readonly Bindable CurrentSkin = new Bindable(); - public readonly Bindable CurrentSkinInfo = new Bindable(SkinInfo.Default) { Default = SkinInfo.Default }; + public readonly Bindable> CurrentSkinInfo = new Bindable>(SkinInfo.Default.ToLive()) { Default = SkinInfo.Default.ToLive() }; private readonly SkinModelManager skinModelManager; diff --git a/osu.Game/Tests/Visual/SkinnableTestScene.cs b/osu.Game/Tests/Visual/SkinnableTestScene.cs index 000e7194bc..cdd3e47930 100644 --- a/osu.Game/Tests/Visual/SkinnableTestScene.cs +++ b/osu.Game/Tests/Visual/SkinnableTestScene.cs @@ -95,7 +95,7 @@ namespace osu.Game.Tests.Visual }, new OsuSpriteText { - Text = skin?.SkinInfo?.Name ?? "none", + Text = skin?.SkinInfo?.Value.Name ?? "none", Scale = new Vector2(1.5f), Padding = new MarginPadding(5), }, From 5db7cf23d35a75f3af71cfd22303efffe352be3a Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 29 Nov 2021 17:59:41 +0900 Subject: [PATCH 10/34] Add pending deletion skin cleanup --- osu.Game/Database/RealmContextFactory.cs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/osu.Game/Database/RealmContextFactory.cs b/osu.Game/Database/RealmContextFactory.cs index 6948918fe7..911c88a61f 100644 --- a/osu.Game/Database/RealmContextFactory.cs +++ b/osu.Game/Database/RealmContextFactory.cs @@ -14,6 +14,7 @@ using osu.Framework.Statistics; using osu.Game.Configuration; using osu.Game.Input.Bindings; using osu.Game.Models; +using osu.Game.Skinning; using osu.Game.Stores; using Realms; @@ -122,6 +123,11 @@ namespace osu.Game.Database realm.Remove(s); } + var pendingDeleteSkins = realm.All().Where(s => s.DeletePending); + + foreach (var s in pendingDeleteSkins) + realm.Remove(s); + transaction.Commit(); } From cd0c811ab1fdb40cd745b1913f5e7e3f727b229f Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 29 Nov 2021 18:00:03 +0900 Subject: [PATCH 11/34] Add the ability to call `ToString` on a `RealmLive` to get the underlying object's implementation --- osu.Game/Database/RealmLive.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/osu.Game/Database/RealmLive.cs b/osu.Game/Database/RealmLive.cs index 6d5410d1cd..6f17493097 100644 --- a/osu.Game/Database/RealmLive.cs +++ b/osu.Game/Database/RealmLive.cs @@ -123,5 +123,7 @@ namespace osu.Game.Database => (fetchedContext != null && SynchronizationContext.Current == fetchedContext) || fetchedThreadId == Thread.CurrentThread.ManagedThreadId; public bool Equals(ILive? other) => ID == other?.ID; + + public override string ToString() => PerformRead(i => i.ToString()); } } From 6b55de2845d22e8f14778bf764ff1208292682e4 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 29 Nov 2021 18:02:09 +0900 Subject: [PATCH 12/34] Use `ILive` for `Skin.SkinInfo` --- osu.Game/Skinning/Skin.cs | 61 +++++++++++++++++++++------------------ 1 file changed, 33 insertions(+), 28 deletions(-) diff --git a/osu.Game/Skinning/Skin.cs b/osu.Game/Skinning/Skin.cs index 2f16270fb7..54fc2340f1 100644 --- a/osu.Game/Skinning/Skin.cs +++ b/osu.Game/Skinning/Skin.cs @@ -15,6 +15,7 @@ using osu.Framework.Graphics.OpenGL.Textures; using osu.Framework.Graphics.Textures; using osu.Framework.Logging; using osu.Game.Audio; +using osu.Game.Database; using osu.Game.Extensions; using osu.Game.IO; using osu.Game.Screens.Play.HUD; @@ -23,7 +24,7 @@ namespace osu.Game.Skinning { public abstract class Skin : IDisposable, ISkin { - public readonly SkinInfo SkinInfo; + public readonly ILive SkinInfo; private readonly IStorageResourceProvider resources; public SkinConfiguration Configuration { get; set; } @@ -42,7 +43,7 @@ namespace osu.Game.Skinning protected Skin(SkinInfo skin, IStorageResourceProvider resources, [CanBeNull] Stream configurationStream = null) { - SkinInfo = skin; + SkinInfo = skin.ToLive(); this.resources = resources; configurationStream ??= getConfigurationStream(); @@ -53,37 +54,41 @@ namespace osu.Game.Skinning else Configuration = new SkinConfiguration(); - // we may want to move this to some kind of async operation in the future. - foreach (SkinnableTarget skinnableTarget in Enum.GetValues(typeof(SkinnableTarget))) + // skininfo files may be null for default skin. + SkinInfo.PerformRead(s => { - string filename = $"{skinnableTarget}.json"; - - // skininfo files may be null for default skin. - var fileInfo = SkinInfo.Files.FirstOrDefault(f => f.Filename == filename); - - if (fileInfo == null) - continue; - - byte[] bytes = resources?.Files.Get(fileInfo.File.GetStoragePath()); - - if (bytes == null) - continue; - - try + // we may want to move this to some kind of async operation in the future. + foreach (SkinnableTarget skinnableTarget in Enum.GetValues(typeof(SkinnableTarget))) { - string jsonContent = Encoding.UTF8.GetString(bytes); - var deserializedContent = JsonConvert.DeserializeObject>(jsonContent); + string filename = $"{skinnableTarget}.json"; - if (deserializedContent == null) + // skininfo files may be null for default skin. + var fileInfo = s.Files.FirstOrDefault(f => f.Filename == filename); + + if (fileInfo == null) continue; - DrawableComponentInfo[skinnableTarget] = deserializedContent.ToArray(); + byte[] bytes = resources?.Files.Get(fileInfo.File.GetStoragePath()); + + if (bytes == null) + continue; + + try + { + string jsonContent = Encoding.UTF8.GetString(bytes); + var deserializedContent = JsonConvert.DeserializeObject>(jsonContent); + + if (deserializedContent == null) + continue; + + DrawableComponentInfo[skinnableTarget] = deserializedContent.ToArray(); + } + catch (Exception ex) + { + Logger.Error(ex, "Failed to load skin configuration."); + } } - catch (Exception ex) - { - Logger.Error(ex, "Failed to load skin configuration."); - } - } + }); } protected virtual void ParseConfigurationStream(Stream stream) @@ -94,7 +99,7 @@ namespace osu.Game.Skinning private Stream getConfigurationStream() { - string path = SkinInfo.Files.SingleOrDefault(f => f.Filename.Equals(@"skin.ini", StringComparison.OrdinalIgnoreCase))?.File.GetStoragePath(); + string path = SkinInfo.PerformRead(s => s.Files.SingleOrDefault(f => f.Filename.Equals(@"skin.ini", StringComparison.OrdinalIgnoreCase))?.File.GetStoragePath()); if (string.IsNullOrEmpty(path)) return null; From 744a5b33f5c261e124de28759ead387a52f4093b Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 29 Nov 2021 18:04:07 +0900 Subject: [PATCH 13/34] Rewrite `SkinSection` to use realm subscriptions and databased defaults --- .../Sections/Maintenance/GeneralSettings.cs | 5 +- .../Overlays/Settings/Sections/SkinSection.cs | 47 ++++++++++++------- 2 files changed, 34 insertions(+), 18 deletions(-) diff --git a/osu.Game/Overlays/Settings/Sections/Maintenance/GeneralSettings.cs b/osu.Game/Overlays/Settings/Sections/Maintenance/GeneralSettings.cs index acdf9cdea6..e7e196555f 100644 --- a/osu.Game/Overlays/Settings/Sections/Maintenance/GeneralSettings.cs +++ b/osu.Game/Overlays/Settings/Sections/Maintenance/GeneralSettings.cs @@ -106,7 +106,10 @@ namespace osu.Game.Overlays.Settings.Sections.Maintenance dialogOverlay?.Push(new MassDeleteConfirmationDialog(() => { deleteSkinsButton.Enabled.Value = false; - Task.Run(() => skins.Delete(skins.GetAllUserSkins())).ContinueWith(t => Schedule(() => deleteSkinsButton.Enabled.Value = true)); + Task.Run(() => + { + skins.Delete(s => !s.DeletePending); + }).ContinueWith(t => Schedule(() => deleteSkinsButton.Enabled.Value = true)); })); } }); diff --git a/osu.Game/Overlays/Settings/Sections/SkinSection.cs b/osu.Game/Overlays/Settings/Sections/SkinSection.cs index 168214bbde..9c5a2dff47 100644 --- a/osu.Game/Overlays/Settings/Sections/SkinSection.cs +++ b/osu.Game/Overlays/Settings/Sections/SkinSection.cs @@ -32,16 +32,16 @@ namespace osu.Game.Overlays.Settings.Sections Icon = FontAwesome.Solid.PaintBrush }; - private readonly Bindable dropdownBindable = new Bindable { Default = SkinInfo.Default }; + private readonly Bindable> dropdownBindable = new Bindable> { Default = SkinInfo.Default.ToLive() }; private readonly Bindable configBindable = new Bindable(); - private static readonly SkinInfo random_skin_info = new SkinInfo + private static readonly ILive random_skin_info = new SkinInfo { ID = SkinInfo.RANDOM_SKIN, Name = "", - }; + }.ToLive(); - private List skinItems; + private List> skinItems; private int firstNonDefaultSkinIndex { @@ -79,6 +79,11 @@ namespace osu.Game.Overlays.Settings.Sections skins.ItemRemoved += itemRemoved; config.BindWith(OsuSetting.Skin, configBindable); + } + + protected override void LoadComplete() + { + base.LoadComplete(); skinDropdown.Current = dropdownBindable; updateItems(); @@ -125,22 +130,30 @@ namespace osu.Game.Overlays.Settings.Sections skinDropdown.Items = skinItems; } - private void itemUpdated(SkinInfo item) => Schedule(() => addItem(item)); + private void itemUpdated(SkinInfo item) => Schedule(() => addItem(item.ToLive())); - private void addItem(SkinInfo item) + private void addItem(ILive item) { - List newDropdownItems = skinDropdown.Items.Where(i => !i.Equals(item)).Append(item).ToList(); + List> newDropdownItems = skinDropdown.Items.Where(i => !i.Equals(item)).Append(item).ToList(); sortUserSkins(newDropdownItems); skinDropdown.Items = newDropdownItems; } - private void itemRemoved(SkinInfo item) => Schedule(() => skinDropdown.Items = skinDropdown.Items.Where(i => !i.Equals(item)).ToArray()); + private void itemRemoved(SkinInfo item) => Schedule(() => skinDropdown.Items = skinDropdown.Items.Where(i => !i.ID.Equals(item.ID)).ToArray()); - private void sortUserSkins(List skinsList) + private void sortUserSkins(List> skinsList) { - // Sort user skins separately from built-in skins - skinsList.Sort(firstNonDefaultSkinIndex, skinsList.Count - firstNonDefaultSkinIndex, - Comparer.Create((a, b) => string.Compare(a.Name, b.Name, StringComparison.OrdinalIgnoreCase))); + try + { + // Sort user skins separately from built-in skins + skinsList.Sort(firstNonDefaultSkinIndex, skinsList.Count - firstNonDefaultSkinIndex, + Comparer>.Create((a, b) => + { + // o_________________________o + return a.PerformRead(ai => b.PerformRead(bi => string.Compare(ai.Name, bi.Name, StringComparison.OrdinalIgnoreCase))); + })); + } + catch { } } protected override void Dispose(bool isDisposing) @@ -154,13 +167,13 @@ namespace osu.Game.Overlays.Settings.Sections } } - private class SkinSettingsDropdown : SettingsDropdown + private class SkinSettingsDropdown : SettingsDropdown> { - protected override OsuDropdown CreateDropdown() => new SkinDropdownControl(); + protected override OsuDropdown> CreateDropdown() => new SkinDropdownControl(); private class SkinDropdownControl : DropdownControl { - protected override LocalisableString GenerateItemText(SkinInfo item) => item.ToString(); + protected override LocalisableString GenerateItemText(ILive item) => item.ToString(); } } @@ -181,14 +194,14 @@ namespace osu.Game.Overlays.Settings.Sections Action = export; currentSkin = skins.CurrentSkin.GetBoundCopy(); - currentSkin.BindValueChanged(skin => Enabled.Value = skin.NewValue.SkinInfo.IsManaged, true); + currentSkin.BindValueChanged(skin => Enabled.Value = skin.NewValue.SkinInfo.PerformRead(s => s.IsManaged), true); } private void export() { try { - new LegacySkinExporter(storage).Export(currentSkin.Value.SkinInfo); + currentSkin.Value.SkinInfo.PerformRead(s => new LegacySkinExporter(storage).Export(s)); } catch (Exception e) { From e2d9a685d7ed48525a9c73465d0fb326ef25c283 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 29 Nov 2021 18:05:36 +0900 Subject: [PATCH 14/34] Update skin implementations to match new structures --- osu.Game/Skinning/DefaultLegacySkin.cs | 4 +- osu.Game/Skinning/DefaultSkin.cs | 2 +- osu.Game/Skinning/LegacyBeatmapSkin.cs | 2 +- .../LegacyDatabasedSkinResourceStore.cs | 49 +++++++++++++++++++ osu.Game/Skinning/LegacySkin.cs | 2 +- osu.Game/Skinning/LegacySkinResourceStore.cs | 6 +-- 6 files changed, 57 insertions(+), 8 deletions(-) create mode 100644 osu.Game/Skinning/LegacyDatabasedSkinResourceStore.cs diff --git a/osu.Game/Skinning/DefaultLegacySkin.cs b/osu.Game/Skinning/DefaultLegacySkin.cs index 95e9a19c99..2332e6e160 100644 --- a/osu.Game/Skinning/DefaultLegacySkin.cs +++ b/osu.Game/Skinning/DefaultLegacySkin.cs @@ -25,7 +25,7 @@ namespace osu.Game.Skinning resources, // A default legacy skin may still have a skin.ini if it is modified by the user. // We must specify the stream directly as we are redirecting storage to the osu-resources location for other files. - new LegacySkinResourceStore(skin, resources.Files).GetStream("skin.ini") + new LegacyDatabasedSkinResourceStore(skin, resources.Files).GetStream("skin.ini") ) { Configuration.CustomColours["SliderBall"] = new Color4(2, 170, 255, 255); @@ -42,7 +42,7 @@ namespace osu.Game.Skinning public static SkinInfo Info { get; } = new SkinInfo { - ID = SkinInfo.CLASSIC_SKIN, // this is temporary until database storage is decided upon. + ID = osu.Game.Skinning.SkinInfo.CLASSIC_SKIN, // this is temporary until database storage is decided upon. Name = "osu!classic", Creator = "team osu!", InstantiationInfo = typeof(DefaultLegacySkin).GetInvariantInstantiationInfo() diff --git a/osu.Game/Skinning/DefaultSkin.cs b/osu.Game/Skinning/DefaultSkin.cs index c377f16f8b..8d3a83f589 100644 --- a/osu.Game/Skinning/DefaultSkin.cs +++ b/osu.Game/Skinning/DefaultSkin.cs @@ -26,7 +26,7 @@ namespace osu.Game.Skinning private readonly IStorageResourceProvider resources; public DefaultSkin(IStorageResourceProvider resources) - : this(SkinInfo.Default, resources) + : this(osu.Game.Skinning.SkinInfo.Default, resources) { } diff --git a/osu.Game/Skinning/LegacyBeatmapSkin.cs b/osu.Game/Skinning/LegacyBeatmapSkin.cs index 0599334b46..d44d3dce49 100644 --- a/osu.Game/Skinning/LegacyBeatmapSkin.cs +++ b/osu.Game/Skinning/LegacyBeatmapSkin.cs @@ -77,6 +77,6 @@ namespace osu.Game.Skinning } private static SkinInfo createSkinInfo(BeatmapInfo beatmapInfo) => - new SkinInfo { Name = beatmapInfo.ToString(), Creator = beatmapInfo.Metadata?.Author.Username }; + new SkinInfo { Name = beatmapInfo.ToString(), Creator = beatmapInfo.Metadata?.Author.Username ?? string.Empty }; } } diff --git a/osu.Game/Skinning/LegacyDatabasedSkinResourceStore.cs b/osu.Game/Skinning/LegacyDatabasedSkinResourceStore.cs new file mode 100644 index 0000000000..ab820a13ab --- /dev/null +++ b/osu.Game/Skinning/LegacyDatabasedSkinResourceStore.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; +using System.Collections.Generic; +using System.Linq; +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 LegacyDatabasedSkinResourceStore : ResourceStore + { + private readonly ILive source; + + public LegacyDatabasedSkinResourceStore(SkinInfo source, IResourceStore underlyingStore) + : base(underlyingStore) + { + this.source = source.ToLive(); + } + + protected override IEnumerable GetFilenames(string name) + { + foreach (string filename in base.GetFilenames(name)) + { + string path = getPathForFile(filename.ToStandardisedPath()); + if (path != null) + yield return path; + } + } + + private string getPathForFile(string filename) => + source.PerformRead(s => + { + if (s.IsManaged) + { + // avoid enumerating all files if this is a managed realm instance. + return s.Files.Filter(@"Filename ==[c] $0", filename).FirstOrDefault()?.File.GetStoragePath(); + } + + return s.Files.FirstOrDefault(f => string.Equals(f.Filename, filename, StringComparison.OrdinalIgnoreCase))?.File.GetStoragePath(); + }); + + public override IEnumerable GetAvailableResources() => source.PerformRead(s => s.Files.Select(f => f.Filename)); + } +} diff --git a/osu.Game/Skinning/LegacySkin.cs b/osu.Game/Skinning/LegacySkin.cs index 64f920de85..e677e2c01b 100644 --- a/osu.Game/Skinning/LegacySkin.cs +++ b/osu.Game/Skinning/LegacySkin.cs @@ -51,7 +51,7 @@ namespace osu.Game.Skinning [UsedImplicitly(ImplicitUseKindFlags.InstantiatedWithFixedConstructorSignature)] public LegacySkin(SkinInfo skin, IStorageResourceProvider resources) - : this(skin, new LegacySkinResourceStore(skin, resources.Files), resources, "skin.ini") + : this(skin, new LegacyDatabasedSkinResourceStore(skin, resources.Files), resources, "skin.ini") { } diff --git a/osu.Game/Skinning/LegacySkinResourceStore.cs b/osu.Game/Skinning/LegacySkinResourceStore.cs index 71efc949b8..2487a469c8 100644 --- a/osu.Game/Skinning/LegacySkinResourceStore.cs +++ b/osu.Game/Skinning/LegacySkinResourceStore.cs @@ -13,9 +13,9 @@ namespace osu.Game.Skinning { public class LegacySkinResourceStore : ResourceStore { - private readonly IHasRealmFiles source; + private readonly IHasNamedFiles source; - public LegacySkinResourceStore(IHasRealmFiles source, IResourceStore underlyingStore) + public LegacySkinResourceStore(IHasNamedFiles source, IResourceStore underlyingStore) : base(underlyingStore) { this.source = source; @@ -32,7 +32,7 @@ namespace osu.Game.Skinning } private string getPathForFile(string filename) => - source.Files.FirstOrDefault(f => string.Equals(f.Filename, filename, StringComparison.OrdinalIgnoreCase))?.FileInfo.GetStoragePath(); + source.Files.FirstOrDefault(f => string.Equals(f.Filename, filename, StringComparison.OrdinalIgnoreCase))?.File.GetStoragePath(); public override IEnumerable GetAvailableResources() => source.Files.Select(f => f.Filename); } From 29d074bdb8629ff480207498c0eeec61b010a530 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 29 Nov 2021 18:07:32 +0900 Subject: [PATCH 15/34] Implement missing behaviours required for skin file operations via `RealmArchiveModelManager` --- osu.Game/OsuGameBase.cs | 10 +- osu.Game/Skinning/SkinManager.cs | 163 ++++++++----------- osu.Game/Skinning/SkinModelManager.cs | 144 ++++++++++------ osu.Game/Stores/RealmArchiveModelManager.cs | 172 ++++++++++++++++++++ 4 files changed, 332 insertions(+), 157 deletions(-) create mode 100644 osu.Game/Stores/RealmArchiveModelManager.cs diff --git a/osu.Game/OsuGameBase.cs b/osu.Game/OsuGameBase.cs index 88c9ab370c..4bae6f3c1d 100644 --- a/osu.Game/OsuGameBase.cs +++ b/osu.Game/OsuGameBase.cs @@ -211,17 +211,9 @@ namespace osu.Game Audio.Samples.PlaybackConcurrency = SAMPLE_CONCURRENCY; - dependencies.Cache(SkinManager = new SkinManager(Storage, contextFactory, Host, Resources, Audio)); + dependencies.Cache(SkinManager = new SkinManager(Storage, realmFactory, Host, Resources, Audio, Scheduler)); dependencies.CacheAs(SkinManager); - // needs to be done here rather than inside SkinManager to ensure thread safety of CurrentSkinInfo. - SkinManager.ItemRemoved += item => Schedule(() => - { - // check the removed skin is not the current user choice. if it is, switch back to default. - if (item.Equals(SkinManager.CurrentSkinInfo.Value)) - SkinManager.CurrentSkinInfo.Value = SkinInfo.Default; - }); - EndpointConfiguration endpoints = UseDevelopmentServer ? (EndpointConfiguration)new DevelopmentEndpointConfiguration() : new ProductionEndpointConfiguration(); MessageFormatter.WebsiteRootUrl = endpoints.WebsiteRootUrl; diff --git a/osu.Game/Skinning/SkinManager.cs b/osu.Game/Skinning/SkinManager.cs index 24ec454276..2ec934ac32 100644 --- a/osu.Game/Skinning/SkinManager.cs +++ b/osu.Game/Skinning/SkinManager.cs @@ -3,14 +3,10 @@ using System; using System.Collections.Generic; -using System.IO; using System.Linq; using System.Linq.Expressions; -using System.Text; using System.Threading; using System.Threading.Tasks; -using Microsoft.EntityFrameworkCore; -using Newtonsoft.Json; using osu.Framework.Audio; using osu.Framework.Audio.Sample; using osu.Framework.Bindables; @@ -20,6 +16,7 @@ using osu.Framework.Graphics.Textures; using osu.Framework.IO.Stores; using osu.Framework.Platform; using osu.Framework.Testing; +using osu.Framework.Threading; using osu.Framework.Utils; using osu.Game.Audio; using osu.Game.Database; @@ -37,7 +34,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, IModelManager + public class SkinManager : ISkinSource, IStorageResourceProvider, IModelImporter { private readonly AudioManager audio; @@ -49,8 +46,7 @@ namespace osu.Game.Skinning public readonly Bindable> CurrentSkinInfo = new Bindable>(SkinInfo.Default.ToLive()) { Default = SkinInfo.Default.ToLive() }; private readonly SkinModelManager skinModelManager; - - private readonly SkinStore skinStore; + private readonly RealmContextFactory contextFactory; private readonly IResourceStore userFiles; @@ -64,69 +60,73 @@ namespace osu.Game.Skinning /// public Skin DefaultLegacySkin { get; } - public SkinManager(Storage storage, DatabaseContextFactory contextFactory, GameHost host, IResourceStore resources, AudioManager audio) + public SkinManager(Storage storage, RealmContextFactory contextFactory, GameHost host, IResourceStore resources, AudioManager audio, Scheduler scheduler) { + this.contextFactory = contextFactory; this.audio = audio; this.host = host; this.resources = resources; - skinStore = new SkinStore(contextFactory, storage); - userFiles = new FileStore(contextFactory, storage).Store; + userFiles = new StorageBackedResourceStore(storage.GetStorageForDirectory("files")); - skinModelManager = new SkinModelManager(storage, contextFactory, skinStore, host, this); + skinModelManager = new SkinModelManager(storage, contextFactory, host, this); DefaultLegacySkin = new DefaultLegacySkin(this); DefaultSkin = new DefaultSkin(this); - CurrentSkinInfo.ValueChanged += skin => CurrentSkin.Value = GetSkin(skin.NewValue); + CurrentSkinInfo.ValueChanged += skin => CurrentSkin.Value = skin.NewValue.PerformRead(GetSkin); CurrentSkin.Value = DefaultSkin; CurrentSkin.ValueChanged += skin => { - if (skin.NewValue.SkinInfo != CurrentSkinInfo.Value) + if (!skin.NewValue.SkinInfo.Equals(CurrentSkinInfo.Value)) throw new InvalidOperationException($"Setting {nameof(CurrentSkin)}'s value directly is not supported. Use {nameof(CurrentSkinInfo)} instead."); SourceChanged?.Invoke(); }; + + // needs to be done here rather than inside SkinManager to ensure thread safety of CurrentSkinInfo. + ItemRemoved += item => scheduler.Add(() => + { + // TODO: fix. + // check the removed skin is not the current user choice. if it is, switch back to default. + // if (item.Equals(CurrentSkinInfo.Value)) + // CurrentSkinInfo.Value = SkinInfo.Default; + }); } /// - /// Returns a list of all usable s. Includes the special default skin plus all skins from . + /// Returns a list of all usable s. Includes the non-databased default skins. /// /// A newly allocated list of available . - public List GetAllUsableSkins() + public List> GetAllUsableSkins() { - var userSkins = GetAllUserSkins(); - userSkins.Insert(0, DefaultSkin.SkinInfo); - userSkins.Insert(1, DefaultLegacySkin.SkinInfo); - return userSkins; - } - - /// - /// Returns a list of all usable s that have been loaded by the user. - /// - /// A newly allocated list of available . - public List GetAllUserSkins(bool includeFiles = false) - { - if (includeFiles) - return skinStore.ConsumableItems.Where(s => !s.DeletePending).ToList(); - - return skinStore.Items.Where(s => !s.DeletePending).ToList(); + using (var context = contextFactory.CreateContext()) + { + var userSkins = context.All().Where(s => !s.DeletePending).ToLive(); + userSkins.Insert(0, DefaultSkin.SkinInfo); + userSkins.Insert(1, DefaultLegacySkin.SkinInfo); + return userSkins; + } } public void SelectRandomSkin() { - // choose from only user skins, removing the current selection to ensure a new one is chosen. - var randomChoices = skinStore.Items.Where(s => !s.DeletePending && s.ID != CurrentSkinInfo.Value.ID).ToArray(); - - if (randomChoices.Length == 0) + using (var context = contextFactory.CreateContext()) { - CurrentSkinInfo.Value = SkinInfo.Default; - return; - } + // choose from only user skins, removing the current selection to ensure a new one is chosen. + var randomChoices = context.All().Where(s => !s.DeletePending && s.ID != CurrentSkinInfo.Value.ID).ToArray(); - var chosen = randomChoices.ElementAt(RNG.Next(0, randomChoices.Length)); - CurrentSkinInfo.Value = skinStore.ConsumableItems.Single(i => i.ID == chosen.ID); + if (randomChoices.Length == 0) + { + CurrentSkinInfo.Value = SkinInfo.Default.ToLive(); + return; + } + + var chosen = randomChoices.ElementAt(RNG.Next(0, randomChoices.Length)); + + CurrentSkinInfo.Value = chosen.ToLive(); + } } /// @@ -142,40 +142,30 @@ namespace osu.Game.Skinning /// public void EnsureMutableSkin() { - if (CurrentSkinInfo.Value.ID >= 1) return; - - var skin = CurrentSkin.Value; - - // if the user is attempting to save one of the default skin implementations, create a copy first. - CurrentSkinInfo.Value = skinModelManager.Import(new SkinInfo + CurrentSkinInfo.Value.PerformRead(s => { - Name = skin.SkinInfo.Name + @" (modified)", - Creator = skin.SkinInfo.Creator, - InstantiationInfo = skin.SkinInfo.InstantiationInfo, - }).Result.Value; + if (s.IsManaged) + return; + + // if the user is attempting to save one of the default skin implementations, create a copy first. + var result = skinModelManager.Import(new SkinInfo + { + Name = s.Name + @" (modified)", + Creator = s.Creator, + InstantiationInfo = s.InstantiationInfo, + }).Result; + + if (result != null) + CurrentSkinInfo.Value = result; + }); } public void Save(Skin skin) { - if (skin.SkinInfo.ID <= 0) + if (!skin.SkinInfo.IsManaged) throw new InvalidOperationException($"Attempting to save a skin which is not yet tracked. Call {nameof(EnsureMutableSkin)} first."); - foreach (var drawableInfo in skin.DrawableComponentInfo) - { - string json = JsonConvert.SerializeObject(drawableInfo.Value, new JsonSerializerSettings { Formatting = Formatting.Indented }); - - using (var streamContent = new MemoryStream(Encoding.UTF8.GetBytes(json))) - { - string filename = @$"{drawableInfo.Key}.json"; - - var oldFile = skin.SkinInfo.Files.FirstOrDefault(f => f.Filename == filename); - - if (oldFile != null) - skinModelManager.ReplaceFile(skin.SkinInfo, oldFile, streamContent); - else - skinModelManager.AddFile(skin.SkinInfo, streamContent, filename); - } - } + skinModelManager.Save(skin); } /// @@ -183,7 +173,11 @@ namespace osu.Game.Skinning /// /// The query. /// The first result for the provided query, or null if no results were found. - public SkinInfo Query(Expression> query) => skinStore.ConsumableItems.AsNoTracking().FirstOrDefault(query); + public ILive Query(Expression> query) + { + using (var context = contextFactory.CreateContext()) + return context.All().FirstOrDefault(query)?.ToLive(); + } public event Action SourceChanged; @@ -301,34 +295,13 @@ namespace osu.Game.Skinning remove => skinModelManager.ItemRemoved -= value; } - public void Update(SkinInfo item) + public void Delete(Expression> filter, bool silent = false) { - skinModelManager.Update(item); - } - - 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); + using (var context = contextFactory.CreateContext()) + { + var items = context.All().Where(filter).ToList(); + skinModelManager.Delete(items, silent); + } } #endregion diff --git a/osu.Game/Skinning/SkinModelManager.cs b/osu.Game/Skinning/SkinModelManager.cs index 572ae5cbfc..059345f9bc 100644 --- a/osu.Game/Skinning/SkinModelManager.cs +++ b/osu.Game/Skinning/SkinModelManager.cs @@ -8,21 +8,26 @@ using System.Linq; using System.Text; using System.Threading; using System.Threading.Tasks; +using Newtonsoft.Json; using osu.Framework.Logging; using osu.Framework.Platform; using osu.Game.Database; using osu.Game.Extensions; using osu.Game.IO; using osu.Game.IO.Archives; +using osu.Game.Stores; +using Realms; + +#nullable enable namespace osu.Game.Skinning { - public class SkinModelManager : ArchiveModelManager + public class SkinModelManager : RealmArchiveModelManager { private readonly IStorageResourceProvider skinResources; - public SkinModelManager(Storage storage, DatabaseContextFactory contextFactory, SkinStore skinStore, GameHost host, IStorageResourceProvider skinResources) - : base(storage, contextFactory, skinStore, host) + public SkinModelManager(Storage storage, RealmContextFactory contextFactory, GameHost host, IStorageResourceProvider skinResources) + : base(storage, contextFactory) { this.skinResources = skinResources; @@ -42,18 +47,27 @@ namespace osu.Game.Skinning protected override bool HasCustomHashFunction => true; - protected override string ComputeHash(SkinInfo item) + protected override Task Populate(SkinInfo model, ArchiveReader? archive, Realm realm, CancellationToken cancellationToken = default) + { + if (string.IsNullOrEmpty(model.InstantiationInfo)) + model.InstantiationInfo = createInstance(model).GetType().GetInvariantInstantiationInfo(); + + checkSkinIniMetadata(model, realm); + + return Task.CompletedTask; + } + + private void checkSkinIniMetadata(SkinInfo item, Realm realm) { var instance = createInstance(item); // This function can be run on fresh import or save. The logic here ensures a skin.ini file is in a good state for both operations. - // `Skin` will parse the skin.ini and populate `Skin.Configuration` during construction above. string skinIniSourcedName = instance.Configuration.SkinInfo.Name; string skinIniSourcedCreator = instance.Configuration.SkinInfo.Creator; string archiveName = item.Name.Replace(@".osk", string.Empty, StringComparison.OrdinalIgnoreCase); - bool isImport = item.ID == 0; + bool isImport = !item.IsManaged; if (isImport) { @@ -71,12 +85,10 @@ namespace osu.Game.Skinning // Regardless of whether this is an import or not, let's write the skin.ini if non-existing or non-matching. // This is (weirdly) done inside ComputeHash to avoid adding a new method to handle this case. After switching to realm it can be moved into another place. if (skinIniSourcedName != item.Name) - updateSkinIniMetadata(item); - - return base.ComputeHash(item); + updateSkinIniMetadata(item, realm); } - private void updateSkinIniMetadata(SkinInfo item) + private void updateSkinIniMetadata(SkinInfo item, Realm realm) { string nameLine = @$"Name: {item.Name}"; string authorLine = @$"Author: {item.Creator}"; @@ -95,39 +107,47 @@ namespace osu.Game.Skinning { // In the case a skin doesn't have a skin.ini yet, let's create one. writeNewSkinIni(); - return; } - - using (Stream stream = new MemoryStream()) + else { - using (var sw = new StreamWriter(stream, Encoding.UTF8, 1024, true)) + using (Stream stream = new MemoryStream()) { - using (var existingStream = Files.Storage.GetStream(existingFile.FileInfo.GetStoragePath())) - using (var sr = new StreamReader(existingStream)) + using (var sw = new StreamWriter(stream, Encoding.UTF8, 1024, true)) { - string line; - while ((line = sr.ReadLine()) != null) + using (var existingStream = Files.Storage.GetStream(existingFile.File.GetStoragePath())) + using (var sr = new StreamReader(existingStream)) + { + string? line; + while ((line = sr.ReadLine()) != null) + sw.WriteLine(line); + } + + sw.WriteLine(); + + foreach (string line in newLines) sw.WriteLine(line); } - sw.WriteLine(); + ReplaceFile(item, existingFile, stream, realm); - foreach (string line in newLines) - sw.WriteLine(line); - } + // can be removed 20220502. + if (!ensureIniWasUpdated(item)) + { + Logger.Log($"Skin {item}'s skin.ini had issues and has been removed. Please report this and provide the problematic skin.", LoggingTarget.Database, LogLevel.Important); - ReplaceFile(item, existingFile, stream); + var existingIni = item.Files.SingleOrDefault(f => f.Filename.Equals(@"skin.ini", StringComparison.OrdinalIgnoreCase)); + if (existingIni != null) + item.Files.Remove(existingIni); - // can be removed 20220502. - if (!ensureIniWasUpdated(item)) - { - Logger.Log($"Skin {item}'s skin.ini had issues and has been removed. Please report this and provide the problematic skin.", LoggingTarget.Database, LogLevel.Important); - - DeleteFile(item, item.Files.SingleOrDefault(f => f.Filename.Equals(@"skin.ini", StringComparison.OrdinalIgnoreCase))); - writeNewSkinIni(); + writeNewSkinIni(); + } } } + // The hash is already populated at this point in import. + // As we have changed files, it needs to be recomputed. + item.Hash = ComputeHash(item); + void writeNewSkinIni() { using (Stream stream = new MemoryStream()) @@ -138,8 +158,10 @@ namespace osu.Game.Skinning sw.WriteLine(line); } - AddFile(item, stream, @"skin.ini"); + AddFile(item, stream, @"skin.ini", realm); } + + item.Hash = ComputeHash(item); } } @@ -154,36 +176,52 @@ namespace osu.Game.Skinning return instance.Configuration.SkinInfo.Name == item.Name; } - protected override Task Populate(SkinInfo model, ArchiveReader archive, CancellationToken cancellationToken = default) - { - var instance = createInstance(model); - - model.InstantiationInfo ??= instance.GetType().GetInvariantInstantiationInfo(); - - model.Name = instance.Configuration.SkinInfo.Name; - model.Creator = instance.Configuration.SkinInfo.Creator; - - return Task.CompletedTask; - } - private void populateMissingHashes() { - var skinsWithoutHashes = ModelStore.ConsumableItems.Where(i => i.Hash == null).ToArray(); - - foreach (SkinInfo skin in skinsWithoutHashes) + using (var realm = ContextFactory.CreateContext()) { - try + var skinsWithoutHashes = realm.All().Where(i => string.IsNullOrEmpty(i.Hash)).ToArray(); + + foreach (SkinInfo skin in skinsWithoutHashes) { - Update(skin); - } - catch (Exception e) - { - Delete(skin); - Logger.Error(e, $"Existing skin {skin} has been deleted during hash recomputation due to being invalid"); + try + { + Update(skin); + } + catch (Exception e) + { + Delete(skin); + Logger.Error(e, $"Existing skin {skin} has been deleted during hash recomputation due to being invalid"); + } } } } private Skin createInstance(SkinInfo item) => item.CreateInstance(skinResources); + + public void Save(Skin skin) + { + skin.SkinInfo.PerformWrite(s => + { + foreach (var drawableInfo in skin.DrawableComponentInfo) + { + string json = JsonConvert.SerializeObject(drawableInfo.Value, new JsonSerializerSettings { Formatting = Formatting.Indented }); + + using (var streamContent = new MemoryStream(Encoding.UTF8.GetBytes(json))) + { + string filename = @$"{drawableInfo.Key}.json"; + + var oldFile = s.Files.FirstOrDefault(f => f.Filename == filename); + + if (oldFile != null) + ReplaceFile(s, oldFile, streamContent, s.Realm); + else + AddFile(s, streamContent, filename, s.Realm); + } + } + + s.Hash = ComputeHash(s); + }); + } } } diff --git a/osu.Game/Stores/RealmArchiveModelManager.cs b/osu.Game/Stores/RealmArchiveModelManager.cs new file mode 100644 index 0000000000..14b7077dd2 --- /dev/null +++ b/osu.Game/Stores/RealmArchiveModelManager.cs @@ -0,0 +1,172 @@ +// 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 System.Threading; +using System.Threading.Tasks; +using osu.Framework.Platform; +using osu.Game.Database; +using osu.Game.IO.Archives; +using osu.Game.Models; +using osu.Game.Overlays.Notifications; +using Realms; + +#nullable enable + +namespace osu.Game.Stores +{ + /// + /// Class which adds all the missing pieces bridging the gap between and . + /// + public abstract class RealmArchiveModelManager : RealmArchiveModelImporter, IModelManager, IModelFileManager + where TModel : RealmObject, IHasRealmFiles, IHasGuidPrimaryKey, ISoftDelete + { + public event Action? ItemUpdated; + public event Action? ItemRemoved; + + private readonly RealmFileStore realmFileStore; + + protected RealmArchiveModelManager(Storage storage, RealmContextFactory contextFactory) + : base(storage, contextFactory) + { + realmFileStore = new RealmFileStore(contextFactory, storage); + } + + public void DeleteFile(TModel item, RealmNamedFileUsage file) => + item.Realm.Write(() => DeleteFile(item, file, item.Realm)); + + public void ReplaceFile(TModel item, RealmNamedFileUsage file, Stream contents) + => item.Realm.Write(() => ReplaceFile(item, file, contents, item.Realm)); + + public void AddFile(TModel item, Stream stream, string filename) + => item.Realm.Write(() => AddFile(item, stream, filename, item.Realm)); + + public void DeleteFile(TModel item, RealmNamedFileUsage file, Realm realm) + { + item.Files.Remove(file); + } + + public void ReplaceFile(TModel model, RealmNamedFileUsage file, Stream contents, Realm realm) + { + file.File = realmFileStore.Add(contents, realm); + } + + public void AddFile(TModel item, Stream stream, string filename, Realm realm) + { + var file = realmFileStore.Add(stream, realm); + var namedUsage = new RealmNamedFileUsage(file, filename); + + item.Files.Add(namedUsage); + } + + public override async Task?> Import(TModel item, ArchiveReader? archive = null, bool lowPriority = false, CancellationToken cancellationToken = default) + { + var imported = await base.Import(item, archive, lowPriority, cancellationToken).ConfigureAwait(false); + + imported?.PerformRead(i => ItemUpdated?.Invoke(i.Detach())); + + return imported; + } + + /// + /// Delete multiple items. + /// This will post notifications tracking progress. + /// + public void Delete(List items, bool silent = false) + { + if (items.Count == 0) return; + + var notification = new ProgressNotification + { + Progress = 0, + Text = $"Preparing to delete all {HumanisedModelName}s...", + CompletionText = $"Deleted all {HumanisedModelName}s!", + State = ProgressNotificationState.Active, + }; + + if (!silent) + PostNotification?.Invoke(notification); + + int i = 0; + + foreach (var b in items) + { + if (notification.State == ProgressNotificationState.Cancelled) + // user requested abort + return; + + notification.Text = $"Deleting {HumanisedModelName}s ({++i} of {items.Count})"; + + Delete(b); + + notification.Progress = (float)i / items.Count; + } + + notification.State = ProgressNotificationState.Completed; + } + + /// + /// Restore multiple items that were previously deleted. + /// This will post notifications tracking progress. + /// + public void Undelete(List items, bool silent = false) + { + if (!items.Any()) return; + + var notification = new ProgressNotification + { + CompletionText = "Restored all deleted items!", + Progress = 0, + State = ProgressNotificationState.Active, + }; + + if (!silent) + PostNotification?.Invoke(notification); + + int i = 0; + + foreach (var item in items) + { + if (notification.State == ProgressNotificationState.Cancelled) + // user requested abort + return; + + notification.Text = $"Restoring ({++i} of {items.Count})"; + + Undelete(item); + + notification.Progress = (float)i / items.Count; + } + + notification.State = ProgressNotificationState.Completed; + } + + public bool Delete(TModel item) + { + if (item.DeletePending) + return false; + + item.Realm.Write(r => item.DeletePending = true); + ItemRemoved?.Invoke(item.Detach()); + return true; + } + + public void Undelete(TModel item) + { + if (!item.DeletePending) + return; + + item.Realm.Write(r => item.DeletePending = false); + ItemUpdated?.Invoke(item); + } + + public virtual bool IsAvailableLocally(TModel model) => false; // TODO: implement + + public void Update(TModel skin) + { + } + } +} From c629a7a36fe1acdd9e1143fb9c6692b5152c38a3 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 29 Nov 2021 16:18:34 +0900 Subject: [PATCH 16/34] Fix random selection and avoid using legacy events for handling skin import/deletion --- .../Overlays/Settings/Sections/SkinSection.cs | 95 +++++++++---------- osu.Game/Skinning/SkinManager.cs | 33 +++---- osu.Game/Stores/RealmArchiveModelManager.cs | 27 ++++-- 3 files changed, 73 insertions(+), 82 deletions(-) diff --git a/osu.Game/Overlays/Settings/Sections/SkinSection.cs b/osu.Game/Overlays/Settings/Sections/SkinSection.cs index 9c5a2dff47..88f60a6004 100644 --- a/osu.Game/Overlays/Settings/Sections/SkinSection.cs +++ b/osu.Game/Overlays/Settings/Sections/SkinSection.cs @@ -18,6 +18,7 @@ using osu.Game.Graphics.UserInterface; using osu.Game.Localisation; using osu.Game.Skinning; using osu.Game.Skinning.Editor; +using Realms; namespace osu.Game.Overlays.Settings.Sections { @@ -43,21 +44,15 @@ namespace osu.Game.Overlays.Settings.Sections private List> skinItems; - private int firstNonDefaultSkinIndex - { - get - { - int index = skinItems.FindIndex(s => s.ID == SkinInfo.CLASSIC_SKIN); - if (index < 0) - index = skinItems.Count; - - return index; - } - } - [Resolved] private SkinManager skins { get; set; } + [Resolved] + private RealmContextFactory realmFactory { get; set; } + + private IDisposable realmSubscription; + private IQueryable realmSkins; + [BackgroundDependencyLoader(permitNulls: true)] private void load(OsuConfigManager config, [CanBeNull] SkinEditorOverlay skinEditor) { @@ -75,9 +70,6 @@ namespace osu.Game.Overlays.Settings.Sections new ExportSkinButton(), }; - skins.ItemUpdated += itemUpdated; - skins.ItemRemoved += itemRemoved; - config.BindWith(OsuSetting.Skin, configBindable); } @@ -86,6 +78,21 @@ namespace osu.Game.Overlays.Settings.Sections base.LoadComplete(); skinDropdown.Current = dropdownBindable; + + realmSkins = realmFactory.Context.All() + .Where(s => !s.DeletePending) + .OrderBy(s => s.Name, StringComparer.OrdinalIgnoreCase); + + realmSubscription = realmSkins + .SubscribeForNotifications((sender, changes, error) => + { + if (changes == null) + return; + + // Eventually this should be handling the individual changes rather than refreshing the whole dropdown. + updateItems(); + }); + updateItems(); // Todo: This should not be necessary when OsuConfigManager is databased @@ -97,7 +104,16 @@ namespace osu.Game.Overlays.Settings.Sections { if (skin.NewValue.Equals(random_skin_info)) { + var skinBefore = skins.CurrentSkinInfo.Value; + skins.SelectRandomSkin(); + + if (skinBefore == skins.CurrentSkinInfo.Value) + { + // the random selection didn't change the skin, so we should manually update the dropdown to match. + dropdownBindable.Value = skins.CurrentSkinInfo.Value; + } + return; } @@ -111,12 +127,13 @@ namespace osu.Game.Overlays.Settings.Sections var skin = skinDropdown.Items.FirstOrDefault(s => s.ID == configId); + // TODO: i don't think this will be required any more. if (skin == null) { // there may be a thread race condition where an item is selected that hasn't yet been added to the dropdown. // to avoid adding complexity, let's just ensure the item is added so we can perform the selection. skin = skins.Query(s => s.ID == configId); - addItem(skin); + updateItems(); } dropdownBindable.Value = skin; @@ -124,47 +141,20 @@ namespace osu.Game.Overlays.Settings.Sections private void updateItems() { - skinItems = skins.GetAllUsableSkins(); - skinItems.Insert(firstNonDefaultSkinIndex, random_skin_info); - sortUserSkins(skinItems); + skinItems = realmSkins.ToLive(); + + skinItems.Insert(0, SkinInfo.Default.ToLive()); + skinItems.Insert(1, DefaultLegacySkin.Info.ToLive()); + skinItems.Insert(2, random_skin_info); + skinDropdown.Items = skinItems; } - private void itemUpdated(SkinInfo item) => Schedule(() => addItem(item.ToLive())); - - private void addItem(ILive item) - { - List> newDropdownItems = skinDropdown.Items.Where(i => !i.Equals(item)).Append(item).ToList(); - sortUserSkins(newDropdownItems); - skinDropdown.Items = newDropdownItems; - } - - private void itemRemoved(SkinInfo item) => Schedule(() => skinDropdown.Items = skinDropdown.Items.Where(i => !i.ID.Equals(item.ID)).ToArray()); - - private void sortUserSkins(List> skinsList) - { - try - { - // Sort user skins separately from built-in skins - skinsList.Sort(firstNonDefaultSkinIndex, skinsList.Count - firstNonDefaultSkinIndex, - Comparer>.Create((a, b) => - { - // o_________________________o - return a.PerformRead(ai => b.PerformRead(bi => string.Compare(ai.Name, bi.Name, StringComparison.OrdinalIgnoreCase))); - })); - } - catch { } - } - protected override void Dispose(bool isDisposing) { base.Dispose(isDisposing); - if (skins != null) - { - skins.ItemUpdated -= itemUpdated; - skins.ItemRemoved -= itemRemoved; - } + realmSubscription?.Dispose(); } private class SkinSettingsDropdown : SettingsDropdown> @@ -192,6 +182,11 @@ namespace osu.Game.Overlays.Settings.Sections { Text = SkinSettingsStrings.ExportSkinButton; Action = export; + } + + protected override void LoadComplete() + { + base.LoadComplete(); currentSkin = skins.CurrentSkin.GetBoundCopy(); currentSkin.BindValueChanged(skin => Enabled.Value = skin.NewValue.SkinInfo.PerformRead(s => s.IsManaged), true); diff --git a/osu.Game/Skinning/SkinManager.cs b/osu.Game/Skinning/SkinManager.cs index 2ec934ac32..bcad5277ab 100644 --- a/osu.Game/Skinning/SkinManager.cs +++ b/osu.Game/Skinning/SkinManager.cs @@ -38,6 +38,8 @@ namespace osu.Game.Skinning { private readonly AudioManager audio; + private readonly Scheduler scheduler; + private readonly GameHost host; private readonly IResourceStore resources; @@ -64,6 +66,7 @@ namespace osu.Game.Skinning { this.contextFactory = contextFactory; this.audio = audio; + this.scheduler = scheduler; this.host = host; this.resources = resources; @@ -84,15 +87,6 @@ namespace osu.Game.Skinning SourceChanged?.Invoke(); }; - - // needs to be done here rather than inside SkinManager to ensure thread safety of CurrentSkinInfo. - ItemRemoved += item => scheduler.Add(() => - { - // TODO: fix. - // check the removed skin is not the current user choice. if it is, switch back to default. - // if (item.Equals(CurrentSkinInfo.Value)) - // CurrentSkinInfo.Value = SkinInfo.Default; - }); } /// @@ -283,23 +277,18 @@ namespace osu.Game.Skinning #region Implementation of IModelManager - public event Action ItemUpdated - { - add => skinModelManager.ItemUpdated += value; - remove => skinModelManager.ItemUpdated -= value; - } - - public event Action ItemRemoved - { - add => skinModelManager.ItemRemoved += value; - remove => skinModelManager.ItemRemoved -= value; - } - - public void Delete(Expression> filter, bool silent = false) + public void Delete([CanBeNull] Expression> filter = null, bool silent = false) { using (var context = contextFactory.CreateContext()) { var items = context.All().Where(filter).ToList(); + + // check the removed skin is not the current user choice. if it is, switch back to default. + Guid currentUserSkin = CurrentSkinInfo.Value.ID; + + if (items.Any(s => s.ID == currentUserSkin)) + scheduler.Add(() => CurrentSkinInfo.Value = SkinInfo.Default.ToLive()); + skinModelManager.Delete(items, silent); } } diff --git a/osu.Game/Stores/RealmArchiveModelManager.cs b/osu.Game/Stores/RealmArchiveModelManager.cs index 14b7077dd2..ae40bcac79 100644 --- a/osu.Game/Stores/RealmArchiveModelManager.cs +++ b/osu.Game/Stores/RealmArchiveModelManager.cs @@ -24,8 +24,21 @@ namespace osu.Game.Stores public abstract class RealmArchiveModelManager : RealmArchiveModelImporter, IModelManager, IModelFileManager where TModel : RealmObject, IHasRealmFiles, IHasGuidPrimaryKey, ISoftDelete { - public event Action? ItemUpdated; - public event Action? ItemRemoved; + public event Action? ItemUpdated + { + // This may be brought back for beatmaps to ease integration. + // The eventual goal would be not requiring this and using realm subscriptions in its place. + add => throw new NotImplementedException(); + remove => throw new NotImplementedException(); + } + + public event Action? ItemRemoved + { + // This may be brought back for beatmaps to ease integration. + // The eventual goal would be not requiring this and using realm subscriptions in its place. + add => throw new NotImplementedException(); + remove => throw new NotImplementedException(); + } private readonly RealmFileStore realmFileStore; @@ -64,11 +77,7 @@ namespace osu.Game.Stores public override async Task?> Import(TModel item, ArchiveReader? archive = null, bool lowPriority = false, CancellationToken cancellationToken = default) { - var imported = await base.Import(item, archive, lowPriority, cancellationToken).ConfigureAwait(false); - - imported?.PerformRead(i => ItemUpdated?.Invoke(i.Detach())); - - return imported; + return await base.Import(item, archive, lowPriority, cancellationToken).ConfigureAwait(false); } /// @@ -150,7 +159,6 @@ namespace osu.Game.Stores return false; item.Realm.Write(r => item.DeletePending = true); - ItemRemoved?.Invoke(item.Detach()); return true; } @@ -160,10 +168,9 @@ namespace osu.Game.Stores return; item.Realm.Write(r => item.DeletePending = false); - ItemUpdated?.Invoke(item); } - public virtual bool IsAvailableLocally(TModel model) => false; // TODO: implement + public virtual bool IsAvailableLocally(TModel model) => false; // Not relevant for skins since they can't be downloaded yet. public void Update(TModel skin) { From f6a3709060b3ba4d81ce4f9d01df52aba7fa18dd Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 29 Nov 2021 17:15:26 +0900 Subject: [PATCH 17/34] Store default skins to database --- .../TestSceneHyperDashColouring.cs | 2 +- .../Editor/TestSceneManiaComposeScreen.cs | 4 +- .../TestSceneSliderApplication.cs | 2 +- .../Gameplay/TestSceneStoryboardSamples.cs | 2 +- .../Gameplay/TestSceneBeatmapSkinFallbacks.cs | 2 +- osu.Game/OsuGame.cs | 6 +-- .../Overlays/Settings/Sections/SkinSection.cs | 11 ++--- osu.Game/Skinning/DefaultLegacySkin.cs | 19 ++++---- osu.Game/Skinning/DefaultSkin.cs | 11 ++++- osu.Game/Skinning/SkinInfo.cs | 11 +---- osu.Game/Skinning/SkinManager.cs | 43 +++++++++++-------- 11 files changed, 61 insertions(+), 52 deletions(-) diff --git a/osu.Game.Rulesets.Catch.Tests/TestSceneHyperDashColouring.cs b/osu.Game.Rulesets.Catch.Tests/TestSceneHyperDashColouring.cs index 70b2c8c82a..14a4d02396 100644 --- a/osu.Game.Rulesets.Catch.Tests/TestSceneHyperDashColouring.cs +++ b/osu.Game.Rulesets.Catch.Tests/TestSceneHyperDashColouring.cs @@ -174,7 +174,7 @@ namespace osu.Game.Rulesets.Catch.Tests private Drawable setupSkinHierarchy(Drawable child, ISkin skin) { - var legacySkinProvider = new SkinProvidingContainer(skins.GetSkin(DefaultLegacySkin.Info)); + var legacySkinProvider = new SkinProvidingContainer(skins.GetSkin(DefaultLegacySkin.CreateInfo())); var testSkinProvider = new SkinProvidingContainer(skin); var legacySkinTransformer = new SkinProvidingContainer(new CatchLegacySkinTransformer(testSkinProvider)); diff --git a/osu.Game.Rulesets.Mania.Tests/Editor/TestSceneManiaComposeScreen.cs b/osu.Game.Rulesets.Mania.Tests/Editor/TestSceneManiaComposeScreen.cs index a7e76e44b5..91f5f93905 100644 --- a/osu.Game.Rulesets.Mania.Tests/Editor/TestSceneManiaComposeScreen.cs +++ b/osu.Game.Rulesets.Mania.Tests/Editor/TestSceneManiaComposeScreen.cs @@ -56,13 +56,13 @@ namespace osu.Game.Rulesets.Mania.Tests.Editor [Test] public void TestDefaultSkin() { - AddStep("set default skin", () => skins.CurrentSkinInfo.Value = SkinInfo.Default.ToLive()); + AddStep("set default skin", () => skins.CurrentSkinInfo.Value = DefaultSkin.CreateInfo().ToLive()); } [Test] public void TestLegacySkin() { - AddStep("set legacy skin", () => skins.CurrentSkinInfo.Value = DefaultLegacySkin.Info.ToLive()); + AddStep("set legacy skin", () => skins.CurrentSkinInfo.Value = DefaultLegacySkin.CreateInfo().ToLive()); } } } diff --git a/osu.Game.Rulesets.Osu.Tests/TestSceneSliderApplication.cs b/osu.Game.Rulesets.Osu.Tests/TestSceneSliderApplication.cs index e698766aac..d673b7a6ac 100644 --- a/osu.Game.Rulesets.Osu.Tests/TestSceneSliderApplication.cs +++ b/osu.Game.Rulesets.Osu.Tests/TestSceneSliderApplication.cs @@ -67,7 +67,7 @@ namespace osu.Game.Rulesets.Osu.Tests AddStep("create slider", () => { - var tintingSkin = skinManager.GetSkin(DefaultLegacySkin.Info); + var tintingSkin = skinManager.GetSkin(DefaultLegacySkin.CreateInfo()); tintingSkin.Configuration.ConfigDictionary["AllowSliderBallTint"] = "1"; Child = new SkinProvidingContainer(tintingSkin) diff --git a/osu.Game.Tests/Gameplay/TestSceneStoryboardSamples.cs b/osu.Game.Tests/Gameplay/TestSceneStoryboardSamples.cs index 3bf6aaac7a..88f35976ad 100644 --- a/osu.Game.Tests/Gameplay/TestSceneStoryboardSamples.cs +++ b/osu.Game.Tests/Gameplay/TestSceneStoryboardSamples.cs @@ -167,7 +167,7 @@ namespace osu.Game.Tests.Gameplay private class TestSkin : LegacySkin { public TestSkin(string resourceName, IStorageResourceProvider resources) - : base(DefaultLegacySkin.Info, new TestResourceStore(resourceName), resources, "skin.ini") + : base(DefaultLegacySkin.CreateInfo(), new TestResourceStore(resourceName), resources, "skin.ini") { } } diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneBeatmapSkinFallbacks.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneBeatmapSkinFallbacks.cs index c174a3edc2..cccc962a3f 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneBeatmapSkinFallbacks.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneBeatmapSkinFallbacks.cs @@ -42,7 +42,7 @@ namespace osu.Game.Tests.Visual.Gameplay [Test] public void TestEmptyLegacyBeatmapSkinFallsBack() { - CreateSkinTest(SkinInfo.Default, () => new LegacyBeatmapSkin(new BeatmapInfo(), null, null)); + CreateSkinTest(DefaultSkin.CreateInfo(), () => new LegacyBeatmapSkin(new BeatmapInfo(), null, null)); AddUntilStep("wait for hud load", () => Player.ChildrenOfType().All(c => c.ComponentsLoaded)); AddAssert("hud from default skin", () => AssertComponentsFromExpectedSource(SkinnableTarget.MainHUDComponents, skinManager.CurrentSkin.Value)); } diff --git a/osu.Game/OsuGame.cs b/osu.Game/OsuGame.cs index 1ffbb4cb85..8af76c1289 100644 --- a/osu.Game/OsuGame.cs +++ b/osu.Game/OsuGame.cs @@ -256,10 +256,10 @@ namespace osu.Game if (skinInfo == null) { if (guid == SkinInfo.CLASSIC_SKIN) - skinInfo = DefaultLegacySkin.Info.ToLive(); + skinInfo = DefaultLegacySkin.CreateInfo().ToLive(); } - SkinManager.CurrentSkinInfo.Value = skinInfo ?? SkinInfo.Default.ToLive(); + SkinManager.CurrentSkinInfo.Value = skinInfo ?? DefaultSkin.CreateInfo().ToLive(); }; configSkin.TriggerChange(); @@ -660,7 +660,7 @@ namespace osu.Game // make config aware of how to lookup skins for on-screen display purposes. // if this becomes a more common thing, tracked settings should be reconsidered to allow local DI. - LocalConfig.LookupSkinName = id => SkinManager.GetAllUsableSkins().FirstOrDefault(s => s.ID == id)?.ToString() ?? "Unknown"; + LocalConfig.LookupSkinName = id => SkinManager.Query(s => s.ID == id)?.ToString() ?? "Unknown"; LocalConfig.LookupKeyBindings = l => { diff --git a/osu.Game/Overlays/Settings/Sections/SkinSection.cs b/osu.Game/Overlays/Settings/Sections/SkinSection.cs index 88f60a6004..bf0f6bf142 100644 --- a/osu.Game/Overlays/Settings/Sections/SkinSection.cs +++ b/osu.Game/Overlays/Settings/Sections/SkinSection.cs @@ -33,7 +33,7 @@ namespace osu.Game.Overlays.Settings.Sections Icon = FontAwesome.Solid.PaintBrush }; - private readonly Bindable> dropdownBindable = new Bindable> { Default = SkinInfo.Default.ToLive() }; + private readonly Bindable> dropdownBindable = new Bindable> { Default = DefaultSkin.CreateInfo().ToLive() }; private readonly Bindable configBindable = new Bindable(); private static readonly ILive random_skin_info = new SkinInfo @@ -81,7 +81,8 @@ namespace osu.Game.Overlays.Settings.Sections realmSkins = realmFactory.Context.All() .Where(s => !s.DeletePending) - .OrderBy(s => s.Name, StringComparer.OrdinalIgnoreCase); + .OrderBy(s => s.Protected) + .ThenBy(s => s.Name, StringComparer.OrdinalIgnoreCase); realmSubscription = realmSkins .SubscribeForNotifications((sender, changes, error) => @@ -141,11 +142,11 @@ namespace osu.Game.Overlays.Settings.Sections private void updateItems() { + int protectedCount = realmSkins.Count(s => s.Protected); + skinItems = realmSkins.ToLive(); - skinItems.Insert(0, SkinInfo.Default.ToLive()); - skinItems.Insert(1, DefaultLegacySkin.Info.ToLive()); - skinItems.Insert(2, random_skin_info); + skinItems.Insert(protectedCount, random_skin_info); skinDropdown.Items = skinItems; } diff --git a/osu.Game/Skinning/DefaultLegacySkin.cs b/osu.Game/Skinning/DefaultLegacySkin.cs index 2332e6e160..c7033d37dc 100644 --- a/osu.Game/Skinning/DefaultLegacySkin.cs +++ b/osu.Game/Skinning/DefaultLegacySkin.cs @@ -12,8 +12,17 @@ namespace osu.Game.Skinning { public class DefaultLegacySkin : LegacySkin { + public static SkinInfo CreateInfo() => new SkinInfo + { + ID = Skinning.SkinInfo.CLASSIC_SKIN, // this is temporary until database storage is decided upon. + Name = "osu!classic", + Creator = "team osu!", + Protected = true, + InstantiationInfo = typeof(DefaultLegacySkin).GetInvariantInstantiationInfo() + }; + public DefaultLegacySkin(IStorageResourceProvider resources) - : this(Info, resources) + : this(CreateInfo(), resources) { } @@ -39,13 +48,5 @@ namespace osu.Game.Skinning Configuration.LegacyVersion = 2.7m; } - - public static SkinInfo Info { get; } = new SkinInfo - { - ID = osu.Game.Skinning.SkinInfo.CLASSIC_SKIN, // this is temporary until database storage is decided upon. - Name = "osu!classic", - Creator = "team osu!", - InstantiationInfo = typeof(DefaultLegacySkin).GetInvariantInstantiationInfo() - }; } } diff --git a/osu.Game/Skinning/DefaultSkin.cs b/osu.Game/Skinning/DefaultSkin.cs index 8d3a83f589..951e3f9cc5 100644 --- a/osu.Game/Skinning/DefaultSkin.cs +++ b/osu.Game/Skinning/DefaultSkin.cs @@ -23,10 +23,19 @@ namespace osu.Game.Skinning { public class DefaultSkin : Skin { + public static SkinInfo CreateInfo() => new SkinInfo + { + ID = osu.Game.Skinning.SkinInfo.DEFAULT_SKIN, + Name = "osu! (triangles)", + Creator = "team osu!", + Protected = true, + InstantiationInfo = typeof(DefaultSkin).GetInvariantInstantiationInfo() + }; + private readonly IStorageResourceProvider resources; public DefaultSkin(IStorageResourceProvider resources) - : this(osu.Game.Skinning.SkinInfo.Default, resources) + : this(CreateInfo(), resources) { } diff --git a/osu.Game/Skinning/SkinInfo.cs b/osu.Game/Skinning/SkinInfo.cs index c09d45c227..9a82964933 100644 --- a/osu.Game/Skinning/SkinInfo.cs +++ b/osu.Game/Skinning/SkinInfo.cs @@ -6,7 +6,6 @@ using System.Collections.Generic; using osu.Framework.Extensions.ObjectExtensions; using osu.Framework.Testing; using osu.Game.Database; -using osu.Game.Extensions; using osu.Game.IO; using osu.Game.Models; using Realms; @@ -32,6 +31,8 @@ namespace osu.Game.Skinning public string Hash { get; set; } = string.Empty; + public bool Protected { get; set; } + public string InstantiationInfo { get; set; } = string.Empty; public virtual Skin CreateInstance(IStorageResourceProvider resources) @@ -48,14 +49,6 @@ namespace osu.Game.Skinning public bool DeletePending { get; set; } - public static SkinInfo Default { get; } = new SkinInfo - { - ID = DEFAULT_SKIN, - Name = "osu! (triangles)", - Creator = "team osu!", - InstantiationInfo = typeof(DefaultSkin).GetInvariantInstantiationInfo() - }; - public bool Equals(SkinInfo? other) { if (ReferenceEquals(this, other)) return true; diff --git a/osu.Game/Skinning/SkinManager.cs b/osu.Game/Skinning/SkinManager.cs index bcad5277ab..5c07244e0f 100644 --- a/osu.Game/Skinning/SkinManager.cs +++ b/osu.Game/Skinning/SkinManager.cs @@ -45,7 +45,11 @@ namespace osu.Game.Skinning private readonly IResourceStore resources; public readonly Bindable CurrentSkin = new Bindable(); - public readonly Bindable> CurrentSkinInfo = new Bindable>(SkinInfo.Default.ToLive()) { Default = SkinInfo.Default.ToLive() }; + + public readonly Bindable> CurrentSkinInfo = new Bindable>(Skinning.DefaultSkin.CreateInfo().ToLive()) + { + Default = Skinning.DefaultSkin.CreateInfo().ToLive() + }; private readonly SkinModelManager skinModelManager; private readonly RealmContextFactory contextFactory; @@ -74,8 +78,24 @@ namespace osu.Game.Skinning skinModelManager = new SkinModelManager(storage, contextFactory, host, this); - DefaultLegacySkin = new DefaultLegacySkin(this); - DefaultSkin = new DefaultSkin(this); + var defaultSkins = new[] + { + DefaultLegacySkin = new DefaultLegacySkin(this), + DefaultSkin = new DefaultSkin(this), + }; + + // Ensure the default entries are present. + using (var context = contextFactory.CreateContext()) + using (var transaction = context.BeginWrite()) + { + foreach (var skin in defaultSkins) + { + if (context.Find(skin.SkinInfo.ID) == null) + context.Add(skin.SkinInfo.Value); + } + + transaction.Commit(); + } CurrentSkinInfo.ValueChanged += skin => CurrentSkin.Value = skin.NewValue.PerformRead(GetSkin); @@ -89,21 +109,6 @@ namespace osu.Game.Skinning }; } - /// - /// Returns a list of all usable s. Includes the non-databased default skins. - /// - /// A newly allocated list of available . - public List> GetAllUsableSkins() - { - using (var context = contextFactory.CreateContext()) - { - var userSkins = context.All().Where(s => !s.DeletePending).ToLive(); - userSkins.Insert(0, DefaultSkin.SkinInfo); - userSkins.Insert(1, DefaultLegacySkin.SkinInfo); - return userSkins; - } - } - public void SelectRandomSkin() { using (var context = contextFactory.CreateContext()) @@ -113,7 +118,7 @@ namespace osu.Game.Skinning if (randomChoices.Length == 0) { - CurrentSkinInfo.Value = SkinInfo.Default.ToLive(); + CurrentSkinInfo.Value = Skinning.DefaultSkin.CreateInfo().ToLive(); return; } From 0d18c83d758e033daf823218015e25eb1458960f Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 29 Nov 2021 17:15:48 +0900 Subject: [PATCH 18/34] Simplify deletion by adding always present conditionals to `Delete` method --- .../Settings/Sections/Maintenance/GeneralSettings.cs | 2 +- osu.Game/Skinning/SkinManager.cs | 10 +++++++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/osu.Game/Overlays/Settings/Sections/Maintenance/GeneralSettings.cs b/osu.Game/Overlays/Settings/Sections/Maintenance/GeneralSettings.cs index e7e196555f..98ccbf85fd 100644 --- a/osu.Game/Overlays/Settings/Sections/Maintenance/GeneralSettings.cs +++ b/osu.Game/Overlays/Settings/Sections/Maintenance/GeneralSettings.cs @@ -108,7 +108,7 @@ namespace osu.Game.Overlays.Settings.Sections.Maintenance deleteSkinsButton.Enabled.Value = false; Task.Run(() => { - skins.Delete(s => !s.DeletePending); + skins.Delete(); }).ContinueWith(t => Schedule(() => deleteSkinsButton.Enabled.Value = true)); })); } diff --git a/osu.Game/Skinning/SkinManager.cs b/osu.Game/Skinning/SkinManager.cs index 5c07244e0f..d6c884d259 100644 --- a/osu.Game/Skinning/SkinManager.cs +++ b/osu.Game/Skinning/SkinManager.cs @@ -7,6 +7,7 @@ using System.Linq; using System.Linq.Expressions; using System.Threading; using System.Threading.Tasks; +using JetBrains.Annotations; using osu.Framework.Audio; using osu.Framework.Audio.Sample; using osu.Framework.Bindables; @@ -286,15 +287,18 @@ namespace osu.Game.Skinning { using (var context = contextFactory.CreateContext()) { - var items = context.All().Where(filter).ToList(); + var items = context.All() + .Where(s => !s.Protected && !s.DeletePending); + if (filter != null) + items = items.Where(filter); // check the removed skin is not the current user choice. if it is, switch back to default. Guid currentUserSkin = CurrentSkinInfo.Value.ID; if (items.Any(s => s.ID == currentUserSkin)) - scheduler.Add(() => CurrentSkinInfo.Value = SkinInfo.Default.ToLive()); + scheduler.Add(() => CurrentSkinInfo.Value = Skinning.DefaultSkin.CreateInfo().ToLive()); - skinModelManager.Delete(items, silent); + skinModelManager.Delete(items.ToList(), silent); } } From 68778674674e7f50598ba9e39d780417eff8a09f Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 29 Nov 2021 17:48:44 +0900 Subject: [PATCH 19/34] Make default fallback logic more robust --- .../Overlays/Settings/Sections/SkinSection.cs | 22 +++++-------------- 1 file changed, 6 insertions(+), 16 deletions(-) diff --git a/osu.Game/Overlays/Settings/Sections/SkinSection.cs b/osu.Game/Overlays/Settings/Sections/SkinSection.cs index bf0f6bf142..4de5d455fe 100644 --- a/osu.Game/Overlays/Settings/Sections/SkinSection.cs +++ b/osu.Game/Overlays/Settings/Sections/SkinSection.cs @@ -96,11 +96,9 @@ namespace osu.Game.Overlays.Settings.Sections updateItems(); - // Todo: This should not be necessary when OsuConfigManager is databased - if (!Guid.TryParse(configBindable.Value, out var configId) || skinDropdown.Items.All(s => s.ID != configId)) - configBindable.Value = string.Empty; + configBindable.BindValueChanged(id => Scheduler.AddOnce(updateSelectedSkinFromConfig)); + updateSelectedSkinFromConfig(); - configBindable.BindValueChanged(id => Scheduler.AddOnce(updateSelectedSkinFromConfig), true); dropdownBindable.BindValueChanged(skin => { if (skin.NewValue.Equals(random_skin_info)) @@ -124,20 +122,12 @@ namespace osu.Game.Overlays.Settings.Sections private void updateSelectedSkinFromConfig() { - if (!Guid.TryParse(configBindable.Value, out var configId)) return; + ILive skin = null; - var skin = skinDropdown.Items.FirstOrDefault(s => s.ID == configId); + if (Guid.TryParse(configBindable.Value, out var configId)) + skin = skinDropdown.Items.FirstOrDefault(s => s.ID == configId); - // TODO: i don't think this will be required any more. - if (skin == null) - { - // there may be a thread race condition where an item is selected that hasn't yet been added to the dropdown. - // to avoid adding complexity, let's just ensure the item is added so we can perform the selection. - skin = skins.Query(s => s.ID == configId); - updateItems(); - } - - dropdownBindable.Value = skin; + dropdownBindable.Value = skin ?? skinDropdown.Items.First(); } private void updateItems() From 6bf9327228e66b49e4c3cd590e055eb5bf7bad8a Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 30 Nov 2021 15:17:16 +0900 Subject: [PATCH 20/34] Add linking property on `SkinFileInfo` to allow EF to understand the link post-rename --- osu.Game/Database/OsuDbContext.cs | 1 + osu.Game/Skinning/SkinFileInfo.cs | 3 +++ 2 files changed, 4 insertions(+) diff --git a/osu.Game/Database/OsuDbContext.cs b/osu.Game/Database/OsuDbContext.cs index 26f287da26..13d362e0be 100644 --- a/osu.Game/Database/OsuDbContext.cs +++ b/osu.Game/Database/OsuDbContext.cs @@ -135,6 +135,7 @@ namespace osu.Game.Database modelBuilder.Entity().HasIndex(b => b.Hash).IsUnique(); modelBuilder.Entity().HasIndex(b => b.DeletePending); + modelBuilder.Entity().HasMany(s => s.Files).WithOne(f => f.SkinInfo); modelBuilder.Entity().HasIndex(b => new { b.RulesetID, b.Variant }); diff --git a/osu.Game/Skinning/SkinFileInfo.cs b/osu.Game/Skinning/SkinFileInfo.cs index db7cd953bb..4a86a6ed94 100644 --- a/osu.Game/Skinning/SkinFileInfo.cs +++ b/osu.Game/Skinning/SkinFileInfo.cs @@ -2,6 +2,7 @@ // See the LICENCE file in the repository root for full licence text. using System.ComponentModel.DataAnnotations; +using System.ComponentModel.DataAnnotations.Schema; using osu.Game.Database; using osu.Game.IO; @@ -13,6 +14,8 @@ namespace osu.Game.Skinning public int SkinInfoID { get; set; } + public EFSkinInfo SkinInfo { get; set; } + public int FileInfoID { get; set; } public FileInfo FileInfo { get; set; } From a943089be261495bbc7c1aec19bff731d5277543 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 30 Nov 2021 15:17:26 +0900 Subject: [PATCH 21/34] Add support for migration of skins from EF to Realm --- osu.Game/Database/RealmContextFactory.cs | 56 ++++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/osu.Game/Database/RealmContextFactory.cs b/osu.Game/Database/RealmContextFactory.cs index 911c88a61f..290b42311c 100644 --- a/osu.Game/Database/RealmContextFactory.cs +++ b/osu.Game/Database/RealmContextFactory.cs @@ -5,6 +5,7 @@ using System; using System.Linq; using System.Reflection; using System.Threading; +using Microsoft.EntityFrameworkCore; using osu.Framework.Allocation; using osu.Framework.Development; using osu.Framework.Input.Bindings; @@ -205,6 +206,61 @@ namespace osu.Game.Database return; using (var db = efContextFactory.GetForWrite()) + { + migrateSettings(db); + migrateSkins(db); + } + + void migrateSkins(DatabaseWriteUsage db) + { + // migrate ruleset settings. can be removed 20220530. + var existingSkins = db.Context.SkinInfo + .Include(s => s.Files) + .ThenInclude(f => f.FileInfo); + + // previous entries in EF are removed post migration. + if (!existingSkins.Any()) + return; + + using (var realm = CreateContext()) + using (var transaction = realm.BeginWrite()) + { + // only migrate data if the realm database is empty. + if (!realm.All().Any(s => !s.Protected)) + { + foreach (var skin in existingSkins) + { + var realmSkin = new SkinInfo + { + Name = skin.Name, + Creator = skin.Creator, + Hash = skin.Hash, + Protected = false, + InstantiationInfo = skin.InstantiationInfo, + }; + + foreach (var file in skin.Files) + { + var realmFile = realm.Find(file.FileInfo.Hash); + + if (realmFile == null) + realm.Add(realmFile = new RealmFile { Hash = file.FileInfo.Hash }); + + realmSkin.Files.Add(new RealmNamedFileUsage(realmFile, file.Filename)); + } + + realm.Add(realmSkin); + } + } + + db.Context.RemoveRange(existingSkins); + // Intentionally don't clean up the files, so they don't get purged by EF. + + transaction.Commit(); + } + } + + void migrateSettings(DatabaseWriteUsage db) { // migrate ruleset settings. can be removed 20220315. var existingSettings = db.Context.DatabasedSetting; From 0efd565c8b974986effde26819648030eb8ea326 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 30 Nov 2021 15:41:07 +0900 Subject: [PATCH 22/34] Remove forgotten using statement --- osu.Game/Skinning/SkinFileInfo.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/osu.Game/Skinning/SkinFileInfo.cs b/osu.Game/Skinning/SkinFileInfo.cs index 4a86a6ed94..f414687b33 100644 --- a/osu.Game/Skinning/SkinFileInfo.cs +++ b/osu.Game/Skinning/SkinFileInfo.cs @@ -2,7 +2,6 @@ // See the LICENCE file in the repository root for full licence text. using System.ComponentModel.DataAnnotations; -using System.ComponentModel.DataAnnotations.Schema; using osu.Game.Database; using osu.Game.IO; From 6f66ecd77be4d6e5fc9b9a03308b2600ebf92c31 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 30 Nov 2021 15:41:18 +0900 Subject: [PATCH 23/34] Move migrations to own file and add user skin choice config migration --- osu.Game/Database/EFToRealmMigrator.cs | 147 +++++++++++++++++++++++ osu.Game/Database/RealmContextFactory.cs | 107 ----------------- osu.Game/OsuGameBase.cs | 2 + 3 files changed, 149 insertions(+), 107 deletions(-) create mode 100644 osu.Game/Database/EFToRealmMigrator.cs diff --git a/osu.Game/Database/EFToRealmMigrator.cs b/osu.Game/Database/EFToRealmMigrator.cs new file mode 100644 index 0000000000..670eb95ad6 --- /dev/null +++ b/osu.Game/Database/EFToRealmMigrator.cs @@ -0,0 +1,147 @@ +// 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 Microsoft.EntityFrameworkCore; +using osu.Game.Configuration; +using osu.Game.Models; +using osu.Game.Skinning; + +#nullable enable + +namespace osu.Game.Database +{ + internal class EFToRealmMigrator + { + private readonly DatabaseContextFactory efContextFactory; + private readonly RealmContextFactory realmContextFactory; + private readonly OsuConfigManager config; + + public EFToRealmMigrator(DatabaseContextFactory efContextFactory, RealmContextFactory realmContextFactory, OsuConfigManager config) + { + this.efContextFactory = efContextFactory; + this.realmContextFactory = realmContextFactory; + this.config = config; + } + + public void Run() + { + using (var db = efContextFactory.GetForWrite()) + { + migrateSettings(db); + migrateSkins(db); + } + } + + private void migrateSkins(DatabaseWriteUsage db) + { + var userSkinChoice = config.GetBindable(OsuSetting.Skin); + int.TryParse(userSkinChoice.Value, out int userSkinInt); + + switch (userSkinInt) + { + case EFSkinInfo.DEFAULT_SKIN: + userSkinChoice.Value = SkinInfo.DEFAULT_SKIN.ToString(); + break; + + case EFSkinInfo.CLASSIC_SKIN: + userSkinChoice.Value = SkinInfo.CLASSIC_SKIN.ToString(); + break; + } + + // migrate ruleset settings. can be removed 20220530. + var existingSkins = db.Context.SkinInfo + .Include(s => s.Files) + .ThenInclude(f => f.FileInfo) + .ToList(); + + // previous entries in EF are removed post migration. + if (!existingSkins.Any()) + return; + + using (var realm = realmContextFactory.CreateContext()) + using (var transaction = realm.BeginWrite()) + { + // only migrate data if the realm database is empty. + if (!realm.All().Any(s => !s.Protected)) + { + foreach (var skin in existingSkins) + { + var realmSkin = new SkinInfo + { + Name = skin.Name, + Creator = skin.Creator, + Hash = skin.Hash, + Protected = false, + InstantiationInfo = skin.InstantiationInfo, + }; + + foreach (var file in skin.Files) + { + var realmFile = realm.Find(file.FileInfo.Hash); + + if (realmFile == null) + realm.Add(realmFile = new RealmFile { Hash = file.FileInfo.Hash }); + + realmSkin.Files.Add(new RealmNamedFileUsage(realmFile, file.Filename)); + } + + realm.Add(realmSkin); + + if (skin.ID == userSkinInt) + userSkinChoice.Value = realmSkin.ID.ToString(); + } + } + + db.Context.RemoveRange(existingSkins); + // Intentionally don't clean up the files, so they don't get purged by EF. + + transaction.Commit(); + } + } + + private void migrateSettings(DatabaseWriteUsage db) + { + // migrate ruleset settings. can be removed 20220315. + var existingSettings = db.Context.DatabasedSetting; + + // previous entries in EF are removed post migration. + if (!existingSettings.Any()) + return; + + using (var realm = realmContextFactory.CreateContext()) + using (var transaction = realm.BeginWrite()) + { + // only migrate data if the realm database is empty. + if (!realm.All().Any()) + { + foreach (var dkb in existingSettings) + { + if (dkb.RulesetID == null) + continue; + + string? shortName = getRulesetShortNameFromLegacyID(dkb.RulesetID.Value); + + if (string.IsNullOrEmpty(shortName)) + continue; + + realm.Add(new RealmRulesetSetting + { + Key = dkb.Key, + Value = dkb.StringValue, + RulesetName = shortName, + Variant = dkb.Variant ?? 0, + }); + } + } + + db.Context.RemoveRange(existingSettings); + + transaction.Commit(); + } + } + + private string? getRulesetShortNameFromLegacyID(long rulesetId) => + efContextFactory.Get().RulesetInfo.FirstOrDefault(r => r.ID == rulesetId)?.ShortName; + } +} diff --git a/osu.Game/Database/RealmContextFactory.cs b/osu.Game/Database/RealmContextFactory.cs index 290b42311c..c6acd3097c 100644 --- a/osu.Game/Database/RealmContextFactory.cs +++ b/osu.Game/Database/RealmContextFactory.cs @@ -5,7 +5,6 @@ using System; using System.Linq; using System.Reflection; using System.Threading; -using Microsoft.EntityFrameworkCore; using osu.Framework.Allocation; using osu.Framework.Development; using osu.Framework.Input.Bindings; @@ -103,10 +102,6 @@ namespace osu.Game.Database // This method triggers the first `CreateContext` call, which will implicitly run realm migrations and bring the schema up-to-date. cleanupPendingDeletions(); - - // Data migration is handled separately from schema migrations. - // This is required as the user may be initialising realm for the first time ever, which would result in no schema migrations running. - migrateDataFromEF(); } private void cleanupPendingDeletions() @@ -200,108 +195,6 @@ namespace osu.Game.Database }; } - private void migrateDataFromEF() - { - if (efContextFactory == null) - return; - - using (var db = efContextFactory.GetForWrite()) - { - migrateSettings(db); - migrateSkins(db); - } - - void migrateSkins(DatabaseWriteUsage db) - { - // migrate ruleset settings. can be removed 20220530. - var existingSkins = db.Context.SkinInfo - .Include(s => s.Files) - .ThenInclude(f => f.FileInfo); - - // previous entries in EF are removed post migration. - if (!existingSkins.Any()) - return; - - using (var realm = CreateContext()) - using (var transaction = realm.BeginWrite()) - { - // only migrate data if the realm database is empty. - if (!realm.All().Any(s => !s.Protected)) - { - foreach (var skin in existingSkins) - { - var realmSkin = new SkinInfo - { - Name = skin.Name, - Creator = skin.Creator, - Hash = skin.Hash, - Protected = false, - InstantiationInfo = skin.InstantiationInfo, - }; - - foreach (var file in skin.Files) - { - var realmFile = realm.Find(file.FileInfo.Hash); - - if (realmFile == null) - realm.Add(realmFile = new RealmFile { Hash = file.FileInfo.Hash }); - - realmSkin.Files.Add(new RealmNamedFileUsage(realmFile, file.Filename)); - } - - realm.Add(realmSkin); - } - } - - db.Context.RemoveRange(existingSkins); - // Intentionally don't clean up the files, so they don't get purged by EF. - - transaction.Commit(); - } - } - - void migrateSettings(DatabaseWriteUsage db) - { - // migrate ruleset settings. can be removed 20220315. - var existingSettings = db.Context.DatabasedSetting; - - // previous entries in EF are removed post migration. - if (!existingSettings.Any()) - return; - - using (var realm = CreateContext()) - using (var transaction = realm.BeginWrite()) - { - // only migrate data if the realm database is empty. - if (!realm.All().Any()) - { - foreach (var dkb in existingSettings) - { - if (dkb.RulesetID == null) - continue; - - string? shortName = getRulesetShortNameFromLegacyID(dkb.RulesetID.Value); - - if (string.IsNullOrEmpty(shortName)) - continue; - - realm.Add(new RealmRulesetSetting - { - Key = dkb.Key, - Value = dkb.StringValue, - RulesetName = shortName, - Variant = dkb.Variant ?? 0, - }); - } - } - - db.Context.RemoveRange(existingSettings); - - transaction.Commit(); - } - } - } - private void onMigration(Migration migration, ulong lastSchemaVersion) { for (ulong i = lastSchemaVersion + 1; i <= schema_version; i++) diff --git a/osu.Game/OsuGameBase.cs b/osu.Game/OsuGameBase.cs index 4bae6f3c1d..f6e2c780ed 100644 --- a/osu.Game/OsuGameBase.cs +++ b/osu.Game/OsuGameBase.cs @@ -198,6 +198,8 @@ namespace osu.Game dependencies.Cache(realmFactory = new RealmContextFactory(Storage, "client", contextFactory)); + new EFToRealmMigrator(contextFactory, realmFactory, LocalConfig).Run(); + dependencies.CacheAs(Storage); var largeStore = new LargeTextureStore(Host.CreateTextureLoaderStore(new NamespacedResourceStore(Resources, @"Textures"))); From d78c18d03f6982f260ef717f53357d2db823d6e2 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 1 Dec 2021 12:44:25 +0900 Subject: [PATCH 24/34] Remove excess brackets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bartłomiej Dach --- osu.Game.Tests/Skins/IO/ImportSkinTest.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Tests/Skins/IO/ImportSkinTest.cs b/osu.Game.Tests/Skins/IO/ImportSkinTest.cs index 9b7e169745..f3c2cd8ba2 100644 --- a/osu.Game.Tests/Skins/IO/ImportSkinTest.cs +++ b/osu.Game.Tests/Skins/IO/ImportSkinTest.cs @@ -268,7 +268,7 @@ namespace osu.Game.Tests.Skins.IO private async Task> loadSkinIntoOsu(OsuGameBase osu, ArchiveReader archive = null) { var skinManager = osu.Dependencies.Get(); - return (await skinManager.Import(archive)); + return await skinManager.Import(archive); } } } From fb2310f82695f146a3c8dfb1a8ebb176ed7fe5b5 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 1 Dec 2021 12:45:47 +0900 Subject: [PATCH 25/34] Specify config file default as `DEFAULT_SKIN`'s guid --- osu.Game/Configuration/OsuConfigManager.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/osu.Game/Configuration/OsuConfigManager.cs b/osu.Game/Configuration/OsuConfigManager.cs index ade817e652..c6a2abecd7 100644 --- a/osu.Game/Configuration/OsuConfigManager.cs +++ b/osu.Game/Configuration/OsuConfigManager.cs @@ -17,6 +17,7 @@ using osu.Game.Overlays; using osu.Game.Rulesets.Scoring; using osu.Game.Screens.Select; using osu.Game.Screens.Select.Filter; +using osu.Game.Skinning; namespace osu.Game.Configuration { @@ -27,7 +28,7 @@ namespace osu.Game.Configuration { // UI/selection defaults SetDefault(OsuSetting.Ruleset, string.Empty); - SetDefault(OsuSetting.Skin, string.Empty); + SetDefault(OsuSetting.Skin, SkinInfo.DEFAULT_SKIN.ToString()); SetDefault(OsuSetting.BeatmapDetailTab, PlayBeatmapDetailArea.TabType.Details); SetDefault(OsuSetting.BeatmapDetailModsFilter, false); From 370135d484bd1bece6925fe577a9074cfaa8e163 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 1 Dec 2021 12:47:32 +0900 Subject: [PATCH 26/34] Remove outdated TODO --- osu.Game/OsuGame.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/osu.Game/OsuGame.cs b/osu.Game/OsuGame.cs index 8af76c1289..047c3b4225 100644 --- a/osu.Game/OsuGame.cs +++ b/osu.Game/OsuGame.cs @@ -247,7 +247,6 @@ namespace osu.Game SkinManager.CurrentSkinInfo.ValueChanged += skin => configSkin.Value = skin.NewValue.ID.ToString(); configSkin.ValueChanged += skinId => { - // TODO: migrate the user skin selection to the new ID format. ILive skinInfo = null; if (Guid.TryParse(skinId.NewValue, out var guid)) From 0e0e8c25e8587e0fd121d1c4a0ad8cfe9055a08f Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 1 Dec 2021 12:48:49 +0900 Subject: [PATCH 27/34] Refactor migration precondition to read better --- osu.Game/Database/EFToRealmMigrator.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Database/EFToRealmMigrator.cs b/osu.Game/Database/EFToRealmMigrator.cs index 670eb95ad6..8f436470af 100644 --- a/osu.Game/Database/EFToRealmMigrator.cs +++ b/osu.Game/Database/EFToRealmMigrator.cs @@ -63,7 +63,7 @@ namespace osu.Game.Database using (var transaction = realm.BeginWrite()) { // only migrate data if the realm database is empty. - if (!realm.All().Any(s => !s.Protected)) + if (realm.All().All(s => s.Protected)) { foreach (var skin in existingSkins) { From 8ce7467e9784f7ebd1a0f6f75fcf437e87832d1c Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 1 Dec 2021 12:50:47 +0900 Subject: [PATCH 28/34] Fix ordering of skins in dropdown being reverse of expected --- osu.Game/Overlays/Settings/Sections/SkinSection.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Overlays/Settings/Sections/SkinSection.cs b/osu.Game/Overlays/Settings/Sections/SkinSection.cs index 4de5d455fe..e3230098e2 100644 --- a/osu.Game/Overlays/Settings/Sections/SkinSection.cs +++ b/osu.Game/Overlays/Settings/Sections/SkinSection.cs @@ -81,7 +81,7 @@ namespace osu.Game.Overlays.Settings.Sections realmSkins = realmFactory.Context.All() .Where(s => !s.DeletePending) - .OrderBy(s => s.Protected) + .OrderByDescending(s => s.Protected) // protected skins should be at the top. .ThenBy(s => s.Name, StringComparer.OrdinalIgnoreCase); realmSubscription = realmSkins From ea66cd6c5e3c31602cec5a9d03d54a9d24cb411d Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 1 Dec 2021 12:55:19 +0900 Subject: [PATCH 29/34] Add xmldoc and make realm ongoing transaction file op methods `protected` instead of `public` --- osu.Game/Stores/RealmArchiveModelManager.cs | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/osu.Game/Stores/RealmArchiveModelManager.cs b/osu.Game/Stores/RealmArchiveModelManager.cs index ae40bcac79..a916e2b53a 100644 --- a/osu.Game/Stores/RealmArchiveModelManager.cs +++ b/osu.Game/Stores/RealmArchiveModelManager.cs @@ -57,17 +57,26 @@ namespace osu.Game.Stores public void AddFile(TModel item, Stream stream, string filename) => item.Realm.Write(() => AddFile(item, stream, filename, item.Realm)); - public void DeleteFile(TModel item, RealmNamedFileUsage file, Realm realm) + /// + /// Delete a file from within an ongoing realm transaction. + /// + protected void DeleteFile(TModel item, RealmNamedFileUsage file, Realm realm) { item.Files.Remove(file); } - public void ReplaceFile(TModel model, RealmNamedFileUsage file, Stream contents, Realm realm) + /// + /// Replace a file from within an ongoing realm transaction. + /// + protected void ReplaceFile(TModel model, RealmNamedFileUsage file, Stream contents, Realm realm) { file.File = realmFileStore.Add(contents, realm); } - public void AddFile(TModel item, Stream stream, string filename, Realm realm) + /// + /// Add a file from within an ongoing realm transaction. + /// + protected void AddFile(TModel item, Stream stream, string filename, Realm realm) { var file = realmFileStore.Add(stream, realm); var namedUsage = new RealmNamedFileUsage(file, filename); From 8bef50cbbaf79a3918f242643f412822bcd313e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 1 Dec 2021 19:35:15 +0100 Subject: [PATCH 30/34] Revert "Refactor migration precondition to read better" Realm cannot translate `.All()` LINQ queries. This reverts commit 0e0e8c25e8587e0fd121d1c4a0ad8cfe9055a08f. --- osu.Game/Database/EFToRealmMigrator.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/osu.Game/Database/EFToRealmMigrator.cs b/osu.Game/Database/EFToRealmMigrator.cs index 8f436470af..3790dc8ae9 100644 --- a/osu.Game/Database/EFToRealmMigrator.cs +++ b/osu.Game/Database/EFToRealmMigrator.cs @@ -63,7 +63,8 @@ namespace osu.Game.Database using (var transaction = realm.BeginWrite()) { // only migrate data if the realm database is empty. - if (realm.All().All(s => s.Protected)) + // note that this cannot be written as: `realm.All().All(s => s.Protected)`, because realm does not support `.All()`. + if (!realm.All().Any(s => !s.Protected)) { foreach (var skin in existingSkins) { From c82195390fb19ed1e69121271ce4b91ea870f954 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 2 Dec 2021 13:24:16 +0900 Subject: [PATCH 31/34] Update usage of `SubscribeForNotifications` --- osu.Game/Overlays/Settings/Sections/SkinSection.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/osu.Game/Overlays/Settings/Sections/SkinSection.cs b/osu.Game/Overlays/Settings/Sections/SkinSection.cs index e3230098e2..c932f4837a 100644 --- a/osu.Game/Overlays/Settings/Sections/SkinSection.cs +++ b/osu.Game/Overlays/Settings/Sections/SkinSection.cs @@ -18,7 +18,6 @@ using osu.Game.Graphics.UserInterface; using osu.Game.Localisation; using osu.Game.Skinning; using osu.Game.Skinning.Editor; -using Realms; namespace osu.Game.Overlays.Settings.Sections { @@ -85,7 +84,7 @@ namespace osu.Game.Overlays.Settings.Sections .ThenBy(s => s.Name, StringComparer.OrdinalIgnoreCase); realmSubscription = realmSkins - .SubscribeForNotifications((sender, changes, error) => + .QueryAsyncWithNotifications((sender, changes, error) => { if (changes == null) return; From 0a14acfd83d636ab2f7a087b6c4efcd134e7ea46 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 2 Dec 2021 13:41:20 +0900 Subject: [PATCH 32/34] Fix incorrect conditional on export/mutate feasability of skin --- osu.Game/Overlays/Settings/Sections/SkinSection.cs | 2 +- osu.Game/Skinning/SkinManager.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game/Overlays/Settings/Sections/SkinSection.cs b/osu.Game/Overlays/Settings/Sections/SkinSection.cs index c932f4837a..bcf0a367ef 100644 --- a/osu.Game/Overlays/Settings/Sections/SkinSection.cs +++ b/osu.Game/Overlays/Settings/Sections/SkinSection.cs @@ -179,7 +179,7 @@ namespace osu.Game.Overlays.Settings.Sections base.LoadComplete(); currentSkin = skins.CurrentSkin.GetBoundCopy(); - currentSkin.BindValueChanged(skin => Enabled.Value = skin.NewValue.SkinInfo.PerformRead(s => s.IsManaged), true); + currentSkin.BindValueChanged(skin => Enabled.Value = skin.NewValue.SkinInfo.PerformRead(s => !s.Protected), true); } private void export() diff --git a/osu.Game/Skinning/SkinManager.cs b/osu.Game/Skinning/SkinManager.cs index d6c884d259..701bf6d0cf 100644 --- a/osu.Game/Skinning/SkinManager.cs +++ b/osu.Game/Skinning/SkinManager.cs @@ -144,7 +144,7 @@ namespace osu.Game.Skinning { CurrentSkinInfo.Value.PerformRead(s => { - if (s.IsManaged) + if (!s.Protected) return; // if the user is attempting to save one of the default skin implementations, create a copy first. From e855a49833fb0a08694233757641e8828f5d3955 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 2 Dec 2021 14:01:18 +0900 Subject: [PATCH 33/34] Add test coverage of default skin edit and export --- .../Navigation/TestSceneEditDefaultSkin.cs | 40 +++++++++++++++++++ .../Overlays/Settings/Sections/SkinSection.cs | 2 +- 2 files changed, 41 insertions(+), 1 deletion(-) create mode 100644 osu.Game.Tests/Visual/Navigation/TestSceneEditDefaultSkin.cs diff --git a/osu.Game.Tests/Visual/Navigation/TestSceneEditDefaultSkin.cs b/osu.Game.Tests/Visual/Navigation/TestSceneEditDefaultSkin.cs new file mode 100644 index 0000000000..f5b9d287ca --- /dev/null +++ b/osu.Game.Tests/Visual/Navigation/TestSceneEditDefaultSkin.cs @@ -0,0 +1,40 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System.IO; +using System.Linq; +using NUnit.Framework; +using osu.Framework.Allocation; +using osu.Framework.Testing; +using osu.Game.Overlays.Settings.Sections; +using osu.Game.Skinning; +using osu.Game.Skinning.Editor; + +namespace osu.Game.Tests.Visual.Navigation +{ + public class TestSceneEditDefaultSkin : OsuGameTestScene + { + private SkinManager skinManager => Game.Dependencies.Get(); + private SkinEditorOverlay skinEditor => Game.Dependencies.Get(); + + [Test] + public void TestEditDefaultSkin() + { + AddAssert("is default skin", () => skinManager.CurrentSkinInfo.Value.ID == SkinInfo.DEFAULT_SKIN); + + AddStep("open settings", () => { Game.Settings.Show(); }); + + // Until step requires as settings has a delayed load. + AddUntilStep("export button disabled", () => Game.Settings.ChildrenOfType().SingleOrDefault()?.Enabled.Value == false); + + // Will create a mutable skin. + AddStep("open skin editor", () => skinEditor.Show()); + + // Until step required as the skin editor may take time to load (and an extra scheduled frame for the mutable part). + AddUntilStep("is modified default skin", () => skinManager.CurrentSkinInfo.Value.ID != SkinInfo.DEFAULT_SKIN); + AddAssert("is not protected", () => skinManager.CurrentSkinInfo.Value.PerformRead(s => !s.Protected)); + + AddUntilStep("export button enabled", () => Game.Settings.ChildrenOfType().SingleOrDefault()?.Enabled.Value == true); + } + } +} diff --git a/osu.Game/Overlays/Settings/Sections/SkinSection.cs b/osu.Game/Overlays/Settings/Sections/SkinSection.cs index bcf0a367ef..b1582d7bee 100644 --- a/osu.Game/Overlays/Settings/Sections/SkinSection.cs +++ b/osu.Game/Overlays/Settings/Sections/SkinSection.cs @@ -157,7 +157,7 @@ namespace osu.Game.Overlays.Settings.Sections } } - private class ExportSkinButton : SettingsButton + public class ExportSkinButton : SettingsButton { [Resolved] private SkinManager skins { get; set; } From ec700e91427549ff29a36ba9300a8e1aa94d2410 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 2 Dec 2021 14:01:57 +0900 Subject: [PATCH 34/34] Remove unused using statement --- osu.Game.Tests/Visual/Navigation/TestSceneEditDefaultSkin.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/osu.Game.Tests/Visual/Navigation/TestSceneEditDefaultSkin.cs b/osu.Game.Tests/Visual/Navigation/TestSceneEditDefaultSkin.cs index f5b9d287ca..06306ad197 100644 --- a/osu.Game.Tests/Visual/Navigation/TestSceneEditDefaultSkin.cs +++ b/osu.Game.Tests/Visual/Navigation/TestSceneEditDefaultSkin.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.IO; using System.Linq; using NUnit.Framework; using osu.Framework.Allocation;