oot
oot copied to clipboard
Match retail BSS ordering in boot and code
Strategies used:
- Declaring some variables in header vs only in the .c file. In -g3 it seems like the block number is assigned the first time a variable is declared. For these I renamed them from
stog(although none of them were technically static before and I'm not sure if that matters). - Adding dummy structs, using the new macros
INCREMENT_BLOCK_NUMBER_BY_1(),INCREMENT_BLOCK_NUMBER_BY_10(), andINCREMENT_BLOCK_NUMBER_BY_100() - Moving variables and struct definitions around
- Adding or removing scopes inside functions by moving local variables around
For some files I left a comment to hopefully make it easier to fix them if headers change. It's hard to do in general though, especially for files which have variables declared both in a header and in the .c file. It wasn't too bad to fix though with the new tooling and a spreadsheet so hopefully it's not too much harder to reorganize headers in the future.
Notes for each file:
- idle.c: I changed gMainThread to sMainThread because it seems it should not be declared in a header, and adjusted block numbers.
- z_actor.c: It could maybe work to move
sCurCeilingPolyandsCurCeilingBgIdlater in the file, but this caused BSS ordering problems for debug. Instead, I added some block number padding in the middle. - z_camera.c: I had to add a large amount of padding between two in-function statics so I'm really unsure of how this was written originally. Padding in other places would also work.
- z_collision_check.c: Without the
sBssDummyvariables added for debug, the block numbers actually line up very nicely. I removed some scopes and moved a struct definition to make room. - z_common_data.c: Added padding
- z_demo.c: Here it does seem like
D_8015FCCC,D_8015FCD0, andD_8015FCE4should be global, and that the variables are declared in a different order than they're defined (maybe other ways are possible but it seems like it would require lots of padding) - z_kankyo.c: This is definitely the jankiest file here. First, it seems like
gLensFlareUnusedshould not be declared in a header, so I renamed it tosLensFlareUnused. Second, it seems likegLightningStrikeshould be declared before the other variables, but with some space in between before the other z_kankyo variables. I couldn't find a clean way to do this so I movedsLightningFlashAlpha,sSunDepthTestX, andsSunDepthTestYto the header too but left a TODO for later reorganization. Finally, I added a large amount of padding beforesNGameOverLightNode. It could be moved farther down in the file, but this causes BSS ordering issues for debug. - z_kaleido_scope_call.c: It seems like
D_8016139Cshould be a global variable here. - z_play.c: It seems like
sPlayVisMonomust be global here, and it would be easier ifsTransitionTilewas too. I think it makes sense given the similarity togVisMonoColorandgTransitionTileStaterespectively - main.c: Seems to require declaring variables in a different order in the header, and I moved
gPadMgrto be closer to the others instead of in padmgr.h. - speed_meter.c: It seems like
D_8016A578should be a global variable here. - sys_math3d.c: Very fragile, requires a specific starting block number.
- fault.c: The current placement of
gFaultMgrmakes this rather tight (it has block number 4 and the other variables must go before it), so I had to add different padding for retail and debug. This should be more lenient if the headers change. - fault_drawer.c: Required no changes and should not be fragile (has 1/256 chance of getting the wrong ordering)
After #1917, #1918, #1921, and #1926, gc-eu-mq is now completely matched up to the end of code (other than the ROM checksum).
For some files it's the only way, for others it's not. In a little bit I'll write more about which files need what
I reverted many of the s->g changes and added notes for each file in the PR description. I initially tried to put as many things in a header as possible because I thought that would be more robust to header changes, but I guess cleaner documentation is more important.
Since this hasn't gotten much feedback yet I pushed BSS ordering fixes for overlays too. Actually the main difficulty there is that we don't have many options for controlling the ordering without -g3 which forces us to unnaturally declare variables at the top instead of spreading them out in the file. Spreading them out would make it easier to match the ordering in retail without tons of padding.
I merged main and this PR now builds OK
From discord: fig thinks the INCREMENT_BLOCK_NUMBER_BY macros are too noisy, maybe a #pragma and some kind of source preprocessor would be better
I introduced tools/preprocess.py which will process #pragma increment_block_number N for all files in src. I made it do the reencoding too because it's already juggling temporary files and changing the encoding is very easy in Python.
This does slow the build down somewhat (by 10-20s with -j8) but maybe it's okay because that's basically how it was with asm-processor
Thanks for being flexible and trying out a new approach, appreciate it. I like the way this looks alot more