OoT-Randomizer
OoT-Randomizer copied to clipboard
Introduce a `GetItemId` enum
As proposed on discord I made a GetItemId
enum, which represents the entries from the item_table
table
This PR includes an enums for both C and Python code
I tried to replace as many instances of GI raw values and replace them with the enum values. If I missed any please tell me!
There's no "compiled" files diff (bundle.o, asm_symbols.txt, etc) because the enum doesn't impact the codegen in any way.
There are a few places where the code had comments indicating to which GI the raw value corresponds to. This is no longer necessary with the enums, but I left them in case you guys still want them, if not I can remove them.
case 0: return GI_GORONS_BRACELET; // Goron Bracelet
case 1: return GI_SILVER_GAUNTLETS; // Silver Gauntlets
default: return GI_GOLD_GAUNTLETS; // Gold Gauntlets
Advantages of using an enum
- Improves readability and searchability.
- No need for comments indicating to which item a magic number corresponds to
- i.e
return GI_GORONS_BRACELET;
doesn't need a comment likereturn 0x54; // Goron Bracelet
- i.e
- Using an enum for the GI items helps a lot for cases when you have a branch/PR that adds new GIs and another PR that adds GIs gets merged to upstream, values for the table would automatically not overlap between them, making merging upstream less error-prone.
Disadvantages of using an enum
- The
item_table
ends up with way too much spacing. I guess I could try reducing the spacing by category or something if preferred.
Future work
- Enums themselves can be used as types, improving readability even more.
For example (click on details to expand):
// example 1
uint16_t resolved_item_id = resolve_upgrades(override);
switch (resolved_item_id) {
case GI_OCARINA_FAIRY: { // Fairy Ocarina
z64_file.items[Z64_SLOT_OCARINA] = 0x07;
break;
}
case GI_OCARINA_OF_TIME: { // Ocarina of Time
z64_file.items[Z64_SLOT_OCARINA] = 0x08;
break;
}
}
// example 2
uint16_t strength_upgrade(z64_file_t *save, override_t override) {
switch ((override.value.base.player == PLAYER_ID || !MW_PROGRESSIVE_ITEMS_ENABLE) ? save->strength_upgrade : MW_PROGRESSIVE_ITEMS_STATE[override.value.base.player].strength) {
case 0: return GI_GORONS_BRACELET; // Goron Bracelet
case 1: return GI_SILVER_GAUNTLETS; // Silver Gauntlets
default: return GI_GOLD_GAUNTLETS; // Gold Gauntlets
}
}
// example 3
item_row_t *get_item_row(uint16_t item_id) {
if (item_id >= GI_RANDO_MAX) {
return NULL;
}
item_row_t *item_row = &(item_table[item_id]);
if (item_row->base_item_id == 0) {
return NULL;
}
return item_row;
}
would becomes this:
// example 1
GetItemID resolved_item_id = resolve_upgrades(override);
switch (resolved_item_id) {
case GI_OCARINA_FAIRY: { // Fairy Ocarina
z64_file.items[Z64_SLOT_OCARINA] = 0x07;
break;
}
case GI_OCARINA_OF_TIME: { // Ocarina of Time
z64_file.items[Z64_SLOT_OCARINA] = 0x08;
break;
}
}
// example 2
GetItemID strength_upgrade(z64_file_t *save, override_t override) {
switch ((override.value.base.player == PLAYER_ID || !MW_PROGRESSIVE_ITEMS_ENABLE) ? save->strength_upgrade : MW_PROGRESSIVE_ITEMS_STATE[override.value.base.player].strength) {
case 0: return GI_GORONS_BRACELET; // Goron Bracelet
case 1: return GI_SILVER_GAUNTLETS; // Silver Gauntlets
default: return GI_GOLD_GAUNTLETS; // Gold Gauntlets
}
}
// example 3
item_row_t *get_item_row(GetItemID item_id) {
if (item_id >= GI_RANDO_MAX) {
return NULL;
}
item_row_t *item_row = &(item_table[item_id]);
if (item_row->base_item_id == 0) {
return NULL;
}
return item_row;
}
Looks good to me from a code perspective, I'd like to see someone play through a seed on this branch though, just to make sure everything's still working correctly
I've played through most of a seed and everything seemed fine. So I'd say this should be good to merge after the upcoming release, a rebase or merge of latest Dev probably couldn't hurt though.