From 5fd97bd0433048b1370b3c8fbee51476d07af984 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 26 Oct 2020 19:47:39 +0900 Subject: [PATCH 01/55] Add basic spectator screen --- .../Visual/Gameplay/TestSceneSpectator.cs | 20 +++++++++++ osu.Game/Screens/Play/Spectator.cs | 36 +++++++++++++++++++ 2 files changed, 56 insertions(+) create mode 100644 osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs create mode 100644 osu.Game/Screens/Play/Spectator.cs diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs new file mode 100644 index 0000000000..25c30dbc5a --- /dev/null +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs @@ -0,0 +1,20 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using NUnit.Framework; +using osu.Game.Screens.Play; +using osu.Game.Users; + +namespace osu.Game.Tests.Visual.Gameplay +{ + public class TestSceneSpectator : ScreenTestScene + { + private readonly User user = new User { Id = 1234, Username = "Test user" }; + + [Test] + public void TestSpectating() + { + AddStep("load screen", () => LoadScreen(new Spectator(user))); + } + } +} diff --git a/osu.Game/Screens/Play/Spectator.cs b/osu.Game/Screens/Play/Spectator.cs new file mode 100644 index 0000000000..ceac6c737f --- /dev/null +++ b/osu.Game/Screens/Play/Spectator.cs @@ -0,0 +1,36 @@ +// 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 JetBrains.Annotations; +using osu.Framework.Allocation; +using osu.Framework.Graphics; +using osu.Game.Graphics.Sprites; +using osu.Game.Users; + +namespace osu.Game.Screens.Play +{ + public class Spectator : OsuScreen + { + private readonly User targetUser; + + public Spectator([NotNull] User targetUser) + { + this.targetUser = targetUser ?? throw new ArgumentNullException(nameof(targetUser)); + } + + [BackgroundDependencyLoader] + private void load() + { + InternalChildren = new Drawable[] + { + new OsuSpriteText + { + Text = $"Watching {targetUser}", + Anchor = Anchor.Centre, + Origin = Anchor.Centre, + }, + }; + } + } +} From 154ccf1b4996401d97a0cf16b0b3d45c17feab1f Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 26 Oct 2020 20:05:11 +0900 Subject: [PATCH 02/55] Expose events from streaming client --- .../Online/Spectator/SpectatorStreamingClient.cs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/osu.Game/Online/Spectator/SpectatorStreamingClient.cs b/osu.Game/Online/Spectator/SpectatorStreamingClient.cs index 5a41316f31..9e554d1d43 100644 --- a/osu.Game/Online/Spectator/SpectatorStreamingClient.cs +++ b/osu.Game/Online/Spectator/SpectatorStreamingClient.cs @@ -64,6 +64,16 @@ namespace osu.Game.Online.Spectator /// public event Action OnNewFrames; + /// + /// Called whenever a user starts a play session. + /// + public event Action OnUserBeganPlaying; + + /// + /// Called whenever a user starts a play session. + /// + public event Action OnUserFinishedPlaying; + [BackgroundDependencyLoader] private void load() { @@ -154,18 +164,24 @@ namespace osu.Game.Online.Spectator if (!playingUsers.Contains(userId)) playingUsers.Add(userId); + OnUserBeganPlaying?.Invoke(userId, state); + return Task.CompletedTask; } Task ISpectatorClient.UserFinishedPlaying(int userId, SpectatorState state) { playingUsers.Remove(userId); + + OnUserFinishedPlaying?.Invoke(userId, state); + return Task.CompletedTask; } Task ISpectatorClient.UserSentFrames(int userId, FrameDataBundle data) { OnNewFrames?.Invoke(userId, data); + return Task.CompletedTask; } From ac4671c594744ede35448e9045736899d2137606 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 26 Oct 2020 20:59:46 +0900 Subject: [PATCH 03/55] Add basic implementation of spectator screen --- osu.Game/Screens/Play/Spectator.cs | 113 +++++++++++++++++++++++++++++ 1 file changed, 113 insertions(+) diff --git a/osu.Game/Screens/Play/Spectator.cs b/osu.Game/Screens/Play/Spectator.cs index ceac6c737f..d73395cc9e 100644 --- a/osu.Game/Screens/Play/Spectator.cs +++ b/osu.Game/Screens/Play/Spectator.cs @@ -2,10 +2,22 @@ // See the LICENCE file in the repository root for full licence text. using System; +using System.Collections.Generic; +using System.Linq; using JetBrains.Annotations; using osu.Framework.Allocation; +using osu.Framework.Bindables; using osu.Framework.Graphics; +using osu.Framework.Screens; +using osu.Game.Beatmaps; using osu.Game.Graphics.Sprites; +using osu.Game.Online.Spectator; +using osu.Game.Replays; +using osu.Game.Rulesets; +using osu.Game.Rulesets.Mods; +using osu.Game.Rulesets.Replays; +using osu.Game.Rulesets.Replays.Types; +using osu.Game.Scoring; using osu.Game.Users; namespace osu.Game.Screens.Play @@ -14,6 +26,26 @@ namespace osu.Game.Screens.Play { private readonly User targetUser; + [Resolved] + private Bindable beatmap { get; set; } + + [Resolved] + private Bindable ruleset { get; set; } + + [Resolved] + private Bindable> mods { get; set; } + + [Resolved] + private SpectatorStreamingClient spectatorStreaming { get; set; } + + [Resolved] + private BeatmapManager beatmaps { get; set; } + + [Resolved] + private RulesetStore rulesets { get; set; } + + private Replay replay; + public Spectator([NotNull] User targetUser) { this.targetUser = targetUser ?? throw new ArgumentNullException(nameof(targetUser)); @@ -31,6 +63,87 @@ namespace osu.Game.Screens.Play Origin = Anchor.Centre, }, }; + + spectatorStreaming.OnUserBeganPlaying += userBeganPlaying; + spectatorStreaming.OnUserFinishedPlaying += userFinishedPlaying; + spectatorStreaming.OnNewFrames += userSentFrames; + + spectatorStreaming.WatchUser((int)targetUser.Id); + } + + private void userSentFrames(int userId, FrameDataBundle data) + { + if (userId != targetUser.Id) + return; + + var rulesetInstance = ruleset.Value.CreateInstance(); + + foreach (var frame in data.Frames) + { + IConvertibleReplayFrame convertibleFrame = rulesetInstance.CreateConvertibleReplayFrame(); + convertibleFrame.FromLegacy(frame, beatmap.Value.Beatmap, null); + + var convertedFrame = (ReplayFrame)convertibleFrame; + convertedFrame.Time = frame.Time; + + replay.Frames.Add(convertedFrame); + } + } + + private void userBeganPlaying(int userId, SpectatorState state) + { + if (userId != targetUser.Id) + return; + + replay ??= new Replay(); + + var resolvedRuleset = rulesets.AvailableRulesets.FirstOrDefault(r => r.ID == state.RulesetID)?.CreateInstance(); + + // ruleset not available + if (resolvedRuleset == null) + return; + + var resolvedBeatmap = beatmaps.QueryBeatmap(b => b.OnlineBeatmapID == state.BeatmapID); + + if (resolvedBeatmap == null) + return; + + var scoreInfo = new ScoreInfo + { + Beatmap = resolvedBeatmap, + Mods = state.Mods.Select(m => m.ToMod(resolvedRuleset)).ToArray(), + Ruleset = resolvedRuleset.RulesetInfo, + }; + + this.MakeCurrent(); + + ruleset.Value = resolvedRuleset.RulesetInfo; + beatmap.Value = beatmaps.GetWorkingBeatmap(resolvedBeatmap); + + this.Push(new ReplayPlayerLoader(new Score + { + ScoreInfo = scoreInfo, + Replay = replay, + })); + } + + private void userFinishedPlaying(int userId, SpectatorState state) + { + // todo: handle this in some way? + } + + protected override void Dispose(bool isDisposing) + { + base.Dispose(isDisposing); + + if (spectatorStreaming != null) + { + spectatorStreaming.OnUserBeganPlaying -= userBeganPlaying; + spectatorStreaming.OnUserFinishedPlaying -= userFinishedPlaying; + spectatorStreaming.OnNewFrames -= userSentFrames; + + spectatorStreaming.StopWatchingUser((int)targetUser.Id); + } } } } From 82a27c73a0077412ee1519c63c0b6d9323810e35 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 26 Oct 2020 21:17:12 +0900 Subject: [PATCH 04/55] Create basic testing setup --- .../Visual/Gameplay/TestSceneSpectator.cs | 29 +++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs index 25c30dbc5a..75ca970c62 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs @@ -2,6 +2,9 @@ // See the LICENCE file in the repository root for full licence text. using NUnit.Framework; +using osu.Framework.Allocation; +using osu.Game.Online.Spectator; +using osu.Game.Replays.Legacy; using osu.Game.Screens.Play; using osu.Game.Users; @@ -9,12 +12,34 @@ namespace osu.Game.Tests.Visual.Gameplay { public class TestSceneSpectator : ScreenTestScene { - private readonly User user = new User { Id = 1234, Username = "Test user" }; + [Cached(typeof(SpectatorStreamingClient))] + private TestSpectatorStreamingClient testSpectatorStreamingClient = new TestSpectatorStreamingClient(); [Test] public void TestSpectating() { - AddStep("load screen", () => LoadScreen(new Spectator(user))); + AddStep("load screen", () => LoadScreen(new Spectator(testSpectatorStreamingClient.StreamingUser))); + AddStep("start play", () => testSpectatorStreamingClient.StartPlay()); + AddStep("send frames", () => testSpectatorStreamingClient.SendFrames()); + } + + internal class TestSpectatorStreamingClient : SpectatorStreamingClient + { + public readonly User StreamingUser = new User { Id = 1234, Username = "Test user" }; + + public void StartPlay() + { + ((ISpectatorClient)this).UserBeganPlaying((int)StreamingUser.Id, new SpectatorState()); + } + + public void SendFrames() + { + ((ISpectatorClient)this).UserSentFrames((int)StreamingUser.Id, new FrameDataBundle(new[] + { + // todo: populate more frames + new LegacyReplayFrame(0, 0, 0, ReplayButtonState.Left1) + })); + } } } } From 9bb2cff8a54a30e83a3ca801d47e84fd66e0288d Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 26 Oct 2020 21:22:01 +0900 Subject: [PATCH 05/55] Convey actual beatmap and ruleset for full testing setup --- .../Visual/Gameplay/TestSceneSpectator.cs | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs index 75ca970c62..1c73427686 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs @@ -1,8 +1,10 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. +using System.Linq; using NUnit.Framework; using osu.Framework.Allocation; +using osu.Game.Beatmaps; using osu.Game.Online.Spectator; using osu.Game.Replays.Legacy; using osu.Game.Screens.Play; @@ -15,9 +17,13 @@ namespace osu.Game.Tests.Visual.Gameplay [Cached(typeof(SpectatorStreamingClient))] private TestSpectatorStreamingClient testSpectatorStreamingClient = new TestSpectatorStreamingClient(); + [Resolved] + private OsuGameBase game { get; set; } + [Test] public void TestSpectating() { + AddStep("add streaming client", () => Add(testSpectatorStreamingClient)); AddStep("load screen", () => LoadScreen(new Spectator(testSpectatorStreamingClient.StreamingUser))); AddStep("start play", () => testSpectatorStreamingClient.StartPlay()); AddStep("send frames", () => testSpectatorStreamingClient.SendFrames()); @@ -25,11 +31,18 @@ namespace osu.Game.Tests.Visual.Gameplay internal class TestSpectatorStreamingClient : SpectatorStreamingClient { + [Resolved] + private BeatmapManager beatmaps { get; set; } + public readonly User StreamingUser = new User { Id = 1234, Username = "Test user" }; public void StartPlay() { - ((ISpectatorClient)this).UserBeganPlaying((int)StreamingUser.Id, new SpectatorState()); + ((ISpectatorClient)this).UserBeganPlaying((int)StreamingUser.Id, new SpectatorState + { + BeatmapID = beatmaps.GetAllUsableBeatmapSets().First().Beatmaps.First().OnlineBeatmapID, + RulesetID = 0, + }); } public void SendFrames() From 67f6d52e35a861c97647139dc16b2020fa3c6e38 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 26 Oct 2020 21:27:05 +0900 Subject: [PATCH 06/55] Setup tests --- .../Visual/Gameplay/TestSceneSpectator.cs | 26 ++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs index 1c73427686..49fc81752c 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs @@ -21,7 +21,7 @@ namespace osu.Game.Tests.Visual.Gameplay private OsuGameBase game { get; set; } [Test] - public void TestSpectating() + public void TestBasicSpectatingFlow() { AddStep("add streaming client", () => Add(testSpectatorStreamingClient)); AddStep("load screen", () => LoadScreen(new Spectator(testSpectatorStreamingClient.StreamingUser))); @@ -29,6 +29,30 @@ namespace osu.Game.Tests.Visual.Gameplay AddStep("send frames", () => testSpectatorStreamingClient.SendFrames()); } + [Test] + public void TestSpectatingDuringGameplay() + { + // should seek immediately to available frames + } + + [Test] + public void TestHostStartsPlayingWhileAlreadyWatching() + { + // should restart either immediately or after running out of frames + } + + [Test] + public void TestHostFails() + { + // should replay until running out of frames then fail + } + + [Test] + public void TestStopWatchingDuringPlay() + { + // should immediately exit and unbind from streaming client + } + internal class TestSpectatorStreamingClient : SpectatorStreamingClient { [Resolved] From 593b0a3adafc9b6683e351ab3ce7db6f060aa57b Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 26 Oct 2020 21:45:37 +0900 Subject: [PATCH 07/55] Setup tests to run headless, add basic pass support --- .../Visual/Gameplay/TestSceneSpectator.cs | 73 ++++++++++++++++++- 1 file changed, 70 insertions(+), 3 deletions(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs index 49fc81752c..006d64491a 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs @@ -4,10 +4,13 @@ using System.Linq; using NUnit.Framework; using osu.Framework.Allocation; +using osu.Framework.Screens; +using osu.Framework.Testing; using osu.Game.Beatmaps; using osu.Game.Online.Spectator; using osu.Game.Replays.Legacy; using osu.Game.Screens.Play; +using osu.Game.Tests.Beatmaps.IO; using osu.Game.Users; namespace osu.Game.Tests.Visual.Gameplay @@ -17,14 +20,28 @@ namespace osu.Game.Tests.Visual.Gameplay [Cached(typeof(SpectatorStreamingClient))] private TestSpectatorStreamingClient testSpectatorStreamingClient = new TestSpectatorStreamingClient(); + private Spectator spectatorScreen; + [Resolved] private OsuGameBase game { get; set; } + public override void SetUpSteps() + { + base.SetUpSteps(); + + AddStep("import beatmap", () => ImportBeatmapTest.LoadOszIntoOsu(game, virtualTrack: true).Wait()); + + AddStep("add streaming client", () => + { + Remove(testSpectatorStreamingClient); + Add(testSpectatorStreamingClient); + }); + } + [Test] public void TestBasicSpectatingFlow() { - AddStep("add streaming client", () => Add(testSpectatorStreamingClient)); - AddStep("load screen", () => LoadScreen(new Spectator(testSpectatorStreamingClient.StreamingUser))); + beginSpectating(); AddStep("start play", () => testSpectatorStreamingClient.StartPlay()); AddStep("send frames", () => testSpectatorStreamingClient.SendFrames()); } @@ -32,27 +49,68 @@ namespace osu.Game.Tests.Visual.Gameplay [Test] public void TestSpectatingDuringGameplay() { + AddStep("start play", () => testSpectatorStreamingClient.StartPlay()); + AddStep("send frames", () => testSpectatorStreamingClient.SendFrames()); + // should seek immediately to available frames + beginSpectating(); } [Test] public void TestHostStartsPlayingWhileAlreadyWatching() { + beginSpectating(); + + AddStep("start play", () => testSpectatorStreamingClient.StartPlay()); + AddStep("send frames", () => testSpectatorStreamingClient.SendFrames()); + + AddStep("start play", () => testSpectatorStreamingClient.StartPlay()); + AddStep("send frames", () => testSpectatorStreamingClient.SendFrames()); // should restart either immediately or after running out of frames } [Test] public void TestHostFails() { + beginSpectating(); + + AddStep("start play", () => testSpectatorStreamingClient.StartPlay()); + AddStep("send frames", () => testSpectatorStreamingClient.SendFrames()); + // todo: send fail state + // should replay until running out of frames then fail } [Test] public void TestStopWatchingDuringPlay() { + beginSpectating(); + + AddStep("start play", () => testSpectatorStreamingClient.StartPlay()); + AddStep("send frames", () => testSpectatorStreamingClient.SendFrames()); + + AddUntilStep("wait for player", () => Stack.CurrentScreen is Player); + // should immediately exit and unbind from streaming client + AddStep("stop spectating", () => (Stack.CurrentScreen as Player)?.Exit()); + + AddUntilStep("spectating stopped", () => spectatorScreen.GetParentScreen() == null); } + [Test] + public void TestWatchingBeatmapThatDoesntExistLocally() + { + beginSpectating(); + + AddStep("start play", () => testSpectatorStreamingClient.StartPlay()); + AddStep("send frames", () => testSpectatorStreamingClient.SendFrames()); + + // player should never arrive. + } + + private void beginSpectating() => + AddStep("load screen", () => LoadScreen(spectatorScreen = new Spectator(testSpectatorStreamingClient.StreamingUser))); + internal class TestSpectatorStreamingClient : SpectatorStreamingClient { [Resolved] @@ -64,7 +122,16 @@ namespace osu.Game.Tests.Visual.Gameplay { ((ISpectatorClient)this).UserBeganPlaying((int)StreamingUser.Id, new SpectatorState { - BeatmapID = beatmaps.GetAllUsableBeatmapSets().First().Beatmaps.First().OnlineBeatmapID, + BeatmapID = beatmaps.GetAllUsableBeatmapSets().First().Beatmaps.First(b => b.RulesetID == 0).OnlineBeatmapID, + RulesetID = 0, + }); + } + + public void EndPlay() + { + ((ISpectatorClient)this).UserFinishedPlaying((int)StreamingUser.Id, new SpectatorState + { + BeatmapID = beatmaps.GetAllUsableBeatmapSets().First().Beatmaps.First(b => b.RulesetID == 0).OnlineBeatmapID, RulesetID = 0, }); } From 400542bc0ba76826341c549cd18db1f12efa1092 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 27 Oct 2020 14:47:15 +0900 Subject: [PATCH 08/55] Ensure frames arrive --- osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs | 9 +++++++++ osu.Game/Rulesets/Replays/FramedReplayInputHandler.cs | 2 +- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs index 006d64491a..533e535bc6 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs @@ -9,6 +9,8 @@ using osu.Framework.Testing; using osu.Game.Beatmaps; using osu.Game.Online.Spectator; using osu.Game.Replays.Legacy; +using osu.Game.Rulesets.Osu; +using osu.Game.Rulesets.Osu.Replays; using osu.Game.Screens.Play; using osu.Game.Tests.Beatmaps.IO; using osu.Game.Users; @@ -38,12 +40,19 @@ namespace osu.Game.Tests.Visual.Gameplay }); } + private OsuFramedReplayInputHandler replayHandler => + (OsuFramedReplayInputHandler)Stack.ChildrenOfType().First().ReplayInputHandler; + [Test] public void TestBasicSpectatingFlow() { beginSpectating(); AddStep("start play", () => testSpectatorStreamingClient.StartPlay()); AddStep("send frames", () => testSpectatorStreamingClient.SendFrames()); + + AddUntilStep("wait for player", () => Stack.CurrentScreen is Player); + + AddAssert("ensure frames arrived", () => replayHandler.HasFrames); } [Test] diff --git a/osu.Game/Rulesets/Replays/FramedReplayInputHandler.cs b/osu.Game/Rulesets/Replays/FramedReplayInputHandler.cs index cf5c88b8fd..62b24f6b55 100644 --- a/osu.Game/Rulesets/Replays/FramedReplayInputHandler.cs +++ b/osu.Game/Rulesets/Replays/FramedReplayInputHandler.cs @@ -82,7 +82,7 @@ namespace osu.Game.Rulesets.Replays /// public bool FrameAccuratePlayback = false; - protected bool HasFrames => Frames.Count > 0; + public bool HasFrames => Frames.Count > 0; private bool inImportantSection { From 9bac8f37922276c7b5808383145436884f845819 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 27 Oct 2020 14:57:00 +0900 Subject: [PATCH 09/55] Add null check on replay as safety measure --- osu.Game/Screens/Play/Spectator.cs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/osu.Game/Screens/Play/Spectator.cs b/osu.Game/Screens/Play/Spectator.cs index d73395cc9e..24334af8d8 100644 --- a/osu.Game/Screens/Play/Spectator.cs +++ b/osu.Game/Screens/Play/Spectator.cs @@ -63,7 +63,11 @@ namespace osu.Game.Screens.Play Origin = Anchor.Centre, }, }; + } + protected override void LoadComplete() + { + base.LoadComplete(); spectatorStreaming.OnUserBeganPlaying += userBeganPlaying; spectatorStreaming.OnUserFinishedPlaying += userFinishedPlaying; spectatorStreaming.OnNewFrames += userSentFrames; @@ -76,6 +80,11 @@ namespace osu.Game.Screens.Play if (userId != targetUser.Id) return; + // this should never happen as the server sends the user's state on watching, + // but is here as a safety measure. + if (replay == null) + return; + var rulesetInstance = ruleset.Value.CreateInstance(); foreach (var frame in data.Frames) From c1e7cd6e4774cbae012a6bc63f1e73e623427966 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 27 Oct 2020 14:57:23 +0900 Subject: [PATCH 10/55] Stop replay playback when frames are starved --- .../Visual/Gameplay/TestSceneSpectator.cs | 31 ++++++++++++++----- .../Spectator/SpectatorStreamingClient.cs | 2 +- 2 files changed, 24 insertions(+), 9 deletions(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs index 533e535bc6..8c03de8c13 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs @@ -43,6 +43,8 @@ namespace osu.Game.Tests.Visual.Gameplay private OsuFramedReplayInputHandler replayHandler => (OsuFramedReplayInputHandler)Stack.ChildrenOfType().First().ReplayInputHandler; + private Player player => Stack.CurrentScreen as Player; + [Test] public void TestBasicSpectatingFlow() { @@ -53,6 +55,9 @@ namespace osu.Game.Tests.Visual.Gameplay AddUntilStep("wait for player", () => Stack.CurrentScreen is Player); AddAssert("ensure frames arrived", () => replayHandler.HasFrames); + + AddUntilStep("wait for frame starvation", () => replayHandler.NextFrame == null); + AddAssert("game is paused", () => !player.ChildrenOfType().First().GameplayClock.IsRunning); } [Test] @@ -127,14 +132,7 @@ namespace osu.Game.Tests.Visual.Gameplay public readonly User StreamingUser = new User { Id = 1234, Username = "Test user" }; - public void StartPlay() - { - ((ISpectatorClient)this).UserBeganPlaying((int)StreamingUser.Id, new SpectatorState - { - BeatmapID = beatmaps.GetAllUsableBeatmapSets().First().Beatmaps.First(b => b.RulesetID == 0).OnlineBeatmapID, - RulesetID = 0, - }); - } + public void StartPlay() => sendState(); public void EndPlay() { @@ -153,6 +151,23 @@ namespace osu.Game.Tests.Visual.Gameplay new LegacyReplayFrame(0, 0, 0, ReplayButtonState.Left1) })); } + + public override void WatchUser(int userId) + { + // usually the server would do this. + sendState(); + + base.WatchUser(userId); + } + + private void sendState() + { + ((ISpectatorClient)this).UserBeganPlaying((int)StreamingUser.Id, new SpectatorState + { + BeatmapID = beatmaps.GetAllUsableBeatmapSets().First().Beatmaps.First(b => b.RulesetID == 0).OnlineBeatmapID, + RulesetID = 0, + }); + } } } } diff --git a/osu.Game/Online/Spectator/SpectatorStreamingClient.cs b/osu.Game/Online/Spectator/SpectatorStreamingClient.cs index 9e554d1d43..481c94e6f3 100644 --- a/osu.Game/Online/Spectator/SpectatorStreamingClient.cs +++ b/osu.Game/Online/Spectator/SpectatorStreamingClient.cs @@ -227,7 +227,7 @@ namespace osu.Game.Online.Spectator connection.SendAsync(nameof(ISpectatorServer.EndPlaySession), currentState); } - public void WatchUser(int userId) + public virtual void WatchUser(int userId) { if (watchingUsers.Contains(userId)) return; From b737a8bf6e821d7bdc3c1046d406164b83cae893 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 27 Oct 2020 16:27:15 +0900 Subject: [PATCH 11/55] Add field to Replay denoting whether the full replay have been received or not --- osu.Game/Replays/Replay.cs | 6 ++++++ osu.Game/Screens/Play/Spectator.cs | 6 ++++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/osu.Game/Replays/Replay.cs b/osu.Game/Replays/Replay.cs index 31d2ed0d70..5430915394 100644 --- a/osu.Game/Replays/Replay.cs +++ b/osu.Game/Replays/Replay.cs @@ -8,6 +8,12 @@ namespace osu.Game.Replays { public class Replay { + /// + /// Whether all frames for this replay have been received. + /// If false, gameplay would be paused to wait for further data, for instance. + /// + public bool HasReceivedAllFrames = true; + public List Frames = new List(); } } diff --git a/osu.Game/Screens/Play/Spectator.cs b/osu.Game/Screens/Play/Spectator.cs index 24334af8d8..dcfd7e829a 100644 --- a/osu.Game/Screens/Play/Spectator.cs +++ b/osu.Game/Screens/Play/Spectator.cs @@ -104,7 +104,7 @@ namespace osu.Game.Screens.Play if (userId != targetUser.Id) return; - replay ??= new Replay(); + replay ??= new Replay { HasReceivedAllFrames = false }; var resolvedRuleset = rulesets.AvailableRulesets.FirstOrDefault(r => r.ID == state.RulesetID)?.CreateInstance(); @@ -138,7 +138,9 @@ namespace osu.Game.Screens.Play private void userFinishedPlaying(int userId, SpectatorState state) { - // todo: handle this in some way? + if (replay == null) return; + + replay.HasReceivedAllFrames = true; } protected override void Dispose(bool isDisposing) From 4dba96e189c2af01d3c947d43d66ddf17c066d34 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 27 Oct 2020 16:28:11 +0900 Subject: [PATCH 12/55] Add more useful frame sending logic to tests --- .../Visual/Gameplay/TestSceneSpectator.cs | 47 +++++++++++-------- 1 file changed, 27 insertions(+), 20 deletions(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs index 8c03de8c13..f33a6876dd 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs @@ -1,16 +1,19 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. +using System.Collections.Generic; using System.Linq; using NUnit.Framework; using osu.Framework.Allocation; using osu.Framework.Screens; using osu.Framework.Testing; +using osu.Framework.Utils; using osu.Game.Beatmaps; using osu.Game.Online.Spectator; using osu.Game.Replays.Legacy; using osu.Game.Rulesets.Osu; using osu.Game.Rulesets.Osu.Replays; +using osu.Game.Rulesets.UI; using osu.Game.Screens.Play; using osu.Game.Tests.Beatmaps.IO; using osu.Game.Users; @@ -50,22 +53,25 @@ namespace osu.Game.Tests.Visual.Gameplay { beginSpectating(); AddStep("start play", () => testSpectatorStreamingClient.StartPlay()); - AddStep("send frames", () => testSpectatorStreamingClient.SendFrames()); - + sendFrames(); AddUntilStep("wait for player", () => Stack.CurrentScreen is Player); AddAssert("ensure frames arrived", () => replayHandler.HasFrames); AddUntilStep("wait for frame starvation", () => replayHandler.NextFrame == null); - AddAssert("game is paused", () => !player.ChildrenOfType().First().GameplayClock.IsRunning); + AddAssert("game is paused", () => player.ChildrenOfType().First().IsPaused.Value); + } + + private void sendFrames(int count = 10) + { + AddStep("send frames", () => testSpectatorStreamingClient.SendFrames(count)); } [Test] public void TestSpectatingDuringGameplay() { AddStep("start play", () => testSpectatorStreamingClient.StartPlay()); - AddStep("send frames", () => testSpectatorStreamingClient.SendFrames()); - + sendFrames(); // should seek immediately to available frames beginSpectating(); } @@ -76,11 +82,9 @@ namespace osu.Game.Tests.Visual.Gameplay beginSpectating(); AddStep("start play", () => testSpectatorStreamingClient.StartPlay()); - AddStep("send frames", () => testSpectatorStreamingClient.SendFrames()); - + sendFrames(); AddStep("start play", () => testSpectatorStreamingClient.StartPlay()); - AddStep("send frames", () => testSpectatorStreamingClient.SendFrames()); - // should restart either immediately or after running out of frames + sendFrames(); } [Test] @@ -89,8 +93,7 @@ namespace osu.Game.Tests.Visual.Gameplay beginSpectating(); AddStep("start play", () => testSpectatorStreamingClient.StartPlay()); - AddStep("send frames", () => testSpectatorStreamingClient.SendFrames()); - // todo: send fail state + sendFrames(); // should replay until running out of frames then fail } @@ -101,8 +104,7 @@ namespace osu.Game.Tests.Visual.Gameplay beginSpectating(); AddStep("start play", () => testSpectatorStreamingClient.StartPlay()); - AddStep("send frames", () => testSpectatorStreamingClient.SendFrames()); - + sendFrames(); AddUntilStep("wait for player", () => Stack.CurrentScreen is Player); // should immediately exit and unbind from streaming client @@ -117,8 +119,7 @@ namespace osu.Game.Tests.Visual.Gameplay beginSpectating(); AddStep("start play", () => testSpectatorStreamingClient.StartPlay()); - AddStep("send frames", () => testSpectatorStreamingClient.SendFrames()); - + sendFrames(); // player should never arrive. } @@ -143,13 +144,19 @@ namespace osu.Game.Tests.Visual.Gameplay }); } - public void SendFrames() + public void SendFrames(int count) { - ((ISpectatorClient)this).UserSentFrames((int)StreamingUser.Id, new FrameDataBundle(new[] + var frames = new List(); + + for (int i = 0; i < count; i++) { - // todo: populate more frames - new LegacyReplayFrame(0, 0, 0, ReplayButtonState.Left1) - })); + frames.Add(new LegacyReplayFrame(i * 100, RNG.Next(0, 512), RNG.Next(0, 512), ReplayButtonState.Left1)); + } + + frames.Add(new LegacyReplayFrame(count * 100, 0, 0, ReplayButtonState.None)); + + var bundle = new FrameDataBundle(frames); + ((ISpectatorClient)this).UserSentFrames((int)StreamingUser.Id, bundle); } public override void WatchUser(int userId) From 63131d46aaeb9794859fc52796bd0420bb5999fe Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 27 Oct 2020 17:10:48 +0900 Subject: [PATCH 13/55] Send initial spectator state more correctly in test component --- .../Visual/Gameplay/TestSceneSpectator.cs | 27 ++++++++++++------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs index f33a6876dd..e95f7a50a4 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs @@ -51,7 +51,7 @@ namespace osu.Game.Tests.Visual.Gameplay [Test] public void TestBasicSpectatingFlow() { - beginSpectating(); + loadSpectatingScreen(); AddStep("start play", () => testSpectatorStreamingClient.StartPlay()); sendFrames(); AddUntilStep("wait for player", () => Stack.CurrentScreen is Player); @@ -73,13 +73,13 @@ namespace osu.Game.Tests.Visual.Gameplay AddStep("start play", () => testSpectatorStreamingClient.StartPlay()); sendFrames(); // should seek immediately to available frames - beginSpectating(); + loadSpectatingScreen(); } [Test] public void TestHostStartsPlayingWhileAlreadyWatching() { - beginSpectating(); + loadSpectatingScreen(); AddStep("start play", () => testSpectatorStreamingClient.StartPlay()); sendFrames(); @@ -90,7 +90,7 @@ namespace osu.Game.Tests.Visual.Gameplay [Test] public void TestHostFails() { - beginSpectating(); + loadSpectatingScreen(); AddStep("start play", () => testSpectatorStreamingClient.StartPlay()); sendFrames(); @@ -101,7 +101,7 @@ namespace osu.Game.Tests.Visual.Gameplay [Test] public void TestStopWatchingDuringPlay() { - beginSpectating(); + loadSpectatingScreen(); AddStep("start play", () => testSpectatorStreamingClient.StartPlay()); sendFrames(); @@ -116,14 +116,14 @@ namespace osu.Game.Tests.Visual.Gameplay [Test] public void TestWatchingBeatmapThatDoesntExistLocally() { - beginSpectating(); + loadSpectatingScreen(); AddStep("start play", () => testSpectatorStreamingClient.StartPlay()); sendFrames(); // player should never arrive. } - private void beginSpectating() => + private void loadSpectatingScreen() => AddStep("load screen", () => LoadScreen(spectatorScreen = new Spectator(testSpectatorStreamingClient.StreamingUser))); internal class TestSpectatorStreamingClient : SpectatorStreamingClient @@ -144,6 +144,8 @@ namespace osu.Game.Tests.Visual.Gameplay }); } + private bool sentState; + public void SendFrames(int count) { var frames = new List(); @@ -157,18 +159,25 @@ namespace osu.Game.Tests.Visual.Gameplay var bundle = new FrameDataBundle(frames); ((ISpectatorClient)this).UserSentFrames((int)StreamingUser.Id, bundle); + + if (!sentState) + sendState(); } public override void WatchUser(int userId) { - // usually the server would do this. - sendState(); + if (sentState) + { + // usually the server would do this. + sendState(); + } base.WatchUser(userId); } private void sendState() { + sentState = true; ((ISpectatorClient)this).UserBeganPlaying((int)StreamingUser.Id, new SpectatorState { BeatmapID = beatmaps.GetAllUsableBeatmapSets().First().Beatmaps.First(b => b.RulesetID == 0).OnlineBeatmapID, From 3ec3321a3ded49588c927c02dee520a46ffd7852 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 27 Oct 2020 17:11:07 +0900 Subject: [PATCH 14/55] Add missing space --- osu.Game/Screens/Play/Spectator.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/osu.Game/Screens/Play/Spectator.cs b/osu.Game/Screens/Play/Spectator.cs index dcfd7e829a..522a636dfb 100644 --- a/osu.Game/Screens/Play/Spectator.cs +++ b/osu.Game/Screens/Play/Spectator.cs @@ -68,6 +68,7 @@ namespace osu.Game.Screens.Play protected override void LoadComplete() { base.LoadComplete(); + spectatorStreaming.OnUserBeganPlaying += userBeganPlaying; spectatorStreaming.OnUserFinishedPlaying += userFinishedPlaying; spectatorStreaming.OnNewFrames += userSentFrames; From 9e6b0a42ec4795eb3878e47946370d900aa8d453 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 27 Oct 2020 16:28:50 +0900 Subject: [PATCH 15/55] Allow FrameStabilityContainer to handle waiting-for-data state better (and pause outwards) --- .../Replays/FramedReplayInputHandler.cs | 28 +++++++++++++++++-- .../Rulesets/UI/FrameStabilityContainer.cs | 3 +- 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/osu.Game/Rulesets/Replays/FramedReplayInputHandler.cs b/osu.Game/Rulesets/Replays/FramedReplayInputHandler.cs index 62b24f6b55..ddf85b7300 100644 --- a/osu.Game/Rulesets/Replays/FramedReplayInputHandler.cs +++ b/osu.Game/Rulesets/Replays/FramedReplayInputHandler.cs @@ -124,20 +124,42 @@ namespace osu.Game.Rulesets.Replays if (HasFrames) { + var next = NextFrame; + // check if the next frame is valid for the current playback direction. // validity is if the next frame is equal or "earlier" - var compare = time.CompareTo(NextFrame?.Time); + var compare = time.CompareTo(next?.Time); - if (compare == 0 || compare == currentDirection) + if (next != null && (compare == 0 || compare == currentDirection)) { if (advanceFrame()) return CurrentTime = CurrentFrame.Time; } else { - // if we didn't change frames, we need to ensure we are allowed to run frames in between, else return null. + // this is the case where the frame can't be advanced (in the replay). + // even so, we may be able to move the clock forward due to being at the end of the replay or + // in a section where replay accuracy doesn't matter. + + // important section is always respected to block the update loop. if (inImportantSection) return null; + + if (next == null) + { + // in the case we have no more frames and haven't received the full replay, block. + if (!replay.HasReceivedAllFrames) + return null; + + // else allow play to end. + } + else if (next.Time.CompareTo(time) != currentDirection) + { + // in the case we have more frames, block if the next frame's time is less than the current time. + return null; + } + + // if we didn't change frames, we need to ensure we are allowed to run frames in between, else return null. } } diff --git a/osu.Game/Rulesets/UI/FrameStabilityContainer.cs b/osu.Game/Rulesets/UI/FrameStabilityContainer.cs index 28b7975a89..fb6c7d4c17 100644 --- a/osu.Game/Rulesets/UI/FrameStabilityContainer.cs +++ b/osu.Game/Rulesets/UI/FrameStabilityContainer.cs @@ -96,6 +96,7 @@ namespace osu.Game.Rulesets.UI public override bool UpdateSubTree() { requireMoreUpdateLoops = true; + validState = !frameStableClock.IsPaused.Value; int loops = 0; @@ -191,7 +192,7 @@ namespace osu.Game.Rulesets.UI finally { if (newProposedTime != manualClock.CurrentTime) - direction = newProposedTime > manualClock.CurrentTime ? 1 : -1; + direction = newProposedTime >= manualClock.CurrentTime ? 1 : -1; manualClock.CurrentTime = newProposedTime; manualClock.Rate = Math.Abs(parentGameplayClock.Rate) * direction; From 851d45d2ebea669f18243e924956a6ab7e0ea695 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 27 Oct 2020 18:13:45 +0900 Subject: [PATCH 16/55] Add sane pausing logic --- osu.Game/Rulesets/UI/FrameStabilityContainer.cs | 12 ++++++++++++ osu.Game/Rulesets/UI/IFrameStableClock.cs | 2 ++ osu.Game/Screens/Play/Player.cs | 8 ++++++++ 3 files changed, 22 insertions(+) diff --git a/osu.Game/Rulesets/UI/FrameStabilityContainer.cs b/osu.Game/Rulesets/UI/FrameStabilityContainer.cs index fb6c7d4c17..987dca7073 100644 --- a/osu.Game/Rulesets/UI/FrameStabilityContainer.cs +++ b/osu.Game/Rulesets/UI/FrameStabilityContainer.cs @@ -101,6 +101,13 @@ namespace osu.Game.Rulesets.UI int loops = 0; + 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; + } + while (validState && requireMoreUpdateLoops && loops++ < MaxCatchUpFrames) { updateClock(); @@ -203,6 +210,7 @@ namespace osu.Game.Rulesets.UI 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 @@ -231,6 +239,8 @@ namespace osu.Game.Rulesets.UI public readonly Bindable IsCatchingUp = new Bindable(); + public readonly Bindable WaitingOnFrames = new Bindable(); + public override IEnumerable> NonGameplayAdjustments => ParentGameplayClock?.NonGameplayAdjustments ?? Enumerable.Empty>(); public FrameStabilityClock(FramedClock underlyingClock) @@ -239,6 +249,8 @@ namespace osu.Game.Rulesets.UI } IBindable IFrameStableClock.IsCatchingUp => IsCatchingUp; + + IBindable IFrameStableClock.WaitingOnFrames => WaitingOnFrames; } } } diff --git a/osu.Game/Rulesets/UI/IFrameStableClock.cs b/osu.Game/Rulesets/UI/IFrameStableClock.cs index d888eefdc6..c1d8733d26 100644 --- a/osu.Game/Rulesets/UI/IFrameStableClock.cs +++ b/osu.Game/Rulesets/UI/IFrameStableClock.cs @@ -9,5 +9,7 @@ namespace osu.Game.Rulesets.UI public interface IFrameStableClock : IFrameBasedClock { IBindable IsCatchingUp { get; } + + IBindable WaitingOnFrames { get; } } } diff --git a/osu.Game/Screens/Play/Player.cs b/osu.Game/Screens/Play/Player.cs index 3c0c643413..3e79ea0840 100644 --- a/osu.Game/Screens/Play/Player.cs +++ b/osu.Game/Screens/Play/Player.cs @@ -238,6 +238,14 @@ namespace osu.Game.Screens.Play skipOverlay.Hide(); } + DrawableRuleset.FrameStableClock.WaitingOnFrames.BindValueChanged(waiting => + { + if (waiting.NewValue) + GameplayClockContainer.Stop(); + else + GameplayClockContainer.Start(); + }); + DrawableRuleset.IsPaused.BindValueChanged(paused => { updateGameplayState(); From d4467d20a2047020e3c59282e8882d89a05cf48d Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 27 Oct 2020 18:13:58 +0900 Subject: [PATCH 17/55] Allow tests to continue sending frames from point they left off --- .../Visual/Gameplay/TestSceneSpectator.cs | 33 ++++++++++++++----- 1 file changed, 24 insertions(+), 9 deletions(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs index e95f7a50a4..98bd90b3b5 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs @@ -30,10 +30,14 @@ namespace osu.Game.Tests.Visual.Gameplay [Resolved] private OsuGameBase game { get; set; } + private int nextFrame = 0; + public override void SetUpSteps() { base.SetUpSteps(); + AddStep("reset sent frames", () => nextFrame = 0); + AddStep("import beatmap", () => ImportBeatmapTest.LoadOszIntoOsu(game, virtualTrack: true).Wait()); AddStep("add streaming client", () => @@ -53,18 +57,20 @@ namespace osu.Game.Tests.Visual.Gameplay { loadSpectatingScreen(); AddStep("start play", () => testSpectatorStreamingClient.StartPlay()); + sendFrames(); + AddUntilStep("wait for player", () => Stack.CurrentScreen is Player); AddAssert("ensure frames arrived", () => replayHandler.HasFrames); AddUntilStep("wait for frame starvation", () => replayHandler.NextFrame == null); AddAssert("game is paused", () => player.ChildrenOfType().First().IsPaused.Value); - } + sendFrames(); + AddUntilStep("game resumed", () => !player.ChildrenOfType().First().IsPaused.Value); - private void sendFrames(int count = 10) - { - AddStep("send frames", () => testSpectatorStreamingClient.SendFrames(count)); + AddUntilStep("wait for frame starvation", () => replayHandler.NextFrame == null); + AddAssert("game is paused", () => player.ChildrenOfType().First().IsPaused.Value); } [Test] @@ -123,6 +129,15 @@ namespace osu.Game.Tests.Visual.Gameplay // player should never arrive. } + private void sendFrames(int count = 10) + { + AddStep("send frames", () => + { + testSpectatorStreamingClient.SendFrames(nextFrame, count); + nextFrame += count; + }); + } + private void loadSpectatingScreen() => AddStep("load screen", () => LoadScreen(spectatorScreen = new Spectator(testSpectatorStreamingClient.StreamingUser))); @@ -146,16 +161,16 @@ namespace osu.Game.Tests.Visual.Gameplay private bool sentState; - public void SendFrames(int count) + public void SendFrames(int index, int count) { var frames = new List(); - for (int i = 0; i < count; i++) + for (int i = index; i < index + count; i++) { - frames.Add(new LegacyReplayFrame(i * 100, RNG.Next(0, 512), RNG.Next(0, 512), ReplayButtonState.Left1)); - } + var buttonState = i == index + count - 1 ? ReplayButtonState.None : ReplayButtonState.Left1; - frames.Add(new LegacyReplayFrame(count * 100, 0, 0, ReplayButtonState.None)); + frames.Add(new LegacyReplayFrame(i * 100, RNG.Next(0, 512), RNG.Next(0, 512), buttonState)); + } var bundle = new FrameDataBundle(frames); ((ISpectatorClient)this).UserSentFrames((int)StreamingUser.Id, bundle); From b3d793a505d919902710673de4e1947e228e43f8 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 27 Oct 2020 18:28:49 +0900 Subject: [PATCH 18/55] Fix gameplay proceeding when no frames have been received yet --- .../Visual/Gameplay/TestSceneSpectator.cs | 15 +++++++++++++++ .../Rulesets/Replays/FramedReplayInputHandler.cs | 6 ++++++ 2 files changed, 21 insertions(+) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs index 98bd90b3b5..14b0ca4d33 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs @@ -56,6 +56,9 @@ namespace osu.Game.Tests.Visual.Gameplay public void TestBasicSpectatingFlow() { loadSpectatingScreen(); + + AddAssert("screen hasn't changed", () => Stack.CurrentScreen is Spectator); + AddStep("start play", () => testSpectatorStreamingClient.StartPlay()); sendFrames(); @@ -73,6 +76,18 @@ namespace osu.Game.Tests.Visual.Gameplay AddAssert("game is paused", () => player.ChildrenOfType().First().IsPaused.Value); } + [Test] + public void TestPlayStartsWithNoFrames() + { + loadSpectatingScreen(); + + AddStep("start play", () => testSpectatorStreamingClient.StartPlay()); + + AddUntilStep("wait for player", () => Stack.CurrentScreen is Player); + + AddUntilStep("game is paused", () => player.ChildrenOfType().First().IsPaused.Value); + } + [Test] public void TestSpectatingDuringGameplay() { diff --git a/osu.Game/Rulesets/Replays/FramedReplayInputHandler.cs b/osu.Game/Rulesets/Replays/FramedReplayInputHandler.cs index ddf85b7300..d7be809d34 100644 --- a/osu.Game/Rulesets/Replays/FramedReplayInputHandler.cs +++ b/osu.Game/Rulesets/Replays/FramedReplayInputHandler.cs @@ -162,6 +162,12 @@ namespace osu.Game.Rulesets.Replays // if we didn't change frames, we need to ensure we are allowed to run frames in between, else return null. } } + else + { + // if we never received frames and are expecting to, block. + if (!replay.HasReceivedAllFrames) + return null; + } return CurrentTime = time; } From a289b7034f5df01ae442c65af5b8b01fe953a8c0 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 27 Oct 2020 18:32:05 +0900 Subject: [PATCH 19/55] Add test helper functions to promote code share --- .../Visual/Gameplay/TestSceneSpectator.cs | 49 +++++++++++-------- 1 file changed, 28 insertions(+), 21 deletions(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs index 14b0ca4d33..61bcea7bfa 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs @@ -59,21 +59,20 @@ namespace osu.Game.Tests.Visual.Gameplay AddAssert("screen hasn't changed", () => Stack.CurrentScreen is Spectator); - AddStep("start play", () => testSpectatorStreamingClient.StartPlay()); - + start(); sendFrames(); - AddUntilStep("wait for player", () => Stack.CurrentScreen is Player); - + waitForPlayer(); AddAssert("ensure frames arrived", () => replayHandler.HasFrames); AddUntilStep("wait for frame starvation", () => replayHandler.NextFrame == null); - AddAssert("game is paused", () => player.ChildrenOfType().First().IsPaused.Value); - sendFrames(); - AddUntilStep("game resumed", () => !player.ChildrenOfType().First().IsPaused.Value); + checkPaused(true); + sendFrames(); + + checkPaused(false); AddUntilStep("wait for frame starvation", () => replayHandler.NextFrame == null); - AddAssert("game is paused", () => player.ChildrenOfType().First().IsPaused.Value); + checkPaused(true); } [Test] @@ -81,17 +80,19 @@ namespace osu.Game.Tests.Visual.Gameplay { loadSpectatingScreen(); - AddStep("start play", () => testSpectatorStreamingClient.StartPlay()); - - AddUntilStep("wait for player", () => Stack.CurrentScreen is Player); - + start(); + waitForPlayer(); AddUntilStep("game is paused", () => player.ChildrenOfType().First().IsPaused.Value); + + sendFrames(); + + checkPaused(false); } [Test] public void TestSpectatingDuringGameplay() { - AddStep("start play", () => testSpectatorStreamingClient.StartPlay()); + start(); sendFrames(); // should seek immediately to available frames loadSpectatingScreen(); @@ -102,9 +103,9 @@ namespace osu.Game.Tests.Visual.Gameplay { loadSpectatingScreen(); - AddStep("start play", () => testSpectatorStreamingClient.StartPlay()); + start(); sendFrames(); - AddStep("start play", () => testSpectatorStreamingClient.StartPlay()); + start(); sendFrames(); } @@ -113,10 +114,10 @@ namespace osu.Game.Tests.Visual.Gameplay { loadSpectatingScreen(); - AddStep("start play", () => testSpectatorStreamingClient.StartPlay()); + start(); sendFrames(); - // should replay until running out of frames then fail + // TODO: should replay until running out of frames then fail } [Test] @@ -124,10 +125,9 @@ namespace osu.Game.Tests.Visual.Gameplay { loadSpectatingScreen(); - AddStep("start play", () => testSpectatorStreamingClient.StartPlay()); + start(); sendFrames(); - AddUntilStep("wait for player", () => Stack.CurrentScreen is Player); - + waitForPlayer(); // should immediately exit and unbind from streaming client AddStep("stop spectating", () => (Stack.CurrentScreen as Player)?.Exit()); @@ -139,11 +139,18 @@ namespace osu.Game.Tests.Visual.Gameplay { loadSpectatingScreen(); - AddStep("start play", () => testSpectatorStreamingClient.StartPlay()); + start(); sendFrames(); // player should never arrive. } + private void waitForPlayer() => AddUntilStep("wait for player", () => Stack.CurrentScreen is Player); + + private void start() => AddStep("start play", () => testSpectatorStreamingClient.StartPlay()); + + private void checkPaused(bool state) => + AddAssert($"game is {(state ? "paused" : "playing")}", () => player.ChildrenOfType().First().IsPaused.Value == state); + private void sendFrames(int count = 10) { AddStep("send frames", () => From 42b3aa335904c97871e7b44fe34384a4e545109e Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 27 Oct 2020 18:56:28 +0900 Subject: [PATCH 20/55] Fix spectating when starting from a point that isn't at the beginning of the beatmap --- .../Visual/Gameplay/TestSceneSpectator.cs | 10 ++++-- .../Screens/Play/GameplayClockContainer.cs | 25 +++++++++------ osu.Game/Screens/Play/Player.cs | 4 ++- osu.Game/Screens/Play/ReplayPlayer.cs | 10 +++--- osu.Game/Screens/Play/Spectator.cs | 2 +- osu.Game/Screens/Play/SpectatorPlayer.cs | 28 ++++++++++++++++ .../Screens/Play/SpectatorPlayerLoader.cs | 32 +++++++++++++++++++ 7 files changed, 93 insertions(+), 18 deletions(-) create mode 100644 osu.Game/Screens/Play/SpectatorPlayer.cs create mode 100644 osu.Game/Screens/Play/SpectatorPlayerLoader.cs diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs index 61bcea7bfa..456fb0b110 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs @@ -93,9 +93,15 @@ namespace osu.Game.Tests.Visual.Gameplay public void TestSpectatingDuringGameplay() { start(); - sendFrames(); - // should seek immediately to available frames + loadSpectatingScreen(); + + AddStep("advance frame count", () => nextFrame = 300); + sendFrames(); + + waitForPlayer(); + + AddUntilStep("playing from correct point in time", () => player.ChildrenOfType().First().FrameStableClock.CurrentTime > 30000); } [Test] diff --git a/osu.Game/Screens/Play/GameplayClockContainer.cs b/osu.Game/Screens/Play/GameplayClockContainer.cs index 6679e56871..6154ec67b8 100644 --- a/osu.Game/Screens/Play/GameplayClockContainer.cs +++ b/osu.Game/Screens/Play/GameplayClockContainer.cs @@ -37,6 +37,7 @@ namespace osu.Game.Screens.Play private readonly DecoupleableInterpolatingFramedClock adjustableClock; private readonly double gameplayStartTime; + private readonly bool startAtGameplayStart; private readonly double firstHitObjectTime; @@ -62,10 +63,11 @@ namespace osu.Game.Screens.Play private readonly FramedOffsetClock platformOffsetClock; - public GameplayClockContainer(WorkingBeatmap beatmap, double gameplayStartTime) + public GameplayClockContainer(WorkingBeatmap beatmap, double gameplayStartTime, bool startAtGameplayStart = false) { this.beatmap = beatmap; this.gameplayStartTime = gameplayStartTime; + this.startAtGameplayStart = startAtGameplayStart; track = beatmap.Track; firstHitObjectTime = beatmap.Beatmap.HitObjects.First().StartTime; @@ -103,16 +105,21 @@ namespace osu.Game.Screens.Play userAudioOffset.BindValueChanged(offset => userOffsetClock.Offset = offset.NewValue, true); // sane default provided by ruleset. - double startTime = Math.Min(0, gameplayStartTime); + double startTime = gameplayStartTime; - // if a storyboard is present, it may dictate the appropriate start time by having events in negative time space. - // this is commonly used to display an intro before the audio track start. - startTime = Math.Min(startTime, beatmap.Storyboard.FirstEventTime); + if (!startAtGameplayStart) + { + startTime = Math.Min(0, startTime); - // some beatmaps specify a current lead-in time which should be used instead of the ruleset-provided value when available. - // this is not available as an option in the live editor but can still be applied via .osu editing. - if (beatmap.BeatmapInfo.AudioLeadIn > 0) - startTime = Math.Min(startTime, firstHitObjectTime - beatmap.BeatmapInfo.AudioLeadIn); + // if a storyboard is present, it may dictate the appropriate start time by having events in negative time space. + // this is commonly used to display an intro before the audio track start. + startTime = Math.Min(startTime, beatmap.Storyboard.FirstEventTime); + + // some beatmaps specify a current lead-in time which should be used instead of the ruleset-provided value when available. + // this is not available as an option in the live editor but can still be applied via .osu editing. + if (beatmap.BeatmapInfo.AudioLeadIn > 0) + startTime = Math.Min(startTime, firstHitObjectTime - beatmap.BeatmapInfo.AudioLeadIn); + } Seek(startTime); diff --git a/osu.Game/Screens/Play/Player.cs b/osu.Game/Screens/Play/Player.cs index 3e79ea0840..f9af1818d0 100644 --- a/osu.Game/Screens/Play/Player.cs +++ b/osu.Game/Screens/Play/Player.cs @@ -199,7 +199,7 @@ namespace osu.Game.Screens.Play if (!ScoreProcessor.Mode.Disabled) config.BindWith(OsuSetting.ScoreDisplayMode, ScoreProcessor.Mode); - InternalChild = GameplayClockContainer = new GameplayClockContainer(Beatmap.Value, DrawableRuleset.GameplayStartTime); + InternalChild = GameplayClockContainer = CreateGameplayClockContainer(Beatmap.Value, DrawableRuleset.GameplayStartTime); AddInternal(gameplayBeatmap = new GameplayBeatmap(playableBeatmap)); AddInternal(screenSuspension = new ScreenSuspensionHandler(GameplayClockContainer)); @@ -288,6 +288,8 @@ namespace osu.Game.Screens.Play IsBreakTime.BindValueChanged(onBreakTimeChanged, true); } + protected virtual GameplayClockContainer CreateGameplayClockContainer(WorkingBeatmap beatmap, double gameplayStart) => new GameplayClockContainer(beatmap, gameplayStart); + private Drawable createUnderlayComponents() => DimmableStoryboard = new DimmableStoryboard(Beatmap.Value.Storyboard) { RelativeSizeAxes = Axes.Both }; diff --git a/osu.Game/Screens/Play/ReplayPlayer.cs b/osu.Game/Screens/Play/ReplayPlayer.cs index 7f5c17a265..eeb3b509fe 100644 --- a/osu.Game/Screens/Play/ReplayPlayer.cs +++ b/osu.Game/Screens/Play/ReplayPlayer.cs @@ -8,7 +8,7 @@ namespace osu.Game.Screens.Play { public class ReplayPlayer : Player { - private readonly Score score; + protected readonly Score Score; // Disallow replays from failing. (see https://github.com/ppy/osu/issues/6108) protected override bool CheckModsAllowFailure() => false; @@ -16,12 +16,12 @@ namespace osu.Game.Screens.Play public ReplayPlayer(Score score, bool allowPause = true, bool showResults = true) : base(allowPause, showResults) { - this.score = score; + this.Score = score; } protected override void PrepareReplay() { - DrawableRuleset?.SetReplayScore(score); + DrawableRuleset?.SetReplayScore(Score); } protected override ResultsScreen CreateResults(ScoreInfo score) => new SoloResultsScreen(score, false); @@ -31,9 +31,9 @@ namespace osu.Game.Screens.Play var baseScore = base.CreateScore(); // Since the replay score doesn't contain statistics, we'll pass them through here. - score.ScoreInfo.HitEvents = baseScore.HitEvents; + Score.ScoreInfo.HitEvents = baseScore.HitEvents; - return score.ScoreInfo; + return Score.ScoreInfo; } } } diff --git a/osu.Game/Screens/Play/Spectator.cs b/osu.Game/Screens/Play/Spectator.cs index 522a636dfb..dcbf502fb2 100644 --- a/osu.Game/Screens/Play/Spectator.cs +++ b/osu.Game/Screens/Play/Spectator.cs @@ -130,7 +130,7 @@ namespace osu.Game.Screens.Play ruleset.Value = resolvedRuleset.RulesetInfo; beatmap.Value = beatmaps.GetWorkingBeatmap(resolvedBeatmap); - this.Push(new ReplayPlayerLoader(new Score + this.Push(new SpectatorPlayerLoader(new Score { ScoreInfo = scoreInfo, Replay = replay, diff --git a/osu.Game/Screens/Play/SpectatorPlayer.cs b/osu.Game/Screens/Play/SpectatorPlayer.cs new file mode 100644 index 0000000000..89e5f8f2dc --- /dev/null +++ b/osu.Game/Screens/Play/SpectatorPlayer.cs @@ -0,0 +1,28 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System.Linq; +using osu.Game.Beatmaps; +using osu.Game.Scoring; + +namespace osu.Game.Screens.Play +{ + public class SpectatorPlayer : ReplayPlayer + { + public SpectatorPlayer(Score score) + : base(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 GameplayClockContainer(beatmap, firstFrameTime.Value, true); + } + } +} diff --git a/osu.Game/Screens/Play/SpectatorPlayerLoader.cs b/osu.Game/Screens/Play/SpectatorPlayerLoader.cs new file mode 100644 index 0000000000..580af81166 --- /dev/null +++ b/osu.Game/Screens/Play/SpectatorPlayerLoader.cs @@ -0,0 +1,32 @@ +// 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.Screens; +using osu.Game.Scoring; + +namespace osu.Game.Screens.Play +{ + public class SpectatorPlayerLoader : PlayerLoader + { + public readonly ScoreInfo Score; + + public SpectatorPlayerLoader(Score score) + : base(() => new SpectatorPlayer(score)) + { + if (score.Replay == null) + throw new ArgumentException($"{nameof(score)} must have a non-null {nameof(score.Replay)}.", nameof(score)); + + Score = score.ScoreInfo; + } + + public override void OnEntering(IScreen last) + { + // these will be reverted thanks to PlayerLoader's lease. + Mods.Value = Score.Mods; + Ruleset.Value = Score.Ruleset; + + base.OnEntering(last); + } + } +} From 2cacdaa11b4c530c425d430a25f613830e112e0a Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 27 Oct 2020 19:23:35 +0900 Subject: [PATCH 21/55] Add basic beatmap download and play flow --- .../Visual/Gameplay/TestSceneSpectator.cs | 18 ++-- osu.Game/Screens/Play/Spectator.cs | 89 ++++++++++++++++++- 2 files changed, 96 insertions(+), 11 deletions(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs index 456fb0b110..458fc1e58b 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs @@ -25,6 +25,9 @@ namespace osu.Game.Tests.Visual.Gameplay [Cached(typeof(SpectatorStreamingClient))] private TestSpectatorStreamingClient testSpectatorStreamingClient = new TestSpectatorStreamingClient(); + // used just to show beatmap card for the time being. + protected override bool UseOnlineAPI => true; + private Spectator spectatorScreen; [Resolved] @@ -134,9 +137,9 @@ namespace osu.Game.Tests.Visual.Gameplay start(); sendFrames(); waitForPlayer(); + // should immediately exit and unbind from streaming client AddStep("stop spectating", () => (Stack.CurrentScreen as Player)?.Exit()); - AddUntilStep("spectating stopped", () => spectatorScreen.GetParentScreen() == null); } @@ -145,14 +148,15 @@ namespace osu.Game.Tests.Visual.Gameplay { loadSpectatingScreen(); - start(); + start(88); sendFrames(); - // player should never arrive. + + AddAssert("screen didn't change", () => Stack.CurrentScreen is Spectator); } private void waitForPlayer() => AddUntilStep("wait for player", () => Stack.CurrentScreen is Player); - private void start() => AddStep("start play", () => testSpectatorStreamingClient.StartPlay()); + private void start(int? beatmapId = null) => AddStep("start play", () => testSpectatorStreamingClient.StartPlay(beatmapId)); private void checkPaused(bool state) => AddAssert($"game is {(state ? "paused" : "playing")}", () => player.ChildrenOfType().First().IsPaused.Value == state); @@ -176,7 +180,7 @@ namespace osu.Game.Tests.Visual.Gameplay public readonly User StreamingUser = new User { Id = 1234, Username = "Test user" }; - public void StartPlay() => sendState(); + public void StartPlay(int? beatmapId = null) => sendState(beatmapId); public void EndPlay() { @@ -218,12 +222,12 @@ namespace osu.Game.Tests.Visual.Gameplay base.WatchUser(userId); } - private void sendState() + private void sendState(int? beatmapId = null) { sentState = true; ((ISpectatorClient)this).UserBeganPlaying((int)StreamingUser.Id, new SpectatorState { - BeatmapID = beatmaps.GetAllUsableBeatmapSets().First().Beatmaps.First(b => b.RulesetID == 0).OnlineBeatmapID, + BeatmapID = beatmapId ?? beatmaps.GetAllUsableBeatmapSets().First().Beatmaps.First(b => b.RulesetID == 0).OnlineBeatmapID, RulesetID = 0, }); } diff --git a/osu.Game/Screens/Play/Spectator.cs b/osu.Game/Screens/Play/Spectator.cs index dcbf502fb2..dd6b434889 100644 --- a/osu.Game/Screens/Play/Spectator.cs +++ b/osu.Game/Screens/Play/Spectator.cs @@ -8,10 +8,15 @@ using JetBrains.Annotations; using osu.Framework.Allocation; using osu.Framework.Bindables; using osu.Framework.Graphics; +using osu.Framework.Graphics.Containers; using osu.Framework.Screens; using osu.Game.Beatmaps; +using osu.Game.Graphics; using osu.Game.Graphics.Sprites; +using osu.Game.Online.API; +using osu.Game.Online.API.Requests; using osu.Game.Online.Spectator; +using osu.Game.Overlays.BeatmapListing.Panels; using osu.Game.Replays; using osu.Game.Rulesets; using osu.Game.Rulesets.Mods; @@ -19,6 +24,7 @@ using osu.Game.Rulesets.Replays; using osu.Game.Rulesets.Replays.Types; using osu.Game.Scoring; using osu.Game.Users; +using osuTK; namespace osu.Game.Screens.Play { @@ -35,6 +41,9 @@ namespace osu.Game.Screens.Play [Resolved] private Bindable> mods { get; set; } + [Resolved] + private IAPIProvider api { get; set; } + [Resolved] private SpectatorStreamingClient spectatorStreaming { get; set; } @@ -46,6 +55,12 @@ namespace osu.Game.Screens.Play private Replay replay; + private Container beatmapPanelContainer; + + private SpectatorState state; + + private IBindable> managerUpdated; + public Spectator([NotNull] User targetUser) { this.targetUser = targetUser ?? throw new ArgumentNullException(nameof(targetUser)); @@ -56,11 +71,42 @@ namespace osu.Game.Screens.Play { InternalChildren = new Drawable[] { - new OsuSpriteText + new FillFlowContainer { - Text = $"Watching {targetUser}", + AutoSizeAxes = Axes.Both, + Direction = FillDirection.Vertical, Anchor = Anchor.Centre, Origin = Anchor.Centre, + Spacing = new Vector2(15), + Children = new Drawable[] + { + new OsuSpriteText + { + Text = "Currently spectating", + Font = OsuFont.Default.With(size: 30), + Anchor = Anchor.Centre, + Origin = Anchor.Centre, + }, + new UserGridPanel(targetUser) + { + Width = 290, + Anchor = Anchor.Centre, + Origin = Anchor.Centre, + }, + new OsuSpriteText + { + Text = "playing", + Font = OsuFont.Default.With(size: 30), + Anchor = Anchor.Centre, + Origin = Anchor.Centre, + }, + beatmapPanelContainer = new Container + { + AutoSizeAxes = Axes.Both, + Anchor = Anchor.Centre, + Origin = Anchor.Centre, + }, + } }, }; } @@ -74,6 +120,15 @@ namespace osu.Game.Screens.Play spectatorStreaming.OnNewFrames += userSentFrames; spectatorStreaming.WatchUser((int)targetUser.Id); + + managerUpdated = beatmaps.ItemUpdated.GetBoundCopy(); + managerUpdated.BindValueChanged(beatmapUpdated); + } + + private void beatmapUpdated(ValueChangedEvent> beatmap) + { + if (beatmap.NewValue.TryGetTarget(out var beatmapSet) && beatmapSet.Beatmaps.Any(b => b.OnlineBeatmapID == state.BeatmapID)) + attemptStart(); } private void userSentFrames(int userId, FrameDataBundle data) @@ -107,16 +162,31 @@ namespace osu.Game.Screens.Play replay ??= new Replay { HasReceivedAllFrames = false }; + this.state = state; + + attemptStart(); + } + + private void attemptStart() + { var resolvedRuleset = rulesets.AvailableRulesets.FirstOrDefault(r => r.ID == state.RulesetID)?.CreateInstance(); // ruleset not available if (resolvedRuleset == null) return; + if (state.BeatmapID == null) + return; + + this.MakeCurrent(); + var resolvedBeatmap = beatmaps.QueryBeatmap(b => b.OnlineBeatmapID == state.BeatmapID); if (resolvedBeatmap == null) + { + showBeatmapPanel(state.BeatmapID.Value); return; + } var scoreInfo = new ScoreInfo { @@ -125,8 +195,6 @@ namespace osu.Game.Screens.Play Ruleset = resolvedRuleset.RulesetInfo, }; - this.MakeCurrent(); - ruleset.Value = resolvedRuleset.RulesetInfo; beatmap.Value = beatmaps.GetWorkingBeatmap(resolvedBeatmap); @@ -137,6 +205,17 @@ namespace osu.Game.Screens.Play })); } + private void showBeatmapPanel(int beatmapId) + { + var req = new GetBeatmapSetRequest(beatmapId, BeatmapSetLookupType.BeatmapId); + req.Success += res => Schedule(() => + { + beatmapPanelContainer.Child = new GridBeatmapPanel(res.ToBeatmapSet(rulesets)); + }); + + api.Queue(req); + } + private void userFinishedPlaying(int userId, SpectatorState state) { if (replay == null) return; @@ -156,6 +235,8 @@ namespace osu.Game.Screens.Play spectatorStreaming.StopWatchingUser((int)targetUser.Id); } + + managerUpdated.UnbindAll(); } } } From d5e0fa322b794f987785063f2b5588d63e9f4a14 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 27 Oct 2020 22:30:45 +0900 Subject: [PATCH 22/55] Fix a couple of inspections --- osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs | 2 +- osu.Game/Screens/Play/ReplayPlayer.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs index 458fc1e58b..a716b0c06b 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs @@ -33,7 +33,7 @@ namespace osu.Game.Tests.Visual.Gameplay [Resolved] private OsuGameBase game { get; set; } - private int nextFrame = 0; + private int nextFrame; public override void SetUpSteps() { diff --git a/osu.Game/Screens/Play/ReplayPlayer.cs b/osu.Game/Screens/Play/ReplayPlayer.cs index eeb3b509fe..3a4298f22d 100644 --- a/osu.Game/Screens/Play/ReplayPlayer.cs +++ b/osu.Game/Screens/Play/ReplayPlayer.cs @@ -16,7 +16,7 @@ namespace osu.Game.Screens.Play public ReplayPlayer(Score score, bool allowPause = true, bool showResults = true) : base(allowPause, showResults) { - this.Score = score; + Score = score; } protected override void PrepareReplay() From 0a7f3dc19bd86057a11d129250b852732af39dc7 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 28 Oct 2020 07:29:07 +0900 Subject: [PATCH 23/55] Avoid null reference on finalization MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bartłomiej Dach --- osu.Game/Screens/Play/Spectator.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Screens/Play/Spectator.cs b/osu.Game/Screens/Play/Spectator.cs index dd6b434889..898d68a7d6 100644 --- a/osu.Game/Screens/Play/Spectator.cs +++ b/osu.Game/Screens/Play/Spectator.cs @@ -236,7 +236,7 @@ namespace osu.Game.Screens.Play spectatorStreaming.StopWatchingUser((int)targetUser.Id); } - managerUpdated.UnbindAll(); + managerUpdated?.UnbindAll(); } } } From 3e5322541d9d25c11f26ca0d8c2a13b070b4ae72 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 28 Oct 2020 14:35:42 +0900 Subject: [PATCH 24/55] Make direction setting more clear --- .../Replays/FramedReplayInputHandler.cs | 26 ++++++++++++------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/osu.Game/Rulesets/Replays/FramedReplayInputHandler.cs b/osu.Game/Rulesets/Replays/FramedReplayInputHandler.cs index d7be809d34..120d81816f 100644 --- a/osu.Game/Rulesets/Replays/FramedReplayInputHandler.cs +++ b/osu.Game/Rulesets/Replays/FramedReplayInputHandler.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using JetBrains.Annotations; using osu.Game.Input.Handlers; using osu.Game.Replays; @@ -112,15 +113,9 @@ namespace osu.Game.Rulesets.Replays /// The usable time value. If null, we should not advance time as we do not have enough data. public override double? SetFrameFromTime(double time) { - if (!CurrentTime.HasValue) - { - currentDirection = 1; - } - else - { - currentDirection = time.CompareTo(CurrentTime); - if (currentDirection == 0) currentDirection = 1; - } + updateDirection(time); + + Debug.Assert(currentDirection != 0); if (HasFrames) { @@ -171,5 +166,18 @@ namespace osu.Game.Rulesets.Replays return CurrentTime = time; } + + private void updateDirection(double time) + { + if (!CurrentTime.HasValue) + { + currentDirection = 1; + } + else + { + currentDirection = time.CompareTo(CurrentTime); + if (currentDirection == 0) currentDirection = 1; + } + } } } From 6eddd76bdcd3917854d8a2eb3e13cf4fc04094bd Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 28 Oct 2020 15:54:58 +0900 Subject: [PATCH 25/55] Simplify FramedReplayInputHandler's SetFrame implementation --- .../Replays/FramedReplayInputHandler.cs | 90 ++++++------------- 1 file changed, 29 insertions(+), 61 deletions(-) diff --git a/osu.Game/Rulesets/Replays/FramedReplayInputHandler.cs b/osu.Game/Rulesets/Replays/FramedReplayInputHandler.cs index 120d81816f..8a4451fdca 100644 --- a/osu.Game/Rulesets/Replays/FramedReplayInputHandler.cs +++ b/osu.Game/Rulesets/Replays/FramedReplayInputHandler.cs @@ -42,40 +42,32 @@ namespace osu.Game.Rulesets.Replays if (!currentFrameIndex.HasValue) return (TFrame)Frames[0]; - if (currentDirection > 0) - return currentFrameIndex == Frames.Count - 1 ? null : (TFrame)Frames[currentFrameIndex.Value + 1]; - else - return currentFrameIndex == 0 ? null : (TFrame)Frames[nextFrameIndex]; + int nextFrame = clampedNextFrameIndex; + + if (nextFrame == currentFrameIndex.Value) + return null; + + return (TFrame)Frames[clampedNextFrameIndex]; } } private int? currentFrameIndex; - private int nextFrameIndex => currentFrameIndex.HasValue ? Math.Clamp(currentFrameIndex.Value + (currentDirection > 0 ? 1 : -1), 0, Frames.Count - 1) : 0; + private int clampedNextFrameIndex => + currentFrameIndex.HasValue ? Math.Clamp(currentFrameIndex.Value + currentDirection, 0, Frames.Count - 1) : 0; protected FramedReplayInputHandler(Replay replay) { this.replay = replay; } - private bool advanceFrame() - { - int newFrame = nextFrameIndex; - - // ensure we aren't at an extent. - if (newFrame == currentFrameIndex) return false; - - currentFrameIndex = newFrame; - return true; - } - private const double sixty_frame_time = 1000.0 / 60; protected virtual double AllowedImportantTimeSpan => sixty_frame_time * 1.2; protected double? CurrentTime { get; private set; } - private int currentDirection; + private int currentDirection = 1; /// /// When set, we will ensure frames executed by nested drawables are frame-accurate to replay data. @@ -117,53 +109,29 @@ namespace osu.Game.Rulesets.Replays Debug.Assert(currentDirection != 0); - if (HasFrames) + TFrame next = NextFrame; + + // check if the next frame is valid for the current playback direction. + // validity is if the next frame is equal or "earlier" than the current point in time (so we can change to it) + int compare = time.CompareTo(next?.Time); + + if (next != null && (compare == 0 || compare == currentDirection)) { - var next = NextFrame; - - // check if the next frame is valid for the current playback direction. - // validity is if the next frame is equal or "earlier" - var compare = time.CompareTo(next?.Time); - - if (next != null && (compare == 0 || compare == currentDirection)) - { - if (advanceFrame()) - return CurrentTime = CurrentFrame.Time; - } - else - { - // this is the case where the frame can't be advanced (in the replay). - // even so, we may be able to move the clock forward due to being at the end of the replay or - // in a section where replay accuracy doesn't matter. - - // important section is always respected to block the update loop. - if (inImportantSection) - return null; - - if (next == null) - { - // in the case we have no more frames and haven't received the full replay, block. - if (!replay.HasReceivedAllFrames) - return null; - - // else allow play to end. - } - else if (next.Time.CompareTo(time) != currentDirection) - { - // in the case we have more frames, block if the next frame's time is less than the current time. - return null; - } - - // if we didn't change frames, we need to ensure we are allowed to run frames in between, else return null. - } - } - else - { - // if we never received frames and are expecting to, block. - if (!replay.HasReceivedAllFrames) - return null; + currentFrameIndex = clampedNextFrameIndex; + return CurrentTime = CurrentFrame.Time; } + // at this point, the frame can't be advanced (in the replay). + // even so, we may be able to move the clock forward due to being at the end of the replay or + // moving towards the next valid frame. + + // the exception is if currently in an important section, which is respected above all. + if (inImportantSection) + return null; + + // in the case we have no next frames and haven't received the full replay, block. + if (next == null && !replay.HasReceivedAllFrames) return null; + return CurrentTime = time; } From 48b0357e7d7daace77365a5507745d589edcda45 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 28 Oct 2020 16:11:14 +0900 Subject: [PATCH 26/55] Fix "finished playing" events handled for potentially incorrect user --- osu.Game/Screens/Play/Spectator.cs | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/osu.Game/Screens/Play/Spectator.cs b/osu.Game/Screens/Play/Spectator.cs index 898d68a7d6..f788dcd8c7 100644 --- a/osu.Game/Screens/Play/Spectator.cs +++ b/osu.Game/Screens/Play/Spectator.cs @@ -167,6 +167,16 @@ namespace osu.Game.Screens.Play attemptStart(); } + private void userFinishedPlaying(int userId, SpectatorState state) + { + if (userId != targetUser.Id) + return; + + if (replay == null) return; + + replay.HasReceivedAllFrames = true; + } + private void attemptStart() { var resolvedRuleset = rulesets.AvailableRulesets.FirstOrDefault(r => r.ID == state.RulesetID)?.CreateInstance(); @@ -216,13 +226,6 @@ namespace osu.Game.Screens.Play api.Queue(req); } - private void userFinishedPlaying(int userId, SpectatorState state) - { - if (replay == null) return; - - replay.HasReceivedAllFrames = true; - } - protected override void Dispose(bool isDisposing) { base.Dispose(isDisposing); From 5fcd39a43d521e7829b1fd39769d4dd56eab090e Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 28 Oct 2020 16:29:06 +0900 Subject: [PATCH 27/55] Ensure spectator screen is loaded before continuing --- osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs index a716b0c06b..5a2230dd64 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs @@ -5,6 +5,7 @@ using System.Collections.Generic; using System.Linq; using NUnit.Framework; using osu.Framework.Allocation; +using osu.Framework.Graphics; using osu.Framework.Screens; using osu.Framework.Testing; using osu.Framework.Utils; @@ -170,8 +171,11 @@ namespace osu.Game.Tests.Visual.Gameplay }); } - private void loadSpectatingScreen() => + private void loadSpectatingScreen() + { AddStep("load screen", () => LoadScreen(spectatorScreen = new Spectator(testSpectatorStreamingClient.StreamingUser))); + AddUntilStep("wait for screen load", () => spectatorScreen.LoadState == LoadState.Loaded); + } internal class TestSpectatorStreamingClient : SpectatorStreamingClient { From 730cc645fb134df2a00994d64405dbadb967ca38 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 28 Oct 2020 16:33:52 +0900 Subject: [PATCH 28/55] Avoid reconstructing ruleset for each frame bundle --- osu.Game/Screens/Play/Spectator.cs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/osu.Game/Screens/Play/Spectator.cs b/osu.Game/Screens/Play/Spectator.cs index f788dcd8c7..2de6c16c45 100644 --- a/osu.Game/Screens/Play/Spectator.cs +++ b/osu.Game/Screens/Play/Spectator.cs @@ -38,6 +38,8 @@ namespace osu.Game.Screens.Play [Resolved] private Bindable ruleset { get; set; } + private Ruleset rulesetInstance; + [Resolved] private Bindable> mods { get; set; } @@ -141,12 +143,10 @@ namespace osu.Game.Screens.Play if (replay == null) return; - var rulesetInstance = ruleset.Value.CreateInstance(); - foreach (var frame in data.Frames) { IConvertibleReplayFrame convertibleFrame = rulesetInstance.CreateConvertibleReplayFrame(); - convertibleFrame.FromLegacy(frame, beatmap.Value.Beatmap, null); + convertibleFrame.FromLegacy(frame, beatmap.Value.Beatmap); var convertedFrame = (ReplayFrame)convertibleFrame; convertedFrame.Time = frame.Time; @@ -206,6 +206,8 @@ namespace osu.Game.Screens.Play }; ruleset.Value = resolvedRuleset.RulesetInfo; + rulesetInstance = resolvedRuleset; + beatmap.Value = beatmaps.GetWorkingBeatmap(resolvedBeatmap); this.Push(new SpectatorPlayerLoader(new Score From 6169349f7c26f12365fbe91113aa4e89035b0a05 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 28 Oct 2020 17:42:04 +0900 Subject: [PATCH 29/55] Fix switching to new beatmap not working correctly --- osu.Game/Screens/Play/Spectator.cs | 6 +++--- osu.Game/Screens/Play/SpectatorPlayer.cs | 18 ++++++++++++++++++ 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/osu.Game/Screens/Play/Spectator.cs b/osu.Game/Screens/Play/Spectator.cs index 2de6c16c45..51cd5b59aa 100644 --- a/osu.Game/Screens/Play/Spectator.cs +++ b/osu.Game/Screens/Play/Spectator.cs @@ -164,7 +164,7 @@ namespace osu.Game.Screens.Play this.state = state; - attemptStart(); + Schedule(attemptStart); } private void userFinishedPlaying(int userId, SpectatorState state) @@ -175,6 +175,7 @@ namespace osu.Game.Screens.Play if (replay == null) return; replay.HasReceivedAllFrames = true; + replay = null; } private void attemptStart() @@ -188,8 +189,6 @@ namespace osu.Game.Screens.Play if (state.BeatmapID == null) return; - this.MakeCurrent(); - var resolvedBeatmap = beatmaps.QueryBeatmap(b => b.OnlineBeatmapID == state.BeatmapID); if (resolvedBeatmap == null) @@ -201,6 +200,7 @@ namespace osu.Game.Screens.Play var scoreInfo = new ScoreInfo { Beatmap = resolvedBeatmap, + User = targetUser, Mods = state.Mods.Select(m => m.ToMod(resolvedRuleset)).ToArray(), Ruleset = resolvedRuleset.RulesetInfo, }; diff --git a/osu.Game/Screens/Play/SpectatorPlayer.cs b/osu.Game/Screens/Play/SpectatorPlayer.cs index 89e5f8f2dc..fbd21b32ba 100644 --- a/osu.Game/Screens/Play/SpectatorPlayer.cs +++ b/osu.Game/Screens/Play/SpectatorPlayer.cs @@ -2,18 +2,36 @@ // See the LICENCE file in the repository root for full licence text. using System.Linq; +using osu.Framework.Allocation; +using osu.Framework.Screens; using osu.Game.Beatmaps; +using osu.Game.Online.Spectator; using osu.Game.Scoring; namespace osu.Game.Screens.Play { public class SpectatorPlayer : ReplayPlayer { + [Resolved] + private SpectatorStreamingClient spectatorStreaming { get; set; } + public SpectatorPlayer(Score score) : base(score) { } + [BackgroundDependencyLoader] + private void load() + { + spectatorStreaming.OnUserBeganPlaying += userBeganPlaying; + } + + private void userBeganPlaying(int userId, SpectatorState state) + { + if (userId == Score.ScoreInfo.UserID) + Schedule(this.Exit); + } + 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. From 25ab3a5feafcd037bc475aeea1c3d8119185e70b Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 28 Oct 2020 22:10:37 +0900 Subject: [PATCH 30/55] Construct replay after being sure a ruleset is available to avoid nullrefs --- osu.Game/Screens/Play/Spectator.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game/Screens/Play/Spectator.cs b/osu.Game/Screens/Play/Spectator.cs index 51cd5b59aa..6a11aeb0e9 100644 --- a/osu.Game/Screens/Play/Spectator.cs +++ b/osu.Game/Screens/Play/Spectator.cs @@ -160,8 +160,6 @@ namespace osu.Game.Screens.Play if (userId != targetUser.Id) return; - replay ??= new Replay { HasReceivedAllFrames = false }; - this.state = state; Schedule(attemptStart); @@ -197,6 +195,8 @@ namespace osu.Game.Screens.Play return; } + replay ??= new Replay { HasReceivedAllFrames = false }; + var scoreInfo = new ScoreInfo { Beatmap = resolvedBeatmap, From 5d02de29ca1476a4989ecf74ee6583e32dd12005 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 28 Oct 2020 22:50:45 +0900 Subject: [PATCH 31/55] Fix attempt to change ruleset/beatmap bindables while screen is not active --- osu.Game/Screens/Play/Spectator.cs | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/osu.Game/Screens/Play/Spectator.cs b/osu.Game/Screens/Play/Spectator.cs index 6a11aeb0e9..2f65dc06d0 100644 --- a/osu.Game/Screens/Play/Spectator.cs +++ b/osu.Game/Screens/Play/Spectator.cs @@ -63,6 +63,11 @@ namespace osu.Game.Screens.Play private IBindable> managerUpdated; + /// + /// Becomes true if a new state is waiting to be loaded (while this screen was not active). + /// + private bool newStatePending; + public Spectator([NotNull] User targetUser) { this.targetUser = targetUser ?? throw new ArgumentNullException(nameof(targetUser)); @@ -162,7 +167,21 @@ namespace osu.Game.Screens.Play this.state = state; - Schedule(attemptStart); + if (this.IsCurrentScreen()) + Schedule(attemptStart); + else + newStatePending = true; + } + + public override void OnResuming(IScreen last) + { + base.OnResuming(last); + + if (newStatePending) + { + attemptStart(); + newStatePending = false; + } } private void userFinishedPlaying(int userId, SpectatorState state) From 8bbcb9be8a7a8bd11b8192f7ee0c0722fe96b62d Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 28 Oct 2020 22:50:57 +0900 Subject: [PATCH 32/55] Always use imported beatmap in tests --- .../Visual/Gameplay/TestSceneSpectator.cs | 36 ++++++++++++------- 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs index 5a2230dd64..b4ab22cfad 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs @@ -36,13 +36,21 @@ namespace osu.Game.Tests.Visual.Gameplay private int nextFrame; + private BeatmapSetInfo importedBeatmap; + + private int importedBeatmapId; + public override void SetUpSteps() { base.SetUpSteps(); AddStep("reset sent frames", () => nextFrame = 0); - AddStep("import beatmap", () => ImportBeatmapTest.LoadOszIntoOsu(game, virtualTrack: true).Wait()); + AddStep("import beatmap", () => + { + importedBeatmap = ImportBeatmapTest.LoadOszIntoOsu(game, virtualTrack: true).Result; + importedBeatmapId = importedBeatmap.Beatmaps.First(b => b.RulesetID == 0).OnlineBeatmapID ?? -1; + }); AddStep("add streaming client", () => { @@ -115,6 +123,7 @@ namespace osu.Game.Tests.Visual.Gameplay start(); sendFrames(); + start(); sendFrames(); } @@ -157,7 +166,7 @@ namespace osu.Game.Tests.Visual.Gameplay private void waitForPlayer() => AddUntilStep("wait for player", () => Stack.CurrentScreen is Player); - private void start(int? beatmapId = null) => AddStep("start play", () => testSpectatorStreamingClient.StartPlay(beatmapId)); + private void start(int? beatmapId = null) => AddStep("start play", () => testSpectatorStreamingClient.StartPlay(beatmapId ?? importedBeatmapId)); private void checkPaused(bool state) => AddAssert($"game is {(state ? "paused" : "playing")}", () => player.ChildrenOfType().First().IsPaused.Value == state); @@ -179,18 +188,21 @@ namespace osu.Game.Tests.Visual.Gameplay internal class TestSpectatorStreamingClient : SpectatorStreamingClient { - [Resolved] - private BeatmapManager beatmaps { get; set; } - public readonly User StreamingUser = new User { Id = 1234, Username = "Test user" }; - public void StartPlay(int? beatmapId = null) => sendState(beatmapId); + private int beatmapId; - public void EndPlay() + public void StartPlay(int beatmapId) + { + this.beatmapId = beatmapId; + sendState(beatmapId); + } + + public void EndPlay(int beatmapId) { ((ISpectatorClient)this).UserFinishedPlaying((int)StreamingUser.Id, new SpectatorState { - BeatmapID = beatmaps.GetAllUsableBeatmapSets().First().Beatmaps.First(b => b.RulesetID == 0).OnlineBeatmapID, + BeatmapID = beatmapId, RulesetID = 0, }); } @@ -212,7 +224,7 @@ namespace osu.Game.Tests.Visual.Gameplay ((ISpectatorClient)this).UserSentFrames((int)StreamingUser.Id, bundle); if (!sentState) - sendState(); + sendState(beatmapId); } public override void WatchUser(int userId) @@ -220,18 +232,18 @@ namespace osu.Game.Tests.Visual.Gameplay if (sentState) { // usually the server would do this. - sendState(); + sendState(beatmapId); } base.WatchUser(userId); } - private void sendState(int? beatmapId = null) + private void sendState(int beatmapId) { sentState = true; ((ISpectatorClient)this).UserBeganPlaying((int)StreamingUser.Id, new SpectatorState { - BeatmapID = beatmapId ?? beatmaps.GetAllUsableBeatmapSets().First().Beatmaps.First(b => b.RulesetID == 0).OnlineBeatmapID, + BeatmapID = beatmapId, RulesetID = 0, }); } From 1d499ec15d5ce31cd05337f9ead39a33562f267e Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 28 Oct 2020 22:51:35 +0900 Subject: [PATCH 33/55] Change beatmap not existing test to specify a beatmap ID that can't possibly exist --- osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs index b4ab22cfad..0b530a303f 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs @@ -158,7 +158,7 @@ namespace osu.Game.Tests.Visual.Gameplay { loadSpectatingScreen(); - start(88); + start(-1234); sendFrames(); AddAssert("screen didn't change", () => Stack.CurrentScreen is Spectator); From 7cc4a7cb5cdd419af637f6bcd1e631bc8321f6ad Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 28 Oct 2020 22:59:54 +0900 Subject: [PATCH 34/55] Add more accurate fail scenario test logic --- .../Visual/Gameplay/TestSceneSpectator.cs | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs index 0b530a303f..7dde493b1a 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs @@ -94,7 +94,7 @@ namespace osu.Game.Tests.Visual.Gameplay start(); waitForPlayer(); - AddUntilStep("game is paused", () => player.ChildrenOfType().First().IsPaused.Value); + checkPaused(true); sendFrames(); @@ -134,8 +134,13 @@ namespace osu.Game.Tests.Visual.Gameplay loadSpectatingScreen(); start(); - sendFrames(); + waitForPlayer(); + checkPaused(true); + + finish(); + + checkPaused(false); // TODO: should replay until running out of frames then fail } @@ -168,8 +173,10 @@ namespace osu.Game.Tests.Visual.Gameplay private void start(int? beatmapId = null) => AddStep("start play", () => testSpectatorStreamingClient.StartPlay(beatmapId ?? importedBeatmapId)); + private void finish(int? beatmapId = null) => AddStep("end play", () => testSpectatorStreamingClient.EndPlay(beatmapId ?? importedBeatmapId)); + private void checkPaused(bool state) => - AddAssert($"game is {(state ? "paused" : "playing")}", () => player.ChildrenOfType().First().IsPaused.Value == state); + AddUntilStep($"game is {(state ? "paused" : "playing")}", () => player.ChildrenOfType().First().IsPaused.Value == state); private void sendFrames(int count = 10) { From 6c2cee7b3fb187348fc099617bbdde37966404a0 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 28 Oct 2020 23:13:42 +0900 Subject: [PATCH 35/55] Avoid cross-pollution between tests of current playing state --- osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs index 7dde493b1a..6485cbdad3 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs @@ -57,6 +57,8 @@ namespace osu.Game.Tests.Visual.Gameplay Remove(testSpectatorStreamingClient); Add(testSpectatorStreamingClient); }); + + finish(); } private OsuFramedReplayInputHandler replayHandler => @@ -212,6 +214,8 @@ namespace osu.Game.Tests.Visual.Gameplay BeatmapID = beatmapId, RulesetID = 0, }); + + sentState = false; } private bool sentState; From d91456dc2993705d44a30ce74c87bfe1595c973c Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 29 Oct 2020 14:25:47 +0900 Subject: [PATCH 36/55] Move initial validity check out of loop for clarity --- osu.Game/Rulesets/UI/FrameStabilityContainer.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/osu.Game/Rulesets/UI/FrameStabilityContainer.cs b/osu.Game/Rulesets/UI/FrameStabilityContainer.cs index f8156164c1..8edeaf851d 100644 --- a/osu.Game/Rulesets/UI/FrameStabilityContainer.cs +++ b/osu.Game/Rulesets/UI/FrameStabilityContainer.cs @@ -96,7 +96,10 @@ namespace osu.Game.Rulesets.UI int loops = MaxCatchUpFrames; - while (state != PlaybackState.NotValid && loops-- > 0) + if (state == PlaybackState.NotValid) + return true; + + while (loops-- > 0) { updateClock(); From db2b00068f024589f26b4ab247b824f030a88b2b Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 29 Oct 2020 14:42:16 +0900 Subject: [PATCH 37/55] Avoid sourcing parent clock when in a paused state --- .../Rulesets/UI/FrameStabilityContainer.cs | 27 +++++++++++-------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/osu.Game/Rulesets/UI/FrameStabilityContainer.cs b/osu.Game/Rulesets/UI/FrameStabilityContainer.cs index 8edeaf851d..75f3aa90ee 100644 --- a/osu.Game/Rulesets/UI/FrameStabilityContainer.cs +++ b/osu.Game/Rulesets/UI/FrameStabilityContainer.cs @@ -85,23 +85,31 @@ namespace osu.Game.Rulesets.UI public override bool UpdateSubTree() { - state = frameStableClock.IsPaused.Value ? PlaybackState.NotValid : PlaybackState.Valid; + double proposedTime = manualClock.CurrentTime; 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. + // when waiting on frames, the update loop still needs to be run (at least once) to check for newly arrived frames. + // time should not be sourced from the parent clock in this case. state = PlaybackState.Valid; } + else if (!frameStableClock.IsPaused.Value) + { + state = PlaybackState.Valid; + proposedTime = parentGameplayClock.CurrentTime; + } + else + { + // time should not advance while paused, not should anything run. + state = PlaybackState.NotValid; + return true; + } int loops = MaxCatchUpFrames; - if (state == PlaybackState.NotValid) - return true; - while (loops-- > 0) { - updateClock(); + updateClock(ref proposedTime); if (state == PlaybackState.NotValid) break; @@ -113,7 +121,7 @@ namespace osu.Game.Rulesets.UI return true; } - private void updateClock() + private void updateClock(ref double proposedTime) { if (parentGameplayClock == null) setClock(); // LoadComplete may not be run yet, but we still want the clock. @@ -121,9 +129,6 @@ namespace osu.Game.Rulesets.UI // each update start with considering things in valid state. state = PlaybackState.Valid; - // our goal is to catch up to the time provided by the parent clock. - var proposedTime = parentGameplayClock.CurrentTime; - 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. From a59ea987b7811ee6e815d03e3af85e3d9bb27bae Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 29 Oct 2020 14:57:36 +0900 Subject: [PATCH 38/55] Make tests more resilient under headless execution --- osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs index 6485cbdad3..6c05049864 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs @@ -82,13 +82,21 @@ namespace osu.Game.Tests.Visual.Gameplay AddUntilStep("wait for frame starvation", () => replayHandler.NextFrame == null); checkPaused(true); + double? pausedTime = null; + + AddStep("store time", () => pausedTime = currentFrameStableTime); + sendFrames(); - checkPaused(false); AddUntilStep("wait for frame starvation", () => replayHandler.NextFrame == null); checkPaused(true); + + AddAssert("time advanced", () => currentFrameStableTime > pausedTime); } + private double currentFrameStableTime + => player.ChildrenOfType().First().FrameStableClock.CurrentTime; + [Test] public void TestPlayStartsWithNoFrames() { @@ -98,7 +106,7 @@ namespace osu.Game.Tests.Visual.Gameplay waitForPlayer(); checkPaused(true); - sendFrames(); + sendFrames(1000); // send enough frames to ensure play won't be paused checkPaused(false); } From 7dd3a748be4b0d7e7514d02f46b2a451a3bb3ae9 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 29 Oct 2020 15:03:38 +0900 Subject: [PATCH 39/55] Add further test logic to ensure retry / restart flow is working correctly --- osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs index 6c05049864..2b1bf1810b 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs @@ -134,8 +134,16 @@ namespace osu.Game.Tests.Visual.Gameplay start(); sendFrames(); + waitForPlayer(); + + Player lastPlayer = null; + AddStep("store first player", () => lastPlayer = player); + start(); sendFrames(); + + waitForPlayer(); + AddAssert("player is different", () => lastPlayer != player); } [Test] From 6a31a313b6236521bb877dced8da5fb394e0dc40 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 29 Oct 2020 15:08:06 +0900 Subject: [PATCH 40/55] Fix stop watching test to check correct screen presence --- osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs index 2b1bf1810b..7c62cf47f4 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs @@ -127,7 +127,7 @@ namespace osu.Game.Tests.Visual.Gameplay } [Test] - public void TestHostStartsPlayingWhileAlreadyWatching() + public void TestHostRetriesWhileWatching() { loadSpectatingScreen(); @@ -171,9 +171,8 @@ namespace osu.Game.Tests.Visual.Gameplay sendFrames(); waitForPlayer(); - // should immediately exit and unbind from streaming client AddStep("stop spectating", () => (Stack.CurrentScreen as Player)?.Exit()); - AddUntilStep("spectating stopped", () => spectatorScreen.GetParentScreen() == null); + AddUntilStep("spectating stopped", () => spectatorScreen.GetChildScreen() == null); } [Test] From ce58bfdc4e845bc7141a7d0123445a3753c60860 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 29 Oct 2020 15:09:12 +0900 Subject: [PATCH 41/55] Add test covering host retry after returning to spectator screen --- .../Visual/Gameplay/TestSceneSpectator.cs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs index 7c62cf47f4..864e297eda 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs @@ -175,6 +175,23 @@ namespace osu.Game.Tests.Visual.Gameplay AddUntilStep("spectating stopped", () => spectatorScreen.GetChildScreen() == null); } + [Test] + public void TestStopWatchingThenHostRetries() + { + loadSpectatingScreen(); + + start(); + sendFrames(); + waitForPlayer(); + + AddStep("stop spectating", () => (Stack.CurrentScreen as Player)?.Exit()); + AddUntilStep("spectating stopped", () => spectatorScreen.GetChildScreen() == null); + + // host starts playing a new session + start(); + waitForPlayer(); + } + [Test] public void TestWatchingBeatmapThatDoesntExistLocally() { From fe409a55e63db279d4f45c5b91aeb935ac94d658 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 29 Oct 2020 15:10:11 +0900 Subject: [PATCH 42/55] Rename starvation test --- osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs index 864e297eda..8821067618 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs @@ -67,7 +67,7 @@ namespace osu.Game.Tests.Visual.Gameplay private Player player => Stack.CurrentScreen as Player; [Test] - public void TestBasicSpectatingFlow() + public void TestFrameStarvationAndResume() { loadSpectatingScreen(); From fa857514254b32a5b3b6ccc5143c1ad5078ad776 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 29 Oct 2020 15:10:42 +0900 Subject: [PATCH 43/55] Move helper functions to bottom of class --- .../Visual/Gameplay/TestSceneSpectator.cs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs index 8821067618..a4df450db9 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs @@ -61,11 +61,6 @@ namespace osu.Game.Tests.Visual.Gameplay finish(); } - private OsuFramedReplayInputHandler replayHandler => - (OsuFramedReplayInputHandler)Stack.ChildrenOfType().First().ReplayInputHandler; - - private Player player => Stack.CurrentScreen as Player; - [Test] public void TestFrameStarvationAndResume() { @@ -94,9 +89,6 @@ namespace osu.Game.Tests.Visual.Gameplay AddAssert("time advanced", () => currentFrameStableTime > pausedTime); } - private double currentFrameStableTime - => player.ChildrenOfType().First().FrameStableClock.CurrentTime; - [Test] public void TestPlayStartsWithNoFrames() { @@ -203,6 +195,14 @@ namespace osu.Game.Tests.Visual.Gameplay AddAssert("screen didn't change", () => Stack.CurrentScreen is Spectator); } + private OsuFramedReplayInputHandler replayHandler => + (OsuFramedReplayInputHandler)Stack.ChildrenOfType().First().ReplayInputHandler; + + private Player player => Stack.CurrentScreen as Player; + + private double currentFrameStableTime + => player.ChildrenOfType().First().FrameStableClock.CurrentTime; + private void waitForPlayer() => AddUntilStep("wait for player", () => Stack.CurrentScreen is Player); private void start(int? beatmapId = null) => AddStep("start play", () => testSpectatorStreamingClient.StartPlay(beatmapId ?? importedBeatmapId)); From 2671d371da4acc2a39c11d8f584a318961a0ac0b Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 29 Oct 2020 15:28:39 +0900 Subject: [PATCH 44/55] Move clock retrieval to new correct location --- osu.Game/Rulesets/UI/FrameStabilityContainer.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/osu.Game/Rulesets/UI/FrameStabilityContainer.cs b/osu.Game/Rulesets/UI/FrameStabilityContainer.cs index 75f3aa90ee..231c5110ea 100644 --- a/osu.Game/Rulesets/UI/FrameStabilityContainer.cs +++ b/osu.Game/Rulesets/UI/FrameStabilityContainer.cs @@ -96,6 +96,10 @@ namespace osu.Game.Rulesets.UI else if (!frameStableClock.IsPaused.Value) { state = PlaybackState.Valid; + + if (parentGameplayClock == null) + setClock(); // LoadComplete may not be run yet, but we still want the clock. + proposedTime = parentGameplayClock.CurrentTime; } else @@ -123,9 +127,6 @@ namespace osu.Game.Rulesets.UI private void updateClock(ref double proposedTime) { - if (parentGameplayClock == null) - setClock(); // LoadComplete may not be run yet, but we still want the clock. - // each update start with considering things in valid state. state = PlaybackState.Valid; From 335d150a134073aff3488a0476eff2d970e5d185 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 29 Oct 2020 18:11:50 +0900 Subject: [PATCH 45/55] Fix aim time being mutated inside update loop --- osu.Game/Rulesets/UI/FrameStabilityContainer.cs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/osu.Game/Rulesets/UI/FrameStabilityContainer.cs b/osu.Game/Rulesets/UI/FrameStabilityContainer.cs index 231c5110ea..e29abfd83e 100644 --- a/osu.Game/Rulesets/UI/FrameStabilityContainer.cs +++ b/osu.Game/Rulesets/UI/FrameStabilityContainer.cs @@ -85,7 +85,7 @@ namespace osu.Game.Rulesets.UI public override bool UpdateSubTree() { - double proposedTime = manualClock.CurrentTime; + double aimTime = manualClock.CurrentTime; if (frameStableClock.WaitingOnFrames.Value) { @@ -100,7 +100,7 @@ namespace osu.Game.Rulesets.UI if (parentGameplayClock == null) setClock(); // LoadComplete may not be run yet, but we still want the clock. - proposedTime = parentGameplayClock.CurrentTime; + aimTime = parentGameplayClock.CurrentTime; } else { @@ -113,7 +113,9 @@ namespace osu.Game.Rulesets.UI while (loops-- > 0) { - updateClock(ref proposedTime); + // update clock is always trying to approach the aim time. + // it should be provided as the original value each loop. + updateClock(aimTime); if (state == PlaybackState.NotValid) break; @@ -125,7 +127,7 @@ namespace osu.Game.Rulesets.UI return true; } - private void updateClock(ref double proposedTime) + private void updateClock(double proposedTime) { // each update start with considering things in valid state. state = PlaybackState.Valid; From 8e6c803900757b019b8d0db051d445c2b99039c8 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 30 Oct 2020 12:39:11 +0900 Subject: [PATCH 46/55] Avoid running full updateClock loop when waiting on frames --- .../Rulesets/UI/FrameStabilityContainer.cs | 21 +++++++++---------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/osu.Game/Rulesets/UI/FrameStabilityContainer.cs b/osu.Game/Rulesets/UI/FrameStabilityContainer.cs index 0990a667ec..4d554124ae 100644 --- a/osu.Game/Rulesets/UI/FrameStabilityContainer.cs +++ b/osu.Game/Rulesets/UI/FrameStabilityContainer.cs @@ -85,30 +85,29 @@ namespace osu.Game.Rulesets.UI public override bool UpdateSubTree() { - double aimTime = manualClock.CurrentTime; + if (parentGameplayClock == null) + setClock(); // LoadComplete may not be run yet, but we still want the clock. + + double aimTime = parentGameplayClock.CurrentTime; if (frameStableClock.WaitingOnFrames.Value) { - // when waiting on frames, the update loop still needs to be run (at least once) to check for newly arrived frames. - // time should not be sourced from the parent clock in this case. - state = PlaybackState.Valid; + // waiting on frames is a special case where we want to avoid doing any update propagation, unless new frame data has arrived. + state = ReplayInputHandler.SetFrameFromTime(aimTime) != null ? PlaybackState.Valid : PlaybackState.NotValid; } else if (!frameStableClock.IsPaused.Value) { state = PlaybackState.Valid; - - if (parentGameplayClock == null) - setClock(); // LoadComplete may not be run yet, but we still want the clock. - - aimTime = parentGameplayClock.CurrentTime; } else { - // time should not advance while paused, not should anything run. + // time should not advance while paused, nor should anything run. state = PlaybackState.NotValid; - return true; } + if (state == PlaybackState.NotValid) + return true; + int loops = MaxCatchUpFrames; while (loops-- > 0) From 326fd0352568770e0cd0c494863ede623a977a73 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 30 Oct 2020 15:25:53 +0900 Subject: [PATCH 47/55] Fix loop not exiting after first valid frame --- osu.Game/Rulesets/UI/FrameStabilityContainer.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game/Rulesets/UI/FrameStabilityContainer.cs b/osu.Game/Rulesets/UI/FrameStabilityContainer.cs index 4d554124ae..1ff8fc9715 100644 --- a/osu.Game/Rulesets/UI/FrameStabilityContainer.cs +++ b/osu.Game/Rulesets/UI/FrameStabilityContainer.cs @@ -110,7 +110,7 @@ namespace osu.Game.Rulesets.UI int loops = MaxCatchUpFrames; - while (loops-- > 0) + do { // update clock is always trying to approach the aim time. // it should be provided as the original value each loop. @@ -121,7 +121,7 @@ namespace osu.Game.Rulesets.UI base.UpdateSubTree(); UpdateSubTreeMasking(this, ScreenSpaceDrawQuad.AABBFloat); - } + } while (state == PlaybackState.RequiresCatchUp && loops-- > 0); return true; } From 0f997386aef703dc440d9afb0e1ee6e0227304ec Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 30 Oct 2020 15:26:21 +0900 Subject: [PATCH 48/55] Fix direction and IsRunning not updating on first frame after becoming valid The parent clock will not unpause until WaitingForFrames becomes false, so I've moved the set of that before we start to propagate its values across. Doesn't fix any visible issue but should make propagation one game loop faster. --- osu.Game/Rulesets/UI/FrameStabilityContainer.cs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/osu.Game/Rulesets/UI/FrameStabilityContainer.cs b/osu.Game/Rulesets/UI/FrameStabilityContainer.cs index 1ff8fc9715..8a7f8d2739 100644 --- a/osu.Game/Rulesets/UI/FrameStabilityContainer.cs +++ b/osu.Game/Rulesets/UI/FrameStabilityContainer.cs @@ -144,22 +144,22 @@ namespace osu.Game.Rulesets.UI state = PlaybackState.NotValid; } - if (proposedTime != manualClock.CurrentTime) + if (state == PlaybackState.Valid) direction = proposedTime >= manualClock.CurrentTime ? 1 : -1; + double timeBehind = Math.Abs(proposedTime - parentGameplayClock.CurrentTime); + + frameStableClock.IsCatchingUp.Value = timeBehind > 200; + frameStableClock.WaitingOnFrames.Value = state == PlaybackState.NotValid; + 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.Valid && timeBehind > 0) state = PlaybackState.RequiresCatchUp; - 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(); From 32e68a6a3c16df655a597b1b3f770c77d2dec2cf Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 30 Oct 2020 16:09:01 +0900 Subject: [PATCH 49/55] Fix FramedReplayInputHandler incorrectly blocking in streaming mode when time requested is before the first frame Most of this is just tidying up the logic to (hopefully) be better to follow, again (again (again)). The actual fix is that we now allow interpolation/playback when the incoming time is less than the first frame's time, regardless of receiving status. --- .../Replays/FramedReplayInputHandler.cs | 56 +++++++++++++------ 1 file changed, 40 insertions(+), 16 deletions(-) diff --git a/osu.Game/Rulesets/Replays/FramedReplayInputHandler.cs b/osu.Game/Rulesets/Replays/FramedReplayInputHandler.cs index 8a4451fdca..b43324bcfa 100644 --- a/osu.Game/Rulesets/Replays/FramedReplayInputHandler.cs +++ b/osu.Game/Rulesets/Replays/FramedReplayInputHandler.cs @@ -40,7 +40,7 @@ namespace osu.Game.Rulesets.Replays return null; if (!currentFrameIndex.HasValue) - return (TFrame)Frames[0]; + return currentDirection > 0 ? (TFrame)Frames[0] : null; int nextFrame = clampedNextFrameIndex; @@ -109,30 +109,54 @@ namespace osu.Game.Rulesets.Replays Debug.Assert(currentDirection != 0); - TFrame next = NextFrame; - - // check if the next frame is valid for the current playback direction. - // validity is if the next frame is equal or "earlier" than the current point in time (so we can change to it) - int compare = time.CompareTo(next?.Time); - - if (next != null && (compare == 0 || compare == currentDirection)) + if (!HasFrames) { - currentFrameIndex = clampedNextFrameIndex; - return CurrentTime = CurrentFrame.Time; + // in the case all frames are received, allow time to progress regardless. + if (replay.HasReceivedAllFrames) + return CurrentTime = time; + + return null; } - // at this point, the frame can't be advanced (in the replay). - // even so, we may be able to move the clock forward due to being at the end of the replay or - // moving towards the next valid frame. + TFrame next = NextFrame; + + // if we have a next frame, check if it is before or at the current time in playback, and advance time to it if so. + if (next != null) + { + int compare = time.CompareTo(next.Time); + + if (compare == 0 || compare == currentDirection) + { + currentFrameIndex = clampedNextFrameIndex; + return CurrentTime = CurrentFrame.Time; + } + } + + // at this point, the frame index can't be advanced. + // even so, we may be able to propose the clock progresses forward due to being at an extent of the replay, + // or moving towards the next valid frame (ie. interpolating in a non-important section). // the exception is if currently in an important section, which is respected above all. if (inImportantSection) + { + Debug.Assert(next != null || !replay.HasReceivedAllFrames); return null; + } - // in the case we have no next frames and haven't received the full replay, block. - if (next == null && !replay.HasReceivedAllFrames) return null; + // if a next frame does exist, allow interpolation. + if (next != null) + return CurrentTime = time; - return CurrentTime = time; + // if all frames have been received, allow playing beyond extents. + if (replay.HasReceivedAllFrames) + return CurrentTime = time; + + // if not all frames are received but we are before the first frame, allow playing. + if (time < Frames[0].Time) + return CurrentTime = time; + + // in the case we have no next frames and haven't received enough frame data, block. + return null; } private void updateDirection(double time) From abaa532766500c71a2dbf5054019dc1b1f7de2be Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 30 Oct 2020 16:24:02 +0900 Subject: [PATCH 50/55] Add test coverage for streaming replay playback --- .../StreamingFramedReplayInputHandlerTest.cs | 296 ++++++++++++++++++ 1 file changed, 296 insertions(+) create mode 100644 osu.Game.Tests/NonVisual/StreamingFramedReplayInputHandlerTest.cs diff --git a/osu.Game.Tests/NonVisual/StreamingFramedReplayInputHandlerTest.cs b/osu.Game.Tests/NonVisual/StreamingFramedReplayInputHandlerTest.cs new file mode 100644 index 0000000000..21ec29b10b --- /dev/null +++ b/osu.Game.Tests/NonVisual/StreamingFramedReplayInputHandlerTest.cs @@ -0,0 +1,296 @@ +// 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 NUnit.Framework; +using osu.Game.Replays; +using osu.Game.Rulesets.Replays; + +namespace osu.Game.Tests.NonVisual +{ + [TestFixture] + public class StreamingFramedReplayInputHandlerTest + { + private Replay replay; + private TestInputHandler handler; + + [SetUp] + public void SetUp() + { + handler = new TestInputHandler(replay = new Replay + { + HasReceivedAllFrames = false, + Frames = new List + { + new TestReplayFrame(0), + new TestReplayFrame(1000), + new TestReplayFrame(2000), + new TestReplayFrame(3000, true), + new TestReplayFrame(4000, true), + new TestReplayFrame(5000, true), + new TestReplayFrame(7000, true), + new TestReplayFrame(8000), + } + }); + } + + [Test] + public void TestNormalPlayback() + { + Assert.IsNull(handler.CurrentFrame); + + confirmCurrentFrame(null); + confirmNextFrame(0); + + setTime(0, 0); + confirmCurrentFrame(0); + confirmNextFrame(1); + + // if we hit the first frame perfectly, time should progress to it. + setTime(1000, 1000); + confirmCurrentFrame(1); + confirmNextFrame(2); + + // in between non-important frames should progress based on input. + setTime(1200, 1200); + confirmCurrentFrame(1); + + setTime(1400, 1400); + confirmCurrentFrame(1); + + // progressing beyond the next frame should force time to that frame once. + setTime(2200, 2000); + confirmCurrentFrame(2); + + // second attempt should progress to input time + setTime(2200, 2200); + confirmCurrentFrame(2); + + // entering important section + setTime(3000, 3000); + confirmCurrentFrame(3); + + // cannot progress within + setTime(3500, null); + confirmCurrentFrame(3); + + setTime(4000, 4000); + confirmCurrentFrame(4); + + // still cannot progress + setTime(4500, null); + confirmCurrentFrame(4); + + setTime(5200, 5000); + confirmCurrentFrame(5); + + // important section AllowedImportantTimeSpan allowance + setTime(5200, 5200); + confirmCurrentFrame(5); + + setTime(7200, 7000); + confirmCurrentFrame(6); + + setTime(7200, null); + confirmCurrentFrame(6); + + // exited important section + setTime(8200, 8000); + confirmCurrentFrame(7); + confirmNextFrame(null); + + setTime(8200, null); + confirmCurrentFrame(7); + confirmNextFrame(null); + + setTime(8400, null); + confirmCurrentFrame(7); + confirmNextFrame(null); + } + + [Test] + public void TestIntroTime() + { + setTime(-1000, -1000); + confirmCurrentFrame(null); + confirmNextFrame(0); + + setTime(-500, -500); + confirmCurrentFrame(null); + confirmNextFrame(0); + + setTime(0, 0); + confirmCurrentFrame(0); + confirmNextFrame(1); + } + + [Test] + public void TestBasicRewind() + { + setTime(2800, 0); + setTime(2800, 1000); + setTime(2800, 2000); + setTime(2800, 2800); + confirmCurrentFrame(2); + confirmNextFrame(3); + + // pivot without crossing a frame boundary + setTime(2700, 2700); + confirmCurrentFrame(2); + confirmNextFrame(1); + + // cross current frame boundary; should not yet update frame + setTime(1980, 1980); + confirmCurrentFrame(2); + confirmNextFrame(1); + + setTime(1200, 1200); + confirmCurrentFrame(2); + confirmNextFrame(1); + + // ensure each frame plays out until start + setTime(-500, 1000); + confirmCurrentFrame(1); + confirmNextFrame(0); + + setTime(-500, 0); + confirmCurrentFrame(0); + confirmNextFrame(null); + + setTime(-500, -500); + confirmCurrentFrame(0); + confirmNextFrame(null); + } + + [Test] + public void TestRewindInsideImportantSection() + { + fastForwardToPoint(3000); + + setTime(4000, 4000); + confirmCurrentFrame(4); + confirmNextFrame(5); + + setTime(3500, null); + confirmCurrentFrame(4); + confirmNextFrame(3); + + setTime(3000, 3000); + confirmCurrentFrame(3); + confirmNextFrame(2); + + setTime(3500, null); + confirmCurrentFrame(3); + confirmNextFrame(4); + + setTime(4000, 4000); + confirmCurrentFrame(4); + confirmNextFrame(5); + + setTime(4500, null); + confirmCurrentFrame(4); + confirmNextFrame(5); + + setTime(4000, null); + confirmCurrentFrame(4); + confirmNextFrame(5); + + setTime(3500, null); + confirmCurrentFrame(4); + confirmNextFrame(3); + + setTime(3000, 3000); + confirmCurrentFrame(3); + confirmNextFrame(2); + } + + [Test] + public void TestRewindOutOfImportantSection() + { + fastForwardToPoint(3500); + + confirmCurrentFrame(3); + confirmNextFrame(4); + + setTime(3200, null); + // next frame doesn't change even though direction reversed, because of important section. + confirmCurrentFrame(3); + confirmNextFrame(4); + + setTime(3000, null); + confirmCurrentFrame(3); + confirmNextFrame(4); + + setTime(2800, 2800); + confirmCurrentFrame(3); + confirmNextFrame(2); + } + + private void fastForwardToPoint(double destination) + { + for (int i = 0; i < 1000; i++) + { + if (handler.SetFrameFromTime(destination) == null) + return; + } + + throw new TimeoutException("Seek was never fulfilled"); + } + + private void setTime(double set, double? expect) + { + Assert.AreEqual(expect, handler.SetFrameFromTime(set)); + } + + private void confirmCurrentFrame(int? frame) + { + if (frame.HasValue) + { + Assert.IsNotNull(handler.CurrentFrame); + Assert.AreEqual(replay.Frames[frame.Value].Time, handler.CurrentFrame.Time); + } + else + { + Assert.IsNull(handler.CurrentFrame); + } + } + + private void confirmNextFrame(int? frame) + { + if (frame.HasValue) + { + Assert.IsNotNull(handler.NextFrame); + Assert.AreEqual(replay.Frames[frame.Value].Time, handler.NextFrame.Time); + } + else + { + Assert.IsNull(handler.NextFrame); + } + } + + private class TestReplayFrame : ReplayFrame + { + public readonly bool IsImportant; + + public TestReplayFrame(double time, bool isImportant = false) + : base(time) + { + IsImportant = isImportant; + } + } + + private class TestInputHandler : FramedReplayInputHandler + { + public TestInputHandler(Replay replay) + : base(replay) + { + FrameAccuratePlayback = true; + } + + protected override double AllowedImportantTimeSpan => 1000; + + protected override bool IsImportant(TestReplayFrame frame) => frame.IsImportant; + } + } +} From 1bd461f229a69dc139fc7c36e4f10cb5a874243c Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 30 Oct 2020 20:20:34 +0900 Subject: [PATCH 51/55] Move clock logic back to inside updateClock method --- .../Rulesets/UI/FrameStabilityContainer.cs | 20 +++++++++---------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/osu.Game/Rulesets/UI/FrameStabilityContainer.cs b/osu.Game/Rulesets/UI/FrameStabilityContainer.cs index 8a7f8d2739..c8f37d75a0 100644 --- a/osu.Game/Rulesets/UI/FrameStabilityContainer.cs +++ b/osu.Game/Rulesets/UI/FrameStabilityContainer.cs @@ -85,15 +85,10 @@ namespace osu.Game.Rulesets.UI public override bool UpdateSubTree() { - if (parentGameplayClock == null) - setClock(); // LoadComplete may not be run yet, but we still want the clock. - - double aimTime = parentGameplayClock.CurrentTime; - if (frameStableClock.WaitingOnFrames.Value) { // waiting on frames is a special case where we want to avoid doing any update propagation, unless new frame data has arrived. - state = ReplayInputHandler.SetFrameFromTime(aimTime) != null ? PlaybackState.Valid : PlaybackState.NotValid; + state = PlaybackState.Valid; } else if (!frameStableClock.IsPaused.Value) { @@ -103,10 +98,8 @@ namespace osu.Game.Rulesets.UI { // time should not advance while paused, nor should anything run. state = PlaybackState.NotValid; - } - - if (state == PlaybackState.NotValid) return true; + } int loops = MaxCatchUpFrames; @@ -114,7 +107,7 @@ namespace osu.Game.Rulesets.UI { // update clock is always trying to approach the aim time. // it should be provided as the original value each loop. - updateClock(aimTime); + updateClock(); if (state == PlaybackState.NotValid) break; @@ -126,8 +119,13 @@ namespace osu.Game.Rulesets.UI return true; } - private void updateClock(double proposedTime) + private void updateClock() { + if (parentGameplayClock == null) + setClock(); // LoadComplete may not be run yet, but we still want the clock. + + double proposedTime = parentGameplayClock.CurrentTime; + // each update start with considering things in valid state. state = PlaybackState.Valid; From b4e53110146b4f7d651cd588d794be0d075e4db1 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 30 Oct 2020 20:37:07 +0900 Subject: [PATCH 52/55] Move initial state set inside updateClock --- .../Rulesets/UI/FrameStabilityContainer.cs | 35 +++++++++---------- 1 file changed, 16 insertions(+), 19 deletions(-) diff --git a/osu.Game/Rulesets/UI/FrameStabilityContainer.cs b/osu.Game/Rulesets/UI/FrameStabilityContainer.cs index c8f37d75a0..e9865f6c8b 100644 --- a/osu.Game/Rulesets/UI/FrameStabilityContainer.cs +++ b/osu.Game/Rulesets/UI/FrameStabilityContainer.cs @@ -85,22 +85,6 @@ namespace osu.Game.Rulesets.UI public override bool UpdateSubTree() { - if (frameStableClock.WaitingOnFrames.Value) - { - // waiting on frames is a special case where we want to avoid doing any update propagation, unless new frame data has arrived. - state = PlaybackState.Valid; - } - else if (!frameStableClock.IsPaused.Value) - { - state = PlaybackState.Valid; - } - else - { - // time should not advance while paused, nor should anything run. - state = PlaybackState.NotValid; - return true; - } - int loops = MaxCatchUpFrames; do @@ -121,14 +105,27 @@ namespace osu.Game.Rulesets.UI private void updateClock() { + if (frameStableClock.WaitingOnFrames.Value) + { + // if waiting on frames, run one update loop to determine if frames have arrived. + state = PlaybackState.Valid; + } + else if (frameStableClock.IsPaused.Value) + { + // time should not advance while paused, nor should anything run. + state = PlaybackState.NotValid; + return; + } + else + { + state = PlaybackState.Valid; + } + if (parentGameplayClock == null) setClock(); // LoadComplete may not be run yet, but we still want the clock. double proposedTime = parentGameplayClock.CurrentTime; - // each update start with considering things in valid state. - state = PlaybackState.Valid; - 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. From 5903c3be9066b6fd101abab48f4eebcb7421a50b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Sun, 1 Nov 2020 14:39:10 +0100 Subject: [PATCH 53/55] Fix inaccurate xmldoc --- osu.Game/Online/Spectator/SpectatorStreamingClient.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Online/Spectator/SpectatorStreamingClient.cs b/osu.Game/Online/Spectator/SpectatorStreamingClient.cs index 481c94e6f3..cb170ad298 100644 --- a/osu.Game/Online/Spectator/SpectatorStreamingClient.cs +++ b/osu.Game/Online/Spectator/SpectatorStreamingClient.cs @@ -70,7 +70,7 @@ namespace osu.Game.Online.Spectator public event Action OnUserBeganPlaying; /// - /// Called whenever a user starts a play session. + /// Called whenever a user finishes a play session. /// public event Action OnUserFinishedPlaying; From b7696c85ad5a42c7c35902579765c4c04c79bffc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Sun, 1 Nov 2020 15:21:24 +0100 Subject: [PATCH 54/55] Add more xmldocs --- osu.Game/Rulesets/UI/IFrameStableClock.cs | 3 +++ osu.Game/Screens/Play/GameplayClockContainer.cs | 8 ++++++++ 2 files changed, 11 insertions(+) diff --git a/osu.Game/Rulesets/UI/IFrameStableClock.cs b/osu.Game/Rulesets/UI/IFrameStableClock.cs index c1d8733d26..569ef5e06c 100644 --- a/osu.Game/Rulesets/UI/IFrameStableClock.cs +++ b/osu.Game/Rulesets/UI/IFrameStableClock.cs @@ -10,6 +10,9 @@ namespace osu.Game.Rulesets.UI { IBindable IsCatchingUp { get; } + /// + /// Whether the frame stable clock is waiting on new frames to arrive to be able to progress time. + /// IBindable WaitingOnFrames { get; } } } diff --git a/osu.Game/Screens/Play/GameplayClockContainer.cs b/osu.Game/Screens/Play/GameplayClockContainer.cs index 6154ec67b8..2c83161614 100644 --- a/osu.Game/Screens/Play/GameplayClockContainer.cs +++ b/osu.Game/Screens/Play/GameplayClockContainer.cs @@ -63,6 +63,14 @@ namespace osu.Game.Screens.Play private readonly FramedOffsetClock platformOffsetClock; + /// + /// Creates a new . + /// + /// The beatmap being played. + /// The suggested time to start gameplay at. + /// + /// Whether should be used regardless of when storyboard events and hitobjects are supposed to start. + /// public GameplayClockContainer(WorkingBeatmap beatmap, double gameplayStartTime, bool startAtGameplayStart = false) { this.beatmap = beatmap; From 716458344fa374a7b83cbeb64b448d4dd046961d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Sun, 1 Nov 2020 16:03:28 +0100 Subject: [PATCH 55/55] Ensure spectator player is unsubscribed to prevent leak --- osu.Game/Screens/Play/SpectatorPlayer.cs | 26 ++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/osu.Game/Screens/Play/SpectatorPlayer.cs b/osu.Game/Screens/Play/SpectatorPlayer.cs index fbd21b32ba..6c1e83f236 100644 --- a/osu.Game/Screens/Play/SpectatorPlayer.cs +++ b/osu.Game/Screens/Play/SpectatorPlayer.cs @@ -26,12 +26,6 @@ namespace osu.Game.Screens.Play spectatorStreaming.OnUserBeganPlaying += userBeganPlaying; } - private void userBeganPlaying(int userId, SpectatorState state) - { - if (userId == Score.ScoreInfo.UserID) - Schedule(this.Exit); - } - 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. @@ -42,5 +36,25 @@ namespace osu.Game.Screens.Play return new GameplayClockContainer(beatmap, firstFrameTime.Value, true); } + + public override bool OnExiting(IScreen next) + { + spectatorStreaming.OnUserBeganPlaying -= userBeganPlaying; + return base.OnExiting(next); + } + + protected override void Dispose(bool isDisposing) + { + base.Dispose(isDisposing); + + if (spectatorStreaming != null) + spectatorStreaming.OnUserBeganPlaying -= userBeganPlaying; + } + + private void userBeganPlaying(int userId, SpectatorState state) + { + if (userId == Score.ScoreInfo.UserID) + Schedule(this.Exit); + } } }