From e1603205f57e0cd98058cb71b7f8b904d579488a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Mon, 6 May 2024 17:54:18 +0200 Subject: [PATCH 1/3] Upgrade Sentry to match game Spectator server dies at runtime without this. --- osu.Server.Spectator/osu.Server.Spectator.csproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Server.Spectator/osu.Server.Spectator.csproj b/osu.Server.Spectator/osu.Server.Spectator.csproj index 604a82cd..34a13025 100644 --- a/osu.Server.Spectator/osu.Server.Spectator.csproj +++ b/osu.Server.Spectator/osu.Server.Spectator.csproj @@ -21,7 +21,7 @@ - + From 9b042bb40cd27aced733063b61c5514943d0d3e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Mon, 6 May 2024 17:54:47 +0200 Subject: [PATCH 2/3] Add failing test for correct rank population --- .../SpectatorHubTest.cs | 55 +++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/osu.Server.Spectator.Tests/SpectatorHubTest.cs b/osu.Server.Spectator.Tests/SpectatorHubTest.cs index fb6ef13b..211ef002 100644 --- a/osu.Server.Spectator.Tests/SpectatorHubTest.cs +++ b/osu.Server.Spectator.Tests/SpectatorHubTest.cs @@ -2,6 +2,7 @@ // See the LICENCE file in the repository root for full licence text. using System; +using System.Collections.Generic; using System.Threading.Tasks; using Microsoft.AspNetCore.SignalR; using Microsoft.Extensions.Caching.Distributed; @@ -436,5 +437,59 @@ await hub.EndPlaySession(new SpectatorState mockScoreStorage.Verify(s => s.WriteAsync(It.Is(score => score.ScoreInfo.OnlineID == 456)), Times.Never); mockReceiver.Verify(clients => clients.UserFinishedPlaying(streamer_id, It.Is(m => m.State == SpectatedUserState.Failed)), Times.Once()); } + + [Fact] + public async Task ScoreRankPopulatedCorrectly() + { + AppSettings.SaveReplays = true; + + Mock> mockClients = new Mock>(); + Mock mockReceiver = new Mock(); + mockClients.Setup(clients => clients.All).Returns(mockReceiver.Object); + mockClients.Setup(clients => clients.Group(SpectatorHub.GetGroupId(streamer_id))).Returns(mockReceiver.Object); + + Mock mockContext = new Mock(); + + mockContext.Setup(context => context.UserIdentifier).Returns(streamer_id.ToString()); + hub.Context = mockContext.Object; + hub.Clients = mockClients.Object; + + mockDatabase.Setup(db => db.GetScoreFromToken(1234)).Returns(Task.FromResult(new SoloScore + { + id = 456, + passed = true + })); + + await hub.BeginPlaySession(1234, new SpectatorState + { + BeatmapID = beatmap_id, + RulesetID = 0, + State = SpectatedUserState.Playing, + }); + + await hub.SendFrameData(new FrameDataBundle( + new FrameHeader(new ScoreInfo + { + Accuracy = 0.95, + Statistics = new Dictionary + { + [HitResult.Great] = 19, + [HitResult.Miss] = 1, + } + }, new ScoreProcessorStatistics()), + new[] { new LegacyReplayFrame(1234, 0, 0, ReplayButtonState.None) })); + + await hub.EndPlaySession(new SpectatorState + { + BeatmapID = beatmap_id, + RulesetID = 0, + State = SpectatedUserState.Passed, + }); + + await scoreUploader.Flush(); + + mockScoreStorage.Verify(s => s.WriteAsync(It.Is(score => score.ScoreInfo.Rank == ScoreRank.A)), Times.Once); + mockReceiver.Verify(clients => clients.UserFinishedPlaying(streamer_id, It.Is(m => m.State == SpectatedUserState.Passed)), Times.Once()); + } } } From 3723905645531affeba79d0aa3c92ca9e9577f29 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Mon, 6 May 2024 17:54:49 +0200 Subject: [PATCH 3/3] Ensure ranks are properly populated in spectated scores This begins to matter after https://github.com/ppy/osu/pull/28058, as with that pull the rank is encoded to the replay, so it must be correctly set on the server side. Otherwise spectator server will default to storing D rank, which the client will happily read and use raw without checking. This is purposefully computed server-side rather than sent in replay headers for the reasons of simplicity and backwards compatibility. You could add score rank to the replay headers, but to ensure that replays recorded with old clients don't have D rank set, recomputing server-side would have to be done at least for the old clients. And you could argue recomputing ranks always server-side removes a tamper vector too (although that's not a very strong argument). --- osu.Server.Spectator/Hubs/Spectator/SpectatorHub.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/osu.Server.Spectator/Hubs/Spectator/SpectatorHub.cs b/osu.Server.Spectator/Hubs/Spectator/SpectatorHub.cs index 052194b8..756039e1 100644 --- a/osu.Server.Spectator/Hubs/Spectator/SpectatorHub.cs +++ b/osu.Server.Spectator/Hubs/Spectator/SpectatorHub.cs @@ -7,6 +7,7 @@ using System.Threading.Tasks; using Microsoft.Extensions.Caching.Distributed; using osu.Game.Beatmaps; +using osu.Game.Database; using osu.Game.Online.Spectator; using osu.Game.Rulesets.Scoring; using osu.Game.Scoring; @@ -116,6 +117,7 @@ public async Task SendFrameData(FrameDataBundle data) if (score == null) return; + score.ScoreInfo.Accuracy = data.Header.Accuracy; score.ScoreInfo.Statistics = data.Header.Statistics; score.ScoreInfo.MaxCombo = data.Header.MaxCombo; score.ScoreInfo.Combo = data.Header.Combo; @@ -166,6 +168,9 @@ private async Task processScore(Score score, long scoreToken) return; score.ScoreInfo.Date = DateTimeOffset.UtcNow; + // this call is a little expensive due to reflection usage, so only run it at the end of score processing + // even though in theory the rank could be recomputed after every replay frame. + score.ScoreInfo.Rank = StandardisedScoreMigrationTools.ComputeRank(score.ScoreInfo); scoreUploader.Enqueue(scoreToken, score); await scoreProcessedSubscriber.RegisterForNotificationAsync(Context.ConnectionId, Context.GetUserId(), scoreToken);