mirror/Shipwright
1
0
Fork 0
mirror of https://github.com/HarbourMasters/Shipwright synced 2026-08-21 06:48:39 -04:00
Code Issues Packages Projects Releases Wiki Activity

Fix Gold Skulltula tokens overwriting the Bow inventory slot (#7021)

Browse Source 
gItemSlots and sExtraItemBases only cover the item IDs that live in the
inventory, but Item_Give and Item_CheckObtainability index them with any
item ID. Reading past gItemSlots lands on gUpgradeShifts, so
INV_CONTENT(ITEM_SKULL_TOKEN) resolved to the Bow slot and stored the
token there, permanently corrupting the save.

Route both through a bounds-checked Item_GetSlot that reports SLOT_NONE
for items that have no slot, and skip the inventory store in that case.

That read is also undefined behaviour, which link-time optimization is
free to exploit: with it present clang dropped the ITEM_SKULL_TOKEN
branch and the ITEM_MEDALLION_WATER horse fixup from Item_Give
entirely, so tokens were never counted either.

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
David Racine
2026-08-05 00:03:12 -04:00
committed by GitHub
parent 7be4a5941a
commit 29c7e75688
1 changed files with 25 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files
soh/src/code/z_parameter.c
+25 -13
View File
@@ -170,6 +170,18 @@ static s16 sExtraItemBases[] = {
ITEM_BOW, ITEM_BOW, ITEM_SEEDS, ITEM_BOMBCHU, ITEM_BOMBCHU, ITEM_STICK, ITEM_STICK, ITEM_NUT, ITEM_NUT,
};
// These tables only cover the item IDs that live in the inventory, so anything else (quest items, songs,
// medallions) must report SLOT_NONE rather than read past their end.
static s16 Item_GetSlot(u8 item) {
if ((item >= ITEM_STICKS_5) && (item < ITEM_STICKS_5 + ARRAY_COUNT(sExtraItemBases))) {
return SLOT(sExtraItemBases[item - ITEM_STICKS_5]);
}
if (item < ARRAY_COUNT(gItemSlots)) {
return SLOT(item);
}
return SLOT_NONE;
}
static s16 sEnvHazard = PLAYER_ENV_HAZARD_NONE;
static s16 sEnvHazardActive = false;
@@ -1870,13 +1882,11 @@ u8 Item_Give(PlayState* play, u8 item) {
// Gameplay stats: Update the time the item was obtained
GameplayStats_SetTimestamp(play, item);
slot = SLOT(item);
if (item >= ITEM_STICKS_5) {
slot = SLOT(sExtraItemBases[item - ITEM_STICKS_5]);
}
slot = Item_GetSlot(item);
osSyncPrintf(VT_FGCOL(YELLOW));
osSyncPrintf("item_get_setting=%d pt=%d z=%x\n", item, slot, gSaveContext.inventory.items[slot]);
osSyncPrintf("item_get_setting=%d pt=%d z=%x\n", item, slot,
(slot != SLOT_NONE) ? gSaveContext.inventory.items[slot] : ITEM_NONE);
osSyncPrintf(VT_RST);
if ((item >= ITEM_MEDALLION_FOREST) && (item <= ITEM_MEDALLION_LIGHT)) {
@@ -2450,23 +2460,25 @@ u8 Item_Give(PlayState* play, u8 item) {
}
return Return_Item(item, MOD_NONE, ITEM_NONE);
}
if (slot == SLOT_NONE) {
// Nothing to store: writing would land on an unrelated slot instead.
return Return_Item(item, MOD_NONE, ITEM_NONE);
}
returnItem = gSaveContext.inventory.items[slot];
osSyncPrintf("Item_Register(%d)=%d %d\n", slot, item, returnItem);
INV_CONTENT(item) = item;
gSaveContext.inventory.items[slot] = item;
return Return_Item(item, MOD_NONE, returnItem);
}
u8 Item_CheckObtainability(u8 item) {
s16 i;
s16 slot = SLOT(item);
s16 slot = Item_GetSlot(item);
s32 temp;
if (item >= ITEM_STICKS_5) {
slot = SLOT(sExtraItemBases[item - ITEM_STICKS_5]);
}
osSyncPrintf(VT_FGCOL(GREEN));
osSyncPrintf("item_get_non_setting=%d pt=%d z=%x\n", item, slot, gSaveContext.inventory.items[slot]);
osSyncPrintf("item_get_non_setting=%d pt=%d z=%x\n", item, slot,
(slot != SLOT_NONE) ? gSaveContext.inventory.items[slot] : ITEM_NONE);
osSyncPrintf(VT_RST);
if (IS_RANDO) {
@@ -2588,7 +2600,7 @@ u8 Item_CheckObtainability(u8 item) {
return ITEM_NONE;
}
return gSaveContext.inventory.items[slot];
return (slot != SLOT_NONE) ? gSaveContext.inventory.items[slot] : ITEM_NONE;
}
void Inventory_DeleteItem(u16 item, u16 invSlot) {