Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SS rank in osu!mania requires all judgements to be PERFECT #24238

Open
bdach opened this issue Jul 16, 2023 · 33 comments · Fixed by #25111 · May be fixed by #29640
Open

SS rank in osu!mania requires all judgements to be PERFECT #24238

bdach opened this issue Jul 16, 2023 · 33 comments · Fixed by #25111 · May be fixed by #29640

Comments

@bdach
Copy link
Collaborator

bdach commented Jul 16, 2023

Type

Game behaviour

Bug description

Currently, the user's score rank is calculated entirely based on their accuracy:

/// <summary>
/// Given an accuracy (0..1), return the correct <see cref="ScoreRank"/>.
/// </summary>
public static ScoreRank RankFromAccuracy(double accuracy)
{
if (accuracy == accuracy_cutoff_x)
return ScoreRank.X;
if (accuracy >= accuracy_cutoff_s)
return ScoreRank.S;
if (accuracy >= accuracy_cutoff_a)
return ScoreRank.A;
if (accuracy >= accuracy_cutoff_b)
return ScoreRank.B;
if (accuracy >= accuracy_cutoff_c)
return ScoreRank.C;
return ScoreRank.D;
}

In mania, the highest hit result is PERFECT:

protected override IEnumerable<HitResult> GetValidHitResults()
{
return new[]
{
HitResult.Perfect,
HitResult.Great,
HitResult.Good,
HitResult.Ok,
HitResult.Meh,
HitResult.LargeTickHit,
};
}

Therefore, hitting GREATs causes an accuracy drop, which means that it is not possible to achieve an SS rank with GREAT judgements at all, which is a departure from what stable does without ScoreV2 active, wherein an SS rank requires just all GREATs or better, and probably needlessly harsh in general.

This was discussed on discord, and the potential solution for this would be to convert the extra 50 score for a PERFECT compared to a GREAT to bonus score.

Screenshots or videos

n/a

Version

current master

Logs

n/a

@bdach bdach added this to the Game balance milestone Jul 16, 2023
@alexpado
Copy link

alexpado commented Jul 28, 2023

I did manage to modify this
image

My only issue is that I modified this (setting to 300):

case HitResult.Perfect:
return 315;

As far as I know, this affect EVERY rulesets, which is a big nope.

Even if I override the ToNumericResult static method, this would be pointless:

currentMaximumBaseScore += Judgement.ToNumericResult(result.Judgement.MaxResult);
currentBaseScore += Judgement.ToNumericResult(result.Type);

It calls directly the static method contained in the Judgement class.

As only the score counts toward the accuracy, the value should be overridable per-rulesets.

private void updateScore()
{
Accuracy.Value = currentMaximumBaseScore > 0 ? currentBaseScore / currentMaximumBaseScore : 1;

From there, I think adding an overridable method that returns the value of the JudgmentResult could be possible, letting any class extending ScoreProcessor freely modify the hit value.

I don't know if it's the right approach, so I'll wait for feedback before going further.

Edit:
For reference, here's the same play without any edits after watching the replay
image

@bdach
Copy link
Collaborator Author

bdach commented Jul 28, 2023

@alexpado that is not the correct direction to proceed with. changing the numerical value of PERFECTs is the wrong approach. PERFECT should be worth 350 315 points, to make sure that total score is positively affected by hitting more PERFECTs.

it's going to either be a question of moving the extra 50 score to an entirely separate bonus judgement, or changing the rank awarding method to no longer be based on pure accuracy, or something along similar lines. but PERFECTs must continue to grant more points than GREATs.

@alexpado
Copy link

alexpado commented Jul 28, 2023

I overlooked this part, my bad.

My question regarding allowing rulesets to change hit values still stand though, which would highly change how things are handled, but could be done in a discussion.

For our issue at hand, what about giving a "weight" for each HitResult which could be used as a separate value from score to process accuracy ? The number could be the score removing the bonus, just like I did to "fix" the issue.

        public static int ToAccuracyWeight(HitResult result)
        {
            switch (result)
            {
                default:
                    return 0;

                case HitResult.SmallTickHit:
                    return 10;

                case HitResult.LargeTickHit:
                    return 30;

                case HitResult.Meh:
                    return 50;

                case HitResult.Ok:
                    return 100;

                case HitResult.Good:
                    return 200;

                case HitResult.Great:
                    return 300;

                case HitResult.Perfect:
                    return 300;

                case HitResult.SmallBonus:
                    return SMALL_BONUS_SCORE;

                case HitResult.LargeBonus:
                    return LARGE_BONUS_SCORE;
            }
        }

This mean that each HitResult could affect independently accuracy and score, while conserving the bonus for score processing.

Also, each HitResult could have their accuracy weigth fine-tuned to make some hit more important for accuracy than others.

Side question: You are stating that PERFECT should be worth 350 points, but actually in the code it is 315, should it be changed to 350 ?

EDIT: Again, this would affect every ruleset, as all of them share the same process. Is this really okay ?

@bdach
Copy link
Collaborator Author

bdach commented Aug 22, 2023

This one I'm not sure what to do with, there are many distinct directions you could pick here, every one as easy to implement as the other, but I have no real idea what the community wants.

@smoogipoo did you perhaps gather some feedback on this at COE? would you want to handle this one yourself?

@smoogipoo
Copy link
Contributor

The general discussion, before and during COE, seems to be to try and get the extra score of PERFECTs into bonus score. Reasoning being, or is supported by, other VSRGs doing the same.

@bdach
Copy link
Collaborator Author

bdach commented Aug 22, 2023

Alright well that seems to settle it. Moving to "needs implementation", seems like something virtually any one of us should be able to handle (but will leave to you if you're so inclined, obviously).

@bdach
Copy link
Collaborator Author

bdach commented Aug 22, 2023

Hmm well maybe one more thing: is that bonus score from PERFECTs gonna be capped or uncapped? That's still something we probably need to figure out before an implementation.

Heads up: crosstalk with ppy/osu-queue-score-statistics#133 here.

@smoogipoo
Copy link
Contributor

There was no discussion on it being capped, afaik.

@stockfishcooker
Copy link

I'd say let the bonus be capped just like ScoreV1 and ScoreV2. The fantastic judgments most likely will only come in handy for breaking ties between SSes made by top players, and I don't want it to be too influencing for normal play.
Of course, I'm not a top player, and this is just an opinion (I only have 2 SSes right now lol). I would like to hear other players that are better than me share their thoughts.

@bdach
Copy link
Collaborator Author

bdach commented Sep 11, 2023

@stockfishcooker at this point we're primarily looking for already-formed opinions from established groups of players rather than opinions of individuals who happen to have access to github, so unfortunately your comment has little bearing on the final direction and therefore does not help us very much. I hope you understand.

@peppy
Copy link
Member

peppy commented Oct 13, 2023

I'm going to give this one a try.

@peppy peppy self-assigned this Oct 13, 2023
@peppy
Copy link
Member

peppy commented Oct 13, 2023

The most feasible approach I can think of would be to adjust ToNumericResult(HitResult.Perfect) to return 300 (as touched on earlier in this thread), and then add a nested bonus judgement which triggers the actual extra score for perfect hits.

Whether this scoring change can be applied globally in the currently static context (with xmldoc explaining how Perfect works) or whether it needs to be somehow customisable per ruleset is another question though.

In the interest of keeping things simple I'd propose obliterating it globally as we are still early in development and custom rulesets can cope with the change (and add their own bonus judgements if they want perfect to give more score), but would need more opinions (@smoogipoo @bdach).

aka:

diff --git a/osu.Game/Rulesets/Judgements/Judgement.cs b/osu.Game/Rulesets/Judgements/Judgement.cs
index f60b3a6c02..cd1e81046d 100644
--- a/osu.Game/Rulesets/Judgements/Judgement.cs
+++ b/osu.Game/Rulesets/Judgements/Judgement.cs
@@ -190,10 +190,9 @@ public static int ToNumericResult(HitResult result)
                     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;
diff --git a/osu.Game/Rulesets/Scoring/HitResult.cs b/osu.Game/Rulesets/Scoring/HitResult.cs
index ccd1f49de4..fed338b012 100644
--- a/osu.Game/Rulesets/Scoring/HitResult.cs
+++ b/osu.Game/Rulesets/Scoring/HitResult.cs
@@ -55,6 +55,13 @@ public enum HitResult
         [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)]

@peppy
Copy link
Member

peppy commented Oct 13, 2023

With regards to the method being static, we may actually need to revisit this for osu!taiko scoring fixes (see #9087)...

@bdach
Copy link
Collaborator Author

bdach commented Oct 13, 2023

I can think of would be to adjust ToNumericResult(HitResult.Perfect) to return 300 (as touched on earlier in this thread), and then add a nested bonus judgement which triggers the actual extra score for perfect hits.

The nested judgement I can agree with for sure, but I'm not sold on scaling HitResult.Perfect back to 300. The primary reason is the result screen.

If you have notes give a Perfect and a nested LargeBonus, you're going to have the count of perfects displaying basically twice which feels bad and redundant. I'd say it should probably be Great + bonus? Dunno.

Health is going to also be an issue here, by the way. LargeBonus has the same HP increase as Great. Making #25052 relevant.

@peppy
Copy link
Member

peppy commented Oct 13, 2023

If you have notes give a Perfect and a nested LargeBonus, you're going to have the count of perfects displaying basically twice which feels bad and redundant

I'd argue that people have come to expect the Perfect counter to exist and moving away from that (aka only showing bonus) will likely not go down well. FWIW I was also thinking in that direction immediately, and even had the proposal typed up, but nuked it all because I don't think it will work from a user-acceptance perspective.

Health is going to also be an issue here, by the way

Will have to deal with that somehow, yeah.

@bdach
Copy link
Collaborator Author

bdach commented Oct 13, 2023

I'd argue that people have come to expect the Perfect counter to exist and moving away from that (aka only showing bonus) will likely not go down well

Fair point.

I think we may be able to just hide away the bonus counter so that it doesn't look weird and redundant. Not declaring it in Ruleset.GetValidHitResults() will probably do it (but needs a proper testing and sweep of usages to ensure no unintended consequences).

@peppy
Copy link
Member

peppy commented Oct 13, 2023

Yeah, I accidentally found that one out (by forgetting to add the new judgement to that list and wondering why it displayed nowhere).

I've got this working pretty well in limited gameplay testing, so PR'd it for further comment. Feels like a pretty solid direction to me.

@bdach
Copy link
Collaborator Author

bdach commented Dec 20, 2023

After #25945, this issue is relevant again.

@bdach bdach reopened this Dec 20, 2023
@peppy
Copy link
Member

peppy commented Dec 21, 2023

Dunno where to stand on this. I guess we listen to people for some weeks and whoever's voice is louder wins?

Personal opinion is that I still agree with bonus scoring, potentially with the bonus reduced to a negligible amount to avoid bonus on long maps overpowering short maps. Dunno.

@Rekkonnect
Copy link
Contributor

Playing mania for years, I barely score any SSes with the stable algorithm, let alone 1,000,000s. We can tell the difference between SS and 1M; the latter being a "real" SS with tournament acc calculation. Maybe a mode where we could see the classic and the tournament acc would be preferred.

It's worthy to note that currently, both MAX and 300 (PERFECT and GREAT respectively) contribute towards 100% score for classic acc, whereas in tournaments GREAT judgements count less than 100% acc, though not too harshly. pp calculation was recently adjusted and is based on tournament accuracy rather than score, though in the client we see the classic accuracy. I'd definitely prefer seeing both kinds of accuracy calculations with a toggle, and showing the respective rank (A, S, SS) based on the currently displayed acc. For example:

Classic 100% SS
Tournament 99.66% S

This also makes it possible to show an A instead of S in certain edge cases close to the boundaries of the ranks, if too many GREATs are scored.

@bdach
Copy link
Collaborator Author

bdach commented Jan 5, 2024

Toggle is 200% not happening and toggle-dependent ranks are 200,000,000% not happening. We are dealing with enough complexity as is. This is non-negotiable.

This does not only impact client. It impacts web, it impacts user statistics (counts of ranks achieved, etc.). Your proposal is unsustainable.

@Rekkonnect
Copy link
Contributor

Rekkonnect commented Jan 5, 2024

From what I can tell, we want a singular rank for each play, to which I stand by keeping the current stable. SS is pretty hard in itself, and S is too common at higher stars. I think that ultimately this system might need a greater revamp, but at the end of the day it's decoratory. The highest the rank has is on the profile and medals afaic.

@Airkek
Copy link
Contributor

Airkek commented Jan 8, 2024

Will 95.00% 'S' play with old scoring become 94.xx% 'A' with new one?
SM5/Simply Love theme (theme for ITG players with it's own scoring for groovestats) have two modes: first counts 'Fantastic' and 'Excellent' as 100% when second counts only 'Fantastic' as 100%. In any way groovestats accepts only first mode, where 'Fantastic' equals to 'Excellent'. So it would be nice if osu!mania will count 'perfect' and 'great' as same accuracy-point (as in stable)

@bdach
Copy link
Collaborator Author

bdach commented Jan 8, 2024

Will 95.00% 'S' play with old scoring become 94.xx% 'A' with new one?

Possible. We'll see what the community feedback is.

So it would be nice if osu!mania will count 'perfect' and 'great' as same accuracy-point (as in stable)

I am completely unable to parse the word salad in the first half of that sentence but if you are suggesting having two accuracy systems functioning in parallel behind some toggle or whatever - that is not happening.

@Airkek
Copy link
Contributor

Airkek commented Jan 8, 2024

if you are suggesting having two accuracy systems functioning in parallel behind some toggle or whatever

im not suggesting this

@peppy
Copy link
Member

peppy commented Jan 15, 2024

This has since been changed in line with what is asked for in this discussion. Closing for now.

@peppy peppy closed this as completed Jan 15, 2024
@bdach
Copy link
Collaborator Author

bdach commented Jan 15, 2024

This has since been changed in line with what is asked for in this discussion

Has it? Where? The title of the issue continues to be accurate as far as I'm aware?

If the decision here is "we're not fixing this", then fine, but that's not what the above says.

@peppy
Copy link
Member

peppy commented Jan 15, 2024

Oh, we undid the fix didn't we...

I don't see much point in keeping this issue open regardless. Let's wait for discussions or hopefully not.

@mcendu
Copy link
Contributor

mcendu commented May 25, 2024

I am starting a poll in G&R.

@bdach
Copy link
Collaborator Author

bdach commented Aug 28, 2024

@peppy this keeps popping up, most recently here. Do we want to reconsider mania SS rank again, or should I never bring it up again?

@smoogipoo
Copy link
Contributor

smoogipoo commented Aug 28, 2024

At the point where osu, taiko, and catch all override ScoreProcessor.RankFromScore(), I see no reason why mania couldn't also. Thankfully, the method signature has changed ever so slightly to accommodate this, since this issue was made :)

@bdach
Copy link
Collaborator Author

bdach commented Aug 28, 2024

Thankfully, the method signature has changed ever so slightly to accommodate this, since this issue was made :)

Is also why I mentioned this again.

The biggest pain point in all of this is that we'd have to reverify ranks for all (mania) scores again.

@peppy
Copy link
Member

peppy commented Aug 29, 2024

I'm fine with this direction yep.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment