Merge pull request #25111 from peppy/mania-bonus-refactor

Change osu!mania "perfect" judgements to only award bonus score

osu.Game.Rulesets.Mania.Tests/Mods/TestSceneManiaModDoubleTime.cs

@@ -17,12 +17,16 @@ namespace osu.Game.Rulesets.Mania.Tests.Mods
     {
         private const double offset = 18;
 
+        protected override bool AllowFail => true;
+
         protected override Ruleset CreatePlayerRuleset() => new ManiaRuleset();
 
         [Test]
         public void TestHitWindowWithoutDoubleTime() => CreateModTest(new ModTestData
         {
-            PassCondition = () => Player.ScoreProcessor.JudgedHits > 0 && Player.ScoreProcessor.Accuracy.Value != 1,
+            PassCondition = () => Player.ScoreProcessor.JudgedHits > 0
+                                  && Player.ScoreProcessor.Accuracy.Value == 1
+                                  && Player.ScoreProcessor.TotalScore.Value == 1_000_000,
             Autoplay = false,
             Beatmap = new Beatmap
             {
@@ -40,24 +44,31 @@ namespace osu.Game.Rulesets.Mania.Tests.Mods
         });
 
         [Test]
-        public void TestHitWindowWithDoubleTime() => CreateModTest(new ModTestData
+        public void TestHitWindowWithDoubleTime()
         {
-            Mod = new ManiaModDoubleTime(),
+            var doubleTime = new ManiaModDoubleTime();
-            PassCondition = () => Player.ScoreProcessor.JudgedHits > 0 && Player.ScoreProcessor.Accuracy.Value == 1,
+
-            Autoplay = false,
+            CreateModTest(new ModTestData
-            Beatmap = new Beatmap
             {
-                BeatmapInfo = { Ruleset = new ManiaRuleset().RulesetInfo },
+                Mod = doubleTime,
-                Difficulty = { OverallDifficulty = 10 },
+                PassCondition = () => Player.ScoreProcessor.JudgedHits > 0
-                HitObjects = new List<HitObject>
+                                      && Player.ScoreProcessor.Accuracy.Value == 1
+                                      && Player.ScoreProcessor.TotalScore.Value == (long)(1_000_010 * doubleTime.ScoreMultiplier),
+                Autoplay = false,
+                Beatmap = new Beatmap
                 {
-                    new Note { StartTime = 1000 }
+                    BeatmapInfo = { Ruleset = new ManiaRuleset().RulesetInfo },
+                    Difficulty = { OverallDifficulty = 10 },
+                    HitObjects = new List<HitObject>
+                    {
+                        new Note { StartTime = 1000 }
+                    },
                 },
-            },
+                ReplayFrames = new List<ReplayFrame>
-            ReplayFrames = new List<ReplayFrame>
+                {
-            {
+                    new ManiaReplayFrame(1000 + offset, ManiaAction.Key1)
-                new ManiaReplayFrame(1000 + offset, ManiaAction.Key1)
+                }
-            }
+            });
-        });
+        }
     }
 }

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

@@ -200,10 +200,12 @@ namespace osu.Game.Rulesets.Mania.Tests
             });
 
             assertHeadJudgement(HitResult.Perfect);
-            assertComboAtJudgement(0, 1);
+            // judgement combo offset by perfect bonus judgement. see logic in DrawableNote.CheckForResult.
+            assertComboAtJudgement(1, 1);
             assertTailJudgement(HitResult.Meh);
-            assertComboAtJudgement(1, 0);
+            assertComboAtJudgement(2, 0);
-            assertComboAtJudgement(2, 1);
+            // judgement combo offset by perfect bonus judgement. see logic in DrawableNote.CheckForResult.
+            assertComboAtJudgement(4, 1);
         }
 
         /// <summary>
@@ -380,7 +382,8 @@ namespace osu.Game.Rulesets.Mania.Tests
         [Test]
         public void TestPressAndReleaseJustAfterTailWithNearbyNote()
         {
-            Note note;
+            // Next note within tail lenience
+            Note note = new Note { StartTime = time_tail + 50 };
 
             var beatmap = new Beatmap<ManiaHitObject>
             {
@@ -392,13 +395,7 @@ namespace osu.Game.Rulesets.Mania.Tests
                         Duration = time_tail - time_head,
                         Column = 0,
                     },
-                    {
+                    note
-                        // Next note within tail lenience
-                        note = new Note
-                        {
-                            StartTime = time_tail + 50
-                        }
-                    }
                 },
                 BeatmapInfo =
                 {

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

@@ -54,7 +54,7 @@ namespace osu.Game.Rulesets.Mania.Tests
             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));
