Sectionful refactor

[mid-kid]

Right now, the current disassembly splits sections arbitrarily. This means that
we have an odd combination of a linker script and INCLUDEs in several files
to sort everything in the ROM. Some sections even cause the game to break when
they're moved around to different banks, such as bank10 (which contains
engine/pokemon/evolve.asm) and Evolutions and Attacks.
To make it easier for a regular user to move things around to different banks
without headaches, I decided this should be fixed.

To achieve this, I plan to do the following:

1. Split everything with the sections as generated
2. Try splitting sections that span over multiple files if something inbetween is completely independent/unrelated
3. Move sections around to different files if necessary/practical
4. Merge whatever needs to be merged to avoid having too many pointless sections inside a single file
5. Remove main.asm and generate all object files separately with only the linker script to specify order. Rename included files to .inc and have the Makefile scan for .asm files.

This Pull Request is mostly to show progress, explain what I'm doing, and
discuss several decisions relating to this endeavour.

Current state

Everything I wanted to do has been done. I'll be reviewing some of the changes made, and so should you.

[mid-kid]

How the section splitting was done

First of all, I'm establishing a definition for what a "section" is: A section is a self-contained blob of code or data that can move around the ROM banks without anything breaking.

Given the current state of the disassembly, I believed this problem could be solved in a semi-objective manner by writing some tools to analyze the code. So, I wrote a couple of tools to do the following:

1. Generate a list of what "functions" (all labels bar local labels, excluding wram/sram) are referenced by each "function", and whether they're referenced through BANK() anywhere in that same "function". (this was done by parsing the compiled object files)
2. Group all "functions" that reference eachother without BANK().
3. Merge any groups that overlap anywhere in the ROM

Doing this gave me actually surprisingly decent results, but it requires working out of false-positives and other quirks. As such, every single one of these steps has an additional step to fix them. I won't go too much into detail about this here but you can read my fix_*.py scripts in the splitting-tools branch.

The biggest flaw of this system is its inability to identify unrelated sections that are right between two other sections in the same ROM bank that relate to eachother. This caused some rather huge chunks of several banks to be a single section in the initial pass I did of this. I've done my best to manually work away the biggest examples of this, but it's by no means perfect, and as such, some currently unidentified areas of the code might need to be split further, later down the line.

[mid-kid]

Current assembler-related quirks

To make sure some sections are in the same bank as others (due to non-BANK()ed references), I use the BANK[] attribute with a static bank. This could be done better if BANK["section"] were to be implemented.
Some sections are only split to use a different charmap in places. This is because the assembler creates a new, clean, section-local charmap when you use charmap in a section. This charmap is then discarded when entering a new one. This could be done better if either of these proposals were to be implemented.
I don't expect either of these to be fixed at the time of merging this PR, but they're things that would be good to fix in the long run.

[mid-kid]

Uncertainities

To be able to get out more sections, I've split Unused_PlaceEnemyHPLevel away from CopyMonToTempMon and DrawEnemyHP. However, since this function is unused, and I doubt it even works as intended, it seems rather useless to make sure it shares the same bank as the other two functions. This is mentioned in a comment, but should the assembly make sure this is the case, anyway?

Some files contain a bunch of tangentially related functions, these include most prominently engine/events/specials.asm and engine/events/std_scripts.asm.
These kinds of functions are only related to eachother through being specials/scripts, or simply being around the same area in the ROM. These can be handled in two ways: Either we leave them split, which clearly signals the user that the different functions may be moved around, and aren't really related to eachother. This would for example allow us to include all of the dummyspecials in engine/events/specials.asm, and do more such related reorganization.

I've tried splitting map scripts and songs on a per-unit basis, since each map and song has a separate file, anyway. However, there's a small problem with maps and songs referencing eachother. This happens in for example the rival theme, or the different ruins of alph chambers. Thankfully, all of the instances where this happens are right after eachother in the ROM, so there's no need to deal with splitting into separate sections. Now, there's multiple ways to deal with this:

