Improve implementation of TestSpectatorClient

There was a lot of weirdness here, such as storing the playing users,
clearing the playing users from test scenes (!!), and storing the users
being wathed.
This was all a thing because the previous implementation overrode the
base method implementations, which is no longer a thing.

osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs

@@ -214,7 +214,7 @@ namespace osu.Game.Tests.Visual.Gameplay
 
         private void start(int? beatmapId = null) => AddStep("start play", () => testSpectatorClient.StartPlay(streamingUser.Id, beatmapId ?? importedBeatmapId));
 
-        private void finish(int? beatmapId = null) => AddStep("end play", () => testSpectatorClient.EndPlay(streamingUser.Id, beatmapId ?? importedBeatmapId));
+        private void finish() => AddStep("end play", () => testSpectatorClient.EndPlay(streamingUser.Id));
 
         private void checkPaused(bool state) =>
             AddUntilStep($"game is {(state ? "paused" : "playing")}", () => player.ChildrenOfType<DrawableRuleset>().First().IsPaused.Value == state);

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

@@ -59,7 +59,7 @@ namespace osu.Game.Tests.Visual.Multiplayer
 
                 foreach (var (userId, clock) in clocks)
                 {
-                    spectatorClient.EndPlay(userId, 0);
+                    spectatorClient.EndPlay(userId);
                     clock.CurrentTime = 0;
                 }
             });

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

@@ -66,7 +66,7 @@ namespace osu.Game.Tests.Visual.Multiplayer
             AddStep("finish previous gameplay", () =>
             {
                 foreach (var id in playingUserIds)
-                    spectatorClient.EndPlay(id, importedBeatmapId);
+                    spectatorClient.EndPlay(id);
                 playingUserIds.Clear();
             });
         }
@@ -258,11 +258,11 @@ namespace osu.Game.Tests.Visual.Multiplayer
             });
         }
 
-        private void finish(int userId, int? beatmapId = null)
+        private void finish(int userId)
         {
             AddStep("end play", () =>
             {
-                spectatorClient.EndPlay(userId, beatmapId ?? importedBeatmapId);
+                spectatorClient.EndPlay(userId);
                 playingUserIds.Remove(userId);
                 nextFrame.Remove(userId);
             });

osu.Game.Tests/Visual/Online/TestSceneCurrentlyPlayingDisplay.cs

@@ -19,6 +19,8 @@ namespace osu.Game.Tests.Visual.Online
 {
     public class TestSceneCurrentlyPlayingDisplay : OsuTestScene
     {
+        private readonly User streamingUser = new User { Id = 2, Username = "Test user" };
+
         [Cached(typeof(SpectatorClient))]
         private TestSpectatorClient testSpectatorClient = new TestSpectatorClient();
 
@@ -55,15 +57,15 @@ namespace osu.Game.Tests.Visual.Online
                 };
             });
 
-            AddStep("Reset players", () => testSpectatorClient.PlayingUsers.Clear());
+            AddStep("Reset players", () => testSpectatorClient.EndPlay(streamingUser.Id));
         }
 
         [Test]
         public void TestBasicDisplay()
         {
-            AddStep("Add playing user", () => testSpectatorClient.PlayingUsers.Add(2));
+            AddStep("Add playing user", () => testSpectatorClient.StartPlay(streamingUser.Id, 0));
             AddUntilStep("Panel loaded", () => currentlyPlaying.ChildrenOfType<UserGridPanel>()?.FirstOrDefault()?.User.Id == 2);
-            AddStep("Remove playing user", () => testSpectatorClient.PlayingUsers.Remove(2));
+            AddStep("Remove playing user", () => testSpectatorClient.EndPlay(streamingUser.Id));
             AddUntilStep("Panel no longer present", () => !currentlyPlaying.ChildrenOfType<UserGridPanel>().Any());
         }
 

osu.Game/Tests/Visual/Spectator/TestSpectatorClient.cs

@@ -3,8 +3,9 @@
 
 #nullable enable
 
-using System.Collections.Concurrent;
 using System.Collections.Generic;
+using System.Diagnostics;
+using System.Linq;
 using System.Threading.Tasks;
 using osu.Framework.Allocation;
 using osu.Framework.Bindables;
