Merge pull request #14338 from frenzibyte/fix-participant-panel-assuming-not-null

Fix few cases where `MultiplayerRoomUser.User` is assumed to not be null

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

@@ -4,6 +4,7 @@
 using System.Collections.Generic;
 using System.Linq;
 using NUnit.Framework;
+using osu.Framework.Extensions.ObjectExtensions;
 using osu.Framework.Graphics;
 using osu.Framework.Graphics.Sprites;
 using osu.Framework.Testing;
@@ -48,9 +49,15 @@ namespace osu.Game.Tests.Visual.Multiplayer
         {
             AddAssert("one unique panel", () => this.ChildrenOfType<ParticipantPanel>().Select(p => p.User).Distinct().Count() == 1);
 
-            AddStep("add non-resolvable user", () => Client.AddNullUser(-3));
+            AddStep("add non-resolvable user", () => Client.AddNullUser());
+            AddAssert("null user added", () => Client.Room.AsNonNull().Users.Count(u => u.User == null) == 1);
 
             AddUntilStep("two unique panels", () => this.ChildrenOfType<ParticipantPanel>().Select(p => p.User).Distinct().Count() == 2);
+
+            AddStep("kick null user", () => this.ChildrenOfType<ParticipantPanel>().Single(p => p.User.User == null)
+                                                .ChildrenOfType<ParticipantPanel.KickButton>().Single().TriggerClick());
+
+            AddAssert("null user kicked", () => Client.Room.AsNonNull().Users.Count == 1);
         }
 
         [Test]

osu.Game/Screens/OnlinePlay/Multiplayer/Participants/ParticipantPanel.cs

@@ -2,7 +2,6 @@
 // See the LICENCE file in the repository root for full licence text.
 
 using System.Collections.Generic;
-using System.Diagnostics;
 using System.Linq;
 using osu.Framework.Allocation;
 using osu.Framework.Extensions.Color4Extensions;
@@ -83,7 +82,7 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Participants
                             Colour = Color4Extensions.FromHex("#F7E65D"),
                             Alpha = 0
                         },
-                        new TeamDisplay(user),
+                        new TeamDisplay(User),
                         new Container
                         {
                             RelativeSizeAxes = Axes.Both,
@@ -168,12 +167,7 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Participants
                             Origin = Anchor.Centre,
                             Alpha = 0,
                             Margin = new MarginPadding(4),
-                            Action = () =>
+                            Action = () => Client.KickUser(User.UserID),
-                            {
-                                Debug.Assert(user != null);
-
-                                Client.KickUser(user.Id);
-                            }
                         },
                     },
                 }

osu.Game/Screens/OnlinePlay/Multiplayer/Participants/TeamDisplay.cs

@@ -11,7 +11,6 @@ using osu.Game.Graphics;
 using osu.Game.Graphics.Containers;
 using osu.Game.Online.Multiplayer;
 using osu.Game.Online.Multiplayer.MatchTypes.TeamVersus;
-using osu.Game.Users;
 using osuTK;
 using osuTK.Graphics;
 
@@ -19,16 +18,14 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Participants
 {
     internal class TeamDisplay : MultiplayerRoomComposite
     {
-        private readonly User user;
+        private readonly MultiplayerRoomUser user;
+
         private Drawable box;
 
         [Resolved]
         private OsuColour colours { get; set; }
 
-        [Resolved]
+        public TeamDisplay(MultiplayerRoomUser user)
-        private MultiplayerClient client { get; set; }
-
-        public TeamDisplay(User user)
         {
             this.user = user;
 
@@ -61,7 +58,7 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Participants
                 }
             };
 
-            if (user.Id == client.LocalUser?.UserID)
+            if (Client.LocalUser?.Equals(user) == true)
             {
                 InternalChild = new OsuClickableContainer
                 {
@@ -79,9 +76,9 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Participants
 
         private void changeTeam()
         {
-            client.SendMatchRequest(new ChangeTeamRequest
+            Client.SendMatchRequest(new ChangeTeamRequest
             {
-                TeamID = ((client.LocalUser?.MatchState as TeamVersusUserState)?.TeamID + 1) % 2 ?? 0,
+                TeamID = ((Client.LocalUser?.MatchState as TeamVersusUserState)?.TeamID + 1) % 2 ?? 0,
             });
         }
 
@@ -93,7 +90,7 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Participants
 
             // we don't have a way of knowing when an individual user's state has updated, so just handle on RoomUpdated for now.
 
-            var userRoomState = Room?.Users.FirstOrDefault(u => u.UserID == user.Id)?.MatchState;
+            var userRoomState = Room?.Users.FirstOrDefault(u => u.Equals(user))?.MatchState;
 
             const double duration = 400;
 

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

@@ -61,7 +61,7 @@ namespace osu.Game.Tests.Visual.Multiplayer
             return roomUser;
         }
 
-        public void AddNullUser(int userId) => ((IMultiplayerClient)this).UserJoined(new MultiplayerRoomUser(userId));
+        public void AddNullUser() => ((IMultiplayerClient)this).UserJoined(new MultiplayerRoomUser(TestUserLookupCache.NULL_USER_ID));
 
         public void RemoveUser(User user)
         {

osu.Game/Tests/Visual/TestUserLookupCache.cs

@@ -10,10 +10,22 @@ namespace osu.Game.Tests.Visual
 {
     public class TestUserLookupCache : UserLookupCache
     {
-        protected override Task<User> ComputeValueAsync(int lookup, CancellationToken token = default) => Task.FromResult(new User
+        /// <summary>
+        /// A special user ID which <see cref="ComputeValueAsync"/> would return a <see langword="null"/> <see cref="User"/> for.
+        /// As a simulation to what a regular <see cref="UserLookupCache"/> would return in the case of failing to fetch the user.
+        /// </summary>
+        public const int NULL_USER_ID = -1;
+
+        protected override Task<User> ComputeValueAsync(int lookup, CancellationToken token = default)
         {
-            Id = lookup,
+            if (lookup == NULL_USER_ID)
-            Username = $"User {lookup}"
+                return Task.FromResult((User)null);
-        });
+
+            return Task.FromResult(new User
+            {
+                Id = lookup,
+                Username = $"User {lookup}"
+            });
+        }
     }
 }