Skip to content

Commit

Permalink
Merge pull request #225 from bdach/unranked-endplaysession-bug
Browse files Browse the repository at this point in the history 
Fix `EndPlaySession()` not broadcasting play end to other users on unranked beatmap plays
  • Loading branch information
@peppy
peppy authored Mar 15, 2024
2 parents 1c622f6 + eb4ed34 commit 6b1c04a
Show file tree
Hide file tree
Showing 2 changed files with 95 additions and 30 deletions.
90 changes: 75 additions & 15 deletions osu.Server.Spectator.Tests/SpectatorHubTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,6 @@ public class SpectatorHubTest
private const int beatmap_id = 88;
private const int watcher_id = 8000;

private static readonly SpectatorState state = new SpectatorState
{
BeatmapID = beatmap_id,
RulesetID = 0,
};

private readonly ScoreUploader scoreUploader;
private readonly Mock<IScoreStorage> mockScoreStorage;
private readonly Mock<IDatabaseAccess> mockDatabase;
Expand Down Expand Up @@ -92,11 +86,17 @@ public async Task NewUserConnectsAndStreamsData()
{
BeatmapID = beatmap_id,
RulesetID = 0,
State = SpectatedUserState.Playing,
});

// check all other users were informed that streaming began
mockClients.Verify(clients => clients.All, Times.Once);
mockReceiver.Verify(clients => clients.UserBeganPlaying(streamer_id, It.Is<SpectatorState>(m => m.Equals(state))), Times.Once());
mockReceiver.Verify(clients => clients.UserBeganPlaying(streamer_id, It.Is<SpectatorState>(m => m.Equals(new SpectatorState
{
BeatmapID = beatmap_id,
RulesetID = 0,
State = SpectatedUserState.Playing,
}))), Times.Once());

var data = new FrameDataBundle(
new FrameHeader(new ScoreInfo(), new ScoreProcessorStatistics()),
Expand Down Expand Up @@ -131,7 +131,13 @@ public async Task ReplayDataIsSaved(bool savingEnabled)
passed = true
}));

await hub.BeginPlaySession(1234, state);
await hub.BeginPlaySession(1234, new SpectatorState
{
BeatmapID = beatmap_id,
RulesetID = 0,
State = SpectatedUserState.Playing,
});

await hub.SendFrameData(new FrameDataBundle(
new FrameHeader(new ScoreInfo
{
Expand All @@ -141,14 +147,22 @@ await hub.SendFrameData(new FrameDataBundle(
}
}, new ScoreProcessorStatistics()),
new[] { new LegacyReplayFrame(1234, 0, 0, ReplayButtonState.None) }));
await hub.EndPlaySession(state);

await hub.EndPlaySession(new SpectatorState
{
BeatmapID = beatmap_id,
RulesetID = 0,
State = SpectatedUserState.Passed,
});

await scoreUploader.Flush();

if (savingEnabled)
mockScoreStorage.Verify(s => s.WriteAsync(It.Is<Score>(score => score.ScoreInfo.OnlineID == 456)), Times.Once);
else
mockScoreStorage.Verify(s => s.WriteAsync(It.IsAny<Score>()), Times.Never);

mockReceiver.Verify(clients => clients.UserFinishedPlaying(streamer_id, It.Is<SpectatorState>(m => m.State == SpectatedUserState.Passed)), Times.Once());
}

[Fact]
Expand All @@ -173,15 +187,28 @@ public async Task ReplaysWithoutAnyHitsAreDiscarded()
passed = true
}));

await hub.BeginPlaySession(1234, state);
await hub.BeginPlaySession(1234, new SpectatorState
{
BeatmapID = beatmap_id,
RulesetID = 0,
State = SpectatedUserState.Playing,
});

await hub.SendFrameData(new FrameDataBundle(
new FrameHeader(new ScoreInfo(), new ScoreProcessorStatistics()),
new[] { new LegacyReplayFrame(1234, 0, 0, ReplayButtonState.None) }));
await hub.EndPlaySession(state);

await hub.EndPlaySession(new SpectatorState
{
BeatmapID = beatmap_id,
RulesetID = 0,
State = SpectatedUserState.Quit,
});

await scoreUploader.Flush();

mockScoreStorage.Verify(s => s.WriteAsync(It.IsAny<Score>()), Times.Never);
mockReceiver.Verify(clients => clients.UserFinishedPlaying(streamer_id, It.Is<SpectatorState>(m => m.State == SpectatedUserState.Quit)), Times.Once());
}

[Theory]
Expand Down Expand Up @@ -209,6 +236,12 @@ public async Task NewUserBeginsWatchingStream(bool ongoing)
hub.Clients = mockClients.Object;
hub.Groups = mockGroups.Object;

var state = new SpectatorState
{
BeatmapID = beatmap_id,
RulesetID = 0,
};

if (ongoing)
{
hub.Context = streamerContext.Object;
Expand Down Expand Up @@ -324,7 +357,13 @@ public async Task ScoresAreOnlySavedOnRankedBeatmaps(BeatmapOnlineStatus status,
checksum = "checksum"
})!);

