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

Reworked hit result generator based on accuracy #198

Merged
merged 8 commits into from
Aug 31, 2024

Conversation

Givikap120
Copy link
Contributor

It used to just spam 50s until it reach target accuracy
Now it's actually trying to fit it in realistic way

Copy link
Member

@tsunyoku tsunyoku left a comment

Choose a reason for hiding this comment

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

Would be nice to add explanation on what this is doing and why. All I see is logic with no clear reasoning on why it does what it does.

@Givikap120
Copy link
Contributor Author

Would be nice to add explanation on what this is doing and why. All I see is logic with no clear reasoning on why it does what it does.

Added very detailed explanation of what I'm doing here. You can remove unnecessary things.

Copy link
Member

@stanriders stanriders left a comment

Choose a reason for hiding this comment

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

@Givikap120
Copy link
Contributor Author

Please copy your implementation to the CLI perfcalc as well: https://github.com/ppy/osu-tools/blob/master/PerformanceCalculator/Simulate/OsuSimulateCommand.cs#L63

Fixed

Copy link
Member

@stanriders stanriders left a comment

Choose a reason for hiding this comment

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

Please fix CI issues

@stanriders
Copy link
Member

image

@smoogipoo
Copy link
Contributor

CodeFactor is fine, don't mind that too much.

Copy link
Member

@stanriders stanriders left a comment

Choose a reason for hiding this comment

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

I'd say a lot of this is redundant (does it really matter if scores below 25% accuracy are accurately calculated?..) but I guess it doesnt hurt to have

@stanriders stanriders merged commit 085a853 into ppy:master Aug 31, 2024
2 of 3 checks passed
@Givikap120 Givikap120 deleted the better_accuracy_estimator branch November 1, 2024 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants