From 4f83c8661a67685f4897e99679d987f416104437 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sun, 30 Jul 2023 16:12:55 +0900 Subject: [PATCH 1/3] Remove unnecessary async fetch of beatmap in `NowPlayingOverlay` No idea if this was historically required for some reason, but it's definitely not required now. --- osu.Game/Overlays/NowPlayingOverlay.cs | 20 ++++---------------- 1 file changed, 4 insertions(+), 16 deletions(-) diff --git a/osu.Game/Overlays/NowPlayingOverlay.cs b/osu.Game/Overlays/NowPlayingOverlay.cs index 15eefb2d9f..f6fad0ef6c 100644 --- a/osu.Game/Overlays/NowPlayingOverlay.cs +++ b/osu.Game/Overlays/NowPlayingOverlay.cs @@ -4,7 +4,6 @@ #nullable disable using System; -using System.Threading.Tasks; using osu.Framework.Allocation; using osu.Framework.Bindables; using osu.Framework.Extensions.Color4Extensions; @@ -293,21 +292,10 @@ namespace osu.Game.Overlays // avoid using scheduler as our scheduler may not be run for a long time, holding references to beatmaps. pendingBeatmapSwitch = delegate { - // todo: this can likely be replaced with WorkingBeatmap.GetBeatmapAsync() - Task.Run(() => - { - if (beatmap?.Beatmap == null) // this is not needed if a placeholder exists - { - title.Text = @"Nothing to play"; - artist.Text = @"Nothing to play"; - } - else - { - BeatmapMetadata metadata = beatmap.Metadata; - title.Text = new RomanisableString(metadata.TitleUnicode, metadata.Title); - artist.Text = new RomanisableString(metadata.ArtistUnicode, metadata.Artist); - } - }); + BeatmapMetadata metadata = beatmap.Metadata; + + title.Text = new RomanisableString(metadata.TitleUnicode, metadata.Title); + artist.Text = new RomanisableString(metadata.ArtistUnicode, metadata.Artist); LoadComponentAsync(new Background(beatmap) { Depth = float.MaxValue }, newBackground => { From 07d224ecb6bfab5f72bb8f79210a8f43bd9a39e7 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sun, 30 Jul 2023 16:17:04 +0900 Subject: [PATCH 2/3] Apply NRT to `NowPlayingOverlay` --- osu.Game/Overlays/NowPlayingOverlay.cs | 39 +++++++++++++------------- 1 file changed, 19 insertions(+), 20 deletions(-) diff --git a/osu.Game/Overlays/NowPlayingOverlay.cs b/osu.Game/Overlays/NowPlayingOverlay.cs index f6fad0ef6c..37cb85983c 100644 --- a/osu.Game/Overlays/NowPlayingOverlay.cs +++ b/osu.Game/Overlays/NowPlayingOverlay.cs @@ -1,12 +1,11 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -#nullable disable - using System; using osu.Framework.Allocation; using osu.Framework.Bindables; using osu.Framework.Extensions.Color4Extensions; +using osu.Framework.Extensions.ObjectExtensions; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Framework.Graphics.Effects; @@ -39,33 +38,33 @@ namespace osu.Game.Overlays private const float bottom_black_area_height = 55; private const float margin = 10; - private Drawable background; - private ProgressBar progressBar; + private Drawable background = null!; + private ProgressBar progressBar = null!; - private IconButton prevButton; - private IconButton playButton; - private IconButton nextButton; - private IconButton playlistButton; + private IconButton prevButton = null!; + private IconButton playButton = null!; + private IconButton nextButton = null!; + private IconButton playlistButton = null!; - private SpriteText title, artist; + private SpriteText title = null!, artist = null!; - private PlaylistOverlay playlist; + private PlaylistOverlay? playlist; - private Container dragContainer; - private Container playerContainer; - private Container playlistContainer; + private Container dragContainer = null!; + private Container playerContainer = null!; + private Container playlistContainer = null!; protected override string PopInSampleName => "UI/now-playing-pop-in"; protected override string PopOutSampleName => "UI/now-playing-pop-out"; [Resolved] - private MusicController musicController { get; set; } + private MusicController musicController { get; set; } = null!; [Resolved] - private Bindable beatmap { get; set; } + private Bindable beatmap { get; set; } = null!; [Resolved] - private OsuColour colours { get; set; } + private OsuColour colours { get; set; } = null!; public NowPlayingOverlay() { @@ -285,7 +284,7 @@ namespace osu.Game.Overlays } } - private Action pendingBeatmapSwitch; + private Action? pendingBeatmapSwitch; private void trackChanged(WorkingBeatmap beatmap, TrackChangeDirection direction = TrackChangeDirection.None) { @@ -338,7 +337,7 @@ namespace osu.Game.Overlays { base.Dispose(isDisposing); - if (musicController != null) + if (musicController.IsNotNull()) musicController.TrackChanged -= trackChanged; } @@ -371,7 +370,7 @@ namespace osu.Game.Overlays private readonly Sprite sprite; private readonly WorkingBeatmap beatmap; - public Background(WorkingBeatmap beatmap = null) + public Background(WorkingBeatmap beatmap) : base(cachedFrameBuffer: true) { this.beatmap = beatmap; @@ -401,7 +400,7 @@ namespace osu.Game.Overlays [BackgroundDependencyLoader] private void load(LargeTextureStore textures) { - sprite.Texture = beatmap?.GetBackground() ?? textures.Get(@"Backgrounds/bg4"); + sprite.Texture = beatmap.GetBackground() ?? textures.Get(@"Backgrounds/bg4"); } } From de61b74e910d2342a6eeafac5a804796fe8418aa Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sun, 30 Jul 2023 16:21:35 +0900 Subject: [PATCH 3/3] Add proper cancellation and out-of-order blocking logic to `NowPlayingOverlay`'s background carousel --- osu.Game/Overlays/NowPlayingOverlay.cs | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/osu.Game/Overlays/NowPlayingOverlay.cs b/osu.Game/Overlays/NowPlayingOverlay.cs index 37cb85983c..89442e29bc 100644 --- a/osu.Game/Overlays/NowPlayingOverlay.cs +++ b/osu.Game/Overlays/NowPlayingOverlay.cs @@ -2,6 +2,7 @@ // See the LICENCE file in the repository root for full licence text. using System; +using System.Threading; using osu.Framework.Allocation; using osu.Framework.Bindables; using osu.Framework.Extensions.Color4Extensions; @@ -286,8 +287,14 @@ namespace osu.Game.Overlays private Action? pendingBeatmapSwitch; + private CancellationTokenSource? backgroundLoadCancellation; + + private WorkingBeatmap? currentBeatmap; + private void trackChanged(WorkingBeatmap beatmap, TrackChangeDirection direction = TrackChangeDirection.None) { + currentBeatmap = beatmap; + // avoid using scheduler as our scheduler may not be run for a long time, holding references to beatmaps. pendingBeatmapSwitch = delegate { @@ -296,8 +303,16 @@ namespace osu.Game.Overlays title.Text = new RomanisableString(metadata.TitleUnicode, metadata.Title); artist.Text = new RomanisableString(metadata.ArtistUnicode, metadata.Artist); + backgroundLoadCancellation?.Cancel(); + LoadComponentAsync(new Background(beatmap) { Depth = float.MaxValue }, newBackground => { + if (beatmap != currentBeatmap) + { + newBackground.Dispose(); + return; + } + switch (direction) { case TrackChangeDirection.Next: @@ -317,7 +332,7 @@ namespace osu.Game.Overlays background = newBackground; playerContainer.Add(newBackground); - }); + }, (backgroundLoadCancellation = new CancellationTokenSource()).Token); }; }