search

pret / pokecrystal

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

Rename `PAL_NPC_SILVER` to `PAL_NPC_EMOTE` #1037

Closed Rangi42 closed 1 year ago

Rangi42 commented 1 year ago

The most prevalent use of this palette is for emotes, which use white as a visible color.

A few NPCs also use it -- the sleeping legendary beasts, the burned-out Firebreather in G/S, and the Silver Trophy room decoration -- but the trophy is a niche usage to name it after, and could be confused with the rival.

The old name could still be supported in legacy macros.

Idain commented 1 year ago

The idea isn't bad, but I like that "Silver" is a more general term that fits with all the NPCs that use the color. Also, I remember at some point we started renaming instances of the rival to say "RIVAL" instead of "SILVER" precisely to avoid confusion.

vulcandth commented 1 year ago

I think i'm fine with PAL_NPC_EMOTE as that is what it most often is used for. Although I could also see something like PAL_NPC_MONO as an option as well.

Also don't forget we will need to update PAL_OW_SILVER as well.

Also i'd also like to bring up possibly renaming the prefixes as well:

PAL_OW_* -> PAL_OBJ_* (In vanilla this value refers to the Pal OBJ index, and also relates this as what is used by the OBJECT itself as well.) PAL_NPC_* -> PAL_EVENT_* (These are used by ObjectEvents to override the sprites default pals. Also not every ObjectEvent is an NPC. Emotes are an example of this.)

Just a thought, i'm also okay if we decide to leave those prefixs as they are to not disrupt too much.

Rangi42 commented 1 year ago

To avoid updating the majority of map scripts, I'd leave those as-is. OW instead of OBJ is because the set of red/blue/green/brown/etc colors is loaded for the overworld, not for all screens where objects may appear. And "NPC" is shorter than "EVENT", plus not every event is an object event.

vulcandth commented 1 year ago

If we rename PAL_NPC_SILVER, I think SILVER_TROPHY can stay especially since we already renamed all instances referencing the rival SILVER to RIVAL.

Rangi42 commented 1 year ago

Well of course, SPRITE_SILVER_TROPHY is for the Silver Trophy. Not the Rival Trophy or the Emote Trophy. :P

vulcandth commented 1 year ago

Ah..lol I may have misinterpreted this statement:

-- but the trophy is a niche usage to name it after, and could be confused with the rival.

It almost sounded like you wanted to rename the trophy.

Rangi42 commented 1 year ago

I meant it would be like having PAL_NPC_GOLD instead of BROWN just because the Gold Trophy is brown.