From 9ee59dd63703a7549d651f7b189a24c6fafef5f1 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 27 Jul 2017 20:38:35 +0900 Subject: [PATCH 01/17] Add the ability to create migrations on a per-store level Now stores store versions to the database itself. --- osu.Game/Beatmaps/BeatmapStore.cs | 33 ++++++++++++++++++++++++ osu.Game/Database/DatabaseBackedStore.cs | 24 +++++++++++++++++ osu.Game/Database/StoreVersion.cs | 15 +++++++++++ osu.Game/OsuGameBase.cs | 3 +++ osu.Game/osu.Game.csproj | 1 + 5 files changed, 76 insertions(+) create mode 100644 osu.Game/Database/StoreVersion.cs diff --git a/osu.Game/Beatmaps/BeatmapStore.cs b/osu.Game/Beatmaps/BeatmapStore.cs index 4a7336535e..f9019537a0 100644 --- a/osu.Game/Beatmaps/BeatmapStore.cs +++ b/osu.Game/Beatmaps/BeatmapStore.cs @@ -17,6 +17,12 @@ namespace osu.Game.Beatmaps public event Action BeatmapSetAdded; public event Action BeatmapSetRemoved; + /// + /// The current version of this store. Used for migrations (see ). + /// The initial version is 1. + /// + protected override int StoreVersion => 1; + public BeatmapStore(SQLiteConnection connection) : base(connection) { @@ -50,6 +56,33 @@ namespace osu.Game.Beatmaps cleanupPendingDeletions(); } + /// + /// Perform migrations between two store versions. + /// + /// The current store version. This will be zero on a fresh database initialisation. + /// The target version which we are migrating to (equal to the current ). + protected override void PerformMigration(int currentVersion, int newVersion) + { + base.PerformMigration(currentVersion, newVersion); + + while (currentVersion++ < newVersion) + { + switch (currentVersion) + { + case 1: + // initialising from a version before we had versioning (or a fresh install). + + // force adding of Protected column (not automatically migrated). + Connection.MigrateTable(); + + // remove all existing beatmaps. + foreach (var b in Connection.GetAllWithChildren(null, true)) + Connection.Delete(b, true); + break; + } + } + } + /// /// Add a to the database. /// diff --git a/osu.Game/Database/DatabaseBackedStore.cs b/osu.Game/Database/DatabaseBackedStore.cs index 8366775483..bb61fc1870 100644 --- a/osu.Game/Database/DatabaseBackedStore.cs +++ b/osu.Game/Database/DatabaseBackedStore.cs @@ -17,6 +17,8 @@ namespace osu.Game.Database protected readonly Storage Storage; protected readonly SQLiteConnection Connection; + protected virtual int StoreVersion => 1; + protected DatabaseBackedStore(SQLiteConnection connection, Storage storage = null) { Storage = storage; @@ -31,6 +33,28 @@ namespace osu.Game.Database Logger.Error(e, $@"Failed to initialise the {GetType()}! Trying again with a clean database..."); Prepare(true); } + + checkMigrations(); + } + + private void checkMigrations() + { + var storeName = GetType().Name; + + var reportedVersion = Connection.Table().FirstOrDefault(s => s.StoreName == storeName) ?? new StoreVersion + { + StoreName = storeName, + Version = 0 + }; + + if (reportedVersion.Version != StoreVersion) + PerformMigration(reportedVersion.Version, reportedVersion.Version = StoreVersion); + + Connection.InsertOrReplace(reportedVersion); + } + + protected virtual void PerformMigration(int currentVersion, int newVersion) + { } /// diff --git a/osu.Game/Database/StoreVersion.cs b/osu.Game/Database/StoreVersion.cs new file mode 100644 index 0000000000..00314875a6 --- /dev/null +++ b/osu.Game/Database/StoreVersion.cs @@ -0,0 +1,15 @@ +// Copyright (c) 2007-2017 ppy Pty Ltd . +// Licensed under the MIT Licence - https://raw.githubusercontent.com/ppy/osu/master/LICENCE + +using SQLite.Net.Attributes; + +namespace osu.Game.Database +{ + public class StoreVersion + { + [PrimaryKey] + public string StoreName { get; set; } + + public int Version { get; set; } + } +} diff --git a/osu.Game/OsuGameBase.cs b/osu.Game/OsuGameBase.cs index b507aa2315..0dec4228de 100644 --- a/osu.Game/OsuGameBase.cs +++ b/osu.Game/OsuGameBase.cs @@ -18,6 +18,7 @@ using osu.Game.Graphics.Processing; using osu.Game.Online.API; using SQLite.Net; using osu.Framework.Graphics.Performance; +using osu.Game.Database; using osu.Game.IO; using osu.Game.Rulesets; using osu.Game.Rulesets.Scoring; @@ -97,6 +98,8 @@ namespace osu.Game SQLiteConnection connection = Host.Storage.GetDatabase(@"client"); + connection.CreateTable(); + dependencies.Cache(RulesetStore = new RulesetStore(connection)); dependencies.Cache(FileStore = new FileStore(connection, Host.Storage)); dependencies.Cache(BeatmapManager = new BeatmapManager(Host.Storage, FileStore, connection, RulesetStore, Host)); diff --git a/osu.Game/osu.Game.csproj b/osu.Game/osu.Game.csproj index f8509314be..5ac76ed00e 100644 --- a/osu.Game/osu.Game.csproj +++ b/osu.Game/osu.Game.csproj @@ -80,6 +80,7 @@ + From e5306997dd816c4dbb7fff6fb2fa42bf1586d97d Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 27 Jul 2017 20:50:26 +0900 Subject: [PATCH 02/17] Fix VisualTests --- osu.Desktop.VisualTests/Tests/TestCasePlaySongSelect.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/osu.Desktop.VisualTests/Tests/TestCasePlaySongSelect.cs b/osu.Desktop.VisualTests/Tests/TestCasePlaySongSelect.cs index 61e87a6621..51c78ff442 100644 --- a/osu.Desktop.VisualTests/Tests/TestCasePlaySongSelect.cs +++ b/osu.Desktop.VisualTests/Tests/TestCasePlaySongSelect.cs @@ -6,6 +6,7 @@ using osu.Desktop.VisualTests.Platform; using osu.Framework.Testing; using osu.Framework.MathUtils; using osu.Game.Beatmaps; +using osu.Game.Database; using osu.Game.Rulesets; using osu.Game.Screens.Select; using osu.Game.Screens.Select.Filter; @@ -29,6 +30,7 @@ namespace osu.Desktop.VisualTests.Tests var storage = new TestStorage(@"TestCasePlaySongSelect"); var backingDatabase = storage.GetDatabase(@"client"); + backingDatabase.CreateTable(); rulesets = new RulesetStore(backingDatabase); manager = new BeatmapManager(storage, null, backingDatabase, rulesets); From 7d4218ea6c9c90142b7ac22c418259cac5810658 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 28 Jul 2017 12:46:38 +0900 Subject: [PATCH 03/17] Add option to import from osu-stable Also adds an option to delete all beatmaps for testing purposes. --- osu.Game/Beatmaps/BeatmapManager.cs | 16 ++++++ .../Sections/Maintenance/GeneralSettings.cs | 51 +++++++++++++++++++ .../Settings/Sections/MaintenanceSection.cs | 2 + osu.Game/osu.Game.csproj | 1 + 4 files changed, 70 insertions(+) create mode 100644 osu.Game/Overlays/Settings/Sections/Maintenance/GeneralSettings.cs diff --git a/osu.Game/Beatmaps/BeatmapManager.cs b/osu.Game/Beatmaps/BeatmapManager.cs index 99117afe35..97031547c0 100644 --- a/osu.Game/Beatmaps/BeatmapManager.cs +++ b/osu.Game/Beatmaps/BeatmapManager.cs @@ -390,5 +390,21 @@ namespace osu.Game.Beatmaps catch { return new TrackVirtual(); } } } + + public void ImportFromStable() + { + + string stableInstallPath = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.LocalApplicationData), @"osu!", "Songs"); + if (!Directory.Exists(stableInstallPath)) + stableInstallPath = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.UserProfile), ".osu", "Songs"); + + if (!Directory.Exists(stableInstallPath)) + { + Logger.Log("Couldn't find an osu!stable installation!", LoggingTarget.Information, LogLevel.Error); + return; + } + + Import(Directory.GetDirectories(stableInstallPath)); + } } } diff --git a/osu.Game/Overlays/Settings/Sections/Maintenance/GeneralSettings.cs b/osu.Game/Overlays/Settings/Sections/Maintenance/GeneralSettings.cs new file mode 100644 index 0000000000..684bfcb39d --- /dev/null +++ b/osu.Game/Overlays/Settings/Sections/Maintenance/GeneralSettings.cs @@ -0,0 +1,51 @@ +// Copyright (c) 2007-2017 ppy Pty Ltd . +// Licensed under the MIT Licence - https://raw.githubusercontent.com/ppy/osu/master/LICENCE + +using System.Threading.Tasks; +using osu.Framework.Allocation; +using osu.Framework.Graphics; +using osu.Game.Beatmaps; +using osu.Game.Graphics.UserInterface; + +namespace osu.Game.Overlays.Settings.Sections.Maintenance +{ + public class GeneralSettings : SettingsSubsection + { + private OsuButton importButton; + private OsuButton deleteButton; + + protected override string Header => "General"; + + [BackgroundDependencyLoader] + private void load(BeatmapManager beatmaps) + { + Children = new Drawable[] + { + importButton = new OsuButton + { + RelativeSizeAxes = Axes.X, + Text = "Import beatmaps from stable", + Action = () => + { + importButton.Enabled.Value = false; + Task.Run(() => beatmaps.ImportFromStable()).ContinueWith(t => Schedule(() => importButton.Enabled.Value = true)); + } + }, + deleteButton = new OsuButton + { + RelativeSizeAxes = Axes.X, + Text = "Delete ALL beatmaps", + Action = () => + { + deleteButton.Enabled.Value = false; + Task.Run(() => + { + foreach (var b in beatmaps.GetAllUsableBeatmapSets()) + beatmaps.Delete(b); + }).ContinueWith(t => Schedule(() => deleteButton.Enabled.Value = true)); + } + }, + }; + } + } +} diff --git a/osu.Game/Overlays/Settings/Sections/MaintenanceSection.cs b/osu.Game/Overlays/Settings/Sections/MaintenanceSection.cs index 529cec79c1..b42c64d324 100644 --- a/osu.Game/Overlays/Settings/Sections/MaintenanceSection.cs +++ b/osu.Game/Overlays/Settings/Sections/MaintenanceSection.cs @@ -3,6 +3,7 @@ using osu.Framework.Graphics; using osu.Game.Graphics; +using osu.Game.Overlays.Settings.Sections.Maintenance; using OpenTK; namespace osu.Game.Overlays.Settings.Sections @@ -17,6 +18,7 @@ namespace osu.Game.Overlays.Settings.Sections FlowContent.Spacing = new Vector2(0, 5); Children = new Drawable[] { + new GeneralSettings() }; } } diff --git a/osu.Game/osu.Game.csproj b/osu.Game/osu.Game.csproj index 5ac76ed00e..b3ed0d2b28 100644 --- a/osu.Game/osu.Game.csproj +++ b/osu.Game/osu.Game.csproj @@ -104,6 +104,7 @@ + From d51ce896f94db37c8cd050bc8624907373c5d600 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 28 Jul 2017 12:46:54 +0900 Subject: [PATCH 04/17] Add locking to all BeatmapManager operations --- osu.Game/Beatmaps/BeatmapManager.cs | 60 ++++++++++++++++++----------- 1 file changed, 38 insertions(+), 22 deletions(-) diff --git a/osu.Game/Beatmaps/BeatmapManager.cs b/osu.Game/Beatmaps/BeatmapManager.cs index 97031547c0..e62fd2fc5d 100644 --- a/osu.Game/Beatmaps/BeatmapManager.cs +++ b/osu.Game/Beatmaps/BeatmapManager.cs @@ -108,9 +108,13 @@ namespace osu.Game.Beatmaps /// The beatmap to be imported. public BeatmapSetInfo Import(ArchiveReader archiveReader) { - BeatmapSetInfo set = importToStorage(archiveReader); - Import(set); - return set; + // let's only allow one concurrent import at a time for now. + lock (this) + { + BeatmapSetInfo set = importToStorage(archiveReader); + Import(set); + return set; + } } /// @@ -132,10 +136,13 @@ namespace osu.Game.Beatmaps /// The beatmap to delete. public void Delete(BeatmapSetInfo beatmapSet) { - if (!beatmaps.Delete(beatmapSet)) return; + lock (this) + { + if (!beatmaps.Delete(beatmapSet)) return; - if (!beatmapSet.Protected) - files.Dereference(beatmapSet.Files); + if (!beatmapSet.Protected) + files.Dereference(beatmapSet.Files); + } } /// @@ -145,9 +152,12 @@ namespace osu.Game.Beatmaps /// The beatmap to restore. public void Undelete(BeatmapSetInfo beatmapSet) { - if (!beatmaps.Undelete(beatmapSet)) return; + lock (this) + { + if (!beatmaps.Undelete(beatmapSet)) return; - files.Reference(beatmapSet.Files); + files.Reference(beatmapSet.Files); + } } /// @@ -158,22 +168,25 @@ namespace osu.Game.Beatmaps /// A instance correlating to the provided . public WorkingBeatmap GetWorkingBeatmap(BeatmapInfo beatmapInfo, WorkingBeatmap previous = null) { - if (beatmapInfo == null || beatmapInfo == DefaultBeatmap?.BeatmapInfo) - return DefaultBeatmap; + lock (this) + { + if (beatmapInfo == null || beatmapInfo == DefaultBeatmap?.BeatmapInfo) + return DefaultBeatmap; - beatmaps.Populate(beatmapInfo); + beatmaps.Populate(beatmapInfo); - if (beatmapInfo.BeatmapSet == null) - throw new InvalidOperationException($@"Beatmap set {beatmapInfo.BeatmapSetInfoID} is not in the local database."); + if (beatmapInfo.BeatmapSet == null) + throw new InvalidOperationException($@"Beatmap set {beatmapInfo.BeatmapSetInfoID} is not in the local database."); - if (beatmapInfo.Metadata == null) - beatmapInfo.Metadata = beatmapInfo.BeatmapSet.Metadata; + if (beatmapInfo.Metadata == null) + beatmapInfo.Metadata = beatmapInfo.BeatmapSet.Metadata; - WorkingBeatmap working = new BeatmapManagerWorkingBeatmap(files.Store, beatmapInfo); + WorkingBeatmap working = new BeatmapManagerWorkingBeatmap(files.Store, beatmapInfo); - previous?.TransferTo(working); + previous?.TransferTo(working); - return working; + return working; + } } /// @@ -325,10 +338,13 @@ namespace osu.Game.Beatmaps /// A list of available . public List GetAllUsableBeatmapSets(bool populate = true) { - if (populate) - return beatmaps.QueryAndPopulate(b => !b.DeletePending).ToList(); - else - return beatmaps.Query(b => !b.DeletePending).ToList(); + lock (this) + { + if (populate) + return beatmaps.QueryAndPopulate(b => !b.DeletePending).ToList(); + else + return beatmaps.Query(b => !b.DeletePending).ToList(); + } } protected class BeatmapManagerWorkingBeatmap : WorkingBeatmap From e448f79154a6d9c33b9f240c65b14051fda0bc59 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 28 Jul 2017 12:47:23 +0900 Subject: [PATCH 05/17] Fix deleted beatmaps not correctly being removed from the playlist --- osu.Game/Overlays/Music/PlaylistList.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Overlays/Music/PlaylistList.cs b/osu.Game/Overlays/Music/PlaylistList.cs index 88f499f9a6..3dd514edeb 100644 --- a/osu.Game/Overlays/Music/PlaylistList.cs +++ b/osu.Game/Overlays/Music/PlaylistList.cs @@ -80,7 +80,7 @@ namespace osu.Game.Overlays.Music public void RemoveBeatmapSet(BeatmapSetInfo beatmapSet) { - PlaylistItem itemToRemove = items.Children.FirstOrDefault(item => item.BeatmapSetInfo == beatmapSet); + PlaylistItem itemToRemove = items.Children.FirstOrDefault(item => item.BeatmapSetInfo.ID == beatmapSet.ID); if (itemToRemove != null) items.Remove(itemToRemove); } From a55586f2ad26f4a37ac3f5b5a420f73a9dd90282 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 28 Jul 2017 13:54:43 +0900 Subject: [PATCH 06/17] FIx potential sequence of execution issues in PlaylistOverlay --- osu.Game/Overlays/Music/PlaylistOverlay.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/osu.Game/Overlays/Music/PlaylistOverlay.cs b/osu.Game/Overlays/Music/PlaylistOverlay.cs index 31fe755d2b..100b397890 100644 --- a/osu.Game/Overlays/Music/PlaylistOverlay.cs +++ b/osu.Game/Overlays/Music/PlaylistOverlay.cs @@ -77,11 +77,11 @@ namespace osu.Game.Overlays.Music }, }; + beatmaps.BeatmapSetAdded += s => Schedule(() => list.AddBeatmapSet(s)); + beatmaps.BeatmapSetRemoved += s => Schedule(() => list.RemoveBeatmapSet(s)); + list.BeatmapSets = BeatmapSets = beatmaps.GetAllUsableBeatmapSets(); - // todo: these should probably be above the query. - beatmaps.BeatmapSetAdded += s => list.AddBeatmapSet(s); - beatmaps.BeatmapSetRemoved += s => list.RemoveBeatmapSet(s); beatmapBacking.BindTo(game.Beatmap); From e691dd12c527132ca80276336cf86a7b364179c0 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 28 Jul 2017 14:22:14 +0900 Subject: [PATCH 07/17] Fix potential sequen of execution issues in BeatmapCarousel --- osu.Game/Screens/Select/BeatmapCarousel.cs | 10 ++++++++-- osu.Game/Screens/Select/SongSelect.cs | 7 ++++++- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index 743a0a0f63..63992915aa 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -107,6 +107,14 @@ namespace osu.Game.Screens.Select }); } + public void RemoveBeatmap(BeatmapSetInfo beatmapSet) + { + Schedule(delegate + { + removeGroup(groups.Find(b => b.BeatmapSet.ID == beatmapSet.ID)); + }); + } + public void SelectBeatmap(BeatmapInfo beatmap, bool animated = true) { if (beatmap == null) @@ -128,8 +136,6 @@ namespace osu.Game.Screens.Select } } - public void RemoveBeatmap(BeatmapSetInfo info) => removeGroup(groups.Find(b => b.BeatmapSet.ID == info.ID)); - public Action SelectionChanged; public Action StartRequested; diff --git a/osu.Game/Screens/Select/SongSelect.cs b/osu.Game/Screens/Select/SongSelect.cs index bbd292870e..d2aec8d919 100644 --- a/osu.Game/Screens/Select/SongSelect.cs +++ b/osu.Game/Screens/Select/SongSelect.cs @@ -280,7 +280,7 @@ namespace osu.Game.Screens.Select carousel.Filter(criteria, debounce); } - private void onBeatmapSetAdded(BeatmapSetInfo s) => carousel.AddBeatmap(s); + private void onBeatmapSetAdded(BeatmapSetInfo s) => Schedule(() => addBeatmapSet(s)); private void onBeatmapSetRemoved(BeatmapSetInfo s) => Schedule(() => removeBeatmapSet(s)); @@ -375,6 +375,11 @@ namespace osu.Game.Screens.Select } } + private void addBeatmapSet(BeatmapSetInfo beatmapSet) + { + carousel.AddBeatmap(beatmapSet); + } + private void removeBeatmapSet(BeatmapSetInfo beatmapSet) { carousel.RemoveBeatmap(beatmapSet); From 6616721e3721ef37e562c54b84185bedae938712 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 28 Jul 2017 14:42:04 +0900 Subject: [PATCH 08/17] Don't block imports and BeatmapStore operations using the same lock --- osu.Game/Beatmaps/BeatmapManager.cs | 43 ++++++++++++++--------------- 1 file changed, 20 insertions(+), 23 deletions(-) diff --git a/osu.Game/Beatmaps/BeatmapManager.cs b/osu.Game/Beatmaps/BeatmapManager.cs index e62fd2fc5d..16a1268e88 100644 --- a/osu.Game/Beatmaps/BeatmapManager.cs +++ b/osu.Game/Beatmaps/BeatmapManager.cs @@ -102,6 +102,8 @@ namespace osu.Game.Beatmaps } } + private object ImportLock = new object(); + /// /// Import a beatmap from an . /// @@ -109,7 +111,7 @@ namespace osu.Game.Beatmaps public BeatmapSetInfo Import(ArchiveReader archiveReader) { // let's only allow one concurrent import at a time for now. - lock (this) + lock (ImportLock) { BeatmapSetInfo set = importToStorage(archiveReader); Import(set); @@ -126,7 +128,8 @@ namespace osu.Game.Beatmaps // If we have an ID then we already exist in the database. if (beatmapSetInfo.ID != 0) return; - beatmaps.Add(beatmapSetInfo); + lock (beatmaps) + beatmaps.Add(beatmapSetInfo); } /// @@ -136,13 +139,11 @@ namespace osu.Game.Beatmaps /// The beatmap to delete. public void Delete(BeatmapSetInfo beatmapSet) { - lock (this) - { + lock (beatmaps) if (!beatmaps.Delete(beatmapSet)) return; - if (!beatmapSet.Protected) - files.Dereference(beatmapSet.Files); - } + if (!beatmapSet.Protected) + files.Dereference(beatmapSet.Files); } /// @@ -152,12 +153,10 @@ namespace osu.Game.Beatmaps /// The beatmap to restore. public void Undelete(BeatmapSetInfo beatmapSet) { - lock (this) - { + lock (beatmaps) if (!beatmaps.Undelete(beatmapSet)) return; - files.Reference(beatmapSet.Files); - } + files.Reference(beatmapSet.Files); } /// @@ -168,25 +167,23 @@ namespace osu.Game.Beatmaps /// A instance correlating to the provided . public WorkingBeatmap GetWorkingBeatmap(BeatmapInfo beatmapInfo, WorkingBeatmap previous = null) { - lock (this) - { - if (beatmapInfo == null || beatmapInfo == DefaultBeatmap?.BeatmapInfo) - return DefaultBeatmap; + if (beatmapInfo == null || beatmapInfo == DefaultBeatmap?.BeatmapInfo) + return DefaultBeatmap; + lock (beatmaps) beatmaps.Populate(beatmapInfo); - if (beatmapInfo.BeatmapSet == null) - throw new InvalidOperationException($@"Beatmap set {beatmapInfo.BeatmapSetInfoID} is not in the local database."); + if (beatmapInfo.BeatmapSet == null) + throw new InvalidOperationException($@"Beatmap set {beatmapInfo.BeatmapSetInfoID} is not in the local database."); - if (beatmapInfo.Metadata == null) - beatmapInfo.Metadata = beatmapInfo.BeatmapSet.Metadata; + if (beatmapInfo.Metadata == null) + beatmapInfo.Metadata = beatmapInfo.BeatmapSet.Metadata; - WorkingBeatmap working = new BeatmapManagerWorkingBeatmap(files.Store, beatmapInfo); + WorkingBeatmap working = new BeatmapManagerWorkingBeatmap(files.Store, beatmapInfo); - previous?.TransferTo(working); + previous?.TransferTo(working); - return working; - } + return working; } /// From d93d9e619019ab0b34f1a3a618ea9de8f2f8e47c Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 28 Jul 2017 15:08:56 +0900 Subject: [PATCH 09/17] Tidy up file deletion after import --- osu.Game/Beatmaps/BeatmapManager.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/osu.Game/Beatmaps/BeatmapManager.cs b/osu.Game/Beatmaps/BeatmapManager.cs index 16a1268e88..646ae3fae9 100644 --- a/osu.Game/Beatmaps/BeatmapManager.cs +++ b/osu.Game/Beatmaps/BeatmapManager.cs @@ -87,11 +87,12 @@ namespace osu.Game.Beatmaps // TODO: Add a check to prevent files from storage to be deleted. try { - File.Delete(path); + if (File.Exists(path)) + File.Delete(path); } catch (Exception e) { - Logger.Error(e, $@"Could not delete file at {path}"); + Logger.Error(e, $@"Could not delete original file after import ({Path.GetFileName(path)})"); } } catch (Exception e) From f5b0253e8234f142f61981bffa17b84acd5ae6ed Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 28 Jul 2017 15:36:23 +0900 Subject: [PATCH 10/17] Apply CI fixes --- osu.Game/Beatmaps/BeatmapManager.cs | 41 +++++++++++++++++++---------- 1 file changed, 27 insertions(+), 14 deletions(-) diff --git a/osu.Game/Beatmaps/BeatmapManager.cs b/osu.Game/Beatmaps/BeatmapManager.cs index 646ae3fae9..de65db1a5a 100644 --- a/osu.Game/Beatmaps/BeatmapManager.cs +++ b/osu.Game/Beatmaps/BeatmapManager.cs @@ -103,7 +103,7 @@ namespace osu.Game.Beatmaps } } - private object ImportLock = new object(); + private readonly object importLock = new object(); /// /// Import a beatmap from an . @@ -112,7 +112,7 @@ namespace osu.Game.Beatmaps public BeatmapSetInfo Import(ArchiveReader archiveReader) { // let's only allow one concurrent import at a time for now. - lock (ImportLock) + lock (importLock) { BeatmapSetInfo set = importToStorage(archiveReader); Import(set); @@ -192,7 +192,8 @@ namespace osu.Game.Beatmaps /// public void Reset() { - beatmaps.Reset(); + lock (beatmaps) + beatmaps.Reset(); } /// @@ -202,12 +203,15 @@ namespace osu.Game.Beatmaps /// The first result for the provided query, or null if no results were found. public BeatmapSetInfo QueryBeatmapSet(Func query) { - BeatmapSetInfo set = beatmaps.Query().FirstOrDefault(query); + lock (beatmaps) + { + BeatmapSetInfo set = beatmaps.Query().FirstOrDefault(query); - if (set != null) - beatmaps.Populate(set); + if (set != null) + beatmaps.Populate(set); - return set; + return set; + } } /// @@ -215,7 +219,10 @@ namespace osu.Game.Beatmaps /// /// The query. /// Results from the provided query. - public List QueryBeatmapSets(Expression> query) => beatmaps.QueryAndPopulate(query); + public List QueryBeatmapSets(Expression> query) + { + lock (beatmaps) return beatmaps.QueryAndPopulate(query); + } /// /// Perform a lookup query on available s. @@ -224,12 +231,15 @@ namespace osu.Game.Beatmaps /// The first result for the provided query, or null if no results were found. public BeatmapInfo QueryBeatmap(Func query) { - BeatmapInfo set = beatmaps.Query().FirstOrDefault(query); + lock (beatmaps) + { + BeatmapInfo set = beatmaps.Query().FirstOrDefault(query); - if (set != null) - beatmaps.Populate(set); + if (set != null) + beatmaps.Populate(set); - return set; + return set; + } } /// @@ -237,7 +247,10 @@ namespace osu.Game.Beatmaps /// /// The query. /// Results from the provided query. - public List QueryBeatmaps(Expression> query) => beatmaps.QueryAndPopulate(query); + public List QueryBeatmaps(Expression> query) + { + lock (beatmaps) return beatmaps.QueryAndPopulate(query); + } /// /// Creates an from a valid storage path. @@ -336,7 +349,7 @@ namespace osu.Game.Beatmaps /// A list of available . public List GetAllUsableBeatmapSets(bool populate = true) { - lock (this) + lock (beatmaps) { if (populate) return beatmaps.QueryAndPopulate(b => !b.DeletePending).ToList(); From c48bf3940e163df31e5291f8da5348ca02317f48 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 28 Jul 2017 16:55:58 +0900 Subject: [PATCH 11/17] Add a progress notification when importing beatmaps --- osu.Game/Beatmaps/BeatmapManager.cs | 29 ++++++++++++++++++++++++++--- osu.Game/OsuGameBase.cs | 2 +- 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/osu.Game/Beatmaps/BeatmapManager.cs b/osu.Game/Beatmaps/BeatmapManager.cs index de65db1a5a..f32dbc8322 100644 --- a/osu.Game/Beatmaps/BeatmapManager.cs +++ b/osu.Game/Beatmaps/BeatmapManager.cs @@ -17,6 +17,7 @@ using osu.Game.Beatmaps.Formats; using osu.Game.Beatmaps.IO; using osu.Game.IO; using osu.Game.IPC; +using osu.Game.Overlays.Notifications; using osu.Game.Rulesets; using SQLite.Net; using FileInfo = osu.Game.IO.FileInfo; @@ -48,13 +49,14 @@ namespace osu.Game.Beatmaps private readonly FileStore files; private readonly RulesetStore rulesets; + private readonly Action postNotification; private readonly BeatmapStore beatmaps; // ReSharper disable once NotAccessedField.Local (we should keep a reference to this so it is not finalised) private BeatmapIPCChannel ipc; - public BeatmapManager(Storage storage, FileStore files, SQLiteConnection connection, RulesetStore rulesets, IIpcHost importHost = null) + public BeatmapManager(Storage storage, FileStore files, SQLiteConnection connection, RulesetStore rulesets, IIpcHost importHost = null, Action postNotification = null) { beatmaps = new BeatmapStore(connection); beatmaps.BeatmapSetAdded += s => BeatmapSetAdded?.Invoke(s); @@ -63,24 +65,43 @@ namespace osu.Game.Beatmaps this.storage = storage; this.files = files; this.rulesets = rulesets; + this.postNotification = postNotification; if (importHost != null) ipc = new BeatmapIPCChannel(importHost, this); } /// - /// Import multiple from filesystem . + /// Import one or more from filesystem . + /// This will post a notification tracking import progress. /// - /// Multiple locations on disk. + /// One or more beatmap locations on disk. public void Import(params string[] paths) { + var notification = new ProgressNotification + { + Text = "Beatmap import is initialising...", + Progress = 0, + State = ProgressNotificationState.Active, + }; + + postNotification?.Invoke(notification); + + int i = 0; foreach (string path in paths) { + if (notification.State == ProgressNotificationState.Cancelled) + // user requested abort + return; + try { + notification.Text = $"Importing ({i} of {paths.Length})\n{Path.GetFileName(path)}"; using (ArchiveReader reader = getReaderFrom(path)) Import(reader); + notification.Progress = (float)++i / paths.Length; + // We may or may not want to delete the file depending on where it is stored. // e.g. reconstructing/repairing database with beatmaps from default storage. // Also, not always a single file, i.e. for LegacyFilesystemReader @@ -101,6 +122,8 @@ namespace osu.Game.Beatmaps Logger.Error(e, @"Could not import beatmap set"); } } + + notification.State = ProgressNotificationState.Completed; } private readonly object importLock = new object(); diff --git a/osu.Game/OsuGameBase.cs b/osu.Game/OsuGameBase.cs index 0dec4228de..8a09c7aac7 100644 --- a/osu.Game/OsuGameBase.cs +++ b/osu.Game/OsuGameBase.cs @@ -102,7 +102,7 @@ namespace osu.Game dependencies.Cache(RulesetStore = new RulesetStore(connection)); dependencies.Cache(FileStore = new FileStore(connection, Host.Storage)); - dependencies.Cache(BeatmapManager = new BeatmapManager(Host.Storage, FileStore, connection, RulesetStore, Host)); + dependencies.Cache(BeatmapManager = new BeatmapManager(Host.Storage, FileStore, connection, RulesetStore, Host, PostNotification)); dependencies.Cache(ScoreStore = new ScoreStore(Host.Storage, connection, Host, BeatmapManager)); dependencies.Cache(new OsuColour()); From df5094c0d4aab957f4894de87de27f77542e1b13 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 31 Jul 2017 18:03:55 +0900 Subject: [PATCH 12/17] Rework how notifications are distributed --- osu.Game/Beatmaps/BeatmapManager.cs | 11 +++++++---- osu.Game/OsuGame.cs | 3 +++ osu.Game/OsuGameBase.cs | 2 +- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/osu.Game/Beatmaps/BeatmapManager.cs b/osu.Game/Beatmaps/BeatmapManager.cs index f32dbc8322..c471502d88 100644 --- a/osu.Game/Beatmaps/BeatmapManager.cs +++ b/osu.Game/Beatmaps/BeatmapManager.cs @@ -49,14 +49,18 @@ namespace osu.Game.Beatmaps private readonly FileStore files; private readonly RulesetStore rulesets; - private readonly Action postNotification; private readonly BeatmapStore beatmaps; // ReSharper disable once NotAccessedField.Local (we should keep a reference to this so it is not finalised) private BeatmapIPCChannel ipc; - public BeatmapManager(Storage storage, FileStore files, SQLiteConnection connection, RulesetStore rulesets, IIpcHost importHost = null, Action postNotification = null) + /// + /// Set an endpoint for notifications to be posted to. + /// + public Action PostNotification { private get; set; } + + public BeatmapManager(Storage storage, FileStore files, SQLiteConnection connection, RulesetStore rulesets, IIpcHost importHost = null) { beatmaps = new BeatmapStore(connection); beatmaps.BeatmapSetAdded += s => BeatmapSetAdded?.Invoke(s); @@ -65,7 +69,6 @@ namespace osu.Game.Beatmaps this.storage = storage; this.files = files; this.rulesets = rulesets; - this.postNotification = postNotification; if (importHost != null) ipc = new BeatmapIPCChannel(importHost, this); @@ -85,7 +88,7 @@ namespace osu.Game.Beatmaps State = ProgressNotificationState.Active, }; - postNotification?.Invoke(notification); + PostNotification?.Invoke(notification); int i = 0; foreach (string path in paths) diff --git a/osu.Game/OsuGame.cs b/osu.Game/OsuGame.cs index fe6d2dbb41..4082ed4ecd 100644 --- a/osu.Game/OsuGame.cs +++ b/osu.Game/OsuGame.cs @@ -149,6 +149,9 @@ namespace osu.Game { base.LoadComplete(); + // hook up notifications to components. + BeatmapManager.PostNotification = n => notificationOverlay?.Post(n); + AddRange(new Drawable[] { new VolumeControlReceptor { diff --git a/osu.Game/OsuGameBase.cs b/osu.Game/OsuGameBase.cs index 8a09c7aac7..0dec4228de 100644 --- a/osu.Game/OsuGameBase.cs +++ b/osu.Game/OsuGameBase.cs @@ -102,7 +102,7 @@ namespace osu.Game dependencies.Cache(RulesetStore = new RulesetStore(connection)); dependencies.Cache(FileStore = new FileStore(connection, Host.Storage)); - dependencies.Cache(BeatmapManager = new BeatmapManager(Host.Storage, FileStore, connection, RulesetStore, Host, PostNotification)); + dependencies.Cache(BeatmapManager = new BeatmapManager(Host.Storage, FileStore, connection, RulesetStore, Host)); dependencies.Cache(ScoreStore = new ScoreStore(Host.Storage, connection, Host, BeatmapManager)); dependencies.Cache(new OsuColour()); From 92b3c7ac08c260fc0c1f5afa3306cbba4157fed2 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 31 Jul 2017 18:38:42 +0900 Subject: [PATCH 13/17] Fix the whole database being retrieved when importing each beatmap --- osu.Game/Beatmaps/BeatmapManager.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Beatmaps/BeatmapManager.cs b/osu.Game/Beatmaps/BeatmapManager.cs index c471502d88..8cced2d5f4 100644 --- a/osu.Game/Beatmaps/BeatmapManager.cs +++ b/osu.Game/Beatmaps/BeatmapManager.cs @@ -308,7 +308,7 @@ namespace osu.Game.Beatmaps var hash = hashable.ComputeSHA2Hash(); // check if this beatmap has already been imported and exit early if so. - var beatmapSet = beatmaps.QueryAndPopulate().FirstOrDefault(b => b.Hash == hash); + var beatmapSet = beatmaps.QueryAndPopulate(b => b.Hash == hash).FirstOrDefault(); if (beatmapSet != null) { Undelete(beatmapSet); From bc8f8de0495e6b8702e800b73456128ecc3f0a2f Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 31 Jul 2017 18:41:54 +0900 Subject: [PATCH 14/17] Make QueryAndPopulate's filter non-optional (you basically *never* want this missing) --- osu.Game/Database/DatabaseBackedStore.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game/Database/DatabaseBackedStore.cs b/osu.Game/Database/DatabaseBackedStore.cs index bb61fc1870..bd25acc41b 100644 --- a/osu.Game/Database/DatabaseBackedStore.cs +++ b/osu.Game/Database/DatabaseBackedStore.cs @@ -83,9 +83,9 @@ namespace osu.Game.Database /// /// Query and populate results. /// - /// An optional filter to refine results. + /// An filter to refine results. /// - public List QueryAndPopulate(Expression> filter = null) + public List QueryAndPopulate(Expression> filter) where T : class { checkType(typeof(T)); From f67822a59b9256f221de0461806df238073f8063 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 31 Jul 2017 18:52:59 +0900 Subject: [PATCH 15/17] Add progress for deleting all maps --- osu.Game/Beatmaps/BeatmapManager.cs | 30 +++++++++++++++++++ .../Sections/Maintenance/GeneralSettings.cs | 6 +--- 2 files changed, 31 insertions(+), 5 deletions(-) diff --git a/osu.Game/Beatmaps/BeatmapManager.cs b/osu.Game/Beatmaps/BeatmapManager.cs index 8cced2d5f4..3890abd341 100644 --- a/osu.Game/Beatmaps/BeatmapManager.cs +++ b/osu.Game/Beatmaps/BeatmapManager.cs @@ -459,5 +459,35 @@ namespace osu.Game.Beatmaps Import(Directory.GetDirectories(stableInstallPath)); } + + public void DeleteAll() + { + var maps = GetAllUsableBeatmapSets().ToArray(); + + if (maps.Length == 0) return; + + var notification = new ProgressNotification + { + Progress = 0, + State = ProgressNotificationState.Active, + }; + + PostNotification?.Invoke(notification); + + int i = 0; + + foreach (var b in maps) + { + if (notification.State == ProgressNotificationState.Cancelled) + // user requested abort + return; + + notification.Text = $"Deleting ({i} of {maps.Length})"; + notification.Progress = (float)++i / maps.Length; + Delete(b); + } + + notification.State = ProgressNotificationState.Completed; + } } } diff --git a/osu.Game/Overlays/Settings/Sections/Maintenance/GeneralSettings.cs b/osu.Game/Overlays/Settings/Sections/Maintenance/GeneralSettings.cs index 684bfcb39d..9d13a2ae2f 100644 --- a/osu.Game/Overlays/Settings/Sections/Maintenance/GeneralSettings.cs +++ b/osu.Game/Overlays/Settings/Sections/Maintenance/GeneralSettings.cs @@ -38,11 +38,7 @@ namespace osu.Game.Overlays.Settings.Sections.Maintenance Action = () => { deleteButton.Enabled.Value = false; - Task.Run(() => - { - foreach (var b in beatmaps.GetAllUsableBeatmapSets()) - beatmaps.Delete(b); - }).ContinueWith(t => Schedule(() => deleteButton.Enabled.Value = true)); + Task.Run(() => beatmaps.DeleteAll()).ContinueWith(t => Schedule(() => deleteButton.Enabled.Value = true)); } }, }; From 6af0629cc0ea8db5b86018a5d51e3374e531bbe6 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 1 Aug 2017 09:37:33 +0900 Subject: [PATCH 16/17] Remove unnecessary newline --- osu.Game/Beatmaps/BeatmapManager.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/osu.Game/Beatmaps/BeatmapManager.cs b/osu.Game/Beatmaps/BeatmapManager.cs index 3890abd341..784d440584 100644 --- a/osu.Game/Beatmaps/BeatmapManager.cs +++ b/osu.Game/Beatmaps/BeatmapManager.cs @@ -446,7 +446,6 @@ namespace osu.Game.Beatmaps public void ImportFromStable() { - string stableInstallPath = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.LocalApplicationData), @"osu!", "Songs"); if (!Directory.Exists(stableInstallPath)) stableInstallPath = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.UserProfile), ".osu", "Songs"); From ed3e78452e6e01ef7d0a82dbd4687ce259b3932b Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 1 Aug 2017 09:38:43 +0900 Subject: [PATCH 17/17] Lock beatmaps for good measure --- osu.Game/Beatmaps/BeatmapManager.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/osu.Game/Beatmaps/BeatmapManager.cs b/osu.Game/Beatmaps/BeatmapManager.cs index 784d440584..a23fab5393 100644 --- a/osu.Game/Beatmaps/BeatmapManager.cs +++ b/osu.Game/Beatmaps/BeatmapManager.cs @@ -308,7 +308,10 @@ namespace osu.Game.Beatmaps var hash = hashable.ComputeSHA2Hash(); // check if this beatmap has already been imported and exit early if so. - var beatmapSet = beatmaps.QueryAndPopulate(b => b.Hash == hash).FirstOrDefault(); + BeatmapSetInfo beatmapSet; + lock (beatmaps) + beatmapSet = beatmaps.QueryAndPopulate(b => b.Hash == hash).FirstOrDefault(); + if (beatmapSet != null) { Undelete(beatmapSet);