Merge pull request #17341 from peppy/fix-spectator-seeks-hot

Fix spectator not starting from current player position (hotfix version)

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

@@ -70,6 +70,56 @@ namespace osu.Game.Tests.Visual.Gameplay
             });
         }
 
+        [Test]
+        public void TestSeekToGameplayStartFramesArriveAfterPlayerLoad()
+        {
+            const double gameplay_start = 10000;
+
+            loadSpectatingScreen();
+
+            start();
+
+            waitForPlayer();
+
+            sendFrames(startTime: gameplay_start);
+
+            AddAssert("time is greater than seek target", () => currentFrameStableTime > gameplay_start);
+        }
+
+        /// <summary>
+        /// Tests the same as <see cref="TestSeekToGameplayStartFramesArriveAfterPlayerLoad"/> but with the frames arriving just as <see cref="Player"/> is transitioning into existence.
+        /// </summary>
+        [Test]
+        public void TestSeekToGameplayStartFramesArriveAsPlayerLoaded()
+        {
+            const double gameplay_start = 10000;
+
+            loadSpectatingScreen();
+
+            start();
+
+            AddUntilStep("wait for player loader", () => (Stack.CurrentScreen as PlayerLoader)?.IsLoaded == true);
+
+            AddUntilStep("queue send frames on player load", () =>
+            {
+                var loadingPlayer = (Stack.CurrentScreen as PlayerLoader)?.CurrentPlayer;
+
+                if (loadingPlayer == null)
+                    return false;
+
+                loadingPlayer.OnLoadComplete += _ =>
+                {
+                    spectatorClient.SendFramesFromUser(streamingUser.Id, 10, gameplay_start);
+                };
+                return true;
+            });
+
+            waitForPlayer();
+
+            AddUntilStep("state is playing", () => spectatorClient.WatchedUserStates[streamingUser.Id].State == SpectatedUserState.Playing);
+            AddAssert("time is greater than seek target", () => currentFrameStableTime > gameplay_start);
+        }
+
         [Test]
         public void TestFrameStarvationAndResume()
         {
@@ -319,9 +369,9 @@ namespace osu.Game.Tests.Visual.Gameplay
         private void checkPaused(bool state) =>
             AddUntilStep($"game is {(state ? "paused" : "playing")}", () => player.ChildrenOfType<DrawableRuleset>().First().IsPaused.Value == state);
 
-        private void sendFrames(int count = 10)
+        private void sendFrames(int count = 10, double startTime = 0)
         {
-            AddStep("send frames", () => spectatorClient.SendFramesFromUser(streamingUser.Id, count));
+            AddStep("send frames", () => spectatorClient.SendFramesFromUser(streamingUser.Id, count, startTime));
         }
 
         private void loadSpectatingScreen()

osu.Game/Screens/Play/Player.cs

@@ -614,6 +614,11 @@ namespace osu.Game.Screens.Play
         /// </remarks>
         /// <param name="time">The destination time to seek to.</param>
         internal void NonFrameStableSeek(double time)
+        {
+            // TODO: This schedule should not be required and is a temporary hotfix.
+            // See https://github.com/ppy/osu/issues/17267 for the issue.
+            // See https://github.com/ppy/osu/pull/17302 for a better fix which needs some more time.
+            ScheduleAfterChildren(() =>
             {
                 if (frameStablePlaybackResetDelegate?.Cancelled == false && !frameStablePlaybackResetDelegate.Completed)
                     frameStablePlaybackResetDelegate.RunTask();
@@ -625,6 +630,7 @@ namespace osu.Game.Screens.Play
 
                 // Delay resetting frame-stable playback for one frame to give the FrameStabilityContainer a chance to seek.
                 frameStablePlaybackResetDelegate = ScheduleAfterChildren(() => DrawableRuleset.FrameStablePlayback = wasFrameStable);
+            });
         }
 
         /// <summary>

osu.Game/Screens/Play/PlayerLoader.cs

@@ -1,10 +1,11 @@
 // 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.
 
+#nullable enable
+
 using System;
 using System.Diagnostics;
 using System.Threading.Tasks;
-using JetBrains.Annotations;
 using ManagedBass.Fx;
 using osu.Framework.Allocation;
 using osu.Framework.Audio;
@@ -48,31 +49,31 @@ namespace osu.Game.Screens.Play
         public override bool HandlePositionalInput => true;
 
         // We show the previous screen status
-        protected override UserActivity InitialActivity => null;
+        protected override UserActivity? InitialActivity => null;
 
         protected override bool PlayResumeSound => false;
 
-        protected BeatmapMetadataDisplay MetadataInfo { get; private set; }
+        protected BeatmapMetadataDisplay MetadataInfo { get; private set; } = null!;
 
         /// <summary>
         /// A fill flow containing the player settings groups, exposed for the ability to hide it from inheritors of the player loader.
         /// </summary>
-        protected FillFlowContainer<PlayerSettingsGroup> PlayerSettings { get; private set; }
+        protected FillFlowContainer<PlayerSettingsGroup> PlayerSettings { get; private set; } = null!;
 
-        protected VisualSettings VisualSettings { get; private set; }
+        protected VisualSettings VisualSettings { get; private set; } = null!;
 
-        protected AudioSettings AudioSettings { get; private set; }
+        protected AudioSettings AudioSettings { get; private set; } = null!;
 
-        protected Task LoadTask { get; private set; }
+        protected Task? LoadTask { get; private set; }
 
-        protected Task DisposalTask { get; private set; }
+        protected Task? DisposalTask { get; private set; }
 
         private bool backgroundBrightnessReduction;
 
         private readonly BindableDouble volumeAdjustment = new BindableDouble(1);
 
-        private AudioFilter lowPassFilter;
+        private AudioFilter lowPassFilter = null!;
-        private AudioFilter highPassFilter;
+        private AudioFilter highPassFilter = null!;
 
         protected bool BackgroundBrightnessReduction
         {
@@ -90,47 +91,49 @@ namespace osu.Game.Screens.Play
         private bool readyForPush =>
             !playerConsumed
             // don't push unless the player is completely loaded
-            && player?.LoadState == LoadState.Ready
+            && CurrentPlayer?.LoadState == LoadState.Ready
             // don't push if the user is hovering one of the panes, unless they are idle.
             && (IsHovered || idleTracker.IsIdle.Value)
             // don't push if the user is dragging a slider or otherwise.
-            && inputManager?.DraggedDrawable == null
+            && inputManager.DraggedDrawable == null
             // don't push if a focused overlay is visible, like settings.
-            && inputManager?.FocusedDrawable == null;
+            && inputManager.FocusedDrawable == null;
 
         private readonly Func<Player> createPlayer;
 
-        private Player player;
+        /// <summary>
+        /// The <see cref="Player"/> instance being loaded by this screen.
+        /// </summary>
+        public Player? CurrentPlayer { get; private set; }
 
         /// <summary>
-        /// Whether the curent player instance has been consumed via <see cref="consumePlayer"/>.
+        /// Whether the current player instance has been consumed via <see cref="consumePlayer"/>.
         /// </summary>
         private bool playerConsumed;
 
-        private LogoTrackingContainer content;
+        private LogoTrackingContainer content = null!;
 
         private bool hideOverlays;
 
-        private InputManager inputManager;
+        private InputManager inputManager = null!;
 
-        private IdleTracker idleTracker;
+        private IdleTracker idleTracker = null!;
 
-        private ScheduledDelegate scheduledPushPlayer;
+        private ScheduledDelegate? scheduledPushPlayer;
 
-        [CanBeNull]
+        private EpilepsyWarning? epilepsyWarning;
-        private EpilepsyWarning epilepsyWarning;
 
         [Resolved(CanBeNull = true)]
-        private NotificationOverlay notificationOverlay { get; set; }
+        private NotificationOverlay? notificationOverlay { get; set; }
 
         [Resolved(CanBeNull = true)]
-        private VolumeOverlay volumeOverlay { get; set; }
+        private VolumeOverlay? volumeOverlay { get; set; }
 
         [Resolved]
-        private AudioManager audioManager { get; set; }
+        private AudioManager audioManager { get; set; } = null!;
 
         [Resolved(CanBeNull = true)]
-        private BatteryInfo batteryInfo { get; set; }
+        private BatteryInfo? batteryInfo { get; set; }
 
         public PlayerLoader(Func<Player> createPlayer)
         {
@@ -237,12 +240,14 @@ namespace osu.Game.Screens.Play
         {
             base.OnResuming(last);
 
-            var lastScore = player.Score;
+            Debug.Assert(CurrentPlayer != null);
+
+            var lastScore = CurrentPlayer.Score;
 
             AudioSettings.ReferenceScore.Value = lastScore?.ScoreInfo;
 
             // prepare for a retry.
-            player = null;
+            CurrentPlayer = null;
             playerConsumed = false;
             cancelLoad();
 
@@ -344,9 +349,10 @@ namespace osu.Game.Screens.Play
         private Player consumePlayer()
         {
             Debug.Assert(!playerConsumed);
+            Debug.Assert(CurrentPlayer != null);
 
             playerConsumed = true;
-            return player;
+            return CurrentPlayer;
         }
 
         private void prepareNewPlayer()
@@ -354,11 +360,11 @@ namespace osu.Game.Screens.Play
             if (!this.IsCurrentScreen())
                 return;
 
-            player = createPlayer();
+            CurrentPlayer = createPlayer();
-            player.RestartCount = restartCount++;
+            CurrentPlayer.RestartCount = restartCount++;
-            player.RestartRequested = restartRequested;
+            CurrentPlayer.RestartRequested = restartRequested;
 
-            LoadTask = LoadComponentAsync(player, _ => MetadataInfo.Loading = false);
+            LoadTask = LoadComponentAsync(CurrentPlayer, _ => MetadataInfo.Loading = false);
         }
 
         private void restartRequested()
@@ -472,7 +478,7 @@ namespace osu.Game.Screens.Play
             if (isDisposing)
             {
                 // if the player never got pushed, we should explicitly dispose it.
-                DisposalTask = LoadTask?.ContinueWith(_ => player?.Dispose());
+                DisposalTask = LoadTask?.ContinueWith(_ => CurrentPlayer?.Dispose());
             }
         }
 
@@ -480,7 +486,7 @@ namespace osu.Game.Screens.Play
 
         #region Mute warning
 
-        private Bindable<bool> muteWarningShownOnce;
+        private Bindable<bool> muteWarningShownOnce = null!;
 
         private int restartCount;
 
@@ -535,7 +541,7 @@ namespace osu.Game.Screens.Play
 
         #region Low battery warning
 
-        private Bindable<bool> batteryWarningShownOnce;
+        private Bindable<bool> batteryWarningShownOnce = null!;
 
         private void showBatteryWarningIfNeeded()
         {

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

@@ -88,7 +88,8 @@ namespace osu.Game.Tests.Visual.Spectator
         /// </summary>
         /// <param name="userId">The user to send frames for.</param>
         /// <param name="count">The total number of frames to send.</param>
-        public void SendFramesFromUser(int userId, int count)
+        /// <param name="startTime">The time to start gameplay frames from.</param>
+        public void SendFramesFromUser(int userId, int count, double startTime = 0)
         {
             var frames = new List<LegacyReplayFrame>();
 
@@ -102,7 +103,7 @@ namespace osu.Game.Tests.Visual.Spectator
                     flush();
 
                 var buttonState = currentFrameIndex == lastFrameIndex ? ReplayButtonState.None : ReplayButtonState.Left1;
-                frames.Add(new LegacyReplayFrame(currentFrameIndex * 100, RNG.Next(0, 512), RNG.Next(0, 512), buttonState));
+                frames.Add(new LegacyReplayFrame(currentFrameIndex * 100 + startTime, RNG.Next(0, 512), RNG.Next(0, 512), buttonState));
             }
 
             flush();