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

Fixes for mania simulate command #252

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

MrHeliX
Copy link

@MrHeliX MrHeliX commented Feb 21, 2025

  • Fixes total hits calculation for mania scores with CL mod (hold notes should only count as a single hit in stable)
  • Implements GetAccuracy for mania simulate command (both for CL and non-CL scores)

@smoogipoo smoogipoo requested a review from stanriders February 21, 2025 10:04
@stanriders
Copy link
Member

@stanriders stanriders added the perfcalc-cli PerformanceCalculator label Feb 21, 2025
@MrHeliX
Copy link
Author

MrHeliX commented Feb 21, 2025

Would you mind doing the same for GUI? https://github.com/ppy/osu-tools/blob/master/PerformanceCalculatorGUI/RulesetHelper.cs

Not too familiar with the GUI code but I think this does the trick

Copy link
Contributor

@minisbett minisbett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On Lazer, even with CL it still does the Lazer way of doing hits, therefore you cannot simply check for CL here.
This is a bit troublesome as there's no way to tell apart Lazer and Stable, so I think the best that can be done is some command-line parameter:

[UsedImplicitly]
[Option(Template = "-S|--stable", Description = "The score is considered an osu! stable score.")]
public bool IsStable { get; }

(source: I forgot but when I refactored all this stuff I ran into this issue, I think @Givikap120 knows more about it)

Edit: I backread older PRs, and read that CL does not exist on Mania. Does that mean CL is a definitive marker for the score origining from Stable? And also, can't you use GetMaxCombo on mania to get the total hits?

@MrHeliX
Copy link
Author

MrHeliX commented Feb 21, 2025

On Lazer, even with CL it still does the Lazer way of doing hits, therefore you cannot simply check for CL here. This is a bit troublesome as there's no way to tell apart Lazer and Stable, so I think the best that can be done is some command-line parameter:

[UsedImplicitly]
[Option(Template = "-S|--stable", Description = "The score is considered an osu! stable score.")]
public bool IsStable { get; }

(source: I forgot but when I refactored all this stuff I ran into this issue, I think @Givikap120 knows more about it)

Edit: I backread older PRs, and read that CL does not exist on Mania. Does that mean CL is a definitive marker for the score origining from Stable? And also, can't you use GetMaxCombo on mania to get the total hits?

Honestly I am not too involved with mania but from what I've learnt today it seems to be all over the place. There are currently three different ways of calculating accuracy: the stable way (with CL), the Lazer way (without CL) and the website way (with judgements according to CL logic but accuracy calculation according to non-CL logic) and even on the site it matters if you have Lazer mode enabled or not.

I implemented it this way because this would make the most sense, with CL plays (only possible on stable) following stable logic and non-CL plays (only possible on Lazer) following Lazer logic. I think adding another parameter like this is just going to make the mess even worse.

GetMaxCombo returns the Lazer max combo so it's not a viable option, unless you want to subtract the LN count which is pretty much equivalent to what is happening here

@minisbett
Copy link
Contributor

minisbett commented Feb 21, 2025

GetMaxCombo returns the Lazer max combo so it's not a viable option
right.

Wonder if that can be PRd to Lazer instead but I'm not sure how much sense that'd make because CL basically only acts as a stable marker on Lazers' mania.

@MaxOhn
Copy link
Contributor

MaxOhn commented Mar 18, 2025

generateHitResults still assumes perfects and greats to generate the same accuracy value but GetAccuracy distinguishes the two by either 305 or 300 values.

@MaxOhn
Copy link
Contributor

MaxOhn commented Mar 22, 2025

generateHitResults still assumes perfects and greats to generate the same accuracy value but GetAccuracy distinguishes the two by either 305 or 300 values.

The math works the same way so it'll look very similar

// Let Perfect=61, Great=60, Good=40, Ok=20, Meh=10, Miss=0. The total should be this.
int targetTotal = (int)Math.Round(accuracy * totalHits * 61);

// Start by assuming every non miss is a meh
// This is how much increase is needed by the rest
int remainingHits = totalHits - countMiss;
int delta = targetTotal - 10 * remainingHits;

// Each perfect increases total by 51 (perfect - meh = 51)
int perfects = Math.Min(delta / 51, remainingHits);
delta -= perfects * 51;
remainingHits -= perfects;

// Each great increases total by 50 (great - meh = 50)
int greats = Math.Min(delta / 50, remainingHits);
delta -= greats * 50;
remainingHits -= greats;

// Each good increases total by 30 (good - meh = 30).
countGood = Math.Min(delta / 30, remainingHits);
delta -= countGood.Value * 30;
remainingHits -= countGood.Value;

// Each ok increases total by 10 (ok - meh = 10).
int oks = delta / 10;
remainingHits -= oks;

// Everything else is a meh, as initially assumed.
countMeh = remainingHits;

Just need to check whether stable or lazer accuracy is required and chose the formula accordingly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
perfcalc-cli PerformanceCalculator
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants