Merge pull request #25079 from peppy/fix-player-startup-stop

Fix multiplayer not correctly pausing the track on initialisation
ppy · Oct 10, 2023 · 0c64fe7 · 0c64fe7
2 parents 1388f72 + b9dadc5
commit 0c64fe7
Show file tree

Hide file tree

Showing 4 changed files with 23 additions and 3 deletions.
diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerPlayer.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerPlayer.cs
@@ -11,6 +11,7 @@
 using osu.Game.Online.Rooms;
 using osu.Game.Rulesets.Osu;
 using osu.Game.Screens.OnlinePlay.Multiplayer;
+using osu.Game.Screens.Play;
 
 namespace osu.Game.Tests.Visual.Multiplayer
 {
@@ -28,6 +29,11 @@ public override void SetUpSteps()
                 Beatmap.Value = CreateWorkingBeatmap(new OsuRuleset().RulesetInfo);
             });
 
+            AddStep("Start track playing", () =>
+            {
+                Beatmap.Value.Track.Start();
+            });
+
             AddStep("initialise gameplay", () =>
             {
                 Stack.Push(player = new MultiplayerPlayer(MultiplayerClient.ServerAPIRoom, new PlaylistItem(Beatmap.Value.BeatmapInfo)
@@ -37,7 +43,14 @@ public override void SetUpSteps()
             });
 
             AddUntilStep("wait for player to be current", () => player.IsCurrentScreen() && player.IsLoaded);
+
+            AddAssert("gameplay clock is paused", () => player.ChildrenOfType<GameplayClockContainer>().Single().IsPaused.Value);
+            AddAssert("gameplay clock is not running", () => !player.ChildrenOfType<GameplayClockContainer>().Single().IsRunning);
+
             AddStep("start gameplay", () => ((IMultiplayerClient)MultiplayerClient).GameplayStarted());
+
+            AddUntilStep("gameplay clock is not paused", () => !player.ChildrenOfType<GameplayClockContainer>().Single().IsPaused.Value);
+            AddAssert("gameplay clock is running", () => player.ChildrenOfType<GameplayClockContainer>().Single().IsRunning);
         }
 
         [Test]

diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerPlayer.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerPlayer.cs
@@ -148,6 +148,9 @@ protected override void StartGameplay()
                 loadingDisplay.Show();
                 client.ChangeState(MultiplayerUserState.ReadyForGameplay);
             }
+
+            // This will pause the clock, pending the gameplay started callback from the server.
+            GameplayClockContainer.Reset();
         }
 
         private void failAndBail(string message = null)

diff --git a/osu.Game/Screens/Play/GameplayClockContainer.cs b/osu.Game/Screens/Play/GameplayClockContainer.cs
@@ -138,12 +138,16 @@ public void Stop()
         /// Resets this <see cref="GameplayClockContainer"/> and the source to an initial state ready for gameplay.
         /// </summary>
         /// <param name="time">The time to seek to on resetting. If <c>null</c>, the existing <see cref="StartTime"/> will be used.</param>
-        /// <param name="startClock">Whether to start the clock immediately, if not already started.</param>
+        /// <param name="startClock">Whether to start the clock immediately. If <c>false</c> and the clock was already paused, the clock will remain paused after this call.
+        /// </param>
         public void Reset(double? time = null, bool startClock = false)
         {
             bool wasPaused = isPaused.Value;
 
-            Stop();
+            // The intention of the Reset method is to get things into a known sane state.
+            // As such, we intentionally stop the underlying clock directly here, bypassing Stop/StopGameplayClock.
+            // This is to avoid any kind of isPaused state checks and frequency ramping (as provided by MasterGameplayClockContainer).
+            GameplayClock.Stop();
 
             if (time != null)
                 StartTime = time.Value;

diff --git a/osu.Game/Screens/Play/Player.cs b/osu.Game/Screens/Play/Player.cs
@@ -1078,7 +1078,7 @@ public override void OnEntering(ScreenTransitionEvent e)
         protected virtual void StartGameplay()
         {
             if (GameplayClockContainer.IsRunning)
-                throw new InvalidOperationException($"{nameof(StartGameplay)} should not be called when the gameplay clock is already running");
+                Logger.Error(new InvalidOperationException($"{nameof(StartGameplay)} should not be called when the gameplay clock is already running"), "Clock failure");
 
             GameplayClockContainer.Reset(startClock: true);