Merge pull request #36639 from bdach/actually-all-valid-hit-results

Refactor hit result methods on `Ruleset`

osu.Game.Rulesets.Catch/CatchRuleset.cs

@@ -176,15 +176,20 @@ namespace osu.Game.Rulesets.Catch
 
         public override Drawable CreateIcon() => new SpriteIcon { Icon = OsuIcon.RulesetCatch };
 
-        protected override IEnumerable<HitResult> GetValidHitResults()
+        public override IEnumerable<HitResult> GetValidHitResults()
         {
             return new[]
             {
                 HitResult.Great,
+                HitResult.Miss,
 
                 HitResult.LargeTickHit,
+                HitResult.LargeTickMiss,
                 HitResult.SmallTickHit,
+                HitResult.SmallTickMiss,
                 HitResult.LargeBonus,
+                HitResult.IgnoreHit,
+                HitResult.IgnoreMiss,
             };
         }
 

osu.Game.Rulesets.Mania/ManiaRuleset.cs

@@ -383,7 +383,7 @@ namespace osu.Game.Rulesets.Mania
             return (PlayfieldType)Enum.GetValues(typeof(PlayfieldType)).Cast<int>().OrderDescending().First(v => variant >= v);
         }
 
-        protected override IEnumerable<HitResult> GetValidHitResults()
+        public override IEnumerable<HitResult> GetValidHitResults()
         {
             return new[]
             {
@@ -392,9 +392,11 @@ namespace osu.Game.Rulesets.Mania
                 HitResult.Good,
                 HitResult.Ok,
                 HitResult.Meh,
+                HitResult.Miss,
 
-                // 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.
+                HitResult.IgnoreHit,
+                HitResult.ComboBreak,
+                HitResult.IgnoreMiss,
             };
         }
 

osu.Game.Rulesets.Osu/OsuRuleset.cs

@@ -277,19 +277,24 @@ namespace osu.Game.Rulesets.Osu
 
         public override IRulesetConfigManager CreateConfig(SettingsStore? settings) => new OsuRulesetConfigManager(settings, RulesetInfo);
 
-        protected override IEnumerable<HitResult> GetValidHitResults()
+        public override IEnumerable<HitResult> GetValidHitResults()
         {
             return new[]
             {
                 HitResult.Great,
                 HitResult.Ok,
                 HitResult.Meh,
+                HitResult.Miss,
 
                 HitResult.LargeTickHit,
+                HitResult.LargeTickMiss,
                 HitResult.SmallTickHit,
+                HitResult.SmallTickMiss,
                 HitResult.SliderTailHit,
                 HitResult.SmallBonus,
                 HitResult.LargeBonus,
+                HitResult.IgnoreHit,
+                HitResult.IgnoreMiss,
             };
         }
 

osu.Game.Rulesets.Taiko/TaikoRuleset.cs

@@ -222,15 +222,18 @@ namespace osu.Game.Rulesets.Taiko
 
         public override RulesetSettingsSubsection CreateSettings() => new TaikoSettingsSubsection(this);
 
-        protected override IEnumerable<HitResult> GetValidHitResults()
+        public override IEnumerable<HitResult> GetValidHitResults()
         {
             return new[]
             {
                 HitResult.Great,
                 HitResult.Ok,
+                HitResult.Miss,
 
                 HitResult.SmallBonus,
                 HitResult.LargeBonus,
+                HitResult.IgnoreHit,
+                HitResult.IgnoreMiss,
             };
         }
 

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

