Merge pull request #19442 from smoogipoo/fix-beginplaying-misordering

Move spectator begin/end playing to SubmittingPlayer
This commit is contained in:
Dean Herbert 2022-07-29 16:09:50 +09:00 committed by GitHub
commit eb92c35335
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 37 additions and 52 deletions

View File

@ -27,7 +27,6 @@
using osu.Game.Rulesets.Replays.Types;
using osu.Game.Rulesets.UI;
using osu.Game.Scoring;
using osu.Game.Screens.Play;
using osu.Game.Tests.Gameplay;
using osu.Game.Tests.Mods;
using osu.Game.Tests.Visual.Spectator;
@ -41,16 +40,12 @@ public class TestSceneSpectatorPlayback : OsuManualInputManagerTestScene
private TestRulesetInputManager playbackManager;
private TestRulesetInputManager recordingManager;
private Replay replay;
private Score recordingScore;
private Replay playbackReplay;
private TestSpectatorClient spectatorClient;
private ManualClock manualClock;
private TestReplayRecorder recorder;
private OsuSpriteText latencyDisplay;
private TestFramedReplayInputHandler replayHandler;
[SetUpSteps]
@ -58,7 +53,16 @@ public void SetUpSteps()
{
AddStep("Setup containers", () =>
{
replay = new Replay();
recordingScore = new Score
{
ScoreInfo =
{
BeatmapInfo = new BeatmapInfo(),
Ruleset = new OsuRuleset().RulesetInfo,
}
};
playbackReplay = new Replay();
manualClock = new ManualClock();
Child = new DependencyProvidingContainer
@ -67,7 +71,6 @@ public void SetUpSteps()
CachedDependencies = new[]
{
(typeof(SpectatorClient), (object)(spectatorClient = new TestSpectatorClient())),
(typeof(GameplayState), TestGameplayState.Create(new OsuRuleset()))
},
Children = new Drawable[]
{
@ -81,7 +84,7 @@ public void SetUpSteps()
{
recordingManager = new TestRulesetInputManager(TestCustomisableModRuleset.CreateTestRulesetInfo(), 0, SimultaneousBindingMode.Unique)
{
Recorder = recorder = new TestReplayRecorder
Recorder = recorder = new TestReplayRecorder(recordingScore)
{
ScreenSpaceToGamefield = pos => recordingManager.ToLocalSpace(pos),
},
@ -112,7 +115,7 @@ public void SetUpSteps()
playbackManager = new TestRulesetInputManager(TestCustomisableModRuleset.CreateTestRulesetInfo(), 0, SimultaneousBindingMode.Unique)
{
Clock = new FramedClock(manualClock),
ReplayInputHandler = replayHandler = new TestFramedReplayInputHandler(replay)
ReplayInputHandler = replayHandler = new TestFramedReplayInputHandler(playbackReplay)
{
GamefieldToScreenSpace = pos => playbackManager.ToScreenSpace(pos),
},
@ -144,6 +147,7 @@ public void SetUpSteps()
}
};
spectatorClient.BeginPlaying(TestGameplayState.Create(new OsuRuleset()), recordingScore);
spectatorClient.OnNewFrames += onNewFrames;
});
}
@ -151,15 +155,15 @@ public void SetUpSteps()
[Test]
public void TestBasic()
{
AddUntilStep("received frames", () => replay.Frames.Count > 50);
AddUntilStep("received frames", () => playbackReplay.Frames.Count > 50);
AddStep("stop sending frames", () => recorder.Expire());
AddUntilStep("wait for all frames received", () => replay.Frames.Count == recorder.SentFrames.Count);
AddUntilStep("wait for all frames received", () => playbackReplay.Frames.Count == recorder.SentFrames.Count);
}
[Test]
public void TestWithSendFailure()
{
AddUntilStep("received frames", () => replay.Frames.Count > 50);
AddUntilStep("received frames", () => playbackReplay.Frames.Count > 50);
int framesReceivedSoFar = 0;
int frameSendAttemptsSoFar = 0;
@ -172,21 +176,21 @@ public void TestWithSendFailure()
AddUntilStep("wait for next send attempt", () =>
{
framesReceivedSoFar = replay.Frames.Count;
framesReceivedSoFar = playbackReplay.Frames.Count;
return spectatorClient.FrameSendAttempts > frameSendAttemptsSoFar + 1;
});
AddUntilStep("wait for more send attempts", () => spectatorClient.FrameSendAttempts > frameSendAttemptsSoFar + 10);
AddAssert("frames did not increase", () => framesReceivedSoFar == replay.Frames.Count);
AddAssert("frames did not increase", () => framesReceivedSoFar == playbackReplay.Frames.Count);
AddStep("stop failing sends", () => spectatorClient.ShouldFailSendingFrames = false);
AddUntilStep("wait for next frames", () => framesReceivedSoFar < replay.Frames.Count);
AddUntilStep("wait for next frames", () => framesReceivedSoFar < playbackReplay.Frames.Count);
AddStep("stop sending frames", () => recorder.Expire());
AddUntilStep("wait for all frames received", () => replay.Frames.Count == recorder.SentFrames.Count);
AddAssert("ensure frames were received in the correct sequence", () => replay.Frames.Select(f => f.Time).SequenceEqual(recorder.SentFrames.Select(f => f.Time)));
AddUntilStep("wait for all frames received", () => playbackReplay.Frames.Count == recorder.SentFrames.Count);
AddAssert("ensure frames were received in the correct sequence", () => playbackReplay.Frames.Select(f => f.Time).SequenceEqual(recorder.SentFrames.Select(f => f.Time)));
}
private void onNewFrames(int userId, FrameDataBundle frames)
@ -195,10 +199,10 @@ private void onNewFrames(int userId, FrameDataBundle frames)
{
var frame = new TestReplayFrame();
frame.FromLegacy(legacyFrame, null);
replay.Frames.Add(frame);
playbackReplay.Frames.Add(frame);
}
Logger.Log($"Received {frames.Frames.Count} new frames (total {replay.Frames.Count} of {recorder.SentFrames.Count})");
Logger.Log($"Received {frames.Frames.Count} new frames (total {playbackReplay.Frames.Count} of {recorder.SentFrames.Count})");
}
private double latency = SpectatorClient.TIME_BETWEEN_SENDS;
@ -219,7 +223,7 @@ protected override void Update()
if (!replayHandler.HasFrames)
return;
var lastFrame = replay.Frames.LastOrDefault();
var lastFrame = playbackReplay.Frames.LastOrDefault();
// this isn't perfect as we basically can't be aware of the rate-of-send here (the streamer is not sending data when not being moved).
// in gameplay playback, the case where NextFrame is null would pause gameplay and handle this correctly; it's strictly a test limitation / best effort implementation.
@ -360,15 +364,8 @@ internal class TestReplayRecorder : ReplayRecorder<TestAction>
{
public List<ReplayFrame> SentFrames = new List<ReplayFrame>();
public TestReplayRecorder()
: base(new Score
{
ScoreInfo =
{
BeatmapInfo = new BeatmapInfo(),
Ruleset = new OsuRuleset().RulesetInfo,
}
})
public TestReplayRecorder(Score score)
: base(score)
{
}

View File

@ -14,7 +14,6 @@
using osu.Game.Online.Spectator;
using osu.Game.Rulesets.Replays;
using osu.Game.Scoring;
using osu.Game.Screens.Play;
using osuTK;
namespace osu.Game.Rulesets.UI
@ -33,9 +32,6 @@ public abstract class ReplayRecorder<T> : ReplayRecorder, IKeyBindingHandler<T>
[Resolved]
private SpectatorClient spectatorClient { get; set; }
[Resolved]
private GameplayState gameplayState { get; set; }
protected ReplayRecorder(Score target)
{
this.target = target;
@ -48,15 +44,7 @@ protected ReplayRecorder(Score target)
protected override void LoadComplete()
{
base.LoadComplete();
inputManager = GetContainingInputManager();
spectatorClient.BeginPlaying(gameplayState, target);
}
protected override void Dispose(bool isDisposing)
{
base.Dispose(isDisposing);
spectatorClient?.EndPlaying(gameplayState);
}
protected override void Update()

View File

@ -26,7 +26,6 @@
using osu.Game.Graphics.Containers;
using osu.Game.IO.Archives;
using osu.Game.Online.API;
using osu.Game.Online.Spectator;
using osu.Game.Overlays;
using osu.Game.Rulesets;
using osu.Game.Rulesets.Mods;
@ -101,9 +100,6 @@ public abstract class Player : ScreenWithBeatmapBackground, ISamplePlaybackDisab
[Resolved]
private MusicController musicController { get; set; }
[Resolved]
private SpectatorClient spectatorClient { get; set; }
public GameplayState GameplayState { get; private set; }
private Ruleset ruleset;
@ -1030,11 +1026,6 @@ public override bool OnExiting(ScreenExitEvent e)
// if arriving here and the results screen preparation task hasn't run, it's safe to say the user has not completed the beatmap.
if (prepareScoreForDisplayTask == null)
ScoreProcessor.FailScore(Score.ScoreInfo);
// EndPlaying() is typically called from ReplayRecorder.Dispose(). Disposal is currently asynchronous.
// To resolve test failures, forcefully end playing synchronously when this screen exits.
// Todo: Replace this with a more permanent solution once osu-framework has a synchronous cleanup method.
spectatorClient.EndPlaying(GameplayState);
}
// GameplayClockContainer performs seeks / start / stop operations on the beatmap's track.

View File

@ -15,6 +15,7 @@
using osu.Game.Database;
using osu.Game.Online.API;
using osu.Game.Online.Rooms;
using osu.Game.Online.Spectator;
using osu.Game.Rulesets.Scoring;
using osu.Game.Scoring;
@ -33,6 +34,9 @@ public abstract class SubmittingPlayer : Player
[Resolved]
private IAPIProvider api { get; set; }
[Resolved]
private SpectatorClient spectatorClient { get; set; }
private TaskCompletionSource<bool> scoreSubmissionSource;
protected SubmittingPlayer(PlayerConfiguration configuration = null)
@ -134,6 +138,8 @@ protected override void StartGameplay()
if (realmBeatmap != null)
realmBeatmap.LastPlayed = DateTimeOffset.Now;
});
spectatorClient.BeginPlaying(GameplayState, Score);
}
public override bool OnExiting(ScreenExitEvent e)
@ -141,7 +147,10 @@ public override bool OnExiting(ScreenExitEvent e)
bool exiting = base.OnExiting(e);
if (LoadedBeatmapSuccessfully)
{
submitScore(Score.DeepClone());
spectatorClient.EndPlaying(GameplayState);
}
return exiting;
}