From 748e890ee4a627e0ad4e56bbbc38698dfaf8e6a1 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Tue, 25 Mar 2025 13:56:40 +0900 Subject: [PATCH 1/5] Refactor multiplayer background to remove selected item bindable --- .../Screens/OnlinePlay/Match/RoomSubScreen.cs | 5 +-- .../MultiplayerRoomBackgroundScreen.cs | 41 +++++++++++++++++++ 2 files changed, 42 insertions(+), 4 deletions(-) create mode 100644 osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerRoomBackgroundScreen.cs diff --git a/osu.Game/Screens/OnlinePlay/Match/RoomSubScreen.cs b/osu.Game/Screens/OnlinePlay/Match/RoomSubScreen.cs index f924ff6980..acf33ec59d 100644 --- a/osu.Game/Screens/OnlinePlay/Match/RoomSubScreen.cs +++ b/osu.Game/Screens/OnlinePlay/Match/RoomSubScreen.cs @@ -39,10 +39,7 @@ namespace osu.Game.Screens.OnlinePlay.Match public override bool? ApplyModTrackAdjustments => true; - protected override BackgroundScreen CreateBackground() => new RoomBackgroundScreen(Room.Playlist.FirstOrDefault()) - { - SelectedItem = { BindTarget = SelectedItem } - }; + protected override BackgroundScreen CreateBackground() => new MultiplayerRoomBackgroundScreen(); public override bool DisallowExternalBeatmapRulesetChanges => true; diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerRoomBackgroundScreen.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerRoomBackgroundScreen.cs new file mode 100644 index 0000000000..a31b002095 --- /dev/null +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerRoomBackgroundScreen.cs @@ -0,0 +1,41 @@ +// 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.Allocation; +using osu.Framework.Extensions.ObjectExtensions; +using osu.Game.Online.Multiplayer; +using osu.Game.Online.Rooms; +using osu.Game.Screens.OnlinePlay.Components; + +namespace osu.Game.Screens.OnlinePlay.Multiplayer +{ + public class MultiplayerRoomBackgroundScreen : OnlinePlayBackgroundScreen + { + [Resolved] + private MultiplayerClient client { get; set; } = null!; + + protected override void LoadComplete() + { + base.LoadComplete(); + + client.RoomUpdated += onRoomUpdated; + onRoomUpdated(); + } + + private void onRoomUpdated() => Scheduler.AddOnce(() => + { + if (client.Room == null) + return; + + PlaylistItem = new PlaylistItem(client.Room.CurrentPlaylistItem); + }); + + protected override void Dispose(bool isDisposing) + { + base.Dispose(isDisposing); + + if (client.IsNotNull()) + client.RoomUpdated -= onRoomUpdated; + } + } +} From d9efa086730ac046a8b81e6583fee1a8d7a510c6 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Tue, 25 Mar 2025 16:11:51 +0900 Subject: [PATCH 2/5] Make class partial --- .../OnlinePlay/Multiplayer/MultiplayerRoomBackgroundScreen.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerRoomBackgroundScreen.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerRoomBackgroundScreen.cs index a31b002095..6cb3b7c688 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerRoomBackgroundScreen.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerRoomBackgroundScreen.cs @@ -9,7 +9,7 @@ using osu.Game.Screens.OnlinePlay.Components; namespace osu.Game.Screens.OnlinePlay.Multiplayer { - public class MultiplayerRoomBackgroundScreen : OnlinePlayBackgroundScreen + public partial class MultiplayerRoomBackgroundScreen : OnlinePlayBackgroundScreen { [Resolved] private MultiplayerClient client { get; set; } = null!; From f907758b8b5f07c63443e3ae0b912f00b9bcd657 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Thu, 27 Mar 2025 00:11:55 +0900 Subject: [PATCH 3/5] Fix room background not working in multiplayer At the core of the problem is that the multiplayer server does not serialise beatmap covers to clients -- only the beatmap ids. Because of this, any components that need to display the background should query it from an online source first (i.e. via `BeatmapLookupCache`). There is a slightly tricky situation here formed of two parts, which I'll try to explain below. `Background.Sprite` is exposed publicly and some inheritors override the sprite's texture in a similar fashion to the way this changeset does. While I frankly believe this is unnaceptable from an encapsulation point of view, I've gone for consistency in this regard. The other fail case is `UpdateableBeatmapBackgroundSprite`. Contrary to its name, that object is _not_ a `Sprite` - it is a `ModelBackedDrawable` that _contains_ a `Sprite`. The logic in this PR could be extracted into a separate object similar to `OnlineBeatmapSetCover` (an actual `Sprite`), but even so it would require `Background` to provide a path for overriding its contained `Sprite`. I went through the path above originally with the changes visible in https://github.com/smoogipoo/osu/tree/fix-mp-backgrounds, but it looks quite daunting and I feel like there would be a lot of emphasis on the correct abstraction model for `Background`, of which I'm not entirely sure of. Instead of dealing with both of the above, this commit presents a direct; local resolution to the problem. --- .../Components/OnlinePlayBackgroundScreen.cs | 113 ++++++++++++------ .../Components/PlaylistItemBackground.cs | 42 ------- 2 files changed, 76 insertions(+), 79 deletions(-) delete mode 100644 osu.Game/Screens/OnlinePlay/Components/PlaylistItemBackground.cs diff --git a/osu.Game/Screens/OnlinePlay/Components/OnlinePlayBackgroundScreen.cs b/osu.Game/Screens/OnlinePlay/Components/OnlinePlayBackgroundScreen.cs index ef7c1747e9..4ca2d8e8a6 100644 --- a/osu.Game/Screens/OnlinePlay/Components/OnlinePlayBackgroundScreen.cs +++ b/osu.Game/Screens/OnlinePlay/Components/OnlinePlayBackgroundScreen.cs @@ -3,10 +3,15 @@ using System.Threading; using osu.Framework.Allocation; +using osu.Framework.Extensions; using osu.Framework.Graphics; using osu.Framework.Graphics.Colour; +using osu.Framework.Graphics.Textures; using osu.Framework.Screens; using osu.Game.Beatmaps; +using osu.Game.Database; +using osu.Game.Graphics.Backgrounds; +using osu.Game.Online.API.Requests.Responses; using osu.Game.Online.Rooms; using osuTK; using osuTK.Graphics; @@ -16,65 +21,57 @@ namespace osu.Game.Screens.OnlinePlay.Components public abstract partial class OnlinePlayBackgroundScreen : BackgroundScreen { private CancellationTokenSource? cancellationSource; - private PlaylistItemBackground? background; + private Background? lastBackground; + private int? beatmapId; [BackgroundDependencyLoader] private void load() { - switchBackground(new PlaylistItemBackground(playlistItem)); + loadNewBackground(); } - private PlaylistItem? playlistItem; - protected PlaylistItem? PlaylistItem { - get => playlistItem; set { - if (playlistItem == value) + if (beatmapId == value?.Beatmap.OnlineID) return; - playlistItem = value; + beatmapId = value?.Beatmap.OnlineID; - if (LoadState > LoadState.Ready) - updateBackground(); + if (LoadState >= LoadState.Ready) + loadNewBackground(); } } - private void updateBackground() + private void loadNewBackground() { - Schedule(() => + cancellationSource?.Cancel(); + cancellationSource = new CancellationTokenSource(); + + if (beatmapId == null) + switchBackground(new DefaultBackground()); + else + LoadComponentAsync(new OnlineBeatmapBackground(beatmapId.Value), switchBackground, cancellationSource.Token); + + void switchBackground(Background newBackground) { - var beatmap = playlistItem?.Beatmap; + float newDepth = 0; - string? lastCover = (background?.Beatmap?.BeatmapSet as IBeatmapSetOnlineInfo)?.Covers.Cover; - string? newCover = (beatmap?.BeatmapSet as IBeatmapSetOnlineInfo)?.Covers.Cover; + if (lastBackground != null) + { + newDepth = lastBackground.Depth + 1; + lastBackground.FinishTransforms(); + lastBackground.FadeOut(250); + lastBackground.Expire(); + } - if (lastCover == newCover) - return; + newBackground.Depth = newDepth; + newBackground.Colour = ColourInfo.GradientVertical(new Color4(0.1f, 0.1f, 0.1f, 1f), new Color4(0.4f, 0.4f, 0.4f, 1f)); + newBackground.BlurTo(new Vector2(10)); - cancellationSource?.Cancel(); - LoadComponentAsync(new PlaylistItemBackground(playlistItem), switchBackground, (cancellationSource = new CancellationTokenSource()).Token); - }); - } - - private void switchBackground(PlaylistItemBackground newBackground) - { - float newDepth = 0; - - if (background != null) - { - newDepth = background.Depth + 1; - background.FinishTransforms(); - background.FadeOut(250); - background.Expire(); + AddInternal(lastBackground = newBackground); } - - newBackground.Depth = newDepth; - newBackground.Colour = ColourInfo.GradientVertical(new Color4(0.1f, 0.1f, 0.1f, 1f), new Color4(0.4f, 0.4f, 0.4f, 1f)); - newBackground.BlurTo(new Vector2(10)); - - AddInternal(background = newBackground); } public override void OnSuspending(ScreenTransitionEvent e) @@ -89,5 +86,47 @@ namespace osu.Game.Screens.OnlinePlay.Components this.MoveToX(0); return result; } + + [LongRunningLoad] + private partial class OnlineBeatmapBackground : Background + { + private readonly int beatmapId; + + public OnlineBeatmapBackground(int beatmapId) + { + this.beatmapId = beatmapId; + } + + [BackgroundDependencyLoader] + private void load(BeatmapLookupCache lookupCache, LargeTextureStore textures, CancellationToken cancellationToken) + { + try + { + APIBeatmap? beatmap = lookupCache.GetBeatmapAsync(beatmapId, cancellationToken).GetResultSafely(); + string? coverImage = beatmap?.BeatmapSet?.Covers.Cover; + + if (coverImage != null) + Sprite.Texture = textures.Get(coverImage); + } + catch + { + } + } + } + + private class DefaultBackground : Background + { + [Resolved] + private BeatmapManager beatmapManager { get; set; } = null!; + + [BackgroundDependencyLoader] + private void load() + { + Sprite.Texture = beatmapManager.DefaultBeatmap.GetBackground(); + } + + public override bool Equals(Background? other) + => other is DefaultBackground; + } } } diff --git a/osu.Game/Screens/OnlinePlay/Components/PlaylistItemBackground.cs b/osu.Game/Screens/OnlinePlay/Components/PlaylistItemBackground.cs deleted file mode 100644 index 6b06eaee1e..0000000000 --- a/osu.Game/Screens/OnlinePlay/Components/PlaylistItemBackground.cs +++ /dev/null @@ -1,42 +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.Allocation; -using osu.Framework.Graphics.Textures; -using osu.Game.Beatmaps; -using osu.Game.Graphics.Backgrounds; -using osu.Game.Online.Rooms; - -namespace osu.Game.Screens.OnlinePlay.Components -{ - public partial class PlaylistItemBackground : Background - { - public readonly IBeatmapInfo? Beatmap; - - public PlaylistItemBackground(PlaylistItem? playlistItem) - { - Beatmap = playlistItem?.Beatmap; - } - - [BackgroundDependencyLoader] - private void load(BeatmapManager beatmaps, LargeTextureStore textures) - { - Texture? texture = null; - - // prefer online cover where available. - if (Beatmap?.BeatmapSet is IBeatmapSetOnlineInfo online) - texture = textures.Get(online.Covers.Cover); - - Sprite.Texture = texture ?? beatmaps.DefaultBeatmap.GetBackground(); - } - - public override bool Equals(Background? other) - { - if (ReferenceEquals(null, other)) return false; - if (ReferenceEquals(this, other)) return true; - - return other.GetType() == GetType() - && ((PlaylistItemBackground)other).Beatmap == Beatmap; - } - } -} From c720f2b33f1bde123d4ba0240be6605ac445f83a Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Thu, 27 Mar 2025 00:35:21 +0900 Subject: [PATCH 4/5] Make class partial --- .../Screens/OnlinePlay/Components/OnlinePlayBackgroundScreen.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Screens/OnlinePlay/Components/OnlinePlayBackgroundScreen.cs b/osu.Game/Screens/OnlinePlay/Components/OnlinePlayBackgroundScreen.cs index 4ca2d8e8a6..4b34987c30 100644 --- a/osu.Game/Screens/OnlinePlay/Components/OnlinePlayBackgroundScreen.cs +++ b/osu.Game/Screens/OnlinePlay/Components/OnlinePlayBackgroundScreen.cs @@ -114,7 +114,7 @@ namespace osu.Game.Screens.OnlinePlay.Components } } - private class DefaultBackground : Background + private partial class DefaultBackground : Background { [Resolved] private BeatmapManager beatmapManager { get; set; } = null!; From 050cc27125c5b23f5134e3e753a951d4e6876ef0 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Thu, 27 Mar 2025 19:24:57 +0900 Subject: [PATCH 5/5] Apply suggestions from review --- .../Components/OnlinePlayBackgroundScreen.cs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/osu.Game/Screens/OnlinePlay/Components/OnlinePlayBackgroundScreen.cs b/osu.Game/Screens/OnlinePlay/Components/OnlinePlayBackgroundScreen.cs index 4b34987c30..4f9d1b9246 100644 --- a/osu.Game/Screens/OnlinePlay/Components/OnlinePlayBackgroundScreen.cs +++ b/osu.Game/Screens/OnlinePlay/Components/OnlinePlayBackgroundScreen.cs @@ -1,12 +1,14 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. +using System; using System.Threading; using osu.Framework.Allocation; using osu.Framework.Extensions; using osu.Framework.Graphics; using osu.Framework.Graphics.Colour; using osu.Framework.Graphics.Textures; +using osu.Framework.Logging; using osu.Framework.Screens; using osu.Game.Beatmaps; using osu.Game.Database; @@ -108,9 +110,13 @@ namespace osu.Game.Screens.OnlinePlay.Components if (coverImage != null) Sprite.Texture = textures.Get(coverImage); } - catch + catch (OperationCanceledException) { } + catch (Exception ex) + { + Logger.Error(ex, $"Failed to retrieve cover image for beatmap {beatmapId}."); + } } } @@ -124,9 +130,6 @@ namespace osu.Game.Screens.OnlinePlay.Components { Sprite.Texture = beatmapManager.DefaultBeatmap.GetBackground(); } - - public override bool Equals(Background? other) - => other is DefaultBackground; } } }