await hub.BeginPlaySession(1234, state);
await hub.BeginPlaySession(1234, new SpectatorState
{
BeatmapID = beatmap_id,
RulesetID = 0,
State = SpectatedUserState.Playing,
});

await hub.SendFrameData(new FrameDataBundle(
new FrameHeader(new ScoreInfo
{
Expand All @@ -334,14 +373,22 @@ await hub.SendFrameData(new FrameDataBundle(
}
}, new ScoreProcessorStatistics()),
new[] { new LegacyReplayFrame(1234, 0, 0, ReplayButtonState.None) }));
await hub.EndPlaySession(state);

await hub.EndPlaySession(new SpectatorState
{
BeatmapID = beatmap_id,
RulesetID = 0,
State = SpectatedUserState.Passed,
});

await scoreUploader.Flush();

if (saved)
mockScoreStorage.Verify(s => s.WriteAsync(It.Is<Score>(score => score.ScoreInfo.OnlineID == 456)), Times.Once);
else
mockScoreStorage.Verify(s => s.WriteAsync(It.Is<Score>(score => score.ScoreInfo.OnlineID == 456)), Times.Never);

mockReceiver.Verify(clients => clients.UserFinishedPlaying(streamer_id, It.Is<SpectatorState>(m => m.State == SpectatedUserState.Passed)), Times.Once());
}

[Fact]
Expand All @@ -366,15 +413,28 @@ public async Task FailedScoresAreNotSaved()
passed = false
}));

await hub.BeginPlaySession(1234, state);
await hub.BeginPlaySession(1234, new SpectatorState
{
BeatmapID = beatmap_id,
RulesetID = 0,
State = SpectatedUserState.Playing,
});

await hub.SendFrameData(new FrameDataBundle(
new FrameHeader(new ScoreInfo(), new ScoreProcessorStatistics()),
new[] { new LegacyReplayFrame(1234, 0, 0, ReplayButtonState.None) }));
await hub.EndPlaySession(state);

await hub.EndPlaySession(new SpectatorState
{
BeatmapID = beatmap_id,
RulesetID = 0,
State = SpectatedUserState.Failed,
});

await scoreUploader.Flush();

mockScoreStorage.Verify(s => s.WriteAsync(It.Is<Score>(score => score.ScoreInfo.OnlineID == 456)), Times.Never);
mockReceiver.Verify(clients => clients.UserFinishedPlaying(streamer_id, It.Is<SpectatorState>(m => m.State == SpectatedUserState.Failed)), Times.Once());
}
}
}
35 changes: 20 additions & 15 deletions osu.Server.Spectator/Hubs/Spectator/SpectatorHub.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -138,24 +138,11 @@ public async Task EndPlaySession(SpectatorState state)

// Score may be null if the BeginPlaySession call failed but the client is still sending frame data.
// For now it's safe to drop these frames.
// Note that this *intentionally* skips the `endPlaySession()` call at the end of method.
if (score == null || scoreToken == null)
return;

// Do nothing with scores on unranked beatmaps.
var status = score.ScoreInfo.BeatmapInfo!.Status;
if (status < min_beatmap_status_for_replays || status > max_beatmap_status_for_replays)
return;

// if the user never hit anything, further processing that depends on the score existing can be waived because the client won't have submitted the score anyway.
// note that this isn't an early return as we still want to end the play session.
// see: https://github.com/ppy/osu/blob/a47ccb8edd2392258b6b7e176b222a9ecd511fc0/osu.Game/Screens/Play/SubmittingPlayer.cs#L281
if (score.ScoreInfo.Statistics.Any(s => s.Key.IsHit() && s.Value > 0))
{
score.ScoreInfo.Date = DateTimeOffset.UtcNow;

scoreUploader.Enqueue(scoreToken.Value, score);
await scoreProcessedSubscriber.RegisterForNotificationAsync(Context.ConnectionId, Context.GetUserId(), scoreToken.Value);
}
await processScore(score, scoreToken.Value);
}
finally
{
Expand All @@ -166,6 +153,24 @@ public async Task EndPlaySession(SpectatorState state)
await endPlaySession(Context.GetUserId(), state);
}

private async Task processScore(Score score, long scoreToken)
{
// Do nothing with scores on unranked beatmaps.
var status = score.ScoreInfo.BeatmapInfo!.Status;
if (status < min_beatmap_status_for_replays || status > max_beatmap_status_for_replays)
return;

// if the user never hit anything, further processing that depends on the score existing can be waived because the client won't have submitted the score anyway.
// see: https://github.com/ppy/osu/blob/a47ccb8edd2392258b6b7e176b222a9ecd511fc0/osu.Game/Screens/Play/SubmittingPlayer.cs#L281
if (!score.ScoreInfo.Statistics.Any(s => s.Key.IsHit() && s.Value > 0))
return;

score.ScoreInfo.Date = DateTimeOffset.UtcNow;

scoreUploader.Enqueue(scoreToken, score);
await scoreProcessedSubscriber.RegisterForNotificationAsync(Context.ConnectionId, Context.GetUserId(), scoreToken);
}

public async Task StartWatchingUser(int userId)
{
Log($"Watching {userId}");
Expand Down

0 comments on commit 6b1c04a

Please sign in to comment.