From ccf6ed1e5b58f8b6810f589969d752f4681d3fc4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 14 Jun 2023 18:10:49 +0200 Subject: [PATCH 01/11] Add failing test cases --- .../TestSceneMaximumScore.cs | 147 ++++++++++++++++++ 1 file changed, 147 insertions(+) create mode 100644 osu.Game.Rulesets.Mania.Tests/TestSceneMaximumScore.cs diff --git a/osu.Game.Rulesets.Mania.Tests/TestSceneMaximumScore.cs b/osu.Game.Rulesets.Mania.Tests/TestSceneMaximumScore.cs new file mode 100644 index 0000000000..3d0abaceb5 --- /dev/null +++ b/osu.Game.Rulesets.Mania.Tests/TestSceneMaximumScore.cs @@ -0,0 +1,147 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System.Collections.Generic; +using System.Linq; +using NUnit.Framework; +using osu.Framework.Screens; +using osu.Game.Beatmaps; +using osu.Game.Beatmaps.ControlPoints; +using osu.Game.Replays; +using osu.Game.Rulesets.Judgements; +using osu.Game.Rulesets.Mania.Objects; +using osu.Game.Rulesets.Mania.Replays; +using osu.Game.Rulesets.Replays; +using osu.Game.Rulesets.Scoring; +using osu.Game.Scoring; +using osu.Game.Screens.Play; +using osu.Game.Tests.Visual; + +namespace osu.Game.Rulesets.Mania.Tests +{ + public partial class TestSceneMaximumScore : RateAdjustedBeatmapTestScene + { + private ScoreAccessibleReplayPlayer currentPlayer = null!; + + private List judgementResults = new List(); + + [Test] + public void TestSimultaneousTickAndNote() + { + performTest( + new List + { + new HoldNote + { + StartTime = 1000, + Duration = 2000, + Column = 0, + }, + new Note + { + StartTime = 2000, + Column = 1 + } + }, + new List + { + new ManiaReplayFrame(1000, ManiaAction.Key1), + new ManiaReplayFrame(2000, ManiaAction.Key1, ManiaAction.Key2), + new ManiaReplayFrame(2001, ManiaAction.Key1), + new ManiaReplayFrame(3000) + }); + + AddAssert("all objects perfectly judged", + () => judgementResults.Select(result => result.Type), + () => Is.EquivalentTo(judgementResults.Select(result => result.Judgement.MaxResult))); + AddAssert("score is 1 million", () => currentPlayer.ScoreProcessor.TotalScore.Value, () => Is.EqualTo(1_000_000)); + } + + [Test] + public void TestSimultaneousLongNotes() + { + performTest( + new List + { + new HoldNote + { + StartTime = 1000, + Duration = 2000, + Column = 0, + }, + new HoldNote + { + StartTime = 2000, + Duration = 2000, + Column = 1 + } + }, + new List + { + new ManiaReplayFrame(1000, ManiaAction.Key1), + new ManiaReplayFrame(2000, ManiaAction.Key1, ManiaAction.Key2), + new ManiaReplayFrame(3000, ManiaAction.Key2), + new ManiaReplayFrame(4000) + }); + + AddAssert("all objects perfectly judged", + () => judgementResults.Select(result => result.Type), + () => Is.EquivalentTo(judgementResults.Select(result => result.Judgement.MaxResult))); + AddAssert("score is 1 million", () => currentPlayer.ScoreProcessor.TotalScore.Value, () => Is.EqualTo(1_000_000)); + } + + private void performTest(List hitObjects, List frames) + { + var beatmap = new Beatmap + { + HitObjects = hitObjects, + BeatmapInfo = + { + Difficulty = new BeatmapDifficulty { SliderTickRate = 4 }, + Ruleset = new ManiaRuleset().RulesetInfo + }, + }; + + beatmap.ControlPointInfo.Add(0, new EffectControlPoint { ScrollSpeed = 0.1f }); + + AddStep("load player", () => + { + Beatmap.Value = CreateWorkingBeatmap(beatmap); + + var p = new ScoreAccessibleReplayPlayer(new Score { Replay = new Replay { Frames = frames } }); + + p.OnLoadComplete += _ => + { + p.ScoreProcessor.NewJudgement += result => + { + if (currentPlayer == p) judgementResults.Add(result); + }; + }; + + LoadScreen(currentPlayer = p); + judgementResults = new List(); + }); + + AddUntilStep("Beatmap at 0", () => Beatmap.Value.Track.CurrentTime == 0); + AddUntilStep("Wait until player is loaded", () => currentPlayer.IsCurrentScreen()); + + AddUntilStep("Wait for completion", () => currentPlayer.ScoreProcessor.HasCompleted.Value); + } + + private partial class ScoreAccessibleReplayPlayer : ReplayPlayer + { + public new ScoreProcessor ScoreProcessor => base.ScoreProcessor; + + protected override bool PauseOnFocusLost => false; + + public ScoreAccessibleReplayPlayer(Score score) + : base(score, new PlayerConfiguration + { + AllowPause = false, + ShowResults = false, + }) + { + } + } + } +} From 04e812b5ab6f2098e548ca8f292053a2f84d2704 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 14 Jun 2023 20:08:14 +0200 Subject: [PATCH 02/11] Make `JudgementProcessor.SimulateAutoplay()` non-virtual Nobody overrides this, and with the structure given, overriders would have to rewrite half of this code anyway. The fact that the class has 2 other overridable members (`CreateResult()`, `GetSimulatedHitResult()`) which cease to have any meaning if `SimulateAutoplay()` is overridden also contributes to taking this decision. --- osu.Game/Rulesets/Scoring/JudgementProcessor.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Rulesets/Scoring/JudgementProcessor.cs b/osu.Game/Rulesets/Scoring/JudgementProcessor.cs index b16c307206..181fbef405 100644 --- a/osu.Game/Rulesets/Scoring/JudgementProcessor.cs +++ b/osu.Game/Rulesets/Scoring/JudgementProcessor.cs @@ -149,7 +149,7 @@ namespace osu.Game.Rulesets.Scoring /// /// This provided temporarily. DO NOT USE. /// The to simulate. - protected virtual void SimulateAutoplay(IBeatmap beatmap) + protected void SimulateAutoplay(IBeatmap beatmap) { IsSimulating = true; From 3295294cbc4cc324dd8281c8ed1258ea7eeed0ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 14 Jun 2023 20:10:34 +0200 Subject: [PATCH 03/11] Reorder autoplay-related virtual methods closer together --- .../Rulesets/Scoring/JudgementProcessor.cs | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/osu.Game/Rulesets/Scoring/JudgementProcessor.cs b/osu.Game/Rulesets/Scoring/JudgementProcessor.cs index 181fbef405..9c86cbfe90 100644 --- a/osu.Game/Rulesets/Scoring/JudgementProcessor.cs +++ b/osu.Game/Rulesets/Scoring/JudgementProcessor.cs @@ -137,13 +137,6 @@ namespace osu.Game.Rulesets.Scoring JudgedHits += count; } - /// - /// Creates the that represents the scoring result for a . - /// - /// The which was judged. - /// The that provides the scoring information. - protected virtual JudgementResult CreateResult(HitObject hitObject, Judgement judgement) => new JudgementResult(hitObject, judgement); - /// /// Simulates an autoplay of the to determine scoring values. /// @@ -174,6 +167,20 @@ namespace osu.Game.Rulesets.Scoring IsSimulating = false; } + /// + /// Creates the that represents the scoring result for a . + /// + /// The which was judged. + /// The that provides the scoring information. + protected virtual JudgementResult CreateResult(HitObject hitObject, Judgement judgement) => new JudgementResult(hitObject, judgement); + + /// + /// Gets a simulated for a judgement. Used during to simulate a "perfect" play. + /// + /// The judgement to simulate a for. + /// The simulated for the judgement. + protected virtual HitResult GetSimulatedHitResult(Judgement judgement) => judgement.MaxResult; + protected override void Update() { base.Update(); @@ -184,12 +191,5 @@ namespace osu.Game.Rulesets.Scoring // Last applied result is guaranteed to be non-null when JudgedHits > 0. || lastAppliedResult.AsNonNull().TimeAbsolute < Clock.CurrentTime); } - - /// - /// Gets a simulated for a judgement. Used during to simulate a "perfect" play. - /// - /// The judgement to simulate a for. - /// The simulated for the judgement. - protected virtual HitResult GetSimulatedHitResult(Judgement judgement) => judgement.MaxResult; } } From 462570801a1441580af32e049400c2f682fb876d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 14 Jun 2023 20:15:43 +0200 Subject: [PATCH 04/11] Introduce new method for enumeration of objects during autoplay simulation --- .../Rulesets/Scoring/JudgementProcessor.cs | 29 ++++++++++++++++--- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/osu.Game/Rulesets/Scoring/JudgementProcessor.cs b/osu.Game/Rulesets/Scoring/JudgementProcessor.cs index 9c86cbfe90..2c42a08864 100644 --- a/osu.Game/Rulesets/Scoring/JudgementProcessor.cs +++ b/osu.Game/Rulesets/Scoring/JudgementProcessor.cs @@ -2,6 +2,7 @@ // See the LICENCE file in the repository root for full licence text. using System; +using System.Collections.Generic; using osu.Framework.Bindables; using osu.Framework.Extensions.ObjectExtensions; using osu.Framework.Extensions.TypeExtensions; @@ -146,14 +147,11 @@ namespace osu.Game.Rulesets.Scoring { IsSimulating = true; - foreach (var obj in beatmap.HitObjects) + foreach (var obj in EnumerateHitObjects(beatmap)) simulate(obj); void simulate(HitObject obj) { - foreach (var nested in obj.NestedHitObjects) - simulate(nested); - var judgement = obj.CreateJudgement(); var result = CreateResult(obj, judgement); @@ -167,6 +165,29 @@ namespace osu.Game.Rulesets.Scoring IsSimulating = false; } + /// + /// Enumerates all s in the given in the order in which they are to be judged. + /// Used in . + /// + /// + /// In Score V2, the score awarded for each object includes a component based on the combo value after the judgement of that object. + /// This means that the score is dependent on the order of evaluation of judgements. + /// This method is provided so that rulesets can specify custom ordering that is correct for them and matches processing order during actual gameplay. + /// + protected virtual IEnumerable EnumerateHitObjects(IBeatmap beatmap) + => enumerateRecursively(beatmap.HitObjects); + + private IEnumerable enumerateRecursively(IEnumerable hitObjects) + { + foreach (var hitObject in hitObjects) + { + foreach (var nested in enumerateRecursively(hitObject.NestedHitObjects)) + yield return nested; + + yield return hitObject; + } + } + /// /// Creates the that represents the scoring result for a . /// From e46b4209c3efecf10143bf2b7ebb4a4b6bbe821d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 14 Jun 2023 20:35:49 +0200 Subject: [PATCH 05/11] Remove no-longer-needed local method --- osu.Game/Rulesets/Scoring/JudgementProcessor.cs | 3 --- 1 file changed, 3 deletions(-) diff --git a/osu.Game/Rulesets/Scoring/JudgementProcessor.cs b/osu.Game/Rulesets/Scoring/JudgementProcessor.cs index 2c42a08864..e9f3bcb949 100644 --- a/osu.Game/Rulesets/Scoring/JudgementProcessor.cs +++ b/osu.Game/Rulesets/Scoring/JudgementProcessor.cs @@ -148,9 +148,6 @@ namespace osu.Game.Rulesets.Scoring IsSimulating = true; foreach (var obj in EnumerateHitObjects(beatmap)) - simulate(obj); - - void simulate(HitObject obj) { var judgement = obj.CreateJudgement(); From 0065334241f77b703c1c7523ab0e917a4ae53fb1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 14 Jun 2023 20:34:06 +0200 Subject: [PATCH 06/11] Fix mania autoplay simulation judging objects in different order to gameplay --- .../Scoring/ManiaScoreProcessor.cs | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/osu.Game.Rulesets.Mania/Scoring/ManiaScoreProcessor.cs b/osu.Game.Rulesets.Mania/Scoring/ManiaScoreProcessor.cs index 6292ed75cd..a0f6ac572d 100644 --- a/osu.Game.Rulesets.Mania/Scoring/ManiaScoreProcessor.cs +++ b/osu.Game.Rulesets.Mania/Scoring/ManiaScoreProcessor.cs @@ -2,7 +2,12 @@ // See the LICENCE file in the repository root for full licence text. using System; +using System.Collections.Generic; +using System.Linq; +using osu.Game.Beatmaps; using osu.Game.Rulesets.Judgements; +using osu.Game.Rulesets.Mania.Objects; +using osu.Game.Rulesets.Objects; using osu.Game.Rulesets.Scoring; namespace osu.Game.Rulesets.Mania.Scoring @@ -16,6 +21,9 @@ namespace osu.Game.Rulesets.Mania.Scoring { } + protected override IEnumerable EnumerateHitObjects(IBeatmap beatmap) + => base.EnumerateHitObjects(beatmap).OrderBy(ho => (ManiaHitObject)ho, JudgementOrderComparer.DEFAULT); + protected override double ComputeTotalScore(double comboProgress, double accuracyProgress, double bonusPortion) { return 10000 * comboProgress @@ -25,5 +33,27 @@ namespace osu.Game.Rulesets.Mania.Scoring protected override double GetComboScoreChange(JudgementResult result) => Judgement.ToNumericResult(result.Type) * Math.Min(Math.Max(0.5, Math.Log(result.ComboAfterJudgement, combo_base)), Math.Log(400, combo_base)); + + private class JudgementOrderComparer : IComparer + { + public static readonly JudgementOrderComparer DEFAULT = new JudgementOrderComparer(); + + public int Compare(ManiaHitObject? x, ManiaHitObject? y) + { + if (ReferenceEquals(x, y)) return 0; + if (ReferenceEquals(x, null)) return -1; + if (ReferenceEquals(y, null)) return 1; + + int result = x.GetEndTime().CompareTo(y.GetEndTime()); + if (result != 0) + return result; + + // due to the way input is handled in mania, notes take precedence over ticks in judging order. + if (x is Note && y is not Note) return -1; + if (x is not Note && y is Note) return 1; + + return x.Column.CompareTo(y.Column); + } + } } } From ca9c31b492dc097b92c172703bfdaa5a4ce8d4a3 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 11 Jul 2023 17:29:28 +0900 Subject: [PATCH 07/11] Add test coverage of slider blueprint end placement failing outside playfield --- .../Editor/TestSceneSliderPlacementBlueprint.cs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderPlacementBlueprint.cs b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderPlacementBlueprint.cs index 7542e00a94..1d136fe9cc 100644 --- a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderPlacementBlueprint.cs +++ b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderPlacementBlueprint.cs @@ -61,6 +61,21 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor assertControlPointType(0, PathType.Linear); } + [Test] + public void TestPlaceWithMouseMovementOutsidePlayfield() + { + addMovementStep(new Vector2(200)); + addClickStep(MouseButton.Left); + + addMovementStep(new Vector2(1400, 200)); + addClickStep(MouseButton.Right); + + assertPlaced(true); + assertLength(1200); + assertControlPointCount(2); + assertControlPointType(0, PathType.Linear); + } + [Test] public void TestPlaceNormalControlPoint() { From a0e6748882caf26d24ec526ec882a47968b7505a Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 11 Jul 2023 17:29:53 +0900 Subject: [PATCH 08/11] Fix slider blueprint placement when ending placement outside the playfield As mentioned in https://github.com/ppy/osu/discussions/24161 --- osu.Game/Rulesets/Edit/PlacementBlueprint.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Rulesets/Edit/PlacementBlueprint.cs b/osu.Game/Rulesets/Edit/PlacementBlueprint.cs index 717c026ded..5cb9adfd72 100644 --- a/osu.Game/Rulesets/Edit/PlacementBlueprint.cs +++ b/osu.Game/Rulesets/Edit/PlacementBlueprint.cs @@ -171,7 +171,7 @@ namespace osu.Game.Rulesets.Edit /// protected void ApplyDefaultsToHitObject() => HitObject.ApplyDefaults(beatmap.ControlPointInfo, beatmap.Difficulty); - public override bool ReceivePositionalInputAt(Vector2 screenSpacePos) => Parent?.ReceivePositionalInputAt(screenSpacePos) == true; + public override bool ReceivePositionalInputAt(Vector2 screenSpacePos) => true; protected override bool Handle(UIEvent e) { From 87570ed238bb303237d40aa3a51a3af46317e17d Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 12 Jul 2023 17:23:31 +0900 Subject: [PATCH 09/11] Fix incorrect slider stacking on very old beatmaps Closes https://github.com/ppy/osu/issues/24185 The stable code has had a bug in this logic forever. So we'll need to reimplement the bug. Basically, sliders have to have `UpdateCalculations` run in order to have a correct `Position2` and `EndTime`, but this wasn't being called in the inner loop before use of `EndTime` at https://github.com/peppy/osu-stable-reference/blob/1531237b63392e82c003c712faa028406073aa8f/osu!/GameplayElements/HitObjectManager.cs#L1813. To fix this, we use `StartTime` in the inner loop to reproduce the bug. --- osu.Game.Rulesets.Osu/Beatmaps/OsuBeatmapProcessor.cs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Beatmaps/OsuBeatmapProcessor.cs b/osu.Game.Rulesets.Osu/Beatmaps/OsuBeatmapProcessor.cs index f51f04bf87..c081df3ac6 100644 --- a/osu.Game.Rulesets.Osu/Beatmaps/OsuBeatmapProcessor.cs +++ b/osu.Game.Rulesets.Osu/Beatmaps/OsuBeatmapProcessor.cs @@ -214,17 +214,24 @@ namespace osu.Game.Rulesets.Osu.Beatmaps ? currSlider.Position + currSlider.Path.PositionAt(1) : currHitObject.Position; + // Note the use of `StartTime` in the code below doesn't match stable's use of `EndTime`. + // This is because in the stable implementation, `UpdateCalculations` is not called on the inner-loop hitobject (j) + // and therefore it does not have a correct `EndTime`, but instead the default of `EndTime = StartTime`. + // + // Effects of this can be seen on https://osu.ppy.sh/beatmapsets/243#osu/1146 at sliders around 86647 ms, where + // if we use `EndTime` here it would result in unexpected stacking. + if (Vector2Extensions.Distance(beatmap.HitObjects[j].Position, currHitObject.Position) < stack_distance) { currHitObject.StackHeight++; - startTime = beatmap.HitObjects[j].GetEndTime(); + startTime = beatmap.HitObjects[j].StartTime; } else if (Vector2Extensions.Distance(beatmap.HitObjects[j].Position, position2) < stack_distance) { // Case for sliders - bump notes down and right, rather than up and left. sliderStack++; beatmap.HitObjects[j].StackHeight -= sliderStack; - startTime = beatmap.HitObjects[j].GetEndTime(); + startTime = beatmap.HitObjects[j].StartTime; } } } From d12845d7b1ecb007b6d2c9602e1042d192842b12 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 12 Jul 2023 17:39:54 +0900 Subject: [PATCH 10/11] Remove no-longer-necessary `ReceivePositionalInputAt` overide in `CatchPlacementBlueprint` --- .../Edit/Blueprints/CatchPlacementBlueprint.cs | 3 --- 1 file changed, 3 deletions(-) diff --git a/osu.Game.Rulesets.Catch/Edit/Blueprints/CatchPlacementBlueprint.cs b/osu.Game.Rulesets.Catch/Edit/Blueprints/CatchPlacementBlueprint.cs index d2d605a6fe..1a2990e4ac 100644 --- a/osu.Game.Rulesets.Catch/Edit/Blueprints/CatchPlacementBlueprint.cs +++ b/osu.Game.Rulesets.Catch/Edit/Blueprints/CatchPlacementBlueprint.cs @@ -6,7 +6,6 @@ using osu.Game.Rulesets.Catch.Objects; using osu.Game.Rulesets.Edit; using osu.Game.Rulesets.UI; using osu.Game.Rulesets.UI.Scrolling; -using osuTK; namespace osu.Game.Rulesets.Catch.Edit.Blueprints { @@ -24,7 +23,5 @@ namespace osu.Game.Rulesets.Catch.Edit.Blueprints : base(new THitObject()) { } - - public override bool ReceivePositionalInputAt(Vector2 screenSpacePos) => true; } } From 547f2476694ace63aa44d8f55324e61804030f2d Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 12 Jul 2023 17:41:58 +0900 Subject: [PATCH 11/11] Fix test to work regardless of screen sizes --- .../Editor/TestSceneSliderPlacementBlueprint.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderPlacementBlueprint.cs b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderPlacementBlueprint.cs index 1d136fe9cc..7d29670daa 100644 --- a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderPlacementBlueprint.cs +++ b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderPlacementBlueprint.cs @@ -67,11 +67,10 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor addMovementStep(new Vector2(200)); addClickStep(MouseButton.Left); - addMovementStep(new Vector2(1400, 200)); + AddStep("move mouse out of screen", () => InputManager.MoveMouseTo(InputManager.ScreenSpaceDrawQuad.TopRight + Vector2.One)); addClickStep(MouseButton.Right); assertPlaced(true); - assertLength(1200); assertControlPointCount(2); assertControlPointType(0, PathType.Linear); }