Merge pull request #13261 from smoogipoo/fix-spectator-frame-conversion

Fix spectator crashing when converting mania replay frames

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

@@ -177,6 +177,7 @@ namespace osu.Game.Tests.Visual.Gameplay
             public override Container Overlays { get; }
             public override Container FrameStableComponents { get; }
             public override IFrameStableClock FrameStableClock { get; }
+            internal override bool FrameStablePlayback { get; set; }
             public override IReadOnlyList<Mod> Mods { get; }
 
             public override double GameplayStartTime { get; }

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

@@ -76,9 +76,9 @@ namespace osu.Game.Tests.Visual.Gameplay
             AddAssert("screen hasn't changed", () => Stack.CurrentScreen is SoloSpectator);
 
             start();
-            sendFrames();
-
             waitForPlayer();
+
+            sendFrames();
             AddAssert("ensure frames arrived", () => replayHandler.HasFrames);
 
             AddUntilStep("wait for frame starvation", () => replayHandler.WaitingForFrame);
@@ -116,12 +116,11 @@ namespace osu.Game.Tests.Visual.Gameplay
             start();
 
             loadSpectatingScreen();
+            waitForPlayer();
 
             AddStep("advance frame count", () => nextFrame = 300);
             sendFrames();
 
-            waitForPlayer();
-
             AddUntilStep("playing from correct point in time", () => player.ChildrenOfType<DrawableRuleset>().First().FrameStableClock.CurrentTime > 30000);
         }
 
@@ -210,7 +209,7 @@ namespace osu.Game.Tests.Visual.Gameplay
         private double currentFrameStableTime
             => player.ChildrenOfType<FrameStabilityContainer>().First().FrameStableClock.CurrentTime;
 
-        private void waitForPlayer() => AddUntilStep("wait for player", () => Stack.CurrentScreen is Player);
+        private void waitForPlayer() => AddUntilStep("wait for player", () => (Stack.CurrentScreen as Player)?.IsLoaded == true);
 
         private void start(int? beatmapId = null) => AddStep("start play", () => testSpectatorClient.StartPlay(streamingUser.Id, beatmapId ?? importedBeatmapId));
 

osu.Game/Rulesets/UI/DrawableRuleset.cs

@@ -68,10 +68,7 @@ namespace osu.Game.Rulesets.UI
 
         private bool frameStablePlayback = true;
 
-        /// <summary>
+        internal override bool FrameStablePlayback
-        /// Whether to enable frame-stable playback.
-        /// </summary>
-        internal bool FrameStablePlayback
         {
             get => frameStablePlayback;
             set
@@ -431,6 +428,11 @@ namespace osu.Game.Rulesets.UI
         /// </summary>
         public abstract IFrameStableClock FrameStableClock { get; }
 
+        /// <summary>
+        /// Whether to enable frame-stable playback.
+        /// </summary>
+        internal abstract bool FrameStablePlayback { get; set; }
+
         /// <summary>
         /// The mods which are to be applied.
         /// </summary>

osu.Game/Screens/Play/MasterGameplayClockContainer.cs

@@ -100,7 +100,13 @@ namespace osu.Game.Screens.Play
         {
             // The source is stopped by a frequency fade first.
             if (isPaused.NewValue)
-                this.TransformBindableTo(pauseFreqAdjust, 0, 200, Easing.Out).OnComplete(_ => AdjustableSource.Stop());
+            {
+                this.TransformBindableTo(pauseFreqAdjust, 0, 200, Easing.Out).OnComplete(_ =>
+                {
+                    if (IsPaused.Value == isPaused.NewValue)
+                        AdjustableSource.Stop();
+                });
+            }
             else
                 this.TransformBindableTo(pauseFreqAdjust, 1, 200, Easing.In);
         }

osu.Game/Screens/Play/Player.cs

@@ -81,10 +81,6 @@ namespace osu.Game.Screens.Play
         [Resolved]
         private ScoreManager scoreManager { get; set; }
 
-        private RulesetInfo rulesetInfo;
-
-        private Ruleset ruleset;
-
         [Resolved]
         private IAPIProvider api { get; set; }
 
@@ -94,6 +90,10 @@ namespace osu.Game.Screens.Play
         [Resolved]
         private SpectatorClient spectatorClient { get; set; }
 
+        protected Ruleset GameplayRuleset { get; private set; }
+
+        protected GameplayBeatmap GameplayBeatmap { get; private set; }
+
         private Sample sampleRestart;
 
         public BreakOverlay BreakOverlay;
@@ -144,8 +144,6 @@ namespace osu.Game.Screens.Play
             Configuration = configuration ?? new PlayerConfiguration();
         }
 
