Suggestion: a ram/ directory

[Rangi42]

The root of the repository could have a single ram.asm file, like main.asm and home.asm:

INCLUDE "constants.asm"

INCLUDE "macros/ram.asm"

INCLUDE "ram/vram.asm"
INCLUDE "ram/wram.asm"
INCLUDE "ram/sram.asm"
INCLUDE "ram/hram.asm"

[Idain]

I'd love to see it. It'd make searching for them a little easier.

[vulcandth]

I'd love to see it. It'd make searching for them a little easier.

I don't see how it would make it easier.. they are literally already in the root of the repo. Lol

However, I think it would make it better organized.

[ISSOtm]

If that's the entire contents of the file, why not go the extra mile and assemble all of the ram/* files separately?

[Idain]

I don't see how it would make it easier.. they are literally already in the root of the repo. Lol

You underestimate how easily I can get lost sometimes.

[Idain]

If that's the entire contents of the file, why not go the extra mile and assemble all of the ram/* files separately?

How so, @ISSOtm?

[ISSOtm]

Er, by assembling them separately instead of as INCLUDEs from another file.

[aaaaaa123456789]

If we do this, I'd recommend also having a linker script for RAM.

@ISSOtm we already have quite a few .o files; there's not much of a reason to split them further... particularly when the .asm files are some of the fastest to assemble. Unless you meant a dedicated ram.o for all RAM? (Don't we have a dedicated .o already?)

[Rangi42]

We already build wram.o separately from other asm, which would be ram.o this way. If you're suggesting separate ram/wram.o, ram/vram.o, etc, I think that could wait for a more general splitting of the project into many smaller .o files, as #425 discusses.

@aaaaaa123456789 Do you mean a separate ram/ram.link that gets INCLUDEd by layout.link? I'm not necessarily against that, but is there any particular reason to, since we don't have home.link, audio.link, etc? (Separate linkerscripts is another thing that would make sense after splitting if that happens, since there'd be a lot more section names to place.)

[Rangi42]

Also, one reason they're assembled in one object is because sram.asm depends on some wram.asm labels:

sGameData::
sPlayerData::  ds wPlayerDataEnd - wPlayerData
sCurMapData::  ds wCurMapDataEnd - wCurMapData
sPokemonData:: ds wPokemonDataEnd - wPokemonData
sGameDataEnd::

[vulcandth]

Also, one reason they're assembled in one object is because sram.asm depends on some wram.asm labels:

sGameData::
sPlayerData::  ds wPlayerDataEnd - wPlayerData
sCurMapData::  ds wCurMapDataEnd - wCurMapData
sPokemonData:: ds wPokemonDataEnd - wPokemonData
sGameDataEnd::

I'm thinking it should be a ram.o file that includes the various rams. I'm indifferent on the splitting of the linker file.

[mid-kid]

One idea I've considered more than once would be splitting the wram files based on what they concern themselves with. Everything in sGameData could be compiled within the same ram/game.asm file containing both the WRAM and SRAM section for everything that goes within sGameData, then have ram/box.asm for box data, ram/battle.asm for battle ram, etc.

There's quite a few places where wram data could be placed right next to the code that actually uses it, too. But I think most of this could be part of a more formal ram documentation effort.

[vulcandth]

One idea I've considered more than once would be splitting the wram files based on what they concern themselves with. Everything in sGameData could be compiled within the same ram/game.asm file containing both the WRAM and SRAM section for everything that goes within sGameData, then have ram/box.asm for box data, ram/battle.asm for battle ram, etc.

There's quite a few places where wram data could be placed right next to the code that actually uses it, too. But I think most of this could be part of a more formal ram documentation effort.

This would be ideal if our target audience wasn't ROM hackers imo. However, since our target audience are ROM hackers, splitting this up too much would be disruptive and make rearranging ram more difficult. While ROM hacking it's better in my opinion to see the whole contents of ram (the bigger picture) and be able to squeeze the last bit out of it if necessary. I don't think we should put logical pseudo-boundaries around sections of ram by splitting parts of it into its own files.

[mid-kid]

The idea was giving a better overview of what chunks of ram are used for what. I've rom hacked a fair bit myself, and I easily get lost in how insanely huge wram.asm is at the moment, as it consists of different blocks that sometimes intersect or overlap, and the nesting of UNION really doesn't help.

I've worked actively with this disassembly for over two years and have read into most of the subsystems, yet I still wouldn't have a clue where to put new variables or look for free space to use/space to optimize. I never really know if a certain variable is later stored in the save file or not, as it's just not obvious and finding out is tedious (have to figure out which boundary labels overlap the variable I'm looking at). I think subdividing the file logically, in a similar way as the original devs would probably have worked on it, would go a long way in helping people understand its overall structure and manage it more precisely, as well as rearrange it without being worried about accidentally crossing boundaries like wCurMapData/wCurMapDataEnd and such. It doesn't necessarily have to be separate files, comments and SECTIONS would help, too, but I think file boundaries make a lot of sense for the broad strokes.

