Merge pret/master up to pret#2283#9871
Open
mrgriffin wants to merge 8 commits intorh-hideout:masterfrom
Open
Conversation
* Implement remaining MEMACC commands * Remove unnecessary memacc cases
The signed INCBINs are unused, I assume they were once for audio data but that is handled in assembly now.
'INCGFX' is like 'INCBIN' except that it specifies the arguments to pass to 'gbagfx' which alleviates the user-unfriendliness caused by catch-all rules. Specifically, users frequently forget to add to 'spritesheet_rules.mk' when adding object event graphics, and then the built '.4bpp' file is not invalidated when the add their rule so they have to 'touch' the source file or similar. The built artifacts are placed in 'build/assets'.
The exact command used to migrate source files was: git ls-files | grep 'src/.*\.[ch]$' | xargs grep -l INCBIN | xargs python3 migrate_incgfx.py The Makefile rules were cleaned up manually by looking for graphics rules and keeping only those ending with '\' (what the migration script calls a "Multi-line rule"). You may think some of wallpaper rules could be removed, but they're inputs for the '@cat $^ >$@' rules which are currently unable to be migrated to 'INCGFX'.
INCGFX: in-source arguments for gbagfx
Collaborator
Author
|
Ready for review, but some ongoing discussion on the best way to merge it so that downstreams are able to revert the EDIT: I'll just split the PR into two when it's ready to merge—force push to drop 91389f4 (so we can merge this), and then PR 91389f4 separately (so we can squash it and thus downstreams easily revert or merge up until before it, "merge it" by keeping their code, and then merge everything after it). |
Reverts all conversions to 'INCGFX', they will be contained into a separate revertable commit.
mrgriffin
force-pushed
the
rhh-master-pret-2283
branch
from
91389f4 to
3ad3a2e
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Checked before/after "Migrate to
INCGFX", sha256sum is5fda794c75769a2c5d8969f5b75449ea27bf57ce5ca96237c4aa49d0c13c54c2.Things to note in the release changelog:
INCGFXavailable inmigration_scripts/1.15/migrate_incgfx.py. Run it from clean (e.g.make clean && git ls-files | grep 'src/.*\.[ch]$' | xargs grep -l INCBIN | xargs python3 migration_scripts/1.15/migrate_incgfx.py).INCGFXexplicitly in 3ad3a2e rather than as part of the 5e5b94d. This should allow downstreams to revert just that change.