Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix judgement counter not showing correct counts when spectating user mid-play #29914

Merged
merged 5 commits into from
Sep 27, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions osu.Game.Tests/Visual/Gameplay/TestSceneSpectator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
using osu.Game.Scoring;
using osu.Game.Screens;
using osu.Game.Screens.Play;
using osu.Game.Screens.Play.HUD.JudgementCounter;
using osu.Game.Tests.Beatmaps.IO;
using osu.Game.Tests.Gameplay;
using osu.Game.Tests.Visual.Multiplayer;
Expand Down Expand Up @@ -167,14 +168,16 @@ public void TestPlayStartsWithNoFrames()
public void TestSpectatingDuringGameplay()
{
start();
sendFrames(300);
sendFrames(300, initialResultCount: 100);

loadSpectatingScreen();
waitForPlayerCurrent();

sendFrames(300);
sendFrames(300, initialResultCount: 100);

AddUntilStep("playing from correct point in time", () => player.ChildrenOfType<DrawableRuleset>().First().FrameStableClock.CurrentTime, () => Is.GreaterThan(30000));
AddAssert("check judgement counts are correct", () => player.ChildrenOfType<JudgementCountController>().Single().Counters.Sum(c => c.ResultCount.Value),
() => Is.GreaterThanOrEqualTo(100));
}

[Test]
Expand Down Expand Up @@ -405,9 +408,9 @@ private double currentFrameStableTime
private void checkPaused(bool state) =>
AddUntilStep($"game is {(state ? "paused" : "playing")}", () => player.ChildrenOfType<DrawableRuleset>().First().IsPaused.Value == state);

private void sendFrames(int count = 10, double startTime = 0)
private void sendFrames(int count = 10, double startTime = 0, int initialResultCount = 0)
{
AddStep("send frames", () => spectatorClient.SendFramesFromUser(streamingUser.Id, count, startTime));
AddStep("send frames", () => spectatorClient.SendFramesFromUser(streamingUser.Id, count, startTime, initialResultCount));
}

private void loadSpectatingScreen()
Expand Down
2 changes: 2 additions & 0 deletions osu.Game/Rulesets/Scoring/ScoreProcessor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,8 @@ public Dictionary<HitResult, int> MaximumStatistics
}
}

public IReadOnlyDictionary<HitResult, int> Statistics => ScoreResultCounts;

private bool beatmapApplied;

protected readonly Dictionary<HitResult, int> ScoreResultCounts = new Dictionary<HitResult, int>();
Expand Down
18 changes: 18 additions & 0 deletions osu.Game/Screens/Play/HUD/JudgementCounter/JudgementCount.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// Copyright (c) ppy Pty Ltd <[email protected]>. Licensed under the MIT Licence.
// See the LICENCE file in the repository root for full licence text.

using osu.Framework.Bindables;
using osu.Framework.Localisation;
using osu.Game.Rulesets.Scoring;