+            AddAssert("score is correct", () => currentPlayer.ScoreProcessor.TotalScore.Value, () => Is.EqualTo(1_000_030));
         }
 
         [Test]
@@ -87,7 +87,7 @@ namespace osu.Game.Rulesets.Mania.Tests
             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));
+            AddAssert("score is correct", () => currentPlayer.ScoreProcessor.TotalScore.Value, () => Is.EqualTo(1_000_040));
         }
 
         private void performTest(List<ManiaHitObject> hitObjects, List<ReplayFrame> frames)

osu.Game.Rulesets.Mania/ManiaRuleset.cs

@@ -385,6 +385,9 @@ namespace osu.Game.Rulesets.Mania
                 HitResult.Good,
                 HitResult.Ok,
                 HitResult.Meh,
+
+                // HitResult.SmallBonus is used for awarding perfect bonus score but is not included here as
+                // it would be a bit redundant to show this to the user.
             };
         }
 

osu.Game.Rulesets.Mania/Objects/Drawables/DrawableHoldNoteTail.cs

@@ -3,7 +3,6 @@
 
 #nullable disable
 
-using System.Diagnostics;
 using osu.Framework.Graphics;
 using osu.Framework.Input.Events;
 using osu.Game.Rulesets.Scoring;
@@ -33,35 +32,19 @@ namespace osu.Game.Rulesets.Mania.Objects.Drawables
 
         public void UpdateResult() => base.UpdateResult(true);
 
