diff --git a/osu.Game.Rulesets.Mania.Tests/TestSceneReplayRewinding.cs b/osu.Game.Rulesets.Mania.Tests/TestSceneReplayRewinding.cs index b58046c9e9..5216358a8b 100644 --- a/osu.Game.Rulesets.Mania.Tests/TestSceneReplayRewinding.cs +++ b/osu.Game.Rulesets.Mania.Tests/TestSceneReplayRewinding.cs @@ -65,5 +65,61 @@ namespace osu.Game.Rulesets.Mania.Tests AddStep(@"exit player", () => currentPlayer.Exit()); } + + [Test] + public void TestCorrectComboAccountingForConcurrentObjects() + { + Score score = null!; + + var beatmap = new ManiaBeatmap(new StageDefinition(4)) + { + HitObjects = + { + new Note + { + StartTime = 500, + Column = 0, + }, + new Note + { + StartTime = 500, + Column = 2, + }, + new HoldNote + { + StartTime = 1000, + EndTime = 1500, + Column = 1, + } + } + }; + + AddStep(@"create replay", () => score = new Score + { + Replay = new Replay + { + Frames = + { + new ManiaReplayFrame(500, ManiaAction.Key1, ManiaAction.Key3), + new ManiaReplayFrame(520), + new ManiaReplayFrame(1000, ManiaAction.Key2), + new ManiaReplayFrame(1500), + } + }, + ScoreInfo = new ScoreInfo() + }); + + AddStep(@"set beatmap", () => Beatmap.Value = CreateWorkingBeatmap(beatmap)); + AddStep(@"set ruleset", () => Ruleset.Value = beatmap.BeatmapInfo.Ruleset); + AddStep(@"push player", () => LoadScreen(currentPlayer = new ReplayPlayer(score))); + + AddUntilStep(@"wait until player is loaded", () => currentPlayer.IsCurrentScreen()); + AddUntilStep(@"wait for objects to be judged", () => currentPlayer.ChildrenOfType().Single().CurrentTime, () => Is.GreaterThan(1600)); + AddStep(@"stop gameplay", () => currentPlayer.ChildrenOfType().Single().Stop()); + AddStep(@"seek to start", () => currentPlayer.Seek(0)); + AddAssert(@"combo is 0", () => currentPlayer.GameplayState.ScoreProcessor.Combo.Value, () => Is.Zero); + + AddStep(@"exit player", () => currentPlayer.Exit()); + } } } diff --git a/osu.Game.Tests/Rulesets/Scoring/ScoreProcessorTest.cs b/osu.Game.Tests/Rulesets/Scoring/ScoreProcessorTest.cs index 1647fbee42..f45422e0c4 100644 --- a/osu.Game.Tests/Rulesets/Scoring/ScoreProcessorTest.cs +++ b/osu.Game.Tests/Rulesets/Scoring/ScoreProcessorTest.cs @@ -421,6 +421,65 @@ namespace osu.Game.Tests.Rulesets.Scoring Assert.That(scoreProcessor.Rank.Value, Is.EqualTo(ScoreRank.SH)); } + [Test] + public void TestComboAccounting([Values] bool shuffleResults) + { + var testBeatmap = new Beatmap + { + HitObjects = Enumerable.Range(1, 40).Select(i => new TestHitObject(HitResult.Perfect, HitResult.Miss)).ToList(), + }; + scoreProcessor.ApplyBeatmap(testBeatmap); + + var results = new List(); + JudgementResult judgementResult; + + for (int i = 0; i < 25; ++i) + { + judgementResult = new JudgementResult(testBeatmap.HitObjects[i], new TestJudgement(HitResult.Perfect, HitResult.Miss)) + { + Type = HitResult.Perfect + }; + results.Add(judgementResult); + scoreProcessor.ApplyResult(judgementResult); + Assert.That(scoreProcessor.Combo.Value, Is.EqualTo(i + 1)); + } + + judgementResult = new JudgementResult(testBeatmap.HitObjects[25], new TestJudgement(HitResult.Perfect, HitResult.Miss)) + { + Type = HitResult.Miss + }; + results.Add(judgementResult); + scoreProcessor.ApplyResult(judgementResult); + Assert.That(scoreProcessor.Combo.Value, Is.EqualTo(0)); + + for (int i = 26; i < 40; ++i) + { + judgementResult = new JudgementResult(testBeatmap.HitObjects[i], new TestJudgement(HitResult.Perfect, HitResult.Miss)) + { + Type = HitResult.Perfect + }; + results.Add(judgementResult); + scoreProcessor.ApplyResult(judgementResult); + Assert.That(scoreProcessor.Combo.Value, Is.EqualTo(i - 25)); + } + + Assert.That(scoreProcessor.MaximumStatistics[HitResult.Perfect], Is.EqualTo(40)); + Assert.That(scoreProcessor.Combo.Value, Is.EqualTo(14)); + Assert.That(scoreProcessor.HighestCombo.Value, Is.EqualTo(25)); + + // random shuffle is VERY extreme and overkill. + // it might not work correctly for any other `ScoreProcessor` property, and the intermediate results likely make no sense. + // the goal is only to demonstrate idempotency to zero when reverting all results. + var random = new Random(20250519); + var toRevert = shuffleResults ? results.OrderBy(_ => random.Next()).ToList() : Enumerable.Reverse(results); + + foreach (var result in toRevert) + scoreProcessor.RevertResult(result); + + Assert.That(scoreProcessor.Combo.Value, Is.Zero); + Assert.That(scoreProcessor.HighestCombo.Value, Is.Zero); + } + private class TestJudgement : Judgement { public override HitResult MaxResult { get; } diff --git a/osu.Game/Rulesets/Judgements/JudgementResult.cs b/osu.Game/Rulesets/Judgements/JudgementResult.cs index 4b98df50d7..ab83ee62b0 100644 --- a/osu.Game/Rulesets/Judgements/JudgementResult.cs +++ b/osu.Game/Rulesets/Judgements/JudgementResult.cs @@ -74,6 +74,11 @@ namespace osu.Game.Rulesets.Judgements /// public int HighestComboAtJudgement { get; internal set; } + /// + /// The highest combo achieved after this occurred. + /// + public int HighestComboAfterJudgement { get; internal set; } + /// /// The health prior to this occurring. /// diff --git a/osu.Game/Rulesets/Scoring/ScoreProcessor.cs b/osu.Game/Rulesets/Scoring/ScoreProcessor.cs index 7b5af9beda..3663e7f008 100644 --- a/osu.Game/Rulesets/Scoring/ScoreProcessor.cs +++ b/osu.Game/Rulesets/Scoring/ScoreProcessor.cs @@ -202,7 +202,6 @@ namespace osu.Game.Rulesets.Scoring { Ruleset = ruleset; - Combo.ValueChanged += combo => HighestCombo.Value = Math.Max(HighestCombo.Value, combo.NewValue); Accuracy.ValueChanged += _ => updateRank(); Mods.ValueChanged += mods => @@ -238,7 +237,10 @@ namespace osu.Game.Rulesets.Scoring else if (result.Type.BreaksCombo()) Combo.Value = 0; + HighestCombo.Value = Math.Max(HighestCombo.Value, Combo.Value); + result.ComboAfterJudgement = Combo.Value; + result.HighestComboAfterJudgement = HighestCombo.Value; if (result.Judgement.MaxResult.AffectsAccuracy()) { @@ -281,8 +283,11 @@ namespace osu.Game.Rulesets.Scoring if (!TrackHitEvents) throw new InvalidOperationException(@$"Rewind is not supported when {nameof(TrackHitEvents)} is disabled."); - Combo.Value = result.ComboAtJudgement; - HighestCombo.Value = result.HighestComboAtJudgement; + // the reason this is written so funnily rather than just using `ComboAtJudgement` + // is to nullify impact of ordering when reverting concurrent judgement results + // (think mania and multiple judgements within a frame). + Combo.Value -= (result.ComboAfterJudgement - result.ComboAtJudgement); + HighestCombo.Value -= (result.HighestComboAfterJudgement - result.HighestComboAtJudgement); if (result.FailedAtJudgement && !ApplyNewJudgementsWhenFailed) return;