Add typedefs for MAPSEC and METLOC values

[DavidJCobb]

This PR adds a gametypes.h header with typedefs for mapsec_t, metloc_t, and variants.

Description

Per discussion on the Discord, it's common for people to have to extend the ranges of certain enums, such as MAPSEC values, but because these don't have dedicated typedefs in the repo, doing so becomes clumsy. Defining a single typedef for each enumeration is complicated, though, because the codebase isn't consistent about sizes (e.g. met locations are stored as u8, but they and map sections are frequently handled as u16s and occasionally as s16s and ints).

This PR introduces typedefs for MAPSEC and METLOC values, borrowing a convention from the <inttypes.h> header:

• foo_t is the smallest type used for "foo" values.
• foo_min#_t is the naming convention for wider types; for example, foo_min16_t is a "foo" that is at least 16 bits wide.
• When foo_t is unsigned, foo_min#_s_t is the naming convention for a wider type that is also signed.
• When foo_t is signed, foo_min#_u_t is the naming convention for a wider type that is also unsigned.
• foo_int_t is the naming convention for when a "foo" is represented as an int.

It matches per make compare on my machine.

Weird edge cases that remain incompletely addressed

• frontier_pass.c: AllocateFrontierPassData: The local i variable is first used as a mapsec_t, and later as a bare u8 for a loop index. Anonymous unions don't work here (at least with agbcc), and a named union would be unnecessarily ugly (i.e. _.mapSecId and _.i). As such, this variable is a mapsec_t with a code comment explaining that it gets repurposed as a bare integer later on.
• pokedex_area_screen.c: sLandmarkData us a u16[][2] wherein the first short is a mapsec_min16_t and the second is an event flag index. Currently, this array's element type is mapsec_min16_t, but if we ever introduce e.g. eventflag_t I'm not sure what we'll want to do here.
  • Similar situation for sRedOutlineFlyDestinations in region_map.c.
• pokemon.c: CreateBoxMon: The local value variable is used for several different things, and so can't be given a meaningful type. It's a u32.

Questions

• Assuming we want to do something like this for more types/enumerations, do we want to put all of the typedefs in a single header, or do we want to have one header per collection of related types? The former option keeps the number of files to a minimum and allows you to review these types (and any others we add in the future) all at once, but it means that widening a type will cause you to have to recompile, like, 90% of the game (already a major issue with headers like global.h).

  If we're open to using one header per "family" of typedefs, then IMO it seems like a good idea to define the paths as e.g. include/game_types/mapsec_metloc.h, include/game_types/task_id.h, and so on. We can take the code comment that explains the naming conventions and reasoning, and move it to a README.md file in that subfolder.

• What other types do we want to do this for? I have my own ideas which I'll present here, but it seemed best to ask before just going and doing them all.

  • Battle engine types
    • battler_t
  • General game data
    • ball_id_t
    • easychatword_t
    • item_id_t
    • language_t
    • mapgroup_t and mapnum_t
    • move_id_t
    • pokepersonality_t (unlikely anyone will want to widen this, but it did get widened to 64 bits in Gen VI)
    • pokespecies_t
  • Graphics and infrastructure (unlikely anyone'll ever want to widen these; they'd be more for semantic value)
    • font_id_t
    • gfxtag_t and paltag_t
    • sprite_id_t
    • task_id_t
    • window_id_t

  If we do want to do this for multiple types, do we want one PR per [group of related] typedef[s] (probably easier), or an all-in-one PR?

Discord contact info

@davidjcobb

[DavidJCobb]

Per discussion on the Discord server, updated typedef name format to <purpose>_<underlying>_t e.g. mapsec_u8_t. Updated code comments in gametypes.h to explain the rationale that was presented on the server.

[mrgriffin]

Awesome work!

I have one general comment, I think we could put the #include "gametypes.h" in global.h and not write it anywhere else (I think technically it's already included in global via pokemon.h, global.fieldmap.h, and global.tv.h). Our header situation is far from best practice afaict, but untangling that seems tricky.

[mrgriffin]

iiuc the ints still in the codebase are a leftover from early in the decomp process and today we'd use s32:

Suggested change

	typedef int mapsec_int_t;
	typedef s32 mapsec_s32_t;

[mrgriffin]

I'm sure there's a reason why this name has changed, but I was wondering if you could elaborate on that reason?
Typo:

Suggested change

	NUM_mapsec_u8_tYPES
	NUM_mapsec_u8_TYPES

[mrgriffin]

Great addition!

Minor style suggestion, wrapping the body of the macro in do { ... } while (0), and not using braces for single-line statements:

Suggested change

	#define SET_BY_WIDTH(lhs) \
	if (sizeof(lhs) == 1) { \
	SET8(lhs); \
	} else if (sizeof(lhs) == 2) { \
	SET16(lhs); \
	} else if (sizeof(lhs) == 4) { \
	SET32(lhs); \
	}
	#define SET_BY_WIDTH(lhs) \
	do { \
	if (sizeof(lhs) == 1) \
	SET8(lhs); \
	else if (sizeof(lhs) == 2) \
	SET16(lhs); \
	else if (sizeof(lhs) == 4) \
	SET32(lhs); \
	} while (0)

[DavidJCobb]

Per feedback:

• Changed PR from direct includes of gametypes.h to transitive includes through global.h, consistent with the repo status quo. The oodles of transitive includes in the repo is pretty inconvenient (copying and pasting code snippets can fail because you've failed to bring along a non-obvious dependency) but inconsistency between transitive and non-transitive includes would probably be even messier.
• s32 now used in favor of int for typedef names and underlying types.
• NUM_MAPSEC_TYPES macro name repaired (it was a faulty find-and-replace; accidentally left it case-insensitive).
• SET_BY_WIDTH adjusted as requested.