Add typedefs for MAPSEC and METLOC values #2183
Open
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds a
gametypes.h
header with typedefs formapsec_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 asu8
, but they and map sections are frequently handled asu16
s and occasionally ass16
s andint
s).This PR introduces typedefs for
MAPSEC
andMETLOC
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.foo_t
is unsigned,foo_min#_s_t
is the naming convention for a wider type that is also signed.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 anint
.It matches per
make compare
on my machine.Weird edge cases that remain incompletely addressed
frontier_pass.c
:AllocateFrontierPassData
: The locali
variable is first used as amapsec_t
, and later as a bareu8
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 amapsec_t
with a code comment explaining that it gets repurposed as a bare integer later on.pokedex_area_screen.c
:sLandmarkData
us au16[][2]
wherein the first short is amapsec_min16_t
and the second is an event flag index. Currently, this array's element type ismapsec_min16_t
, but if we ever introduce e.g.eventflag_t
I'm not sure what we'll want to do here.sRedOutlineFlyDestinations
inregion_map.c
.pokemon.c
:CreateBoxMon
: The localvalue
variable is used for several different things, and so can't be given a meaningful type. It's au32
.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 aREADME.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.
battler_t
ball_id_t
easychatword_t
item_id_t
language_t
mapgroup_t
andmapnum_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
font_id_t
gfxtag_t
andpaltag_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