-        protected GameplayBeatmap GameplayBeatmap { get; private set; }
-
         private ScreenSuspensionHandler screenSuspension;
 
         private DependencyContainer dependencies;
@@ -164,7 +162,7 @@ namespace osu.Game.Screens.Play
 
             // ensure the score is in a consistent state with the current player.
             Score.ScoreInfo.Beatmap = Beatmap.Value.BeatmapInfo;
-            Score.ScoreInfo.Ruleset = rulesetInfo;
+            Score.ScoreInfo.Ruleset = GameplayRuleset.RulesetInfo;
             Score.ScoreInfo.Mods = Mods.Value.ToArray();
 
             PrepareReplay();
@@ -211,16 +209,16 @@ namespace osu.Game.Screens.Play
             if (game is OsuGame osuGame)
                 LocalUserPlaying.BindTo(osuGame.LocalUserPlaying);
 
-            DrawableRuleset = ruleset.CreateDrawableRulesetWith(playableBeatmap, Mods.Value);
+            DrawableRuleset = GameplayRuleset.CreateDrawableRulesetWith(playableBeatmap, Mods.Value);
             dependencies.CacheAs(DrawableRuleset);
 
-            ScoreProcessor = ruleset.CreateScoreProcessor();
+            ScoreProcessor = GameplayRuleset.CreateScoreProcessor();
             ScoreProcessor.ApplyBeatmap(playableBeatmap);
             ScoreProcessor.Mods.BindTo(Mods);
 
             dependencies.CacheAs(ScoreProcessor);
 
-            HealthProcessor = ruleset.CreateHealthProcessor(playableBeatmap.HitObjects[0].StartTime);
+            HealthProcessor = GameplayRuleset.CreateHealthProcessor(playableBeatmap.HitObjects[0].StartTime);
             HealthProcessor.ApplyBeatmap(playableBeatmap);
 
             dependencies.CacheAs(HealthProcessor);
@@ -239,7 +237,7 @@ namespace osu.Game.Screens.Play
 
             // the beatmapSkinProvider is used as the fallback source here to allow the ruleset-specific skin implementation
             // full access to all skin sources.
-            var rulesetSkinProvider = new SkinProvidingContainer(ruleset.CreateLegacySkinProvider(beatmapSkinProvider, playableBeatmap));
+            var rulesetSkinProvider = new SkinProvidingContainer(GameplayRuleset.CreateLegacySkinProvider(beatmapSkinProvider, playableBeatmap));
 
             // load the skinning hierarchy first.
             // this is intentionally done in two stages to ensure things are in a loaded state before exposing the ruleset to skin sources.
@@ -254,7 +252,7 @@ namespace osu.Game.Screens.Play
 
             // also give the HUD a ruleset container to allow rulesets to potentially override HUD elements (used to disable combo counters etc.)
             // we may want to limit this in the future to disallow rulesets from outright replacing elements the user expects to be there.
-            var hudRulesetContainer = new SkinProvidingContainer(ruleset.CreateLegacySkinProvider(beatmapSkinProvider, playableBeatmap));
+            var hudRulesetContainer = new SkinProvidingContainer(GameplayRuleset.CreateLegacySkinProvider(beatmapSkinProvider, playableBeatmap));
 
             // add the overlay components as a separate step as they proxy some elements from the above underlay/gameplay components.
             GameplayClockContainer.Add(hudRulesetContainer.WithChild(createOverlayComponents(Beatmap.Value)));
@@ -480,18 +478,18 @@ namespace osu.Game.Screens.Play
                 if (Beatmap.Value.Beatmap == null)
                     throw new InvalidOperationException("Beatmap was not loaded");
 
-                rulesetInfo = Ruleset.Value ?? Beatmap.Value.BeatmapInfo.Ruleset;
+                var rulesetInfo = Ruleset.Value ?? Beatmap.Value.BeatmapInfo.Ruleset;
-                ruleset = rulesetInfo.CreateInstance();
+                GameplayRuleset = rulesetInfo.CreateInstance();
 
                 try
                 {
-                    playable = Beatmap.Value.GetPlayableBeatmap(ruleset.RulesetInfo, Mods.Value);
+                    playable = Beatmap.Value.GetPlayableBeatmap(GameplayRuleset.RulesetInfo, Mods.Value);
                 }
                 catch (BeatmapInvalidForRulesetException)
                 {
                     // A playable beatmap may not be creatable with the user's preferred ruleset, so try using the beatmap's default ruleset
                     rulesetInfo = Beatmap.Value.BeatmapInfo.Ruleset;
-                    ruleset = rulesetInfo.CreateInstance();
+                    GameplayRuleset = rulesetInfo.CreateInstance();
 
                     playable = Beatmap.Value.GetPlayableBeatmap(rulesetInfo, Mods.Value);
                 }
