From 61d539dc6794b1108aeab5e065a322f7e7b53795 Mon Sep 17 00:00:00 2001
From: smoogipoo <smoogipoo@smgi.me>
Date: Fri, 14 Feb 2020 23:39:39 +0900
Subject: [PATCH] Fix first playlist item not getting selected

---
 .../Multiplayer/TestSceneMatchSubScreen.cs    | 99 +++++++++++++------
 .../Screens/Multi/Match/MatchSubScreen.cs     | 21 ++--
 2 files changed, 81 insertions(+), 39 deletions(-)

diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneMatchSubScreen.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneMatchSubScreen.cs
index 61def4be1a..7f79e306ad 100644
--- a/osu.Game.Tests/Visual/Multiplayer/TestSceneMatchSubScreen.cs
+++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneMatchSubScreen.cs
@@ -3,19 +3,28 @@
 
 using System;
 using System.Collections.Generic;
+using System.Linq;
 using NUnit.Framework;
 using osu.Framework.Allocation;
 using osu.Framework.Bindables;
+using osu.Framework.Screens;
+using osu.Framework.Testing;
 using osu.Game.Beatmaps;
+using osu.Game.Graphics.UserInterface;
 using osu.Game.Online.Multiplayer;
 using osu.Game.Rulesets;
+using osu.Game.Rulesets.Osu;
+using osu.Game.Screens.Multi;
 using osu.Game.Screens.Multi.Match;
 using osu.Game.Screens.Multi.Match.Components;
+using osu.Game.Tests.Beatmaps;
 using osu.Game.Users;
+using osuTK.Input;
+using Header = osu.Game.Screens.Multi.Match.Components.Header;
 
 namespace osu.Game.Tests.Visual.Multiplayer
 {
-    public class TestSceneMatchSubScreen : ScreenTestScene
+    public class TestSceneMatchSubScreen : MultiplayerTestScene
     {
         protected override bool UseOnlineAPI => true;
 
@@ -27,8 +36,8 @@ namespace osu.Game.Tests.Visual.Multiplayer
             typeof(Footer)
         };
 
-        [Cached]
-        private readonly Bindable<Room> currentRoom = new Bindable<Room>();
+        [Cached(typeof(IRoomManager))]
+        private readonly TestRoomManager roomManager = new TestRoomManager();
 
         [Resolved]
         private BeatmapManager beatmaps { get; set; }
@@ -36,51 +45,77 @@ namespace osu.Game.Tests.Visual.Multiplayer
         [Resolved]
         private RulesetStore rulesets { get; set; }
 
-        public TestSceneMatchSubScreen()
+        private TestMatchSubScreen match;
+
+        [SetUp]
+        public void Setup() => Schedule(() =>
         {
-            currentRoom.Value = new Room();
+            Room.CopyFrom(new Room());
+        });
+
+        [SetUpSteps]
+        public void SetupSteps()
+        {
+            AddStep("load match", () => LoadScreen(match = new TestMatchSubScreen(Room)));
+            AddUntilStep("wait for load", () => match.IsCurrentScreen());
         }
 
         [Test]
-        public void TestShowRoom()
+        public void TestPlaylistItemSelectedOnCreate()
         {
-            AddStep(@"show", () =>
+            AddStep("set room properties", () =>
             {
-                currentRoom.Value.RoomID.Value = 1;
-                currentRoom.Value.Availability.Value = RoomAvailability.Public;
-                currentRoom.Value.Duration.Value = TimeSpan.FromHours(24);
-                currentRoom.Value.Host.Value = new User { Username = "peppy", Id = 2 };
-                currentRoom.Value.Name.Value = "super secret room";
-                currentRoom.Value.Participants.AddRange(new[]
+                Room.Name.Value = "my awesome room";
+                Room.Host.Value = new User { Id = 2, Username = "peppy" };
+                Room.Playlist.Add(new PlaylistItem
                 {
-                    new User { Username = "peppy", Id = 2 },
-                    new User { Username = "smoogipoo", Id = 1040328 }
+                    Beatmap = { Value = new TestBeatmap(new OsuRuleset().RulesetInfo).BeatmapInfo },
+                    Ruleset = { Value = new OsuRuleset().RulesetInfo }
                 });
-                currentRoom.Value.Playlist.Add(new PlaylistItem
-                {
-                    Beatmap = { Value = beatmaps.GetAllUsableBeatmapSets()[0].Beatmaps[0] },
-                    Ruleset = { Value = rulesets.GetRuleset(2) },
-                });
-
-                LoadScreen(new MatchSubScreen(currentRoom.Value));
             });
+
+            AddStep("move mouse to create button", () =>
+            {
+                var footer = match.ChildrenOfType<Footer>().Single();
+                InputManager.MoveMouseTo(footer.ChildrenOfType<OsuButton>().Single());
+            });
+
+            AddStep("click", () => InputManager.Click(MouseButton.Left));
+
+            AddAssert("first playlist item selected", () => match.SelectedItem.Value == Room.Playlist[0]);
         }
 
-        [Test]
-        public void TestShowSettings()
+        private class TestMatchSubScreen : MatchSubScreen
         {
-            AddStep(@"show", () =>
+            public new Bindable<PlaylistItem> SelectedItem => base.SelectedItem;
+
+            public TestMatchSubScreen(Room room)
+                : base(room)
             {
-                currentRoom.Value.RoomID.Value = null;
-                LoadScreen(new MatchSubScreen(currentRoom.Value));
-            });
+            }
         }
 
-        protected override IReadOnlyDependencyContainer CreateChildDependencies(IReadOnlyDependencyContainer parent)
+        private class TestRoomManager : IRoomManager
         {
-            var dependencies = new CachedModelDependencyContainer<Room>(base.CreateChildDependencies(parent));
-            dependencies.Model.BindTo(currentRoom);
-            return dependencies;
+            public event Action RoomsUpdated
+            {
+                add => throw new NotImplementedException();
+                remove => throw new NotImplementedException();
+            }
+
+            public IBindableList<Room> Rooms { get; } = new BindableList<Room>();
+
+            public void CreateRoom(Room room, Action<Room> onSuccess = null, Action<string> onError = null)
+            {
+                room.RoomID.Value = 1;
+                onSuccess?.Invoke(room);
+            }
+
+            public void JoinRoom(Room room, Action<Room> onSuccess = null, Action<string> onError = null) => onSuccess?.Invoke(room);
+
+            public void PartRoom()
+            {
+            }
         }
     }
 }
