Merge pull request #13786 from peppy/multiplayer-test-fixes

Fix multiplayer test failures due to leaderboard load process

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

@@ -11,6 +11,7 @@ using osu.Framework.Platform;
 using osu.Framework.Screens;
 using osu.Framework.Testing;
 using osu.Game.Beatmaps;
+using osu.Game.Database;
 using osu.Game.Graphics.UserInterface;
 using osu.Game.Online.Multiplayer;
 using osu.Game.Online.Rooms;
@@ -39,10 +40,8 @@ namespace osu.Game.Tests.Visual.Multiplayer
         private TestMultiplayer multiplayerScreen;
         private TestMultiplayerClient client;
 
-        public TestSceneMultiplayer()
+        [Cached(typeof(UserLookupCache))]
-        {
+        private UserLookupCache lookupCache = new TestUserLookupCache();
-            loadMultiplayer();
-        }
 
         [BackgroundDependencyLoader]
         private void load(GameHost host, AudioManager audio)
@@ -51,18 +50,43 @@ namespace osu.Game.Tests.Visual.Multiplayer
             Dependencies.Cache(beatmaps = new BeatmapManager(LocalStorage, ContextFactory, rulesets, null, audio, Resources, host, Beatmap.Default));
         }
 
-        [SetUp]
+        public override void SetUpSteps()
-        public void Setup() => Schedule(() =>
         {
-            beatmaps.Import(TestResources.GetQuickTestBeatmapForImport()).Wait();
+            base.SetUpSteps();
-            importedSet = beatmaps.GetAllUsableBeatmapSetsEnumerable(IncludedDetails.All).First();
+
-        });
+            AddStep("import beatmap", () =>
+            {
+                beatmaps.Import(TestResources.GetQuickTestBeatmapForImport()).Wait();
+                importedSet = beatmaps.GetAllUsableBeatmapSetsEnumerable(IncludedDetails.All).First();
+            });
+
+            AddStep("create multiplayer screen", () => multiplayerScreen = new TestMultiplayer());
+
+            AddStep("load dependencies", () =>
+            {
+                client = new TestMultiplayerClient(multiplayerScreen.RoomManager);
+
+                // The screen gets suspended so it stops receiving updates.
+                Child = client;
+
+                LoadScreen(dependenciesScreen = new DependenciesScreen(client));
+            });
+
+            AddUntilStep("wait for dependencies to load", () => dependenciesScreen.IsLoaded);
+
+            AddStep("load multiplayer", () => LoadScreen(multiplayerScreen));
+            AddUntilStep("wait for multiplayer to load", () => multiplayerScreen.IsLoaded);
+        }
+
+        [Test]
+        public void TestEmpty()
+        {
+            // used to test the flow of multiplayer from visual tests.
+        }
 
         [Test]
         public void TestUserSetToIdleWhenBeatmapDeleted()
         {
-            loadMultiplayer();
-
             createRoom(() => new Room
             {
                 Name = { Value = "Test Room" },
@@ -85,8 +109,6 @@ namespace osu.Game.Tests.Visual.Multiplayer
         [Test]
         public void TestLocalPlayDoesNotStartWhileSpectatingWithNoBeatmap()
         {
-            loadMultiplayer();
-
             createRoom(() => new Room
             {
                 Name = { Value = "Test Room" },
@@ -123,8 +145,6 @@ namespace osu.Game.Tests.Visual.Multiplayer
         [Test]
         public void TestLocalPlayStartsWhileSpectatingWhenBeatmapBecomesAvailable()
         {
-            loadMultiplayer();
-
             createRoom(() => new Room
             {
                 Name = { Value = "Test Room" },
@@ -167,8 +187,6 @@ namespace osu.Game.Tests.Visual.Multiplayer
         [Test]
         public void TestLeaveNavigation()
         {
-            loadMultiplayer();
-
             createRoom(() => new Room
             {
                 Name = { Value = "Test Room" },
@@ -227,26 +245,6 @@ namespace osu.Game.Tests.Visual.Multiplayer
             AddUntilStep("wait for join", () => client.Room != null);
         }
 
-        private void loadMultiplayer()
-        {
-            AddStep("create multiplayer screen", () => multiplayerScreen = new TestMultiplayer());
-
-            AddStep("load dependencies", () =>
-            {
-                client = new TestMultiplayerClient(multiplayerScreen.RoomManager);
-
-                // The screen gets suspended so it stops receiving updates.
-                Child = client;
-
-                LoadScreen(dependenciesScreen = new DependenciesScreen(client));
-            });
-
-            AddUntilStep("wait for dependencies to load", () => dependenciesScreen.IsLoaded);
-
-            AddStep("load multiplayer", () => LoadScreen(multiplayerScreen));
-            AddUntilStep("wait for multiplayer to load", () => multiplayerScreen.IsLoaded);
-        }
-
         /// <summary>
         /// Used for the sole purpose of adding <see cref="TestMultiplayerClient"/> as a resolvable dependency.
         /// </summary>

osu.Game/Database/UserLookupCache.cs

@@ -27,6 +27,30 @@ namespace osu.Game.Database
         [ItemCanBeNull]
         public Task<User> GetUserAsync(int userId, CancellationToken token = default) => GetAsync(userId, token);
 
+        /// <summary>
+        /// Perform an API lookup on the specified users, populating a <see cref="User"/> model.
+        /// </summary>
+        /// <param name="userIds">The users to lookup.</param>
+        /// <param name="token">An optional cancellation token.</param>
+        /// <returns>The populated users. May include null results for failed retrievals.</returns>
+        public Task<User[]> GetUsersAsync(int[] userIds, CancellationToken token = default)
+        {
+            var userLookupTasks = new List<Task<User>>();
+
+            foreach (var u in userIds)
+            {
+                userLookupTasks.Add(GetUserAsync(u, token).ContinueWith(task =>
+                {
+                    if (!task.IsCompletedSuccessfully)
+                        return null;
+
+                    return task.Result;
+                }, token));
+            }
+
+            return Task.WhenAll(userLookupTasks);
+        }
+
         protected override async Task<User> ComputeValueAsync(int lookup, CancellationToken token = default)
             => await queryUser(lookup).ConfigureAwait(false);
 

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

@@ -125,9 +125,7 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer
         {
             const float padding = 44; // enough margin to avoid the hit error display.
 
-            leaderboard.Position = new Vector2(
+            leaderboard.Position = new Vector2(padding, padding + HUDOverlay.TopScoringElementsHeight);
-                padding,
-                padding + HUDOverlay.TopScoringElementsHeight);
         }
 
         private void onMatchStarted() => Scheduler.Add(() =>

osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/MultiSpectatorLeaderboard.cs

@@ -19,7 +19,7 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Spectate
         public void AddClock(int userId, IClock clock)
         {
             if (!UserScores.TryGetValue(userId, out var data))
-                return;
+                throw new ArgumentException(@"Provided user is not tracked by this leaderboard", nameof(userId));
 
             ((SpectatingTrackedUserData)data).Clock = clock;
         }
@@ -27,7 +27,7 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Spectate
         public void RemoveClock(int userId)
         {
             if (!UserScores.TryGetValue(userId, out var data))
-                return;
+                throw new ArgumentException(@"Provided user is not tracked by this leaderboard", nameof(userId));
 
             ((SpectatingTrackedUserData)data).Clock = null;
         }

osu.Game/Screens/Play/HUD/MultiplayerGameplayLeaderboard.cs

@@ -55,20 +55,27 @@ namespace osu.Game.Screens.Play.HUD
 
             foreach (var userId in playingUsers)
             {
-                // probably won't be required in the final implementation.
-                var resolvedUser = userLookupCache.GetUserAsync(userId).Result;
-
                 var trackedUser = CreateUserData(userId, scoreProcessor);
                 trackedUser.ScoringMode.BindTo(scoringMode);
-
-                var leaderboardScore = AddPlayer(resolvedUser, resolvedUser?.Id == api.LocalUser.Value.Id);
-                leaderboardScore.Accuracy.BindTo(trackedUser.Accuracy);
-                leaderboardScore.TotalScore.BindTo(trackedUser.Score);
-                leaderboardScore.Combo.BindTo(trackedUser.CurrentCombo);
-                leaderboardScore.HasQuit.BindTo(trackedUser.UserQuit);
-
                 UserScores[userId] = trackedUser;
             }
+
+            userLookupCache.GetUsersAsync(playingUsers.ToArray()).ContinueWith(users => Schedule(() =>
+            {
+                foreach (var user in users.Result)
+                {
+                    if (user == null)
+                        continue;
+
+                    var trackedUser = UserScores[user.Id];
+
+                    var leaderboardScore = AddPlayer(user, user.Id == api.LocalUser.Value.Id);
+                    leaderboardScore.Accuracy.BindTo(trackedUser.Accuracy);
+                    leaderboardScore.TotalScore.BindTo(trackedUser.Score);
+                    leaderboardScore.Combo.BindTo(trackedUser.CurrentCombo);
+                    leaderboardScore.HasQuit.BindTo(trackedUser.UserQuit);
+                }
+            }));
         }
 
         protected override void LoadComplete()
@@ -84,6 +91,8 @@ namespace osu.Game.Screens.Play.HUD
                     usersChanged(this, new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Remove, new[] { userId }));
             }
 
+            // bind here is to support players leaving the match.
+            // new players are not supported.
             playingUsers.BindTo(multiplayerClient.CurrentMatchPlayingUserIds);
             playingUsers.BindCollectionChanged(usersChanged);
 

osu.Game/Screens/Spectate/SpectatorScreen.cs

@@ -5,7 +5,6 @@ using System;
 using System.Collections.Generic;
 using System.Diagnostics;
 using System.Linq;
-using System.Threading.Tasks;
 using JetBrains.Annotations;
 using osu.Framework.Allocation;
 using osu.Framework.Bindables;
@@ -61,10 +60,15 @@ namespace osu.Game.Screens.Spectate
         {
             base.LoadComplete();
 
-            getAllUsers().ContinueWith(users => Schedule(() =>
+            userLookupCache.GetUsersAsync(userIds.ToArray()).ContinueWith(users => Schedule(() =>
             {
                 foreach (var u in users.Result)
+                {
+                    if (u == null)
+                        continue;
+
                     userMap[u.Id] = u;
+                }
 
                 playingUserStates.BindTo(spectatorClient.PlayingUserStates);
                 playingUserStates.BindCollectionChanged(onPlayingUserStatesChanged, true);
@@ -77,24 +81,6 @@ namespace osu.Game.Screens.Spectate
             }));
         }
 
-        private Task<User[]> getAllUsers()
-        {
-            var userLookupTasks = new List<Task<User>>();
-
-            foreach (var u in userIds)
-            {
-                userLookupTasks.Add(userLookupCache.GetUserAsync(u).ContinueWith(task =>
-                {
-                    if (!task.IsCompletedSuccessfully)
-                        return null;
-
-                    return task.Result;
-                }));
-            }
-
-            return Task.WhenAll(userLookupTasks);
-        }
-
         private void beatmapUpdated(ValueChangedEvent<WeakReference<BeatmapSetInfo>> e)
         {
             if (!e.NewValue.TryGetTarget(out var beatmapSet))