• Include both in the same file/section.
• Rename one to .inc and include it with the INCLUDE keyword into the other.
• Keep both as separate .asm files and sections but make sure both sections are in the same bank.

I'm leaning towards the second option but this might mess with map editors that rely on everything being .asm. Though all of the options might mess with editors in different capacities.

[Rangi42]

For maps or songs that reference each other, I'd recommend option 3, "Keep both as separate .asm files and sections but make sure both sections are in the same bank." Since making sure that multiple sections share a bank is something that needs doing elsewhere anyway (currently with BANK[$xx], ideally with BANK["Section Name"]). Including one inside another would have to arbitrarily pick which one gets included.

[mid-kid]

Including one inside another would have to arbitrarily pick which one gets included.

While I agree this is the case with maps, it's actually rather obvious which one is the "parent" track with music. But I like consistency, so doing the same for maps as for audio tracks is probably the best idea.

[mid-kid]

The case for select_menu.asm

engine/pokemon/mon_menu.asm is only ever used by engine/menus/start_menu.asm, so it makes sense to include them into eachother.
However, this isn't really the case for engine/overworld/select_menu.asm. This file is only ever used from the overworld code, and does never touch any of the start menu code, nor the other way around.
The problem lies, in that it's the only user of CantUseItem. This function is right smack in the middle of mon_menu.asm. I have no idea if this is yet another artifact caused by our beloved localization team, but that's the way things are.
The ideal solution here would be to split CantUseItem off into its own section, that would then be included in select_menu.asm. However, to do so, you'd also have to split up at least three references across the function: StartMenu_Quit -> StartMenuYesNo, StartMenu_Pokemon -> PokemonActionSubmenu, TossItemFromPC -> PartyMonItemName. This would generate a bunch of sections that would depend on being located in the same bank, anyway.
This all means that there's no natural way to split this up, and we'd end up with a mess of SECTIONs and BANK[] attributes I'd rather not have. Hence, I'm probably not going to split apart select_menu.asm anytime soon. Maybe when we get a proper BANK["Section"] attribute.

[Rangi42]

mon_menu.asm wasn't originally in data/pokemon/ because everything in data/pokemon/ is related to the individual Pokémon indexes. Otherwise party_menu_qualities.asm and growth_rates.asm would also be there.

[mid-kid]

Reverted.

[mid-kid]

Alright, so for the section names, I'm currently doing the following:

• If there's only one section in the file, the section's name is the filename.
• If there's multiple, it follows the "path/to/file.asm@name" format, where name is:
  • Nothing if this can be considered the "main" section in the file (for example when the only other section is a dummy predef)
  • A label name if there's only one label in the section
  • The "main" label's name if there's multiple and there's a clear function everything revolves around
  • A "generic" name if no label's name fits. This can be part of other labels (for example FindPartyMon in the section with FindPartyMonAboveLevel, FindPartyMonThatSpecies, etc...), or something completely custom.

Big exception: anything in mobile/. I won't be able to name its sections properly before it's disassembled further. I expect anyone disassembling it to do so accordingly. The placeholder names will largely be "mobile/file.asm@StartLabel - EndLabel"

[mid-kid]

BANK_CRYSTAL

Bank $12 is a bit entangled, having been used mostly for crystal-related things. There's 5 files with a bunch of references across eachother:

• mobile/mobile_22
• engine/menus/init_gender
• engine/gfx/crystal_layouts
• engine/menus/main_menu
• mobile/mobile_menu

I've merged the first two, and the last two, and made 3 separate sections of these, to "liberate" the couple of files in-between. These files should be looked at in the future, especially mobile_22, and the rest among them, to see whether they can be split further.

[Rangi42]

Issue: a depfile won't be updated if an included file updates.

So if an .asm file INCLUDEs an .inc file which INCBINs a .2bpp file, and then you only change the .2bpp file, make won't update the ROM.

[PikalaxALT]

Is this PR still active?

[mid-kid]

Yes, I intend on merging parts of this PR upstream first, however, since that'll allow separate review of those changes.

[mid-kid]

Closing for the time being, as mentioned in a previous comment, this should be split up.