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

Populate neutral nature stat associations #377

Merged
merged 2 commits into from
Mar 8, 2020
Merged

Conversation

ajhyndman
Copy link
Contributor

Technically, the neutral natures are each associated with a plus and minus stat of the same value. This would be nice to include, for lookup purposes.

I'm not 100% sure if this will break anything though, so definitely need another pair of eyes.

Associations sourced from: https://pokemondb.net/mechanics/natures

Technically, the neutral natures are each associated with a plus and minus stat of the same value.  This would be nice to include, for lookup purposes.

I'm not 100% sure if this will break anything though, so definitely need another pair of eyes.
@scheibo
Copy link
Contributor

scheibo commented Mar 8, 2020

This will break calcStat, as the failing tests indicate. I'm also not sure this provides any value because its not displayed to the UI and because to make the implementation work with the plus and minus both set the same youd need to do more work than just checking for the absence.

We plan to switch to a different implementation of Nature, but it doesnt represent both the plus and minus of the same stat either.

I appreciate your interest in contributing - have you found #348 with ideas for where you might help out? Please also feel free to join http://spo.ink/dev and ask in #other-dev if you want to get involved.

@ajhyndman
Copy link
Contributor Author

Thanks for the context!

I just discovered that this library was published to npm, and was inspired to try my hand at my own calculator interface: https://github.com/ajhyndman/visual-pokemon-calc

In my app, I'm trying out allowing nature selection from the stat, instead of as a single list of natures. To do that, I wanted to support reverse lookup on all of the natures.

https://github.com/ajhyndman/visual-pokemon-calc/blob/master/src/PokemonPicker.tsx#L104-L109

I can easily patch the NATURES object for my purposes though, so if it makes things messier here, it's no big deal.

@ajhyndman
Copy link
Contributor Author

I pushed a fix for calcStat, but I won't take it personally if you prefer to close this!

@scheibo
Copy link
Contributor

scheibo commented Mar 8, 2020

prettiers formatting of this is regrettable, but I'm probably going to switch out the formatter anyway so I won't make you wrestle with it.

@scheibo scheibo merged commit 7e55b3b into smogon:master Mar 8, 2020
@scheibo
Copy link
Contributor

scheibo commented Mar 24, 2020

This seems like it actually causes some issues with how the descriptions are displayed:

https://www.smogon.com/forums/threads/pok%C3%A9mon-showdown-damage-calculator.3593546/post-8401679

@ajhyndman ajhyndman deleted the patch-2 branch April 4, 2020 21:56
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