From 54f72d68cab6ba44f9f8b5fe4d82b8b0cede0225 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 5 Nov 2021 18:05:31 +0900 Subject: [PATCH 1/6] Revert weird event flow in model manager/importers --- .../Beatmaps/IO/ImportBeatmapTest.cs | 4 +- .../Online/TestSceneDirectDownloadButton.cs | 4 +- osu.Game/Beatmaps/BeatmapManager.cs | 37 +++++++-- osu.Game/Beatmaps/BeatmapModelManager.cs | 13 +-- osu.Game/Database/ArchiveModelManager.cs | 13 +-- osu.Game/Database/IModelDownloader.cs | 5 +- osu.Game/Database/IModelManager.cs | 11 +-- osu.Game/Database/ModelDownloader.cs | 15 ++-- osu.Game/Online/BeatmapDownloadTracker.cs | 82 +++++++------------ osu.Game/Online/ScoreDownloadTracker.cs | 82 +++++++------------ osu.Game/OsuGameBase.cs | 27 ++---- osu.Game/Overlays/MusicController.cs | 45 ++++------ .../Overlays/Settings/Sections/SkinSection.cs | 33 ++++---- osu.Game/Scoring/ScoreManager.cs | 24 +++++- .../Screens/OnlinePlay/Match/RoomSubScreen.cs | 15 ++-- osu.Game/Screens/Select/BeatmapCarousel.cs | 57 +++++-------- .../Screens/Select/Carousel/TopLocalRank.cs | 31 +++---- .../Select/Leaderboards/BeatmapLeaderboard.cs | 37 ++++----- osu.Game/Screens/Spectate/SpectatorScreen.cs | 16 ++-- 19 files changed, 236 insertions(+), 315 deletions(-) diff --git a/osu.Game.Tests/Beatmaps/IO/ImportBeatmapTest.cs b/osu.Game.Tests/Beatmaps/IO/ImportBeatmapTest.cs index d68d43c998..f815598122 100644 --- a/osu.Game.Tests/Beatmaps/IO/ImportBeatmapTest.cs +++ b/osu.Game.Tests/Beatmaps/IO/ImportBeatmapTest.cs @@ -408,8 +408,8 @@ namespace osu.Game.Tests.Beatmaps.IO var manager = osu.Dependencies.Get(); // ReSharper disable once AccessToModifiedClosure - manager.ItemUpdated.BindValueChanged(_ => Interlocked.Increment(ref itemAddRemoveFireCount)); - manager.ItemRemoved.BindValueChanged(_ => Interlocked.Increment(ref itemAddRemoveFireCount)); + manager.ItemUpdated += _ => Interlocked.Increment(ref itemAddRemoveFireCount); + manager.ItemRemoved += _ => Interlocked.Increment(ref itemAddRemoveFireCount); var imported = await LoadOszIntoOsu(osu); diff --git a/osu.Game.Tests/Visual/Online/TestSceneDirectDownloadButton.cs b/osu.Game.Tests/Visual/Online/TestSceneDirectDownloadButton.cs index 3d828077c8..a865bbe950 100644 --- a/osu.Game.Tests/Visual/Online/TestSceneDirectDownloadButton.cs +++ b/osu.Game.Tests/Visual/Online/TestSceneDirectDownloadButton.cs @@ -45,10 +45,10 @@ namespace osu.Game.Tests.Visual.Online AddStep("import soleily", () => beatmaps.Import(TestResources.GetQuickTestBeatmapForImport())); AddUntilStep("wait for beatmap import", () => beatmaps.GetAllUsableBeatmapSets().Any(b => b.OnlineBeatmapSetID == 241526)); - AddAssert("button state downloaded", () => downloadButton.DownloadState == DownloadState.LocallyAvailable); + AddUntilStep("button state downloaded", () => downloadButton.DownloadState == DownloadState.LocallyAvailable); createButtonWithBeatmap(createSoleily()); - AddAssert("button state downloaded", () => downloadButton.DownloadState == DownloadState.LocallyAvailable); + AddUntilStep("button state downloaded", () => downloadButton.DownloadState == DownloadState.LocallyAvailable); ensureSoleilyRemoved(); AddAssert("button state not downloaded", () => downloadButton.DownloadState == DownloadState.NotDownloaded); } diff --git a/osu.Game/Beatmaps/BeatmapManager.cs b/osu.Game/Beatmaps/BeatmapManager.cs index 7a9f3e0a45..98c0ba648e 100644 --- a/osu.Game/Beatmaps/BeatmapManager.cs +++ b/osu.Game/Beatmaps/BeatmapManager.cs @@ -10,7 +10,6 @@ using System.Threading; using System.Threading.Tasks; using JetBrains.Annotations; using osu.Framework.Audio; -using osu.Framework.Bindables; using osu.Framework.IO.Stores; using osu.Framework.Platform; using osu.Framework.Testing; @@ -101,12 +100,20 @@ namespace osu.Game.Beatmaps /// /// Fired when a single difficulty has been hidden. /// - public IBindable> BeatmapHidden => beatmapModelManager.BeatmapHidden; + public Action BeatmapHidden + { + get => beatmapModelManager.BeatmapHidden; + set => beatmapModelManager.BeatmapHidden = value; + } /// /// Fired when a single difficulty has been restored. /// - public IBindable> BeatmapRestored => beatmapModelManager.BeatmapRestored; + public Action BeatmapRestored + { + get => beatmapModelManager.BeatmapRestored; + set => beatmapModelManager.BeatmapRestored = value; + } /// /// Saves an file against a given . @@ -198,9 +205,17 @@ namespace osu.Game.Beatmaps return beatmapModelManager.IsAvailableLocally(model); } - public IBindable> ItemUpdated => beatmapModelManager.ItemUpdated; + public Action ItemUpdated + { + get => beatmapModelManager.ItemUpdated; + set => beatmapModelManager.ItemUpdated = value; + } - public IBindable> ItemRemoved => beatmapModelManager.ItemRemoved; + public Action ItemRemoved + { + get => beatmapModelManager.ItemRemoved; + set => beatmapModelManager.ItemRemoved = value; + } public Task ImportFromStableAsync(StableStorage stableStorage) { @@ -246,9 +261,17 @@ namespace osu.Game.Beatmaps #region Implementation of IModelDownloader - public IBindable>> DownloadBegan => beatmapModelDownloader.DownloadBegan; + public Action> DownloadBegan + { + get => beatmapModelDownloader.DownloadBegan; + set => beatmapModelDownloader.DownloadBegan = value; + } - public IBindable>> DownloadFailed => beatmapModelDownloader.DownloadFailed; + public Action> DownloadFailed + { + get => beatmapModelDownloader.DownloadFailed; + set => beatmapModelDownloader.DownloadFailed = value; + } // Temporary method until this class supports IBeatmapSetInfo or otherwise. public bool Download(IBeatmapSetInfo model, bool minimiseDownloadSize = false) diff --git a/osu.Game/Beatmaps/BeatmapModelManager.cs b/osu.Game/Beatmaps/BeatmapModelManager.cs index f148d05aca..1a78a625d9 100644 --- a/osu.Game/Beatmaps/BeatmapModelManager.cs +++ b/osu.Game/Beatmaps/BeatmapModelManager.cs @@ -11,7 +11,6 @@ using System.Threading; using System.Threading.Tasks; using Microsoft.EntityFrameworkCore; using osu.Framework.Audio.Track; -using osu.Framework.Bindables; using osu.Framework.Extensions; using osu.Framework.Graphics.Textures; using osu.Framework.Logging; @@ -37,14 +36,12 @@ namespace osu.Game.Beatmaps /// /// Fired when a single difficulty has been hidden. /// - public IBindable> BeatmapHidden => beatmapHidden; - - private readonly Bindable> beatmapHidden = new Bindable>(); + public Action BeatmapHidden; /// /// Fired when a single difficulty has been restored. /// - public IBindable> BeatmapRestored => beatmapRestored; + public Action BeatmapRestored; /// /// An online lookup queue component which handles populating online beatmap metadata. @@ -56,8 +53,6 @@ namespace osu.Game.Beatmaps /// public IWorkingBeatmapCache WorkingBeatmapCache { private get; set; } - private readonly Bindable> beatmapRestored = new Bindable>(); - public override IEnumerable HandledExtensions => new[] { ".osz" }; protected override string[] HashableFileTypes => new[] { ".osu" }; @@ -75,8 +70,8 @@ namespace osu.Game.Beatmaps this.rulesets = rulesets; beatmaps = (BeatmapStore)ModelStore; - beatmaps.BeatmapHidden += b => beatmapHidden.Value = new WeakReference(b); - beatmaps.BeatmapRestored += b => beatmapRestored.Value = new WeakReference(b); + beatmaps.BeatmapHidden += b => BeatmapHidden?.Invoke(b); + beatmaps.BeatmapRestored += b => BeatmapRestored?.Invoke(b); beatmaps.ItemRemoved += b => WorkingBeatmapCache?.Invalidate(b); beatmaps.ItemUpdated += obj => WorkingBeatmapCache?.Invalidate(obj); } diff --git a/osu.Game/Database/ArchiveModelManager.cs b/osu.Game/Database/ArchiveModelManager.cs index 4e40e52051..bc4d0cdabd 100644 --- a/osu.Game/Database/ArchiveModelManager.cs +++ b/osu.Game/Database/ArchiveModelManager.cs @@ -10,7 +10,6 @@ using System.Threading.Tasks; using Humanizer; using JetBrains.Annotations; using Microsoft.EntityFrameworkCore; -using osu.Framework.Bindables; using osu.Framework.Extensions; using osu.Framework.Extensions.IEnumerableExtensions; using osu.Framework.Logging; @@ -63,17 +62,13 @@ namespace osu.Game.Database /// Fired when a new or updated becomes available in the database. /// This is not guaranteed to run on the update thread. /// - public IBindable> ItemUpdated => itemUpdated; - - private readonly Bindable> itemUpdated = new Bindable>(); + public Action ItemUpdated { get; set; } /// /// Fired when a is removed from the database. /// This is not guaranteed to run on the update thread. /// - public IBindable> ItemRemoved => itemRemoved; - - private readonly Bindable> itemRemoved = new Bindable>(); + public Action ItemRemoved { get; set; } public virtual IEnumerable HandledExtensions => new[] { @".zip" }; @@ -93,8 +88,8 @@ namespace osu.Game.Database ContextFactory = contextFactory; ModelStore = modelStore; - ModelStore.ItemUpdated += item => handleEvent(() => itemUpdated.Value = new WeakReference(item)); - ModelStore.ItemRemoved += item => handleEvent(() => itemRemoved.Value = new WeakReference(item)); + ModelStore.ItemUpdated += item => handleEvent(() => ItemUpdated?.Invoke(item)); + ModelStore.ItemRemoved += item => handleEvent(() => ItemRemoved?.Invoke(item)); exportStorage = storage.GetStorageForDirectory(@"exports"); diff --git a/osu.Game/Database/IModelDownloader.cs b/osu.Game/Database/IModelDownloader.cs index a5573b2190..09bf838eeb 100644 --- a/osu.Game/Database/IModelDownloader.cs +++ b/osu.Game/Database/IModelDownloader.cs @@ -3,7 +3,6 @@ using System; using osu.Game.Online.API; -using osu.Framework.Bindables; namespace osu.Game.Database { @@ -18,13 +17,13 @@ namespace osu.Game.Database /// Fired when a download begins. /// This is NOT run on the update thread and should be scheduled. /// - IBindable>> DownloadBegan { get; } + Action> DownloadBegan { get; set; } /// /// Fired when a download is interrupted, either due to user cancellation or failure. /// This is NOT run on the update thread and should be scheduled. /// - IBindable>> DownloadFailed { get; } + Action> DownloadFailed { get; set; } /// /// Begin a download for the requested . diff --git a/osu.Game/Database/IModelManager.cs b/osu.Game/Database/IModelManager.cs index f5e401cdfb..d8e5775f4d 100644 --- a/osu.Game/Database/IModelManager.cs +++ b/osu.Game/Database/IModelManager.cs @@ -5,7 +5,6 @@ using System; using System.Collections.Generic; using System.IO; using System.Threading.Tasks; -using osu.Framework.Bindables; using osu.Game.IO; namespace osu.Game.Database @@ -18,16 +17,14 @@ namespace osu.Game.Database where TModel : class { /// - /// A bindable which contains a weak reference to the last item that was updated. - /// This is not thread-safe and should be scheduled locally if consumed from a drawable component. + /// Fired when an item is updated. /// - IBindable> ItemUpdated { get; } + Action ItemUpdated { get; set; } /// - /// A bindable which contains a weak reference to the last item that was removed. - /// This is not thread-safe and should be scheduled locally if consumed from a drawable component. + /// Fired when an item is removed. /// - IBindable> ItemRemoved { get; } + Action ItemRemoved { get; set; } /// /// This is a temporary method and will likely be replaced by a full-fledged (and more correctly placed) migration process in the future. diff --git a/osu.Game/Database/ModelDownloader.cs b/osu.Game/Database/ModelDownloader.cs index 12bf5e9ce7..39ca4780eb 100644 --- a/osu.Game/Database/ModelDownloader.cs +++ b/osu.Game/Database/ModelDownloader.cs @@ -6,7 +6,6 @@ using System.Collections.Generic; using System.Linq; using System.Threading.Tasks; using Humanizer; -using osu.Framework.Bindables; using osu.Framework.Logging; using osu.Framework.Platform; using osu.Game.Online.API; @@ -19,13 +18,9 @@ namespace osu.Game.Database { public Action PostNotification { protected get; set; } - public IBindable>> DownloadBegan => downloadBegan; + public Action> DownloadBegan { get; set; } - private readonly Bindable>> downloadBegan = new Bindable>>(); - - public IBindable>> DownloadFailed => downloadFailed; - - private readonly Bindable>> downloadFailed = new Bindable>>(); + public Action> DownloadFailed { get; set; } private readonly IModelManager modelManager; private readonly IAPIProvider api; @@ -72,7 +67,7 @@ namespace osu.Game.Database // for now a failed import will be marked as a failed download for simplicity. if (!imported.Any()) - downloadFailed.Value = new WeakReference>(request); + DownloadFailed?.Invoke(request); CurrentDownloads.Remove(request); }, TaskCreationOptions.LongRunning); @@ -91,14 +86,14 @@ namespace osu.Game.Database api.PerformAsync(request); - downloadBegan.Value = new WeakReference>(request); + DownloadBegan?.Invoke(request); return true; void triggerFailure(Exception error) { CurrentDownloads.Remove(request); - downloadFailed.Value = new WeakReference>(request); + DownloadFailed?.Invoke(request); notification.State = ProgressNotificationState.Cancelled; diff --git a/osu.Game/Online/BeatmapDownloadTracker.cs b/osu.Game/Online/BeatmapDownloadTracker.cs index 4a7d0b660a..6637c0705e 100644 --- a/osu.Game/Online/BeatmapDownloadTracker.cs +++ b/osu.Game/Online/BeatmapDownloadTracker.cs @@ -3,7 +3,6 @@ using System; using osu.Framework.Allocation; -using osu.Framework.Bindables; using osu.Game.Beatmaps; using osu.Game.Online.API; @@ -23,11 +22,6 @@ namespace osu.Game.Online { } - private IBindable>? managerUpdated; - private IBindable>? managerRemoved; - private IBindable>>? managerDownloadBegan; - private IBindable>>? managerDownloadFailed; - [BackgroundDependencyLoader(true)] private void load() { @@ -42,39 +36,23 @@ namespace osu.Game.Online else attachDownload(Manager.GetExistingDownload(beatmapSetInfo)); - 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); + Manager.DownloadBegan += downloadBegan; + Manager.DownloadFailed += downloadFailed; + Manager.ItemUpdated += itemUpdated; + Manager.ItemRemoved += itemRemoved; } - private void downloadBegan(ValueChangedEvent>> weakRequest) + private void downloadBegan(ArchiveDownloadRequest request) => Schedule(() => { - if (weakRequest.NewValue.TryGetTarget(out var request)) - { - Schedule(() => - { - if (checkEquality(request.Model, TrackedItem)) - attachDownload(request); - }); - } - } + if (checkEquality(request.Model, TrackedItem)) + attachDownload(request); + }); - private void downloadFailed(ValueChangedEvent>> weakRequest) + private void downloadFailed(ArchiveDownloadRequest request) => Schedule(() => { - if (weakRequest.NewValue.TryGetTarget(out var request)) - { - Schedule(() => - { - if (checkEquality(request.Model, TrackedItem)) - attachDownload(null); - }); - } - } + if (checkEquality(request.Model, TrackedItem)) + attachDownload(null); + }); private void attachDownload(ArchiveDownloadRequest? request) { @@ -116,29 +94,17 @@ namespace osu.Game.Online private void onRequestFailure(Exception e) => Schedule(() => attachDownload(null)); - private void itemUpdated(ValueChangedEvent> weakItem) + private void itemUpdated(BeatmapSetInfo item) => Schedule(() => { - if (weakItem.NewValue.TryGetTarget(out var item)) - { - Schedule(() => - { - if (checkEquality(item, TrackedItem)) - UpdateState(DownloadState.LocallyAvailable); - }); - } - } + if (checkEquality(item, TrackedItem)) + UpdateState(DownloadState.LocallyAvailable); + }); - private void itemRemoved(ValueChangedEvent> weakItem) + private void itemRemoved(BeatmapSetInfo item) => Schedule(() => { - if (weakItem.NewValue.TryGetTarget(out var item)) - { - Schedule(() => - { - if (checkEquality(item, TrackedItem)) - UpdateState(DownloadState.NotDownloaded); - }); - } - } + if (checkEquality(item, TrackedItem)) + UpdateState(DownloadState.NotDownloaded); + }); private bool checkEquality(IBeatmapSetInfo x, IBeatmapSetInfo y) => x.OnlineID == y.OnlineID; @@ -148,6 +114,14 @@ namespace osu.Game.Online { base.Dispose(isDisposing); attachDownload(null); + + if (Manager != null) + { + Manager.DownloadBegan -= downloadBegan; + Manager.DownloadFailed -= downloadFailed; + Manager.ItemUpdated -= itemUpdated; + Manager.ItemRemoved -= itemRemoved; + } } #endregion diff --git a/osu.Game/Online/ScoreDownloadTracker.cs b/osu.Game/Online/ScoreDownloadTracker.cs index e679071ac1..37031ed930 100644 --- a/osu.Game/Online/ScoreDownloadTracker.cs +++ b/osu.Game/Online/ScoreDownloadTracker.cs @@ -3,7 +3,6 @@ using System; using osu.Framework.Allocation; -using osu.Framework.Bindables; using osu.Game.Online.API; using osu.Game.Scoring; @@ -23,11 +22,6 @@ namespace osu.Game.Online { } - private IBindable>? managerUpdated; - private IBindable>? managerRemoved; - private IBindable>>? managerDownloadBegan; - private IBindable>>? managerDownloadFailed; - [BackgroundDependencyLoader(true)] private void load() { @@ -46,39 +40,23 @@ namespace osu.Game.Online else attachDownload(Manager.GetExistingDownload(scoreInfo)); - 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); + Manager.DownloadBegan += downloadBegan; + Manager.DownloadFailed += downloadFailed; + Manager.ItemUpdated += itemUpdated; + Manager.ItemRemoved += itemRemoved; } - private void downloadBegan(ValueChangedEvent>> weakRequest) + private void downloadBegan(ArchiveDownloadRequest request) => Schedule(() => { - if (weakRequest.NewValue.TryGetTarget(out var request)) - { - Schedule(() => - { - if (checkEquality(request.Model, TrackedItem)) - attachDownload(request); - }); - } - } + if (checkEquality(request.Model, TrackedItem)) + attachDownload(request); + }); - private void downloadFailed(ValueChangedEvent>> weakRequest) + private void downloadFailed(ArchiveDownloadRequest request) => Schedule(() => { - if (weakRequest.NewValue.TryGetTarget(out var request)) - { - Schedule(() => - { - if (checkEquality(request.Model, TrackedItem)) - attachDownload(null); - }); - } - } + if (checkEquality(request.Model, TrackedItem)) + attachDownload(null); + }); private void attachDownload(ArchiveDownloadRequest? request) { @@ -120,29 +98,17 @@ namespace osu.Game.Online private void onRequestFailure(Exception e) => Schedule(() => attachDownload(null)); - private void itemUpdated(ValueChangedEvent> weakItem) + private void itemUpdated(ScoreInfo item) => Schedule(() => { - if (weakItem.NewValue.TryGetTarget(out var item)) - { - Schedule(() => - { - if (checkEquality(item, TrackedItem)) - UpdateState(DownloadState.LocallyAvailable); - }); - } - } + if (checkEquality(item, TrackedItem)) + UpdateState(DownloadState.LocallyAvailable); + }); - private void itemRemoved(ValueChangedEvent> weakItem) + private void itemRemoved(ScoreInfo item) => Schedule(() => { - if (weakItem.NewValue.TryGetTarget(out var item)) - { - Schedule(() => - { - if (checkEquality(item, TrackedItem)) - UpdateState(DownloadState.NotDownloaded); - }); - } - } + if (checkEquality(item, TrackedItem)) + UpdateState(DownloadState.LocallyAvailable); + }); private bool checkEquality(ScoreInfo x, ScoreInfo y) => x.OnlineScoreID == y.OnlineScoreID; @@ -152,6 +118,14 @@ namespace osu.Game.Online { base.Dispose(isDisposing); attachDownload(null); + + if (Manager != null) + { + Manager.DownloadBegan -= downloadBegan; + Manager.DownloadFailed -= downloadFailed; + Manager.ItemUpdated -= itemUpdated; + Manager.ItemRemoved -= itemRemoved; + } } #endregion diff --git a/osu.Game/OsuGameBase.cs b/osu.Game/OsuGameBase.cs index f6ec22a536..bf4f968e4e 100644 --- a/osu.Game/OsuGameBase.cs +++ b/osu.Game/OsuGameBase.cs @@ -206,17 +206,11 @@ namespace osu.Game dependencies.CacheAs(SkinManager); // needs to be done here rather than inside SkinManager to ensure thread safety of CurrentSkinInfo. - SkinManager.ItemRemoved.BindValueChanged(weakRemovedInfo => + SkinManager.ItemRemoved += item => Schedule(() => { - if (weakRemovedInfo.NewValue.TryGetTarget(out var removedInfo)) - { - Schedule(() => - { - // check the removed skin is not the current user choice. if it is, switch back to default. - if (removedInfo.ID == SkinManager.CurrentSkinInfo.Value.ID) - SkinManager.CurrentSkinInfo.Value = SkinInfo.Default; - }); - } + // check the removed skin is not the current user choice. if it is, switch back to default. + if (item.ID == SkinManager.CurrentSkinInfo.Value.ID) + SkinManager.CurrentSkinInfo.Value = SkinInfo.Default; }); EndpointConfiguration endpoints = UseDevelopmentServer ? (EndpointConfiguration)new DevelopmentEndpointConfiguration() : new ProductionEndpointConfiguration(); @@ -246,17 +240,8 @@ namespace osu.Game return ScoreManager.QueryScores(s => beatmapIds.Contains(s.BeatmapInfo.ID)).ToList(); } - BeatmapManager.ItemRemoved.BindValueChanged(i => - { - if (i.NewValue.TryGetTarget(out var item)) - ScoreManager.Delete(getBeatmapScores(item), true); - }); - - BeatmapManager.ItemUpdated.BindValueChanged(i => - { - if (i.NewValue.TryGetTarget(out var item)) - ScoreManager.Undelete(getBeatmapScores(item), true); - }); + BeatmapManager.ItemRemoved += item => ScoreManager.Delete(getBeatmapScores(item), true); + BeatmapManager.ItemUpdated += item => ScoreManager.Undelete(getBeatmapScores(item), true); dependencies.Cache(difficultyCache = new BeatmapDifficultyCache()); AddInternal(difficultyCache); diff --git a/osu.Game/Overlays/MusicController.cs b/osu.Game/Overlays/MusicController.cs index 97c7aaeaeb..829ff5be25 100644 --- a/osu.Game/Overlays/MusicController.cs +++ b/osu.Game/Overlays/MusicController.cs @@ -65,16 +65,11 @@ namespace osu.Game.Overlays [NotNull] public DrawableTrack CurrentTrack { get; private set; } = new DrawableTrack(new TrackVirtual(1000)); - private IBindable> managerUpdated; - private IBindable> managerRemoved; - [BackgroundDependencyLoader] private void load() { - managerUpdated = beatmaps.ItemUpdated.GetBoundCopy(); - managerUpdated.BindValueChanged(beatmapUpdated); - managerRemoved = beatmaps.ItemRemoved.GetBoundCopy(); - managerRemoved.BindValueChanged(beatmapRemoved); + beatmaps.ItemUpdated += beatmapUpdated; + beatmaps.ItemRemoved += beatmapRemoved; beatmapSets.AddRange(beatmaps.GetAllUsableBeatmapSets(IncludedDetails.Minimal, true).OrderBy(_ => RNG.Next())); @@ -110,28 +105,13 @@ namespace osu.Game.Overlays /// public bool TrackLoaded => CurrentTrack.TrackLoaded; - private void beatmapUpdated(ValueChangedEvent> weakSet) + private void beatmapUpdated(BeatmapSetInfo set) => Schedule(() => { - if (weakSet.NewValue.TryGetTarget(out var set)) - { - Schedule(() => - { - beatmapSets.Remove(set); - beatmapSets.Add(set); - }); - } - } + beatmapSets.Remove(set); + beatmapSets.Add(set); + }); - private void beatmapRemoved(ValueChangedEvent> weakSet) - { - if (weakSet.NewValue.TryGetTarget(out var set)) - { - Schedule(() => - { - beatmapSets.RemoveAll(s => s.ID == set.ID); - }); - } - } + private void beatmapRemoved(BeatmapSetInfo set) => Schedule(() => beatmapSets.RemoveAll(s => s.ID == set.ID)); private ScheduledDelegate seekDelegate; @@ -437,6 +417,17 @@ namespace osu.Game.Overlays mod.ApplyToTrack(CurrentTrack); } } + + protected override void Dispose(bool isDisposing) + { + base.Dispose(isDisposing); + + if (beatmaps != null) + { + beatmaps.ItemUpdated -= beatmapUpdated; + beatmaps.ItemRemoved -= beatmapRemoved; + } + } } public enum TrackChangeDirection diff --git a/osu.Game/Overlays/Settings/Sections/SkinSection.cs b/osu.Game/Overlays/Settings/Sections/SkinSection.cs index cf5d70ba91..0714b28b47 100644 --- a/osu.Game/Overlays/Settings/Sections/SkinSection.cs +++ b/osu.Game/Overlays/Settings/Sections/SkinSection.cs @@ -56,9 +56,6 @@ namespace osu.Game.Overlays.Settings.Sections [Resolved] private SkinManager skins { get; set; } - private IBindable> managerUpdated; - private IBindable> managerRemoved; - [BackgroundDependencyLoader(permitNulls: true)] private void load(OsuConfigManager config, [CanBeNull] SkinEditorOverlay skinEditor) { @@ -76,11 +73,8 @@ namespace osu.Game.Overlays.Settings.Sections new ExportSkinButton(), }; - managerUpdated = skins.ItemUpdated.GetBoundCopy(); - managerUpdated.BindValueChanged(itemUpdated); - - managerRemoved = skins.ItemRemoved.GetBoundCopy(); - managerRemoved.BindValueChanged(itemRemoved); + skins.ItemUpdated += itemUpdated; + skins.ItemRemoved += itemRemoved; config.BindWith(OsuSetting.Skin, configBindable); @@ -129,11 +123,7 @@ namespace osu.Game.Overlays.Settings.Sections skinDropdown.Items = skinItems; } - private void itemUpdated(ValueChangedEvent> weakItem) - { - if (weakItem.NewValue.TryGetTarget(out var item)) - Schedule(() => addItem(item)); - } + private void itemUpdated(SkinInfo item) => Schedule(() => addItem(item)); private void addItem(SkinInfo item) { @@ -142,11 +132,7 @@ namespace osu.Game.Overlays.Settings.Sections skinDropdown.Items = newDropdownItems; } - private void itemRemoved(ValueChangedEvent> weakItem) - { - if (weakItem.NewValue.TryGetTarget(out var item)) - Schedule(() => skinDropdown.Items = skinDropdown.Items.Where(i => i.ID != item.ID).ToArray()); - } + private void itemRemoved(SkinInfo item) => Schedule(() => skinDropdown.Items = skinDropdown.Items.Where(i => i.ID != item.ID).ToArray()); private void sortUserSkins(List skinsList) { @@ -155,6 +141,17 @@ namespace osu.Game.Overlays.Settings.Sections Comparer.Create((a, b) => string.Compare(a.Name, b.Name, StringComparison.OrdinalIgnoreCase))); } + protected override void Dispose(bool isDisposing) + { + base.Dispose(isDisposing); + + if (skins != null) + { + skins.ItemUpdated -= itemUpdated; + skins.ItemRemoved -= itemRemoved; + } + } + private class SkinSettingsDropdown : SettingsDropdown { protected override OsuDropdown CreateDropdown() => new SkinDropdownControl(); diff --git a/osu.Game/Scoring/ScoreManager.cs b/osu.Game/Scoring/ScoreManager.cs index 253591eb56..b5348cd179 100644 --- a/osu.Game/Scoring/ScoreManager.cs +++ b/osu.Game/Scoring/ScoreManager.cs @@ -246,9 +246,17 @@ namespace osu.Game.Scoring #region Implementation of IModelManager - public IBindable> ItemUpdated => scoreModelManager.ItemUpdated; + public Action ItemUpdated + { + get => scoreModelManager.ItemUpdated; + set => scoreModelManager.ItemUpdated = value; + } - public IBindable> ItemRemoved => scoreModelManager.ItemRemoved; + public Action ItemRemoved + { + get => scoreModelManager.ItemRemoved; + set => scoreModelManager.ItemRemoved = value; + } public Task ImportFromStableAsync(StableStorage stableStorage) { @@ -350,9 +358,17 @@ namespace osu.Game.Scoring #region Implementation of IModelDownloader - public IBindable>> DownloadBegan => scoreModelDownloader.DownloadBegan; + public Action> DownloadBegan + { + get => scoreModelDownloader.DownloadBegan; + set => scoreModelDownloader.DownloadBegan = value; + } - public IBindable>> DownloadFailed => scoreModelDownloader.DownloadFailed; + public Action> DownloadFailed + { + get => scoreModelDownloader.DownloadFailed; + set => scoreModelDownloader.DownloadFailed = value; + } public bool Download(ScoreInfo model, bool minimiseDownloadSize) { diff --git a/osu.Game/Screens/OnlinePlay/Match/RoomSubScreen.cs b/osu.Game/Screens/OnlinePlay/Match/RoomSubScreen.cs index 2cb29262e2..d6016ec4b9 100644 --- a/osu.Game/Screens/OnlinePlay/Match/RoomSubScreen.cs +++ b/osu.Game/Screens/OnlinePlay/Match/RoomSubScreen.cs @@ -62,8 +62,6 @@ namespace osu.Game.Screens.OnlinePlay.Match [Resolved(canBeNull: true)] protected OnlinePlayScreen ParentScreen { get; private set; } - private IBindable> managerUpdated; - [Cached] private OnlinePlayBeatmapAvailabilityTracker beatmapAvailabilityTracker { get; set; } @@ -246,8 +244,7 @@ namespace osu.Game.Screens.OnlinePlay.Match SelectedItem.BindValueChanged(_ => Scheduler.AddOnce(selectedItemChanged)); - managerUpdated = beatmapManager.ItemUpdated.GetBoundCopy(); - managerUpdated.BindValueChanged(beatmapUpdated); + beatmapManager.ItemUpdated += beatmapUpdated; UserMods.BindValueChanged(_ => Scheduler.AddOnce(UpdateMods)); } @@ -362,7 +359,7 @@ namespace osu.Game.Screens.OnlinePlay.Match } } - private void beatmapUpdated(ValueChangedEvent> weakSet) => Schedule(updateWorkingBeatmap); + private void beatmapUpdated(BeatmapSetInfo set) => Schedule(updateWorkingBeatmap); private void updateWorkingBeatmap() { @@ -431,6 +428,14 @@ namespace osu.Game.Screens.OnlinePlay.Match /// The room to change the settings of. protected abstract RoomSettingsOverlay CreateRoomSettingsOverlay(Room room); + protected override void Dispose(bool isDisposing) + { + base.Dispose(isDisposing); + + if (beatmapManager != null) + beatmapManager.ItemUpdated -= beatmapUpdated; + } + public class UserModSelectButton : PurpleTriangleButton { } diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index e5e28d2fde..0c593ebea1 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -143,11 +143,6 @@ namespace osu.Game.Screens.Select private CarouselRoot root; - private IBindable> itemUpdated; - private IBindable> itemRemoved; - private IBindable> itemHidden; - private IBindable> itemRestored; - private readonly DrawablePool setPool = new DrawablePool(100); public BeatmapCarousel() @@ -179,14 +174,10 @@ namespace osu.Game.Screens.Select RightClickScrollingEnabled.ValueChanged += enabled => Scroll.RightMouseScrollbar = enabled.NewValue; RightClickScrollingEnabled.TriggerChange(); - itemUpdated = beatmaps.ItemUpdated.GetBoundCopy(); - itemUpdated.BindValueChanged(beatmapUpdated); - itemRemoved = beatmaps.ItemRemoved.GetBoundCopy(); - itemRemoved.BindValueChanged(beatmapRemoved); - itemHidden = beatmaps.BeatmapHidden.GetBoundCopy(); - itemHidden.BindValueChanged(beatmapHidden); - itemRestored = beatmaps.BeatmapRestored.GetBoundCopy(); - itemRestored.BindValueChanged(beatmapRestored); + beatmaps.ItemUpdated += beatmapUpdated; + beatmaps.ItemRemoved += beatmapRemoved; + beatmaps.BeatmapHidden += beatmapHidden; + beatmaps.BeatmapRestored += beatmapRestored; if (!beatmapSets.Any()) loadBeatmapSets(GetLoadableBeatmaps()); @@ -675,29 +666,10 @@ namespace osu.Game.Screens.Select return (firstIndex, lastIndex); } - private void beatmapRemoved(ValueChangedEvent> weakItem) - { - if (weakItem.NewValue.TryGetTarget(out var item)) - RemoveBeatmapSet(item); - } - - private void beatmapUpdated(ValueChangedEvent> weakItem) - { - if (weakItem.NewValue.TryGetTarget(out var item)) - UpdateBeatmapSet(item); - } - - private void beatmapRestored(ValueChangedEvent> weakItem) - { - if (weakItem.NewValue.TryGetTarget(out var b)) - UpdateBeatmapSet(beatmaps.QueryBeatmapSet(s => s.ID == b.BeatmapSetInfoID)); - } - - private void beatmapHidden(ValueChangedEvent> weakItem) - { - if (weakItem.NewValue.TryGetTarget(out var b)) - UpdateBeatmapSet(beatmaps.QueryBeatmapSet(s => s.ID == b.BeatmapSetInfoID)); - } + private void beatmapRemoved(BeatmapSetInfo item) => RemoveBeatmapSet(item); + private void beatmapUpdated(BeatmapSetInfo item) => UpdateBeatmapSet(item); + private void beatmapRestored(BeatmapInfo b) => UpdateBeatmapSet(beatmaps.QueryBeatmapSet(s => s.ID == b.BeatmapSetInfoID)); + private void beatmapHidden(BeatmapInfo b) => UpdateBeatmapSet(beatmaps.QueryBeatmapSet(s => s.ID == b.BeatmapSetInfoID)); private CarouselBeatmapSet createCarouselSet(BeatmapSetInfo beatmapSet) { @@ -956,5 +928,18 @@ namespace osu.Game.Screens.Select return base.OnDragStart(e); } } + + protected override void Dispose(bool isDisposing) + { + base.Dispose(isDisposing); + + if (beatmaps != null) + { + beatmaps.ItemUpdated -= beatmapUpdated; + beatmaps.ItemRemoved -= beatmapRemoved; + beatmaps.BeatmapHidden -= beatmapHidden; + beatmaps.BeatmapRestored -= beatmapRestored; + } + } } } diff --git a/osu.Game/Screens/Select/Carousel/TopLocalRank.cs b/osu.Game/Screens/Select/Carousel/TopLocalRank.cs index f2485587d8..34129f232c 100644 --- a/osu.Game/Screens/Select/Carousel/TopLocalRank.cs +++ b/osu.Game/Screens/Select/Carousel/TopLocalRank.cs @@ -1,7 +1,6 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -using System; using System.Linq; using osu.Framework.Allocation; using osu.Framework.Bindables; @@ -28,9 +27,6 @@ namespace osu.Game.Screens.Select.Carousel [Resolved] private IAPIProvider api { get; set; } - private IBindable> itemUpdated; - private IBindable> itemRemoved; - public TopLocalRank(BeatmapInfo beatmapInfo) : base(null) { @@ -40,24 +36,18 @@ namespace osu.Game.Screens.Select.Carousel [BackgroundDependencyLoader] private void load() { - itemUpdated = scores.ItemUpdated.GetBoundCopy(); - itemUpdated.BindValueChanged(scoreChanged); - - itemRemoved = scores.ItemRemoved.GetBoundCopy(); - itemRemoved.BindValueChanged(scoreChanged); + scores.ItemUpdated += scoreChanged; + scores.ItemRemoved += scoreChanged; ruleset.ValueChanged += _ => fetchAndLoadTopScore(); fetchAndLoadTopScore(); } - private void scoreChanged(ValueChangedEvent> weakScore) + private void scoreChanged(ScoreInfo score) { - if (weakScore.NewValue.TryGetTarget(out var score)) - { - if (score.BeatmapInfoID == beatmapInfo.ID) - fetchAndLoadTopScore(); - } + if (score.BeatmapInfoID == beatmapInfo.ID) + fetchAndLoadTopScore(); } private ScheduledDelegate scheduledRankUpdate; @@ -86,5 +76,16 @@ namespace osu.Game.Screens.Select.Carousel .OrderByDescending(s => s.TotalScore) .FirstOrDefault(); } + + protected override void Dispose(bool isDisposing) + { + base.Dispose(isDisposing); + + if (scores != null) + { + scores.ItemUpdated -= scoreChanged; + scores.ItemRemoved -= scoreChanged; + } + } } } diff --git a/osu.Game/Screens/Select/Leaderboards/BeatmapLeaderboard.cs b/osu.Game/Screens/Select/Leaderboards/BeatmapLeaderboard.cs index de1ba5cf2e..9205c6c0d2 100644 --- a/osu.Game/Screens/Select/Leaderboards/BeatmapLeaderboard.cs +++ b/osu.Game/Screens/Select/Leaderboards/BeatmapLeaderboard.cs @@ -44,10 +44,6 @@ namespace osu.Game.Screens.Select.Leaderboards private bool filterMods; - private IBindable> itemRemoved; - - private IBindable> itemAdded; - /// /// Whether to apply the game's currently selected mods as a filter when retrieving scores. /// @@ -90,11 +86,8 @@ namespace osu.Game.Screens.Select.Leaderboards UpdateScores(); }; - itemRemoved = scoreManager.ItemRemoved.GetBoundCopy(); - itemRemoved.BindValueChanged(onScoreRemoved); - - itemAdded = scoreManager.ItemUpdated.GetBoundCopy(); - itemAdded.BindValueChanged(onScoreAdded); + scoreManager.ItemRemoved += scoreStoreChanged; + scoreManager.ItemUpdated += scoreStoreChanged; } protected override void Reset() @@ -103,22 +96,13 @@ namespace osu.Game.Screens.Select.Leaderboards TopScore = null; } - private void onScoreRemoved(ValueChangedEvent> score) => - scoreStoreChanged(score); - - private void onScoreAdded(ValueChangedEvent> score) => - scoreStoreChanged(score); - - private void scoreStoreChanged(ValueChangedEvent> score) + private void scoreStoreChanged(ScoreInfo score) { if (Scope != BeatmapLeaderboardScope.Local) return; - if (score.NewValue.TryGetTarget(out var scoreInfo)) - { - if (BeatmapInfo?.ID != scoreInfo.BeatmapInfoID) - return; - } + if (BeatmapInfo?.ID != score.BeatmapInfoID) + return; RefreshScores(); } @@ -215,5 +199,16 @@ namespace osu.Game.Screens.Select.Leaderboards { Action = () => ScoreSelected?.Invoke(model) }; + + protected override void Dispose(bool isDisposing) + { + base.Dispose(isDisposing); + + if (scoreManager != null) + { + scoreManager.ItemRemoved -= scoreStoreChanged; + scoreManager.ItemUpdated -= scoreStoreChanged; + } + } } } diff --git a/osu.Game/Screens/Spectate/SpectatorScreen.cs b/osu.Game/Screens/Spectate/SpectatorScreen.cs index c8df01dae6..f797f6e808 100644 --- a/osu.Game/Screens/Spectate/SpectatorScreen.cs +++ b/osu.Game/Screens/Spectate/SpectatorScreen.cs @@ -1,7 +1,6 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -using System; using System.Collections.Generic; using System.Diagnostics; using System.Linq; @@ -45,8 +44,6 @@ namespace osu.Game.Screens.Spectate private readonly Dictionary userMap = new Dictionary(); private readonly Dictionary gameplayStates = new Dictionary(); - private IBindable> managerUpdated; - /// /// Creates a new . /// @@ -73,20 +70,16 @@ namespace osu.Game.Screens.Spectate playingUserStates.BindTo(spectatorClient.PlayingUserStates); playingUserStates.BindCollectionChanged(onPlayingUserStatesChanged, true); - managerUpdated = beatmaps.ItemUpdated.GetBoundCopy(); - managerUpdated.BindValueChanged(beatmapUpdated); + beatmaps.ItemUpdated += beatmapUpdated; foreach ((int id, var _) in userMap) spectatorClient.WatchUser(id); })); } - private void beatmapUpdated(ValueChangedEvent> e) + private void beatmapUpdated(BeatmapSetInfo beatmapSet) { - if (!e.NewValue.TryGetTarget(out var beatmapSet)) - return; - - foreach ((int userId, var _) in userMap) + foreach ((int userId, _) in userMap) { if (!playingUserStates.TryGetValue(userId, out var userState)) continue; @@ -223,7 +216,8 @@ namespace osu.Game.Screens.Spectate spectatorClient.StopWatchingUser(userId); } - managerUpdated?.UnbindAll(); + if (beatmaps != null) + beatmaps.ItemUpdated -= beatmapUpdated; } } } From b8fb22b769d15a26abdc9a11c0ca9c8840e7b8d7 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sat, 6 Nov 2021 16:45:27 +0900 Subject: [PATCH 2/6] Add missing test coverage of score import process being tracked correctly --- .../Gameplay/TestSceneReplayDownloadButton.cs | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneReplayDownloadButton.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneReplayDownloadButton.cs index 64d9addc77..e178550298 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneReplayDownloadButton.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneReplayDownloadButton.cs @@ -11,6 +11,7 @@ using osu.Game.Users; using osu.Framework.Allocation; using osu.Framework.Bindables; using osu.Framework.Testing; +using osu.Game.Database; using osu.Game.Graphics.UserInterface; using osu.Game.Rulesets; using osu.Game.Rulesets.Osu; @@ -113,6 +114,36 @@ namespace osu.Game.Tests.Visual.Gameplay AddAssert("button is not enabled", () => !downloadButton.ChildrenOfType().First().Enabled.Value); } + [Resolved] + private ScoreManager scoreManager { get; set; } + + [Test] + public void TestScoreImportThenDelete() + { + ILive imported = null; + + AddStep("create button without replay", () => + { + Child = downloadButton = new TestReplayDownloadButton(getScoreInfo(false)) + { + Anchor = Anchor.Centre, + Origin = Anchor.Centre, + }; + }); + + AddUntilStep("wait for load", () => downloadButton.IsLoaded); + + AddUntilStep("state is not downloaded", () => downloadButton.State.Value == DownloadState.NotDownloaded); + + AddStep("import score", () => imported = scoreManager.Import(getScoreInfo(true)).Result); + + AddUntilStep("state is available", () => downloadButton.State.Value == DownloadState.LocallyAvailable); + + AddStep("delete score", () => scoreManager.Delete(imported.Value)); + + AddUntilStep("state is not downloaded", () => downloadButton.State.Value == DownloadState.NotDownloaded); + } + [Test] public void CreateButtonWithNoScore() { From 96ef210a4dda4b4b4fd6a78b3005a39590bd3262 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sat, 6 Nov 2021 16:38:42 +0900 Subject: [PATCH 3/6] Fix backward state set --- osu.Game/Online/ScoreDownloadTracker.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Online/ScoreDownloadTracker.cs b/osu.Game/Online/ScoreDownloadTracker.cs index 554aaba39d..32307fc50e 100644 --- a/osu.Game/Online/ScoreDownloadTracker.cs +++ b/osu.Game/Online/ScoreDownloadTracker.cs @@ -107,7 +107,7 @@ namespace osu.Game.Online private void itemRemoved(ScoreInfo item) => Schedule(() => { if (checkEquality(item, TrackedItem)) - UpdateState(DownloadState.LocallyAvailable); + UpdateState(DownloadState.NotDownloaded); }); private bool checkEquality(IScoreInfo x, IScoreInfo y) => x.OnlineID == y.OnlineID; From 89cc2523efbc3b4e62f395f55401d2144b624df8 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sat, 6 Nov 2021 22:31:49 +0900 Subject: [PATCH 4/6] Fix incorrectly specified events --- osu.Game/Beatmaps/BeatmapManager.cs | 24 ++++++++++++------------ osu.Game/Database/ArchiveModelManager.cs | 4 ++-- osu.Game/Database/IModelDownloader.cs | 4 ++-- osu.Game/Database/IModelManager.cs | 4 ++-- osu.Game/Database/ModelDownloader.cs | 4 ++-- osu.Game/Scoring/ScoreManager.cs | 24 ++++++++++++------------ 6 files changed, 32 insertions(+), 32 deletions(-) diff --git a/osu.Game/Beatmaps/BeatmapManager.cs b/osu.Game/Beatmaps/BeatmapManager.cs index da0c379f90..0f2cf6a730 100644 --- a/osu.Game/Beatmaps/BeatmapManager.cs +++ b/osu.Game/Beatmaps/BeatmapManager.cs @@ -205,16 +205,16 @@ namespace osu.Game.Beatmaps return beatmapModelManager.IsAvailableLocally(model); } - public Action ItemUpdated + public event Action ItemUpdated { - get => beatmapModelManager.ItemUpdated; - set => beatmapModelManager.ItemUpdated = value; + add => beatmapModelManager.ItemUpdated += value; + remove => beatmapModelManager.ItemUpdated -= value; } - public Action ItemRemoved + public event Action ItemRemoved { - get => beatmapModelManager.ItemRemoved; - set => beatmapModelManager.ItemRemoved = value; + add => beatmapModelManager.ItemRemoved += value; + remove => beatmapModelManager.ItemRemoved -= value; } public Task ImportFromStableAsync(StableStorage stableStorage) @@ -261,16 +261,16 @@ namespace osu.Game.Beatmaps #region Implementation of IModelDownloader - public Action> DownloadBegan + public event Action> DownloadBegan { - get => beatmapModelDownloader.DownloadBegan; - set => beatmapModelDownloader.DownloadBegan = value; + add => beatmapModelDownloader.DownloadBegan += value; + remove => beatmapModelDownloader.DownloadBegan -= value; } - public Action> DownloadFailed + public event Action> DownloadFailed { - get => beatmapModelDownloader.DownloadFailed; - set => beatmapModelDownloader.DownloadFailed = value; + add => beatmapModelDownloader.DownloadFailed += value; + remove => beatmapModelDownloader.DownloadFailed -= value; } public bool Download(IBeatmapSetInfo model, bool minimiseDownloadSize = false) diff --git a/osu.Game/Database/ArchiveModelManager.cs b/osu.Game/Database/ArchiveModelManager.cs index 85419aeedb..019749760f 100644 --- a/osu.Game/Database/ArchiveModelManager.cs +++ b/osu.Game/Database/ArchiveModelManager.cs @@ -62,13 +62,13 @@ namespace osu.Game.Database /// Fired when a new or updated becomes available in the database. /// This is not guaranteed to run on the update thread. /// - public Action ItemUpdated { get; set; } + public event Action ItemUpdated; /// /// Fired when a is removed from the database. /// This is not guaranteed to run on the update thread. /// - public Action ItemRemoved { get; set; } + public event Action ItemRemoved; public virtual IEnumerable HandledExtensions => new[] { @".zip" }; diff --git a/osu.Game/Database/IModelDownloader.cs b/osu.Game/Database/IModelDownloader.cs index d8e3dad8b5..81fba14244 100644 --- a/osu.Game/Database/IModelDownloader.cs +++ b/osu.Game/Database/IModelDownloader.cs @@ -17,13 +17,13 @@ namespace osu.Game.Database /// Fired when a download begins. /// This is NOT run on the update thread and should be scheduled. /// - Action> DownloadBegan { get; set; } + event Action> DownloadBegan; /// /// Fired when a download is interrupted, either due to user cancellation or failure. /// This is NOT run on the update thread and should be scheduled. /// - Action> DownloadFailed { get; set; } + event Action> DownloadFailed; /// /// Begin a download for the requested . diff --git a/osu.Game/Database/IModelManager.cs b/osu.Game/Database/IModelManager.cs index c9257f5877..15ad455f21 100644 --- a/osu.Game/Database/IModelManager.cs +++ b/osu.Game/Database/IModelManager.cs @@ -19,12 +19,12 @@ namespace osu.Game.Database /// /// Fired when an item is updated. /// - Action ItemUpdated { get; set; } + event Action ItemUpdated; /// /// Fired when an item is removed. /// - Action ItemRemoved { get; set; } + event Action ItemRemoved; /// /// This is a temporary method and will likely be replaced by a full-fledged (and more correctly placed) migration process in the future. diff --git a/osu.Game/Database/ModelDownloader.cs b/osu.Game/Database/ModelDownloader.cs index 3684dea456..3c1f181f24 100644 --- a/osu.Game/Database/ModelDownloader.cs +++ b/osu.Game/Database/ModelDownloader.cs @@ -19,9 +19,9 @@ namespace osu.Game.Database { public Action PostNotification { protected get; set; } - public Action> DownloadBegan { get; set; } + public event Action> DownloadBegan; - public Action> DownloadFailed { get; set; } + public event Action> DownloadFailed; private readonly IModelImporter importer; private readonly IAPIProvider api; diff --git a/osu.Game/Scoring/ScoreManager.cs b/osu.Game/Scoring/ScoreManager.cs index 0cb5ae2236..9b4216084e 100644 --- a/osu.Game/Scoring/ScoreManager.cs +++ b/osu.Game/Scoring/ScoreManager.cs @@ -246,16 +246,16 @@ namespace osu.Game.Scoring #region Implementation of IModelManager - public Action ItemUpdated + public event Action ItemUpdated { - get => scoreModelManager.ItemUpdated; - set => scoreModelManager.ItemUpdated = value; + add => scoreModelManager.ItemUpdated += value; + remove => scoreModelManager.ItemUpdated -= value; } - public Action ItemRemoved + public event Action ItemRemoved { - get => scoreModelManager.ItemRemoved; - set => scoreModelManager.ItemRemoved = value; + add => scoreModelManager.ItemRemoved += value; + remove => scoreModelManager.ItemRemoved -= value; } public Task ImportFromStableAsync(StableStorage stableStorage) @@ -358,16 +358,16 @@ namespace osu.Game.Scoring #region Implementation of IModelDownloader - public Action> DownloadBegan + public event Action> DownloadBegan { - get => scoreModelDownloader.DownloadBegan; - set => scoreModelDownloader.DownloadBegan = value; + add => scoreModelDownloader.DownloadBegan += value; + remove => scoreModelDownloader.DownloadBegan -= value; } - public Action> DownloadFailed + public event Action> DownloadFailed { - get => scoreModelDownloader.DownloadFailed; - set => scoreModelDownloader.DownloadFailed = value; + add => scoreModelDownloader.DownloadFailed += value; + remove => scoreModelDownloader.DownloadFailed -= value; } public bool Download(IScoreInfo model, bool minimiseDownloadSize) => From f0809801c5784f68c947cda253fa18467cb96564 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sun, 7 Nov 2021 11:34:37 +0900 Subject: [PATCH 5/6] Update remaining actions to events --- osu.Game/Beatmaps/BeatmapManager.cs | 12 ++++++------ osu.Game/Beatmaps/BeatmapModelManager.cs | 4 ++-- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/osu.Game/Beatmaps/BeatmapManager.cs b/osu.Game/Beatmaps/BeatmapManager.cs index 0f2cf6a730..b330cc2b4c 100644 --- a/osu.Game/Beatmaps/BeatmapManager.cs +++ b/osu.Game/Beatmaps/BeatmapManager.cs @@ -100,19 +100,19 @@ namespace osu.Game.Beatmaps /// /// Fired when a single difficulty has been hidden. /// - public Action BeatmapHidden + public event Action BeatmapHidden { - get => beatmapModelManager.BeatmapHidden; - set => beatmapModelManager.BeatmapHidden = value; + add => beatmapModelManager.BeatmapHidden += value; + remove => beatmapModelManager.BeatmapHidden -= value; } /// /// Fired when a single difficulty has been restored. /// - public Action BeatmapRestored + public event Action BeatmapRestored { - get => beatmapModelManager.BeatmapRestored; - set => beatmapModelManager.BeatmapRestored = value; + add => beatmapModelManager.BeatmapRestored += value; + remove => beatmapModelManager.BeatmapRestored -= value; } /// diff --git a/osu.Game/Beatmaps/BeatmapModelManager.cs b/osu.Game/Beatmaps/BeatmapModelManager.cs index 1a78a625d9..eb1bf598a4 100644 --- a/osu.Game/Beatmaps/BeatmapModelManager.cs +++ b/osu.Game/Beatmaps/BeatmapModelManager.cs @@ -36,12 +36,12 @@ namespace osu.Game.Beatmaps /// /// Fired when a single difficulty has been hidden. /// - public Action BeatmapHidden; + public event Action BeatmapHidden; /// /// Fired when a single difficulty has been restored. /// - public Action BeatmapRestored; + public event Action BeatmapRestored; /// /// An online lookup queue component which handles populating online beatmap metadata. From 6b6dd93e9e75e38bd73bd40b5a943f07c19b57d4 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 8 Nov 2021 14:17:47 +0900 Subject: [PATCH 6/6] Fix `LinkFlowContainer` not creating user links supporting full `IUser` specification --- .../Graphics/Containers/LinkFlowContainer.cs | 10 +++---- osu.Game/Online/Chat/MessageFormatter.cs | 8 ++--- osu.Game/OsuGame.cs | 29 ++++++++++++------- osu.Game/Overlays/Chat/ChatLine.cs | 2 +- .../Sections/Recent/DrawableRecentActivity.cs | 2 +- 5 files changed, 29 insertions(+), 22 deletions(-) diff --git a/osu.Game/Graphics/Containers/LinkFlowContainer.cs b/osu.Game/Graphics/Containers/LinkFlowContainer.cs index 41b7df228e..1d286d3487 100644 --- a/osu.Game/Graphics/Containers/LinkFlowContainer.cs +++ b/osu.Game/Graphics/Containers/LinkFlowContainer.cs @@ -46,7 +46,7 @@ namespace osu.Game.Graphics.Containers AddText(text[previousLinkEnd..link.Index]); string displayText = text.Substring(link.Index, link.Length); - string linkArgument = link.Argument; + object linkArgument = link.Argument; string tooltip = displayText == link.Url ? null : link.Url; AddLink(displayText, link.Action, linkArgument, tooltip); @@ -62,16 +62,16 @@ namespace osu.Game.Graphics.Containers public void AddLink(LocalisableString text, Action action, string tooltipText = null, Action creationParameters = null) => createLink(CreateChunkFor(text, true, CreateSpriteText, creationParameters), new LinkDetails(LinkAction.Custom, string.Empty), tooltipText, action); - public void AddLink(LocalisableString text, LinkAction action, string argument, string tooltipText = null, Action creationParameters = null) + public void AddLink(LocalisableString text, LinkAction action, object argument, string tooltipText = null, Action creationParameters = null) => createLink(CreateChunkFor(text, true, CreateSpriteText, creationParameters), new LinkDetails(action, argument), tooltipText); - public void AddLink(IEnumerable text, LinkAction action, string linkArgument, string tooltipText = null) + public void AddLink(IEnumerable text, LinkAction action, object linkArgument, string tooltipText = null) { createLink(new TextPartManual(text), new LinkDetails(action, linkArgument), tooltipText); } public void AddUserLink(IUser user, Action creationParameters = null) - => createLink(CreateChunkFor(user.Username, true, CreateSpriteText, creationParameters), new LinkDetails(LinkAction.OpenUserProfile, user.OnlineID.ToString()), "view profile"); + => createLink(CreateChunkFor(user.Username, true, CreateSpriteText, creationParameters), new LinkDetails(LinkAction.OpenUserProfile, user), "view profile"); private void createLink(ITextPart textPart, LinkDetails link, LocalisableString tooltipText, Action action = null) { @@ -83,7 +83,7 @@ namespace osu.Game.Graphics.Containers game.HandleLink(link); // fallback to handle cases where OsuGame is not available, ie. tournament client. else if (link.Action == LinkAction.External) - host.OpenUrlExternally(link.Argument); + host.OpenUrlExternally(link.Argument.ToString()); }; AddPart(new TextLink(textPart, tooltipText, onClickAction)); diff --git a/osu.Game/Online/Chat/MessageFormatter.cs b/osu.Game/Online/Chat/MessageFormatter.cs index 5e0f66443b..92911f0f51 100644 --- a/osu.Game/Online/Chat/MessageFormatter.cs +++ b/osu.Game/Online/Chat/MessageFormatter.cs @@ -320,9 +320,9 @@ namespace osu.Game.Online.Chat { public readonly LinkAction Action; - public readonly string Argument; + public readonly object Argument; - public LinkDetails(LinkAction action, string argument) + public LinkDetails(LinkAction action, object argument) { Action = action; Argument = argument; @@ -351,9 +351,9 @@ namespace osu.Game.Online.Chat public int Index; public int Length; public LinkAction Action; - public string Argument; + public object Argument; - public Link(string url, int startIndex, int length, LinkAction action, string argument) + public Link(string url, int startIndex, int length, LinkAction action, object argument) { Url = url; Index = startIndex; diff --git a/osu.Game/OsuGame.cs b/osu.Game/OsuGame.cs index ea8682e696..bf757a153f 100644 --- a/osu.Game/OsuGame.cs +++ b/osu.Game/OsuGame.cs @@ -289,25 +289,27 @@ namespace osu.Game /// The link to load. public void HandleLink(LinkDetails link) => Schedule(() => { + string argString = link.Argument.ToString(); + switch (link.Action) { case LinkAction.OpenBeatmap: // TODO: proper query params handling - if (int.TryParse(link.Argument.Contains('?') ? link.Argument.Split('?')[0] : link.Argument, out int beatmapId)) + if (int.TryParse(argString.Contains('?') ? argString.Split('?')[0] : argString, out int beatmapId)) ShowBeatmap(beatmapId); break; case LinkAction.OpenBeatmapSet: - if (int.TryParse(link.Argument, out int setId)) + if (int.TryParse(argString, out int setId)) ShowBeatmapSet(setId); break; case LinkAction.OpenChannel: - ShowChannel(link.Argument); + ShowChannel(argString); break; case LinkAction.SearchBeatmapSet: - SearchBeatmapSet(link.Argument); + SearchBeatmapSet(argString); break; case LinkAction.OpenEditorTimestamp: @@ -321,26 +323,31 @@ namespace osu.Game break; case LinkAction.External: - OpenUrlExternally(link.Argument); + OpenUrlExternally(argString); break; case LinkAction.OpenUserProfile: - ShowUser(int.TryParse(link.Argument, out int userId) - ? new APIUser { Id = userId } - : new APIUser { Username = link.Argument }); + if (!(link.Argument is IUser user)) + { + user = int.TryParse(argString, out int userId) + ? new APIUser { Id = userId } + : new APIUser { Username = argString }; + } + + ShowUser(user); break; case LinkAction.OpenWiki: - ShowWiki(link.Argument); + ShowWiki(argString); break; case LinkAction.OpenChangelog: - if (string.IsNullOrEmpty(link.Argument)) + if (string.IsNullOrEmpty(argString)) ShowChangelogListing(); else { - string[] changelogArgs = link.Argument.Split("/"); + string[] changelogArgs = argString.Split("/"); ShowChangelogBuild(changelogArgs[0], changelogArgs[1]); } diff --git a/osu.Game/Overlays/Chat/ChatLine.cs b/osu.Game/Overlays/Chat/ChatLine.cs index 97cd913b56..87d1b1a3ad 100644 --- a/osu.Game/Overlays/Chat/ChatLine.cs +++ b/osu.Game/Overlays/Chat/ChatLine.cs @@ -209,7 +209,7 @@ namespace osu.Game.Overlays.Chat username.Text = $@"{message.Sender.Username}" + (senderHasBackground || message.IsAction ? "" : ":"); // remove non-existent channels from the link list - message.Links.RemoveAll(link => link.Action == LinkAction.OpenChannel && chatManager?.AvailableChannels.Any(c => c.Name == link.Argument) != true); + message.Links.RemoveAll(link => link.Action == LinkAction.OpenChannel && chatManager?.AvailableChannels.Any(c => c.Name == link.Argument.ToString()) != true); ContentFlow.Clear(); ContentFlow.AddLinks(message.DisplayContent, message.Links); diff --git a/osu.Game/Overlays/Profile/Sections/Recent/DrawableRecentActivity.cs b/osu.Game/Overlays/Profile/Sections/Recent/DrawableRecentActivity.cs index 49b46f7e7a..cb8dae0bbc 100644 --- a/osu.Game/Overlays/Profile/Sections/Recent/DrawableRecentActivity.cs +++ b/osu.Game/Overlays/Profile/Sections/Recent/DrawableRecentActivity.cs @@ -216,7 +216,7 @@ namespace osu.Game.Overlays.Profile.Sections.Recent private void addBeatmapsetLink() => content.AddLink(activity.Beatmapset?.Title, LinkAction.OpenBeatmapSet, getLinkArgument(activity.Beatmapset?.Url), creationParameters: t => t.Font = getLinkFont()); - private string getLinkArgument(string url) => MessageFormatter.GetLinkDetails($"{api.APIEndpointUrl}{url}").Argument; + private string getLinkArgument(string url) => MessageFormatter.GetLinkDetails($"{api.APIEndpointUrl}{url}").Argument.ToString(); private FontUsage getLinkFont(FontWeight fontWeight = FontWeight.Regular) => OsuFont.GetFont(size: font_size, weight: fontWeight, italics: true);