Merge branch 'master' into fix-quit-user-showing-in-leaderboard

osu.Game/Online/Leaderboards/LeaderboardScore.cs

@@ -78,7 +78,7 @@ namespace osu.Game.Online.Leaderboards
 
             statisticsLabels = GetStatistics(score).Select(s => new ScoreComponentLabel(s)).ToList();
 
-            DrawableAvatar innerAvatar;
+            ClickableAvatar innerAvatar;
 
             Children = new Drawable[]
             {
@@ -115,7 +115,7 @@ namespace osu.Game.Online.Leaderboards
                             Children = new[]
                             {
                                 avatar = new DelayedLoadWrapper(
-                                    innerAvatar = new DrawableAvatar(user)
+                                    innerAvatar = new ClickableAvatar(user)
                                     {
                                         RelativeSizeAxes = Axes.Both,
                                         CornerRadius = corner_radius,

osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs

@@ -4,8 +4,10 @@
 #nullable enable
 
 using System;
+using System.Collections.Generic;
 using System.Diagnostics;
 using System.Linq;
+using System.Threading;
 using System.Threading.Tasks;
 using osu.Framework.Allocation;
 using osu.Framework.Bindables;
@@ -31,7 +33,7 @@ namespace osu.Game.Online.Multiplayer
         /// <summary>
         /// Invoked when any change occurs to the multiplayer room.
         /// </summary>
-        public event Action? RoomChanged;
+        public event Action? RoomUpdated;
 
         /// <summary>
         /// Invoked when the multiplayer server requests the current beatmap to be loaded into play.
@@ -108,8 +110,9 @@ namespace osu.Game.Online.Multiplayer
 
             Debug.Assert(Room != null);
 
-            foreach (var user in Room.Users)
+            var users = getRoomUsers();
-                await PopulateUser(user);
+
+            await Task.WhenAll(users.Select(PopulateUser));
 
             updateLocalRoomSettings(Room.Settings);
         }
@@ -122,14 +125,17 @@ namespace osu.Game.Online.Multiplayer
         protected abstract Task<MultiplayerRoom> JoinRoom(long roomId);
 
         public virtual Task LeaveRoom()
+        {
+            Scheduler.Add(() =>
             {
                 if (Room == null)
-                return Task.CompletedTask;
+                    return;
 
                 apiRoom = null;
                 Room = null;
 
-            Schedule(() => RoomChanged?.Invoke());
+                RoomUpdated?.Invoke();
+            }, false);
 
             return Task.CompletedTask;
         }
@@ -184,7 +190,7 @@ namespace osu.Game.Online.Multiplayer
             if (Room == null)
                 return Task.CompletedTask;
 
-            Schedule(() =>
+            Scheduler.Add(() =>
             {
                 if (Room == null)
                     return;
@@ -208,8 +214,8 @@ namespace osu.Game.Online.Multiplayer
                         break;
                 }
 
-                RoomChanged?.Invoke();
+                RoomUpdated?.Invoke();
-            });
+            }, false);
 
             return Task.CompletedTask;
         }
@@ -221,7 +227,7 @@ namespace osu.Game.Online.Multiplayer
 
             await PopulateUser(user);
 
-            Schedule(() =>
+            Scheduler.Add(() =>
             {
                 if (Room == null)
                     return;
@@ -232,8 +238,8 @@ namespace osu.Game.Online.Multiplayer
 
                 Room.Users.Add(user);
 
-                RoomChanged?.Invoke();
+                RoomUpdated?.Invoke();
-            });
+            }, false);
         }
 
         Task IMultiplayerClient.UserLeft(MultiplayerRoomUser user)
@@ -241,7 +247,7 @@ namespace osu.Game.Online.Multiplayer
             if (Room == null)
                 return Task.CompletedTask;
 
-            Schedule(() =>
+            Scheduler.Add(() =>
             {
                 if (Room == null)
                     return;
@@ -249,8 +255,8 @@ namespace osu.Game.Online.Multiplayer
                 Room.Users.Remove(user);
                 PlayingUsers.Remove(user.UserID);
 
-                RoomChanged?.Invoke();
+                RoomUpdated?.Invoke();
-            });
+            }, false);
 
             return Task.CompletedTask;
         }
@@ -260,7 +266,7 @@ namespace osu.Game.Online.Multiplayer
             if (Room == null)
                 return Task.CompletedTask;
 
-            Schedule(() =>
+            Scheduler.Add(() =>
             {
                 if (Room == null)
                     return;
@@ -272,8 +278,8 @@ namespace osu.Game.Online.Multiplayer
                 Room.Host = user;
                 apiRoom.Host.Value = user?.User;
 
-                RoomChanged?.Invoke();
+                RoomUpdated?.Invoke();
-            });
+            }, false);
 
             return Task.CompletedTask;
         }
@@ -289,7 +295,7 @@ namespace osu.Game.Online.Multiplayer
             if (Room == null)
                 return Task.CompletedTask;
 
-            Schedule(() =>
+            Scheduler.Add(() =>
             {
                 if (Room == null)
                     return;
@@ -299,8 +305,8 @@ namespace osu.Game.Online.Multiplayer
                 if (state != MultiplayerUserState.Playing)
                     PlayingUsers.Remove(userId);
 
-                RoomChanged?.Invoke();
+                RoomUpdated?.Invoke();
-            });
+            }, false);
 
             return Task.CompletedTask;
         }
@@ -310,13 +316,13 @@ namespace osu.Game.Online.Multiplayer
             if (Room == null)
                 return Task.CompletedTask;
 
-            Schedule(() =>
+            Scheduler.Add(() =>
             {
                 if (Room == null)
                     return;
 
                 LoadRequested?.Invoke();
-            });
+            }, false);
 
             return Task.CompletedTask;
         }
@@ -326,7 +332,7 @@ namespace osu.Game.Online.Multiplayer
             if (Room == null)
                 return Task.CompletedTask;
 
-            Schedule(() =>
+            Scheduler.Add(() =>
             {
                 if (Room == null)
                     return;
@@ -334,7 +340,7 @@ namespace osu.Game.Online.Multiplayer
                 PlayingUsers.AddRange(Room.Users.Where(u => u.State == MultiplayerUserState.Playing).Select(u => u.UserID));
 
                 MatchStarted?.Invoke();
-            });
+            }, false);
 
             return Task.CompletedTask;
         }
@@ -344,13 +350,13 @@ namespace osu.Game.Online.Multiplayer
             if (Room == null)
                 return Task.CompletedTask;
 
-            Schedule(() =>
+            Scheduler.Add(() =>
             {
                 if (Room == null)
                     return;
 
                 ResultsReady?.Invoke();
-            });
+            }, false);
 
             return Task.CompletedTask;
         }
@@ -361,6 +367,31 @@ namespace osu.Game.Online.Multiplayer
         /// <param name="multiplayerUser">The <see cref="MultiplayerRoomUser"/> to populate.</param>
         protected async Task PopulateUser(MultiplayerRoomUser multiplayerUser) => multiplayerUser.User ??= await userLookupCache.GetUserAsync(multiplayerUser.UserID);
 
+        /// <summary>
+        /// Retrieve a copy of users currently in the joined <see cref="Room"/> in a thread-safe manner.
+        /// This should be used whenever accessing users from outside of an Update thread context (ie. when not calling <see cref="Drawable.Schedule"/>).
+        /// </summary>
+        /// <returns>A copy of users in the current room, or null if unavailable.</returns>
+        private List<MultiplayerRoomUser>? getRoomUsers()
+        {
+            List<MultiplayerRoomUser>? users = null;
+
+            ManualResetEventSlim resetEvent = new ManualResetEventSlim();
+
+            // at some point we probably want to replace all these schedule calls with Room.LockForUpdate.
+            // for now, as this would require quite some consideration due to the number of accesses to the room instance,
+            // let's just add a manual schedule for the non-scheduled usages instead.
+            Scheduler.Add(() =>
+            {
+                users = Room?.Users.ToList();
+                resetEvent.Set();
+            }, false);
+
+            resetEvent.Wait(100);
+
+            return users;
+        }
+
         /// <summary>
         /// Updates the local room settings with the given <see cref="MultiplayerRoomSettings"/>.
         /// </summary>
