Skip to content

Update type icons to match Pokerogue sprites#22

Open
gsajith wants to merge 1 commit into
roguedex-dev:mainfrom
gsajith:gauthams/update_type_sprites
Open

Update type icons to match Pokerogue sprites#22
gsajith wants to merge 1 commit into
roguedex-dev:mainfrom
gsajith:gauthams/update_type_sprites

Conversation

@gsajith

@gsajith gsajith commented May 10, 2024

Copy link
Copy Markdown
Contributor

What changed

I've copied the type icons from Pokerogue and added them as resources in the Chrome extension, and we're loading those instead of hitting the PokeAPI endpoint.

This depends on #21 being merged first.

Before After
Screenshot 2024-05-10 at 4 47 46 PM Screenshot 2024-05-10 at 4 51 25 PM

How to test

  • In both Chrome and Firefox:
    • Load the extension unpacked and see the changes
    • Ensure that the images are loading properly

@roguedex-dev

Copy link
Copy Markdown
Owner

Can you keep the Chrome V3 PR and this one separate? I.e. this should work regardless of manifest upgrade, and viceversa. That way we can immediately merge this one while you complete the other PR

@gsajith
gsajith force-pushed the gauthams/update_type_sprites branch from 0e9c8cc to db53cf0 Compare May 11, 2024 18:57
@gsajith

gsajith commented May 11, 2024

Copy link
Copy Markdown
Contributor Author

Updated!

@roguedex-dev

Copy link
Copy Markdown
Owner

I'm not totally sure about this change yet, I really do like the modern looking icons, although I can definitely see that using the same type icons that the game itself uses might be more user friendly. I'll let people debate about this

@ArchOwlen

Copy link
Copy Markdown
Contributor

I think this should be a toggle rather than an overwrite
If/when we'd make a config menu to let users adjust settings

And we should also probably ask PokeRogue themselves if we can use their sprites as a default
(if it's not stated in their github if we can use it)

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