Fix OnItemReceive hook for items in the extendedVanillaGetItem table#3063
Conversation
Malkierian
left a comment
There was a problem hiding this comment.
Just one minor thing, then I think it's ready.
9f69d0f to
d5fef77
Compare
d5fef77 to
4f5c9ea
Compare
briaguya0
left a comment
There was a problem hiding this comment.
Super happy to see things move to enums from ints, left a little comment about the _MAX usage
| if (ItemIDtoRandomizerGetMap.contains(itemID)) { | ||
| return ItemIDtoRandomizerGetMap.at(itemID); | ||
| } | ||
| return RG_MAX; |
There was a problem hiding this comment.
what's the reasoning behind using RG_MAX here? it seems like it's being used to mean "invalid", so maybe it makes sense to add RG_INVALID to the enum?
There was a problem hiding this comment.
From my limited experience in Cpp land, I've seen ENUM_MAX be used to communicate an invalid value, as well as doing arbitraryValue < ENUM_MAX to validate it's a valid option before continuing (eg: we're doing this a few times with this enum in z_actor.c)
|
Would this also depend on whether we wanted to maintain separate tables? Though, if it doesn't change anything else, I wouldn't mind adding it or the other one in the meantime to make maintaining Anchor easier while we work on the refactor. |
If you're meaning whether or not we want to merge the extendedVanillaGetItem into the randomizer table, then yes. But I don't think that will happen before v3 |
5f0082b to
03d7d15
Compare
03d7d15 to
e28b382
Compare
Currently both the OnItemReceive and OnSaleEnd hooks are receiving inaccurate information about the item that was received if the item belongs to the extendedVanillaGetItem table.
For example if you receive the kokiri emerald, as far as the hooks are concerned you recieved a bottom of the well small key, because the table lookup is done with the ItemID instead of the GetItemID
Getting the Kokiri Emerald:
Build Artifacts