Merge branch 'frame-stability-clean-up' into spectator-replay-watcher

osu.Game/Rulesets/UI/FrameStabilityContainer.cs

@@ -73,21 +73,11 @@ namespace osu.Game.Rulesets.UI
             setClock();
         }
 
-        /// <summary>
-        /// Whether we are running up-to-date with our parent clock.
-        /// If not, we will need to keep processing children until we catch up.
-        /// </summary>
-        private bool requireMoreUpdateLoops;
+        private PlaybackState state;
 
-        /// <summary>
-        /// Whether we are in a valid state (ie. should we keep processing children frames).
-        /// This should be set to false when the replay is, for instance, waiting for future frames to arrive.
-        /// </summary>
-        private bool validState;
+        protected override bool RequiresChildrenUpdate => base.RequiresChildrenUpdate && state != PlaybackState.NotValid;
 
-        protected override bool RequiresChildrenUpdate => base.RequiresChildrenUpdate && validState;
-
-        private bool isAttached => ReplayInputHandler != null;
+        private bool hasReplayAttached => ReplayInputHandler != null;
 
         private const double sixty_frame_time = 1000.0 / 60;
 
@@ -95,28 +85,26 @@ namespace osu.Game.Rulesets.UI
 
         public override bool UpdateSubTree()
         {
-            requireMoreUpdateLoops = true;
-
-            validState = !frameStableClock.IsPaused.Value;
-
-            int loops = 0;
+            state = frameStableClock.IsPaused.Value ? PlaybackState.NotValid : PlaybackState.Valid;
 
             if (frameStableClock.WaitingOnFrames.Value)
             {
                 // for now, force one update loop to check if frames have arrived
                 // this may have to change in the future where we want stable user pausing during replay playback.
-                validState = true;
+                state = PlaybackState.Valid;
             }
 
-            while (validState && requireMoreUpdateLoops && loops++ < MaxCatchUpFrames)
+            int loops = MaxCatchUpFrames;
+
+            while (state != PlaybackState.NotValid && loops-- > 0)
             {
                 updateClock();
 
-                if (validState)
-                {
-                    base.UpdateSubTree();
-                    UpdateSubTreeMasking(this, ScreenSpaceDrawQuad.AABBFloat);
-                }
+                if (state == PlaybackState.NotValid)
+                    break;
+
+                base.UpdateSubTree();
+                UpdateSubTreeMasking(this, ScreenSpaceDrawQuad.AABBFloat);
             }
 
             return true;
@@ -127,94 +115,103 @@ namespace osu.Game.Rulesets.UI
             if (parentGameplayClock == null)
                 setClock(); // LoadComplete may not be run yet, but we still want the clock.
 
-            validState = true;
-            requireMoreUpdateLoops = false;
+            // each update start with considering things in valid state.
+            state = PlaybackState.Valid;
 
-            var newProposedTime = parentGameplayClock.CurrentTime;
+            // our goal is to catch up to the time provided by the parent clock.
+            var proposedTime = parentGameplayClock.CurrentTime;
 
-            try
+            if (FrameStablePlayback)
+                // if we require frame stability, the proposed time will be adjusted to move at most one known
+                // frame interval in the current direction.
+                applyFrameStability(ref proposedTime);
+
+            if (hasReplayAttached)
+                state = updateReplay(ref proposedTime);
+
+            if (proposedTime != manualClock.CurrentTime)
+                direction = proposedTime >= manualClock.CurrentTime ? 1 : -1;
+
+            manualClock.CurrentTime = proposedTime;
+            manualClock.Rate = Math.Abs(parentGameplayClock.Rate) * direction;
+            manualClock.IsRunning = parentGameplayClock.IsRunning;
+
+            double timeBehind = Math.Abs(manualClock.CurrentTime - parentGameplayClock.CurrentTime);
+
+            // determine whether catch-up is required.
+            if (state != PlaybackState.NotValid)
+                state = timeBehind > 0 ? PlaybackState.RequiresCatchUp : PlaybackState.Valid;
+
+            frameStableClock.IsCatchingUp.Value = timeBehind > 200;
+            frameStableClock.WaitingOnFrames.Value = state == PlaybackState.NotValid;
+
+            // The manual clock time has changed in the above code. The framed clock now needs to be updated
+            // to ensure that the its time is valid for our children before input is processed
+            framedClock.ProcessFrame();
+        }
+
+        /// <summary>
+        /// Attempt to advance replay playback for a given time.
+        /// </summary>
+        /// <param name="proposedTime">The time which is to be displayed.</param>
+        private PlaybackState updateReplay(ref double proposedTime)
+        {
+            double? newTime;
+
+            if (FrameStablePlayback)
             {
-                if (FrameStablePlayback)
+                // when stability is turned on, we shouldn't execute for time values the replay is unable to satisfy.
+                newTime = ReplayInputHandler.SetFrameFromTime(proposedTime);
+            }
+            else
+            {
+                // when stability is disabled, we don't really care about accuracy.
+                // looping over the replay will allow it to catch up and feed out the required values
+                // for the current time.
+                while ((newTime = ReplayInputHandler.SetFrameFromTime(proposedTime)) != proposedTime)
                 {
-                    if (firstConsumption)
+                    if (newTime == null)
                     {
-                        // On the first update, frame-stability seeking would result in unexpected/unwanted behaviour.
-                        // Instead we perform an initial seek to the proposed time.
-
-                        // process frame (in addition to finally clause) to clear out ElapsedTime
-                        manualClock.CurrentTime = newProposedTime;
-                        framedClock.ProcessFrame();
-
-                        firstConsumption = false;
+                        // special case for when the replay actually can't arrive at the required time.
+                        // protects from potential endless loop.
+                        break;
                     }
-                    else if (manualClock.CurrentTime < gameplayStartTime)
-                        manualClock.CurrentTime = newProposedTime = Math.Min(gameplayStartTime, newProposedTime);
-                    else if (Math.Abs(manualClock.CurrentTime - newProposedTime) > sixty_frame_time * 1.2f)
-                    {
-                        newProposedTime = newProposedTime > manualClock.CurrentTime
-                            ? Math.Min(newProposedTime, manualClock.CurrentTime + sixty_frame_time)
-                            : Math.Max(newProposedTime, manualClock.CurrentTime - sixty_frame_time);
-                    }
-                }
-
-                if (isAttached)
-                {
-                    double? newTime;
-
-                    if (FrameStablePlayback)
-                    {
-                        // when stability is turned on, we shouldn't execute for time values the replay is unable to satisfy.
-                        if ((newTime = ReplayInputHandler.SetFrameFromTime(newProposedTime)) == null)
-                        {
-                            // setting invalid state here ensures that gameplay will not continue (ie. our child
-                            // hierarchy won't be updated).
-                            validState = false;
-
-                            // potentially loop to catch-up playback.
-                            requireMoreUpdateLoops = true;
-
-                            return;
-                        }
-                    }
-                    else
-                    {
-                        // when stability is disabled, we don't really care about accuracy.
-                        // looping over the replay will allow it to catch up and feed out the required values
-                        // for the current time.
-                        while ((newTime = ReplayInputHandler.SetFrameFromTime(newProposedTime)) != newProposedTime)
-                        {
-                            if (newTime == null)
-                            {
-                                // special case for when the replay actually can't arrive at the required time.
-                                // protects from potential endless loop.
-                                validState = false;
-                                return;
-                            }
-                        }
-                    }
-
-                    newProposedTime = newTime.Value;
                 }
             }
-            finally
+
+            if (newTime == null)
+                return PlaybackState.NotValid;
+
+            proposedTime = newTime.Value;
+            return PlaybackState.Valid;
+        }
+
+        /// <summary>
+        /// Apply frame stability modifier to a time.
+        /// </summary>
+        /// <param name="proposedTime">The time which is to be displayed.</param>
+        private void applyFrameStability(ref double proposedTime)
+        {
+            if (firstConsumption)
             {
-                if (newProposedTime != manualClock.CurrentTime)
-                    direction = newProposedTime >= manualClock.CurrentTime ? 1 : -1;
+                // On the first update, frame-stability seeking would result in unexpected/unwanted behaviour.
+                // Instead we perform an initial seek to the proposed time.
 
-                manualClock.CurrentTime = newProposedTime;
-                manualClock.Rate = Math.Abs(parentGameplayClock.Rate) * direction;
-                manualClock.IsRunning = parentGameplayClock.IsRunning;
-
-                double timeBehind = Math.Abs(manualClock.CurrentTime - parentGameplayClock.CurrentTime);
-
-                requireMoreUpdateLoops |= timeBehind != 0;
-
-                frameStableClock.IsCatchingUp.Value = timeBehind > 200;
-                frameStableClock.WaitingOnFrames.Value = !validState;
-
-                // The manual clock time has changed in the above code. The framed clock now needs to be updated
-                // to ensure that the its time is valid for our children before input is processed
+                // process frame (in addition to finally clause) to clear out ElapsedTime
+                manualClock.CurrentTime = proposedTime;
                 framedClock.ProcessFrame();
+
+                firstConsumption = false;
+                return;
+            }
+
+            if (manualClock.CurrentTime < gameplayStartTime)
+                manualClock.CurrentTime = proposedTime = Math.Min(gameplayStartTime, proposedTime);
+            else if (Math.Abs(manualClock.CurrentTime - proposedTime) > sixty_frame_time * 1.2f)
+            {
+                proposedTime = proposedTime > manualClock.CurrentTime
+                    ? Math.Min(proposedTime, manualClock.CurrentTime + sixty_frame_time)
+                    : Math.Max(proposedTime, manualClock.CurrentTime - sixty_frame_time);
             }
         }
 
