Make resyncing a bit more resilient

osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerSpectator.cs

@@ -1,12 +1,15 @@
 // 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;
 using System.Collections.Generic;
 using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
 using NUnit.Framework;
 using osu.Framework.Allocation;
+using osu.Framework.Audio;
+using osu.Framework.Bindables;
 using osu.Framework.Graphics;
 using osu.Framework.Testing;
 using osu.Framework.Utils;
@@ -153,6 +156,27 @@ namespace osu.Game.Tests.Visual.Multiplayer
             checkPausedInstant(55, false);
         }
 
+        [Test]
+        public void TestPlayerStartsCatchingUpOnlyAfterExceedingMaxOffset()
+        {
+            start(new[] { 55, 56 });
+            loadSpectateScreen();
+
+            sendFrames(55, 1000);
+            sendFrames(56, 1000);
+
+            Bindable<double> slowDownAdjustment;
+
+            AddStep("slow down player 2", () =>
+            {
+                slowDownAdjustment = new Bindable<double>(0.99);
+                getInstance(56).Beatmap.Track.AddAdjustment(AdjustableProperty.Frequency, slowDownAdjustment);
+            });
+
+            AddUntilStep("exceeded min offset but not catching up", () => getGameplayOffset(55, 56) > PlayerInstance.MAX_OFFSET && !getInstance(56).IsCatchingUp);
+            AddUntilStep("catching up or caught up", () => getInstance(56).IsCatchingUp || Math.Abs(getGameplayOffset(55, 56)) < PlayerInstance.SYNC_TARGET * 2);
+        }
+
         [Test]
         public void TestPlayersCatchUpAfterFallingBehind()
         {
@@ -174,7 +198,9 @@ namespace osu.Game.Tests.Visual.Multiplayer
             checkPausedInstant(56, false);
 
             // Player 2 should catch up to player 1 after unpausing.
-            AddUntilStep("player 1 time == player 2 time", () => Precision.AlmostEquals(getGameplayTime(55), getGameplayTime(56), 16));
+            AddUntilStep("player 2 not catching up", () => !getInstance(56).IsCatchingUp);
+            AddAssert("player 1 time == player 2 time", () => Math.Abs(getGameplayOffset(55, 56)) <= 2 * PlayerInstance.SYNC_TARGET);
+            AddWaitStep("wait a bit", 5);
         }
 
         private void loadSpectateScreen()
@@ -236,6 +262,11 @@ namespace osu.Game.Tests.Visual.Multiplayer
         private void checkPausedInstant(int userId, bool state) =>
             AddAssert($"{userId} is {(state ? "paused" : "playing")}", () => getPlayer(userId).ChildrenOfType<GameplayClockContainer>().First().IsPaused.Value == state);
 
+        /// <summary>
+        /// Returns time(user1) - time(user2).
+        /// </summary>
+        private double getGameplayOffset(int user1, int user2) => getGameplayTime(user1) - getGameplayTime(user2);
+
         private double getGameplayTime(int userId) => getPlayer(userId).ChildrenOfType<GameplayClockContainer>().Single().GameplayClock.CurrentTime;
 
         private Player getPlayer(int userId) => getInstance(userId).ChildrenOfType<Player>().Single();

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

@@ -6,6 +6,7 @@ using osu.Framework.Audio;
 using osu.Framework.Bindables;
 using osu.Framework.Graphics;
 using osu.Framework.Graphics.Containers;
+using osu.Framework.Logging;
 using osu.Game.Beatmaps;
 using osu.Game.Scoring;
 using osu.Game.Screens.Play;
@@ -15,8 +16,20 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Spectate
 {
     public class PlayerInstance : CompositeDrawable
     {
+        /// <summary>
+        /// The rate at which a user catches up after becoming desynchronised.
+        /// </summary>
         private const double catchup_rate = 2;
-        private const double max_sync_offset = 50;
+
+        /// <summary>
+        /// The offset from the expected time at which to START synchronisation.
+        /// </summary>
+        public const double MAX_OFFSET = 50;
+
+        /// <summary>
+        /// The maximum offset from the expected time at which to STOP synchronisation.
+        /// </summary>
+        public const double SYNC_TARGET = 16;
 
         public bool PlayerLoaded => stack?.CurrentScreen is Player;
 
@@ -26,6 +39,8 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Spectate
 
         public readonly Score Score;
 
+        public bool IsCatchingUp { get; private set; }
+
         private OsuScreenStack stack;
         private MultiplayerSpectatorPlayer player;
 
@@ -63,7 +78,6 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Spectate
 
         private readonly BindableDouble catchupFrequencyAdjustment = new BindableDouble(catchup_rate);
         private double targetTrackTime;
-        private bool isCatchingUp;
 
         private void updateCatchup()
         {
@@ -77,18 +91,27 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Spectate
                 return;
 
             double currentTime = Beatmap.Track.CurrentTime;
-            bool catchupRequired = targetTrackTime > currentTime + max_sync_offset;
+            double timeBehind = targetTrackTime - currentTime;
 
-            // Skip catchup if nothing needs to be done.
+            double offsetForCatchup = IsCatchingUp ? SYNC_TARGET : MAX_OFFSET;
-            if (catchupRequired == isCatchingUp)
+            bool catchupRequired = timeBehind > offsetForCatchup;
+
+            // Skip catchup if no work needs to be done.
+            if (catchupRequired == IsCatchingUp)
                 return;
 
             if (catchupRequired)
+            {
                 Beatmap.Track.AddAdjustment(AdjustableProperty.Frequency, catchupFrequencyAdjustment);
+                Logger.Log($"{User.Id} catchup started (behind: {timeBehind})");
+            }
             else
+            {
                 Beatmap.Track.RemoveAdjustment(AdjustableProperty.Frequency, catchupFrequencyAdjustment);
+                Logger.Log($"{User.Id} catchup finished (behind: {timeBehind})");
+            }
 
-            isCatchingUp = catchupRequired;
+            IsCatchingUp = catchupRequired;
         }
 
         public double GetCurrentGameplayTime()