pret / pokecrystal

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

Define relationship between map_object and object_events #1054

Closed vulcandth closed 1 year ago

vulcandth commented 1 year ago

Opening this PR to further demonstrate what I'm suggesting should be done in issue #1031. Please review and look it over. If you don't think this is a good idea, or feel we want to have more conversation before this; feel free to close this PR or draft it.

Resolves #1031

mid-kid commented 1 year ago

If you're moving OBJECT_EVENT_SIZE to be more closely related to MAPOBJECT_*, I'd name it something more apt like MAPOBJECT_STRUCT_LENGTH (we don't have a standard naming scheme for struct sizes...), and move all the other script command macros to their respective structure definitions, too, creating them where missing.

Rangi42 commented 1 year ago

This is also an opportunity to define field constants for all of the warp/coord/bg/object event macros, instead of hard-coding their total sizes.

vulcandth commented 1 year ago

Hmm. I liked keeping all the *OBJECT_LENGTH constants together. Although object_event fields do happen to correspond directly to a subset of map_object fields. How about changing script_constants.asm:

; An object_event is a map_object without its leading MAPOBJECT_OBJECT_STRUCT_ID
DEF OBJECT_EVENT_SIZE EQU MAPOBJECT_LENGTH - 1 ; 13

Unfortunately It is without the leading MAPOBJECT_OBJECT_STRUCT_ID and the last two unused bytes at the end of the struct. (See the rb_skip 2).

So it would have to be something like:

DEF OBJECT_EVENT_SIZE EQU MAPOBJECT_LENGTH - 3 ; 13

We could just define it as it's own struct... but... seems kinda redundant.

Rangi42 commented 1 year ago

I'm okay with this:

; An object_event is a map_object without its initial MAPOBJECT_OBJECT_STRUCT_ID or final padding
DEF OBJECT_EVENT_SIZE EQU MAPOBJECT_LENGTH - 3 ; 13