@@ -526,7 +526,7 @@ namespace osu.Game.Tests.Rulesets.Scoring
             // ReSharper disable once MemberHidesStaticFromOuterClass
             private class TestRuleset : Ruleset
             {
-                protected override IEnumerable<HitResult> GetValidHitResults() => new[] { HitResult.Great };
+                public override IEnumerable<HitResult> GetValidHitResults() => new[] { HitResult.Great };
 
                 public override IEnumerable<Mod> GetModsFor(ModType type) => throw new NotImplementedException();
 

osu.Game/Database/StandardisedScoreMigrationTools.cs

@@ -51,7 +51,7 @@ namespace osu.Game.Database
 
             var beatmap = new Beatmap();
 
-            HitResult maxRulesetJudgement = ruleset.GetHitResults().First().result;
+            HitResult maxRulesetJudgement = ruleset.GetHitResultsForDisplay().First().result;
 
             // This is a list of all results, ordered from best to worst.
             // We are constructing a "best possible" score from the statistics provided because it's the best we can do.

osu.Game/Overlays/BeatmapSet/Scores/ScoreTable.cs

@@ -106,7 +106,7 @@ namespace osu.Game.Overlays.BeatmapSet.Scores
 
             var ruleset = scores.First().Ruleset.CreateInstance();
 
-            foreach (var resultGroup in ruleset.GetHitResults().GroupBy(r => r.displayName))
+            foreach (var resultGroup in ruleset.GetHitResultsForDisplay().GroupBy(r => r.displayName))
             {
                 if (!resultGroup.Any(r => allScoreStatistics.Contains(r.result)))
                     continue;

osu.Game/Rulesets/Ruleset.cs

@@ -13,6 +13,7 @@ using osu.Framework.Graphics.Sprites;
 using osu.Framework.Input.Bindings;
 using osu.Framework.IO.Stores;
 using osu.Framework.Localisation;
+using osu.Framework.Utils;
 using osu.Game.Beatmaps;
 using osu.Game.Beatmaps.Legacy;
 using osu.Game.Configuration;
@@ -334,13 +335,17 @@ namespace osu.Game.Rulesets
         public virtual StatisticItem[] CreateStatisticsForScore(ScoreInfo score, IBeatmap playableBeatmap) => Array.Empty<StatisticItem>();
 
         /// <summary>
-        /// Get all valid <see cref="HitResult"/>s for this ruleset.
-        /// Generally used for results display purposes, where it can't be determined if zero-count means the user has not achieved any or the type is not used by this ruleset.
+        /// Get all <see cref="HitResult"/>s for this ruleset which are important enough to displayed to the end user.
+        /// Used for results display purposes, where it can't be determined if zero-count means the user has not achieved any or the type is not used by this ruleset.
         /// </summary>
+        /// <remarks>
+        /// <see cref="HitResult.Miss"/> is implicitly included. Special types like <see cref="HitResult.IgnoreHit"/> are not returned by this method.
+        /// Values are returned as ordered by <see cref="OrderAttribute"/>.
+        /// </remarks>
         /// <returns>
-        /// All valid <see cref="HitResult"/>s along with a display-friendly name.
+        /// All relevant <see cref="HitResult"/>s along with a display-friendly name.
         /// </returns>
-        public IEnumerable<(HitResult result, LocalisableString displayName)> GetHitResults()
+        public IEnumerable<(HitResult result, LocalisableString displayName)> GetHitResultsForDisplay()
         {
             var validResults = GetValidHitResults();
 
@@ -353,6 +358,7 @@ namespace osu.Game.Rulesets
                     case HitResult.None:
                     case HitResult.IgnoreHit:
                     case HitResult.IgnoreMiss:
+                    case HitResult.ComboBreak:
                     // display is handled as a completion count with corresponding "hit" type.
                     case HitResult.LargeTickMiss:
                     case HitResult.SmallTickMiss:
@@ -366,12 +372,10 @@ namespace osu.Game.Rulesets
 
         /// <summary>
         /// Get all valid <see cref="HitResult"/>s for this ruleset.
-        /// Generally used for results display purposes, where it can't be determined if zero-count means the user has not achieved any or the type is not used by this ruleset.
+        /// Used for strict validation purposes. The ruleset should return ALL applicable <see cref="HitResult"/> types here
+        /// (except <see cref="HitResult.None"/> and obsolete types).
         /// </summary>
-        /// <remarks>
-        /// <see cref="HitResult.Miss"/> is implicitly included. Special types like <see cref="HitResult.IgnoreHit"/> are ignored even when specified.
-        /// </remarks>
-        protected virtual IEnumerable<HitResult> GetValidHitResults() => EnumExtensions.GetValuesInOrder<HitResult>();
+        public virtual IEnumerable<HitResult> GetValidHitResults() => EnumExtensions.GetValuesInOrder<HitResult>();
 
         /// <summary>
         /// Get a display friendly name for the specified result type.

osu.Game/Scoring/Legacy/LegacyScoreDecoder.cs

@@ -211,7 +211,7 @@ namespace osu.Game.Scoring.Legacy
             var scoreProcessor = rulesetInstance.CreateScoreProcessor();
 
             // Populate the maximum statistics.
-            HitResult maxBasicResult = rulesetInstance.GetHitResults()
+            HitResult maxBasicResult = rulesetInstance.GetHitResultsForDisplay()
                                                       .Select(h => h.result)
                                                       .Where(h => h.IsBasic()).MaxBy(scoreProcessor.GetBaseScoreForResult);
 

osu.Game/Scoring/ScoreInfo.cs

@@ -361,7 +361,7 @@ namespace osu.Game.Scoring
 
         public IEnumerable<HitResultDisplayStatistic> GetStatisticsForDisplay()
         {
-            foreach (var r in Ruleset.CreateInstance().GetHitResults())
+            foreach (var r in Ruleset.CreateInstance().GetHitResultsForDisplay())
             {
                 int value = Statistics.GetValueOrDefault(r.result);
 

osu.Game/Screens/Play/HUD/JudgementCounter/JudgementCountController.cs

@@ -32,7 +32,7 @@ namespace osu.Game.Screens.Play.HUD.JudgementCounter
         {
             // Due to weirdness in judgements, some results have the same name and should be aggregated for display purposes.
             // There's only one case of this right now ("slider end").
-            foreach (var group in ruleset.Value.CreateInstance().GetHitResults().GroupBy(r => r.displayName))
+            foreach (var group in ruleset.Value.CreateInstance().GetHitResultsForDisplay().GroupBy(r => r.displayName))
             {
                 var judgementCount = new JudgementCount
                 {