pret / pokecrystal

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

Make types easier to edit with rept #1070

Closed Rangi42 closed 1 year ago

Rangi42 commented 1 year ago
TypeNames:
; entries correspond to types (see constants/type_constants.asm)
    table_width 2, TypeNames
    dw Normal
    ...
    dw Steel
rept UNUSED_TYPES_END - UNUSED_TYPES - 1 ; discount CURSE_TYPE
    dw Normal
endr
    dw CurseType
    dw Fire
    ...
    dw Dark
    assert_table_length TYPES_END
mid-kid commented 1 year ago

I can only imagine this adding confusion for little gain. It does look neater, however.

Rangi42 commented 1 year ago

My rationale is repeatedly seeing people say "I added a new type and it doesn't work" or "I made Dark/Fairy/etc physical and it doesn't work". The constants look like this:

    const_def

DEF PHYSICAL EQU const_value
    const NORMAL
    ...
    const STEEL

DEF UNUSED_TYPES EQU const_value
    const_next 19
    const CURSE_TYPE
DEF UNUSED_TYPES_END EQU const_value

DEF SPECIAL EQU const_value
    const FIRE
    ...
    const DARK
DEF TYPES_END EQU const_value

I expect the rept to be easier to work with than the current bunch of dw Normals that need manually curating when the physical types change. It would only fail if you add more than 19 physical types, which I've never heard of.

mid-kid commented 1 year ago

You're right, the behavior of both tables should match. I don't mind unrolling the const table (instead of using const_next) either.

Rangi42 commented 1 year ago

Hmm. I'd prefer not to unroll the const table, because using const_next shows how the special types start at 20. Also even if the const_next 19 were replaced with const_skip, it would be idiomatic to do one const_skip N, not on const_skip line for each dw Normal line.