@@ -585,6 +583,29 @@ namespace osu.Game.Screens.Play
         /// <param name="time">The destination time to seek to.</param>
         public void Seek(double time) => GameplayClockContainer.Seek(time);
 
+        private ScheduledDelegate frameStablePlaybackResetDelegate;
+
+        /// <summary>
+        /// Seeks to a specific time in gameplay, bypassing frame stability.
+        /// </summary>
+        /// <remarks>
+        /// Intermediate hitobject judgements may not be applied or reverted correctly during this seek.
+        /// </remarks>
+        /// <param name="time">The destination time to seek to.</param>
+        internal void NonFrameStableSeek(double time)
+        {
+            if (frameStablePlaybackResetDelegate?.Cancelled == false && !frameStablePlaybackResetDelegate.Completed)
+                frameStablePlaybackResetDelegate.RunTask();
+
+            bool wasFrameStable = DrawableRuleset.FrameStablePlayback;
+            DrawableRuleset.FrameStablePlayback = false;
+
+            Seek(time);
+
+            // Delay resetting frame-stable playback for one frame to give the FrameStabilityContainer a chance to seek.
+            frameStablePlaybackResetDelegate = ScheduleAfterChildren(() => DrawableRuleset.FrameStablePlayback = wasFrameStable);
+        }
+
         /// <summary>
         /// Restart gameplay via a parent <see cref="PlayerLoader"/>.
         /// <remarks>This can be called from a child screen in order to trigger the restart process.</remarks>
@@ -918,11 +939,10 @@ namespace osu.Game.Screens.Play
         /// Creates the player's <see cref="Scoring.Score"/>.
         /// </summary>
         /// <returns>The <see cref="Scoring.Score"/>.</returns>
-        protected virtual Score CreateScore() =>
+        protected virtual Score CreateScore() => new Score
-            new Score
+        {
-            {
+            ScoreInfo = new ScoreInfo { User = api.LocalUser.Value },
-                ScoreInfo = new ScoreInfo { User = api.LocalUser.Value },
+        };
-            };
 
         /// <summary>
         /// Imports the player's <see cref="Scoring.Score"/> to the local database.

osu.Game/Screens/Play/SpectatorPlayer.cs

@@ -1,14 +1,14 @@
 // 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 System.Linq;
 using osu.Framework.Allocation;
 using osu.Framework.Graphics;
 using osu.Framework.Screens;
-using osu.Game.Beatmaps;
 using osu.Game.Graphics;
 using osu.Game.Graphics.Sprites;
 using osu.Game.Online.Spectator;
+using osu.Game.Rulesets.Replays;
+using osu.Game.Rulesets.Replays.Types;
 using osu.Game.Scoring;
 using osu.Game.Screens.Ranking;
 
@@ -16,11 +16,11 @@ namespace osu.Game.Screens.Play
 {
     public class SpectatorPlayer : Player
     {
-        private readonly Score score;
-
         [Resolved]
         private SpectatorClient spectatorClient { get; set; }
 
+        private readonly Score score;
+
         protected override bool CheckModsAllowFailure() => false; // todo: better support starting mid-way through beatmap
 
         public SpectatorPlayer(Score score)
@@ -28,11 +28,6 @@ namespace osu.Game.Screens.Play
             this.score = score;
         }
 
-        protected override Score CreateScore() => score;
-
-        protected override ResultsScreen CreateResults(ScoreInfo score)
-            => new SpectatorResultsScreen(score);
-
         [BackgroundDependencyLoader]
         private void load()
         {
@@ -48,25 +43,66 @@ namespace osu.Game.Screens.Play
             });
         }
 
