-
-
Notifications
You must be signed in to change notification settings - Fork 954
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
feature: type name icons #1116
feature: type name icons #1116
Conversation
via adding `TypeSprites` to db containing type name icons from all games (excluding tera types, for now)
also rename `type` to `type_obj` to avoid reassigning the `type` function
@@ -629,6 +629,45 @@ def csv_record_to_objects(info): | |||
|
|||
build_generic((TypeEfficacyPast,), "type_efficacy_past.csv", csv_record_to_objects) | |||
|
|||
def csv_record_to_objects(info): | |||
game_map = { | |||
"generation-iii": [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi!, I'm not too fond of the hardcoded map here. Can't we fetch it dynamically?
Why do we need this map anyway?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't we do as we did for the pokemon?
https://github.com/gabeklavans/pokeapi/blob/feature/type_name_icons/data/v2/build.py#L1447
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I may be misunderstanding, but the pokemon sprites looks just as hardcoded as the type sprites I've added. The structure of the sprites
object is written out. I just used a map so that I wouldn't have to actually write the full structure and can make the code more compact with some for loops. The only thing I see about the pokemon sprites that could be considered more "dynamic" is the try_image_names
function, but that just seems to be accounting for inconsistencies in the pokemon sprite file names/extensions, which isn't a problem with the type sprite files since there are much fewer of them and should be easier to keep the same format, IMO.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oopsie, you're absolutely right! Maybe we could formalize that object into a single structure that can be used for pokemon, forms, items and types. But that's for the future. I only now realize that the map is basically representing this folder: https://github.com/PokeAPI/sprites/tree/master/sprites/types
Ok, sorry and thanks for pointing it out :)
A PokeAPI/api-data refresh has started. In ~45 minutes the staging branch of PokeAPI/api-data will be pushed with the new generated data. |
The updater script has finished its job and has now opened a Pull Request towards PokeAPI/api-data with the updated data. |
closes #1066
Adds
sprites
field totype
objects, currently containing the type name icons for all types across all applicable gens.To test, retrieve a type as normal, and there should be a new json field
sprites
for the retrieved type. There will be keys for every gen and game, and if the type existed in that game, there will be a link to the sprite corresponding to the name icon under thename_icon
field.In the future, type symbol icons can be added next to
name_icon
.I've ignored tera types for now, since I couldn't quickly come up with a way to fit them in. Let me know if you want me to work that in to this PR as well.
I'm not sure about the tests; my understanding is that they are designed to just ensure that the flow of hitting a spoof client doesn't corrupt whatever data was in the db, not necessarily that the data in the real db is correct. Let me know if I messed anything up with the new test.
I also included a fix for item sprite tests, since it looked like they were using the "pokemon" sprite path instead of the "items" sprite path.