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

feat: implement shiny chance #19

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

koffydrop
Copy link

Implemented a default random shiny chance that can be overridden to force a shiny or normal with --shiny true|false (which defaults to true if not specified), and to change the rate with --shiny-rate <number>.

@talwat
Copy link
Owner

talwat commented Mar 20, 2025

I feel like adding options for this is a bit redundant. Perhaps replacing these options with environment variables could work? For example, POKEGET_SHINY_RATE. I also do not understand why shiny was made into a Option<bool>.

The benefit with environment variables is that they don't need to clutter the help screen or the main clap struct.

@koffydrop
Copy link
Author

Shiny being made into an Option<bool> is so that if it's None, the the shiny chance takes effect, if it's specified without a value it defaults to true, and you can pass a truthy or falsy value still. It makes sense that way in my head but I'm open to change it (or if there's a better way of implementing it).

I can definitely see the rate being cluttered and look into making it an environment variable though.

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.

2 participants