Merge pull request #14488 from frenzibyte/multi-spectator-player-leaving

Gray out and stop player instances who quit during multi-spectator sessions

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

@@ -5,6 +5,7 @@ using System.Collections.Generic;
 using System.Linq;
 using NUnit.Framework;
 using osu.Framework.Allocation;
+using osu.Framework.Extensions.ObjectExtensions;
 using osu.Framework.Graphics;
 using osu.Framework.Graphics.Containers;
 using osu.Framework.Testing;
@@ -19,6 +20,7 @@ using osu.Game.Screens.Play.HUD;
 using osu.Game.Screens.Play.PlayerSettings;
 using osu.Game.Tests.Beatmaps.IO;
 using osu.Game.Users;
+using osuTK.Graphics;
 
 namespace osu.Game.Tests.Visual.Multiplayer
 {
@@ -78,7 +80,7 @@ namespace osu.Game.Tests.Visual.Multiplayer
         [Test]
         public void TestGeneral()
         {
-            int[] userIds = Enumerable.Range(0, 4).Select(i => PLAYER_1_ID + i).ToArray();
+            int[] userIds = getPlayerIds(4);
 
             start(userIds);
             loadSpectateScreen();
@@ -314,6 +316,36 @@ namespace osu.Game.Tests.Visual.Multiplayer
             AddUntilStep("player 2 playing from correct point in time", () => getPlayer(PLAYER_2_ID).ChildrenOfType<DrawableRuleset>().Single().FrameStableClock.CurrentTime > 30000);
         }
 
+        [Test]
+        public void TestPlayersLeaveWhileSpectating()
+        {
+            start(getPlayerIds(4));
+            sendFrames(getPlayerIds(4), 300);
+
+            loadSpectateScreen();
+
+            for (int count = 3; count >= 0; count--)
+            {
+                var id = PLAYER_1_ID + count;
+
+                end(id);
+                AddUntilStep($"{id} area grayed", () => getInstance(id).Colour != Color4.White);
+                AddUntilStep($"{id} score quit set", () => getLeaderboardScore(id).HasQuit.Value);
+                sendFrames(getPlayerIds(count), 300);
+            }
+
+            Player player = null;
+
+            AddStep($"get {PLAYER_1_ID} player instance", () => player = getInstance(PLAYER_1_ID).ChildrenOfType<Player>().Single());
+
+            start(new[] { PLAYER_1_ID });
+            sendFrames(PLAYER_1_ID, 300);
+
+            AddAssert($"{PLAYER_1_ID} player instance still same", () => getInstance(PLAYER_1_ID).ChildrenOfType<Player>().Single() == player);
+            AddAssert($"{PLAYER_1_ID} area still grayed", () => getInstance(PLAYER_1_ID).Colour != Color4.White);
+            AddAssert($"{PLAYER_1_ID} score quit still set", () => getLeaderboardScore(PLAYER_1_ID).HasQuit.Value);
+        }
+
         private void loadSpectateScreen(bool waitForPlayerLoad = true)
         {
             AddStep("load screen", () =>
@@ -333,14 +365,32 @@ namespace osu.Game.Tests.Visual.Multiplayer
             {
                 foreach (int id in userIds)
                 {
-                    OnlinePlayDependencies.Client.AddUser(new User { Id = id }, true);
+                    var user = new MultiplayerRoomUser(id)
+                    {
+                        User = new User { Id = id },
+                    };
 
+                    OnlinePlayDependencies.Client.AddUser(user.User, true);
                     SpectatorClient.StartPlay(id, beatmapId ?? importedBeatmapId);
-                    playingUsers.Add(new MultiplayerRoomUser(id));
+
+                    playingUsers.Add(user);
                 }
             });
         }
 
+        private void end(int userId)
+        {
+            AddStep($"end play for {userId}", () =>
+            {
+                var user = playingUsers.Single(u => u.UserID == userId);
+
+                OnlinePlayDependencies.Client.RemoveUser(user.User.AsNonNull());
+                SpectatorClient.EndPlay(userId);
+
+                playingUsers.Remove(user);
+            });
+        }
+
         private void sendFrames(int userId, int count = 10) => sendFrames(new[] { userId }, count);
 
         private void sendFrames(int[] userIds, int count = 10)
@@ -374,5 +424,9 @@ namespace osu.Game.Tests.Visual.Multiplayer
         private Player getPlayer(int userId) => getInstance(userId).ChildrenOfType<Player>().Single();
 
         private PlayerArea getInstance(int userId) => spectatorScreen.ChildrenOfType<PlayerArea>().Single(p => p.UserId == userId);
+
+        private GameplayLeaderboardScore getLeaderboardScore(int userId) => spectatorScreen.ChildrenOfType<GameplayLeaderboardScore>().Single(s => s.User?.Id == userId);
+
+        private int[] getPlayerIds(int count) => Enumerable.Range(PLAYER_1_ID, count).ToArray();
     }
 }

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