@@ -233,6 +230,26 @@ namespace osu.Game.Rulesets.UI
 
         public ReplayInputHandler ReplayInputHandler { get; set; }
 
+        private enum PlaybackState
+        {
+            /// <summary>
+            /// Playback is not possible. Child hierarchy should not be processed.
+            /// </summary>
+            NotValid,
+
+            /// <summary>
+            /// Whether we are running up-to-date with our parent clock.
+            /// If not, we will need to keep processing children until we catch up.
+            /// </summary>
+            RequiresCatchUp,
+
+            /// <summary>
+            /// Whether we are in a valid state (ie. should we keep processing children frames).
+            /// This should be set to false when the replay is, for instance, waiting for future frames to arrive.
+            /// </summary>
+            Valid
+        }
+
         private class FrameStabilityClock : GameplayClock, IFrameStableClock
         {
             public GameplayClock ParentGameplayClock;

osu.Game/Screens/Edit/Editor.cs

@@ -43,8 +43,9 @@ using osuTK.Input;
 namespace osu.Game.Screens.Edit
 {
     [Cached(typeof(IBeatSnapProvider))]
+    [Cached(typeof(ISamplePlaybackDisabler))]
     [Cached]
-    public class Editor : ScreenWithBeatmapBackground, IKeyBindingHandler<GlobalAction>, IKeyBindingHandler<PlatformAction>, IBeatSnapProvider
+    public class Editor : ScreenWithBeatmapBackground, IKeyBindingHandler<GlobalAction>, IKeyBindingHandler<PlatformAction>, IBeatSnapProvider, ISamplePlaybackDisabler
     {
         public override float BackgroundParallaxAmount => 0.1f;
 
@@ -64,6 +65,10 @@ namespace osu.Game.Screens.Edit
         [Resolved(canBeNull: true)]
         private DialogOverlay dialogOverlay { get; set; }
 
+        public IBindable<bool> SamplePlaybackDisabled => samplePlaybackDisabled;
+
+        private readonly Bindable<bool> samplePlaybackDisabled = new Bindable<bool>();
+
         private bool exitConfirmed;
 
         private string lastSavedHash;
@@ -109,9 +114,10 @@ namespace osu.Game.Screens.Edit
             UpdateClockSource();
 
             dependencies.CacheAs(clock);
-            dependencies.CacheAs<ISamplePlaybackDisabler>(clock);
             AddInternal(clock);
 
+            clock.SeekingOrStopped.BindValueChanged(_ => updateSampleDisabledState());
+
             // todo: remove caching of this and consume via editorBeatmap?
             dependencies.Cache(beatDivisor);
 
@@ -557,40 +563,52 @@ namespace osu.Game.Screens.Edit
                 .ScaleTo(0.98f, 200, Easing.OutQuint)
                 .FadeOut(200, Easing.OutQuint);
 
-            if ((currentScreen = screenContainer.SingleOrDefault(s => s.Type == e.NewValue)) != null)
+            try
             {
-                screenContainer.ChangeChildDepth(currentScreen, lastScreen?.Depth + 1 ?? 0);
+                if ((currentScreen = screenContainer.SingleOrDefault(s => s.Type == e.NewValue)) != null)
+                {
+                    screenContainer.ChangeChildDepth(currentScreen, lastScreen?.Depth + 1 ?? 0);
 
-                currentScreen
-                    .ScaleTo(1, 200, Easing.OutQuint)
-                    .FadeIn(200, Easing.OutQuint);
-                return;
+                    currentScreen
+                        .ScaleTo(1, 200, Easing.OutQuint)
+                        .FadeIn(200, Easing.OutQuint);
+                    return;
+                }
+
+                switch (e.NewValue)
+                {
+                    case EditorScreenMode.SongSetup:
+                        currentScreen = new SetupScreen();
+                        break;
+
+                    case EditorScreenMode.Compose:
+                        currentScreen = new ComposeScreen();
+                        break;
+
+                    case EditorScreenMode.Design:
+                        currentScreen = new DesignScreen();
+                        break;
+
+                    case EditorScreenMode.Timing:
+                        currentScreen = new TimingScreen();
+                        break;
+                }
+
+                LoadComponentAsync(currentScreen, newScreen =>
+                {
+                    if (newScreen == currentScreen)
+                        screenContainer.Add(newScreen);
+                });
             }
-
-            switch (e.NewValue)
+            finally
             {
-                case EditorScreenMode.SongSetup:
-                    currentScreen = new SetupScreen();
-                    break;
-
-                case EditorScreenMode.Compose:
-                    currentScreen = new ComposeScreen();
-                    break;
-
-                case EditorScreenMode.Design:
-                    currentScreen = new DesignScreen();
-                    break;
-
-                case EditorScreenMode.Timing:
-                    currentScreen = new TimingScreen();
-                    break;
+                updateSampleDisabledState();
             }
+        }
 
-            LoadComponentAsync(currentScreen, newScreen =>
-            {
-                if (newScreen == currentScreen)
-                    screenContainer.Add(newScreen);
-            });
+        private void updateSampleDisabledState()
+        {
+            samplePlaybackDisabled.Value = clock.SeekingOrStopped.Value || !(currentScreen is ComposeScreen);
         }
 
         private void seek(UIEvent e, int direction)

osu.Game/Screens/Edit/EditorClock.cs

@@ -11,14 +11,13 @@ using osu.Framework.Timing;
 using osu.Framework.Utils;
 using osu.Game.Beatmaps;
 using osu.Game.Beatmaps.ControlPoints;
-using osu.Game.Screens.Play;
 
 namespace osu.Game.Screens.Edit
 {
     /// <summary>
     /// A decoupled clock which adds editor-specific functionality, such as snapping to a user-defined beat divisor.
     /// </summary>
-    public class EditorClock : Component, IFrameBasedClock, IAdjustableClock, ISourceChangeableClock, ISamplePlaybackDisabler
+    public class EditorClock : Component, IFrameBasedClock, IAdjustableClock, ISourceChangeableClock
     {
         public IBindable<Track> Track => track;
 
@@ -32,9 +31,9 @@ namespace osu.Game.Screens.Edit
 
         private readonly DecoupleableInterpolatingFramedClock underlyingClock;
 
-        public IBindable<bool> SamplePlaybackDisabled => samplePlaybackDisabled;
+        public IBindable<bool> SeekingOrStopped => seekingOrStopped;
 
-        private readonly Bindable<bool> samplePlaybackDisabled = new Bindable<bool>();
+        private readonly Bindable<bool> seekingOrStopped = new Bindable<bool>(true);
 
         public EditorClock(WorkingBeatmap beatmap, BindableBeatDivisor beatDivisor)
             : this(beatmap.Beatmap.ControlPointInfo, beatmap.Track.Length, beatDivisor)
@@ -171,13 +170,13 @@ namespace osu.Game.Screens.Edit
 
         public void Stop()
         {
-            samplePlaybackDisabled.Value = true;
+            seekingOrStopped.Value = true;
             underlyingClock.Stop();
         }
 
         public bool Seek(double position)
         {
-            samplePlaybackDisabled.Value = true;
+            seekingOrStopped.Value = true;
 
             ClearTransforms();
             return underlyingClock.Seek(position);
@@ -228,7 +227,7 @@ namespace osu.Game.Screens.Edit
 
         private void updateSeekingState()
         {
-            if (samplePlaybackDisabled.Value)
+            if (seekingOrStopped.Value)
             {
                 if (track.Value?.IsRunning != true)
                 {
@@ -240,13 +239,13 @@ namespace osu.Game.Screens.Edit
                 // we are either running a seek tween or doing an immediate seek.
                 // in the case of an immediate seek the seeking bool will be set to false after one update.
                 // this allows for silencing hit sounds and the likes.
-                samplePlaybackDisabled.Value = Transforms.Any();
+                seekingOrStopped.Value = Transforms.Any();
             }
         }
 
         public void SeekTo(double seekDestination)
         {
-            samplePlaybackDisabled.Value = true;
+            seekingOrStopped.Value = true;
 
             if (IsRunning)
                 Seek(seekDestination);