1
0
mirror of https://github.com/ppy/osu.git synced 2024-09-21 17:27:24 +08:00

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:

    b4cefe0cc2/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).
This commit is contained in:
Bartłomiej Dach 2024-06-25 11:28:10 +02:00
parent b4cefe0cc2
commit 0d2a47167c
No known key found for this signature in database
3 changed files with 14 additions and 4 deletions

View File

@ -6,6 +6,7 @@ using System.Linq;
using Humanizer; using Humanizer;
using Humanizer.Localisation; using Humanizer.Localisation;
using osu.Framework.Bindables; using osu.Framework.Bindables;
using osu.Game.Rulesets;
using osu.Game.Utils; using osu.Game.Utils;
namespace osu.Game.Online.Rooms namespace osu.Game.Online.Rooms
@ -42,14 +43,14 @@ namespace osu.Game.Online.Rooms
/// <summary> /// <summary>
/// Returns the total duration from the <see cref="PlaylistItem"/> in playlist order from the supplied <paramref name="playlist"/>, /// Returns the total duration from the <see cref="PlaylistItem"/> in playlist order from the supplied <paramref name="playlist"/>,
/// </summary> /// </summary>
public static string GetTotalDuration(this BindableList<PlaylistItem> playlist) => public static string GetTotalDuration(this BindableList<PlaylistItem> playlist, RulesetStore rulesetStore) =>
playlist.Select(p => playlist.Select(p =>
{ {
double rate = 1; double rate = 1;
if (p.RequiredMods.Length > 0) 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))); rate = ModUtils.CalculateRateWithMods(p.RequiredMods.Select(mod => mod.ToMod(ruleset)));
} }

View File

@ -1,12 +1,17 @@
// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence. // Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
// See the LICENCE file in the repository root for full licence text. // See the LICENCE file in the repository root for full licence text.
using osu.Framework.Allocation;
using osu.Game.Online.Rooms; using osu.Game.Online.Rooms;
using osu.Game.Rulesets;
namespace osu.Game.Screens.OnlinePlay.Components namespace osu.Game.Screens.OnlinePlay.Components
{ {
public partial class OverlinedPlaylistHeader : OverlinedHeader public partial class OverlinedPlaylistHeader : OverlinedHeader
{ {
[Resolved]
private RulesetStore rulesets { get; set; } = null!;
public OverlinedPlaylistHeader() public OverlinedPlaylistHeader()
: base("Playlist") : base("Playlist")
{ {
@ -16,7 +21,7 @@ namespace osu.Game.Screens.OnlinePlay.Components
{ {
base.LoadComplete(); base.LoadComplete();
Playlist.BindCollectionChanged((_, _) => Details.Value = Playlist.GetTotalDuration(), true); Playlist.BindCollectionChanged((_, _) => Details.Value = Playlist.GetTotalDuration(rulesets), true);
} }
} }
} }

View File

@ -24,6 +24,7 @@ using osu.Game.Overlays;
using osu.Game.Screens.OnlinePlay.Match.Components; using osu.Game.Screens.OnlinePlay.Match.Components;
using osuTK; using osuTK;
using osu.Game.Localisation; using osu.Game.Localisation;
using osu.Game.Rulesets;
namespace osu.Game.Screens.OnlinePlay.Playlists namespace osu.Game.Screens.OnlinePlay.Playlists
{ {
@ -78,6 +79,9 @@ namespace osu.Game.Screens.OnlinePlay.Playlists
[Resolved] [Resolved]
private IAPIProvider api { get; set; } = null!; private IAPIProvider api { get; set; } = null!;
[Resolved]
private RulesetStore rulesets { get; set; } = null!;
private IBindable<APIUser> localUser = null!; private IBindable<APIUser> localUser = null!;
private readonly Room room; private readonly Room room;
@ -366,7 +370,7 @@ namespace osu.Game.Screens.OnlinePlay.Playlists
public void SelectBeatmap() => editPlaylistButton.TriggerClick(); public void SelectBeatmap() => editPlaylistButton.TriggerClick();
private void onPlaylistChanged(object? sender, NotifyCollectionChangedEventArgs e) => 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 private bool hasValidSettings => RoomID.Value == null && NameField.Text.Length > 0 && Playlist.Count > 0
&& hasValidDuration; && hasValidDuration;