@@ -61,7 +61,11 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Spectate
             playerClocks.Add(clock);
         }
 
-        public void RemovePlayerClock(ISpectatorPlayerClock clock) => playerClocks.Remove(clock);
+        public void RemovePlayerClock(ISpectatorPlayerClock clock)
+        {
+            playerClocks.Remove(clock);
+            clock.Stop();
+        }
 
         protected override void Update()
         {

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

@@ -8,6 +8,7 @@ using osu.Framework.Allocation;
 using osu.Framework.Bindables;
 using osu.Framework.Graphics;
 using osu.Framework.Graphics.Containers;
+using osu.Game.Graphics;
 using osu.Game.Online.Multiplayer;
 using osu.Game.Online.Spectator;
 using osu.Game.Screens.Play;
@@ -32,6 +33,9 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Spectate
         /// </summary>
         public bool AllPlayersLoaded => instances.All(p => p?.PlayerLoaded == true);
 
+        [Resolved]
+        private OsuColour colours { get; set; }
+
         [Resolved]
         private SpectatorClient spectatorClient { get; set; }
 
@@ -215,6 +219,11 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Spectate
         protected override void EndGameplay(int userId)
         {
             RemoveUser(userId);
+
+            var instance = instances.Single(i => i.UserId == userId);
+
+            instance.FadeColour(colours.Gray4, 400, Easing.OutQuint);
+            syncManager.RemovePlayerClock(instance.GameplayClock);
             leaderboard.RemoveClock(userId);
         }
 

osu.Game/Screens/Play/SoloSpectator.cs

@@ -211,7 +211,7 @@ namespace osu.Game.Screens.Play
                 Beatmap.Value = gameplayState.Beatmap;
                 Ruleset.Value = gameplayState.Ruleset.RulesetInfo;
 
-                this.Push(new SpectatorPlayerLoader(gameplayState.Score));
+                this.Push(new SpectatorPlayerLoader(gameplayState.Score, () => new SoloSpectatorPlayer(gameplayState.Score)));
             }
         }
 

osu.Game/Screens/Play/SoloSpectatorPlayer.cs

@@ -0,0 +1,52 @@
+// 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.Screens;
+using osu.Game.Online.Spectator;
+using osu.Game.Scoring;
+
+namespace osu.Game.Screens.Play
+{
+    public class SoloSpectatorPlayer : SpectatorPlayer
+    {
+        private readonly Score score;
+
+        public SoloSpectatorPlayer(Score score, PlayerConfiguration configuration = null)
+            : base(score, configuration)
+        {
+            this.score = score;
+        }
+
+        [BackgroundDependencyLoader]
+        private void load()
+        {
+            SpectatorClient.OnUserBeganPlaying += userBeganPlaying;
+        }
+
+        public override bool OnExiting(IScreen next)
+        {
+            SpectatorClient.OnUserBeganPlaying -= userBeganPlaying;
+
+            return base.OnExiting(next);
+        }
+
+        private void userBeganPlaying(int userId, SpectatorState state)
+        {
+            if (userId != score.ScoreInfo.UserID) return;
+
+            Schedule(() =>
+            {
+                if (this.IsCurrentScreen()) this.Exit();
+            });
+        }
+
+        protected override void Dispose(bool isDisposing)
+        {
+            base.Dispose(isDisposing);
+
+            if (SpectatorClient != null)
+                SpectatorClient.OnUserBeganPlaying -= userBeganPlaying;
+        }
+    }
+}