-        protected override void CheckForResult(bool userTriggered, double timeOffset)
+        protected override void CheckForResult(bool userTriggered, double timeOffset) =>
-        {
-            Debug.Assert(HitObject.HitWindows != null);
-
             // Factor in the release lenience
-            timeOffset /= TailNote.RELEASE_WINDOW_LENIENCE;
+            base.CheckForResult(userTriggered, timeOffset / TailNote.RELEASE_WINDOW_LENIENCE);
 
-            if (!userTriggered)
+        protected override HitResult GetCappedResult(HitResult result)
-            {
+        {
-                if (!HitObject.HitWindows.CanBeHit(timeOffset))
+            // If the head wasn't hit or the hold note was broken, cap the max score to Meh.
-                    ApplyResult(r => r.Type = r.Judgement.MinResult);
+            bool hasComboBreak = !HoldNote.Head.IsHit || HoldNote.Body.HasHoldBreak;
 
-                return;
+            if (result > HitResult.Meh && hasComboBreak)
-            }
+                return HitResult.Meh;
 
-            var result = HitObject.HitWindows.ResultFor(timeOffset);
+            return result;
-            if (result == HitResult.None)
-                return;
-
-            ApplyResult(r =>
-            {
-                // If the head wasn't hit or the hold note was broken, cap the max score to Meh.
-                bool hasComboBreak = !HoldNote.Head.IsHit || HoldNote.Body.HasHoldBreak;
-
-                if (result > HitResult.Meh && hasComboBreak)
-                    result = HitResult.Meh;
-
-                r.Type = result;
-            });
         }
 
         public override bool OnPressed(KeyBindingPressEvent<ManiaAction> e) => false; // Handled by the hold note

osu.Game.Rulesets.Mania/Objects/Drawables/DrawableNote.cs

@@ -13,6 +13,8 @@ using osu.Game.Beatmaps;
 using osu.Game.Graphics;
 using osu.Game.Rulesets.Mania.Configuration;
 using osu.Game.Rulesets.Mania.Skinning.Default;
+using osu.Game.Rulesets.Objects;
+using osu.Game.Rulesets.Objects.Drawables;
 using osu.Game.Rulesets.Scoring;
 using osu.Game.Rulesets.UI.Scrolling;
 using osu.Game.Screens.Edit;
@@ -38,6 +40,8 @@ namespace osu.Game.Rulesets.Mania.Objects.Drawables
 
         private Drawable headPiece;
 
+        private DrawableNotePerfectBonus perfectBonus;
+
         public DrawableNote()
             : this(null)
         {
@@ -89,7 +93,11 @@ namespace osu.Game.Rulesets.Mania.Objects.Drawables
             if (!userTriggered)
             {
                 if (!HitObject.HitWindows.CanBeHit(timeOffset))
+                {
+                    perfectBonus.TriggerResult(false);
                     ApplyResult(r => r.Type = r.Judgement.MinResult);
+                }
+
                 return;
             }
 
@@ -97,9 +105,23 @@ namespace osu.Game.Rulesets.Mania.Objects.Drawables
             if (result == HitResult.None)
                 return;
 
+            result = GetCappedResult(result);
+
+            perfectBonus.TriggerResult(result == HitResult.Perfect);
             ApplyResult(r => r.Type = result);
         }
 
+        public override void MissForcefully()
+        {
+            perfectBonus.TriggerResult(false);
+            base.MissForcefully();
+        }
+
+        /// <summary>
+        /// Some objects in mania may want to limit the max result.
+        /// </summary>
+        protected virtual HitResult GetCappedResult(HitResult result) => result;
+
         public virtual bool OnPressed(KeyBindingPressEvent<ManiaAction> e)
         {
             if (e.Action != Action.Value)
@@ -115,6 +137,32 @@ namespace osu.Game.Rulesets.Mania.Objects.Drawables
         {
         }
 
+        protected override void AddNestedHitObject(DrawableHitObject hitObject)
+        {
+            switch (hitObject)
+            {
+                case DrawableNotePerfectBonus bonus:
+                    AddInternal(perfectBonus = bonus);
+                    break;
+            }
+        }
+
+        protected override void ClearNestedHitObjects()
+        {
+            RemoveInternal(perfectBonus, false);
+        }
+
+        protected override DrawableHitObject CreateNestedHitObject(HitObject hitObject)
+        {
+            switch (hitObject)
+            {
+                case NotePerfectBonus bonus:
+                    return new DrawableNotePerfectBonus(bonus);
+            }
+
+            return base.CreateNestedHitObject(hitObject);
+        }
+
         private void updateSnapColour()
         {
             if (beatmap == null || HitObject == null) return;

osu.Game.Rulesets.Mania/Objects/Drawables/DrawableNotePerfectBonus.cs

@@ -0,0 +1,26 @@
+// 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.
+
+namespace osu.Game.Rulesets.Mania.Objects.Drawables
+{
+    public partial class DrawableNotePerfectBonus : DrawableManiaHitObject<NotePerfectBonus>
+    {
+        public override bool DisplayResult => false;
+
+        public DrawableNotePerfectBonus()
+            : this(null!)
+        {
+        }
+
+        public DrawableNotePerfectBonus(NotePerfectBonus hitObject)
+            : base(hitObject)
+        {
+        }
+
+        /// <summary>
+        /// Apply a judgement result.
+        /// </summary>
+        /// <param name="hit">Whether this tick was reached.</param>
+        internal void TriggerResult(bool hit) => ApplyResult(r => r.Type = hit ? r.Judgement.MaxResult : r.Judgement.MinResult);
+    }
+}

osu.Game.Rulesets.Mania/Objects/Note.cs

@@ -1,6 +1,7 @@
 // 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.Threading;
 using osu.Game.Rulesets.Judgements;
 using osu.Game.Rulesets.Mania.Judgements;
 
@@ -12,5 +13,12 @@ namespace osu.Game.Rulesets.Mania.Objects
     public class Note : ManiaHitObject
     {
         public override Judgement CreateJudgement() => new ManiaJudgement();
+
+        protected override void CreateNestedHitObjects(CancellationToken cancellationToken)
+        {
+            base.CreateNestedHitObjects(cancellationToken);
+
+            AddNested(new NotePerfectBonus { StartTime = StartTime });
+        }
     }
 }

osu.Game.Rulesets.Mania/Objects/NotePerfectBonus.cs

@@ -0,0 +1,20 @@
+// 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 osu.Game.Rulesets.Judgements;
+using osu.Game.Rulesets.Mania.Judgements;
+using osu.Game.Rulesets.Scoring;
+
+namespace osu.Game.Rulesets.Mania.Objects
+{
+    public class NotePerfectBonus : ManiaHitObject
+    {
+        public override Judgement CreateJudgement() => new NotePerfectBonusJudgement();
+        protected override HitWindows CreateHitWindows() => HitWindows.Empty;
+
+        public class NotePerfectBonusJudgement : ManiaJudgement
+        {
+            public override HitResult MaxResult => HitResult.SmallBonus;
+        }
+    }
+}

osu.Game.Rulesets.Mania/Skinning/Legacy/LegacyBodyPiece.cs

@@ -123,9 +123,18 @@ namespace osu.Game.Rulesets.Mania.Skinning.Legacy
 
         private void applyCustomUpdateState(DrawableHitObject hitObject, ArmedState state)
         {
-            // ensure that the hold note is also faded out when the head/tail/any tick is missed.
+            switch (hitObject)
-            if (state == ArmedState.Miss)
+            {
-                missFadeTime.Value ??= hitObject.HitStateUpdateTime;
+                // Ensure that the hold note is also faded out when the head/tail/body is missed.
+                // Importantly, we filter out unrelated objects like DrawableNotePerfectBonus.
+                case DrawableHoldNoteTail:
+                case DrawableHoldNoteHead:
+                case DrawableHoldNoteBody:
+                    if (state == ArmedState.Miss)
+                        missFadeTime.Value ??= hitObject.HitStateUpdateTime;
+
+                    break;
+            }
         }
 
         private void onIsHittingChanged(ValueChangedEvent<bool> isHitting)

osu.Game.Rulesets.Mania/UI/Column.cs

@@ -109,6 +109,7 @@ namespace osu.Game.Rulesets.Mania.UI
             TopLevelContainer.Add(HitObjectArea.Explosions.CreateProxy());
 
             RegisterPool<Note, DrawableNote>(10, 50);
+            RegisterPool<NotePerfectBonus, DrawableNotePerfectBonus>(10, 50);
             RegisterPool<HoldNote, DrawableHoldNote>(10, 50);
             RegisterPool<HeadNote, DrawableHoldNoteHead>(10, 50);
             RegisterPool<TailNote, DrawableHoldNoteTail>(10, 50);

osu.Game.Tests/Rulesets/Scoring/ScoreProcessorTest.cs

@@ -74,7 +74,7 @@ namespace osu.Game.Tests.Rulesets.Scoring
         [TestCase(ScoringMode.Standardised, HitResult.Miss, HitResult.Great, 0)]
         [TestCase(ScoringMode.Standardised, HitResult.Meh, HitResult.Great, 79_333)]
         [TestCase(ScoringMode.Standardised, HitResult.Ok, HitResult.Great, 158_667)]
-        [TestCase(ScoringMode.Standardised, HitResult.Good, HitResult.Perfect, 302_402)]
+        [TestCase(ScoringMode.Standardised, HitResult.Good, HitResult.Perfect, 317_626)]
         [TestCase(ScoringMode.Standardised, HitResult.Great, HitResult.Great, 492_894)]
         [TestCase(ScoringMode.Standardised, HitResult.Perfect, HitResult.Perfect, 492_894)]
         [TestCase(ScoringMode.Standardised, HitResult.SmallTickMiss, HitResult.SmallTickHit, 0)]
@@ -86,7 +86,7 @@ namespace osu.Game.Tests.Rulesets.Scoring
         [TestCase(ScoringMode.Classic, HitResult.Miss, HitResult.Great, 0)]
         [TestCase(ScoringMode.Classic, HitResult.Meh, HitResult.Great, 7_975)]
         [TestCase(ScoringMode.Classic, HitResult.Ok, HitResult.Great, 15_949)]
-        [TestCase(ScoringMode.Classic, HitResult.Good, HitResult.Perfect, 30_398)]
+        [TestCase(ScoringMode.Classic, HitResult.Good, HitResult.Perfect, 31_928)]
         [TestCase(ScoringMode.Classic, HitResult.Great, HitResult.Great, 49_546)]
         [TestCase(ScoringMode.Classic, HitResult.Perfect, HitResult.Perfect, 49_546)]
         [TestCase(ScoringMode.Classic, HitResult.SmallTickMiss, HitResult.SmallTickHit, 0)]

osu.Game/Rulesets/Judgements/Judgement.cs

@@ -190,10 +190,9 @@ namespace osu.Game.Rulesets.Judgements
                     return 200;
 
                 case HitResult.Great:
-                    return 300;
+                // Perfect doesn't actually give more score / accuracy directly.
-
                 case HitResult.Perfect:
-                    return 315;
+                    return 300;
 
                 case HitResult.SmallBonus:
                     return SMALL_BONUS_SCORE;

osu.Game/Rulesets/Scoring/HitResult.cs

@@ -55,6 +55,13 @@ namespace osu.Game.Rulesets.Scoring
         [Order(1)]
         Great,
 
+        /// <summary>
+        /// This is an optional timing window tighter than <see cref="Great"/>.
+        /// </summary>
+        /// <remarks>
+        /// By default, this does not give any bonus accuracy or score.
+        /// To have it affect scoring, consider adding a nested bonus object.
+        /// </remarks>
         [Description(@"Perfect")]
         [EnumMember(Value = "perfect")]
         [Order(0)]