From b4de51b612c62b563ec1303b84dead7a219be938 Mon Sep 17 00:00:00 2001 From: naoey Date: Tue, 11 Jun 2019 18:29:33 +0530 Subject: [PATCH 01/25] Create a generic base archive download manager class --- .../Database/ArchiveDownloadModelManager.cs | 117 ++++++++++++++++++ .../Online/API/ArchiveDownloadModelRequest.cs | 23 ++++ 2 files changed, 140 insertions(+) create mode 100644 osu.Game/Database/ArchiveDownloadModelManager.cs create mode 100644 osu.Game/Online/API/ArchiveDownloadModelRequest.cs diff --git a/osu.Game/Database/ArchiveDownloadModelManager.cs b/osu.Game/Database/ArchiveDownloadModelManager.cs new file mode 100644 index 0000000000..8c7a0fba87 --- /dev/null +++ b/osu.Game/Database/ArchiveDownloadModelManager.cs @@ -0,0 +1,117 @@ +using osu.Framework.Logging; +using osu.Framework.Platform; +using osu.Game.Online.API; +using osu.Game.Overlays.Notifications; +using System; +using System.Collections.Generic; +using System.Threading.Tasks; + +namespace osu.Game.Database +{ + public abstract class ArchiveDownloadModelManager : ArchiveModelManager + where TModel : class, IHasFiles, IHasPrimaryKey, ISoftDelete + where TFileModel : INamedFileInfo, new() + where TDownloadRequestModel : ArchiveDownloadModelRequest + { + public event Action DownloadBegan; + + public event Action DownloadFailed; + + private readonly IAPIProvider api; + + private readonly List currentDownloads = new List(); + + protected ArchiveDownloadModelManager(Storage storage, IDatabaseContextFactory contextFactory, IAPIProvider api, MutableDatabaseBackedStoreWithFileIncludes modelStore, IIpcHost importHost = null) + :base(storage, contextFactory, modelStore, importHost) + { + this.api = api; + } + + protected abstract TDownloadRequestModel CreateDownloadRequest(TModel model); + + public bool Download(TModel model) + { + var existing = GetExistingDownload(model); + + if (existing != null || api == null) return false; + + DownloadNotification notification = new DownloadNotification + { + Text = $"Downloading {model}", + }; + + var request = CreateDownloadRequest(model); + + request.DownloadProgressed += progress => + { + notification.State = ProgressNotificationState.Active; + notification.Progress = progress; + }; + + request.Success += filename => + { + Task.Factory.StartNew(() => + { + Import(notification, filename); + currentDownloads.Remove(request); + }, TaskCreationOptions.LongRunning); + }; + + request.Failure += error => + { + DownloadFailed?.Invoke(request); + + if (error is OperationCanceledException) return; + + notification.State = ProgressNotificationState.Cancelled; + // TODO: implement a Name for every model that we can use in this message + Logger.Error(error, "Download failed!"); + currentDownloads.Remove(request); + }; + + notification.CancelRequested += () => + { + request.Cancel(); + currentDownloads.Remove(request); + notification.State = ProgressNotificationState.Cancelled; + return true; + }; + + currentDownloads.Add(request); + PostNotification?.Invoke(notification); + + Task.Factory.StartNew(() => + { + try + { + request.Perform(api); + } + catch + { + } + }, TaskCreationOptions.LongRunning); + + DownloadBegan?.Invoke(request); + + return true; + } + + public TDownloadRequestModel GetExistingDownload(TModel model) => currentDownloads.Find(r => r.Info.Equals(model)); + + private class DownloadNotification : ProgressNotification + { + public override bool IsImportant => false; + + protected override Notification CreateCompletionNotification() => new SilencedProgressCompletionNotification + { + Activated = CompletionClickAction, + Text = CompletionText + }; + + private class SilencedProgressCompletionNotification : ProgressCompletionNotification + { + public override bool IsImportant => false; + } + } + } +} diff --git a/osu.Game/Online/API/ArchiveDownloadModelRequest.cs b/osu.Game/Online/API/ArchiveDownloadModelRequest.cs new file mode 100644 index 0000000000..377166e657 --- /dev/null +++ b/osu.Game/Online/API/ArchiveDownloadModelRequest.cs @@ -0,0 +1,23 @@ +using System; +using System.Collections.Generic; +using System.Text; + +namespace osu.Game.Online.API +{ + public abstract class ArchiveDownloadModelRequest : APIDownloadRequest + where TModel : class + { + public readonly TModel Info; + + public float Progress; + + public event Action DownloadProgressed; + + protected ArchiveDownloadModelRequest(TModel model) + { + Info = model; + + Progressed += (current, total) => DownloadProgressed?.Invoke(Progress = (float)current / total); + } + } +} From 341d137f5c10c9e575419100e355fc6d312c0103 Mon Sep 17 00:00:00 2001 From: naoey Date: Tue, 11 Jun 2019 19:36:08 +0530 Subject: [PATCH 02/25] Make BeatmapManager inherit from new base class --- osu.Game/Beatmaps/BeatmapManager.cs | 113 +----------------- .../Database/ArchiveDownloadModelManager.cs | 29 +++-- .../Online/API/ArchiveDownloadModelRequest.cs | 2 - .../API/Requests/DownloadBeatmapSetRequest.cs | 10 +- .../Direct/DownloadTrackingComposite.cs | 4 +- 5 files changed, 25 insertions(+), 133 deletions(-) diff --git a/osu.Game/Beatmaps/BeatmapManager.cs b/osu.Game/Beatmaps/BeatmapManager.cs index b6fe7f88fa..c4975501ed 100644 --- a/osu.Game/Beatmaps/BeatmapManager.cs +++ b/osu.Game/Beatmaps/BeatmapManager.cs @@ -27,7 +27,7 @@ namespace osu.Game.Beatmaps /// /// Handles the storage and retrieval of Beatmaps/WorkingBeatmaps. /// - public partial class BeatmapManager : ArchiveModelManager + public partial class BeatmapManager : ArchiveDownloadModelManager { /// /// Fired when a single difficulty has been hidden. @@ -39,16 +39,6 @@ namespace osu.Game.Beatmaps /// public event Action BeatmapRestored; - /// - /// Fired when a beatmap download begins. - /// - public event Action BeatmapDownloadBegan; - - /// - /// Fired when a beatmap download is interrupted, due to user cancellation or other failures. - /// - public event Action BeatmapDownloadFailed; - /// /// A default representation of a WorkingBeatmap to use when no beatmap is available. /// @@ -74,7 +64,7 @@ namespace osu.Game.Beatmaps public BeatmapManager(Storage storage, IDatabaseContextFactory contextFactory, RulesetStore rulesets, IAPIProvider api, AudioManager audioManager, GameHost host = null, WorkingBeatmap defaultBeatmap = null) - : base(storage, contextFactory, new BeatmapStore(contextFactory), host) + : base(storage, contextFactory, api, new BeatmapStore(contextFactory), host) { this.rulesets = rulesets; this.api = api; @@ -88,6 +78,8 @@ namespace osu.Game.Beatmaps beatmaps.BeatmapRestored += b => BeatmapRestored?.Invoke(b); } + protected override DownloadBeatmapSetRequest CreateDownloadRequest(BeatmapSetInfo set) => new DownloadBeatmapSetRequest(set, false); + protected override void Populate(BeatmapSetInfo beatmapSet, ArchiveReader archive) { if (archive != null) @@ -153,87 +145,6 @@ namespace osu.Game.Beatmaps void resetIds() => beatmapSet.Beatmaps.ForEach(b => b.OnlineBeatmapID = null); } - /// - /// Downloads a beatmap. - /// This will post notifications tracking progress. - /// - /// The to be downloaded. - /// Whether the beatmap should be downloaded without video. Defaults to false. - /// Downloading can happen - public bool Download(BeatmapSetInfo beatmapSetInfo, bool noVideo = false) - { - var existing = GetExistingDownload(beatmapSetInfo); - - if (existing != null || api == null) return false; - - var downloadNotification = new DownloadNotification - { - Text = $"Downloading {beatmapSetInfo}", - }; - - var request = new DownloadBeatmapSetRequest(beatmapSetInfo, noVideo); - - request.DownloadProgressed += progress => - { - downloadNotification.State = ProgressNotificationState.Active; - downloadNotification.Progress = progress; - }; - - request.Success += filename => - { - Task.Factory.StartNew(() => - { - // This gets scheduled back to the update thread, but we want the import to run in the background. - Import(downloadNotification, filename); - currentDownloads.Remove(request); - }, TaskCreationOptions.LongRunning); - }; - - request.Failure += error => - { - BeatmapDownloadFailed?.Invoke(request); - - if (error is OperationCanceledException) return; - - downloadNotification.State = ProgressNotificationState.Cancelled; - Logger.Error(error, "Beatmap download failed!"); - currentDownloads.Remove(request); - }; - - downloadNotification.CancelRequested += () => - { - request.Cancel(); - currentDownloads.Remove(request); - downloadNotification.State = ProgressNotificationState.Cancelled; - return true; - }; - - currentDownloads.Add(request); - PostNotification?.Invoke(downloadNotification); - - // don't run in the main api queue as this is a long-running task. - Task.Factory.StartNew(() => - { - try - { - request.Perform(api); - } - catch - { - // no need to handle here as exceptions will filter down to request.Failure above. - } - }, TaskCreationOptions.LongRunning); - BeatmapDownloadBegan?.Invoke(request); - return true; - } - - /// - /// Get an existing download request if it exists. - /// - /// The whose download request is wanted. - /// The object if it exists, or null. - public DownloadBeatmapSetRequest GetExistingDownload(BeatmapSetInfo beatmap) => currentDownloads.Find(d => d.BeatmapSet.OnlineBeatmapSetID == beatmap.OnlineBeatmapSetID); - /// /// Delete a beatmap difficulty. /// @@ -439,21 +350,5 @@ namespace osu.Game.Beatmaps protected override Texture GetBackground() => null; protected override Track GetTrack() => null; } - - private class DownloadNotification : ProgressNotification - { - public override bool IsImportant => false; - - protected override Notification CreateCompletionNotification() => new SilencedProgressCompletionNotification - { - Activated = CompletionClickAction, - Text = CompletionText - }; - - private class SilencedProgressCompletionNotification : ProgressCompletionNotification - { - public override bool IsImportant => false; - } - } } } diff --git a/osu.Game/Database/ArchiveDownloadModelManager.cs b/osu.Game/Database/ArchiveDownloadModelManager.cs index 8c7a0fba87..6580da0d54 100644 --- a/osu.Game/Database/ArchiveDownloadModelManager.cs +++ b/osu.Game/Database/ArchiveDownloadModelManager.cs @@ -31,17 +31,26 @@ namespace osu.Game.Database public bool Download(TModel model) { - var existing = GetExistingDownload(model); - - if (existing != null || api == null) return false; - - DownloadNotification notification = new DownloadNotification - { - Text = $"Downloading {model}", - }; + if (!canDownload(model)) return false; var request = CreateDownloadRequest(model); + performDownloadWithRequest(request); + + return true; + } + + public TDownloadRequestModel GetExistingDownload(TModel model) => currentDownloads.Find(r => r.Info.Equals(model)); + + private bool canDownload(TModel model) => GetExistingDownload(model) == null && api != null; + + private void performDownloadWithRequest(TDownloadRequestModel request) + { + DownloadNotification notification = new DownloadNotification + { + Text = $"Downloading {request.Info}", + }; + request.DownloadProgressed += progress => { notification.State = ProgressNotificationState.Active; @@ -92,12 +101,8 @@ namespace osu.Game.Database }, TaskCreationOptions.LongRunning); DownloadBegan?.Invoke(request); - - return true; } - public TDownloadRequestModel GetExistingDownload(TModel model) => currentDownloads.Find(r => r.Info.Equals(model)); - private class DownloadNotification : ProgressNotification { public override bool IsImportant => false; diff --git a/osu.Game/Online/API/ArchiveDownloadModelRequest.cs b/osu.Game/Online/API/ArchiveDownloadModelRequest.cs index 377166e657..862d017228 100644 --- a/osu.Game/Online/API/ArchiveDownloadModelRequest.cs +++ b/osu.Game/Online/API/ArchiveDownloadModelRequest.cs @@ -1,6 +1,4 @@ using System; -using System.Collections.Generic; -using System.Text; namespace osu.Game.Online.API { diff --git a/osu.Game/Online/API/Requests/DownloadBeatmapSetRequest.cs b/osu.Game/Online/API/Requests/DownloadBeatmapSetRequest.cs index 26e8acc2fc..7d0a8f9f46 100644 --- a/osu.Game/Online/API/Requests/DownloadBeatmapSetRequest.cs +++ b/osu.Game/Online/API/Requests/DownloadBeatmapSetRequest.cs @@ -2,26 +2,20 @@ // See the LICENCE file in the repository root for full licence text. using osu.Game.Beatmaps; -using System; namespace osu.Game.Online.API.Requests { - public class DownloadBeatmapSetRequest : APIDownloadRequest + public class DownloadBeatmapSetRequest : ArchiveDownloadModelRequest { public readonly BeatmapSetInfo BeatmapSet; - public float Progress; - - public event Action DownloadProgressed; - private readonly bool noVideo; public DownloadBeatmapSetRequest(BeatmapSetInfo set, bool noVideo) + : base(set) { this.noVideo = noVideo; BeatmapSet = set; - - Progressed += (current, total) => DownloadProgressed?.Invoke(Progress = (float)current / total); } protected override string Target => $@"beatmapsets/{BeatmapSet.OnlineBeatmapSetID}/download{(noVideo ? "?noVideo=1" : "")}"; diff --git a/osu.Game/Overlays/Direct/DownloadTrackingComposite.cs b/osu.Game/Overlays/Direct/DownloadTrackingComposite.cs index 37f13aefc8..9beedb195f 100644 --- a/osu.Game/Overlays/Direct/DownloadTrackingComposite.cs +++ b/osu.Game/Overlays/Direct/DownloadTrackingComposite.cs @@ -47,7 +47,7 @@ namespace osu.Game.Overlays.Direct attachDownload(beatmaps.GetExistingDownload(setInfo.NewValue)); }, true); - beatmaps.BeatmapDownloadBegan += download => + beatmaps.DownloadBegan += download => { if (download.BeatmapSet.OnlineBeatmapSetID == BeatmapSet.Value?.OnlineBeatmapSetID) attachDownload(download); @@ -65,7 +65,7 @@ namespace osu.Game.Overlays.Direct if (beatmaps != null) { - beatmaps.BeatmapDownloadBegan -= attachDownload; + beatmaps.DownloadBegan -= attachDownload; beatmaps.ItemAdded -= setAdded; } From 8ff26a8fbcf7ef1f1d3483d4dd0047807ef05ff1 Mon Sep 17 00:00:00 2001 From: naoey Date: Tue, 11 Jun 2019 19:49:10 +0530 Subject: [PATCH 03/25] Add license headers and xmldoc --- .../Database/ArchiveDownloadModelManager.cs | 31 +++++++++++++++++-- .../Online/API/ArchiveDownloadModelRequest.cs | 5 ++- 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/osu.Game/Database/ArchiveDownloadModelManager.cs b/osu.Game/Database/ArchiveDownloadModelManager.cs index 6580da0d54..d0881fb251 100644 --- a/osu.Game/Database/ArchiveDownloadModelManager.cs +++ b/osu.Game/Database/ArchiveDownloadModelManager.cs @@ -1,4 +1,7 @@ -using osu.Framework.Logging; +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using osu.Framework.Logging; using osu.Framework.Platform; using osu.Game.Online.API; using osu.Game.Overlays.Notifications; @@ -8,13 +11,26 @@ using System.Threading.Tasks; namespace osu.Game.Database { + /// + /// An that has the ability to download models using an and + /// import them into the store. + /// + /// The model type. + /// The associated file join type. + /// The associated for this model. public abstract class ArchiveDownloadModelManager : ArchiveModelManager where TModel : class, IHasFiles, IHasPrimaryKey, ISoftDelete where TFileModel : INamedFileInfo, new() where TDownloadRequestModel : ArchiveDownloadModelRequest { + /// + /// Fired when a download begins. + /// public event Action DownloadBegan; + /// + /// Fired when a download is interrupted, either due to user cancellation or failure. + /// public event Action DownloadFailed; private readonly IAPIProvider api; @@ -29,6 +45,12 @@ namespace osu.Game.Database protected abstract TDownloadRequestModel CreateDownloadRequest(TModel model); + /// + /// Downloads a . + /// This will post notifications tracking progress. + /// + /// The to be downloaded. + /// Whether downloading can happen. public bool Download(TModel model) { if (!canDownload(model)) return false; @@ -40,6 +62,11 @@ namespace osu.Game.Database return true; } + /// + /// Gets an existing download request if it exists. + /// + /// The whose request is wanted. + /// The object if it exists, otherwise null. public TDownloadRequestModel GetExistingDownload(TModel model) => currentDownloads.Find(r => r.Info.Equals(model)); private bool canDownload(TModel model) => GetExistingDownload(model) == null && api != null; @@ -73,7 +100,7 @@ namespace osu.Game.Database if (error is OperationCanceledException) return; notification.State = ProgressNotificationState.Cancelled; - // TODO: implement a Name for every model that we can use in this message + // TODO: maybe implement a Name for every model that we can use in this message? Logger.Error(error, "Download failed!"); currentDownloads.Remove(request); }; diff --git a/osu.Game/Online/API/ArchiveDownloadModelRequest.cs b/osu.Game/Online/API/ArchiveDownloadModelRequest.cs index 862d017228..7f161f1e98 100644 --- a/osu.Game/Online/API/ArchiveDownloadModelRequest.cs +++ b/osu.Game/Online/API/ArchiveDownloadModelRequest.cs @@ -1,4 +1,7 @@ -using System; +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System; namespace osu.Game.Online.API { From 802f48712d95b148efc45ae25bf8fa4fdc040423 Mon Sep 17 00:00:00 2001 From: naoey Date: Tue, 11 Jun 2019 20:14:36 +0530 Subject: [PATCH 04/25] Add ability to perform a download request with options --- osu.Game/Beatmaps/BeatmapManager.cs | 2 +- .../Database/ArchiveDownloadModelManager.cs | 29 +++++++++++++++++-- 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/osu.Game/Beatmaps/BeatmapManager.cs b/osu.Game/Beatmaps/BeatmapManager.cs index c4975501ed..6f27cbd7c6 100644 --- a/osu.Game/Beatmaps/BeatmapManager.cs +++ b/osu.Game/Beatmaps/BeatmapManager.cs @@ -78,7 +78,7 @@ namespace osu.Game.Beatmaps beatmaps.BeatmapRestored += b => BeatmapRestored?.Invoke(b); } - protected override DownloadBeatmapSetRequest CreateDownloadRequest(BeatmapSetInfo set) => new DownloadBeatmapSetRequest(set, false); + protected override DownloadBeatmapSetRequest CreateDownloadRequest(BeatmapSetInfo set, object[] options) => new DownloadBeatmapSetRequest(set, (options?.FirstOrDefault() as bool?) ?? false); protected override void Populate(BeatmapSetInfo beatmapSet, ArchiveReader archive) { diff --git a/osu.Game/Database/ArchiveDownloadModelManager.cs b/osu.Game/Database/ArchiveDownloadModelManager.cs index d0881fb251..a4d2180559 100644 --- a/osu.Game/Database/ArchiveDownloadModelManager.cs +++ b/osu.Game/Database/ArchiveDownloadModelManager.cs @@ -43,7 +43,15 @@ namespace osu.Game.Database this.api = api; } - protected abstract TDownloadRequestModel CreateDownloadRequest(TModel model); + /// + /// Creates the download request for this . + /// The parameters will be provided when the download was initiated with extra options meant + /// to be used in the creation of the request. + /// + /// The to be downloaded. + /// Extra parameters for request creation, null if none were passed. + /// The request object. + protected abstract TDownloadRequestModel CreateDownloadRequest(TModel model, object[] options); /// /// Downloads a . @@ -55,7 +63,24 @@ namespace osu.Game.Database { if (!canDownload(model)) return false; - var request = CreateDownloadRequest(model); + var request = CreateDownloadRequest(model, null); + + performDownloadWithRequest(request); + + return true; + } + + /// + /// Downloads a with optional parameters for the download request. + /// + /// The to be downloaded. + /// Optional parameters to be used for creating the download request. + /// Whether downloading can happen. + public bool Download(TModel model, params object[] extra) + { + if (!canDownload(model)) return false; + + var request = CreateDownloadRequest(model, extra); performDownloadWithRequest(request); From 709ca03a08a3cad591a3911b01734fd100ad3923 Mon Sep 17 00:00:00 2001 From: naoey Date: Tue, 11 Jun 2019 20:21:06 +0530 Subject: [PATCH 05/25] Remove unused usings --- osu.Game/Beatmaps/BeatmapManager.cs | 2 -- osu.Game/Database/ArchiveDownloadModelManager.cs | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/osu.Game/Beatmaps/BeatmapManager.cs b/osu.Game/Beatmaps/BeatmapManager.cs index 6f27cbd7c6..6ad5ce6617 100644 --- a/osu.Game/Beatmaps/BeatmapManager.cs +++ b/osu.Game/Beatmaps/BeatmapManager.cs @@ -6,7 +6,6 @@ using System.Collections.Generic; using System.IO; using System.Linq; using System.Linq.Expressions; -using System.Threading.Tasks; using Microsoft.EntityFrameworkCore; using osu.Framework.Audio; using osu.Framework.Audio.Track; @@ -19,7 +18,6 @@ using osu.Game.Database; using osu.Game.IO.Archives; using osu.Game.Online.API; using osu.Game.Online.API.Requests; -using osu.Game.Overlays.Notifications; using osu.Game.Rulesets; namespace osu.Game.Beatmaps diff --git a/osu.Game/Database/ArchiveDownloadModelManager.cs b/osu.Game/Database/ArchiveDownloadModelManager.cs index a4d2180559..629cb81440 100644 --- a/osu.Game/Database/ArchiveDownloadModelManager.cs +++ b/osu.Game/Database/ArchiveDownloadModelManager.cs @@ -38,7 +38,7 @@ namespace osu.Game.Database private readonly List currentDownloads = new List(); protected ArchiveDownloadModelManager(Storage storage, IDatabaseContextFactory contextFactory, IAPIProvider api, MutableDatabaseBackedStoreWithFileIncludes modelStore, IIpcHost importHost = null) - :base(storage, contextFactory, modelStore, importHost) + : base(storage, contextFactory, modelStore, importHost) { this.api = api; } From f4dab4da85df762b3998c1ad4d157529a42d031b Mon Sep 17 00:00:00 2001 From: naoey Date: Tue, 11 Jun 2019 20:53:44 +0530 Subject: [PATCH 06/25] Add method to check if model exists locally already --- osu.Game/Database/ArchiveDownloadModelManager.cs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/osu.Game/Database/ArchiveDownloadModelManager.cs b/osu.Game/Database/ArchiveDownloadModelManager.cs index 629cb81440..3c0de0b566 100644 --- a/osu.Game/Database/ArchiveDownloadModelManager.cs +++ b/osu.Game/Database/ArchiveDownloadModelManager.cs @@ -7,6 +7,7 @@ using osu.Game.Online.API; using osu.Game.Overlays.Notifications; using System; using System.Collections.Generic; +using System.Linq; using System.Threading.Tasks; namespace osu.Game.Database @@ -37,10 +38,13 @@ namespace osu.Game.Database private readonly List currentDownloads = new List(); + private readonly MutableDatabaseBackedStoreWithFileIncludes modelStore; + protected ArchiveDownloadModelManager(Storage storage, IDatabaseContextFactory contextFactory, IAPIProvider api, MutableDatabaseBackedStoreWithFileIncludes modelStore, IIpcHost importHost = null) : base(storage, contextFactory, modelStore, importHost) { this.api = api; + this.modelStore = modelStore; } /// @@ -70,6 +74,13 @@ namespace osu.Game.Database return true; } + /// + /// Checks whether a given is available in the local store already. + /// + /// The whose existence needs to be checked. + /// Whether the exists locally. + public bool IsAvailableLocally(TModel model) => modelStore.ConsumableItems.Any(m => m.Equals(model)); + /// /// Downloads a with optional parameters for the download request. /// From 06a558c4b7127107b494d8dfe9db3f50263e86e5 Mon Sep 17 00:00:00 2001 From: naoey Date: Tue, 11 Jun 2019 21:11:30 +0530 Subject: [PATCH 07/25] Remove unecessary third generic and change usages to match --- osu.Game/Beatmaps/BeatmapManager.cs | 6 ++---- .../Database/ArchiveDownloadModelManager.cs | 18 ++++++++---------- .../API/Requests/DownloadBeatmapSetRequest.cs | 7 +++---- .../Direct/DownloadTrackingComposite.cs | 8 ++++---- 4 files changed, 17 insertions(+), 22 deletions(-) diff --git a/osu.Game/Beatmaps/BeatmapManager.cs b/osu.Game/Beatmaps/BeatmapManager.cs index 6ad5ce6617..2dcd1b137c 100644 --- a/osu.Game/Beatmaps/BeatmapManager.cs +++ b/osu.Game/Beatmaps/BeatmapManager.cs @@ -25,7 +25,7 @@ namespace osu.Game.Beatmaps /// /// Handles the storage and retrieval of Beatmaps/WorkingBeatmaps. /// - public partial class BeatmapManager : ArchiveDownloadModelManager + public partial class BeatmapManager : ArchiveDownloadModelManager { /// /// Fired when a single difficulty has been hidden. @@ -58,8 +58,6 @@ namespace osu.Game.Beatmaps private readonly GameHost host; - private readonly List currentDownloads = new List(); - public BeatmapManager(Storage storage, IDatabaseContextFactory contextFactory, RulesetStore rulesets, IAPIProvider api, AudioManager audioManager, GameHost host = null, WorkingBeatmap defaultBeatmap = null) : base(storage, contextFactory, api, new BeatmapStore(contextFactory), host) @@ -76,7 +74,7 @@ namespace osu.Game.Beatmaps beatmaps.BeatmapRestored += b => BeatmapRestored?.Invoke(b); } - protected override DownloadBeatmapSetRequest CreateDownloadRequest(BeatmapSetInfo set, object[] options) => new DownloadBeatmapSetRequest(set, (options?.FirstOrDefault() as bool?) ?? false); + protected override ArchiveDownloadModelRequest CreateDownloadRequest(BeatmapSetInfo set, object[] options) => new DownloadBeatmapSetRequest(set, (options?.FirstOrDefault() as bool?) ?? false); protected override void Populate(BeatmapSetInfo beatmapSet, ArchiveReader archive) { diff --git a/osu.Game/Database/ArchiveDownloadModelManager.cs b/osu.Game/Database/ArchiveDownloadModelManager.cs index 3c0de0b566..64920710bc 100644 --- a/osu.Game/Database/ArchiveDownloadModelManager.cs +++ b/osu.Game/Database/ArchiveDownloadModelManager.cs @@ -18,25 +18,23 @@ namespace osu.Game.Database /// /// The model type. /// The associated file join type. - /// The associated for this model. - public abstract class ArchiveDownloadModelManager : ArchiveModelManager + public abstract class ArchiveDownloadModelManager : ArchiveModelManager where TModel : class, IHasFiles, IHasPrimaryKey, ISoftDelete where TFileModel : INamedFileInfo, new() - where TDownloadRequestModel : ArchiveDownloadModelRequest { /// /// Fired when a download begins. /// - public event Action DownloadBegan; + public event Action> DownloadBegan; /// /// Fired when a download is interrupted, either due to user cancellation or failure. /// - public event Action DownloadFailed; + public event Action> DownloadFailed; private readonly IAPIProvider api; - private readonly List currentDownloads = new List(); + private readonly List> currentDownloads = new List>(); private readonly MutableDatabaseBackedStoreWithFileIncludes modelStore; @@ -55,7 +53,7 @@ namespace osu.Game.Database /// The to be downloaded. /// Extra parameters for request creation, null if none were passed. /// The request object. - protected abstract TDownloadRequestModel CreateDownloadRequest(TModel model, object[] options); + protected abstract ArchiveDownloadModelRequest CreateDownloadRequest(TModel model, object[] options); /// /// Downloads a . @@ -102,12 +100,12 @@ namespace osu.Game.Database /// Gets an existing download request if it exists. /// /// The whose request is wanted. - /// The object if it exists, otherwise null. - public TDownloadRequestModel GetExistingDownload(TModel model) => currentDownloads.Find(r => r.Info.Equals(model)); + /// The object if it exists, otherwise null. + public ArchiveDownloadModelRequest GetExistingDownload(TModel model) => currentDownloads.Find(r => r.Info.Equals(model)); private bool canDownload(TModel model) => GetExistingDownload(model) == null && api != null; - private void performDownloadWithRequest(TDownloadRequestModel request) + private void performDownloadWithRequest(ArchiveDownloadModelRequest request) { DownloadNotification notification = new DownloadNotification { diff --git a/osu.Game/Online/API/Requests/DownloadBeatmapSetRequest.cs b/osu.Game/Online/API/Requests/DownloadBeatmapSetRequest.cs index 7d0a8f9f46..8d636f6730 100644 --- a/osu.Game/Online/API/Requests/DownloadBeatmapSetRequest.cs +++ b/osu.Game/Online/API/Requests/DownloadBeatmapSetRequest.cs @@ -7,17 +7,16 @@ namespace osu.Game.Online.API.Requests { public class DownloadBeatmapSetRequest : ArchiveDownloadModelRequest { - public readonly BeatmapSetInfo BeatmapSet; - private readonly bool noVideo; + private readonly BeatmapSetInfo set; public DownloadBeatmapSetRequest(BeatmapSetInfo set, bool noVideo) : base(set) { this.noVideo = noVideo; - BeatmapSet = set; + this.set = set; } - protected override string Target => $@"beatmapsets/{BeatmapSet.OnlineBeatmapSetID}/download{(noVideo ? "?noVideo=1" : "")}"; + protected override string Target => $@"beatmapsets/{set.OnlineBeatmapSetID}/download{(noVideo ? "?noVideo=1" : "")}"; } } diff --git a/osu.Game/Overlays/Direct/DownloadTrackingComposite.cs b/osu.Game/Overlays/Direct/DownloadTrackingComposite.cs index 9beedb195f..c1ff6ecb60 100644 --- a/osu.Game/Overlays/Direct/DownloadTrackingComposite.cs +++ b/osu.Game/Overlays/Direct/DownloadTrackingComposite.cs @@ -7,7 +7,7 @@ using osu.Framework.Allocation; using osu.Framework.Bindables; using osu.Framework.Graphics.Containers; using osu.Game.Beatmaps; -using osu.Game.Online.API.Requests; +using osu.Game.Online.API; namespace osu.Game.Overlays.Direct { @@ -49,7 +49,7 @@ namespace osu.Game.Overlays.Direct beatmaps.DownloadBegan += download => { - if (download.BeatmapSet.OnlineBeatmapSetID == BeatmapSet.Value?.OnlineBeatmapSetID) + if (download.Info.OnlineBeatmapSetID == BeatmapSet.Value?.OnlineBeatmapSetID) attachDownload(download); }; @@ -76,9 +76,9 @@ namespace osu.Game.Overlays.Direct #endregion - private DownloadBeatmapSetRequest attachedRequest; + private ArchiveDownloadModelRequest attachedRequest; - private void attachDownload(DownloadBeatmapSetRequest request) + private void attachDownload(ArchiveDownloadModelRequest request) { if (attachedRequest != null) { From d903ad2186ed7c6e81fc2c4982eef2e5b89e5ee3 Mon Sep 17 00:00:00 2001 From: naoey Date: Tue, 11 Jun 2019 21:30:25 +0530 Subject: [PATCH 08/25] Fix order --- osu.Game/Database/ArchiveDownloadModelManager.cs | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/osu.Game/Database/ArchiveDownloadModelManager.cs b/osu.Game/Database/ArchiveDownloadModelManager.cs index 64920710bc..2fb661ccce 100644 --- a/osu.Game/Database/ArchiveDownloadModelManager.cs +++ b/osu.Game/Database/ArchiveDownloadModelManager.cs @@ -72,15 +72,9 @@ namespace osu.Game.Database return true; } - /// - /// Checks whether a given is available in the local store already. - /// - /// The whose existence needs to be checked. - /// Whether the exists locally. - public bool IsAvailableLocally(TModel model) => modelStore.ConsumableItems.Any(m => m.Equals(model)); - /// /// Downloads a with optional parameters for the download request. + /// This will post notifications tracking progress. /// /// The to be downloaded. /// Optional parameters to be used for creating the download request. @@ -96,6 +90,13 @@ namespace osu.Game.Database return true; } + /// + /// Checks whether a given is available in the local store already. + /// + /// The whose existence needs to be checked. + /// Whether the exists locally. + public bool IsAvailableLocally(TModel model) => modelStore.ConsumableItems.Any(m => m.Equals(model)); + /// /// Gets an existing download request if it exists. /// From 4a6074865e270a66c170fa508a9cb1ee126f4dec Mon Sep 17 00:00:00 2001 From: naoey Date: Tue, 11 Jun 2019 22:42:57 +0530 Subject: [PATCH 09/25] Create interfaces for DownloadTrackingComposite to consume --- osu.Game/Beatmaps/BeatmapManager.cs | 2 + .../Database/ArchiveDownloadModelManager.cs | 28 +---------- osu.Game/Database/ArchiveModelManager.cs | 2 +- osu.Game/Database/IDownloadModelManager.cs | 47 +++++++++++++++++++ osu.Game/Database/IModelManager.cs | 20 ++++++++ 5 files changed, 72 insertions(+), 27 deletions(-) create mode 100644 osu.Game/Database/IDownloadModelManager.cs create mode 100644 osu.Game/Database/IModelManager.cs diff --git a/osu.Game/Beatmaps/BeatmapManager.cs b/osu.Game/Beatmaps/BeatmapManager.cs index 2dcd1b137c..e124e38dd9 100644 --- a/osu.Game/Beatmaps/BeatmapManager.cs +++ b/osu.Game/Beatmaps/BeatmapManager.cs @@ -229,6 +229,8 @@ namespace osu.Game.Beatmaps /// Results from the provided query. public IQueryable QueryBeatmaps(Expression> query) => beatmaps.Beatmaps.AsNoTracking().Where(query); + public override bool IsAvailableLocally(BeatmapSetInfo set) => beatmaps.ConsumableItems.Any(s => s.OnlineBeatmapSetID == set.OnlineBeatmapSetID && !s.DeletePending && !s.Protected); + protected override BeatmapSetInfo CreateModel(ArchiveReader reader) { // let's make sure there are actually .osu files to import. diff --git a/osu.Game/Database/ArchiveDownloadModelManager.cs b/osu.Game/Database/ArchiveDownloadModelManager.cs index 2fb661ccce..c868b8d1d5 100644 --- a/osu.Game/Database/ArchiveDownloadModelManager.cs +++ b/osu.Game/Database/ArchiveDownloadModelManager.cs @@ -18,18 +18,12 @@ namespace osu.Game.Database /// /// The model type. /// The associated file join type. - public abstract class ArchiveDownloadModelManager : ArchiveModelManager + public abstract class ArchiveDownloadModelManager : ArchiveModelManager, IDownloadModelManager where TModel : class, IHasFiles, IHasPrimaryKey, ISoftDelete where TFileModel : INamedFileInfo, new() { - /// - /// Fired when a download begins. - /// public event Action> DownloadBegan; - /// - /// Fired when a download is interrupted, either due to user cancellation or failure. - /// public event Action> DownloadFailed; private readonly IAPIProvider api; @@ -55,12 +49,6 @@ namespace osu.Game.Database /// The request object. protected abstract ArchiveDownloadModelRequest CreateDownloadRequest(TModel model, object[] options); - /// - /// Downloads a . - /// This will post notifications tracking progress. - /// - /// The to be downloaded. - /// Whether downloading can happen. public bool Download(TModel model) { if (!canDownload(model)) return false; @@ -72,13 +60,6 @@ namespace osu.Game.Database return true; } - /// - /// Downloads a with optional parameters for the download request. - /// This will post notifications tracking progress. - /// - /// The to be downloaded. - /// Optional parameters to be used for creating the download request. - /// Whether downloading can happen. public bool Download(TModel model, params object[] extra) { if (!canDownload(model)) return false; @@ -90,12 +71,7 @@ namespace osu.Game.Database return true; } - /// - /// Checks whether a given is available in the local store already. - /// - /// The whose existence needs to be checked. - /// Whether the exists locally. - public bool IsAvailableLocally(TModel model) => modelStore.ConsumableItems.Any(m => m.Equals(model)); + public virtual bool IsAvailableLocally(TModel model) => modelStore.ConsumableItems.Any(m => m.Equals(model) && !m.DeletePending); /// /// Gets an existing download request if it exists. diff --git a/osu.Game/Database/ArchiveModelManager.cs b/osu.Game/Database/ArchiveModelManager.cs index 54dbae9ddc..50cb9dac8b 100644 --- a/osu.Game/Database/ArchiveModelManager.cs +++ b/osu.Game/Database/ArchiveModelManager.cs @@ -29,7 +29,7 @@ namespace osu.Game.Database /// /// The model type. /// The associated file join type. - public abstract class ArchiveModelManager : ICanAcceptFiles + public abstract class ArchiveModelManager : ICanAcceptFiles, IModelManager where TModel : class, IHasFiles, IHasPrimaryKey, ISoftDelete where TFileModel : INamedFileInfo, new() { diff --git a/osu.Game/Database/IDownloadModelManager.cs b/osu.Game/Database/IDownloadModelManager.cs new file mode 100644 index 0000000000..3231d855ea --- /dev/null +++ b/osu.Game/Database/IDownloadModelManager.cs @@ -0,0 +1,47 @@ +using osu.Game.Online.API; +using System; +using System.Collections.Generic; +using System.Text; + +namespace osu.Game.Database +{ + public interface IDownloadModelManager : IModelManager + where TModel : class + { + /// + /// Fired when a download begins. + /// + event Action> DownloadBegan; + + /// + /// Fired when a download is interrupted, either due to user cancellation or failure. + /// + event Action> DownloadFailed; + + bool IsAvailableLocally(TModel model); + + /// + /// Downloads a . + /// This will post notifications tracking progress. + /// + /// The to be downloaded. + /// Whether downloading can happen. + bool Download(TModel model); + + /// + /// Downloads a with optional parameters for the download request. + /// This will post notifications tracking progress. + /// + /// The to be downloaded. + /// Optional parameters to be used for creating the download request. + /// Whether downloading can happen. + bool Download(TModel model, params object[] extra); + + /// + /// Checks whether a given is available in the local store already. + /// + /// The whose existence needs to be checked. + /// Whether the exists locally. + ArchiveDownloadModelRequest GetExistingDownload(TModel model); + } +} diff --git a/osu.Game/Database/IModelManager.cs b/osu.Game/Database/IModelManager.cs new file mode 100644 index 0000000000..6a0b2bde44 --- /dev/null +++ b/osu.Game/Database/IModelManager.cs @@ -0,0 +1,20 @@ + +using System; + +namespace osu.Game.Database +{ + public interface IModelManager + { + /// + /// Fired when a new becomes available in the database. + /// This is not guaranteed to run on the update thread. + /// + event Action ItemAdded; + + /// + /// Fired when a is removed from the database. + /// This is not guaranteed to run on the update thread. + /// + event Action ItemRemoved; + } +} From c320b6110cf463a1f5e74050e320468a26a484a1 Mon Sep 17 00:00:00 2001 From: naoey Date: Tue, 11 Jun 2019 23:47:05 +0530 Subject: [PATCH 10/25] Rename interface - Fix wrong inheritance in ArchiveModelManager - Add license headers --- osu.Game/Database/ArchiveDownloadModelManager.cs | 2 +- osu.Game/Database/ArchiveModelManager.cs | 4 ++-- ...{IDownloadModelManager.cs => IModelDownloader.cs} | 9 +++++---- osu.Game/Database/IModelManager.cs | 12 +++--------- 4 files changed, 11 insertions(+), 16 deletions(-) rename osu.Game/Database/{IDownloadModelManager.cs => IModelDownloader.cs} (87%) diff --git a/osu.Game/Database/ArchiveDownloadModelManager.cs b/osu.Game/Database/ArchiveDownloadModelManager.cs index c868b8d1d5..8d221bd3ea 100644 --- a/osu.Game/Database/ArchiveDownloadModelManager.cs +++ b/osu.Game/Database/ArchiveDownloadModelManager.cs @@ -18,7 +18,7 @@ namespace osu.Game.Database /// /// The model type. /// The associated file join type. - public abstract class ArchiveDownloadModelManager : ArchiveModelManager, IDownloadModelManager + public abstract class ArchiveDownloadModelManager : ArchiveModelManager, IModelDownloader where TModel : class, IHasFiles, IHasPrimaryKey, ISoftDelete where TFileModel : INamedFileInfo, new() { diff --git a/osu.Game/Database/ArchiveModelManager.cs b/osu.Game/Database/ArchiveModelManager.cs index 50cb9dac8b..ccaba99427 100644 --- a/osu.Game/Database/ArchiveModelManager.cs +++ b/osu.Game/Database/ArchiveModelManager.cs @@ -29,7 +29,7 @@ namespace osu.Game.Database /// /// The model type. /// The associated file join type. - public abstract class ArchiveModelManager : ICanAcceptFiles, IModelManager + public abstract class ArchiveModelManager : ICanAcceptFiles, IModelManager where TModel : class, IHasFiles, IHasPrimaryKey, ISoftDelete where TFileModel : INamedFileInfo, new() { @@ -44,7 +44,7 @@ namespace osu.Game.Database /// Fired when a new becomes available in the database. /// This is not guaranteed to run on the update thread. /// - public event ItemAddedDelegate ItemAdded; + public event Action ItemAdded; /// /// Fired when a is removed from the database. diff --git a/osu.Game/Database/IDownloadModelManager.cs b/osu.Game/Database/IModelDownloader.cs similarity index 87% rename from osu.Game/Database/IDownloadModelManager.cs rename to osu.Game/Database/IModelDownloader.cs index 3231d855ea..bf987bb53c 100644 --- a/osu.Game/Database/IDownloadModelManager.cs +++ b/osu.Game/Database/IModelDownloader.cs @@ -1,11 +1,12 @@ -using osu.Game.Online.API; +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using osu.Game.Online.API; using System; -using System.Collections.Generic; -using System.Text; namespace osu.Game.Database { - public interface IDownloadModelManager : IModelManager + public interface IModelDownloader : IModelManager where TModel : class { /// diff --git a/osu.Game/Database/IModelManager.cs b/osu.Game/Database/IModelManager.cs index 6a0b2bde44..ee78df3db4 100644 --- a/osu.Game/Database/IModelManager.cs +++ b/osu.Game/Database/IModelManager.cs @@ -1,20 +1,14 @@ - +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + using System; namespace osu.Game.Database { public interface IModelManager { - /// - /// Fired when a new becomes available in the database. - /// This is not guaranteed to run on the update thread. - /// event Action ItemAdded; - /// - /// Fired when a is removed from the database. - /// This is not guaranteed to run on the update thread. - /// event Action ItemRemoved; } } From c69d3e2d388c794a0fcc33efdbc8b37999c48c68 Mon Sep 17 00:00:00 2001 From: naoey Date: Wed, 12 Jun 2019 00:02:53 +0530 Subject: [PATCH 11/25] Fix doc move derp --- osu.Game/Database/ArchiveDownloadModelManager.cs | 10 +++++----- osu.Game/Database/IModelDownloader.cs | 6 +++--- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/osu.Game/Database/ArchiveDownloadModelManager.cs b/osu.Game/Database/ArchiveDownloadModelManager.cs index 8d221bd3ea..ded44337dd 100644 --- a/osu.Game/Database/ArchiveDownloadModelManager.cs +++ b/osu.Game/Database/ArchiveDownloadModelManager.cs @@ -71,13 +71,13 @@ namespace osu.Game.Database return true; } + /// + /// Checks whether a given is available in the local store already. + /// + /// The whose existence needs to be checked. + /// Whether the exists locally. public virtual bool IsAvailableLocally(TModel model) => modelStore.ConsumableItems.Any(m => m.Equals(model) && !m.DeletePending); - /// - /// Gets an existing download request if it exists. - /// - /// The whose request is wanted. - /// The object if it exists, otherwise null. public ArchiveDownloadModelRequest GetExistingDownload(TModel model) => currentDownloads.Find(r => r.Info.Equals(model)); private bool canDownload(TModel model) => GetExistingDownload(model) == null && api != null; diff --git a/osu.Game/Database/IModelDownloader.cs b/osu.Game/Database/IModelDownloader.cs index bf987bb53c..150ad9522f 100644 --- a/osu.Game/Database/IModelDownloader.cs +++ b/osu.Game/Database/IModelDownloader.cs @@ -39,10 +39,10 @@ namespace osu.Game.Database bool Download(TModel model, params object[] extra); /// - /// Checks whether a given is available in the local store already. + /// Gets an existing download request if it exists. /// - /// The whose existence needs to be checked. - /// Whether the exists locally. + /// The whose request is wanted. + /// The object if it exists, otherwise null. ArchiveDownloadModelRequest GetExistingDownload(TModel model); } } From eaeeffaa866fd7a16108ffc96ac7544319232cf8 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 12 Jun 2019 13:28:44 +0900 Subject: [PATCH 12/25] Rename to DownloadableArchiveModelManager --- osu.Game/Beatmaps/BeatmapManager.cs | 2 +- ...loadModelManager.cs => DownloadableArchiveModelManager.cs} | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) rename osu.Game/Database/{ArchiveDownloadModelManager.cs => DownloadableArchiveModelManager.cs} (94%) diff --git a/osu.Game/Beatmaps/BeatmapManager.cs b/osu.Game/Beatmaps/BeatmapManager.cs index e124e38dd9..023b6c74ea 100644 --- a/osu.Game/Beatmaps/BeatmapManager.cs +++ b/osu.Game/Beatmaps/BeatmapManager.cs @@ -25,7 +25,7 @@ namespace osu.Game.Beatmaps /// /// Handles the storage and retrieval of Beatmaps/WorkingBeatmaps. /// - public partial class BeatmapManager : ArchiveDownloadModelManager + public partial class BeatmapManager : DownloadableArchiveModelManager { /// /// Fired when a single difficulty has been hidden. diff --git a/osu.Game/Database/ArchiveDownloadModelManager.cs b/osu.Game/Database/DownloadableArchiveModelManager.cs similarity index 94% rename from osu.Game/Database/ArchiveDownloadModelManager.cs rename to osu.Game/Database/DownloadableArchiveModelManager.cs index ded44337dd..71bbbc4f78 100644 --- a/osu.Game/Database/ArchiveDownloadModelManager.cs +++ b/osu.Game/Database/DownloadableArchiveModelManager.cs @@ -18,7 +18,7 @@ namespace osu.Game.Database /// /// The model type. /// The associated file join type. - public abstract class ArchiveDownloadModelManager : ArchiveModelManager, IModelDownloader + public abstract class DownloadableArchiveModelManager : ArchiveModelManager, IModelDownloader where TModel : class, IHasFiles, IHasPrimaryKey, ISoftDelete where TFileModel : INamedFileInfo, new() { @@ -32,7 +32,7 @@ namespace osu.Game.Database private readonly MutableDatabaseBackedStoreWithFileIncludes modelStore; - protected ArchiveDownloadModelManager(Storage storage, IDatabaseContextFactory contextFactory, IAPIProvider api, MutableDatabaseBackedStoreWithFileIncludes modelStore, IIpcHost importHost = null) + protected DownloadableArchiveModelManager(Storage storage, IDatabaseContextFactory contextFactory, IAPIProvider api, MutableDatabaseBackedStoreWithFileIncludes modelStore, IIpcHost importHost = null) : base(storage, contextFactory, modelStore, importHost) { this.api = api; From c591a6f1fadd26a0c39855a867d95c2056bd6d82 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 12 Jun 2019 13:30:23 +0900 Subject: [PATCH 13/25] Rename request type to be less verbose --- osu.Game/Beatmaps/BeatmapManager.cs | 2 +- osu.Game/Database/DownloadableArchiveModelManager.cs | 12 ++++++------ osu.Game/Database/IModelDownloader.cs | 8 ++++---- ...loadModelRequest.cs => ArchiveDownloadRequest.cs} | 4 ++-- .../Online/API/Requests/DownloadBeatmapSetRequest.cs | 2 +- .../Overlays/Direct/DownloadTrackingComposite.cs | 4 ++-- 6 files changed, 16 insertions(+), 16 deletions(-) rename osu.Game/Online/API/{ArchiveDownloadModelRequest.cs => ArchiveDownloadRequest.cs} (78%) diff --git a/osu.Game/Beatmaps/BeatmapManager.cs b/osu.Game/Beatmaps/BeatmapManager.cs index 023b6c74ea..3fc3de41e6 100644 --- a/osu.Game/Beatmaps/BeatmapManager.cs +++ b/osu.Game/Beatmaps/BeatmapManager.cs @@ -74,7 +74,7 @@ namespace osu.Game.Beatmaps beatmaps.BeatmapRestored += b => BeatmapRestored?.Invoke(b); } - protected override ArchiveDownloadModelRequest CreateDownloadRequest(BeatmapSetInfo set, object[] options) => new DownloadBeatmapSetRequest(set, (options?.FirstOrDefault() as bool?) ?? false); + protected override ArchiveDownloadRequest CreateDownloadRequest(BeatmapSetInfo set, object[] options) => new DownloadBeatmapSetRequest(set, (options?.FirstOrDefault() as bool?) ?? false); protected override void Populate(BeatmapSetInfo beatmapSet, ArchiveReader archive) { diff --git a/osu.Game/Database/DownloadableArchiveModelManager.cs b/osu.Game/Database/DownloadableArchiveModelManager.cs index 71bbbc4f78..c22c8d7508 100644 --- a/osu.Game/Database/DownloadableArchiveModelManager.cs +++ b/osu.Game/Database/DownloadableArchiveModelManager.cs @@ -22,13 +22,13 @@ namespace osu.Game.Database where TModel : class, IHasFiles, IHasPrimaryKey, ISoftDelete where TFileModel : INamedFileInfo, new() { - public event Action> DownloadBegan; + public event Action> DownloadBegan; - public event Action> DownloadFailed; + public event Action> DownloadFailed; private readonly IAPIProvider api; - private readonly List> currentDownloads = new List>(); + private readonly List> currentDownloads = new List>(); private readonly MutableDatabaseBackedStoreWithFileIncludes modelStore; @@ -47,7 +47,7 @@ namespace osu.Game.Database /// The to be downloaded. /// Extra parameters for request creation, null if none were passed. /// The request object. - protected abstract ArchiveDownloadModelRequest CreateDownloadRequest(TModel model, object[] options); + protected abstract ArchiveDownloadRequest CreateDownloadRequest(TModel model, object[] options); public bool Download(TModel model) { @@ -78,11 +78,11 @@ namespace osu.Game.Database /// Whether the exists locally. public virtual bool IsAvailableLocally(TModel model) => modelStore.ConsumableItems.Any(m => m.Equals(model) && !m.DeletePending); - public ArchiveDownloadModelRequest GetExistingDownload(TModel model) => currentDownloads.Find(r => r.Info.Equals(model)); + public ArchiveDownloadRequest GetExistingDownload(TModel model) => currentDownloads.Find(r => r.Info.Equals(model)); private bool canDownload(TModel model) => GetExistingDownload(model) == null && api != null; - private void performDownloadWithRequest(ArchiveDownloadModelRequest request) + private void performDownloadWithRequest(ArchiveDownloadRequest request) { DownloadNotification notification = new DownloadNotification { diff --git a/osu.Game/Database/IModelDownloader.cs b/osu.Game/Database/IModelDownloader.cs index 150ad9522f..83427bdc17 100644 --- a/osu.Game/Database/IModelDownloader.cs +++ b/osu.Game/Database/IModelDownloader.cs @@ -12,12 +12,12 @@ namespace osu.Game.Database /// /// Fired when a download begins. /// - event Action> DownloadBegan; + event Action> DownloadBegan; /// /// Fired when a download is interrupted, either due to user cancellation or failure. /// - event Action> DownloadFailed; + event Action> DownloadFailed; bool IsAvailableLocally(TModel model); @@ -42,7 +42,7 @@ namespace osu.Game.Database /// Gets an existing download request if it exists. /// /// The whose request is wanted. - /// The object if it exists, otherwise null. - ArchiveDownloadModelRequest GetExistingDownload(TModel model); + /// The object if it exists, otherwise null. + ArchiveDownloadRequest GetExistingDownload(TModel model); } } diff --git a/osu.Game/Online/API/ArchiveDownloadModelRequest.cs b/osu.Game/Online/API/ArchiveDownloadRequest.cs similarity index 78% rename from osu.Game/Online/API/ArchiveDownloadModelRequest.cs rename to osu.Game/Online/API/ArchiveDownloadRequest.cs index 7f161f1e98..01f066694d 100644 --- a/osu.Game/Online/API/ArchiveDownloadModelRequest.cs +++ b/osu.Game/Online/API/ArchiveDownloadRequest.cs @@ -5,7 +5,7 @@ using System; namespace osu.Game.Online.API { - public abstract class ArchiveDownloadModelRequest : APIDownloadRequest + public abstract class ArchiveDownloadRequest : APIDownloadRequest where TModel : class { public readonly TModel Info; @@ -14,7 +14,7 @@ namespace osu.Game.Online.API public event Action DownloadProgressed; - protected ArchiveDownloadModelRequest(TModel model) + protected ArchiveDownloadRequest(TModel model) { Info = model; diff --git a/osu.Game/Online/API/Requests/DownloadBeatmapSetRequest.cs b/osu.Game/Online/API/Requests/DownloadBeatmapSetRequest.cs index 8d636f6730..999864a6eb 100644 --- a/osu.Game/Online/API/Requests/DownloadBeatmapSetRequest.cs +++ b/osu.Game/Online/API/Requests/DownloadBeatmapSetRequest.cs @@ -5,7 +5,7 @@ using osu.Game.Beatmaps; namespace osu.Game.Online.API.Requests { - public class DownloadBeatmapSetRequest : ArchiveDownloadModelRequest + public class DownloadBeatmapSetRequest : ArchiveDownloadRequest { private readonly bool noVideo; private readonly BeatmapSetInfo set; diff --git a/osu.Game/Overlays/Direct/DownloadTrackingComposite.cs b/osu.Game/Overlays/Direct/DownloadTrackingComposite.cs index c1ff6ecb60..9d0266c00e 100644 --- a/osu.Game/Overlays/Direct/DownloadTrackingComposite.cs +++ b/osu.Game/Overlays/Direct/DownloadTrackingComposite.cs @@ -76,9 +76,9 @@ namespace osu.Game.Overlays.Direct #endregion - private ArchiveDownloadModelRequest attachedRequest; + private ArchiveDownloadRequest attachedRequest; - private void attachDownload(ArchiveDownloadModelRequest request) + private void attachDownload(ArchiveDownloadRequest request) { if (attachedRequest != null) { From 9cd5519da3d2244b6a250d676a25fd951578197d Mon Sep 17 00:00:00 2001 From: naoey Date: Wed, 12 Jun 2019 18:35:34 +0530 Subject: [PATCH 14/25] Remove unused delegate, use model name in notifications, add more xmldoc - Applies a `class` constraint to the generic type in `IModelManager` - Add xmldoc --- osu.Game/Beatmaps/BeatmapManager.cs | 5 +---- osu.Game/Database/ArchiveModelManager.cs | 2 -- osu.Game/Database/DownloadableArchiveModelManager.cs | 4 ++-- osu.Game/Database/IModelDownloader.cs | 8 ++++++-- osu.Game/Database/IModelManager.cs | 5 +++++ 5 files changed, 14 insertions(+), 10 deletions(-) diff --git a/osu.Game/Beatmaps/BeatmapManager.cs b/osu.Game/Beatmaps/BeatmapManager.cs index c2adf1ac5b..2cb7e8b901 100644 --- a/osu.Game/Beatmaps/BeatmapManager.cs +++ b/osu.Game/Beatmaps/BeatmapManager.cs @@ -55,8 +55,6 @@ namespace osu.Game.Beatmaps private readonly BeatmapStore beatmaps; - private readonly IAPIProvider api; - private readonly AudioManager audioManager; private readonly GameHost host; @@ -68,7 +66,6 @@ namespace osu.Game.Beatmaps : base(storage, contextFactory, api, new BeatmapStore(contextFactory), host) { this.rulesets = rulesets; - this.api = api; this.audioManager = audioManager; this.host = host; @@ -80,7 +77,7 @@ namespace osu.Game.Beatmaps updateQueue = new BeatmapUpdateQueue(api); } - + protected override ArchiveDownloadRequest CreateDownloadRequest(BeatmapSetInfo set, object[] options) => new DownloadBeatmapSetRequest(set, (options?.FirstOrDefault() as bool?) ?? false); protected override Task Populate(BeatmapSetInfo beatmapSet, ArchiveReader archive, CancellationToken cancellationToken = default) diff --git a/osu.Game/Database/ArchiveModelManager.cs b/osu.Game/Database/ArchiveModelManager.cs index 49e0330c21..434e5b9525 100644 --- a/osu.Game/Database/ArchiveModelManager.cs +++ b/osu.Game/Database/ArchiveModelManager.cs @@ -35,8 +35,6 @@ namespace osu.Game.Database where TModel : class, IHasFiles, IHasPrimaryKey, ISoftDelete where TFileModel : INamedFileInfo, new() { - public delegate void ItemAddedDelegate(TModel model, bool existing); - /// /// Set an endpoint for notifications to be posted to. /// diff --git a/osu.Game/Database/DownloadableArchiveModelManager.cs b/osu.Game/Database/DownloadableArchiveModelManager.cs index 519b22b912..4a21673d2b 100644 --- a/osu.Game/Database/DownloadableArchiveModelManager.cs +++ b/osu.Game/Database/DownloadableArchiveModelManager.cs @@ -1,6 +1,7 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. +using Humanizer; using osu.Framework.Logging; using osu.Framework.Platform; using osu.Game.Online.API; @@ -112,8 +113,7 @@ namespace osu.Game.Database if (error is OperationCanceledException) return; notification.State = ProgressNotificationState.Cancelled; - // TODO: maybe implement a Name for every model that we can use in this message? - Logger.Error(error, "Download failed!"); + Logger.Error(error, $"{HumanisedModelName.Titleize()} download failed!"); currentDownloads.Remove(request); }; diff --git a/osu.Game/Database/IModelDownloader.cs b/osu.Game/Database/IModelDownloader.cs index 83427bdc17..42c64ba67b 100644 --- a/osu.Game/Database/IModelDownloader.cs +++ b/osu.Game/Database/IModelDownloader.cs @@ -6,6 +6,10 @@ using System; namespace osu.Game.Database { + /// + /// Represents a that can download new models from an external source. + /// + /// The model type. public interface IModelDownloader : IModelManager where TModel : class { @@ -23,7 +27,7 @@ namespace osu.Game.Database /// /// Downloads a . - /// This will post notifications tracking progress. + /// This may post notifications tracking progress. /// /// The to be downloaded. /// Whether downloading can happen. @@ -31,7 +35,7 @@ namespace osu.Game.Database /// /// Downloads a with optional parameters for the download request. - /// This will post notifications tracking progress. + /// This may post notifications tracking progress. /// /// The to be downloaded. /// Optional parameters to be used for creating the download request. diff --git a/osu.Game/Database/IModelManager.cs b/osu.Game/Database/IModelManager.cs index ee78df3db4..cb80ce49b2 100644 --- a/osu.Game/Database/IModelManager.cs +++ b/osu.Game/Database/IModelManager.cs @@ -5,7 +5,12 @@ using System; namespace osu.Game.Database { + /// + /// Represents a model manager that publishes events when s are added or removed. + /// + /// The model type. public interface IModelManager + where TModel : class { event Action ItemAdded; From 7ba676ad31328fc63353d98036d471498f983afe Mon Sep 17 00:00:00 2001 From: naoey Date: Wed, 12 Jun 2019 21:56:36 +0530 Subject: [PATCH 15/25] Rename Info to Model --- osu.Game/Database/DownloadableArchiveModelManager.cs | 4 ++-- osu.Game/Online/API/ArchiveDownloadRequest.cs | 4 ++-- osu.Game/Overlays/Direct/DownloadTrackingComposite.cs | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/osu.Game/Database/DownloadableArchiveModelManager.cs b/osu.Game/Database/DownloadableArchiveModelManager.cs index 4a21673d2b..0735452ce3 100644 --- a/osu.Game/Database/DownloadableArchiveModelManager.cs +++ b/osu.Game/Database/DownloadableArchiveModelManager.cs @@ -79,7 +79,7 @@ namespace osu.Game.Database /// Whether the exists locally. public virtual bool IsAvailableLocally(TModel model) => modelStore.ConsumableItems.Any(m => m.Equals(model) && !m.DeletePending); - public ArchiveDownloadRequest GetExistingDownload(TModel model) => currentDownloads.Find(r => r.Info.Equals(model)); + public ArchiveDownloadRequest GetExistingDownload(TModel model) => currentDownloads.Find(r => r.Model.Equals(model)); private bool canDownload(TModel model) => GetExistingDownload(model) == null && api != null; @@ -87,7 +87,7 @@ namespace osu.Game.Database { DownloadNotification notification = new DownloadNotification { - Text = $"Downloading {request.Info}", + Text = $"Downloading {request.Model}", }; request.DownloadProgressed += progress => diff --git a/osu.Game/Online/API/ArchiveDownloadRequest.cs b/osu.Game/Online/API/ArchiveDownloadRequest.cs index 01f066694d..f1966aeb2b 100644 --- a/osu.Game/Online/API/ArchiveDownloadRequest.cs +++ b/osu.Game/Online/API/ArchiveDownloadRequest.cs @@ -8,7 +8,7 @@ namespace osu.Game.Online.API public abstract class ArchiveDownloadRequest : APIDownloadRequest where TModel : class { - public readonly TModel Info; + public readonly TModel Model; public float Progress; @@ -16,7 +16,7 @@ namespace osu.Game.Online.API protected ArchiveDownloadRequest(TModel model) { - Info = model; + Model = model; Progressed += (current, total) => DownloadProgressed?.Invoke(Progress = (float)current / total); } diff --git a/osu.Game/Overlays/Direct/DownloadTrackingComposite.cs b/osu.Game/Overlays/Direct/DownloadTrackingComposite.cs index 9d0266c00e..494b18307e 100644 --- a/osu.Game/Overlays/Direct/DownloadTrackingComposite.cs +++ b/osu.Game/Overlays/Direct/DownloadTrackingComposite.cs @@ -49,7 +49,7 @@ namespace osu.Game.Overlays.Direct beatmaps.DownloadBegan += download => { - if (download.Info.OnlineBeatmapSetID == BeatmapSet.Value?.OnlineBeatmapSetID) + if (download.Model.OnlineBeatmapSetID == BeatmapSet.Value?.OnlineBeatmapSetID) attachDownload(download); }; From 3c2a2b23901baad45667c3fddbe044a1c0da7339 Mon Sep 17 00:00:00 2001 From: naoey Date: Thu, 13 Jun 2019 21:28:32 +0530 Subject: [PATCH 16/25] Move doc to interface --- osu.Game/Database/DownloadableArchiveModelManager.cs | 5 ----- osu.Game/Database/IModelDownloader.cs | 6 ++++++ 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/osu.Game/Database/DownloadableArchiveModelManager.cs b/osu.Game/Database/DownloadableArchiveModelManager.cs index 0735452ce3..fc50a4720a 100644 --- a/osu.Game/Database/DownloadableArchiveModelManager.cs +++ b/osu.Game/Database/DownloadableArchiveModelManager.cs @@ -72,11 +72,6 @@ namespace osu.Game.Database return true; } - /// - /// Checks whether a given is available in the local store already. - /// - /// The whose existence needs to be checked. - /// Whether the exists locally. public virtual bool IsAvailableLocally(TModel model) => modelStore.ConsumableItems.Any(m => m.Equals(model) && !m.DeletePending); public ArchiveDownloadRequest GetExistingDownload(TModel model) => currentDownloads.Find(r => r.Model.Equals(model)); diff --git a/osu.Game/Database/IModelDownloader.cs b/osu.Game/Database/IModelDownloader.cs index 42c64ba67b..b4f8c1e24a 100644 --- a/osu.Game/Database/IModelDownloader.cs +++ b/osu.Game/Database/IModelDownloader.cs @@ -23,6 +23,12 @@ namespace osu.Game.Database /// event Action> DownloadFailed; + + /// + /// Checks whether a given is already available in the local store. + /// + /// The whose existence needs to be checked. + /// Whether the exists. bool IsAvailableLocally(TModel model); /// From 4a16ac53bab407adeceadc992c5ec6c8eb1dd10e Mon Sep 17 00:00:00 2001 From: naoey Date: Sat, 15 Jun 2019 12:28:23 +0530 Subject: [PATCH 17/25] Remove extra newline --- osu.Game/Database/IModelDownloader.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/osu.Game/Database/IModelDownloader.cs b/osu.Game/Database/IModelDownloader.cs index b4f8c1e24a..b4df7cc0e3 100644 --- a/osu.Game/Database/IModelDownloader.cs +++ b/osu.Game/Database/IModelDownloader.cs @@ -23,7 +23,6 @@ namespace osu.Game.Database /// event Action> DownloadFailed; - /// /// Checks whether a given is already available in the local store. /// From f2e0ced05272f44851ffc067c4fe59552e4893a9 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 19 Jun 2019 01:32:37 +0900 Subject: [PATCH 18/25] Move private event handling logic to bottom of class --- osu.Game/Database/ArchiveModelManager.cs | 92 ++++++++++++------------ 1 file changed, 48 insertions(+), 44 deletions(-) diff --git a/osu.Game/Database/ArchiveModelManager.cs b/osu.Game/Database/ArchiveModelManager.cs index 434e5b9525..a64bca57cb 100644 --- a/osu.Game/Database/ArchiveModelManager.cs +++ b/osu.Game/Database/ArchiveModelManager.cs @@ -65,50 +65,6 @@ namespace osu.Game.Database // ReSharper disable once NotAccessedField.Local (we should keep a reference to this so it is not finalised) private ArchiveImportIPCChannel ipc; - private readonly List queuedEvents = new List(); - - /// - /// Allows delaying of outwards events until an operation is confirmed (at a database level). - /// - private bool delayingEvents; - - /// - /// Begin delaying outwards events. - /// - private void delayEvents() => delayingEvents = true; - - /// - /// Flush delayed events and disable delaying. - /// - /// Whether the flushed events should be performed. - private void flushEvents(bool perform) - { - Action[] events; - - lock (queuedEvents) - { - events = queuedEvents.ToArray(); - queuedEvents.Clear(); - } - - if (perform) - { - foreach (var a in events) - a.Invoke(); - } - - delayingEvents = false; - } - - private void handleEvent(Action a) - { - if (delayingEvents) - lock (queuedEvents) - queuedEvents.Add(a); - else - a.Invoke(); - } - protected ArchiveModelManager(Storage storage, IDatabaseContextFactory contextFactory, MutableDatabaseBackedStoreWithFileIncludes modelStore, IIpcHost importHost = null) { ContextFactory = contextFactory; @@ -630,6 +586,54 @@ namespace osu.Game.Database throw new InvalidFormatException($"{path} is not a valid archive"); } + + #region Event handling / delaying + + private readonly List queuedEvents = new List(); + + /// + /// Allows delaying of outwards events until an operation is confirmed (at a database level). + /// + private bool delayingEvents; + + /// + /// Begin delaying outwards events. + /// + private void delayEvents() => delayingEvents = true; + + /// + /// Flush delayed events and disable delaying. + /// + /// Whether the flushed events should be performed. + private void flushEvents(bool perform) + { + Action[] events; + + lock (queuedEvents) + { + events = queuedEvents.ToArray(); + queuedEvents.Clear(); + } + + if (perform) + { + foreach (var a in events) + a.Invoke(); + } + + delayingEvents = false; + } + + private void handleEvent(Action a) + { + if (delayingEvents) + lock (queuedEvents) + queuedEvents.Add(a); + else + a.Invoke(); + } + + #endregion } public abstract class ArchiveModelManager From 341dc74834187d6e7fb498d4c0ec138cf45d7d92 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 19 Jun 2019 01:41:19 +0900 Subject: [PATCH 19/25] Simplify download method --- osu.Game/Beatmaps/BeatmapManager.cs | 3 ++- .../DownloadableArchiveModelManager.cs | 27 +++++++------------ osu.Game/Database/IModelDownloader.cs | 17 +++--------- 3 files changed, 16 insertions(+), 31 deletions(-) diff --git a/osu.Game/Beatmaps/BeatmapManager.cs b/osu.Game/Beatmaps/BeatmapManager.cs index 2cb7e8b901..b7a08c068f 100644 --- a/osu.Game/Beatmaps/BeatmapManager.cs +++ b/osu.Game/Beatmaps/BeatmapManager.cs @@ -78,7 +78,8 @@ namespace osu.Game.Beatmaps updateQueue = new BeatmapUpdateQueue(api); } - protected override ArchiveDownloadRequest CreateDownloadRequest(BeatmapSetInfo set, object[] options) => new DownloadBeatmapSetRequest(set, (options?.FirstOrDefault() as bool?) ?? false); + protected override ArchiveDownloadRequest CreateDownloadRequest(BeatmapSetInfo set, bool minimiseDownloadSize) => + new DownloadBeatmapSetRequest(set, minimiseDownloadSize); protected override Task Populate(BeatmapSetInfo beatmapSet, ArchiveReader archive, CancellationToken cancellationToken = default) { diff --git a/osu.Game/Database/DownloadableArchiveModelManager.cs b/osu.Game/Database/DownloadableArchiveModelManager.cs index fc50a4720a..d9ebd0c06a 100644 --- a/osu.Game/Database/DownloadableArchiveModelManager.cs +++ b/osu.Game/Database/DownloadableArchiveModelManager.cs @@ -42,30 +42,23 @@ namespace osu.Game.Database /// /// Creates the download request for this . - /// The parameters will be provided when the download was initiated with extra options meant - /// to be used in the creation of the request. /// /// The to be downloaded. - /// Extra parameters for request creation, null if none were passed. + /// Whether this download should be optimised for slow connections. Generally means extras are not included in the download bundle.. /// The request object. - protected abstract ArchiveDownloadRequest CreateDownloadRequest(TModel model, object[] options); + protected abstract ArchiveDownloadRequest CreateDownloadRequest(TModel model, bool minimiseDownloadSize); - public bool Download(TModel model) + /// + /// Begin a download for the requested . + /// + /// The to be downloaded. + /// Whether this download should be optimised for slow connections. Generally means extras are not included in the download bundle.. + /// Whether the download was started. + public bool Download(TModel model, bool minimiseDownloadSize = false) { if (!canDownload(model)) return false; - var request = CreateDownloadRequest(model, null); - - performDownloadWithRequest(request); - - return true; - } - - public bool Download(TModel model, params object[] extra) - { - if (!canDownload(model)) return false; - - var request = CreateDownloadRequest(model, extra); + var request = CreateDownloadRequest(model, minimiseDownloadSize); performDownloadWithRequest(request); diff --git a/osu.Game/Database/IModelDownloader.cs b/osu.Game/Database/IModelDownloader.cs index b4df7cc0e3..f6f4b0aa42 100644 --- a/osu.Game/Database/IModelDownloader.cs +++ b/osu.Game/Database/IModelDownloader.cs @@ -31,21 +31,12 @@ namespace osu.Game.Database bool IsAvailableLocally(TModel model); /// - /// Downloads a . - /// This may post notifications tracking progress. + /// Begin a download for the requested . /// /// The to be downloaded. - /// Whether downloading can happen. - bool Download(TModel model); - - /// - /// Downloads a with optional parameters for the download request. - /// This may post notifications tracking progress. - /// - /// The to be downloaded. - /// Optional parameters to be used for creating the download request. - /// Whether downloading can happen. - bool Download(TModel model, params object[] extra); + /// Whether this download should be optimised for slow connections. Generally means extras are not included in the download bundle.. + /// Whether the download was started. + bool Download(TModel model, bool minimiseDownloadSize); /// /// Gets an existing download request if it exists. From 1bcff8a3e26c43c31890456cc3528d1c63bcd0ce Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 19 Jun 2019 01:57:38 +0900 Subject: [PATCH 20/25] Make generic covariant --- osu.Game/Database/IModelManager.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Database/IModelManager.cs b/osu.Game/Database/IModelManager.cs index cb80ce49b2..e41ef3efef 100644 --- a/osu.Game/Database/IModelManager.cs +++ b/osu.Game/Database/IModelManager.cs @@ -9,7 +9,7 @@ namespace osu.Game.Database /// Represents a model manager that publishes events when s are added or removed. /// /// The model type. - public interface IModelManager + public interface IModelManager where TModel : class { event Action ItemAdded; From 4b46601eae6efda450b70dc70aa9c86cb5f01fa5 Mon Sep 17 00:00:00 2001 From: naoey Date: Wed, 19 Jun 2019 19:43:09 +0530 Subject: [PATCH 21/25] Remove redundant variable, handle all request failures --- .../DownloadableArchiveModelManager.cs | 26 +++++++++++-------- .../API/Requests/DownloadBeatmapSetRequest.cs | 4 +-- 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/osu.Game/Database/DownloadableArchiveModelManager.cs b/osu.Game/Database/DownloadableArchiveModelManager.cs index d9ebd0c06a..90d5ec3f17 100644 --- a/osu.Game/Database/DownloadableArchiveModelManager.cs +++ b/osu.Game/Database/DownloadableArchiveModelManager.cs @@ -94,16 +94,7 @@ namespace osu.Game.Database }, TaskCreationOptions.LongRunning); }; - request.Failure += error => - { - DownloadFailed?.Invoke(request); - - if (error is OperationCanceledException) return; - - notification.State = ProgressNotificationState.Cancelled; - Logger.Error(error, $"{HumanisedModelName.Titleize()} download failed!"); - currentDownloads.Remove(request); - }; + request.Failure += error => handleRequestFailure(request, notification, error); notification.CancelRequested += () => { @@ -122,14 +113,27 @@ namespace osu.Game.Database { request.Perform(api); } - catch + catch (Exception e) { + // 404s (and maybe other failures) don't fire request.Failure so for now they handled here as well + handleRequestFailure(request, notification, e); } }, TaskCreationOptions.LongRunning); DownloadBegan?.Invoke(request); } + private void handleRequestFailure(ArchiveDownloadRequest req, ProgressNotification notification, Exception e) + { + DownloadFailed?.Invoke(req); + + if (e is OperationCanceledException) return; + + notification.State = ProgressNotificationState.Cancelled; + Logger.Error(e, $"{HumanisedModelName.Titleize()} download failed!"); + currentDownloads.Remove(req); + } + private class DownloadNotification : ProgressNotification { public override bool IsImportant => false; diff --git a/osu.Game/Online/API/Requests/DownloadBeatmapSetRequest.cs b/osu.Game/Online/API/Requests/DownloadBeatmapSetRequest.cs index 999864a6eb..707c59436d 100644 --- a/osu.Game/Online/API/Requests/DownloadBeatmapSetRequest.cs +++ b/osu.Game/Online/API/Requests/DownloadBeatmapSetRequest.cs @@ -8,15 +8,13 @@ namespace osu.Game.Online.API.Requests public class DownloadBeatmapSetRequest : ArchiveDownloadRequest { private readonly bool noVideo; - private readonly BeatmapSetInfo set; public DownloadBeatmapSetRequest(BeatmapSetInfo set, bool noVideo) : base(set) { this.noVideo = noVideo; - this.set = set; } - protected override string Target => $@"beatmapsets/{set.OnlineBeatmapSetID}/download{(noVideo ? "?noVideo=1" : "")}"; + protected override string Target => $@"beatmapsets/{Model.OnlineBeatmapSetID}/download{(noVideo ? "?noVideo=1" : "")}"; } } From 6f5fbd7ea136ca5f7295dc53aac34a9cbf151b56 Mon Sep 17 00:00:00 2001 From: naoey Date: Tue, 25 Jun 2019 18:28:59 +0530 Subject: [PATCH 22/25] Remove unnecessary try-catch block --- .../Database/DownloadableArchiveModelManager.cs | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/osu.Game/Database/DownloadableArchiveModelManager.cs b/osu.Game/Database/DownloadableArchiveModelManager.cs index 90d5ec3f17..cff2e8f01b 100644 --- a/osu.Game/Database/DownloadableArchiveModelManager.cs +++ b/osu.Game/Database/DownloadableArchiveModelManager.cs @@ -107,18 +107,7 @@ namespace osu.Game.Database currentDownloads.Add(request); PostNotification?.Invoke(notification); - Task.Factory.StartNew(() => - { - try - { - request.Perform(api); - } - catch (Exception e) - { - // 404s (and maybe other failures) don't fire request.Failure so for now they handled here as well - handleRequestFailure(request, notification, e); - } - }, TaskCreationOptions.LongRunning); + Task.Factory.StartNew(() => request.Perform(api), TaskCreationOptions.LongRunning); DownloadBegan?.Invoke(request); } From c476e46a8e3817706918cce627c512880622a8f6 Mon Sep 17 00:00:00 2001 From: naoey Date: Tue, 25 Jun 2019 21:16:30 +0530 Subject: [PATCH 23/25] Remove unnecessary private methods and inline used-once code --- .../DownloadableArchiveModelManager.cs | 37 ++++++++----------- 1 file changed, 15 insertions(+), 22 deletions(-) diff --git a/osu.Game/Database/DownloadableArchiveModelManager.cs b/osu.Game/Database/DownloadableArchiveModelManager.cs index cff2e8f01b..e0ac221cfc 100644 --- a/osu.Game/Database/DownloadableArchiveModelManager.cs +++ b/osu.Game/Database/DownloadableArchiveModelManager.cs @@ -60,19 +60,6 @@ namespace osu.Game.Database var request = CreateDownloadRequest(model, minimiseDownloadSize); - performDownloadWithRequest(request); - - return true; - } - - public virtual bool IsAvailableLocally(TModel model) => modelStore.ConsumableItems.Any(m => m.Equals(model) && !m.DeletePending); - - public ArchiveDownloadRequest GetExistingDownload(TModel model) => currentDownloads.Find(r => r.Model.Equals(model)); - - private bool canDownload(TModel model) => GetExistingDownload(model) == null && api != null; - - private void performDownloadWithRequest(ArchiveDownloadRequest request) - { DownloadNotification notification = new DownloadNotification { Text = $"Downloading {request.Model}", @@ -94,7 +81,16 @@ namespace osu.Game.Database }, TaskCreationOptions.LongRunning); }; - request.Failure += error => handleRequestFailure(request, notification, error); + request.Failure += error => + { + DownloadFailed?.Invoke(request); + + if (error is OperationCanceledException) return; + + notification.State = ProgressNotificationState.Cancelled; + Logger.Error(error, $"{HumanisedModelName.Titleize()} download failed!"); + currentDownloads.Remove(request); + }; notification.CancelRequested += () => { @@ -110,18 +106,15 @@ namespace osu.Game.Database Task.Factory.StartNew(() => request.Perform(api), TaskCreationOptions.LongRunning); DownloadBegan?.Invoke(request); + + return true; } - private void handleRequestFailure(ArchiveDownloadRequest req, ProgressNotification notification, Exception e) - { - DownloadFailed?.Invoke(req); + public virtual bool IsAvailableLocally(TModel model) => modelStore.ConsumableItems.Any(m => m.Equals(model) && !m.DeletePending); - if (e is OperationCanceledException) return; + public ArchiveDownloadRequest GetExistingDownload(TModel model) => currentDownloads.Find(r => r.Model.Equals(model)); - notification.State = ProgressNotificationState.Cancelled; - Logger.Error(e, $"{HumanisedModelName.Titleize()} download failed!"); - currentDownloads.Remove(req); - } + private bool canDownload(TModel model) => GetExistingDownload(model) == null && api != null; private class DownloadNotification : ProgressNotification { From 9e1cb90dd86c76aeaa2b3f7d8dae9ce8730bfae6 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 26 Jun 2019 11:40:33 +0900 Subject: [PATCH 24/25] Remove existing argument for ItemAdded event For all usages, it looks like this was unnecessary. --- .../Beatmaps/IO/ImportBeatmapTest.cs | 2 +- osu.Game/Database/ArchiveModelManager.cs | 6 +- osu.Game/Database/IModelManager.cs | 2 +- osu.Game/OsuGameBase.cs | 2 +- .../Direct/DownloadTrackingComposite.cs | 2 +- osu.Game/Overlays/Music/PlaylistList.cs | 7 +-- osu.Game/Overlays/MusicController.cs | 10 +--- .../Overlays/Settings/Sections/SkinSection.cs | 8 +-- .../Multi/Match/Components/ReadyButton.cs | 5 +- .../Screens/Multi/Match/MatchSubScreen.cs | 2 +- osu.Game/Screens/Select/BeatmapCarousel.cs | 59 +++++++++---------- osu.Game/Screens/Select/SongSelect.cs | 2 +- 12 files changed, 42 insertions(+), 65 deletions(-) diff --git a/osu.Game.Tests/Beatmaps/IO/ImportBeatmapTest.cs b/osu.Game.Tests/Beatmaps/IO/ImportBeatmapTest.cs index 5fc05a4b2f..ad0ed00989 100644 --- a/osu.Game.Tests/Beatmaps/IO/ImportBeatmapTest.cs +++ b/osu.Game.Tests/Beatmaps/IO/ImportBeatmapTest.cs @@ -108,7 +108,7 @@ namespace osu.Game.Tests.Beatmaps.IO var manager = osu.Dependencies.Get(); // ReSharper disable once AccessToModifiedClosure - manager.ItemAdded += (_, __) => Interlocked.Increment(ref itemAddRemoveFireCount); + manager.ItemAdded += _ => Interlocked.Increment(ref itemAddRemoveFireCount); manager.ItemRemoved += _ => Interlocked.Increment(ref itemAddRemoveFireCount); var imported = await LoadOszIntoOsu(osu); diff --git a/osu.Game/Database/ArchiveModelManager.cs b/osu.Game/Database/ArchiveModelManager.cs index a64bca57cb..01455e7d50 100644 --- a/osu.Game/Database/ArchiveModelManager.cs +++ b/osu.Game/Database/ArchiveModelManager.cs @@ -44,7 +44,7 @@ namespace osu.Game.Database /// Fired when a new becomes available in the database. /// This is not guaranteed to run on the update thread. /// - public event Action ItemAdded; + public event Action ItemAdded; /// /// Fired when a is removed from the database. @@ -70,7 +70,7 @@ namespace osu.Game.Database ContextFactory = contextFactory; ModelStore = modelStore; - ModelStore.ItemAdded += item => handleEvent(() => ItemAdded?.Invoke(item, false)); + ModelStore.ItemAdded += item => handleEvent(() => ItemAdded?.Invoke(item)); ModelStore.ItemRemoved += s => handleEvent(() => ItemRemoved?.Invoke(s)); Files = new FileStore(contextFactory, storage); @@ -299,8 +299,6 @@ namespace osu.Game.Database { Undelete(existing); LogForModel(item, $"Found existing {HumanisedModelName} for {item} (ID {existing.ID}) – skipping import."); - handleEvent(() => ItemAdded?.Invoke(existing, true)); - // existing item will be used; rollback new import and exit early. rollback(); flushEvents(true); diff --git a/osu.Game/Database/IModelManager.cs b/osu.Game/Database/IModelManager.cs index e41ef3efef..884814cb38 100644 --- a/osu.Game/Database/IModelManager.cs +++ b/osu.Game/Database/IModelManager.cs @@ -12,7 +12,7 @@ namespace osu.Game.Database public interface IModelManager where TModel : class { - event Action ItemAdded; + event Action ItemAdded; event Action ItemRemoved; } diff --git a/osu.Game/OsuGameBase.cs b/osu.Game/OsuGameBase.cs index 87ff721bbb..3bdf37d769 100644 --- a/osu.Game/OsuGameBase.cs +++ b/osu.Game/OsuGameBase.cs @@ -183,7 +183,7 @@ namespace osu.Game } BeatmapManager.ItemRemoved += i => ScoreManager.Delete(getBeatmapScores(i), true); - BeatmapManager.ItemAdded += (i, existing) => ScoreManager.Undelete(getBeatmapScores(i), true); + BeatmapManager.ItemAdded += i => ScoreManager.Undelete(getBeatmapScores(i), true); dependencies.Cache(KeyBindingStore = new KeyBindingStore(contextFactory, RulesetStore)); dependencies.Cache(SettingsStore = new SettingsStore(contextFactory)); diff --git a/osu.Game/Overlays/Direct/DownloadTrackingComposite.cs b/osu.Game/Overlays/Direct/DownloadTrackingComposite.cs index 494b18307e..3e68f58837 100644 --- a/osu.Game/Overlays/Direct/DownloadTrackingComposite.cs +++ b/osu.Game/Overlays/Direct/DownloadTrackingComposite.cs @@ -118,7 +118,7 @@ namespace osu.Game.Overlays.Direct private void onRequestFailure(Exception e) => Schedule(() => attachDownload(null)); - private void setAdded(BeatmapSetInfo s, bool existing) => setDownloadStateFromManager(s, DownloadState.LocallyAvailable); + private void setAdded(BeatmapSetInfo s) => setDownloadStateFromManager(s, DownloadState.LocallyAvailable); private void setRemoved(BeatmapSetInfo s) => setDownloadStateFromManager(s, DownloadState.NotDownloaded); diff --git a/osu.Game/Overlays/Music/PlaylistList.cs b/osu.Game/Overlays/Music/PlaylistList.cs index 89d166b788..07040f166d 100644 --- a/osu.Game/Overlays/Music/PlaylistList.cs +++ b/osu.Game/Overlays/Music/PlaylistList.cs @@ -75,7 +75,7 @@ namespace osu.Game.Overlays.Music [BackgroundDependencyLoader] private void load(BeatmapManager beatmaps, IBindable beatmap) { - beatmaps.GetAllUsableBeatmapSets().ForEach(b => addBeatmapSet(b, false)); + beatmaps.GetAllUsableBeatmapSets().ForEach(addBeatmapSet); beatmaps.ItemAdded += addBeatmapSet; beatmaps.ItemRemoved += removeBeatmapSet; @@ -83,11 +83,8 @@ namespace osu.Game.Overlays.Music beatmapBacking.ValueChanged += _ => updateSelectedSet(); } - private void addBeatmapSet(BeatmapSetInfo obj, bool existing) => Schedule(() => + private void addBeatmapSet(BeatmapSetInfo obj) => Schedule(() => { - if (existing) - return; - var newItem = new PlaylistItem(obj) { OnSelect = set => Selected?.Invoke(set) }; items.Add(newItem); diff --git a/osu.Game/Overlays/MusicController.cs b/osu.Game/Overlays/MusicController.cs index 36937def2b..a312ab711d 100644 --- a/osu.Game/Overlays/MusicController.cs +++ b/osu.Game/Overlays/MusicController.cs @@ -218,15 +218,9 @@ namespace osu.Game.Overlays beatmapSets.Insert(index, beatmapSetInfo); } - private void handleBeatmapAdded(BeatmapSetInfo obj, bool existing) - { - if (existing) - return; + private void handleBeatmapAdded(BeatmapSetInfo set) => Schedule(() => beatmapSets.Add(set)); - Schedule(() => beatmapSets.Add(obj)); - } - - private void handleBeatmapRemoved(BeatmapSetInfo obj) => Schedule(() => beatmapSets.RemoveAll(s => s.ID == obj.ID)); + private void handleBeatmapRemoved(BeatmapSetInfo set) => Schedule(() => beatmapSets.RemoveAll(s => s.ID == set.ID)); protected override void LoadComplete() { diff --git a/osu.Game/Overlays/Settings/Sections/SkinSection.cs b/osu.Game/Overlays/Settings/Sections/SkinSection.cs index 51c687314a..5d7542ca2b 100644 --- a/osu.Game/Overlays/Settings/Sections/SkinSection.cs +++ b/osu.Game/Overlays/Settings/Sections/SkinSection.cs @@ -82,13 +82,7 @@ namespace osu.Game.Overlays.Settings.Sections private void itemRemoved(SkinInfo s) => Schedule(() => skinDropdown.Items = skinDropdown.Items.Where(i => i.ID != s.ID).ToArray()); - private void itemAdded(SkinInfo s, bool existing) - { - if (existing) - return; - - Schedule(() => skinDropdown.Items = skinDropdown.Items.Append(s).ToArray()); - } + private void itemAdded(SkinInfo s) => Schedule(() => skinDropdown.Items = skinDropdown.Items.Append(s).ToArray()); protected override void Dispose(bool isDisposing) { diff --git a/osu.Game/Screens/Multi/Match/Components/ReadyButton.cs b/osu.Game/Screens/Multi/Match/Components/ReadyButton.cs index 86b4cfbfa7..8ab0b8f61f 100644 --- a/osu.Game/Screens/Multi/Match/Components/ReadyButton.cs +++ b/osu.Game/Screens/Multi/Match/Components/ReadyButton.cs @@ -54,11 +54,8 @@ namespace osu.Game.Screens.Multi.Match.Components hasBeatmap = beatmaps.QueryBeatmap(b => b.OnlineBeatmapID == beatmap.OnlineBeatmapID) != null; } - private void beatmapAdded(BeatmapSetInfo model, bool existing) + private void beatmapAdded(BeatmapSetInfo model) { - if (Beatmap.Value == null) - return; - if (model.Beatmaps.Any(b => b.OnlineBeatmapID == Beatmap.Value.OnlineBeatmapID)) Schedule(() => hasBeatmap = true); } diff --git a/osu.Game/Screens/Multi/Match/MatchSubScreen.cs b/osu.Game/Screens/Multi/Match/MatchSubScreen.cs index a80056d164..ced9fcfa59 100644 --- a/osu.Game/Screens/Multi/Match/MatchSubScreen.cs +++ b/osu.Game/Screens/Multi/Match/MatchSubScreen.cs @@ -202,7 +202,7 @@ namespace osu.Game.Screens.Multi.Match /// /// Handle the case where a beatmap is imported (and can be used by this match). /// - private void beatmapAdded(BeatmapSetInfo model, bool existing) => Schedule(() => + private void beatmapAdded(BeatmapSetInfo model) => Schedule(() => { if (Beatmap.Value != beatmapManager.DefaultBeatmap) return; diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index cf21c78c7f..a6fbb1ff94 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -148,40 +148,37 @@ namespace osu.Game.Screens.Select }); } - public void UpdateBeatmapSet(BeatmapSetInfo beatmapSet) + public void UpdateBeatmapSet(BeatmapSetInfo beatmapSet) => Schedule(() => { - Schedule(() => + int? previouslySelectedID = null; + CarouselBeatmapSet existingSet = beatmapSets.FirstOrDefault(b => b.BeatmapSet.ID == beatmapSet.ID); + + // If the selected beatmap is about to be removed, store its ID so it can be re-selected if required + if (existingSet?.State?.Value == CarouselItemState.Selected) + previouslySelectedID = selectedBeatmap?.Beatmap.ID; + + var newSet = createCarouselSet(beatmapSet); + + if (existingSet != null) + root.RemoveChild(existingSet); + + if (newSet == null) { - int? previouslySelectedID = null; - CarouselBeatmapSet existingSet = beatmapSets.FirstOrDefault(b => b.BeatmapSet.ID == beatmapSet.ID); - - // If the selected beatmap is about to be removed, store its ID so it can be re-selected if required - if (existingSet?.State?.Value == CarouselItemState.Selected) - previouslySelectedID = selectedBeatmap?.Beatmap.ID; - - var newSet = createCarouselSet(beatmapSet); - - if (existingSet != null) - root.RemoveChild(existingSet); - - if (newSet == null) - { - itemsCache.Invalidate(); - return; - } - - root.AddChild(newSet); - - applyActiveCriteria(false, false); - - //check if we can/need to maintain our current selection. - if (previouslySelectedID != null) - select((CarouselItem)newSet.Beatmaps.FirstOrDefault(b => b.Beatmap.ID == previouslySelectedID) ?? newSet); - itemsCache.Invalidate(); - Schedule(() => BeatmapSetsChanged?.Invoke()); - }); - } + return; + } + + root.AddChild(newSet); + + applyActiveCriteria(false, false); + + //check if we can/need to maintain our current selection. + if (previouslySelectedID != null) + select((CarouselItem)newSet.Beatmaps.FirstOrDefault(b => b.Beatmap.ID == previouslySelectedID) ?? newSet); + + itemsCache.Invalidate(); + Schedule(() => BeatmapSetsChanged?.Invoke()); + }); /// /// Selects a given beatmap on the carousel. diff --git a/osu.Game/Screens/Select/SongSelect.cs b/osu.Game/Screens/Select/SongSelect.cs index dfda252deb..7c62a49a97 100644 --- a/osu.Game/Screens/Select/SongSelect.cs +++ b/osu.Game/Screens/Select/SongSelect.cs @@ -578,7 +578,7 @@ namespace osu.Game.Screens.Select } } - private void onBeatmapSetAdded(BeatmapSetInfo s, bool existing) => Carousel.UpdateBeatmapSet(s); + private void onBeatmapSetAdded(BeatmapSetInfo s) => Carousel.UpdateBeatmapSet(s); private void onBeatmapSetRemoved(BeatmapSetInfo s) => Carousel.RemoveBeatmapSet(s); private void onBeatmapRestored(BeatmapInfo b) => Carousel.UpdateBeatmapSet(beatmaps.QueryBeatmapSet(s => s.ID == b.BeatmapSetInfoID)); private void onBeatmapHidden(BeatmapInfo b) => Carousel.UpdateBeatmapSet(beatmaps.QueryBeatmapSet(s => s.ID == b.BeatmapSetInfoID)); From da65658bc3009405a33a39aea367de970903eb1a Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 26 Jun 2019 20:07:01 +0900 Subject: [PATCH 25/25] Fix comments --- osu.Game/Database/DownloadableArchiveModelManager.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game/Database/DownloadableArchiveModelManager.cs b/osu.Game/Database/DownloadableArchiveModelManager.cs index e0ac221cfc..647eb3cbd3 100644 --- a/osu.Game/Database/DownloadableArchiveModelManager.cs +++ b/osu.Game/Database/DownloadableArchiveModelManager.cs @@ -44,7 +44,7 @@ namespace osu.Game.Database /// Creates the download request for this . /// /// The to be downloaded. - /// Whether this download should be optimised for slow connections. Generally means extras are not included in the download bundle.. + /// Whether this download should be optimised for slow connections. Generally means extras are not included in the download bundle. /// The request object. protected abstract ArchiveDownloadRequest CreateDownloadRequest(TModel model, bool minimiseDownloadSize); @@ -52,7 +52,7 @@ namespace osu.Game.Database /// Begin a download for the requested . /// /// The to be downloaded. - /// Whether this download should be optimised for slow connections. Generally means extras are not included in the download bundle.. + /// Whether this download should be optimised for slow connections. Generally means extras are not included in the download bundle. /// Whether the download was started. public bool Download(TModel model, bool minimiseDownloadSize = false) {