Give OnlinePlaySongSelect a reference PlaylistItem

osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerMatchSongSelect.cs

@@ -152,8 +152,8 @@ namespace osu.Game.Tests.Visual.Multiplayer
 
             public new BeatmapCarousel Carousel => base.Carousel;
 
-            public TestMultiplayerMatchSongSelect(Room room, WorkingBeatmap beatmap = null, RulesetInfo ruleset = null)
+            public TestMultiplayerMatchSongSelect(Room room)
-                : base(room, null, beatmap, ruleset)
+                : base(room)
             {
             }
         }

osu.Game/Screens/OnlinePlay/Multiplayer/Match/MultiplayerMatchSettingsOverlay.cs

@@ -4,7 +4,6 @@
 using System;
 using System.ComponentModel;
 using System.Diagnostics;
-using System.Linq;
 using osu.Framework.Allocation;
 using osu.Framework.Bindables;
 using osu.Framework.Extensions;
@@ -13,7 +12,6 @@ using osu.Framework.Graphics;
 using osu.Framework.Graphics.Containers;
 using osu.Framework.Graphics.Shapes;
 using osu.Framework.Screens;
-using osu.Game.Beatmaps;
 using osu.Game.Graphics;
 using osu.Game.Graphics.Containers;
 using osu.Game.Graphics.Sprites;
@@ -72,9 +70,6 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Match
             private OsuSpriteText typeLabel = null!;
             private LoadingLayer loadingLayer = null!;
 
-            [Resolved]
-            private BeatmapManager beatmapManager { get; set; } = null!;
-
             public void SelectBeatmap()
             {
                 if (matchSubScreen.IsCurrentScreen())

osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerMatchSongSelect.cs

@@ -8,11 +8,9 @@ using osu.Framework.Allocation;
 using osu.Framework.Bindables;
 using osu.Framework.Logging;
 using osu.Framework.Screens;
-using osu.Game.Beatmaps;
 using osu.Game.Graphics.UserInterface;
 using osu.Game.Online.Multiplayer;
 using osu.Game.Online.Rooms;
-using osu.Game.Rulesets;
 using osu.Game.Rulesets.Mods;
 using osu.Game.Screens.Select;
 
@@ -27,7 +25,7 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer
         private OngoingOperationTracker operationTracker { get; set; } = null!;
 
         private readonly IBindable<bool> operationInProgress = new Bindable<bool>();
-        private readonly long? itemToEdit;
+        private readonly PlaylistItem? itemToEdit;
 
         private LoadingLayer loadingLayer = null!;
         private IDisposable? selectionOperation;
@@ -37,21 +35,10 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer
         /// </summary>
         /// <param name="room">The room.</param>
         /// <param name="itemToEdit">The item to be edited. May be null, in which case a new item will be added to the playlist.</param>
-        /// <param name="beatmap">An optional initial beatmap selection to perform.</param>
+        public MultiplayerMatchSongSelect(Room room, PlaylistItem? itemToEdit = null)
-        /// <param name="ruleset">An optional initial ruleset selection to perform.</param>
+            : base(room, itemToEdit)
-        public MultiplayerMatchSongSelect(Room room, long? itemToEdit = null, WorkingBeatmap? beatmap = null, RulesetInfo? ruleset = null)
-            : base(room)
         {
             this.itemToEdit = itemToEdit;
-
-            if (beatmap != null || ruleset != null)
-            {
-                Schedule(() =>
-                {
-                    if (beatmap != null) Beatmap.Value = beatmap;
-                    if (ruleset != null) Ruleset.Value = ruleset;
-                });
-            }
         }
 
         [BackgroundDependencyLoader]
@@ -80,7 +67,7 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer
         {
             if (operationInProgress.Value)
             {
-                Logger.Log($"{nameof(SelectedItem)} aborted due to {nameof(operationInProgress)}");
+                Logger.Log($"{nameof(SelectItem)} aborted due to {nameof(operationInProgress)}");
                 return false;
             }
 
@@ -92,7 +79,7 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer
 
                 var multiplayerItem = new MultiplayerPlaylistItem
                 {
-                    ID = itemToEdit ?? 0,
+                    ID = itemToEdit?.ID ?? 0,
                     BeatmapID = item.Beatmap.OnlineID,
                     BeatmapChecksum = item.Beatmap.MD5Hash,
                     RulesetID = item.RulesetID,

osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerMatchSubScreen.cs

@@ -49,11 +49,6 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer
         [Resolved]
         private MultiplayerClient client { get; set; }
 
-        [Resolved]
-        private BeatmapManager beatmapManager { get; set; }
-
-        private readonly IBindable<bool> isConnected = new Bindable<bool>();
-
         private AddItemButton addItemButton;
 
         public MultiplayerMatchSubScreen(Room room)
@@ -227,12 +222,7 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer
             if (!this.IsCurrentScreen())
                 return;
 
-            int id = itemToEdit?.Beatmap.OnlineID ?? Room.Playlist.Last().Beatmap.OnlineID;
+            this.Push(new MultiplayerMatchSongSelect(Room, itemToEdit));
-            var localBeatmap = beatmapManager.QueryBeatmap(b => b.OnlineID == id);
-
-            var workingBeatmap = localBeatmap == null ? null : beatmapManager.GetWorkingBeatmap(localBeatmap);
-
-            this.Push(new MultiplayerMatchSongSelect(Room, itemToEdit?.ID, workingBeatmap));
         }
 
         protected override Drawable CreateFooter() => new MultiplayerMatchFooter();
@@ -424,7 +414,7 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer
                 return;
             }
 
-            this.Push(new MultiplayerMatchSongSelect(Room, client.Room.Settings.PlaylistItemId, beatmap, ruleset));
+            this.Push(new MultiplayerMatchSongSelect(Room, Room.Playlist.Single(item => item.ID == client.Room.Settings.PlaylistItemId)));
         }
 
         protected override void Dispose(bool isDisposing)