osu.Game/Screens/Play/SpectatorPlayer.cs

@@ -14,16 +14,16 @@ using osu.Game.Screens.Ranking;
 
 namespace osu.Game.Screens.Play
 {
-    public class SpectatorPlayer : Player
+    public abstract class SpectatorPlayer : Player
     {
         [Resolved]
-        private SpectatorClient spectatorClient { get; set; }
+        protected SpectatorClient SpectatorClient { get; private set; }
 
         private readonly Score score;
 
         protected override bool CheckModsAllowFailure() => false; // todo: better support starting mid-way through beatmap
 
-        public SpectatorPlayer(Score score, PlayerConfiguration configuration = null)
+        protected SpectatorPlayer(Score score, PlayerConfiguration configuration = null)
             : base(configuration)
         {
             this.score = score;
@@ -32,8 +32,6 @@ namespace osu.Game.Screens.Play
         [BackgroundDependencyLoader]
         private void load()
         {
-            spectatorClient.OnUserBeganPlaying += userBeganPlaying;
-
             AddInternal(new OsuSpriteText
             {
                 Text = $"Watching {score.ScoreInfo.User.Username} playing live!",
@@ -50,7 +48,7 @@ namespace osu.Game.Screens.Play
 
             // Start gameplay along with the very first arrival frame (the latest one).
             score.Replay.Frames.Clear();
-            spectatorClient.OnNewFrames += userSentFrames;
+            SpectatorClient.OnNewFrames += userSentFrames;
         }
 
         private void userSentFrames(int userId, FrameDataBundle bundle)
@@ -93,31 +91,17 @@ namespace osu.Game.Screens.Play
 
         public override bool OnExiting(IScreen next)
         {
-            spectatorClient.OnUserBeganPlaying -= userBeganPlaying;
+            SpectatorClient.OnNewFrames -= userSentFrames;
-            spectatorClient.OnNewFrames -= userSentFrames;
 
             return base.OnExiting(next);
         }
 
-        private void userBeganPlaying(int userId, SpectatorState state)
-        {
-            if (userId != score.ScoreInfo.UserID) return;
-
-            Schedule(() =>
-            {
-                if (this.IsCurrentScreen()) this.Exit();
-            });
-        }
-
         protected override void Dispose(bool isDisposing)
         {
             base.Dispose(isDisposing);
 
-            if (spectatorClient != null)
+            if (SpectatorClient != null)
-            {
+                SpectatorClient.OnNewFrames -= userSentFrames;
-                spectatorClient.OnUserBeganPlaying -= userBeganPlaying;
-                spectatorClient.OnNewFrames -= userSentFrames;
-            }
         }
     }
 }

osu.Game/Screens/Play/SpectatorPlayerLoader.cs

@@ -11,12 +11,7 @@ namespace osu.Game.Screens.Play
     {
         public readonly ScoreInfo Score;
 
-        public SpectatorPlayerLoader(Score score)
+        public SpectatorPlayerLoader(Score score, Func<SpectatorPlayer> createPlayer)
-            : this(score, () => new SpectatorPlayer(score))
-        {
-        }
-
-        public SpectatorPlayerLoader(Score score, Func<Player> createPlayer)
             : base(createPlayer)
         {
             if (score.Replay == null)