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

Show top Pokemon percentages and top 20 instead of top 10 #59

Merged
merged 7 commits into from
Jul 9, 2022

Conversation

Anna-28
Copy link
Contributor

@Anna-28 Anna-28 commented Jul 6, 2022

It is handy to see what % of total spawns a specific Pokemon is. And top 20 gives a wider variety than top 10.

Anna-28 added 2 commits July 6, 2022 17:58
Percentage shows how many of all pokemon spawns were this pokemon.
Copy link
Contributor

@TurtIeSocks TurtIeSocks left a comment

Choose a reason for hiding this comment

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

Works as expected 👍

@versx
Copy link
Owner

versx commented Jul 7, 2022

@Anna-28 Looks good, any thoughts about making this configurable via the config?

@Anna-28
Copy link
Contributor Author

Anna-28 commented Jul 7, 2022

Could make it configurable, where would the config value be located? And would it allow people to set it to top 100 for example or should it have an upper limit?

Do you mean the % shown as a config setting also?

@versx
Copy link
Owner

versx commented Jul 7, 2022

Could make it configurable, where would the config value be located? And would it allow people to set it to top 100 for example or should it have an upper limit?

Do you mean the % shown as a config setting also?

I submitted a PR to your repo if you're happy with the changes, not much work but it gets it done. I'm perfectly fine with your current changes as well. It's not checking an upper limit of the variable, pretty much the Admins responsibility to set it to a reasonable number but it could be added of course. Might be worth changing the top20s to just top to prevent confusion but honestly aesthetics imo.

Anna-28#1

versx and others added 2 commits July 7, 2022 18:23
@versx versx merged commit 4362fdb into versx:master Jul 9, 2022
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.

3 participants