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

refactor: simplify !with command argument parsing #356

Closed
wants to merge 23 commits into from

Conversation

minisbett
Copy link
Contributor

@minisbett minisbett commented Jan 10, 2023

I simplified the parsing, as well as adding some more "implicit" parameters to make stuff more consistent.
I also added support for mania (katu, geki)
In summary, you can now do stuff such as

!with 5x100 2x50
!with 99% 3m
!with 4x100 9m 388x
!with 97.59% 2m 473x +HDDT
!with 5xkatu 3xgeki

Dependent on #526

@minisbett minisbett requested a review from cmyui as a code owner January 10, 2023 00:52
@NiceAesth NiceAesth changed the title !with command rework refactor: simplify !with command argument parsing Jan 13, 2023
@minisbett
Copy link
Contributor Author

should be good to merge now

@NiceAesth NiceAesth added the code quality Something is implemented poorly label Feb 21, 2023
Copy link
Contributor

@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.

setattr is woozy, generally not a fan of this pr being in favour of the current logic. open to opinions from other reviewers though

@minisbett
Copy link
Contributor Author

setattr is woozy, generally not a fan of this pr being in favour of the current logic. open to opinions from other reviewers though

It's a fair point, although I think it's fair to use in such a use-case of dynamically parsing user input like this. The logic is much simpler, the command itself is easier to use imo and it also adds missing support.

@minisbett
Copy link
Contributor Author

replaced by #640

@minisbett minisbett closed this Feb 26, 2024
@minisbett minisbett deleted the with-rework branch February 26, 2024 20:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code quality Something is implemented poorly
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants