From 0d2a47167c72d29b2fa9bb46ead50c2e8e968960 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Tue, 25 Jun 2024 11:28:10 +0200 Subject: [PATCH] Fix crash on calculating playlist duration when rate-changing mods are present Regressed in https://github.com/ppy/osu/pull/28399. To reproduce, enter a playlist that has an item with a rate-changing mod (rather than create it yourself). This is happening because `APIRuleset` has `CreateInstance()` unimplemented: https://github.com/ppy/osu/blob/b4cefe0cc2fda0ab4b5af6138ee158bd32262f9a/osu.Game/Online/API/Requests/Responses/APIBeatmap.cs#L159 and only triggers when the playlist items in question originate from web. This is why it is bad to have interface implementations throw outside of maybe mock implementations for tests. `CreateInstance()` is a scourge elsewhere in general, we need way less of it in the codebase (because while convenient, it's also problematic to implement in online contexts, and also expensive because reflection). --- osu.Game/Online/Rooms/PlaylistExtensions.cs | 5 +++-- .../OnlinePlay/Components/OverlinedPlaylistHeader.cs | 7 ++++++- .../OnlinePlay/Playlists/PlaylistsRoomSettingsOverlay.cs | 6 +++++- 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/osu.Game/Online/Rooms/PlaylistExtensions.cs b/osu.Game/Online/Rooms/PlaylistExtensions.cs index e9a0519f3d..8591b5bb47 100644 --- a/osu.Game/Online/Rooms/PlaylistExtensions.cs +++ b/osu.Game/Online/Rooms/PlaylistExtensions.cs @@ -6,6 +6,7 @@ using System.Linq; using Humanizer; using Humanizer.Localisation; using osu.Framework.Bindables; +using osu.Game.Rulesets; using osu.Game.Utils; namespace osu.Game.Online.Rooms @@ -42,14 +43,14 @@ namespace osu.Game.Online.Rooms /// /// Returns the total duration from the in playlist order from the supplied , /// - public static string GetTotalDuration(this BindableList playlist) => + public static string GetTotalDuration(this BindableList playlist, RulesetStore rulesetStore) => playlist.Select(p => { double rate = 1; if (p.RequiredMods.Length > 0) { - var ruleset = p.Beatmap.Ruleset.CreateInstance(); + var ruleset = rulesetStore.GetRuleset(p.RulesetID)!.CreateInstance(); rate = ModUtils.CalculateRateWithMods(p.RequiredMods.Select(mod => mod.ToMod(ruleset))); } diff --git a/osu.Game/Screens/OnlinePlay/Components/OverlinedPlaylistHeader.cs b/osu.Game/Screens/OnlinePlay/Components/OverlinedPlaylistHeader.cs index fc86cbbbdd..dd728e460b 100644 --- a/osu.Game/Screens/OnlinePlay/Components/OverlinedPlaylistHeader.cs +++ b/osu.Game/Screens/OnlinePlay/Components/OverlinedPlaylistHeader.cs @@ -1,12 +1,17 @@ // 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.Game.Online.Rooms; +using osu.Game.Rulesets; namespace osu.Game.Screens.OnlinePlay.Components { public partial class OverlinedPlaylistHeader : OverlinedHeader { + [Resolved] + private RulesetStore rulesets { get; set; } = null!; + public OverlinedPlaylistHeader() : base("Playlist") { @@ -16,7 +21,7 @@ namespace osu.Game.Screens.OnlinePlay.Components { base.LoadComplete(); - Playlist.BindCollectionChanged((_, _) => Details.Value = Playlist.GetTotalDuration(), true); + Playlist.BindCollectionChanged((_, _) => Details.Value = Playlist.GetTotalDuration(rulesets), true); } } } diff --git a/osu.Game/Screens/OnlinePlay/Playlists/PlaylistsRoomSettingsOverlay.cs b/osu.Game/Screens/OnlinePlay/Playlists/PlaylistsRoomSettingsOverlay.cs index 84e419d67a..9166cac9de 100644 --- a/osu.Game/Screens/OnlinePlay/Playlists/PlaylistsRoomSettingsOverlay.cs +++ b/osu.Game/Screens/OnlinePlay/Playlists/PlaylistsRoomSettingsOverlay.cs @@ -24,6 +24,7 @@ using osu.Game.Overlays; using osu.Game.Screens.OnlinePlay.Match.Components; using osuTK; using osu.Game.Localisation; +using osu.Game.Rulesets; namespace osu.Game.Screens.OnlinePlay.Playlists { @@ -78,6 +79,9 @@ namespace osu.Game.Screens.OnlinePlay.Playlists [Resolved] private IAPIProvider api { get; set; } = null!; + [Resolved] + private RulesetStore rulesets { get; set; } = null!; + private IBindable localUser = null!; private readonly Room room; @@ -366,7 +370,7 @@ namespace osu.Game.Screens.OnlinePlay.Playlists public void SelectBeatmap() => editPlaylistButton.TriggerClick(); private void onPlaylistChanged(object? sender, NotifyCollectionChangedEventArgs e) => - playlistLength.Text = $"Length: {Playlist.GetTotalDuration()}"; + playlistLength.Text = $"Length: {Playlist.GetTotalDuration(rulesets)}"; private bool hasValidSettings => RoomID.Value == null && NameField.Text.Length > 0 && Playlist.Count > 0 && hasValidDuration;