Misc things to rename.

[mid-kid]

Map setup command constants don't match their function counterpart.

See constants/map_setup_constants.asm and engine/overworld/map_setup.asm. For example map_face_down vs SpawnInFacingDown.

While it is true that some of these functions are used for more than just map setup scripts, we should make sure the names match to the best of our ability, to make them easier to find.

[mid-kid]

As an extension, RestoreFacingAfterWarp doesn't seem to be doing what the name implies.

[Rangi42]

Taking this as a "misc things to rename" issue:

It's only clear that TILESET_BATTLE_TOWER is for the indoor maps because BATTLE_TOWER_OUTSIDE exists. Can it be renamed to TILESET_BATTLE_TOWER_INSIDE?

[mid-kid]

There's discrepancies between object_struct as defined in constants/map_object_constants.asm and macros/wram.asm, for example, OBJECT_FACING maps to \1Direction, where OBJECT_FACING_STEP maps to \1Facing.

[mid-kid]

The length of the map_object struct is OBJECT_LENGTH while the length of the object_struct struct is OBJECT_STRUCT_LENGTH. I propose to fix this to be MAPOBJECT_LENGTH and OBJECT_LENGTH, to better correspond with their other constant names.

[Rangi42]

Not many people mess with the sprite system internals, so the change in meaning of OBJECT_LENGTH should be okay.

[mid-kid]

The buttonsound and waitbutton commands are related to eachother: They're both used to wait for the player to press a button to end a textbox. However, the former places the blinking "▼" at the bottom-right corner of the textbox, and plays a sound. I don't think buttonsound is a good name, but I don't know of any good alternatives right now.

[Rangi42]

Maybe waitbuttonsfx/WaitButtonSfx? Or promptwaitbutton/PromptWaitButton, as a parallel to text prompt which also shows a cursor.

[Rangi42]

gfx/intro/*.tilemap need renaming to go with their tileset graphics. Also 011.tilemap is actually the attrmap corresponding to 012.tilemap.

[Rangi42]

gfx/unknown/17eb8e.attrmap, gfx/mobile/havewant_map.bin, and gfx/sgb/sgb_border.bin are all the same format (interlaced tiles+attributes) and should share an extension. Also, 17eb8e goes with gfx/mobile/pokemon_news.png.

[mid-kid]

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.

[Rangi42]

We have the unused gfx/mobile/electro_ball_nonmatching.png, which illustrates how electro_ball.png would look without duplicate tiles removed (apparently not all of them are removed, so we can't rely on the gfx tool to convert it). I'm okay with this, but let's also add gfx/sgb/sgb_border_nonmatching.bin, with the 20x18 screen's 00 bytes added in the middle so it looks aligned at 32x28.

[aaaaaa123456789]

In engine/pokemon/mon_menu.asm:

Function132d3 → PlaceMoveScreenArrows
Function132da → PlaceMoveScreenLeftArrow
Function132fe → PlaceMoveScreenRightArrow

[Rangi42]

Crossing out points addressed by #654 :

• Map setup command constants vs. functions
• map_warp_face vs. RestoreFacingAfterWarp are misnomers
• TILESET_BATTLE_TOWER -> TILESET_BATTLE_TOWER_INSIDE
• object_struct constants vs. WRAM labels
• OBJECT_LENGTH/OBJECT_STRUCT_LENGTH -> MAPOBJECT_LENGTH/OBJECT_LENGTH
• buttonsound -> promptbutton
• gfx/intro/*.tilemap
• gfx/unknown/17eb8e.attrmap
• *MapObject* -> *ObjectEvent*
• gfx/sgb/sgb_border_nonmatching.bin
• engine/pokemon/mon_menu.asm functions

[mid-kid]

I had made a start at this issue, but didn't get much further than:

• Map setup command constants vs. functions

Imo, that one warrants a separate commit/PR anyway, so I'll finish it up and make one.

[rawr51919]

There's discrepancies between object_struct as defined in constants/map_object_constants.asm and macros/wram.asm, for example, OBJECT_FACING maps to \1Direction, where OBJECT_FACING_STEP maps to \1Facing.

Would it make sense to have OBJECT_FACING become OBJECT_FACING_DIRECTION and OBJECT_FACING_STEP become OBJECT_FACING? Might need something more descriptive for those, though, as then it'd be hard to figure out which one is which.

[Rangi42]

Try to avoid changing the meaning of a particular name like OBJECT_FACING unless it's absolutely necessary. This already happened with loadvar and OBJECT_LENGTH, and it can lead to confusion for older projects.

[rawr51919]

Try to avoid changing the meaning of a particular name like OBJECT_FACING unless it's absolutely necessary. This already happened with loadvar and OBJECT_LENGTH, and it can lead to confusion for older projects.

Would it make sense to change the macro names, then (\1Direction to \1Facing and \1Facing to \1Step)? I don't see how this could reasonably be done without changing the meaning of some things. Might as well run these by you all first.