vinnymac / PokeNurse

💉 A tool for Pokémon Go to aid in transferring and evolving Pokémon
284 stars 55 forks source link

Added animated sprites for details view #192

Closed hacknug closed 7 years ago

hacknug commented 7 years ago

To-do


Some useful links for the future:

hacknug commented 7 years ago

It would be great if someone could make the app append the -f suffix to the image name so it uses the right sprite for female mons. This should only be implemented for species on this list.

hacknug commented 7 years ago

Also, I only added the image-rendering: pixelated; line to the CSS file because I'm not sure if we need to cover different browsers or not. Just in case, here's what's needed to make this bullet-proof in most browsers.

-ms-interpolation-mode: nearest-neighbor;
image-rendering: -webkit-optimize-contrast;
image-rendering: -moz-crisp-edges;
image-rendering: pixelated;
YesThatAllen commented 7 years ago

It's interesting to see the gender differences. I hope the animation can be disabled, however, as I wouldn't want to see so many icons dancing all the time.

hacknug commented 7 years ago

This will only replace the images in the details modal. Everything in the list/table will stay the way it is (for) now.

hacknug commented 7 years ago

It will also fix the problem with images taking more space than they should like in that Shuckle screenshot you just shared in the &times issue @YesThatAllen

YesThatAllen commented 7 years ago

If these are replacing copywrited images, that's one great reason to implement this change.

hacknug commented 7 years ago

I don't think we would be getting rid of the copyrighting issues but implementing something that would bring more consistency to the images shown in the modal. Right new PokéNurse is using different sources for the two existing gens and, since we can't easily get Pokémon GO's sprites, this would make it look more GO-y (the game models and sprites from Pokémon GO are based on the ones from Pokémon X/Y).

This is the preview I've shared earlier on Discord just in case you haven't seen it:

2017-02-27 03_23_23 2017-02-27 03_23_37

vinnymac commented 7 years ago

@hacknug to answer your question about supporting various browsers, we do not have to. Electron uses the chrome APIs under the hood, so we only need to support whatever version of electron we use. It is one of the greatest things about building apps this way.

Note I already fixed the &times issue, it was a missing ;, so that can go out with this whenever we merge this in.

hacknug commented 7 years ago

I could add the rest of the animations up to the latest generation but do we really need to have them there? Maybe including the shiny versions would be more important? (This is already implemented in Niantic's API so I guess it's coming soon)

I don't think the third generation is coming any time soon nor that we need to support mega-evolutions yet. I can include most of the sprites easily (except for females which I need to do manually because of the way sprites are named in the source).

I already ran the GIFs through imageoptim before submitting the PR. Only thing I didn't do was to compress them to just 16 colors because some of the ones I checked were looking way worse. Since we're up-scaling the images, I thought it would be better to just reduce the file size and mantain quality.

vinnymac commented 7 years ago

@hacknug maybe we should just make adding the other generations a separate issue instead of waiting on the others, what do you say?

That way you can focus on the ones that look odd lol

hacknug commented 7 years ago

The thing is the set of sprites with the pokemon_id as their filename is compressed an looks bad/weird in most of the images. The other set I have uses name as the filename and I'm not sure what would happen with Mr. Mime, Nidoran ♀, Ho-Oh and the likes.

What would you do?

vinnymac commented 7 years ago

I'd probably write a script to rename them locally using Node JS' fs module. Then we can commit that script and reuse it whenever we come across future data sets that are using the name instead of the id.

hacknug commented 7 years ago

I messed up when I sent this from my develop branch. I opened a new PR with the changes, it's #214. I'm closing this one and moving the discussion over there.