Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Map/object/event renaming #1032

Open
vulcandth opened this issue Jan 17, 2023 · 5 comments · May be fixed by #1076
Open

Map/object/event renaming #1032

vulcandth opened this issue Jan 17, 2023 · 5 comments · May be fixed by #1076
Assignees
Labels

Comments

@vulcandth
Copy link
Collaborator

vulcandth commented Jan 17, 2023

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 Rangi42 changed the title Further Object Renaming consideration Map/object/event renaming Jan 23, 2023
@Rangi42
Copy link
Member

Rangi42 commented Jan 23, 2023

Followup to #655.

@vulcandth
Copy link
Collaborator Author

@mid-kid
#639 (comment)

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
Copy link
Member

mid-kid commented Feb 10, 2023

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

@vulcandth
Copy link
Collaborator Author

vulcandth commented Feb 11, 2023

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
Copy link
Collaborator Author

I updated my label recommendations in the original post.

@vulcandth vulcandth linked a pull request Aug 22, 2023 that will close this issue
@vulcandth vulcandth self-assigned this Aug 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants