pokeemerald
pokeemerald copied to clipboard
[Build System Rewrite] Everything at once: Includes `build/assets`
This pull request incorporates all changes I have made in relation to the build system. I consider this a draft while the others haven't been merged, but you could merge this and it would include all of the following:
- [ ] #1954
- [ ] #1950
- [ ] #1949
- [ ] #1950
- [ ] #1957
- [ ] #1956
- [x] #1953
- [x] #1952
To those of you considering my PRs, the intention was to subdivide changes to make them easier to adopt, as not all changes have the same levels of impact. Please read through them so you know which does what before discussion.
Additionally:
- All pokeemerald build artifacts are moved into a
build/assets
. This makes the number of files changed quite high, but its essentially a find/replace operation. In the context of graphical assets that don't have a special rule (*bpp, gbapal, lz, rl
) this is optional, and could be made optional for general audio. This is to help hacks - Graphical rules are now inside
graphics_rules.mk
. -
build/emerald
is nowbuild/emerald_modern
at the request of the Discord.
Performance:
- Initial builds are slower for a mixture of reasons (lunos4026: ~45s difference)
- Subsequent builds are way faster than the current implementation (lunos4026: 6s opposed to 40s), but are slightly slower than #1954 due to increased
mkdir
calls.
Additional comments
I don't particularly want to be working much more on this. Its taken much more effort than I originally planned.
Discord contact info
icedude907
Just commented on the non-drafts for now. For this one though I will say I'd still much rather have build artifacts outside build
than introduce the build/assets
path changes throughout the repo, primarily because I'm anticipating two points of confusion from novice users:
- Some
INCBIN
paths will be prefixed withbuild/assets
and some won't (namely uncompressed.bin
files). I suspect at some point someone will doINCBIN(build/assets/graphics/my_tilemap.bin);
. If that works as expected then it's not a big deal, but I'm sure we'll get questions about why there's a difference in paths. - I'm not sure this alleviates the "how do I make a 4bpp file" problem people get confused about, which to me is a major appeal of hiding these artifacts away in
build
. Because all theINCBIN
paths specify thebuild
folder I imagine people will still follow it to find a folder full of4bpp
files etc., and wonder how to edit them / make their own. If people instead follow it to the rootgraphics
folder they'll find a.png
of the same name, which is a little clearer (Aside, even more ideally we could specify a path to a.png
, and gbagfx could interpret the bit depth as needed or from a specified option. That way.4bpp
files are completely out-of-sight).
With those in mind I'm wondering whether moving the generated files to build
is worth the widespread changes and conflicts. The upsides I can think of are that you don't need to scroll past them in the graphics
folders (and helps point 2 above for people browsing graphics
) and it avoids some potential pitfalls of editing generated files (which atm is "handled" by warning labels at the top, though in both cases people will still continue to edit these by mistake when errors direct them to those files). I don't think I'd consider those upsides worth the trouble (obviously this is all just my opinion).
These are some very good points.
For generated code, I think build/assets
is worth due to reduced pitfalls as you mentioned. Additionally, all includes of those files now have generated/
on their path, which should make their nature more obvious.
In terms of improving clarity around INCBIN...
For me the biggest thing was reducing clutter in the filesystem so I hadn't actually thought much on introducing that perceived inconsistency between some types of files.
There's already a lot of wizardry going on (e.g.: conjuring .gbapal
from .png
and .aif
files seemingly becoming .bin
files) so maybe we should insert some basic documentation in INSTALL.md
which can be read while users are doing the initial build.
In terms of a more comprehensive solution, some kind of macro like INCBIN_GENERATED("path/to/source.png", 4bpp)
could be an option. Scaninc + the preprocessor could sort the rest.
As an aside about .bin
tilemaps, it might be worth giving them a separate extension (like .tilemap
). That'd make their purpose more obvious and would make a file explorer's "open in default app" a useful option.