diff --git a/osu.Game/Screens/Multi/Match/MatchSubScreen.cs b/osu.Game/Screens/Multi/Match/MatchSubScreen.cs
index 44fccdaa13..75f836fe75 100644
--- a/osu.Game/Screens/Multi/Match/MatchSubScreen.cs
+++ b/osu.Game/Screens/Multi/Match/MatchSubScreen.cs
@@ -49,7 +49,8 @@ namespace osu.Game.Screens.Multi.Match
         [Resolved(canBeNull: true)]
         private Multiplayer multiplayer { get; set; }
 
-        private readonly Bindable<PlaylistItem> selectedItem = new Bindable<PlaylistItem>();
+        protected readonly Bindable<PlaylistItem> SelectedItem = new Bindable<PlaylistItem>();
+
         private LeaderboardChatDisplay leaderboardChatDisplay;
         private MatchSettingsOverlay settingsOverlay;
 
@@ -115,7 +116,7 @@ namespace osu.Game.Screens.Multi.Match
                                                                 Padding = new MarginPadding { Horizontal = 5 },
                                                                 Child = new OverlinedPlaylist(true) // Temporarily always allow selection
                                                                 {
-                                                                    SelectedItem = { BindTarget = selectedItem }
+                                                                    SelectedItem = { BindTarget = SelectedItem }
                                                                 }
                                                             },
                                                             new Container
@@ -143,7 +144,7 @@ namespace osu.Game.Screens.Multi.Match
                             new Footer
                             {
                                 OnStart = onStart,
-                                SelectedItem = { BindTarget = selectedItem }
+                                SelectedItem = { BindTarget = SelectedItem }
                             }
                         }
                     },
@@ -171,11 +172,17 @@ namespace osu.Game.Screens.Multi.Match
                 if (id.NewValue == null)
                     settingsOverlay.Show();
                 else
+                {
                     settingsOverlay.Hide();
+
+                    // Set the first playlist item.
+                    // This is scheduled since updating the room and playlist may happen in an arbitrary order (via Room.CopyFrom()).
+                    Schedule(() => SelectedItem.Value = playlist.FirstOrDefault());
+                }
             }, true);
 
-            selectedItem.BindValueChanged(selectedItemChanged);
-            selectedItem.Value = playlist.FirstOrDefault();
+            SelectedItem.BindValueChanged(selectedItemChanged);
+            SelectedItem.Value = playlist.FirstOrDefault();
 
             beatmapManager.ItemAdded += beatmapAdded;
         }
@@ -200,7 +207,7 @@ namespace osu.Game.Screens.Multi.Match
 
         private void updateWorkingBeatmap()
         {
-            var beatmap = selectedItem.Value?.Beatmap.Value;
+            var beatmap = SelectedItem.Value?.Beatmap.Value;
 
             // Retrieve the corresponding local beatmap, since we can't directly use the playlist's beatmap info
             var localBeatmap = beatmap == null ? null : beatmapManager.QueryBeatmap(b => b.OnlineBeatmapID == beatmap.OnlineBeatmapID);
@@ -222,7 +229,7 @@ namespace osu.Game.Screens.Multi.Match
             {
                 default:
                 case GameTypeTimeshift _:
-                    multiplayer?.Start(() => new TimeshiftPlayer(selectedItem.Value)
+                    multiplayer?.Start(() => new TimeshiftPlayer(SelectedItem.Value)
                     {
                         Exited = () => leaderboardChatDisplay.RefreshScores()
                     });