namespace osu.Game.Screens.Play.HUD.JudgementCounter
{
public struct JudgementCount
{
public LocalisableString DisplayName { get; set; }

public HitResult[] Types { get; set; }

public BindableInt ResultCount { get; set; }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
using osu.Framework.Allocation;
using osu.Framework.Bindables;
using osu.Framework.Graphics;
using osu.Framework.Localisation;
using osu.Game.Rulesets;
using osu.Game.Rulesets.Judgements;
using osu.Game.Rulesets.Scoring;
Expand Down Expand Up @@ -53,8 +52,40 @@ protected override void LoadComplete()
{
base.LoadComplete();

scoreProcessor.OnResetFromReplayFrame += updateAllCounts;
scoreProcessor.NewJudgement += judgement => updateCount(judgement, false);
scoreProcessor.JudgementReverted += judgement => updateCount(judgement, true);

updateAllCounts();
}

private void updateAllCounts()
{
// This flow is made to handle cases of watching from the middle of a replay / spectating session.
//
// Once we get an initial state, we can rely on `NewJudgement` and `JudgementReverted`, so
// as a preemptive optimisation, only do a full re-sync if we have all-zero counts.
bool hasCounts = false;

foreach (var r in results)
{
if (r.Value.ResultCount.Value > 0)
{
hasCounts = true;
break;
}
}
Copy link
Collaborator

@bdach bdach Sep 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In testing on production, it appears that this breaks the PR, because it appears that the resets-from-replay-frame occur at the end of a frame bundle, at which point part of gameplay has been replayed and very likely incurred an actual judgement through the expected flows.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've applied a simple fix which looks good on paper and seems to work in initial testing. There's a brief period where counts won't be updated (until the first reset event) but I think this is okay.


if (hasCounts)
return;

foreach (var kvp in scoreProcessor.Statistics)
{
if (!results.TryGetValue(kvp.Key, out var count))
continue;

count.ResultCount.Value = kvp.Value;
}
}

private void updateCount(JudgementResult judgement, bool revert)
Expand All @@ -67,14 +98,5 @@ private void updateCount(JudgementResult judgement, bool revert)
else
count.ResultCount.Value++;
}

public struct JudgementCount
{
public LocalisableString DisplayName { get; set; }

public HitResult[] Types { get; set; }

public BindableInt ResultCount { get; set; }
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
using osu.Game.Graphics;
using osu.Game.Graphics.Sprites;
using osu.Game.Graphics.UserInterface;
using osu.Game.Rulesets;
using osu.Game.Rulesets.Scoring;

namespace osu.Game.Screens.Play.HUD.JudgementCounter
Expand All @@ -19,16 +18,16 @@ public partial class JudgementCounter : VisibilityContainer
public BindableBool ShowName = new BindableBool();
public Bindable<FillDirection> Direction = new Bindable<FillDirection>();

public readonly JudgementCountController.JudgementCount Result;
public readonly JudgementCount Result;

public JudgementCounter(JudgementCountController.JudgementCount result) => Result = result;
public JudgementCounter(JudgementCount result) => Result = result;

public OsuSpriteText ResultName = null!;
private FillFlowContainer flowContainer = null!;
private JudgementRollingCounter counter = null!;

[BackgroundDependencyLoader]
private void load(OsuColour colours, IBindable<RulesetInfo> ruleset)
private void load(OsuColour colours)
{
AutoSizeAxes = Axes.Both;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ private FillDirection getFillDirection(Direction flow)
}
}

private JudgementCounter createCounter(JudgementCountController.JudgementCount info) =>
private JudgementCounter createCounter(JudgementCount info) =>
new JudgementCounter(info)
{
State = { Value = Visibility.Hidden },
Expand Down
26 changes: 24 additions & 2 deletions osu.Game/Tests/Visual/Spectator/TestSpectatorClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
using osu.Game.Online.Spectator;
using osu.Game.Replays.Legacy;
using osu.Game.Rulesets;
using osu.Game.Rulesets.Judgements;
using osu.Game.Rulesets.Objects;
using osu.Game.Rulesets.Replays;
using osu.Game.Rulesets.Scoring;
using osu.Game.Scoring;
Expand Down Expand Up @@ -99,13 +101,24 @@ public void SendEndPlay(int userId, SpectatedUserState state = SpectatedUserStat
/// <param name="userId">The user to send frames for.</param>
/// <param name="count">The total number of frames to send.</param>
/// <param name="startTime">The time to start gameplay frames from.</param>
public void SendFramesFromUser(int userId, int count, double startTime = 0)
/// <param name="initialResultCount">Add a number of misses to frame header data for testing purposes.</param>
public void SendFramesFromUser(int userId, int count, double startTime = 0, int initialResultCount = 0)
{
var frames = new List<LegacyReplayFrame>();

int currentFrameIndex = userNextFrameDictionary[userId];
int lastFrameIndex = currentFrameIndex + count - 1;

var scoreProcessor = new ScoreProcessor(rulesetStore.GetRuleset(0)!.CreateInstance());

for (int i = 0; i < initialResultCount; i++)
{
scoreProcessor.ApplyResult(new JudgementResult(new HitObject(), new Judgement())
{
Type = HitResult.Miss,
});
}

for (; currentFrameIndex <= lastFrameIndex; currentFrameIndex++)
{
// This is done in the next frame so that currentFrameIndex is updated to the correct value.
Expand All @@ -130,7 +143,16 @@ void flush()
Combo = currentFrameIndex,
TotalScore = (long)(currentFrameIndex * 123478 * RNG.NextDouble(0.99, 1.01)),
Accuracy = RNG.NextDouble(0.98, 1),
}, new ScoreProcessor(rulesetStore.GetRuleset(0)!.CreateInstance()), frames.ToArray());
Statistics = scoreProcessor.Statistics.ToDictionary(),
}, scoreProcessor, frames.ToArray());

if (initialResultCount > 0)
{
foreach (var f in frames)
f.Header = bundle.Header;
}

scoreProcessor.ResetFromReplayFrame(frames.Last());
((ISpectatorClient)this).UserSentFrames(userId, bundle);

frames.Clear();
Expand Down
Loading