Improve handling of biddable and hidden fields #10
Conversation
Sets more intelligent values when these fields are left unspecified. This in turn removes superfluous "both biddable and hidden" warnings.
|
Does this need a reviewer? |
|
I think it's fine, but I made a PR in case someone wanted to eyeball it and comment |
|
I left some comments, rest looks good |
|
I don't see any comments for this PR though. What's the url that shows me your comments? I'm looking at https://github.com/TwitchPlaysPokemon/pokecat/pull/10/files |
|
Here's what I see at that link: https://imgur.com/a/VwOSQFa |
| warn("Set is explicitly biddable, but also explicitly hidden, which doesn't make sense.") | ||
| else: | ||
| # fix default biddable value | ||
| if pokeset["biddable"] is None: |
There was a problem hiding this comment.
if the above warnings emits, this codepath isn't executed, meaning biddable and hidden may still be None after population. Is this expected behaviour?
There was a problem hiding this comment.
also the fields aren't being validated either. The above should be an error, or cause sensible defaults I guess
| if pokeset["hidden"] is True: | ||
| pokeset["biddable"] = False | ||
| else: | ||
| pokeset["biddable"] = not pokeset["shiny"] |
There was a problem hiding this comment.
The paragraph on biddable in pokesetspec.md needs updating
| if pokeset["hidden"] is None: | ||
| if pokeset["biddable"]: | ||
| # User set as biddable, OR user didn't specify and pokeset is not shiny | ||
| pokeset["hidden"] = False |
There was a problem hiding this comment.
I suppose hidden in pokesetspec.md needs updating too
|
oops, yea :) |
Sets more intelligent values when these fields are left unspecified.
This in turn removes superfluous "both biddable and hidden" warnings.