@@ -373,7 +404,7 @@ namespace osu.Game.Online.Multiplayer
             if (Room == null)
                 return;
 
-            Schedule(() =>
+            Scheduler.Add(() =>
             {
                 if (Room == null)
                     return;
@@ -388,13 +419,13 @@ namespace osu.Game.Online.Multiplayer
                 // In-order for the client to not display an outdated beatmap, the playlist is forcefully cleared here.
                 apiRoom.Playlist.Clear();
 
-                RoomChanged?.Invoke();
+                RoomUpdated?.Invoke();
 
                 var req = new GetBeatmapSetRequest(settings.BeatmapID, BeatmapSetLookupType.BeatmapId);
                 req.Success += res => updatePlaylist(settings, res);
 
                 api.Queue(req);
-            });
+            }, false);
         }
 
         private void updatePlaylist(MultiplayerRoomSettings settings, APIBeatmapSet onlineSet)

osu.Game/Online/Rooms/Room.cs

@@ -131,6 +131,9 @@ namespace osu.Game.Online.Rooms
             RoomID.Value = other.RoomID.Value;
             Name.Value = other.Name.Value;
 
+            if (other.Category.Value != RoomCategory.Spotlight)
+                Category.Value = other.Category.Value;
+
             if (other.Host.Value != null && Host.Value?.Id != other.Host.Value.Id)
                 Host.Value = other.Host.Value;
 

osu.Game/Overlays/Chat/Tabs/PrivateChannelTabItem.cs

@@ -25,7 +25,7 @@ namespace osu.Game.Overlays.Chat.Tabs
             if (value.Type != ChannelType.PM)
                 throw new ArgumentException("Argument value needs to have the targettype user!");
 
-            DrawableAvatar avatar;
+            ClickableAvatar avatar;
 
             AddRange(new Drawable[]
             {
@@ -48,7 +48,7 @@ namespace osu.Game.Overlays.Chat.Tabs
                             Anchor = Anchor.Centre,
                             Origin = Anchor.Centre,
                             Masking = true,
-                            Child = new DelayedLoadWrapper(avatar = new DrawableAvatar(value.Users.First())
+                            Child = new DelayedLoadWrapper(avatar = new ClickableAvatar(value.Users.First())
                             {
                                 RelativeSizeAxes = Axes.Both,
                                 OpenOnClick = { Value = false },

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

@@ -56,11 +56,13 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Match
             sampleReadyCount = audio.Samples.Get(@"SongSelect/select-difficulty");
         }
 
-        protected override void OnRoomChanged()
+        protected override void OnRoomUpdated()
         {
-            base.OnRoomChanged();
+            base.OnRoomUpdated();
+
+            // this method is called on leaving the room, so the local user may not exist in the room any more.
+            localUser = Room?.Users.SingleOrDefault(u => u.User?.Id == api.LocalUser.Value.Id);
 
-            localUser = Room?.Users.Single(u => u.User?.Id == api.LocalUser.Value.Id);
             button.Enabled.Value = Client.Room?.State == MultiplayerRoomState.Open;
             updateState();
         }

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

@@ -19,18 +19,21 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer
         {
             base.LoadComplete();
 
-            Client.RoomChanged += OnRoomChanged;
+            Client.RoomUpdated += OnRoomUpdated;
-            OnRoomChanged();
+            OnRoomUpdated();
         }
 
-        protected virtual void OnRoomChanged()
+        /// <summary>
+        /// Invoked when any change occurs to the multiplayer room.
+        /// </summary>
+        protected virtual void OnRoomUpdated()
         {
         }
 
         protected override void Dispose(bool isDisposing)
         {
             if (Client != null)
-                Client.RoomChanged -= OnRoomChanged;
+                Client.RoomUpdated -= OnRoomUpdated;
 
             base.Dispose(isDisposing);
         }

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

@@ -135,9 +135,9 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Participants
             };
         }
 
-        protected override void OnRoomChanged()
+        protected override void OnRoomUpdated()
         {
-            base.OnRoomChanged();
+            base.OnRoomUpdated();
 
             if (Room == null)
                 return;

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

@@ -36,9 +36,9 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Participants
             };
         }
 
-        protected override void OnRoomChanged()
+        protected override void OnRoomUpdated()
         {
-            base.OnRoomChanged();
+            base.OnRoomUpdated();
 
             if (Room == null)
                 panels.Clear();

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

@@ -79,6 +79,8 @@ namespace osu.Game.Screens.Play.HUD
         [BackgroundDependencyLoader]
         private void load(OsuColour colours)
         {
+            Container avatarContainer;
+
             InternalChildren = new Drawable[]
             {
                 mainFillContainer = new Container
@@ -153,7 +155,7 @@ namespace osu.Game.Screens.Play.HUD
                                         Spacing = new Vector2(4f, 0f),
                                         Children = new Drawable[]
                                         {
-                                            new CircularContainer
+                                            avatarContainer = new CircularContainer
                                             {
                                                 Masking = true,
                                                 Anchor = Anchor.CentreLeft,
@@ -167,11 +169,7 @@ namespace osu.Game.Screens.Play.HUD
                                                         Alpha = 0.3f,
                                                         RelativeSizeAxes = Axes.Both,
                                                         Colour = colours.Gray4,
-                                                    },
+                                                    }
-                                                    new UpdateableAvatar(User)
-                                                    {
-                                                        RelativeSizeAxes = Axes.Both,
-                                                    },
                                                 }
                                             },
                                             usernameText = new OsuSpriteText
@@ -228,6 +226,8 @@ namespace osu.Game.Screens.Play.HUD
                 }
             };
 
+            LoadComponentAsync(new DrawableAvatar(User), avatarContainer.Add);
+
             TotalScore.BindValueChanged(v => scoreText.Text = v.NewValue.ToString("N0"), true);
             Accuracy.BindValueChanged(v => accuracyText.Text = v.NewValue.FormatAccuracy(), true);
             Combo.BindValueChanged(v => comboText.Text = $"{v.NewValue}x", true);

osu.Game/Users/Drawables/ClickableAvatar.cs

@@ -0,0 +1,73 @@
+// 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.
+
+using osu.Framework.Allocation;
+using osu.Framework.Bindables;
+using osu.Framework.Graphics;
+using osu.Framework.Graphics.Containers;
+using osu.Framework.Graphics.Textures;
+using osu.Framework.Input.Events;
+using osu.Game.Graphics.Containers;
+
+namespace osu.Game.Users.Drawables
+{
+    public class ClickableAvatar : Container
+    {
+        /// <summary>
+        /// Whether to open the user's profile when clicked.
+        /// </summary>
+        public readonly BindableBool OpenOnClick = new BindableBool(true);
+
+        private readonly User user;
+
+        [Resolved(CanBeNull = true)]
+        private OsuGame game { get; set; }
+
+        /// <summary>
+        /// A clickable avatar for the specified user, with UI sounds included.
+        /// If <see cref="OpenOnClick"/> is <c>true</c>, clicking will open the user's profile.
+        /// </summary>
+        /// <param name="user">The user. A null value will get a placeholder avatar.</param>
+        public ClickableAvatar(User user = null)
+        {
+            this.user = user;
+        }
+
+        [BackgroundDependencyLoader]
+        private void load(LargeTextureStore textures)
+        {
+            ClickableArea clickableArea;
+            Add(clickableArea = new ClickableArea
+            {
+                RelativeSizeAxes = Axes.Both,
+                Action = openProfile
+            });
+
+            LoadComponentAsync(new DrawableAvatar(user), clickableArea.Add);
+
+            clickableArea.Enabled.BindTo(OpenOnClick);
+        }
+
+        private void openProfile()
+        {
+            if (!OpenOnClick.Value)
+                return;
+
+            if (user?.Id > 1)
+                game?.ShowUser(user.Id);
+        }
+
+        private class ClickableArea : OsuClickableContainer
+        {
+            public override string TooltipText => Enabled.Value ? @"view profile" : null;
+
+            protected override bool OnClick(ClickEvent e)
+            {
+                if (!Enabled.Value)
+                    return false;
+
+                return base.OnClick(e);
+            }
+        }
+    }
+}

