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

Spritesheet rework #2348

Open
wants to merge 92 commits into
base: develop
Choose a base branch
from

Conversation

pogo-excalibur
Copy link
Contributor

@pogo-excalibur pogo-excalibur commented Oct 17, 2017

Description

This moves all* of the images used by the project into a single spritesheet.

* 'all' doesn't include:

  • cluster markers - as markercluster.js doesn't support this
  • the 'loading' gif - as it's a gif
  • the favicons/app icons - as these (as far as I'm aware) have to be individual files

Motivation and Context

The current sprite system requires a lot of manual work in order to issue new sprites. This PR aims to make the process for generating sprites much easier.

Additionally, this should make using custom (potentially map-specific) icons much easier.

Shoutout to @sirlindqvist for their help and advice throughout putting this together 🥇 .

How Has This Been Tested?

Small local map.

Screenshots (if appropriate):

spritesheet

Types of changes

  • New feature (non-breaking change which adds functionality)

Checklist:

  • My code follows the code style of this project.

@pogo-excalibur pogo-excalibur force-pushed the spritesheet-rework branch 2 times, most recently from 7cb859c to 506a5e2 Compare October 17, 2017 14:47
@@ -0,0 +1,2541 @@
{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shouldn't be in the PR.

.gitignore Outdated
static/icons/*.png
static/sass/sprite.sass
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be static/sass/sprite.scss
Missing static/data/sprites_map.json and static/spritesheet.png

@sirlindqvist
Copy link
Contributor

Can we use just a "?" on unknown raid boss in same font/size/style as the standard circles/numbers?

Copy link
Contributor

@sirlindqvist sirlindqvist left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using 'rebuild = true' in 'static/js/build_icons.js' overwrite custom icons with default numbers/circles.

@fosJoddie
Copy link
Contributor

This is a very "needed" feature, and the amount of work that has been put into this is tremendous. Well done.

My system:

Windows Server 2012
Intel i5-3570 3.4 GHz 16 GB of downloaded RAMs

npm install adds about 4000 more files to node-modules over the current RocketMap install. This is an increase from 11552 to 15164 files. I have not done any considerations into whether or not all the new imported modules are strictly needed or just fluff that could have been replaced by own code etc.

The sprites are generated in a timely (I guess) fashion and is also replaced by custom ones in the spritesheet if you put custom ones in.

The resulting spritesheet for some custom images is 19,697 kB - this feels like a lot when the current one is sitting around 2,256kB for icons-large-sprite.png

Lastly there seems to be some antialiasing/resizing issues going on if you use large high quality source images:

For normal icon size the map sprites are quite a bit fuzzier than before
image

This could be an artifact of the raw size of the icons - but there's a absolutely a difference from current spritesheet quality.

Using smaller icons to generate the spritesheet gives a better quality for normal sized sprites:
image

@pogo-excalibur
Copy link
Contributor Author

pogo-excalibur commented Jan 11, 2018

Apparently I forgot the 'karp.

(Cheers @sirlindqvist 😄 )

@sirlindqvist
Copy link
Contributor

Yopp, now rebase proper!

@billyjbryant
Copy link
Contributor

does this handle pokemon forms for spritegen? is it possible to also pull in the sprites for costume from #2461

@billyjbryant billyjbryant mentioned this pull request Feb 1, 2018
2 tasks
@pogo-excalibur
Copy link
Contributor Author

It adds support for Pokemon forms, but it doesn't add support for costumes.

That's a possible addition - I'll consider adding it to the PR. But this PR is already quite large (and behind schedule) so that may get delegated to a followup PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants