Fix 1 million score being unachievable on some mania beatmaps

osu.Game.Rulesets.Mania.Tests/TestSceneMaximumScore.cs

@@ -0,0 +1,147 @@
+// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. 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<JudgementResult> judgementResults = new List<JudgementResult>();
+
+        [Test]
+        public void TestSimultaneousTickAndNote()
+        {
+            performTest(
+                new List<ManiaHitObject>
+                {
+                    new HoldNote
+                    {
+                        StartTime = 1000,
+                        Duration = 2000,
+                        Column = 0,
+                    },
+                    new Note
+                    {
+                        StartTime = 2000,
+                        Column = 1
+                    }
+                },
+                new List<ReplayFrame>
+                {
+                    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<ManiaHitObject>
+                {
+                    new HoldNote
+                    {
+                        StartTime = 1000,
+                        Duration = 2000,
+                        Column = 0,
+                    },
+                    new HoldNote
+                    {
+                        StartTime = 2000,
+                        Duration = 2000,
+                        Column = 1
+                    }
+                },
+                new List<ReplayFrame>
+                {
+                    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<ManiaHitObject> hitObjects, List<ReplayFrame> frames)
+        {
+            var beatmap = new Beatmap<ManiaHitObject>
+            {
+                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<JudgementResult>();
+            });
+
+            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,
+                })
+            {
+            }
+        }
+    }
+}

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 @@ public ManiaScoreProcessor()
         {
         }
 
+        protected override IEnumerable<HitObject> 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 @@ protected override double ComputeTotalScore(double comboProgress, double accurac
 
         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<ManiaHitObject>
+        {
+            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);
+            }
+        }
     }
 }

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;
@@ -137,30 +138,17 @@ public virtual void ResetFromReplayFrame(ReplayFrame frame)
                 JudgedHits += count;
         }
 
-        /// <summary>
-        /// Creates the <see cref="JudgementResult"/> that represents the scoring result for a <see cref="HitObject"/>.
-        /// </summary>
-        /// <param name="hitObject">The <see cref="HitObject"/> which was judged.</param>
-        /// <param name="judgement">The <see cref="Judgement"/> that provides the scoring information.</param>
-        protected virtual JudgementResult CreateResult(HitObject hitObject, Judgement judgement) => new JudgementResult(hitObject, judgement);
-
         /// <summary>
         /// Simulates an autoplay of the <see cref="IBeatmap"/> to determine scoring values.
         /// </summary>
         /// <remarks>This provided temporarily. DO NOT USE.</remarks>
         /// <param name="beatmap">The <see cref="IBeatmap"/> to simulate.</param>
-        protected virtual void SimulateAutoplay(IBeatmap beatmap)
+        protected void SimulateAutoplay(IBeatmap beatmap)
         {
             IsSimulating = true;
 
-            foreach (var obj in beatmap.HitObjects)
-                simulate(obj);
-
-            void simulate(HitObject obj)
+            foreach (var obj in EnumerateHitObjects(beatmap))
             {
-                foreach (var nested in obj.NestedHitObjects)
-                    simulate(nested);
-
                 var judgement = obj.CreateJudgement();
 
                 var result = CreateResult(obj, judgement);
@@ -174,22 +162,52 @@ void simulate(HitObject obj)
             IsSimulating = false;
         }
 
-        protected override void Update()
+        /// <summary>
+        /// Enumerates all <see cref="HitObject"/>s in the given <paramref name="beatmap"/> in the order in which they are to be judged.
+        /// Used in <see cref="SimulateAutoplay"/>.
+        /// </summary>
+        /// <remarks>
+        /// 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.
+        /// </remarks>
+        protected virtual IEnumerable<HitObject> EnumerateHitObjects(IBeatmap beatmap)
+            => enumerateRecursively(beatmap.HitObjects);
+
+        private IEnumerable<HitObject> enumerateRecursively(IEnumerable<HitObject> hitObjects)
         {
-            base.Update();
+            foreach (var hitObject in hitObjects)
+            {
+                foreach (var nested in enumerateRecursively(hitObject.NestedHitObjects))
+                    yield return nested;
 
-            hasCompleted.Value =
-                JudgedHits == MaxHits
-                && (JudgedHits == 0
-                    // Last applied result is guaranteed to be non-null when JudgedHits > 0.
-                    || lastAppliedResult.AsNonNull().TimeAbsolute < Clock.CurrentTime);
+                yield return hitObject;
+            }
         }
 
+        /// <summary>
+        /// Creates the <see cref="JudgementResult"/> that represents the scoring result for a <see cref="HitObject"/>.
+        /// </summary>
+        /// <param name="hitObject">The <see cref="HitObject"/> which was judged.</param>
+        /// <param name="judgement">The <see cref="Judgement"/> that provides the scoring information.</param>
+        protected virtual JudgementResult CreateResult(HitObject hitObject, Judgement judgement) => new JudgementResult(hitObject, judgement);
+
         /// <summary>
         /// Gets a simulated <see cref="HitResult"/> for a judgement. Used during <see cref="SimulateAutoplay"/> to simulate a "perfect" play.
         /// </summary>
         /// <param name="judgement">The judgement to simulate a <see cref="HitResult"/> for.</param>
         /// <returns>The simulated <see cref="HitResult"/> for the judgement.</returns>
         protected virtual HitResult GetSimulatedHitResult(Judgement judgement) => judgement.MaxResult;
+
+        protected override void Update()
+        {
+            base.Update();
+
+            hasCompleted.Value =
+                JudgedHits == MaxHits
+                && (JudgedHits == 0
+                    // Last applied result is guaranteed to be non-null when JudgedHits > 0.
+                    || lastAppliedResult.AsNonNull().TimeAbsolute < Clock.CurrentTime);
+        }
     }
 }