osu.Game/Users/Drawables/DrawableAvatar.cs

@@ -1,88 +1,45 @@
-// 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.
 
-using System;
 using osu.Framework.Allocation;
-using osu.Framework.Bindables;
 using osu.Framework.Graphics;
-using osu.Framework.Graphics.Containers;
 using osu.Framework.Graphics.Sprites;
 using osu.Framework.Graphics.Textures;
-using osu.Framework.Input.Events;
-using osu.Game.Graphics.Containers;
 
 namespace osu.Game.Users.Drawables
 {
     [LongRunningLoad]
-    public class DrawableAvatar : Container
+    public class DrawableAvatar : Sprite
     {
-        /// <summary>
-        /// Whether to open the user's profile when clicked.
-        /// </summary>
-        public readonly BindableBool OpenOnClick = new BindableBool(true);
-
         private readonly User user;
 
-        [Resolved(CanBeNull = true)]
-        private OsuGame game { get; set; }
-
         /// <summary>
-        /// An avatar for specified user.
+        /// A simple, non-interactable avatar sprite for the specified user.
         /// </summary>
         /// <param name="user">The user. A null value will get a placeholder avatar.</param>
         public DrawableAvatar(User user = null)
         {
             this.user = user;
+
+            RelativeSizeAxes = Axes.Both;
+            FillMode = FillMode.Fit;
+            Anchor = Anchor.Centre;
+            Origin = Anchor.Centre;
         }
 
         [BackgroundDependencyLoader]
         private void load(LargeTextureStore textures)
         {
-            if (textures == null)
+            if (user != null && user.Id > 1)
-                throw new ArgumentNullException(nameof(textures));
+                Texture = textures.Get($@"https://a.ppy.sh/{user.Id}");
 
-            Texture texture = null;
+            Texture ??= textures.Get(@"Online/avatar-guest");
-            if (user != null && user.Id > 1) texture = textures.Get($@"https://a.ppy.sh/{user.Id}");
-            texture ??= textures.Get(@"Online/avatar-guest");
-
-            ClickableArea clickableArea;
-            Add(clickableArea = new ClickableArea
-            {
-                RelativeSizeAxes = Axes.Both,
-                Child = new Sprite
-                {
-                    RelativeSizeAxes = Axes.Both,
-                    Texture = texture,
-                    FillMode = FillMode.Fit,
-                    Anchor = Anchor.Centre,
-                    Origin = Anchor.Centre
-                },
-                Action = openProfile
-            });
-
-            clickableArea.Enabled.BindTo(OpenOnClick);
         }
 
-        private void openProfile()
+        protected override void LoadComplete()
         {
-            if (!OpenOnClick.Value)
+            base.LoadComplete();
-                return;
+            this.FadeInFromZero(300, Easing.OutQuint);
-
-            if (user?.Id > 1)
-                game?.ShowUser(user.Id);
-        }
-
-        private class ClickableArea : OsuClickableContainer
-        {
-            public override string TooltipText => Enabled.Value ? @"view profile" : null;
-
-            protected override bool OnClick(ClickEvent e)
-            {
-                if (!Enabled.Value)
-                    return false;
-
-                return base.OnClick(e);
-            }
         }
     }
 }

osu.Game/Users/Drawables/UpdateableAvatar.cs

@@ -65,12 +65,11 @@ namespace osu.Game.Users.Drawables
             if (user == null && !ShowGuestOnNull)
                 return null;
 
-            var avatar = new DrawableAvatar(user)
+            var avatar = new ClickableAvatar(user)
             {
                 RelativeSizeAxes = Axes.Both,
             };
 
-            avatar.OnLoadComplete += d => d.FadeInFromZero(300, Easing.OutQuint);
             avatar.OpenOnClick.BindTo(OpenOnClick);
 
             return avatar;