+        protected override void StartGameplay()
+        {
+            base.StartGameplay();
+
+            spectatorClient.OnNewFrames += userSentFrames;
+            seekToGameplay();
+        }
+
+        private void userSentFrames(int userId, FrameDataBundle bundle)
+        {
+            if (userId != score.ScoreInfo.User.Id)
+                return;
+
+            if (!LoadedBeatmapSuccessfully)
+                return;
+
+            if (!this.IsCurrentScreen())
+                return;
+
+            foreach (var frame in bundle.Frames)
+            {
+                IConvertibleReplayFrame convertibleFrame = GameplayRuleset.CreateConvertibleReplayFrame();
+                convertibleFrame.FromLegacy(frame, GameplayBeatmap.PlayableBeatmap);
+
+                var convertedFrame = (ReplayFrame)convertibleFrame;
+                convertedFrame.Time = frame.Time;
+
+                score.Replay.Frames.Add(convertedFrame);
+            }
+
+            seekToGameplay();
+        }
+
+        private bool seekedToGameplay;
+
+        private void seekToGameplay()
+        {
+            if (seekedToGameplay || score.Replay.Frames.Count == 0)
+                return;
+
+            NonFrameStableSeek(score.Replay.Frames[0].Time);
+
+            seekedToGameplay = true;
+        }
+
+        protected override Score CreateScore() => score;
+
+        protected override ResultsScreen CreateResults(ScoreInfo score)
+            => new SpectatorResultsScreen(score);
+
         protected override void PrepareReplay()
         {
             DrawableRuleset?.SetReplayScore(score);
         }
 
-        protected override GameplayClockContainer CreateGameplayClockContainer(WorkingBeatmap beatmap, double gameplayStart)
-        {
-            // if we already have frames, start gameplay at the point in time they exist, should they be too far into the beatmap.
-            double? firstFrameTime = score.Replay.Frames.FirstOrDefault()?.Time;
-
-            if (firstFrameTime == null || firstFrameTime <= gameplayStart + 5000)
-                return base.CreateGameplayClockContainer(beatmap, gameplayStart);
-
-            return new MasterGameplayClockContainer(beatmap, firstFrameTime.Value, true);
-        }
-
         public override bool OnExiting(IScreen next)
         {
             spectatorClient.OnUserBeganPlaying -= userBeganPlaying;
+            spectatorClient.OnNewFrames -= userSentFrames;
+
             return base.OnExiting(next);
         }
 
@@ -85,7 +121,10 @@ namespace osu.Game.Screens.Play
             base.Dispose(isDisposing);
 
             if (spectatorClient != null)
+            {
                 spectatorClient.OnUserBeganPlaying -= userBeganPlaying;
+                spectatorClient.OnNewFrames -= userSentFrames;
+            }
         }
     }
 }

osu.Game/Screens/Spectate/SpectatorScreen.cs

@@ -15,8 +15,6 @@ using osu.Game.Database;
 using osu.Game.Online.Spectator;
 using osu.Game.Replays;
 using osu.Game.Rulesets;
-using osu.Game.Rulesets.Replays;
-using osu.Game.Rulesets.Replays.Types;
 using osu.Game.Scoring;
 using osu.Game.Users;
 
@@ -71,8 +69,6 @@ namespace osu.Game.Screens.Spectate
                 playingUserStates.BindTo(spectatorClient.PlayingUserStates);
                 playingUserStates.BindCollectionChanged(onPlayingUserStatesChanged, true);
 
-                spectatorClient.OnNewFrames += userSentFrames;
-
                 managerUpdated = beatmaps.ItemUpdated.GetBoundCopy();
                 managerUpdated.BindValueChanged(beatmapUpdated);
 
@@ -197,29 +193,6 @@ namespace osu.Game.Screens.Spectate
             Schedule(() => StartGameplay(userId, gameplayState));
         }
 
-        private void userSentFrames(int userId, FrameDataBundle bundle)
-        {
-            if (!userMap.ContainsKey(userId))
-                return;
-
-            if (!gameplayStates.TryGetValue(userId, out var gameplayState))
-                return;
-
-            // The ruleset instance should be guaranteed to be in sync with the score via ScoreLock.
-            Debug.Assert(gameplayState.Ruleset != null && gameplayState.Ruleset.RulesetInfo.Equals(gameplayState.Score.ScoreInfo.Ruleset));
-
-            foreach (var frame in bundle.Frames)
-            {
-                IConvertibleReplayFrame convertibleFrame = gameplayState.Ruleset.CreateConvertibleReplayFrame();
-                convertibleFrame.FromLegacy(frame, gameplayState.Beatmap.Beatmap);
-
-                var convertedFrame = (ReplayFrame)convertibleFrame;
-                convertedFrame.Time = frame.Time;
-
-                gameplayState.Score.Replay.Frames.Add(convertedFrame);
-            }
-        }
-
         /// <summary>
         /// Invoked when a spectated user's state has changed.
         /// </summary>
@@ -260,8 +233,6 @@ namespace osu.Game.Screens.Spectate
 
             if (spectatorClient != null)
             {
-                spectatorClient.OnNewFrames -= userSentFrames;
-
                 foreach (var (userId, _) in userMap)
                     spectatorClient.StopWatchingUser(userId);
             }