osu.Game/Screens/OnlinePlay/OnlinePlaySongSelect.cs

@@ -1,13 +1,11 @@
 // 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.
 
-#nullable disable
-
 using System;
 using System.Collections.Generic;
+using System.Diagnostics;
 using System.Linq;
 using Humanizer;
-using JetBrains.Annotations;
 using osu.Framework.Allocation;
 using osu.Framework.Bindables;
 using osu.Framework.Graphics;
@@ -35,32 +33,39 @@ namespace osu.Game.Screens.OnlinePlay
         public override bool AllowEditing => false;
 
         [Resolved(typeof(Room), nameof(Room.Playlist))]
-        protected BindableList<PlaylistItem> Playlist { get; private set; }
+        protected BindableList<PlaylistItem> Playlist { get; private set; } = null!;
-
-        [CanBeNull]
-        [Resolved(CanBeNull = true)]
-        protected IBindable<PlaylistItem> SelectedItem { get; private set; }
 
         [Resolved]
-        private RulesetStore rulesets { get; set; }
+        private RulesetStore rulesets { get; set; } = null!;
+
+        [Resolved]
+        private BeatmapManager beatmapManager { get; set; } = null!;
 
         protected override UserActivity InitialActivity => new UserActivity.InLobby(room);
 
         protected readonly Bindable<IReadOnlyList<Mod>> FreeMods = new Bindable<IReadOnlyList<Mod>>(Array.Empty<Mod>());
 
         private readonly Room room;
+        private readonly PlaylistItem? initialItem;
+        private readonly FreeModSelectOverlay freeModSelectOverlay;
 
-        private WorkingBeatmap initialBeatmap;
+        private WorkingBeatmap initialBeatmap = null!;
-        private RulesetInfo initialRuleset;
+        private RulesetInfo initialRuleset = null!;
-        private IReadOnlyList<Mod> initialMods;
+        private IReadOnlyList<Mod> initialMods = null!;
         private bool itemSelected;
 
