Merge pull request #19936 from peppy/catch-up-woes-2

Move `MasterClockState` handling in to `SpectatorSyncManager`

osu.Game.Tests/OnlinePlay/TestSceneCatchUpSyncManager.cs

@@ -146,12 +146,12 @@ namespace osu.Game.Tests.OnlinePlay
         }
 
         private void setWaiting(Func<SpectatorPlayerClock> playerClock, bool waiting)
-            => AddStep($"set player clock {clocksById[playerClock()]} waiting = {waiting}", () => playerClock().WaitingOnFrames.Value = waiting);
+            => AddStep($"set player clock {clocksById[playerClock()]} waiting = {waiting}", () => playerClock().WaitingOnFrames = waiting);
 
         private void setAllWaiting(bool waiting) => AddStep($"set all player clocks waiting = {waiting}", () =>
         {
-            player1.WaitingOnFrames.Value = waiting;
+            player1.WaitingOnFrames = waiting;
-            player2.WaitingOnFrames.Value = waiting;
+            player2.WaitingOnFrames = waiting;
         });
 
         private void setMasterTime(double time)

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

@@ -2,7 +2,6 @@
 // See the LICENCE file in the repository root for full licence text.
 
 using osu.Framework.Allocation;
-using osu.Framework.Bindables;
 using osu.Game.Beatmaps;
 using osu.Game.Scoring;
 using osu.Game.Screens.Play;
@@ -14,7 +13,6 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Spectate
     /// </summary>
     public class MultiSpectatorPlayer : SpectatorPlayer
     {
-        private readonly Bindable<bool> waitingOnFrames = new Bindable<bool>(true);
         private readonly SpectatorPlayerClock spectatorPlayerClock;
 
         /// <summary>
@@ -31,8 +29,6 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Spectate
         [BackgroundDependencyLoader]
         private void load()
         {
-            spectatorPlayerClock.WaitingOnFrames.BindTo(waitingOnFrames);
-
             HUDOverlay.PlayerSettingsOverlay.Expire();
             HUDOverlay.HoldToQuit.Expire();
         }
@@ -53,7 +49,7 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Spectate
             base.UpdateAfterChildren();
 
             // This is required because the frame stable clock is set to WaitingOnFrames = false for one frame.
-            waitingOnFrames.Value = DrawableRuleset.FrameStableClock.WaitingOnFrames.Value || Score.Replay.Frames.Count == 0;
+            spectatorPlayerClock.WaitingOnFrames = DrawableRuleset.FrameStableClock.WaitingOnFrames.Value || Score.Replay.Frames.Count == 0;
         }
 
         protected override GameplayClockContainer CreateGameplayClockContainer(WorkingBeatmap beatmap, double gameplayStart)

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

@@ -4,12 +4,9 @@
 using System;
 using System.Linq;
 using osu.Framework.Allocation;
-using osu.Framework.Bindables;
 using osu.Framework.Extensions.ObjectExtensions;
 using osu.Framework.Graphics;
 using osu.Framework.Graphics.Containers;
-using osu.Framework.Logging;
-using osu.Game.Beatmaps;
 using osu.Game.Graphics;
 using osu.Game.Online.Multiplayer;
 using osu.Game.Online.Rooms;
@@ -52,7 +49,6 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Spectate
         private PlayerGrid grid = null!;
         private MultiSpectatorLeaderboard leaderboard = null!;
         private PlayerArea? currentAudioSource;
-        private bool canStartMasterClock;
 
         private readonly Room room;
         private readonly MultiplayerRoomUser[] users;
@@ -77,50 +73,54 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Spectate
             FillFlowContainer leaderboardFlow;
             Container scoreDisplayContainer;
 