@@ -20,33 +21,44 @@ namespace osu.Game.Tests.Visual.Spectator
     {
         public override IBindable<bool> IsConnected { get; } = new Bindable<bool>(true);
 
-        public new BindableList<int> PlayingUsers => (BindableList<int>)base.PlayingUsers;
-        private readonly ConcurrentDictionary<int, byte> watchingUsers = new ConcurrentDictionary<int, byte>();
-
         private readonly Dictionary<int, int> userBeatmapDictionary = new Dictionary<int, int>();
-        private readonly Dictionary<int, bool> userSentStateDictionary = new Dictionary<int, bool>();
 
         [Resolved]
         private IAPIProvider api { get; set; } = null!;
 
+        /// <summary>
+        /// Starts play for an arbitrary user.
+        /// </summary>
+        /// <param name="userId">The user to start play for.</param>
+        /// <param name="beatmapId">The playing beatmap id.</param>
         public void StartPlay(int userId, int beatmapId)
         {
             userBeatmapDictionary[userId] = beatmapId;
-            sendState(userId, beatmapId);
+            sendPlayingState(userId);
         }
 
-        public void EndPlay(int userId, int beatmapId)
+        /// <summary>
+        /// Ends play for an arbitrary user.
+        /// </summary>
+        /// <param name="userId">The user to end play for.</param>
+        public void EndPlay(int userId)
         {
+            if (!PlayingUsers.Contains(userId))
+                return;
+
             ((ISpectatorClient)this).UserFinishedPlaying(userId, new SpectatorState
             {
-                BeatmapID = beatmapId,
+                BeatmapID = userBeatmapDictionary[userId],
                 RulesetID = 0,
             });
-
-            userBeatmapDictionary.Remove(userId);
-            userSentStateDictionary.Remove(userId);
         }
 
+        /// <summary>
+        /// Sends frames for an arbitrary user.
+        /// </summary>
+        /// <param name="userId">The user to send frames for.</param>
+        /// <param name="index">The frame index.</param>
+        /// <param name="count">The number of frames to send.</param>
         public void SendFrames(int userId, int index, int count)
         {
             var frames = new List<LegacyReplayFrame>();
@@ -60,12 +72,16 @@ namespace osu.Game.Tests.Visual.Spectator
 
             var bundle = new FrameDataBundle(new ScoreInfo { Combo = index + count }, frames);
             ((ISpectatorClient)this).UserSentFrames(userId, bundle);
-
-            if (!userSentStateDictionary[userId])
-                sendState(userId, userBeatmapDictionary[userId]);
         }
 
-        protected override Task BeginPlayingInternal(SpectatorState state) => ((ISpectatorClient)this).UserBeganPlaying(api.LocalUser.Value.Id, state);
+        protected override Task BeginPlayingInternal(SpectatorState state)
+        {
+            // Track the local user's playing beatmap ID.
+            Debug.Assert(state.BeatmapID != null);
+            userBeatmapDictionary[api.LocalUser.Value.Id] = state.BeatmapID.Value;
+
+            return ((ISpectatorClient)this).UserBeganPlaying(api.LocalUser.Value.Id, state);
+        }
 
         protected override Task SendFramesInternal(FrameDataBundle data) => ((ISpectatorClient)this).UserSentFrames(api.LocalUser.Value.Id, data);
 
@@ -74,27 +90,21 @@ namespace osu.Game.Tests.Visual.Spectator
         protected override Task WatchUserInternal(int userId)
         {
             // When newly watching a user, the server sends the playing state immediately.
-            if (watchingUsers.TryAdd(userId, 0) && PlayingUsers.Contains(userId))
+            if (PlayingUsers.Contains(userId))
-                sendState(userId, userBeatmapDictionary[userId]);
+                sendPlayingState(userId);
 
             return Task.CompletedTask;
         }
 
-        protected override Task StopWatchingUserInternal(int userId)
+        protected override Task StopWatchingUserInternal(int userId) => Task.CompletedTask;
-        {
-            watchingUsers.TryRemove(userId, out _);
-            return Task.CompletedTask;
-        }
 
-        private void sendState(int userId, int beatmapId)
+        private void sendPlayingState(int userId)
         {
             ((ISpectatorClient)this).UserBeganPlaying(userId, new SpectatorState
             {
-                BeatmapID = beatmapId,
+                BeatmapID = userBeatmapDictionary[userId],
                 RulesetID = 0,
             });
-
-            userSentStateDictionary[userId] = true;
         }
     }
 }