pret / pokecrystal

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

Map/object/event renaming #1032

Open vulcandth opened 1 year ago

vulcandth commented 1 year ago

I know at this point we are probably tired of renaming these things... but I've always found their prefixes rather confusing... It is easy to get the two mixed up.

Reminder of how these work: Each object_event macro in the map header is loaded into the map_object struct. When the object is visible on the screen, meets time of day requirements, and a few other checks, it is copied from the map_object struct into the object_struct. When objects are no longer visible (unless a don't delete flag is specified), they are deleted from object_struct.

My recommendation is to rename the object_struct to something like active_object_struct. There are a few other recommendations in regards to the macro name's, associated labels, and constants. See the original names on the left, and the recommended changes on the right.

Original Labels Recommended Labels
object_struct (current name is fine)
wObjectStructs wObjects
wPlayerStruct / wPlayerSprite wPlayerObject/wPlayerObjectSprite
wObject#Struct / wObject#Sprite wObject#/wObject#Sprite
OBJECT_LENGTH (current name is fine)
Original Labels Recommended Labels
map_object object_event_struct
wMapObjects wObjectEvents
wPlayerObject/wPlayerObjectSprite wPlayerObjectEvent/wPlayerObjectEventSprite
wMap#Object/wMap#ObjectSprite wObjectEvent# / wObjectEvent#Sprite
MAPOBJECT_LENGTH OBJECT_EVENT_LENGTH

I am just opening this issue simply start a conversation... I too would like to avoid the unnecessary renaming of things if we are not all in agreeance. Thanks!

Rangi42 commented 1 year ago

Followup to #655.

vulcandth commented 1 year ago

@mid-kid https://github.com/pret/pokecrystal/issues/639#issuecomment-522173584

wMapObjects is still left over from when we renamed "map object"s to "object event"s. I vaguely recall having talked about this issue before, and decided there wasn't a good solution, but I don't remember why. Anyway, anything called MapObject should probably be renamed to ObjectEvent, unless someone reminds me of a reason not to.

The label ObjectEvent is currently already used here:

ObjectEvent::
    jumptextfaceplayer ObjectEventText

ObjectEventText::
    text_far _ObjectEventText
    text_end

And by many of the object_event macros:

object_event  7,  3, SPRITE_RAIKOU, SPRITEMOVEDATA_POKEMON, 0, 0, -1, -1, PAL_NPC_BROWN, OBJECTTYPE_SCRIPT, 0, ObjectEvent, EVENT_BURNED_TOWER_B1F_BEASTS_1

This ObjectEvent should ideally be renamed to ObjectEventScript, although It could be a annoying-ish disruptive change.

What are your thoughts Pfero?

mid-kid commented 1 year ago

wMapObjects is the array where these object_event structs are copied into, the name isn't conflicting from what I remember.

vulcandth commented 1 year ago

I was referring to renaming it to ObjectEvent... But... I just realized I was probably overthinking it... As I forgot the wram addresses start with w. 😂 So it would be something like wObjectEvent. Thus no conflict.

vulcandth commented 1 year ago

I updated my label recommendations in the original post.