From 9015ac6ba8d9fd25ceb23321c9a0e9d3dc551221 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 27 Oct 2021 19:52:41 +0900 Subject: [PATCH 01/15] Implement new version of download tracker --- osu.Game/Database/ModelDownloader.cs | 4 +- osu.Game/Online/BeatmapDownloadTracker.cs | 172 ++++++++++++++++++++++ osu.Game/Online/DownloadTracker.cs | 39 +++++ osu.Game/Online/ScoreDownloadTracker.cs | 172 ++++++++++++++++++++++ osu.Game/Scoring/ScoreInfo.cs | 2 +- 5 files changed, 386 insertions(+), 3 deletions(-) create mode 100644 osu.Game/Online/BeatmapDownloadTracker.cs create mode 100644 osu.Game/Online/DownloadTracker.cs create mode 100644 osu.Game/Online/ScoreDownloadTracker.cs diff --git a/osu.Game/Database/ModelDownloader.cs b/osu.Game/Database/ModelDownloader.cs index e613b39b6b..b9677c1b2b 100644 --- a/osu.Game/Database/ModelDownloader.cs +++ b/osu.Game/Database/ModelDownloader.cs @@ -15,7 +15,7 @@ using osu.Game.Overlays.Notifications; namespace osu.Game.Database { public abstract class ModelDownloader : IModelDownloader - where TModel : class, IHasPrimaryKey, ISoftDelete, IEquatable + where TModel : class, IHasPrimaryKey, ISoftDelete, IEquatable, IHasOnlineID { public Action PostNotification { protected get; set; } @@ -107,7 +107,7 @@ namespace osu.Game.Database } } - public ArchiveDownloadRequest GetExistingDownload(TModel model) => currentDownloads.Find(r => r.Model.Equals(model)); + public ArchiveDownloadRequest GetExistingDownload(TModel model) => currentDownloads.Find(r => r.Model.OnlineID == model.OnlineID); private bool canDownload(TModel model) => GetExistingDownload(model) == null && api != null; diff --git a/osu.Game/Online/BeatmapDownloadTracker.cs b/osu.Game/Online/BeatmapDownloadTracker.cs new file mode 100644 index 0000000000..969cb9af36 --- /dev/null +++ b/osu.Game/Online/BeatmapDownloadTracker.cs @@ -0,0 +1,172 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System; +using osu.Framework.Allocation; +using osu.Framework.Bindables; +using osu.Game.Beatmaps; +using osu.Game.Online.API; + +#nullable enable + +namespace osu.Game.Online +{ + public class BeatmapDownloadTracker : DownloadTracker + { + [Resolved(CanBeNull = true)] + protected BeatmapManager? Manager { get; private set; } + + private ArchiveDownloadRequest? attachedRequest; + + public BeatmapDownloadTracker(IBeatmapSetInfo trackedItem) + : base(trackedItem) + { + } + + private IBindable>? managerUpdated; + private IBindable>? managerRemoved; + private IBindable>>? managerDownloadBegan; + private IBindable>>? managerDownloadFailed; + + [BackgroundDependencyLoader(true)] + private void load() + { + // Used to interact with manager classes that don't support interface types. Will eventually be replaced. + var beatmapSetInfo = new BeatmapSetInfo { OnlineBeatmapSetID = TrackedItem.OnlineID }; + + if ((TrackedItem as BeatmapSetInfo)?.ID > 0 || Manager?.IsAvailableLocally(beatmapSetInfo) == true) + UpdateState(DownloadState.LocallyAvailable); + else if (Manager != null) + attachDownload(Manager.GetExistingDownload(beatmapSetInfo)); + + if (Manager != null) + { + managerDownloadBegan = Manager.DownloadBegan.GetBoundCopy(); + managerDownloadBegan.BindValueChanged(downloadBegan); + managerDownloadFailed = Manager.DownloadFailed.GetBoundCopy(); + managerDownloadFailed.BindValueChanged(downloadFailed); + managerUpdated = Manager.ItemUpdated.GetBoundCopy(); + managerUpdated.BindValueChanged(itemUpdated); + managerRemoved = Manager.ItemRemoved.GetBoundCopy(); + managerRemoved.BindValueChanged(itemRemoved); + } + } + + /// + /// Checks that a database model matches the one expected to be downloaded. + /// + /// + /// For online play, this could be used to check that the databased model matches the online beatmap. + /// + /// The model in database. + protected virtual bool VerifyDatabasedModel(BeatmapSetInfo databasedModel) => true; // TODO: do we still need this? + + private void downloadBegan(ValueChangedEvent>> weakRequest) + { + if (weakRequest.NewValue.TryGetTarget(out var request)) + { + Schedule(() => + { + if (checkEquality(request.Model, TrackedItem)) + attachDownload(request); + }); + } + } + + private void downloadFailed(ValueChangedEvent>> weakRequest) + { + if (weakRequest.NewValue.TryGetTarget(out var request)) + { + Schedule(() => + { + if (checkEquality(request.Model, TrackedItem)) + attachDownload(null); + }); + } + } + + private void attachDownload(ArchiveDownloadRequest? request) + { + if (attachedRequest != null) + { + attachedRequest.Failure -= onRequestFailure; + attachedRequest.DownloadProgressed -= onRequestProgress; + attachedRequest.Success -= onRequestSuccess; + } + + attachedRequest = request; + + if (attachedRequest != null) + { + if (attachedRequest.Progress == 1) + { + UpdateProgress(1); + UpdateState(DownloadState.Importing); + } + else + { + UpdateProgress(attachedRequest.Progress); + UpdateState(DownloadState.Downloading); + + attachedRequest.Failure += onRequestFailure; + attachedRequest.DownloadProgressed += onRequestProgress; + attachedRequest.Success += onRequestSuccess; + } + } + else + { + UpdateState(DownloadState.NotDownloaded); + } + } + + private void onRequestSuccess(string _) => Schedule(() => UpdateState(DownloadState.Importing)); + + private void onRequestProgress(float progress) => Schedule(() => UpdateProgress(progress)); + + private void onRequestFailure(Exception e) => Schedule(() => attachDownload(null)); + + private void itemUpdated(ValueChangedEvent> weakItem) + { + if (weakItem.NewValue.TryGetTarget(out var item)) + { + Schedule(() => + { + if (!checkEquality(item, TrackedItem)) + return; + + if (!VerifyDatabasedModel(item)) + { + UpdateState(DownloadState.NotDownloaded); + return; + } + + UpdateState(DownloadState.LocallyAvailable); + }); + } + } + + private void itemRemoved(ValueChangedEvent> weakItem) + { + if (weakItem.NewValue.TryGetTarget(out var item)) + { + Schedule(() => + { + if (checkEquality(item, TrackedItem)) + UpdateState(DownloadState.NotDownloaded); + }); + } + } + + private bool checkEquality(IBeatmapSetInfo x, IBeatmapSetInfo y) => x.OnlineID == y.OnlineID; + + #region Disposal + + protected override void Dispose(bool isDisposing) + { + base.Dispose(isDisposing); + attachDownload(null); + } + + #endregion + } +} diff --git a/osu.Game/Online/DownloadTracker.cs b/osu.Game/Online/DownloadTracker.cs new file mode 100644 index 0000000000..357c64b6a3 --- /dev/null +++ b/osu.Game/Online/DownloadTracker.cs @@ -0,0 +1,39 @@ +// 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.Bindables; +using osu.Framework.Graphics; + +#nullable enable + +namespace osu.Game.Online +{ + public abstract class DownloadTracker : Component + where T : class + { + public readonly T TrackedItem; + + /// + /// Holds the current download state of the download - whether is has already been downloaded, is in progress, or is not downloaded. + /// + public IBindable State => state; + + private readonly Bindable state = new Bindable(); + + /// + /// The progress of an active download. + /// + public IBindableNumber Progress => progress; + + private readonly BindableNumber progress = new BindableNumber { MinValue = 0, MaxValue = 1 }; + + protected DownloadTracker(T trackedItem) + { + TrackedItem = trackedItem; + } + + protected void UpdateState(DownloadState newState) => state.Value = newState; + + protected void UpdateProgress(double newProgress) => progress.Value = newProgress; + } +} diff --git a/osu.Game/Online/ScoreDownloadTracker.cs b/osu.Game/Online/ScoreDownloadTracker.cs new file mode 100644 index 0000000000..47cbabbee8 --- /dev/null +++ b/osu.Game/Online/ScoreDownloadTracker.cs @@ -0,0 +1,172 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System; +using osu.Framework.Allocation; +using osu.Framework.Bindables; +using osu.Game.Online.API; +using osu.Game.Scoring; + +#nullable enable + +namespace osu.Game.Online +{ + public class ScoreDownloadTracker : DownloadTracker + { + [Resolved(CanBeNull = true)] + protected ScoreManager? Manager { get; private set; } + + private ArchiveDownloadRequest? attachedRequest; + + public ScoreDownloadTracker(ScoreInfo trackedItem) + : base(trackedItem) + { + } + + private IBindable>? managerUpdated; + private IBindable>? managerRemoved; + private IBindable>>? managerDownloadBegan; + private IBindable>>? managerDownloadFailed; + + [BackgroundDependencyLoader(true)] + private void load() + { + // Used to interact with manager classes that don't support interface types. Will eventually be replaced. + var beatmapSetInfo = new ScoreInfo { OnlineScoreID = TrackedItem.OnlineScoreID }; + + if (TrackedItem.ID > 0 || Manager?.IsAvailableLocally(beatmapSetInfo) == true) + UpdateState(DownloadState.LocallyAvailable); + else if (Manager != null) + attachDownload(Manager.GetExistingDownload(beatmapSetInfo)); + + if (Manager != null) + { + managerDownloadBegan = Manager.DownloadBegan.GetBoundCopy(); + managerDownloadBegan.BindValueChanged(downloadBegan); + managerDownloadFailed = Manager.DownloadFailed.GetBoundCopy(); + managerDownloadFailed.BindValueChanged(downloadFailed); + managerUpdated = Manager.ItemUpdated.GetBoundCopy(); + managerUpdated.BindValueChanged(itemUpdated); + managerRemoved = Manager.ItemRemoved.GetBoundCopy(); + managerRemoved.BindValueChanged(itemRemoved); + } + } + + /// + /// Checks that a database model matches the one expected to be downloaded. + /// + /// + /// For online play, this could be used to check that the databased model matches the online beatmap. + /// + /// The model in database. + protected virtual bool VerifyDatabasedModel(ScoreInfo databasedModel) => true; // TODO: do we still need this? + + private void downloadBegan(ValueChangedEvent>> weakRequest) + { + if (weakRequest.NewValue.TryGetTarget(out var request)) + { + Schedule(() => + { + if (checkEquality(request.Model, TrackedItem)) + attachDownload(request); + }); + } + } + + private void downloadFailed(ValueChangedEvent>> weakRequest) + { + if (weakRequest.NewValue.TryGetTarget(out var request)) + { + Schedule(() => + { + if (checkEquality(request.Model, TrackedItem)) + attachDownload(null); + }); + } + } + + private void attachDownload(ArchiveDownloadRequest? request) + { + if (attachedRequest != null) + { + attachedRequest.Failure -= onRequestFailure; + attachedRequest.DownloadProgressed -= onRequestProgress; + attachedRequest.Success -= onRequestSuccess; + } + + attachedRequest = request; + + if (attachedRequest != null) + { + if (attachedRequest.Progress == 1) + { + UpdateProgress(1); + UpdateState(DownloadState.Importing); + } + else + { + UpdateProgress(attachedRequest.Progress); + UpdateState(DownloadState.Downloading); + + attachedRequest.Failure += onRequestFailure; + attachedRequest.DownloadProgressed += onRequestProgress; + attachedRequest.Success += onRequestSuccess; + } + } + else + { + UpdateState(DownloadState.NotDownloaded); + } + } + + private void onRequestSuccess(string _) => Schedule(() => UpdateState(DownloadState.Importing)); + + private void onRequestProgress(float progress) => Schedule(() => UpdateProgress(progress)); + + private void onRequestFailure(Exception e) => Schedule(() => attachDownload(null)); + + private void itemUpdated(ValueChangedEvent> weakItem) + { + if (weakItem.NewValue.TryGetTarget(out var item)) + { + Schedule(() => + { + if (!checkEquality(item, TrackedItem)) + return; + + if (!VerifyDatabasedModel(item)) + { + UpdateState(DownloadState.NotDownloaded); + return; + } + + UpdateState(DownloadState.LocallyAvailable); + }); + } + } + + private void itemRemoved(ValueChangedEvent> weakItem) + { + if (weakItem.NewValue.TryGetTarget(out var item)) + { + Schedule(() => + { + if (checkEquality(item, TrackedItem)) + UpdateState(DownloadState.NotDownloaded); + }); + } + } + + private bool checkEquality(ScoreInfo x, ScoreInfo y) => x.OnlineScoreID == y.OnlineScoreID; + + #region Disposal + + protected override void Dispose(bool isDisposing) + { + base.Dispose(isDisposing); + attachDownload(null); + } + + #endregion + } +} diff --git a/osu.Game/Scoring/ScoreInfo.cs b/osu.Game/Scoring/ScoreInfo.cs index 5cf22f7945..1bc04cd21e 100644 --- a/osu.Game/Scoring/ScoreInfo.cs +++ b/osu.Game/Scoring/ScoreInfo.cs @@ -19,7 +19,7 @@ using osu.Game.Utils; namespace osu.Game.Scoring { - public class ScoreInfo : IHasFiles, IHasPrimaryKey, ISoftDelete, IEquatable, IDeepCloneable + public class ScoreInfo : IHasFiles, IHasPrimaryKey, ISoftDelete, IEquatable, IDeepCloneable, IHasOnlineID { public int ID { get; set; } From 617e6febb6bb81c61fbf126089a836edf7860817 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 27 Oct 2021 20:02:51 +0900 Subject: [PATCH 02/15] Refactor `ModelDownloader` to allow for different `OnlineID matching` --- osu.Game/Beatmaps/BeatmapModelDownloader.cs | 3 +++ osu.Game/Database/ModelDownloader.cs | 12 ++++++------ osu.Game/Scoring/ScoreInfo.cs | 2 +- osu.Game/Scoring/ScoreModelDownloader.cs | 3 +++ 4 files changed, 13 insertions(+), 7 deletions(-) diff --git a/osu.Game/Beatmaps/BeatmapModelDownloader.cs b/osu.Game/Beatmaps/BeatmapModelDownloader.cs index 30dc95a966..001726e741 100644 --- a/osu.Game/Beatmaps/BeatmapModelDownloader.cs +++ b/osu.Game/Beatmaps/BeatmapModelDownloader.cs @@ -13,6 +13,9 @@ namespace osu.Game.Beatmaps protected override ArchiveDownloadRequest CreateDownloadRequest(BeatmapSetInfo set, bool minimiseDownloadSize) => new DownloadBeatmapSetRequest(set, minimiseDownloadSize); + public override ArchiveDownloadRequest GetExistingDownload(BeatmapSetInfo model) + => CurrentDownloads.Find(r => r.Model.OnlineID == model.OnlineID); + public BeatmapModelDownloader(IBeatmapModelManager beatmapModelManager, IAPIProvider api, GameHost host = null) : base(beatmapModelManager, api, host) { diff --git a/osu.Game/Database/ModelDownloader.cs b/osu.Game/Database/ModelDownloader.cs index b9677c1b2b..12bf5e9ce7 100644 --- a/osu.Game/Database/ModelDownloader.cs +++ b/osu.Game/Database/ModelDownloader.cs @@ -15,7 +15,7 @@ using osu.Game.Overlays.Notifications; namespace osu.Game.Database { public abstract class ModelDownloader : IModelDownloader - where TModel : class, IHasPrimaryKey, ISoftDelete, IEquatable, IHasOnlineID + where TModel : class, IHasPrimaryKey, ISoftDelete, IEquatable { public Action PostNotification { protected get; set; } @@ -30,7 +30,7 @@ namespace osu.Game.Database private readonly IModelManager modelManager; private readonly IAPIProvider api; - private readonly List> currentDownloads = new List>(); + protected readonly List> CurrentDownloads = new List>(); protected ModelDownloader(IModelManager modelManager, IAPIProvider api, IIpcHost importHost = null) { @@ -74,7 +74,7 @@ namespace osu.Game.Database if (!imported.Any()) downloadFailed.Value = new WeakReference>(request); - currentDownloads.Remove(request); + CurrentDownloads.Remove(request); }, TaskCreationOptions.LongRunning); }; @@ -86,7 +86,7 @@ namespace osu.Game.Database return true; }; - currentDownloads.Add(request); + CurrentDownloads.Add(request); PostNotification?.Invoke(notification); api.PerformAsync(request); @@ -96,7 +96,7 @@ namespace osu.Game.Database void triggerFailure(Exception error) { - currentDownloads.Remove(request); + CurrentDownloads.Remove(request); downloadFailed.Value = new WeakReference>(request); @@ -107,7 +107,7 @@ namespace osu.Game.Database } } - public ArchiveDownloadRequest GetExistingDownload(TModel model) => currentDownloads.Find(r => r.Model.OnlineID == model.OnlineID); + public abstract ArchiveDownloadRequest GetExistingDownload(TModel model); private bool canDownload(TModel model) => GetExistingDownload(model) == null && api != null; diff --git a/osu.Game/Scoring/ScoreInfo.cs b/osu.Game/Scoring/ScoreInfo.cs index 1bc04cd21e..5cf22f7945 100644 --- a/osu.Game/Scoring/ScoreInfo.cs +++ b/osu.Game/Scoring/ScoreInfo.cs @@ -19,7 +19,7 @@ using osu.Game.Utils; namespace osu.Game.Scoring { - public class ScoreInfo : IHasFiles, IHasPrimaryKey, ISoftDelete, IEquatable, IDeepCloneable, IHasOnlineID + public class ScoreInfo : IHasFiles, IHasPrimaryKey, ISoftDelete, IEquatable, IDeepCloneable { public int ID { get; set; } diff --git a/osu.Game/Scoring/ScoreModelDownloader.cs b/osu.Game/Scoring/ScoreModelDownloader.cs index b3c1e2928a..52355585a9 100644 --- a/osu.Game/Scoring/ScoreModelDownloader.cs +++ b/osu.Game/Scoring/ScoreModelDownloader.cs @@ -16,5 +16,8 @@ namespace osu.Game.Scoring } protected override ArchiveDownloadRequest CreateDownloadRequest(ScoreInfo score, bool minimiseDownload) => new DownloadReplayRequest(score); + + public override ArchiveDownloadRequest GetExistingDownload(ScoreInfo model) + => CurrentDownloads.Find(r => r.Model.OnlineScoreID == model.OnlineScoreID); } } From 692db9a9ec2f424153e0374923247edeee0eb98e Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 27 Oct 2021 20:21:00 +0900 Subject: [PATCH 03/15] Remove unnecessary verification flow from score download tracking --- osu.Game/Online/BeatmapDownloadTracker.cs | 15 --------------- osu.Game/Online/ScoreDownloadTracker.cs | 17 +---------------- 2 files changed, 1 insertion(+), 31 deletions(-) diff --git a/osu.Game/Online/BeatmapDownloadTracker.cs b/osu.Game/Online/BeatmapDownloadTracker.cs index 969cb9af36..6df74efe9c 100644 --- a/osu.Game/Online/BeatmapDownloadTracker.cs +++ b/osu.Game/Online/BeatmapDownloadTracker.cs @@ -52,15 +52,6 @@ namespace osu.Game.Online } } - /// - /// Checks that a database model matches the one expected to be downloaded. - /// - /// - /// For online play, this could be used to check that the databased model matches the online beatmap. - /// - /// The model in database. - protected virtual bool VerifyDatabasedModel(BeatmapSetInfo databasedModel) => true; // TODO: do we still need this? - private void downloadBegan(ValueChangedEvent>> weakRequest) { if (weakRequest.NewValue.TryGetTarget(out var request)) @@ -134,12 +125,6 @@ namespace osu.Game.Online if (!checkEquality(item, TrackedItem)) return; - if (!VerifyDatabasedModel(item)) - { - UpdateState(DownloadState.NotDownloaded); - return; - } - UpdateState(DownloadState.LocallyAvailable); }); } diff --git a/osu.Game/Online/ScoreDownloadTracker.cs b/osu.Game/Online/ScoreDownloadTracker.cs index 47cbabbee8..7a714a7a0e 100644 --- a/osu.Game/Online/ScoreDownloadTracker.cs +++ b/osu.Game/Online/ScoreDownloadTracker.cs @@ -52,15 +52,6 @@ namespace osu.Game.Online } } - /// - /// Checks that a database model matches the one expected to be downloaded. - /// - /// - /// For online play, this could be used to check that the databased model matches the online beatmap. - /// - /// The model in database. - protected virtual bool VerifyDatabasedModel(ScoreInfo databasedModel) => true; // TODO: do we still need this? - private void downloadBegan(ValueChangedEvent>> weakRequest) { if (weakRequest.NewValue.TryGetTarget(out var request)) @@ -134,13 +125,7 @@ namespace osu.Game.Online if (!checkEquality(item, TrackedItem)) return; - if (!VerifyDatabasedModel(item)) - { - UpdateState(DownloadState.NotDownloaded); - return; - } - - UpdateState(DownloadState.LocallyAvailable); + UpdateState(DownloadState.NotDownloaded); }); } } From 6339064dbd152ac4ebf121e8911037b32c9756b9 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 27 Oct 2021 20:07:00 +0900 Subject: [PATCH 04/15] Remove old versions of `DownloadTrackingComposite` --- osu.Game/Online/DownloadTrackingComposite.cs | 196 ------------------ .../BeatmapDownloadTrackingComposite.cs | 19 -- 2 files changed, 215 deletions(-) delete mode 100644 osu.Game/Online/DownloadTrackingComposite.cs delete mode 100644 osu.Game/Overlays/BeatmapDownloadTrackingComposite.cs diff --git a/osu.Game/Online/DownloadTrackingComposite.cs b/osu.Game/Online/DownloadTrackingComposite.cs deleted file mode 100644 index 2a96051427..0000000000 --- a/osu.Game/Online/DownloadTrackingComposite.cs +++ /dev/null @@ -1,196 +0,0 @@ -// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. -// See the LICENCE file in the repository root for full licence text. - -using System; -using JetBrains.Annotations; -using osu.Framework.Allocation; -using osu.Framework.Bindables; -using osu.Framework.Graphics.Containers; -using osu.Game.Database; -using osu.Game.Online.API; - -namespace osu.Game.Online -{ - /// - /// A component which tracks a through potential download/import/deletion. - /// - public abstract class DownloadTrackingComposite : CompositeDrawable - where TModel : class, IEquatable - where TModelManager : class, IModelDownloader, IModelManager - { - protected readonly Bindable Model = new Bindable(); - - [Resolved(CanBeNull = true)] - protected TModelManager Manager { get; private set; } - - /// - /// Holds the current download state of the , whether is has already been downloaded, is in progress, or is not downloaded. - /// - protected readonly Bindable State = new Bindable(); - - protected readonly BindableNumber Progress = new BindableNumber { MinValue = 0, MaxValue = 1 }; - - protected DownloadTrackingComposite(TModel model = null) - { - Model.Value = model; - } - - private IBindable> managerUpdated; - private IBindable> managerRemoved; - private IBindable>> managerDownloadBegan; - private IBindable>> managerDownloadFailed; - - [BackgroundDependencyLoader(true)] - private void load() - { - Model.BindValueChanged(modelInfo => - { - if (modelInfo.NewValue == null) - attachDownload(null); - else if (IsModelAvailableLocally()) - State.Value = DownloadState.LocallyAvailable; - else - attachDownload(Manager?.GetExistingDownload(modelInfo.NewValue)); - }, true); - - if (Manager == null) - return; - - managerDownloadBegan = Manager.DownloadBegan.GetBoundCopy(); - managerDownloadBegan.BindValueChanged(downloadBegan); - managerDownloadFailed = Manager.DownloadFailed.GetBoundCopy(); - managerDownloadFailed.BindValueChanged(downloadFailed); - managerUpdated = Manager.ItemUpdated.GetBoundCopy(); - managerUpdated.BindValueChanged(itemUpdated); - managerRemoved = Manager.ItemRemoved.GetBoundCopy(); - managerRemoved.BindValueChanged(itemRemoved); - } - - /// - /// Checks that a database model matches the one expected to be downloaded. - /// - /// - /// For online play, this could be used to check that the databased model matches the online beatmap. - /// - /// The model in database. - protected virtual bool VerifyDatabasedModel([NotNull] TModel databasedModel) => true; - - /// - /// Whether the given model is available in the database. - /// By default, this calls , - /// but can be overriden to add additional checks for verifying the model in database. - /// - protected virtual bool IsModelAvailableLocally() => Manager?.IsAvailableLocally(Model.Value) == true; - - private void downloadBegan(ValueChangedEvent>> weakRequest) - { - if (weakRequest.NewValue.TryGetTarget(out var request)) - { - Schedule(() => - { - if (request.Model.Equals(Model.Value)) - attachDownload(request); - }); - } - } - - private void downloadFailed(ValueChangedEvent>> weakRequest) - { - if (weakRequest.NewValue.TryGetTarget(out var request)) - { - Schedule(() => - { - if (request.Model.Equals(Model.Value)) - attachDownload(null); - }); - } - } - - private ArchiveDownloadRequest attachedRequest; - - private void attachDownload(ArchiveDownloadRequest request) - { - if (attachedRequest != null) - { - attachedRequest.Failure -= onRequestFailure; - attachedRequest.DownloadProgressed -= onRequestProgress; - attachedRequest.Success -= onRequestSuccess; - } - - attachedRequest = request; - - if (attachedRequest != null) - { - if (attachedRequest.Progress == 1) - { - Progress.Value = 1; - State.Value = DownloadState.Importing; - } - else - { - Progress.Value = attachedRequest.Progress; - State.Value = DownloadState.Downloading; - - attachedRequest.Failure += onRequestFailure; - attachedRequest.DownloadProgressed += onRequestProgress; - attachedRequest.Success += onRequestSuccess; - } - } - else - { - State.Value = DownloadState.NotDownloaded; - } - } - - private void onRequestSuccess(string _) => Schedule(() => State.Value = DownloadState.Importing); - - private void onRequestProgress(float progress) => Schedule(() => Progress.Value = progress); - - private void onRequestFailure(Exception e) => Schedule(() => attachDownload(null)); - - private void itemUpdated(ValueChangedEvent> weakItem) - { - if (weakItem.NewValue.TryGetTarget(out var item)) - { - Schedule(() => - { - if (!item.Equals(Model.Value)) - return; - - if (!VerifyDatabasedModel(item)) - { - State.Value = DownloadState.NotDownloaded; - return; - } - - State.Value = DownloadState.LocallyAvailable; - }); - } - } - - private void itemRemoved(ValueChangedEvent> weakItem) - { - if (weakItem.NewValue.TryGetTarget(out var item)) - { - Schedule(() => - { - if (item.Equals(Model.Value)) - State.Value = DownloadState.NotDownloaded; - }); - } - } - - #region Disposal - - protected override void Dispose(bool isDisposing) - { - base.Dispose(isDisposing); - - State.UnbindAll(); - - attachDownload(null); - } - - #endregion - } -} diff --git a/osu.Game/Overlays/BeatmapDownloadTrackingComposite.cs b/osu.Game/Overlays/BeatmapDownloadTrackingComposite.cs deleted file mode 100644 index f6b5b181c3..0000000000 --- a/osu.Game/Overlays/BeatmapDownloadTrackingComposite.cs +++ /dev/null @@ -1,19 +0,0 @@ -// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. -// See the LICENCE file in the repository root for full licence text. - -using osu.Framework.Bindables; -using osu.Game.Beatmaps; -using osu.Game.Online; - -namespace osu.Game.Overlays -{ - public abstract class BeatmapDownloadTrackingComposite : DownloadTrackingComposite - { - public Bindable BeatmapSet => Model; - - protected BeatmapDownloadTrackingComposite(BeatmapSetInfo set = null) - : base(set) - { - } - } -} From dd06c617b149f2459b209640808b6a88d9bee440 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 27 Oct 2021 20:19:34 +0900 Subject: [PATCH 05/15] Update `OnlinePlayBeatmapAvailabilityTracker` in line with new download tracker The structure here has changed a bit - rather than deriving from the donwload tracker I moved the additional logic into the callbacks. I think this feels better? --- .../OnlinePlayBeatmapAvailabilityTracker.cs | 93 ++++++++++--------- 1 file changed, 48 insertions(+), 45 deletions(-) diff --git a/osu.Game/Online/Rooms/OnlinePlayBeatmapAvailabilityTracker.cs b/osu.Game/Online/Rooms/OnlinePlayBeatmapAvailabilityTracker.cs index 52aa115083..6dd1204555 100644 --- a/osu.Game/Online/Rooms/OnlinePlayBeatmapAvailabilityTracker.cs +++ b/osu.Game/Online/Rooms/OnlinePlayBeatmapAvailabilityTracker.cs @@ -2,8 +2,9 @@ // See the LICENCE file in the repository root for full licence text. using System; -using System.Linq; +using osu.Framework.Allocation; using osu.Framework.Bindables; +using osu.Framework.Graphics.Containers; using osu.Framework.Logging; using osu.Framework.Threading; using osu.Game.Beatmaps; @@ -16,10 +17,13 @@ namespace osu.Game.Online.Rooms /// This differs from a regular download tracking composite as this accounts for the /// databased beatmap set's checksum, to disallow from playing with an altered version of the beatmap. /// - public class OnlinePlayBeatmapAvailabilityTracker : DownloadTrackingComposite + public class OnlinePlayBeatmapAvailabilityTracker : CompositeDrawable { public readonly IBindable SelectedItem = new Bindable(); + [Resolved] + private BeatmapManager beatmapManager { get; set; } + /// /// The availability state of the currently selected playlist item. /// @@ -29,6 +33,8 @@ namespace osu.Game.Online.Rooms private ScheduledDelegate progressUpdate; + private BeatmapDownloadTracker downloadTracker; + protected override void LoadComplete() { base.LoadComplete(); @@ -40,58 +46,39 @@ namespace osu.Game.Online.Rooms if (item.NewValue == null) return; - Model.Value = item.NewValue.Beatmap.Value.BeatmapSet; + downloadTracker?.Expire(); + downloadTracker = new BeatmapDownloadTracker(item.NewValue.Beatmap.Value.BeatmapSet); + + downloadTracker.Progress.BindValueChanged(_ => + { + if (downloadTracker.State.Value != DownloadState.Downloading) + return; + + // incoming progress changes are going to be at a very high rate. + // we don't want to flood the network with this, so rate limit how often we send progress updates. + if (progressUpdate?.Completed != false) + progressUpdate = Scheduler.AddDelayed(updateAvailability, progressUpdate == null ? 0 : 500); + }); + + downloadTracker.State.BindValueChanged(_ => updateAvailability(), true); + + AddInternal(downloadTracker); }, true); - - Progress.BindValueChanged(_ => - { - if (State.Value != DownloadState.Downloading) - return; - - // incoming progress changes are going to be at a very high rate. - // we don't want to flood the network with this, so rate limit how often we send progress updates. - if (progressUpdate?.Completed != false) - progressUpdate = Scheduler.AddDelayed(updateAvailability, progressUpdate == null ? 0 : 500); - }); - - State.BindValueChanged(_ => updateAvailability(), true); - } - - protected override bool VerifyDatabasedModel(BeatmapSetInfo databasedSet) - { - int beatmapId = SelectedItem.Value?.Beatmap.Value.OnlineID ?? -1; - string checksum = SelectedItem.Value?.Beatmap.Value.MD5Hash; - - var matchingBeatmap = databasedSet.Beatmaps.FirstOrDefault(b => b.OnlineBeatmapID == beatmapId && b.MD5Hash == checksum); - - if (matchingBeatmap == null) - { - Logger.Log("The imported beatmap set does not match the online version.", LoggingTarget.Runtime, LogLevel.Important); - return false; - } - - return true; - } - - protected override bool IsModelAvailableLocally() - { - int onlineId = SelectedItem.Value.Beatmap.Value.OnlineID; - string checksum = SelectedItem.Value.Beatmap.Value.MD5Hash; - - var beatmap = Manager.QueryBeatmap(b => b.OnlineBeatmapID == onlineId && b.MD5Hash == checksum); - return beatmap?.BeatmapSet.DeletePending == false; } private void updateAvailability() { - switch (State.Value) + if (downloadTracker == null) + return; + + switch (downloadTracker.State.Value) { case DownloadState.NotDownloaded: availability.Value = BeatmapAvailability.NotDownloaded(); break; case DownloadState.Downloading: - availability.Value = BeatmapAvailability.Downloading((float)Progress.Value); + availability.Value = BeatmapAvailability.Downloading((float)downloadTracker.Progress.Value); break; case DownloadState.Importing: @@ -99,12 +86,28 @@ namespace osu.Game.Online.Rooms break; case DownloadState.LocallyAvailable: - availability.Value = BeatmapAvailability.LocallyAvailable(); + bool hashMatches = checkHashValidity(); + + availability.Value = hashMatches ? BeatmapAvailability.LocallyAvailable() : BeatmapAvailability.NotDownloaded(); + + // only display a message to the user if a download seems to have just completed. + if (!hashMatches && downloadTracker.Progress.Value == 1) + Logger.Log("The imported beatmap set does not match the online version.", LoggingTarget.Runtime, LogLevel.Important); + break; default: - throw new ArgumentOutOfRangeException(nameof(State)); + throw new ArgumentOutOfRangeException(); } } + + private bool checkHashValidity() + { + int onlineId = SelectedItem.Value.Beatmap.Value.OnlineID; + string checksum = SelectedItem.Value.Beatmap.Value.MD5Hash; + + var beatmap = beatmapManager.QueryBeatmap(b => b.OnlineBeatmapID == onlineId && b.MD5Hash == checksum); + return beatmap?.BeatmapSet.DeletePending == false; + } } } From f014ceaead3556b47c8e0ea510cb3382206455a8 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 27 Oct 2021 20:49:15 +0900 Subject: [PATCH 06/15] Update remaining usages of download tracking --- .../Online/TestSceneDirectDownloadButton.cs | 3 +- .../Panels/BeatmapPanelDownloadButton.cs | 36 +++++++++++++------ .../Panels/DownloadProgressBar.cs | 18 +++++++--- .../BeatmapSet/BeatmapSetHeaderContent.cs | 19 +++++++--- .../Buttons/HeaderDownloadButton.cs | 33 ++++++++++------- .../OnlinePlay/DrawableRoomPlaylistItem.cs | 2 +- .../Screens/Ranking/ReplayDownloadButton.cs | 35 ++++++++++++------ 7 files changed, 100 insertions(+), 46 deletions(-) diff --git a/osu.Game.Tests/Visual/Online/TestSceneDirectDownloadButton.cs b/osu.Game.Tests/Visual/Online/TestSceneDirectDownloadButton.cs index bb7fcc2fce..879e35057b 100644 --- a/osu.Game.Tests/Visual/Online/TestSceneDirectDownloadButton.cs +++ b/osu.Game.Tests/Visual/Online/TestSceneDirectDownloadButton.cs @@ -5,6 +5,7 @@ using System.Linq; using NUnit.Framework; using osu.Framework.Allocation; using osu.Framework.Graphics; +using osu.Framework.Testing; using osu.Game.Beatmaps; using osu.Game.Online; using osu.Game.Online.API.Requests.Responses; @@ -144,7 +145,7 @@ namespace osu.Game.Tests.Visual.Online { public new bool DownloadEnabled => base.DownloadEnabled; - public DownloadState DownloadState => State.Value; + public DownloadState DownloadState => this.ChildrenOfType().First().State.Value; public TestDownloadButton(BeatmapSetInfo beatmapSet) : base(beatmapSet) diff --git a/osu.Game/Overlays/BeatmapListing/Panels/BeatmapPanelDownloadButton.cs b/osu.Game/Overlays/BeatmapListing/Panels/BeatmapPanelDownloadButton.cs index a8c4334ffb..5c6472cd1b 100644 --- a/osu.Game/Overlays/BeatmapListing/Panels/BeatmapPanelDownloadButton.cs +++ b/osu.Game/Overlays/BeatmapListing/Panels/BeatmapPanelDownloadButton.cs @@ -5,6 +5,7 @@ using System; using osu.Framework.Allocation; using osu.Framework.Bindables; using osu.Framework.Graphics; +using osu.Framework.Graphics.Containers; using osu.Game.Beatmaps; using osu.Game.Configuration; using osu.Game.Graphics.Containers; @@ -13,7 +14,7 @@ using osu.Game.Online; namespace osu.Game.Overlays.BeatmapListing.Panels { - public class BeatmapPanelDownloadButton : BeatmapDownloadTrackingComposite + public class BeatmapPanelDownloadButton : CompositeDrawable { protected bool DownloadEnabled => button.Enabled.Value; @@ -26,16 +27,29 @@ namespace osu.Game.Overlays.BeatmapListing.Panels private readonly DownloadButton button; private Bindable noVideoSetting; + protected readonly BeatmapDownloadTracker DownloadTracker; + + protected readonly Bindable State = new Bindable(); + + private readonly BeatmapSetInfo beatmapSet; + public BeatmapPanelDownloadButton(BeatmapSetInfo beatmapSet) - : base(beatmapSet) { - InternalChild = shakeContainer = new ShakeContainer + this.beatmapSet = beatmapSet; + InternalChildren = new Drawable[] { - RelativeSizeAxes = Axes.Both, - Child = button = new DownloadButton + shakeContainer = new ShakeContainer { RelativeSizeAxes = Axes.Both, + Child = button = new DownloadButton + { + RelativeSizeAxes = Axes.Both, + }, }, + DownloadTracker = new BeatmapDownloadTracker(beatmapSet) + { + State = { BindTarget = State } + } }; button.Add(new DownloadProgressBar(beatmapSet) @@ -50,7 +64,7 @@ namespace osu.Game.Overlays.BeatmapListing.Panels { base.LoadComplete(); - button.State.BindTo(State); + ((IBindable)button.State).BindTo(DownloadTracker.State); FinishTransforms(true); } @@ -61,7 +75,7 @@ namespace osu.Game.Overlays.BeatmapListing.Panels button.Action = () => { - switch (State.Value) + switch (DownloadTracker.State.Value) { case DownloadState.Downloading: case DownloadState.Importing: @@ -73,16 +87,16 @@ namespace osu.Game.Overlays.BeatmapListing.Panels if (SelectedBeatmap.Value != null) findPredicate = b => b.OnlineBeatmapID == SelectedBeatmap.Value.OnlineBeatmapID; - game?.PresentBeatmap(BeatmapSet.Value, findPredicate); + game?.PresentBeatmap(beatmapSet, findPredicate); break; default: - beatmaps.Download(BeatmapSet.Value, noVideoSetting.Value); + beatmaps.Download(beatmapSet, noVideoSetting.Value); break; } }; - State.BindValueChanged(state => + DownloadTracker.State.BindValueChanged(state => { switch (state.NewValue) { @@ -92,7 +106,7 @@ namespace osu.Game.Overlays.BeatmapListing.Panels break; default: - if (BeatmapSet.Value?.OnlineInfo?.Availability.DownloadDisabled ?? false) + if (beatmapSet.OnlineInfo?.Availability.DownloadDisabled ?? false) { button.Enabled.Value = false; button.TooltipText = "this beatmap is currently not available for download."; diff --git a/osu.Game/Overlays/BeatmapListing/Panels/DownloadProgressBar.cs b/osu.Game/Overlays/BeatmapListing/Panels/DownloadProgressBar.cs index ca94078401..24f929f55e 100644 --- a/osu.Game/Overlays/BeatmapListing/Panels/DownloadProgressBar.cs +++ b/osu.Game/Overlays/BeatmapListing/Panels/DownloadProgressBar.cs @@ -4,6 +4,7 @@ using osu.Framework.Allocation; using osu.Framework.Extensions.Color4Extensions; using osu.Framework.Graphics; +using osu.Framework.Graphics.Containers; using osu.Game.Beatmaps; using osu.Game.Graphics; using osu.Game.Graphics.UserInterface; @@ -12,13 +13,22 @@ using osuTK.Graphics; namespace osu.Game.Overlays.BeatmapListing.Panels { - public class DownloadProgressBar : BeatmapDownloadTrackingComposite + public class DownloadProgressBar : CompositeDrawable { private readonly ProgressBar progressBar; + private readonly BeatmapDownloadTracker downloadTracker; public DownloadProgressBar(BeatmapSetInfo beatmapSet) - : base(beatmapSet) { + InternalChildren = new Drawable[] + { + progressBar = new ProgressBar(false) + { + Height = 0, + Alpha = 0, + }, + downloadTracker = new BeatmapDownloadTracker(beatmapSet), + }; AddInternal(progressBar = new ProgressBar(false) { Height = 0, @@ -34,9 +44,9 @@ namespace osu.Game.Overlays.BeatmapListing.Panels { progressBar.FillColour = colours.Blue; progressBar.BackgroundColour = Color4.Black.Opacity(0.7f); - progressBar.Current = Progress; + progressBar.Current.BindTarget = downloadTracker.Progress; - State.BindValueChanged(state => + downloadTracker.State.BindValueChanged(state => { switch (state.NewValue) { diff --git a/osu.Game/Overlays/BeatmapSet/BeatmapSetHeaderContent.cs b/osu.Game/Overlays/BeatmapSet/BeatmapSetHeaderContent.cs index 6f85846720..e0a09ed520 100644 --- a/osu.Game/Overlays/BeatmapSet/BeatmapSetHeaderContent.cs +++ b/osu.Game/Overlays/BeatmapSet/BeatmapSetHeaderContent.cs @@ -3,12 +3,14 @@ using System.Linq; using osu.Framework.Allocation; +using osu.Framework.Bindables; using osu.Framework.Extensions.Color4Extensions; using osu.Framework.Graphics; using osu.Framework.Graphics.Colour; using osu.Framework.Graphics.Containers; using osu.Framework.Graphics.Shapes; using osu.Framework.Localisation; +using osu.Game.Beatmaps; using osu.Game.Beatmaps.Drawables; using osu.Game.Graphics; using osu.Game.Graphics.Sprites; @@ -21,8 +23,10 @@ using osuTK; namespace osu.Game.Overlays.BeatmapSet { - public class BeatmapSetHeaderContent : BeatmapDownloadTrackingComposite + public class BeatmapSetHeaderContent : CompositeDrawable { + public readonly Bindable BeatmapSet = new Bindable(); + private const float transition_duration = 200; private const float buttons_height = 45; private const float buttons_spacing = 5; @@ -45,6 +49,8 @@ namespace osu.Game.Overlays.BeatmapSet private readonly FillFlowContainer fadeContent; private readonly LoadingSpinner loading; + private BeatmapDownloadTracker downloadTracker; + [Resolved] private IAPIProvider api { get; set; } @@ -222,13 +228,13 @@ namespace osu.Game.Overlays.BeatmapSet { coverGradient.Colour = ColourInfo.GradientVertical(colourProvider.Background6.Opacity(0.3f), colourProvider.Background6.Opacity(0.8f)); - State.BindValueChanged(_ => updateDownloadButtons()); - BeatmapSet.BindValueChanged(setInfo => { Picker.BeatmapSet = rulesetSelector.BeatmapSet = author.BeatmapSet = beatmapAvailability.BeatmapSet = Details.BeatmapSet = setInfo.NewValue; cover.BeatmapSet = setInfo.NewValue; + downloadTracker?.Expire(); + if (setInfo.NewValue == null) { onlineStatusPill.FadeTo(0.5f, 500, Easing.OutQuint); @@ -241,6 +247,9 @@ namespace osu.Game.Overlays.BeatmapSet } else { + downloadTracker = new BeatmapDownloadTracker(setInfo.NewValue); + downloadTracker.State.BindValueChanged(_ => updateDownloadButtons()); + fadeContent.FadeIn(500, Easing.OutQuint); loading.Hide(); @@ -266,13 +275,13 @@ namespace osu.Game.Overlays.BeatmapSet { if (BeatmapSet.Value == null) return; - if (BeatmapSet.Value.OnlineInfo.Availability.DownloadDisabled && State.Value != DownloadState.LocallyAvailable) + if (BeatmapSet.Value.OnlineInfo.Availability.DownloadDisabled && downloadTracker.State.Value != DownloadState.LocallyAvailable) { downloadButtonsContainer.Clear(); return; } - switch (State.Value) + switch (downloadTracker.State.Value) { case DownloadState.LocallyAvailable: // temporary for UX until new design is implemented. diff --git a/osu.Game/Overlays/BeatmapSet/Buttons/HeaderDownloadButton.cs b/osu.Game/Overlays/BeatmapSet/Buttons/HeaderDownloadButton.cs index e7a55079ec..88d0778ae4 100644 --- a/osu.Game/Overlays/BeatmapSet/Buttons/HeaderDownloadButton.cs +++ b/osu.Game/Overlays/BeatmapSet/Buttons/HeaderDownloadButton.cs @@ -22,7 +22,7 @@ using osuTK.Graphics; namespace osu.Game.Overlays.BeatmapSet.Buttons { - public class HeaderDownloadButton : BeatmapDownloadTrackingComposite, IHasTooltip + public class HeaderDownloadButton : CompositeDrawable, IHasTooltip { private const int text_size = 12; @@ -35,9 +35,12 @@ namespace osu.Game.Overlays.BeatmapSet.Buttons private ShakeContainer shakeContainer; private HeaderButton button; + private BeatmapDownloadTracker downloadTracker; + private readonly BeatmapSetInfo beatmapSet; + public HeaderDownloadButton(BeatmapSetInfo beatmapSet, bool noVideo = false) - : base(beatmapSet) { + this.beatmapSet = beatmapSet; this.noVideo = noVideo; Width = 120; @@ -49,13 +52,17 @@ namespace osu.Game.Overlays.BeatmapSet.Buttons { FillFlowContainer textSprites; - AddInternal(shakeContainer = new ShakeContainer + InternalChildren = new Drawable[] { - RelativeSizeAxes = Axes.Both, - Masking = true, - CornerRadius = 5, - Child = button = new HeaderButton { RelativeSizeAxes = Axes.Both }, - }); + shakeContainer = new ShakeContainer + { + RelativeSizeAxes = Axes.Both, + Masking = true, + CornerRadius = 5, + Child = button = new HeaderButton { RelativeSizeAxes = Axes.Both }, + }, + downloadTracker = new BeatmapDownloadTracker(beatmapSet), + }; button.AddRange(new Drawable[] { @@ -83,7 +90,7 @@ namespace osu.Game.Overlays.BeatmapSet.Buttons }, } }, - new DownloadProgressBar(BeatmapSet.Value) + new DownloadProgressBar(beatmapSet) { Anchor = Anchor.BottomLeft, Origin = Anchor.BottomLeft, @@ -92,20 +99,20 @@ namespace osu.Game.Overlays.BeatmapSet.Buttons button.Action = () => { - if (State.Value != DownloadState.NotDownloaded) + if (downloadTracker.State.Value != DownloadState.NotDownloaded) { shakeContainer.Shake(); return; } - beatmaps.Download(BeatmapSet.Value, noVideo); + beatmaps.Download(beatmapSet, noVideo); }; localUser.BindTo(api.LocalUser); localUser.BindValueChanged(userChanged, true); button.Enabled.BindValueChanged(enabledChanged, true); - State.BindValueChanged(state => + downloadTracker.State.BindValueChanged(state => { switch (state.NewValue) { @@ -161,7 +168,7 @@ namespace osu.Game.Overlays.BeatmapSet.Buttons private LocalisableString getVideoSuffixText() { - if (!BeatmapSet.Value.OnlineInfo.HasVideo) + if (!beatmapSet.OnlineInfo.HasVideo) return string.Empty; return noVideo ? BeatmapsetsStrings.ShowDetailsDownloadNoVideo : BeatmapsetsStrings.ShowDetailsDownloadVideo; diff --git a/osu.Game/Screens/OnlinePlay/DrawableRoomPlaylistItem.cs b/osu.Game/Screens/OnlinePlay/DrawableRoomPlaylistItem.cs index 585b024623..16dd7b498b 100644 --- a/osu.Game/Screens/OnlinePlay/DrawableRoomPlaylistItem.cs +++ b/osu.Game/Screens/OnlinePlay/DrawableRoomPlaylistItem.cs @@ -302,7 +302,7 @@ namespace osu.Game.Screens.OnlinePlay { base.LoadComplete(); - State.BindValueChanged(stateChanged, true); + DownloadTracker.State.BindValueChanged(stateChanged, true); FinishTransforms(true); } diff --git a/osu.Game/Screens/Ranking/ReplayDownloadButton.cs b/osu.Game/Screens/Ranking/ReplayDownloadButton.cs index e644eb671a..f7325d2eb4 100644 --- a/osu.Game/Screens/Ranking/ReplayDownloadButton.cs +++ b/osu.Game/Screens/Ranking/ReplayDownloadButton.cs @@ -4,6 +4,7 @@ using osu.Framework.Allocation; using osu.Framework.Bindables; using osu.Framework.Graphics; +using osu.Framework.Graphics.Containers; using osu.Game.Graphics.Containers; using osu.Game.Graphics.UserInterface; using osu.Game.Online; @@ -12,13 +13,17 @@ using osuTK; namespace osu.Game.Screens.Ranking { - public class ReplayDownloadButton : DownloadTrackingComposite + public class ReplayDownloadButton : CompositeDrawable { - public Bindable Score => Model; + public readonly Bindable Score = new Bindable(); + + protected readonly Bindable State = new Bindable(); private DownloadButton button; private ShakeContainer shakeContainer; + private ScoreDownloadTracker downloadTracker; + private ReplayAvailability replayAvailability { get @@ -26,7 +31,7 @@ namespace osu.Game.Screens.Ranking if (State.Value == DownloadState.LocallyAvailable) return ReplayAvailability.Local; - if (!string.IsNullOrEmpty(Model.Value?.Hash)) + if (!string.IsNullOrEmpty(Score.Value?.Hash)) return ReplayAvailability.Online; return ReplayAvailability.NotAvailable; @@ -34,8 +39,8 @@ namespace osu.Game.Screens.Ranking } public ReplayDownloadButton(ScoreInfo score) - : base(score) { + Score.Value = score; Size = new Vector2(50, 30); } @@ -56,11 +61,11 @@ namespace osu.Game.Screens.Ranking switch (State.Value) { case DownloadState.LocallyAvailable: - game?.PresentScore(Model.Value, ScorePresentType.Gameplay); + game?.PresentScore(Score.Value, ScorePresentType.Gameplay); break; case DownloadState.NotDownloaded: - scores.Download(Model.Value, false); + scores.Download(Score.Value, false); break; case DownloadState.Importing: @@ -70,17 +75,25 @@ namespace osu.Game.Screens.Ranking } }; - State.BindValueChanged(state => + Score.BindValueChanged(score => { - button.State.Value = state.NewValue; + downloadTracker?.Expire(); + if (score.NewValue != null) + { + AddInternal(downloadTracker = new ScoreDownloadTracker(score.NewValue) + { + State = { BindTarget = State } + }); + } + + button.Enabled.Value = replayAvailability != ReplayAvailability.NotAvailable; updateTooltip(); }, true); - Model.BindValueChanged(_ => + State.BindValueChanged(state => { - button.Enabled.Value = replayAvailability != ReplayAvailability.NotAvailable; - + button.State.Value = state.NewValue; updateTooltip(); }, true); } From 746d6a4c166f274e456b36a7e7f40fcd25ae39f4 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 27 Oct 2021 21:26:26 +0900 Subject: [PATCH 07/15] Fix some oversights and test failures --- .../Online/TestSceneDirectDownloadButton.cs | 3 +-- osu.Game/Online/BeatmapDownloadTracker.cs | 8 +++----- .../OnlinePlayBeatmapAvailabilityTracker.cs | 12 +++++++----- .../Panels/BeatmapPanelDownloadButton.cs | 18 +++++++++--------- .../BeatmapSet/BeatmapSetHeaderContent.cs | 2 +- .../OnlinePlay/DrawableRoomPlaylistItem.cs | 7 +++---- .../Screens/OnlinePlay/Match/RoomSubScreen.cs | 2 -- .../Multiplayer/MultiplayerMatchSubScreen.cs | 4 ++-- .../Screens/Ranking/ReplayDownloadButton.cs | 2 +- 9 files changed, 27 insertions(+), 31 deletions(-) diff --git a/osu.Game.Tests/Visual/Online/TestSceneDirectDownloadButton.cs b/osu.Game.Tests/Visual/Online/TestSceneDirectDownloadButton.cs index 879e35057b..bb7fcc2fce 100644 --- a/osu.Game.Tests/Visual/Online/TestSceneDirectDownloadButton.cs +++ b/osu.Game.Tests/Visual/Online/TestSceneDirectDownloadButton.cs @@ -5,7 +5,6 @@ using System.Linq; using NUnit.Framework; using osu.Framework.Allocation; using osu.Framework.Graphics; -using osu.Framework.Testing; using osu.Game.Beatmaps; using osu.Game.Online; using osu.Game.Online.API.Requests.Responses; @@ -145,7 +144,7 @@ namespace osu.Game.Tests.Visual.Online { public new bool DownloadEnabled => base.DownloadEnabled; - public DownloadState DownloadState => this.ChildrenOfType().First().State.Value; + public DownloadState DownloadState => State.Value; public TestDownloadButton(BeatmapSetInfo beatmapSet) : base(beatmapSet) diff --git a/osu.Game/Online/BeatmapDownloadTracker.cs b/osu.Game/Online/BeatmapDownloadTracker.cs index 6df74efe9c..181968c02f 100644 --- a/osu.Game/Online/BeatmapDownloadTracker.cs +++ b/osu.Game/Online/BeatmapDownloadTracker.cs @@ -34,7 +34,7 @@ namespace osu.Game.Online // Used to interact with manager classes that don't support interface types. Will eventually be replaced. var beatmapSetInfo = new BeatmapSetInfo { OnlineBeatmapSetID = TrackedItem.OnlineID }; - if ((TrackedItem as BeatmapSetInfo)?.ID > 0 || Manager?.IsAvailableLocally(beatmapSetInfo) == true) + if (Manager?.IsAvailableLocally(beatmapSetInfo) == true) UpdateState(DownloadState.LocallyAvailable); else if (Manager != null) attachDownload(Manager.GetExistingDownload(beatmapSetInfo)); @@ -122,10 +122,8 @@ namespace osu.Game.Online { Schedule(() => { - if (!checkEquality(item, TrackedItem)) - return; - - UpdateState(DownloadState.LocallyAvailable); + if (checkEquality(item, TrackedItem)) + UpdateState(DownloadState.LocallyAvailable); }); } } diff --git a/osu.Game/Online/Rooms/OnlinePlayBeatmapAvailabilityTracker.cs b/osu.Game/Online/Rooms/OnlinePlayBeatmapAvailabilityTracker.cs index 6dd1204555..5e09f42aa8 100644 --- a/osu.Game/Online/Rooms/OnlinePlayBeatmapAvailabilityTracker.cs +++ b/osu.Game/Online/Rooms/OnlinePlayBeatmapAvailabilityTracker.cs @@ -4,6 +4,7 @@ using System; using osu.Framework.Allocation; using osu.Framework.Bindables; +using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Framework.Logging; using osu.Framework.Threading; @@ -17,10 +18,12 @@ namespace osu.Game.Online.Rooms /// This differs from a regular download tracking composite as this accounts for the /// databased beatmap set's checksum, to disallow from playing with an altered version of the beatmap. /// - public class OnlinePlayBeatmapAvailabilityTracker : CompositeDrawable + public sealed class OnlinePlayBeatmapAvailabilityTracker : CompositeDrawable { public readonly IBindable SelectedItem = new Bindable(); + protected override bool RequiresChildrenUpdate => true; + [Resolved] private BeatmapManager beatmapManager { get; set; } @@ -46,9 +49,10 @@ namespace osu.Game.Online.Rooms if (item.NewValue == null) return; - downloadTracker?.Expire(); - downloadTracker = new BeatmapDownloadTracker(item.NewValue.Beatmap.Value.BeatmapSet); + downloadTracker?.RemoveAndDisposeImmediately(); + downloadTracker = new BeatmapDownloadTracker(item.NewValue.Beatmap.Value.BeatmapSet); + downloadTracker.State.BindValueChanged(_ => updateAvailability()); downloadTracker.Progress.BindValueChanged(_ => { if (downloadTracker.State.Value != DownloadState.Downloading) @@ -60,8 +64,6 @@ namespace osu.Game.Online.Rooms progressUpdate = Scheduler.AddDelayed(updateAvailability, progressUpdate == null ? 0 : 500); }); - downloadTracker.State.BindValueChanged(_ => updateAvailability(), true); - AddInternal(downloadTracker); }, true); } diff --git a/osu.Game/Overlays/BeatmapListing/Panels/BeatmapPanelDownloadButton.cs b/osu.Game/Overlays/BeatmapListing/Panels/BeatmapPanelDownloadButton.cs index 5c6472cd1b..dd12e8e467 100644 --- a/osu.Game/Overlays/BeatmapListing/Panels/BeatmapPanelDownloadButton.cs +++ b/osu.Game/Overlays/BeatmapListing/Panels/BeatmapPanelDownloadButton.cs @@ -36,6 +36,7 @@ namespace osu.Game.Overlays.BeatmapListing.Panels public BeatmapPanelDownloadButton(BeatmapSetInfo beatmapSet) { this.beatmapSet = beatmapSet; + InternalChildren = new Drawable[] { shakeContainer = new ShakeContainer @@ -44,6 +45,7 @@ namespace osu.Game.Overlays.BeatmapListing.Panels Child = button = new DownloadButton { RelativeSizeAxes = Axes.Both, + State = { BindTarget = State } }, }, DownloadTracker = new BeatmapDownloadTracker(beatmapSet) @@ -60,14 +62,6 @@ namespace osu.Game.Overlays.BeatmapListing.Panels }); } - protected override void LoadComplete() - { - base.LoadComplete(); - - ((IBindable)button.State).BindTo(DownloadTracker.State); - FinishTransforms(true); - } - [BackgroundDependencyLoader(true)] private void load(OsuGame game, BeatmapManager beatmaps, OsuConfigManager osuConfig) { @@ -96,7 +90,7 @@ namespace osu.Game.Overlays.BeatmapListing.Panels } }; - DownloadTracker.State.BindValueChanged(state => + State.BindValueChanged(state => { switch (state.NewValue) { @@ -116,5 +110,11 @@ namespace osu.Game.Overlays.BeatmapListing.Panels } }, true); } + + protected override void LoadComplete() + { + base.LoadComplete(); + FinishTransforms(true); + } } } diff --git a/osu.Game/Overlays/BeatmapSet/BeatmapSetHeaderContent.cs b/osu.Game/Overlays/BeatmapSet/BeatmapSetHeaderContent.cs index e0a09ed520..624d07fb0f 100644 --- a/osu.Game/Overlays/BeatmapSet/BeatmapSetHeaderContent.cs +++ b/osu.Game/Overlays/BeatmapSet/BeatmapSetHeaderContent.cs @@ -233,7 +233,7 @@ namespace osu.Game.Overlays.BeatmapSet Picker.BeatmapSet = rulesetSelector.BeatmapSet = author.BeatmapSet = beatmapAvailability.BeatmapSet = Details.BeatmapSet = setInfo.NewValue; cover.BeatmapSet = setInfo.NewValue; - downloadTracker?.Expire(); + downloadTracker?.RemoveAndDisposeImmediately(); if (setInfo.NewValue == null) { diff --git a/osu.Game/Screens/OnlinePlay/DrawableRoomPlaylistItem.cs b/osu.Game/Screens/OnlinePlay/DrawableRoomPlaylistItem.cs index 16dd7b498b..5639265617 100644 --- a/osu.Game/Screens/OnlinePlay/DrawableRoomPlaylistItem.cs +++ b/osu.Game/Screens/OnlinePlay/DrawableRoomPlaylistItem.cs @@ -289,7 +289,8 @@ namespace osu.Game.Screens.OnlinePlay [Resolved] private BeatmapManager beatmapManager { get; set; } - public override bool IsPresent => base.IsPresent || Scheduler.HasPendingTasks; + // required for download tracking, as this button hides itself. can probably be removed with a bit of consideration. + public override bool IsPresent => true; public PlaylistDownloadButton(PlaylistItem playlistItem) : base(playlistItem.Beatmap.Value.BeatmapSet) @@ -300,10 +301,8 @@ namespace osu.Game.Screens.OnlinePlay protected override void LoadComplete() { + State.BindValueChanged(stateChanged, true); base.LoadComplete(); - - DownloadTracker.State.BindValueChanged(stateChanged, true); - FinishTransforms(true); } private void stateChanged(ValueChangedEvent state) diff --git a/osu.Game/Screens/OnlinePlay/Match/RoomSubScreen.cs b/osu.Game/Screens/OnlinePlay/Match/RoomSubScreen.cs index bcb793062b..270d12ec97 100644 --- a/osu.Game/Screens/OnlinePlay/Match/RoomSubScreen.cs +++ b/osu.Game/Screens/OnlinePlay/Match/RoomSubScreen.cs @@ -67,8 +67,6 @@ namespace osu.Game.Screens.OnlinePlay.Match [Cached] protected OnlinePlayBeatmapAvailabilityTracker BeatmapAvailabilityTracker { get; private set; } - protected IBindable BeatmapAvailability => BeatmapAvailabilityTracker.Availability; - public readonly Room Room; private readonly bool allowEdit; diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerMatchSubScreen.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerMatchSubScreen.cs index 544eac4127..0af41c977b 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerMatchSubScreen.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerMatchSubScreen.cs @@ -66,7 +66,7 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer SelectedItem.BindTo(client.CurrentMatchPlayingItem); - BeatmapAvailability.BindValueChanged(updateBeatmapAvailability, true); + BeatmapAvailabilityTracker.Availability.BindValueChanged(updateBeatmapAvailability, true); UserMods.BindValueChanged(onUserModsChanged); client.LoadRequested += onLoadRequested; @@ -362,7 +362,7 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer private void onLoadRequested() { - if (BeatmapAvailability.Value.State != DownloadState.LocallyAvailable) + if (BeatmapAvailabilityTracker.Availability.Value.State != DownloadState.LocallyAvailable) return; // In the case of spectating, IMultiplayerClient.LoadRequested can be fired while the game is still spectating a previous session. diff --git a/osu.Game/Screens/Ranking/ReplayDownloadButton.cs b/osu.Game/Screens/Ranking/ReplayDownloadButton.cs index f7325d2eb4..66b3c973f5 100644 --- a/osu.Game/Screens/Ranking/ReplayDownloadButton.cs +++ b/osu.Game/Screens/Ranking/ReplayDownloadButton.cs @@ -77,7 +77,7 @@ namespace osu.Game.Screens.Ranking Score.BindValueChanged(score => { - downloadTracker?.Expire(); + downloadTracker?.RemoveAndDisposeImmediately(); if (score.NewValue != null) { From 347f41f367c67e2e7c2d79f76e1bdcc66ea907f3 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 27 Oct 2021 23:54:02 +0900 Subject: [PATCH 08/15] Change test to allow for potentially single-frame delayed events firing --- .../Online/TestSceneOnlinePlayBeatmapAvailabilityTracker.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Tests/Online/TestSceneOnlinePlayBeatmapAvailabilityTracker.cs b/osu.Game.Tests/Online/TestSceneOnlinePlayBeatmapAvailabilityTracker.cs index 558b874234..5e7ce3abf5 100644 --- a/osu.Game.Tests/Online/TestSceneOnlinePlayBeatmapAvailabilityTracker.cs +++ b/osu.Game.Tests/Online/TestSceneOnlinePlayBeatmapAvailabilityTracker.cs @@ -128,7 +128,7 @@ namespace osu.Game.Tests.Online private void addAvailabilityCheckStep(string description, Func expected) { - AddAssert(description, () => availabilityTracker.Availability.Value.Equals(expected.Invoke())); + AddUntilStep(description, () => availabilityTracker.Availability.Value.Equals(expected.Invoke())); } private static BeatmapInfo getTestBeatmapInfo(string archiveFile) From 290f583ae0d19aa35549d00e101598d96de7500a Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 28 Oct 2021 00:59:48 +0900 Subject: [PATCH 09/15] Fix incorrect initial state --- osu.Game/Online/Rooms/OnlinePlayBeatmapAvailabilityTracker.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Online/Rooms/OnlinePlayBeatmapAvailabilityTracker.cs b/osu.Game/Online/Rooms/OnlinePlayBeatmapAvailabilityTracker.cs index 5e09f42aa8..87348fbd1c 100644 --- a/osu.Game/Online/Rooms/OnlinePlayBeatmapAvailabilityTracker.cs +++ b/osu.Game/Online/Rooms/OnlinePlayBeatmapAvailabilityTracker.cs @@ -32,7 +32,7 @@ namespace osu.Game.Online.Rooms /// public IBindable Availability => availability; - private readonly Bindable availability = new Bindable(BeatmapAvailability.LocallyAvailable()); + private readonly Bindable availability = new Bindable(BeatmapAvailability.NotDownloaded()); private ScheduledDelegate progressUpdate; From 748003b016ce84bf4d0f13f98a70ed494cbb8915 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 29 Oct 2021 11:54:19 +0900 Subject: [PATCH 10/15] Simplify `Manager` null check to a single pre-check --- osu.Game/Online/BeatmapDownloadTracker.cs | 26 +++++++++++------------ 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/osu.Game/Online/BeatmapDownloadTracker.cs b/osu.Game/Online/BeatmapDownloadTracker.cs index 181968c02f..4a7d0b660a 100644 --- a/osu.Game/Online/BeatmapDownloadTracker.cs +++ b/osu.Game/Online/BeatmapDownloadTracker.cs @@ -31,25 +31,25 @@ namespace osu.Game.Online [BackgroundDependencyLoader(true)] private void load() { + if (Manager == null) + return; + // Used to interact with manager classes that don't support interface types. Will eventually be replaced. var beatmapSetInfo = new BeatmapSetInfo { OnlineBeatmapSetID = TrackedItem.OnlineID }; - if (Manager?.IsAvailableLocally(beatmapSetInfo) == true) + if (Manager.IsAvailableLocally(beatmapSetInfo)) UpdateState(DownloadState.LocallyAvailable); - else if (Manager != null) + else attachDownload(Manager.GetExistingDownload(beatmapSetInfo)); - if (Manager != null) - { - managerDownloadBegan = Manager.DownloadBegan.GetBoundCopy(); - managerDownloadBegan.BindValueChanged(downloadBegan); - managerDownloadFailed = Manager.DownloadFailed.GetBoundCopy(); - managerDownloadFailed.BindValueChanged(downloadFailed); - managerUpdated = Manager.ItemUpdated.GetBoundCopy(); - managerUpdated.BindValueChanged(itemUpdated); - managerRemoved = Manager.ItemRemoved.GetBoundCopy(); - managerRemoved.BindValueChanged(itemRemoved); - } + managerDownloadBegan = Manager.DownloadBegan.GetBoundCopy(); + managerDownloadBegan.BindValueChanged(downloadBegan); + managerDownloadFailed = Manager.DownloadFailed.GetBoundCopy(); + managerDownloadFailed.BindValueChanged(downloadFailed); + managerUpdated = Manager.ItemUpdated.GetBoundCopy(); + managerUpdated.BindValueChanged(itemUpdated); + managerRemoved = Manager.ItemRemoved.GetBoundCopy(); + managerRemoved.BindValueChanged(itemRemoved); } private void downloadBegan(ValueChangedEvent>> weakRequest) From 75a088d4f4f8082986a418c901e7def8b7eae859 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 29 Oct 2021 11:54:42 +0900 Subject: [PATCH 11/15] Fix variable name mismatch --- osu.Game/Online/ScoreDownloadTracker.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/osu.Game/Online/ScoreDownloadTracker.cs b/osu.Game/Online/ScoreDownloadTracker.cs index 7a714a7a0e..ac700fc587 100644 --- a/osu.Game/Online/ScoreDownloadTracker.cs +++ b/osu.Game/Online/ScoreDownloadTracker.cs @@ -32,12 +32,12 @@ namespace osu.Game.Online private void load() { // Used to interact with manager classes that don't support interface types. Will eventually be replaced. - var beatmapSetInfo = new ScoreInfo { OnlineScoreID = TrackedItem.OnlineScoreID }; + var scoreInfo = new ScoreInfo { OnlineScoreID = TrackedItem.OnlineScoreID }; - if (TrackedItem.ID > 0 || Manager?.IsAvailableLocally(beatmapSetInfo) == true) + if (TrackedItem.ID > 0 || Manager?.IsAvailableLocally(scoreInfo) == true) UpdateState(DownloadState.LocallyAvailable); else if (Manager != null) - attachDownload(Manager.GetExistingDownload(beatmapSetInfo)); + attachDownload(Manager.GetExistingDownload(scoreInfo)); if (Manager != null) { From dc44734db20e25c4a39b52faa4df0ff62740ee21 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 29 Oct 2021 11:56:17 +0900 Subject: [PATCH 12/15] Simplify null check in `ScoreDownloadTracker` and remove unnecessary ID check --- osu.Game/Online/ScoreDownloadTracker.cs | 26 ++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/osu.Game/Online/ScoreDownloadTracker.cs b/osu.Game/Online/ScoreDownloadTracker.cs index ac700fc587..8222a5382c 100644 --- a/osu.Game/Online/ScoreDownloadTracker.cs +++ b/osu.Game/Online/ScoreDownloadTracker.cs @@ -31,25 +31,25 @@ namespace osu.Game.Online [BackgroundDependencyLoader(true)] private void load() { + if (Manager == null) + return; + // Used to interact with manager classes that don't support interface types. Will eventually be replaced. var scoreInfo = new ScoreInfo { OnlineScoreID = TrackedItem.OnlineScoreID }; - if (TrackedItem.ID > 0 || Manager?.IsAvailableLocally(scoreInfo) == true) + if (Manager.IsAvailableLocally(scoreInfo)) UpdateState(DownloadState.LocallyAvailable); - else if (Manager != null) + else attachDownload(Manager.GetExistingDownload(scoreInfo)); - if (Manager != null) - { - managerDownloadBegan = Manager.DownloadBegan.GetBoundCopy(); - managerDownloadBegan.BindValueChanged(downloadBegan); - managerDownloadFailed = Manager.DownloadFailed.GetBoundCopy(); - managerDownloadFailed.BindValueChanged(downloadFailed); - managerUpdated = Manager.ItemUpdated.GetBoundCopy(); - managerUpdated.BindValueChanged(itemUpdated); - managerRemoved = Manager.ItemRemoved.GetBoundCopy(); - managerRemoved.BindValueChanged(itemRemoved); - } + managerDownloadBegan = Manager.DownloadBegan.GetBoundCopy(); + managerDownloadBegan.BindValueChanged(downloadBegan); + managerDownloadFailed = Manager.DownloadFailed.GetBoundCopy(); + managerDownloadFailed.BindValueChanged(downloadFailed); + managerUpdated = Manager.ItemUpdated.GetBoundCopy(); + managerUpdated.BindValueChanged(itemUpdated); + managerRemoved = Manager.ItemRemoved.GetBoundCopy(); + managerRemoved.BindValueChanged(itemRemoved); } private void downloadBegan(ValueChangedEvent>> weakRequest) From f3811edb0b83377ceee067571f4d553bb8bb5b1a Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 29 Oct 2021 11:57:54 +0900 Subject: [PATCH 13/15] Add inline comment explaining usage of `RequiresChildrenUpdate` override --- osu.Game/Online/Rooms/OnlinePlayBeatmapAvailabilityTracker.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/osu.Game/Online/Rooms/OnlinePlayBeatmapAvailabilityTracker.cs b/osu.Game/Online/Rooms/OnlinePlayBeatmapAvailabilityTracker.cs index 87348fbd1c..88793c7cfc 100644 --- a/osu.Game/Online/Rooms/OnlinePlayBeatmapAvailabilityTracker.cs +++ b/osu.Game/Online/Rooms/OnlinePlayBeatmapAvailabilityTracker.cs @@ -22,6 +22,7 @@ namespace osu.Game.Online.Rooms { public readonly IBindable SelectedItem = new Bindable(); + // Required to allow child components to update. Can potentially be replaced with a `CompositeComponent` class if or when we make one. protected override bool RequiresChildrenUpdate => true; [Resolved] From b252b176d45770bbf9e8e1fbd089fa37408b5a51 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 29 Oct 2021 11:59:07 +0900 Subject: [PATCH 14/15] Seal implementation and add comment about `LoadComplete` execution order --- osu.Game/Screens/OnlinePlay/DrawableRoomPlaylistItem.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/osu.Game/Screens/OnlinePlay/DrawableRoomPlaylistItem.cs b/osu.Game/Screens/OnlinePlay/DrawableRoomPlaylistItem.cs index 1a59f36c11..c946bf12fd 100644 --- a/osu.Game/Screens/OnlinePlay/DrawableRoomPlaylistItem.cs +++ b/osu.Game/Screens/OnlinePlay/DrawableRoomPlaylistItem.cs @@ -282,7 +282,7 @@ namespace osu.Game.Screens.OnlinePlay return true; } - private class PlaylistDownloadButton : BeatmapPanelDownloadButton + private sealed class PlaylistDownloadButton : BeatmapPanelDownloadButton { private readonly PlaylistItem playlistItem; @@ -302,6 +302,8 @@ namespace osu.Game.Screens.OnlinePlay protected override void LoadComplete() { State.BindValueChanged(stateChanged, true); + + // base implementation calls FinishTransforms, so should be run after the above state update. base.LoadComplete(); } From 414d920ca25caf0f06e619a0c810414212d8b5a9 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 29 Oct 2021 12:02:10 +0900 Subject: [PATCH 15/15] Revert to previous exposure of `RoomSubScreen.BeatmapAvailability` --- osu.Game/Screens/OnlinePlay/Match/RoomSubScreen.cs | 8 +++++--- .../OnlinePlay/Multiplayer/MultiplayerMatchSubScreen.cs | 4 ++-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/osu.Game/Screens/OnlinePlay/Match/RoomSubScreen.cs b/osu.Game/Screens/OnlinePlay/Match/RoomSubScreen.cs index 270d12ec97..2015a050bb 100644 --- a/osu.Game/Screens/OnlinePlay/Match/RoomSubScreen.cs +++ b/osu.Game/Screens/OnlinePlay/Match/RoomSubScreen.cs @@ -65,7 +65,9 @@ namespace osu.Game.Screens.OnlinePlay.Match private IBindable> managerUpdated; [Cached] - protected OnlinePlayBeatmapAvailabilityTracker BeatmapAvailabilityTracker { get; private set; } + private OnlinePlayBeatmapAvailabilityTracker beatmapAvailabilityTracker { get; set; } + + protected IBindable BeatmapAvailability => beatmapAvailabilityTracker.Availability; public readonly Room Room; private readonly bool allowEdit; @@ -86,7 +88,7 @@ namespace osu.Game.Screens.OnlinePlay.Match Padding = new MarginPadding { Top = Header.HEIGHT }; - BeatmapAvailabilityTracker = new OnlinePlayBeatmapAvailabilityTracker + beatmapAvailabilityTracker = new OnlinePlayBeatmapAvailabilityTracker { SelectedItem = { BindTarget = SelectedItem } }; @@ -101,7 +103,7 @@ namespace osu.Game.Screens.OnlinePlay.Match InternalChildren = new Drawable[] { - BeatmapAvailabilityTracker, + beatmapAvailabilityTracker, new GridContainer { RelativeSizeAxes = Axes.Both, diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerMatchSubScreen.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerMatchSubScreen.cs index 0af41c977b..544eac4127 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerMatchSubScreen.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerMatchSubScreen.cs @@ -66,7 +66,7 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer SelectedItem.BindTo(client.CurrentMatchPlayingItem); - BeatmapAvailabilityTracker.Availability.BindValueChanged(updateBeatmapAvailability, true); + BeatmapAvailability.BindValueChanged(updateBeatmapAvailability, true); UserMods.BindValueChanged(onUserModsChanged); client.LoadRequested += onLoadRequested; @@ -362,7 +362,7 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer private void onLoadRequested() { - if (BeatmapAvailabilityTracker.Availability.Value.State != DownloadState.LocallyAvailable) + if (BeatmapAvailability.Value.State != DownloadState.LocallyAvailable) return; // In the case of spectating, IMultiplayerClient.LoadRequested can be fired while the game is still spectating a previous session.