pokeemerald
pokeemerald copied to clipboard
pret
Ignore num_tiles if it would truncate non-transparent tiles
Fairly often we get people in #pokeemerald who wonder why their new tiles aren't displaying correctly, and it's often the -num_tiles in graphics_file_rules.mk.
This change makes gbagix emit a warning and ignore -num_tiles if any tile that would be excluded by -num_tiles has a non-transparent pixel. This means that adding extra tiles will automatically work (if somebody comes in the server and asks about their warning we can then tell them to delete the -num_tiles part).
e.g., I drew in the 160th tile of Petalburg's tiles.png and get this output:
tools/gbagfx/gbagfx data/tilesets/secondary/petalburg/tiles.png data/tilesets/secondary/petalburg/tiles.4bpp -num_tiles 159
Ignoring -num_tiles 159 because tile 160 contains non-transparent pixels.
Sorry, this didn't pass. I must have tested on an unclean repo which didn't build everything. I'll fix it ASAP.
I couldn't work out how to make Gimp export grayscale images so graphics/battle_transitions/rayquaza.png, graphics/picture_frame/lobby.png, and graphics/battle_transitions/regis.png are now all indexed.
Re: discussion in Discord, we might want either a "force" flag / version of num_tiles, or make that the default and add some "safe" flag / version that cancels for non empty pixels (though again, I'd be surprised if someone were using this for anything but matching).
I definitely prefer this proposal to the name hashing pokecrystal was using, or to the current state that silently drops non empty tiles from edited graphics.
Re: discussion in Discord, we might want either a "force" flag / version of
num_tiles, or make that the default and add some "safe" flag / version that cancels for non empty pixels (though again, I'd be surprised if someone were using this for anything but matching).
I'm happy to code that if it'll get the PR accepted. Which design would you want? I think I'm leaning more towards making my behavior be called something like -soft_num_tiles and putting -num_tiles back to how it was, plus changing all uses of -num_tiles into -soft_num_tiles?
(Although I still do think that a feature that isn't used anywhere in the repo and doesn't seem to have any applications isn't a great use of time :sweat_smile:)
Yeah.. I think if it had worked this way from the start and had a name that accurately described that behavior I wouldn't feel the need to have an unused 'force the number of tiles' feature. It does feel weird to have an unused option like that, but also to have a feature that might not behave the way someone expects.
Design is up to you, soft_num_tiles is fine with me. If you're all set with your time on this I can push those changes here at some point instead.
You're more than welcome to push changes if you like. I'm away over the weekend, but I could probably get the change done sometime next week if you don't get to it first :)
Instead of -soft_num_tiles I've added two new options -Wnum_tiles and -Werror=num_tiles (loosely modeled after the names they might have in something like agbcc), and used -Wnum_tiles everywhere. If there are any additional non-transparent tiles, specifying neither silently ignores them as it does today, -Wnum_tiles warns and ignores -num_tiles, and -Werror=num_tiles aborts.
It's not the best thing I've ever written so feel free to change the implementation before merging, but I don't want to spend any more time on a feature that nobody will use :sweat_smile:
Probably best to squash merge this one, I don't think anybody cares about having both commits. I'd have force pushed, but left the old commit there just in case you'd rather start from that when changing the implementation.