-            masterClockContainer = CreateMasterGameplayClockContainer(Beatmap.Value);
+            InternalChildren = new Drawable[]
-
-            InternalChildren = new[]
             {
-                (Drawable)(syncManager = new SpectatorSyncManager(masterClockContainer)),
+                masterClockContainer = new MasterGameplayClockContainer(Beatmap.Value, 0)
-                masterClockContainer.WithChild(new GridContainer
                 {
-                    RelativeSizeAxes = Axes.Both,
+                    Child = new GridContainer
-                    RowDimensions = new[] { new Dimension(GridSizeMode.AutoSize) },
-                    Content = new[]
                     {
-                        new Drawable[]
+                        RelativeSizeAxes = Axes.Both,
+                        RowDimensions = new[] { new Dimension(GridSizeMode.AutoSize) },
+                        Content = new[]
                         {
-                            scoreDisplayContainer = new Container
+                            new Drawable[]
                             {
-                                RelativeSizeAxes = Axes.X,
+                                scoreDisplayContainer = new Container
-                                AutoSizeAxes = Axes.Y
-                            },
-                        },
-                        new Drawable[]
-                        {
-                            new GridContainer
-                            {
-                                RelativeSizeAxes = Axes.Both,
-                                ColumnDimensions = new[] { new Dimension(GridSizeMode.AutoSize) },
-                                Content = new[]
                                 {
-                                    new Drawable[]
+                                    RelativeSizeAxes = Axes.X,
+                                    AutoSizeAxes = Axes.Y
+                                },
+                            },
+                            new Drawable[]
+                            {
+                                new GridContainer
+                                {
+                                    RelativeSizeAxes = Axes.Both,
+                                    ColumnDimensions = new[] { new Dimension(GridSizeMode.AutoSize) },
+                                    Content = new[]
                                     {
-                                        leaderboardFlow = new FillFlowContainer
+                                        new Drawable[]
                                         {
-                                            Anchor = Anchor.CentreLeft,
+                                            leaderboardFlow = new FillFlowContainer
-                                            Origin = Anchor.CentreLeft,
+                                            {
-                                            AutoSizeAxes = Axes.Both,
+                                                Anchor = Anchor.CentreLeft,
-                                            Direction = FillDirection.Vertical,
+                                                Origin = Anchor.CentreLeft,
-                                            Spacing = new Vector2(5)
+                                                AutoSizeAxes = Axes.Both,
-                                        },
+                                                Direction = FillDirection.Vertical,
-                                        grid = new PlayerGrid { RelativeSizeAxes = Axes.Both }
+                                                Spacing = new Vector2(5)
+                                            },
+                                            grid = new PlayerGrid { RelativeSizeAxes = Axes.Both }
+                                        }
                                     }
                                 }
                             }
                         }
                     }
-                })
+                },
+                syncManager = new SpectatorSyncManager(masterClockContainer)
+                {
+                    ReadyToStart = performInitialSeek,
+                }
             };
 
             for (int i = 0; i < Users.Count; i++)
@@ -157,9 +157,6 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Spectate
             base.LoadComplete();
 
             masterClockContainer.Reset();
-
-            syncManager.ReadyToStart += onReadyToStart;
-            syncManager.MasterState.BindValueChanged(onMasterStateChanged, true);
         }
 
         protected override void Update()
@@ -178,9 +175,9 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Spectate
         }
 
         private bool isCandidateAudioSource(SpectatorPlayerClock? clock)
-            => clock?.IsRunning == true && !clock.IsCatchingUp && !clock.WaitingOnFrames.Value;
+            => clock?.IsRunning == true && !clock.IsCatchingUp && !clock.WaitingOnFrames;
 
-        private void onReadyToStart()
+        private void performInitialSeek()
         {
             // Seek the master clock to the gameplay time.
             // This is chosen as the first available frame in the players' replays, which matches the seek by each individual SpectatorPlayer.
@@ -192,27 +189,6 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Spectate
 
             masterClockContainer.StartTime = startTime;
             masterClockContainer.Reset(true);
-
-            // Although the clock has been started, this flag is set to allow for later synchronisation state changes to also be able to start it.
-            canStartMasterClock = true;
-        }
-
-        private void onMasterStateChanged(ValueChangedEvent<MasterClockState> state)
-        {
-            Logger.Log($"{nameof(MultiSpectatorScreen)}'s master clock become {state.NewValue}");
-
-            switch (state.NewValue)
-            {
-                case MasterClockState.Synchronised:
-                    if (canStartMasterClock)
-                        masterClockContainer.Start();
-
-                    break;
-
-                case MasterClockState.TooFarAhead:
-                    masterClockContainer.Stop();
-                    break;
-            }
         }
 
         protected override void OnNewPlayingUserState(int userId, SpectatorState spectatorState)
@@ -254,7 +230,5 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Spectate
 
             return base.OnBackButton();
         }
-
-        protected virtual MasterGameplayClockContainer CreateMasterGameplayClockContainer(WorkingBeatmap beatmap) => new MasterGameplayClockContainer(beatmap, 0);
     }
 }

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

@@ -2,7 +2,6 @@
 // See the LICENCE file in the repository root for full licence text.
 
 using System;
-using osu.Framework.Bindables;
 using osu.Framework.Timing;
 using osu.Game.Screens.Play;
 
@@ -16,7 +15,7 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Spectate
         /// <summary>
         /// The catch up rate.
         /// </summary>
-        public const double CATCHUP_RATE = 2;
+        private const double catchup_rate = 2;
 
         private readonly GameplayClockContainer masterClock;
 
@@ -25,7 +24,7 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Spectate
         /// <summary>
         /// Whether this clock is waiting on frames to continue playback.
         /// </summary>
-        public Bindable<bool> WaitingOnFrames { get; } = new Bindable<bool>(true);
+        public bool WaitingOnFrames { get; set; } = true;
 
         /// <summary>
         /// Whether this clock is behind the master clock and running at a higher rate to catch up to it.
@@ -68,23 +67,14 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Spectate
         {
         }
 
-        public double Rate => IsCatchingUp ? CATCHUP_RATE : 1;
+        public double Rate
-
-        double IAdjustableClock.Rate
         {
-            get => Rate;
+            get => IsCatchingUp ? catchup_rate : 1;
-            set => throw new NotSupportedException();
+            set => throw new NotImplementedException();
         }
 
-        double IClock.Rate => Rate;
-
         public void ProcessFrame()
         {
-            ElapsedFrameTime = 0;
-            FramesPerSecond = 0;
-
-            masterClock.ProcessFrame();
-
             if (IsRunning)
             {
                 double elapsedSource = masterClock.ElapsedFrameTime;
@@ -94,6 +84,11 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Spectate
                 ElapsedFrameTime = elapsed;
                 FramesPerSecond = masterClock.FramesPerSecond;
             }
+            else
+            {
+                ElapsedFrameTime = 0;
+                FramesPerSecond = 0;
+            }
         }
 
         public double ElapsedFrameTime { get; private set; }

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

@@ -4,8 +4,8 @@
 using System;
 using System.Collections.Generic;
 using System.Linq;
-using osu.Framework.Bindables;
 using osu.Framework.Graphics;
+using osu.Framework.Logging;
 using osu.Game.Screens.Play;
 
 namespace osu.Game.Screens.OnlinePlay.Multiplayer.Spectate
@@ -33,12 +33,7 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Spectate
         /// <summary>
         /// An event which is invoked when gameplay is ready to start.
         /// </summary>
-        public event Action? ReadyToStart;
+        public Action? ReadyToStart;
-
-        /// <summary>
-        /// The catch-up state of the master clock.
-        /// </summary>
-        public IBindable<MasterClockState> MasterState => masterState;
 
         public double CurrentMasterTime => masterClock.CurrentTime;
 
@@ -52,9 +47,10 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Spectate
         /// </summary>
         private readonly List<SpectatorPlayerClock> playerClocks = new List<SpectatorPlayerClock>();
 
-        private readonly Bindable<MasterClockState> masterState = new Bindable<MasterClockState>();
+        private MasterClockState masterState = MasterClockState.Synchronised;
 
         private bool hasStarted;
+
         private double? firstStartAttemptTime;
 
         public SpectatorSyncManager(GameplayClockContainer master)
@@ -111,7 +107,7 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Spectate
             if (playerClocks.Count == 0)
                 return false;
 
-            int readyCount = playerClocks.Count(s => !s.WaitingOnFrames.Value);
+            int readyCount = playerClocks.Count(s => !s.WaitingOnFrames);
 
             if (readyCount == playerClocks.Count)
                 return performStart();
@@ -158,7 +154,7 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Spectate
                 }
 
                 // Make sure the player clock is running if it can.
-                clock.IsRunning = !clock.WaitingOnFrames.Value;
+                clock.IsRunning = !clock.WaitingOnFrames;
 
                 if (clock.IsCatchingUp)
                 {
@@ -180,8 +176,26 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Spectate
         /// </summary>
         private void updateMasterState()
         {
-            bool anyInSync = playerClocks.Any(s => !s.IsCatchingUp);
+            MasterClockState newState = playerClocks.Any(s => !s.IsCatchingUp) ? MasterClockState.Synchronised : MasterClockState.TooFarAhead;
-            masterState.Value = anyInSync ? MasterClockState.Synchronised : MasterClockState.TooFarAhead;
+
+            if (masterState == newState)
+                return;
+
+            masterState = newState;
+            Logger.Log($"{nameof(SpectatorSyncManager)}'s master clock become {masterState}");
+
+            switch (masterState)
+            {
+                case MasterClockState.Synchronised:
+                    if (hasStarted)
+                        masterClock.Start();
+
+                    break;
+
+                case MasterClockState.TooFarAhead:
+                    masterClock.Stop();
+                    break;
+            }
         }
     }
 }