At the end of the day software development (and as an extension, ROM hacking) is just as much about seeing the overall picture of things as it is delving into the fine-grained details. Can't really do the latter without the former (knowing what you're looking at).

[Rangi42]

Sounds like grouping the existing files in ram/ will be a good start then, and we may expand on that to split up the files further, split up the linkerscript, etc.

[mid-kid]

Sounds good!

[ISSOtm]

However, [...] splitting this up too much would be disruptive and make rearranging ram more difficult.

To the contrary; RAM is easier to rearrange if you simply have some smaller SECTIONs, since they can be trivially be moved around. (In a normal setting the linker would automatically take care of it, but since saving is performed by bulk copying RAM, you'd have to swap lines in the linker script.)

While ROM hacking it's better in my opinion to see the whole contents of ram (the bigger picture) and be able to squeeze the last bit out of it if necessary.

That's the linker's job; and I can tell you first-hand that the smaller the sections you feed it, the better the result it'll provide.

[ISSOtm]

I'd note that in my experience (https://github.com/ISSOtm/motherboard-gb), a ram/ directory is worse than allocating variables next to their usage point. That's a longer-term change, though, I suppose.

[aaaaaa123456789]

How would you reconcile that change with building a matching ROM?
Also, most RAM locations are used throughout the entire game.

[ISSOtm]

Matching can simply be done with a linker script to hardcode the sections to where applicable (then simply delete it if you don't care about matching).

Reused RAM locations can easily be taken care of by SECTION UNION.

[aaaaaa123456789]

I'm not talking about reusing RAM addresses, but reusing the data. Most data usage isn't localized by any means.

It should be fairly obvious how the entire game uses wPartyMons, for instance.

[ISSOtm]

Sure, but you can assign that to e.g. the party menu code.

[aaaaaa123456789]

While you can, that's strictly worse than having it in a centralized location, because now you have memory declarations in completely random places, far away from their usage and a lot harder to discover.

[ISSOtm]

That's not the case, because you can find the declarations using either editor features (I use Sublime's "Go to symbol" a lot), or at worst, a regex. As for usage, it's far from some of the use points regardless, so it's a gain for most data that isn't used like this, and for wPartyMons, it's marginal.

[aaaaaa123456789]

So the codebase, currently quickly searchable by looking on wram.asm, now requires an IDE. Bad tradeoff.

[ISSOtm]

Did you ignore the "regex" part on purpose?

[aaaaaa123456789]

No. grep -rnF wPartyMons: (which is equivalent to a regex) is clearly slower and more inconvenient than just Ctrl+F'ing wram.asm.

[ISSOtm]

It's easier to read, I agree, but also more annoying to modify. And I believe that the latter use case is more prevalent than the former.

[ISSOtm]

Additionally, this does not work for a lot of such data that is generated by macros (including all individual party mon labels), so your point is mostly moot anyway.

[aaaaaa123456789]

If anything, that makes wram.asm a better solution, because should you have to fall back to just reading through the file, it's at least one file, not the entire 400k line codebase.

[ISSOtm]

Fall back for what?

[aaaaaa123456789]

Finding a label whose declaration isn't written out, such as wPartyMon1HP.

[ISSOtm]

Such are often constructed by macros, so even having to parse through that file does not yield useful info. (I've been there several times.)

[aaaaaa123456789]

Worst case scenario, you open the file, Ctrl+F the label, and as you type it you'll be taken to somewhere near the correct declaration. Failing that, you can simply read the file, top to bottom, and you'll see a struct declaration for things related to wPartyMon1.

Neither of those things is reasonably possible with declarations scattered across the codebase and located in random places. I insist on random places because most declarations aren't used by a single file.

[ISSOtm]

The file is 3600-ish lines long, that's not a reasonable thing to suggest anyway. But sure, macro-defined labels can stay in a single (common.asm?) file to avoid that issue.

[aaaaaa123456789]

3,600 lines isn't that long to be unreasonable. Particularly since you're likely to be able to skip large chunks of it.

That being said, if only a small number of labels would be moved to the files that use them, I'd say there's no point. Maybe it would help clearing up some unions, but at some point this brings in other problems — for instance, whether a datum is between wGameData and wGameDataEnd is relevant (because that's what gets saved when you save the game), and scattering the data across sections hides this very important link.

Minor benefit plus potential for bugs creates a very bad combination. Remember that the codebase has to be friendly both for reading it unmodified and for making ROM hacks based on it.

[ISSOtm]

I'm suggesting this because AFAIK, a significant amount of labels would be moved.

[Rangi42]

There are at least some chunks of WRAM labels which would make sense to be defined in specific files at their point of use. (For example, moving wTreeMonCoordScore and wTreeMonOTIDScore to a SECTION UNION in engine/events/treemons.asm.) If the sectionful refactor discussed in #425 and prototyped in #631 actually happens, I'd be in favor of splitting up wram.asm the same way main.asm has already been split. Widely-used RAM like the whole SECTION "Party" could even go in their own files like engine/pokemon/party_ram.asm.