pret / pokecrystal

Disassembly of Pokémon Crystal
https://pret.github.io/pokecrystal/
2.07k stars 778 forks source link

Use a `fardw` macro to assert data is in the expected bank #1042

Closed Rangi42 closed 1 year ago

Rangi42 commented 1 year ago

Polished Crystal recently added (https://github.com/Rangi42/polishedcrystal/commit/fff83087d40569db49f5c957f09cc8173f86b1ef) a fardw macro that acts like dw, but asserts that its argument points to a bank previously set by farbank.

Typically when a pointer table has all its data in the same bank, it's efficient to use dw instead of dba. This would ensure that the dws actually point where they're supposed to.

For example:

PokedexDataPointerTable:
; entries correspond to constants/pokemon_constants.asm
    table_width 2, PokedexDataPointerTable
    farbank "Pokedex Entries 001-064"
    fardw BulbasaurPokedexEntry
    ...
    fardw KadabraPokedexEntry
    farbank "Pokedex Entries 065-128"
    fardw AlakazamPokedexEntry
    ...
    fardw TaurosPokedexEntry
    farbank "Pokedex Entries 129-192"
    fardw MagikarpPokedexEntry
    fardw GyaradosPokedexEntry
    ...
    fardw SunfloraPokedexEntry
    farbank "Pokedex Entries 193-251"
    fardw YanmaPokedexEntry
    ...
    fardw CelebiPokedexEntry
    assert_table_length NUM_POKEMON
aaaaaa123456789 commented 1 year ago

The idea isn't bad, but the name is about as misleading as it gets...

mid-kid commented 1 year ago

Seconding the very misleading name, but aside from that, I can't think of an application for it beyond the dex entry pointers. In every other case the pointer targets all reside within the same bank.

aaaaaa123456789 commented 1 year ago

I guess it's more of a hack helper than anything, because there are many pointer tables that do point to the same bank, but don't actually need to, because they're being referenced by far reads anyway.

mid-kid commented 1 year ago

I'm not sure how that'd help, you need to modify the code to put them anywhere else. fardw doesn't store the bank number.

Rangi42 commented 1 year ago

Yeah, never mind this.