From 9eecb23fefa623356228bc916dba930e784dc035 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Thu, 10 Jun 2021 21:35:52 +0900 Subject: [PATCH 01/16] Only seek to the end on first frame arrival --- osu.Game/Screens/Play/SpectatorPlayer.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/osu.Game/Screens/Play/SpectatorPlayer.cs b/osu.Game/Screens/Play/SpectatorPlayer.cs index 0286b6b8a6..304bc4d7ed 100644 --- a/osu.Game/Screens/Play/SpectatorPlayer.cs +++ b/osu.Game/Screens/Play/SpectatorPlayer.cs @@ -48,7 +48,6 @@ namespace osu.Game.Screens.Play base.StartGameplay(); spectatorClient.OnNewFrames += userSentFrames; - seekToGameplay(); } private void userSentFrames(int userId, FrameDataBundle bundle) From 5073ede115f1c3da87e4c8055b8b39bbde31bdd8 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Thu, 10 Jun 2021 21:38:20 +0900 Subject: [PATCH 02/16] Clear existing frames when starting gameplay --- osu.Game/Screens/Play/SpectatorPlayer.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/osu.Game/Screens/Play/SpectatorPlayer.cs b/osu.Game/Screens/Play/SpectatorPlayer.cs index 304bc4d7ed..4d3b5eefb1 100644 --- a/osu.Game/Screens/Play/SpectatorPlayer.cs +++ b/osu.Game/Screens/Play/SpectatorPlayer.cs @@ -47,6 +47,8 @@ namespace osu.Game.Screens.Play { base.StartGameplay(); + // Start gameplay along with the very first arrival frame (the latest one). + score.Replay.Frames.Clear(); spectatorClient.OnNewFrames += userSentFrames; } From e18f4cd4bae79bd52801857a6e4deeefff3f35d5 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Thu, 10 Jun 2021 21:39:13 +0900 Subject: [PATCH 03/16] Combine methods --- osu.Game/Screens/Play/SpectatorPlayer.cs | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/osu.Game/Screens/Play/SpectatorPlayer.cs b/osu.Game/Screens/Play/SpectatorPlayer.cs index 4d3b5eefb1..f662a479ec 100644 --- a/osu.Game/Screens/Play/SpectatorPlayer.cs +++ b/osu.Game/Screens/Play/SpectatorPlayer.cs @@ -63,6 +63,8 @@ namespace osu.Game.Screens.Play if (!this.IsCurrentScreen()) return; + bool isFirstBundle = score.Replay.Frames.Count == 0; + foreach (var frame in bundle.Frames) { IConvertibleReplayFrame convertibleFrame = GameplayRuleset.CreateConvertibleReplayFrame(); @@ -74,19 +76,8 @@ namespace osu.Game.Screens.Play 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; + if (isFirstBundle && score.Replay.Frames.Count > 0) + NonFrameStableSeek(score.Replay.Frames[0].Time); } protected override Score CreateScore() => score; From 58d71e4aead45fd8a756d574d904ded7ee5e3e3e Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Thu, 10 Jun 2021 22:41:38 +0900 Subject: [PATCH 04/16] Remove local "next frame" storage --- .../Visual/Gameplay/TestSceneSpectator.cs | 17 +++---- .../TestSceneMultiSpectatorLeaderboard.cs | 4 +- .../TestSceneMultiSpectatorScreen.cs | 12 +---- .../Visual/Spectator/TestSpectatorClient.cs | 44 ++++++++++++++----- 4 files changed, 42 insertions(+), 35 deletions(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs index 6eeb3596a8..0926b8778b 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs @@ -41,8 +41,6 @@ namespace osu.Game.Tests.Visual.Gameplay [Resolved] private OsuGameBase game { get; set; } - private int nextFrame; - private BeatmapSetInfo importedBeatmap; private int importedBeatmapId; @@ -51,8 +49,6 @@ namespace osu.Game.Tests.Visual.Gameplay { base.SetUpSteps(); - AddStep("reset sent frames", () => nextFrame = 0); - AddStep("import beatmap", () => { importedBeatmap = ImportBeatmapTest.LoadOszIntoOsu(game, virtualTrack: true).Result; @@ -105,7 +101,8 @@ namespace osu.Game.Tests.Visual.Gameplay waitForPlayer(); checkPaused(true); - sendFrames(1000); // send enough frames to ensure play won't be paused + // send enough frames to ensure play won't be paused + sendFrames(100); checkPaused(false); } @@ -114,12 +111,12 @@ namespace osu.Game.Tests.Visual.Gameplay public void TestSpectatingDuringGameplay() { start(); + sendFrames(300); loadSpectatingScreen(); waitForPlayer(); - AddStep("advance frame count", () => nextFrame = 300); - sendFrames(); + sendFrames(300); AddUntilStep("playing from correct point in time", () => player.ChildrenOfType().First().FrameStableClock.CurrentTime > 30000); } @@ -220,11 +217,7 @@ namespace osu.Game.Tests.Visual.Gameplay private void sendFrames(int count = 10) { - AddStep("send frames", () => - { - testSpectatorClient.SendFrames(streamingUser.Id, nextFrame, count); - nextFrame += count; - }); + AddStep("send frames", () => testSpectatorClient.SendFrames(streamingUser.Id, count)); } private void loadSpectatingScreen() diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiSpectatorLeaderboard.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiSpectatorLeaderboard.cs index 5ad35be0ec..7f8a44fb98 100644 --- a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiSpectatorLeaderboard.cs +++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiSpectatorLeaderboard.cs @@ -96,10 +96,10 @@ namespace osu.Game.Tests.Visual.Multiplayer // For player 2, send frames in sets of 10. for (int i = 0; i < 100; i++) { - spectatorClient.SendFrames(PLAYER_1_ID, i, 1); + spectatorClient.SendFrames(PLAYER_1_ID, 1); if (i % 10 == 0) - spectatorClient.SendFrames(PLAYER_2_ID, i, 10); + spectatorClient.SendFrames(PLAYER_2_ID, 10); } }); diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiSpectatorScreen.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiSpectatorScreen.cs index b91391c409..f5032fc2a5 100644 --- a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiSpectatorScreen.cs +++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiSpectatorScreen.cs @@ -37,7 +37,6 @@ namespace osu.Game.Tests.Visual.Multiplayer private MultiSpectatorScreen spectatorScreen; private readonly List playingUserIds = new List(); - private readonly Dictionary nextFrame = new Dictionary(); private BeatmapSetInfo importedSet; private BeatmapInfo importedBeatmap; @@ -55,8 +54,6 @@ namespace osu.Game.Tests.Visual.Multiplayer { base.SetUpSteps(); - AddStep("reset sent frames", () => nextFrame.Clear()); - AddStep("add streaming client", () => { Remove(spectatorClient); @@ -80,8 +77,6 @@ namespace osu.Game.Tests.Visual.Multiplayer Client.CurrentMatchPlayingUserIds.Add(PLAYER_2_ID); playingUserIds.Add(PLAYER_1_ID); playingUserIds.Add(PLAYER_2_ID); - nextFrame[PLAYER_1_ID] = 0; - nextFrame[PLAYER_2_ID] = 0; }); loadSpectateScreen(false); @@ -253,7 +248,6 @@ namespace osu.Game.Tests.Visual.Multiplayer Client.CurrentMatchPlayingUserIds.Add(id); spectatorClient.StartPlay(id, beatmapId ?? importedBeatmapId); playingUserIds.Add(id); - nextFrame[id] = 0; } }); } @@ -264,7 +258,6 @@ namespace osu.Game.Tests.Visual.Multiplayer { spectatorClient.EndPlay(userId); playingUserIds.Remove(userId); - nextFrame.Remove(userId); }); } @@ -275,10 +268,7 @@ namespace osu.Game.Tests.Visual.Multiplayer AddStep("send frames", () => { foreach (int id in userIds) - { - spectatorClient.SendFrames(id, nextFrame[id], count); - nextFrame[id] += count; - } + spectatorClient.SendFrames(id, count); }); } diff --git a/osu.Game/Tests/Visual/Spectator/TestSpectatorClient.cs b/osu.Game/Tests/Visual/Spectator/TestSpectatorClient.cs index c7aa43b377..f206d4f8b0 100644 --- a/osu.Game/Tests/Visual/Spectator/TestSpectatorClient.cs +++ b/osu.Game/Tests/Visual/Spectator/TestSpectatorClient.cs @@ -20,9 +20,15 @@ namespace osu.Game.Tests.Visual.Spectator { public class TestSpectatorClient : SpectatorClient { + /// + /// Maximum number of frames sent per bundle via . + /// + public const int FRAME_BUNDLE_SIZE = 10; + public override IBindable IsConnected { get; } = new Bindable(true); private readonly Dictionary userBeatmapDictionary = new Dictionary(); + private readonly Dictionary userNextFrameDictionary = new Dictionary(); [Resolved] private IAPIProvider api { get; set; } = null!; @@ -35,6 +41,7 @@ namespace osu.Game.Tests.Visual.Spectator public void StartPlay(int userId, int beatmapId) { userBeatmapDictionary[userId] = beatmapId; + userNextFrameDictionary[userId] = 0; sendPlayingState(userId); } @@ -57,24 +64,41 @@ namespace osu.Game.Tests.Visual.Spectator public new void Schedule(Action action) => base.Schedule(action); /// - /// Sends frames for an arbitrary user. + /// Sends frames for an arbitrary user, in bundles containing 10 frames each. /// /// The user to send frames for. - /// The frame index. - /// The number of frames to send. - public void SendFrames(int userId, int index, int count) + /// The total number of frames to send. + public void SendFrames(int userId, int count) { var frames = new List(); - for (int i = index; i < index + count; i++) - { - var buttonState = i == index + count - 1 ? ReplayButtonState.None : ReplayButtonState.Left1; + int currentFrameIndex = userNextFrameDictionary[userId]; + int lastFrameIndex = currentFrameIndex + count - 1; - frames.Add(new LegacyReplayFrame(i * 100, RNG.Next(0, 512), RNG.Next(0, 512), buttonState)); + for (; currentFrameIndex <= lastFrameIndex; currentFrameIndex++) + { + // This is done in the next frame so that currentFrameIndex is updated to the correct value. + if (frames.Count == FRAME_BUNDLE_SIZE) + flush(); + + var buttonState = currentFrameIndex == lastFrameIndex ? ReplayButtonState.None : ReplayButtonState.Left1; + frames.Add(new LegacyReplayFrame(currentFrameIndex * 100, RNG.Next(0, 512), RNG.Next(0, 512), buttonState)); } - var bundle = new FrameDataBundle(new ScoreInfo { Combo = index + count }, frames); - ((ISpectatorClient)this).UserSentFrames(userId, bundle); + flush(); + + userNextFrameDictionary[userId] = currentFrameIndex; + + void flush() + { + if (frames.Count == 0) + return; + + var bundle = new FrameDataBundle(new ScoreInfo { Combo = currentFrameIndex }, frames.ToArray()); + ((ISpectatorClient)this).UserSentFrames(userId, bundle); + + frames.Clear(); + } } protected override Task BeginPlayingInternal(SpectatorState state) From 1c67ef7c9190ae8bdfe36cce0ed6d6e5576b040c Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Fri, 11 Jun 2021 16:23:59 +0900 Subject: [PATCH 05/16] Make catchup clock support seeking --- .../Multiplayer/Spectate/CatchUpSpectatorPlayerClock.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/CatchUpSpectatorPlayerClock.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/CatchUpSpectatorPlayerClock.cs index 9e1a020eca..20d12d62a3 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/CatchUpSpectatorPlayerClock.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/CatchUpSpectatorPlayerClock.cs @@ -34,7 +34,11 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Spectate public void Stop() => IsRunning = false; - public bool Seek(double position) => true; + public bool Seek(double position) + { + CurrentTime = position; + return true; + } public void ResetSpeedAdjustments() { From 75d825c85cb01be1a55e56e8a2adfb1dd745c4db Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Fri, 11 Jun 2021 16:24:52 +0900 Subject: [PATCH 06/16] Dont control master clock from sync manager --- .../Multiplayer/Spectate/CatchUpSyncManager.cs | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/CatchUpSyncManager.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/CatchUpSyncManager.cs index efc12eaaa5..2d0e88238d 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/CatchUpSyncManager.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/CatchUpSyncManager.cs @@ -63,7 +63,6 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Spectate } updateCatchup(); - updateMasterClock(); } /// @@ -133,21 +132,5 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Spectate } } } - - /// - /// Updates the master clock's running state. - /// - private void updateMasterClock() - { - bool anyInSync = playerClocks.Any(s => !s.IsCatchingUp); - - if (MasterClock.IsRunning != anyInSync) - { - if (anyInSync) - MasterClock.Start(); - else - MasterClock.Stop(); - } - } } } From a99cb79738f61d4cacbbda13c9b87283fba04733 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Fri, 11 Jun 2021 16:25:45 +0900 Subject: [PATCH 07/16] Seek master clock on multi-spectator start --- .../Multiplayer/Spectate/CatchUpSyncManager.cs | 13 +++++++++++-- .../Multiplayer/Spectate/ISyncManager.cs | 6 ++++++ .../Multiplayer/Spectate/MultiSpectatorScreen.cs | 14 ++++++++++++++ 3 files changed, 31 insertions(+), 2 deletions(-) diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/CatchUpSyncManager.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/CatchUpSyncManager.cs index 2d0e88238d..0f7466e44b 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/CatchUpSyncManager.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/CatchUpSyncManager.cs @@ -1,6 +1,7 @@ // Copyright (c) ppy Pty Ltd . 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 osu.Framework.Graphics; @@ -33,6 +34,8 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Spectate /// public IAdjustableClock MasterClock { get; } + public event Action ReadyToStart; + /// /// The player clocks. /// @@ -80,14 +83,20 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Spectate int readyCount = playerClocks.Count(s => !s.WaitingOnFrames.Value); if (readyCount == playerClocks.Count) - return hasStarted = true; + return performStart(); if (readyCount > 0) { firstStartAttemptTime ??= Time.Current; if (Time.Current - firstStartAttemptTime > MAXIMUM_START_DELAY) - return hasStarted = true; + return performStart(); + } + + bool performStart() + { + ReadyToStart?.Invoke(); + return hasStarted = true; } return false; diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/ISyncManager.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/ISyncManager.cs index bd698108f6..f59c1a75f7 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/ISyncManager.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/ISyncManager.cs @@ -1,6 +1,7 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. +using System; using osu.Framework.Timing; namespace osu.Game.Screens.OnlinePlay.Multiplayer.Spectate @@ -15,6 +16,11 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Spectate /// IAdjustableClock MasterClock { get; } + /// + /// An event which is invoked when gameplay is ready to start. + /// + event Action ReadyToStart; + /// /// Adds an to manage. /// diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/MultiSpectatorScreen.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/MultiSpectatorScreen.cs index 983daac909..e401c45933 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/MultiSpectatorScreen.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/MultiSpectatorScreen.cs @@ -101,6 +101,8 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Spectate Anchor = Anchor.CentreLeft, Origin = Anchor.CentreLeft, }, leaderboardContainer.Add); + + syncManager.ReadyToStart += onReadyToStart; } protected override void LoadComplete() @@ -129,6 +131,18 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Spectate private bool isCandidateAudioSource([CanBeNull] ISpectatorPlayerClock clock) => clock?.IsRunning == true && !clock.IsCatchingUp && !clock.WaitingOnFrames.Value; + private void onReadyToStart() + { + var startTime = instances.Where(i => i.Score != null) + .SelectMany(i => i.Score.Replay.Frames) + .Select(f => f.Time) + .DefaultIfEmpty(0) + .Max(); + + masterClockContainer.Seek(startTime); + masterClockContainer.Start(); + } + protected override void OnUserStateChanged(int userId, SpectatorState spectatorState) { } From 9eaaac6bb790616db3dcb259669e2b5d48b74b39 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Fri, 11 Jun 2021 17:59:27 +0900 Subject: [PATCH 08/16] Remove master clock state assertions --- .../OnlinePlay/TestSceneCatchUpSyncManager.cs | 24 +++++++------------ 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/osu.Game.Tests/OnlinePlay/TestSceneCatchUpSyncManager.cs b/osu.Game.Tests/OnlinePlay/TestSceneCatchUpSyncManager.cs index d4e591cf09..6851df3832 100644 --- a/osu.Game.Tests/OnlinePlay/TestSceneCatchUpSyncManager.cs +++ b/osu.Game.Tests/OnlinePlay/TestSceneCatchUpSyncManager.cs @@ -31,32 +31,24 @@ namespace osu.Game.Tests.OnlinePlay } [Test] - public void TestMasterClockStartsWhenAllPlayerClocksHaveFrames() + public void TestPlayerClocksStartWhenAllHaveFrames() { setWaiting(() => player1, false); - assertMasterState(false); assertPlayerClockState(() => player1, false); assertPlayerClockState(() => player2, false); setWaiting(() => player2, false); - assertMasterState(true); assertPlayerClockState(() => player1, true); assertPlayerClockState(() => player2, true); } [Test] - public void TestMasterClockDoesNotStartWhenNoneReadyForMaximumDelayTime() - { - AddWaitStep($"wait {CatchUpSyncManager.MAXIMUM_START_DELAY} milliseconds", (int)Math.Ceiling(CatchUpSyncManager.MAXIMUM_START_DELAY / TimePerAction)); - assertMasterState(false); - } - - [Test] - public void TestMasterClockStartsWhenAnyReadyForMaximumDelayTime() + public void TestReadyPlayersStartWhenReadyForMaximumDelayTime() { setWaiting(() => player1, false); AddWaitStep($"wait {CatchUpSyncManager.MAXIMUM_START_DELAY} milliseconds", (int)Math.Ceiling(CatchUpSyncManager.MAXIMUM_START_DELAY / TimePerAction)); - assertMasterState(true); + assertPlayerClockState(() => player1, true); + assertPlayerClockState(() => player2, false); } [Test] @@ -153,9 +145,6 @@ namespace osu.Game.Tests.OnlinePlay private void setPlayerClockTime(Func playerClock, double offsetFromMaster) => AddStep($"set player clock {playerClock().Id} = master - {offsetFromMaster}", () => playerClock().Seek(master.CurrentTime - offsetFromMaster)); - private void assertMasterState(bool running) - => AddAssert($"master clock {(running ? "is" : "is not")} running", () => master.IsRunning == running); - private void assertCatchingUp(Func playerClock, bool catchingUp) => AddAssert($"player clock {playerClock().Id} {(catchingUp ? "is" : "is not")} catching up", () => playerClock().IsCatchingUp == catchingUp); @@ -201,6 +190,11 @@ namespace osu.Game.Tests.OnlinePlay private class TestManualClock : ManualClock, IAdjustableClock { + public TestManualClock() + { + IsRunning = true; + } + public void Start() => IsRunning = true; public void Stop() => IsRunning = false; From e9ebbd298d4c7129123e40c1f8b8352abfd9ed96 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Fri, 11 Jun 2021 18:13:54 +0900 Subject: [PATCH 09/16] Add a few more tests --- .../TestSceneMultiSpectatorScreen.cs | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiSpectatorScreen.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiSpectatorScreen.cs index f5032fc2a5..35d096a92e 100644 --- a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiSpectatorScreen.cs +++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiSpectatorScreen.cs @@ -12,6 +12,7 @@ using osu.Framework.Testing; using osu.Game.Beatmaps; using osu.Game.Database; using osu.Game.Online.Spectator; +using osu.Game.Rulesets.UI; using osu.Game.Screens.OnlinePlay.Multiplayer.Spectate; using osu.Game.Screens.Play; using osu.Game.Tests.Beatmaps.IO; @@ -224,6 +225,36 @@ namespace osu.Game.Tests.Visual.Multiplayer assertMuted(PLAYER_2_ID, true); } + [Test] + public void TestSpectatingDuringGameplay() + { + var players = new[] { PLAYER_1_ID, PLAYER_2_ID }; + + start(players); + sendFrames(players, 300); + + loadSpectateScreen(); + sendFrames(players, 300); + + AddUntilStep("playing from correct point in time", () => this.ChildrenOfType().All(r => r.FrameStableClock.CurrentTime > 30000)); + } + + [Test] + public void TestSpectatingDuringGameplayWithLateFrames() + { + start(new[] { PLAYER_1_ID, PLAYER_2_ID }); + sendFrames(new[] { PLAYER_1_ID, PLAYER_2_ID }, 300); + + loadSpectateScreen(); + sendFrames(PLAYER_1_ID, 300); + + AddWaitStep("wait maximum start delay seconds", (int)(CatchUpSyncManager.MAXIMUM_START_DELAY / TimePerAction)); + checkPaused(PLAYER_1_ID, false); + + sendFrames(PLAYER_2_ID, 300); + AddUntilStep("player 2 playing from correct point in time", () => getPlayer(PLAYER_2_ID).ChildrenOfType().Single().FrameStableClock.CurrentTime > 30000); + } + private void loadSpectateScreen(bool waitForPlayerLoad = true) { AddStep("load screen", () => From 263b8ff097ed688e3fe0ff6e9bb90d1a17c93057 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Fri, 11 Jun 2021 18:14:37 +0900 Subject: [PATCH 10/16] Wait for full player load --- osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/PlayerArea.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/PlayerArea.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/PlayerArea.cs index fe79e5db72..95ccc08608 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/PlayerArea.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/PlayerArea.cs @@ -27,7 +27,7 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Spectate /// /// Whether a is loaded in the area. /// - public bool PlayerLoaded => stack?.CurrentScreen is Player; + public bool PlayerLoaded => (stack?.CurrentScreen as Player)?.IsLoaded == true; /// /// The user id this corresponds to. From 59eda70c12204f0ed71a551d711d1bb7aa1adbb3 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Fri, 11 Jun 2021 18:39:50 +0900 Subject: [PATCH 11/16] Seek to the least most-recent frame instead --- .../OnlinePlay/Multiplayer/Spectate/MultiSpectatorScreen.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/MultiSpectatorScreen.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/MultiSpectatorScreen.cs index e401c45933..1ea63e454a 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/MultiSpectatorScreen.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/MultiSpectatorScreen.cs @@ -133,11 +133,13 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Spectate private void onReadyToStart() { + // Seek the master clock to the gameplay time. + // This is chosen as the first available frame in the players' replays, which matches the seek by each individual SpectatorPlayer. var startTime = instances.Where(i => i.Score != null) .SelectMany(i => i.Score.Replay.Frames) .Select(f => f.Time) .DefaultIfEmpty(0) - .Max(); + .Min(); masterClockContainer.Seek(startTime); masterClockContainer.Start(); From 0a8daab4f78e369481e4bff7c382e2bd845386fd Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Fri, 11 Jun 2021 19:15:53 +0900 Subject: [PATCH 12/16] Pause master clock when too far ahead --- .../TestSceneMultiSpectatorScreen.cs | 17 +++++++++++++++ .../Spectate/CatchUpSyncManager.cs | 21 ++++++++++++++++--- .../Multiplayer/Spectate/ISyncManager.cs | 10 +++++++-- .../Multiplayer/Spectate/MasterClockState.cs | 18 ++++++++++++++++ .../Spectate/MultiSpectatorScreen.cs | 10 +++++++++ 5 files changed, 71 insertions(+), 5 deletions(-) create mode 100644 osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/MasterClockState.cs diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiSpectatorScreen.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiSpectatorScreen.cs index 35d096a92e..18eb7e1c1e 100644 --- a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiSpectatorScreen.cs +++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiSpectatorScreen.cs @@ -103,6 +103,23 @@ namespace osu.Game.Tests.Visual.Multiplayer AddWaitStep("wait a bit", 20); } + [Test] + public void TestTimeDoesNotProgressWhileAllPlayersPaused() + { + start(new[] { PLAYER_1_ID, PLAYER_2_ID }); + loadSpectateScreen(); + + sendFrames(PLAYER_1_ID, 20); + sendFrames(PLAYER_2_ID, 10); + + checkPaused(PLAYER_2_ID, true); + checkPausedInstant(PLAYER_1_ID, false); + AddAssert("master clock still running", () => this.ChildrenOfType().Single().IsRunning); + + checkPaused(PLAYER_1_ID, true); + AddUntilStep("master clock paused", () => !this.ChildrenOfType().Single().IsRunning); + } + [Test] public void TestPlayersMustStartSimultaneously() { diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/CatchUpSyncManager.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/CatchUpSyncManager.cs index 0f7466e44b..781123f5bb 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/CatchUpSyncManager.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/CatchUpSyncManager.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.Linq; +using osu.Framework.Bindables; using osu.Framework.Graphics; using osu.Framework.Timing; @@ -29,18 +30,22 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Spectate /// public const double MAXIMUM_START_DELAY = 15000; + public event Action ReadyToStart; + /// /// The master clock which is used to control the timing of all player clocks clocks. /// public IAdjustableClock MasterClock { get; } - public event Action ReadyToStart; + public IBindable MasterState => masterState; /// /// The player clocks. /// private readonly List playerClocks = new List(); + private readonly Bindable masterState = new Bindable(); + private bool hasStarted; private double? firstStartAttemptTime; @@ -65,7 +70,8 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Spectate return; } - updateCatchup(); + updatePlayerCatchup(); + updateMasterState(); } /// @@ -105,7 +111,7 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Spectate /// /// Updates the catchup states of all player clocks clocks. /// - private void updateCatchup() + private void updatePlayerCatchup() { for (int i = 0; i < playerClocks.Count; i++) { @@ -141,5 +147,14 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Spectate } } } + + /// + /// Updates the state of the master clock. + /// + private void updateMasterState() + { + bool anyInSync = playerClocks.Any(s => !s.IsCatchingUp); + masterState.Value = anyInSync ? MasterClockState.Synchronised : MasterClockState.TooFarAhead; + } } } diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/ISyncManager.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/ISyncManager.cs index f59c1a75f7..3c644ccb78 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/ISyncManager.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/ISyncManager.cs @@ -2,6 +2,7 @@ // See the LICENCE file in the repository root for full licence text. using System; +using osu.Framework.Bindables; using osu.Framework.Timing; namespace osu.Game.Screens.OnlinePlay.Multiplayer.Spectate @@ -11,15 +12,20 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Spectate /// public interface ISyncManager { + /// + /// An event which is invoked when gameplay is ready to start. + /// + event Action ReadyToStart; + /// /// The master clock which player clocks should synchronise to. /// IAdjustableClock MasterClock { get; } /// - /// An event which is invoked when gameplay is ready to start. + /// An event which is invoked when the state of is changed. /// - event Action ReadyToStart; + IBindable MasterState { get; } /// /// Adds an to manage. diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/MasterClockState.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/MasterClockState.cs new file mode 100644 index 0000000000..8982d1669d --- /dev/null +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/MasterClockState.cs @@ -0,0 +1,18 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +namespace osu.Game.Screens.OnlinePlay.Multiplayer.Spectate +{ + public enum MasterClockState + { + /// + /// The master clock is synchronised with at least one player clock. + /// + Synchronised, + + /// + /// The master clock is too far ahead of any player clock and needs to slow down. + /// + TooFarAhead + } +} diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/MultiSpectatorScreen.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/MultiSpectatorScreen.cs index 1ea63e454a..ee1968aa5f 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/MultiSpectatorScreen.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/MultiSpectatorScreen.cs @@ -5,6 +5,7 @@ using System; using System.Linq; using JetBrains.Annotations; using osu.Framework.Allocation; +using osu.Framework.Bindables; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Game.Online.Multiplayer; @@ -103,6 +104,7 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Spectate }, leaderboardContainer.Add); syncManager.ReadyToStart += onReadyToStart; + syncManager.MasterState.BindValueChanged(onMasterStateChanged, true); } protected override void LoadComplete() @@ -145,6 +147,14 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Spectate masterClockContainer.Start(); } + private void onMasterStateChanged(ValueChangedEvent state) + { + if (state.NewValue == MasterClockState.Synchronised) + masterClockContainer.Start(); + else + masterClockContainer.Stop(); + } + protected override void OnUserStateChanged(int userId, SpectatorState spectatorState) { } From 9f163f7f20ca155ab331d63d2fae99fdb1c2591d Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Fri, 11 Jun 2021 19:23:25 +0900 Subject: [PATCH 13/16] Use switch statement to be more explicit about state --- .../Multiplayer/Spectate/MultiSpectatorScreen.cs | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/MultiSpectatorScreen.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/MultiSpectatorScreen.cs index ee1968aa5f..013e5551cf 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/MultiSpectatorScreen.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/MultiSpectatorScreen.cs @@ -149,10 +149,16 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Spectate private void onMasterStateChanged(ValueChangedEvent state) { - if (state.NewValue == MasterClockState.Synchronised) - masterClockContainer.Start(); - else - masterClockContainer.Stop(); + switch (state.NewValue) + { + case MasterClockState.Synchronised: + masterClockContainer.Start(); + break; + + case MasterClockState.TooFarAhead: + masterClockContainer.Stop(); + break; + } } protected override void OnUserStateChanged(int userId, SpectatorState spectatorState) From bfdbe3c3fe534fc3b0060f4253635065f30b074b Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Tue, 29 Jun 2021 17:29:25 +0900 Subject: [PATCH 14/16] Fix clocks getting added a second time --- .../Multiplayer/Spectate/CatchUpSyncManager.cs | 7 ++++++- .../Spectate/MultiSpectatorScreen.cs | 17 ++++++++--------- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/CatchUpSyncManager.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/CatchUpSyncManager.cs index 781123f5bb..94278a47b6 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/CatchUpSyncManager.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/CatchUpSyncManager.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.Linq; using osu.Framework.Bindables; using osu.Framework.Graphics; @@ -54,7 +55,11 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Spectate MasterClock = master; } - public void AddPlayerClock(ISpectatorPlayerClock clock) => playerClocks.Add(clock); + public void AddPlayerClock(ISpectatorPlayerClock clock) + { + Debug.Assert(!playerClocks.Contains(clock)); + playerClocks.Add(clock); + } public void RemovePlayerClock(ISpectatorPlayerClock clock) => playerClocks.Remove(clock); diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/MultiSpectatorScreen.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/MultiSpectatorScreen.cs index 013e5551cf..e709cba0ee 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/MultiSpectatorScreen.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/MultiSpectatorScreen.cs @@ -101,7 +101,13 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Spectate Expanded = { Value = true }, Anchor = Anchor.CentreLeft, Origin = Anchor.CentreLeft, - }, leaderboardContainer.Add); + }, l => + { + foreach (var instance in instances) + leaderboard.AddClock(instance.UserId, instance.GameplayClock); + + leaderboardContainer.Add(leaderboard); + }); syncManager.ReadyToStart += onReadyToStart; syncManager.MasterState.BindValueChanged(onMasterStateChanged, true); @@ -166,14 +172,7 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Spectate } protected override void StartGameplay(int userId, GameplayState gameplayState) - { - var instance = instances.Single(i => i.UserId == userId); - - instance.LoadScore(gameplayState.Score); - - syncManager.AddPlayerClock(instance.GameplayClock); - leaderboard.AddClock(instance.UserId, instance.GameplayClock); - } + => instances.Single(i => i.UserId == userId).LoadScore(gameplayState.Score); protected override void EndGameplay(int userId) { From 6dd3c6fe9307b5d71c850ef8dbe6d1733d2e8643 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Tue, 29 Jun 2021 22:45:18 +0900 Subject: [PATCH 15/16] Make MultiSpectatorScreen and tests more resillient to timing --- .../Multiplayer/TestSceneMultiSpectatorScreen.cs | 12 +++++++++--- .../Multiplayer/Spectate/MultiSpectatorScreen.cs | 16 +++++++++++----- 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiSpectatorScreen.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiSpectatorScreen.cs index a308e854c6..8c63fb8924 100644 --- a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiSpectatorScreen.cs +++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiSpectatorScreen.cs @@ -82,8 +82,8 @@ namespace osu.Game.Tests.Visual.Multiplayer start(new[] { PLAYER_1_ID, PLAYER_2_ID }); loadSpectateScreen(); - sendFrames(PLAYER_1_ID, 20); - sendFrames(PLAYER_2_ID, 10); + sendFrames(PLAYER_1_ID, 40); + sendFrames(PLAYER_2_ID, 20); checkPaused(PLAYER_2_ID, true); checkPausedInstant(PLAYER_1_ID, false); @@ -196,6 +196,7 @@ namespace osu.Game.Tests.Visual.Multiplayer sendFrames(PLAYER_1_ID, 10); sendFrames(PLAYER_2_ID, 20); + checkPaused(PLAYER_1_ID, false); assertMuted(PLAYER_1_ID, false); assertMuted(PLAYER_2_ID, true); @@ -297,7 +298,12 @@ namespace osu.Game.Tests.Visual.Multiplayer => AddUntilStep($"{userId} is {(state ? "paused" : "playing")}", () => getPlayer(userId).ChildrenOfType().First().GameplayClock.IsRunning != state); private void checkPausedInstant(int userId, bool state) - => AddAssert($"{userId} is {(state ? "paused" : "playing")}", () => getPlayer(userId).ChildrenOfType().First().GameplayClock.IsRunning != state); + { + checkPaused(userId, state); + + // Todo: The following should work, but is broken because SpectatorScreen retrieves the WorkingBeatmap via the BeatmapManager, bypassing the test scene clock and running real-time. + // AddAssert($"{userId} is {(state ? "paused" : "playing")}", () => getPlayer(userId).ChildrenOfType().First().GameplayClock.IsRunning != state); + } private void assertMuted(int userId, bool muted) => AddAssert($"{userId} {(muted ? "is" : "is not")} muted", () => getInstance(userId).Mute == muted); diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/MultiSpectatorScreen.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/MultiSpectatorScreen.cs index e709cba0ee..2a2759e0dd 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/MultiSpectatorScreen.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/MultiSpectatorScreen.cs @@ -43,6 +43,7 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Spectate private PlayerGrid grid; private MultiSpectatorLeaderboard leaderboard; private PlayerArea currentAudioSource; + private bool canStartMasterClock; /// /// Creates a new . @@ -108,17 +109,17 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Spectate leaderboardContainer.Add(leaderboard); }); - - syncManager.ReadyToStart += onReadyToStart; - syncManager.MasterState.BindValueChanged(onMasterStateChanged, true); } protected override void LoadComplete() { base.LoadComplete(); - masterClockContainer.Stop(); masterClockContainer.Reset(); + masterClockContainer.Stop(); + + syncManager.ReadyToStart += onReadyToStart; + syncManager.MasterState.BindValueChanged(onMasterStateChanged, true); } protected override void Update() @@ -151,6 +152,9 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Spectate masterClockContainer.Seek(startTime); masterClockContainer.Start(); + + // Although the clock has been started, this flag is set to allow for later synchronisation state changes to also be able to start it. + canStartMasterClock = true; } private void onMasterStateChanged(ValueChangedEvent state) @@ -158,7 +162,9 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Spectate switch (state.NewValue) { case MasterClockState.Synchronised: - masterClockContainer.Start(); + if (canStartMasterClock) + masterClockContainer.Start(); + break; case MasterClockState.TooFarAhead: From 331b7237ab5f175067a886668d2ef3de34baf664 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Tue, 29 Jun 2021 22:53:31 +0900 Subject: [PATCH 16/16] Attempt to fix one more intermittent test failure --- .../Visual/Multiplayer/TestSceneMultiSpectatorScreen.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiSpectatorScreen.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiSpectatorScreen.cs index 8c63fb8924..783db49d36 100644 --- a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiSpectatorScreen.cs +++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiSpectatorScreen.cs @@ -197,8 +197,7 @@ namespace osu.Game.Tests.Visual.Multiplayer sendFrames(PLAYER_1_ID, 10); sendFrames(PLAYER_2_ID, 20); checkPaused(PLAYER_1_ID, false); - assertMuted(PLAYER_1_ID, false); - assertMuted(PLAYER_2_ID, true); + assertOneNotMuted(); checkPaused(PLAYER_1_ID, true); assertMuted(PLAYER_1_ID, true); @@ -305,6 +304,8 @@ namespace osu.Game.Tests.Visual.Multiplayer // AddAssert($"{userId} is {(state ? "paused" : "playing")}", () => getPlayer(userId).ChildrenOfType().First().GameplayClock.IsRunning != state); } + private void assertOneNotMuted() => AddAssert("one player not muted", () => spectatorScreen.ChildrenOfType().Count(p => !p.Mute) == 1); + private void assertMuted(int userId, bool muted) => AddAssert($"{userId} {(muted ? "is" : "is not")} muted", () => getInstance(userId).Mute == muted);