-        private readonly FreeModSelectOverlay freeModSelectOverlay;
+        private IDisposable? freeModSelectOverlayRegistration;
-        private IDisposable freeModSelectOverlayRegistration;
 
-        protected OnlinePlaySongSelect(Room room)
+        /// <summary>
+        /// Creates a new <see cref="OnlinePlaySongSelect"/>.
+        /// </summary>
+        /// <param name="room">The room.</param>
+        /// <param name="initialItem">An optional initial <see cref="PlaylistItem"/> to use for the initial beatmap/ruleset/mods.
+        /// If <c>null</c>, the last <see cref="PlaylistItem"/> in the room will be used.</param>
+        protected OnlinePlaySongSelect(Room room, PlaylistItem? initialItem = null)
         {
             this.room = room;
+            this.initialItem = initialItem ?? room.Playlist.LastOrDefault();
 
             Padding = new MarginPadding { Horizontal = HORIZONTAL_OVERFLOW_PADDING };
 
@@ -76,6 +81,7 @@ namespace osu.Game.Screens.OnlinePlay
         {
             LeftArea.Padding = new MarginPadding { Top = Header.HEIGHT };
 
+            // Store the initial beatmap/ruleset/mods at the point of entering song select, so they can be reverted to upon exit.
             initialBeatmap = Beatmap.Value;
             initialRuleset = Ruleset.Value;
             initialMods = Mods.Value.ToList();
@@ -87,14 +93,35 @@ namespace osu.Game.Screens.OnlinePlay
         {
             base.LoadComplete();
 
-            var rulesetInstance = SelectedItem?.Value?.RulesetID == null ? null : rulesets.GetRuleset(SelectedItem.Value.RulesetID)?.CreateInstance();
+            if (initialItem != null)
-
-            if (rulesetInstance != null)
             {
-                // At this point, Mods contains both the required and allowed mods. For selection purposes, it should only contain the required mods.
+                // Prefer using a local databased beatmap lookup since OnlineId may be -1 for an invalid beatmap selection.
-                // Similarly, freeMods is currently empty but should only contain the allowed mods.
+                BeatmapInfo? beatmapInfo = initialItem.Beatmap as BeatmapInfo;
-                Mods.Value = SelectedItem.Value.RequiredMods.Select(m => m.ToMod(rulesetInstance)).ToArray();
+
-                FreeMods.Value = SelectedItem.Value.AllowedMods.Select(m => m.ToMod(rulesetInstance)).ToArray();
+                // And in the case that this isn't a local databased beatmap, query by online ID.
+                if (beatmapInfo == null)
+                {
+                    int onlineId = initialItem.Beatmap.OnlineID;
+                    beatmapInfo = beatmapManager.QueryBeatmap(b => b.OnlineID == onlineId);
+                }
+
+                if (beatmapInfo != null)
+                    Beatmap.Value = beatmapManager.GetWorkingBeatmap(beatmapInfo);
+
+                RulesetInfo? ruleset = rulesets.GetRuleset(initialItem.RulesetID);
+
+                if (ruleset != null)
+                {
+                    Ruleset.Value = ruleset;
+
+                    var rulesetInstance = ruleset.CreateInstance();
+                    Debug.Assert(rulesetInstance != null);
+
+                    // At this point, Mods contains both the required and allowed mods. For selection purposes, it should only contain the required mods.
+                    // Similarly, freeMods is currently empty but should only contain the allowed mods.
+                    Mods.Value = initialItem.RequiredMods.Select(m => m.ToMod(rulesetInstance)).ToArray();
+                    FreeMods.Value = initialItem.AllowedMods.Select(m => m.ToMod(rulesetInstance)).ToArray();
+                }
             }
 
             Mods.BindValueChanged(onModsChanged);
@@ -199,7 +226,6 @@ namespace osu.Game.Screens.OnlinePlay
         protected override void Dispose(bool isDisposing)
         {
             base.Dispose(isDisposing);
-
             freeModSelectOverlayRegistration?.Dispose();
         }
     }