From f907758b8b5f07c63443e3ae0b912f00b9bcd657 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Thu, 27 Mar 2025 00:11:55 +0900 Subject: [PATCH] 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; - } - } -}