From be1e69b8a60ae604ba79c86fe4024cdd84f55d6f Mon Sep 17 00:00:00 2001 From: William Casarin Date: Tue, 1 Jul 2025 13:56:46 -0700 Subject: [PATCH 1/4] build: add -Wformat-security On some distros format-security is turned on to detect possible issues with non-string literals as format strings Let's explicitly turn it on and fix the ImgUi text formatting to avoid compile issues on those platforms Signed-off-by: William Casarin --- soh/CMakeLists.txt | 3 +++ soh/soh/Enhancements/Presets/Presets.cpp | 2 +- soh/soh/Enhancements/randomizer/Plandomizer.cpp | 14 +++++++------- .../randomizer/randomizer_check_tracker.cpp | 4 ++-- 4 files changed, 13 insertions(+), 10 deletions(-) diff --git a/soh/CMakeLists.txt b/soh/CMakeLists.txt index e74c14f4c..ef7cbc8c0 100644 --- a/soh/CMakeLists.txt +++ b/soh/CMakeLists.txt @@ -502,6 +502,7 @@ if (CMAKE_CXX_COMPILER_ID MATCHES "GNU|Clang|AppleClang") if (CMAKE_SYSTEM_NAME STREQUAL "Darwin") target_compile_options(${PROJECT_NAME} PRIVATE -Wall -Wextra -Wno-error + -Wformat-security -Wno-return-type -Wno-unused-parameter -Wno-unused-function @@ -529,6 +530,7 @@ if (CMAKE_CXX_COMPILER_ID MATCHES "GNU|Clang|AppleClang") elseif (CMAKE_SYSTEM_NAME STREQUAL "NintendoSwitch") target_compile_options(${PROJECT_NAME} PRIVATE -Wall -Wextra -Wno-error + -Wformat-security -Wno-return-type -Wno-unused-parameter -Wno-unused-function @@ -579,6 +581,7 @@ if (CMAKE_CXX_COMPILER_ID MATCHES "GNU|Clang|AppleClang") target_compile_options(${PROJECT_NAME} PRIVATE -Wall -Wextra -Wno-error + -Wformat-security -Wno-unused-parameter -Wno-unused-function -Wno-unused-variable diff --git a/soh/soh/Enhancements/Presets/Presets.cpp b/soh/soh/Enhancements/Presets/Presets.cpp index b84cb95ca..0132efd6c 100644 --- a/soh/soh/Enhancements/Presets/Presets.cpp +++ b/soh/soh/Enhancements/Presets/Presets.cpp @@ -395,7 +395,7 @@ void PresetsCustomWidget(WidgetInfo& info) { ImGui::TableNextRow(); ImGui::TableNextColumn(); ImGui::AlignTextToFramePadding(); - ImGui::Text(name.c_str()); + ImGui::Text("%s", name.c_str()); for (int i = PRESET_SECTION_SETTINGS; i < PRESET_SECTION_MAX; i++) { ImGui::TableNextColumn(); DrawSectionCheck(name, !info.presetValues["blocks"].contains(blockInfo[i].names[1]), &info.apply[i], diff --git a/soh/soh/Enhancements/randomizer/Plandomizer.cpp b/soh/soh/Enhancements/randomizer/Plandomizer.cpp index 4e03e8b8f..06bb69003 100644 --- a/soh/soh/Enhancements/randomizer/Plandomizer.cpp +++ b/soh/soh/Enhancements/randomizer/Plandomizer.cpp @@ -647,7 +647,7 @@ void PlandomizerOverlayText(std::pair drawObject) { imageMax.y - ImGui::CalcTextSize(std::to_string(drawObject.second).c_str()).y - 2); ImGui::SetCursorScreenPos(textPos); - ImGui::Text(std::to_string(drawObject.second).c_str()); + ImGui::Text("%s", std::to_string(drawObject.second).c_str()); // Overlay item info if (drawObject.first.GetRandomizerGet() >= RG_PROGRESSIVE_HOOKSHOT && @@ -665,7 +665,7 @@ void PlandomizerOverlayText(std::pair drawObject) { ImGui::SetCursorScreenPos(textPos); std::string overlayText = "+"; overlayText += extractNumberInParentheses(drawObject.first.GetName().english.c_str()); - ImGui::Text(overlayText.c_str()); + ImGui::Text("%s", overlayText.c_str()); } if (drawObject.first.GetRandomizerGet() >= RG_FOREST_TEMPLE_BOSS_KEY && drawObject.first.GetRandomizerGet() <= RG_GANONS_CASTLE_BOSS_KEY) { @@ -678,7 +678,7 @@ void PlandomizerOverlayText(std::pair drawObject) { break; } } - ImGui::Text(shortName.c_str()); + ImGui::Text("%s", shortName.c_str()); } if (drawObject.first.GetRandomizerGet() >= RG_OCARINA_A_BUTTON && drawObject.first.GetRandomizerGet() <= RG_OCARINA_C_RIGHT_BUTTON) { @@ -691,7 +691,7 @@ void PlandomizerOverlayText(std::pair drawObject) { break; } } - ImGui::Text(shortName.c_str()); + ImGui::Text("%s", shortName.c_str()); } } @@ -1066,7 +1066,7 @@ void PlandomizerDrawHintsWindow() { ImGui::SeparatorText(hintData.hintName.c_str()); ImGui::Text("Current Hint: "); ImGui::SameLine(); - ImGui::TextWrapped(hintData.hintText.c_str()); + ImGui::TextWrapped("%s", hintData.hintText.c_str()); if (spoilerHintData.size() > 0) { hintInputText = plandoHintData[index].hintText.c_str(); @@ -1115,9 +1115,9 @@ void PlandomizerDrawLocationsWindow(RandomizerCheckArea rcArea) { auto randoArea = Rando::StaticData::GetLocation(checkID)->GetArea(); if (rcArea == RCAREA_INVALID || rcArea == randoArea) { ImGui::TableNextColumn(); - ImGui::TextWrapped(spoilerData.checkName.c_str()); + ImGui::TextWrapped("%s", spoilerData.checkName.c_str()); ImGui::TableNextColumn(); - ImGui::TextWrapped(spoilerData.checkRewardItem.GetName().english.c_str()); + ImGui::TextWrapped("%s", spoilerData.checkRewardItem.GetName().english.c_str()); ImGui::TableNextColumn(); PlandomizerDrawItemSlots(index); if (plandoLogData[index].checkRewardItem.GetRandomizerGet() == RG_ICE_TRAP) { diff --git a/soh/soh/Enhancements/randomizer/randomizer_check_tracker.cpp b/soh/soh/Enhancements/randomizer/randomizer_check_tracker.cpp index d6334b6a0..37cb9f549 100644 --- a/soh/soh/Enhancements/randomizer/randomizer_check_tracker.cpp +++ b/soh/soh/Enhancements/randomizer/randomizer_check_tracker.cpp @@ -1085,7 +1085,7 @@ void CheckTrackerWindow::DrawElement() { totalChecksSS << totalChecksAvailable << " Available / "; } totalChecksSS << totalChecksGotten << " Checked / " << totalChecks << " Total"; - ImGui::Text(totalChecksSS.str().c_str()); + ImGui::Text("%s", totalChecksSS.str().c_str()); UIWidgets::PaddedSeparator(); @@ -1194,7 +1194,7 @@ void CheckTrackerWindow::DrawElement() { } } - ImGui::Text(areaTotalsSS.str().c_str()); + ImGui::Text("%s", areaTotalsSS.str().c_str()); UIWidgets::Tooltip(areaTotalsTooltipSS.str().c_str()); } else { ImGui::Text("???"); From 47cb94061bdc4f09ec1e69d97978132c2b3506ab Mon Sep 17 00:00:00 2001 From: William Casarin Date: Sat, 12 Jul 2025 10:02:36 -0700 Subject: [PATCH 2/4] items: introduce Item_GetIcon This is a safe wrapper around the icon table so that we don't immediately crash when we try to use an id that isn't expected Signed-off-by: William Casarin --- soh/include/functions.h | 1 + soh/src/code/z_inventory.c | 12 +++++++++++ soh/src/code/z_message_PAL.c | 8 ++++---- soh/src/code/z_parameter.c | 20 +++++++++---------- .../ovl_kaleido_scope/z_kaleido_collect.c | 8 ++++---- .../ovl_kaleido_scope/z_kaleido_equipment.c | 8 ++++---- .../misc/ovl_kaleido_scope/z_kaleido_item.c | 6 +++--- 7 files changed, 38 insertions(+), 25 deletions(-) diff --git a/soh/include/functions.h b/soh/include/functions.h index 8deb36953..b322b5095 100644 --- a/soh/include/functions.h +++ b/soh/include/functions.h @@ -1238,6 +1238,7 @@ void func_80097534(PlayState* play, RoomContext* roomCtx); void Sample_Destroy(GameState* thisx); void Sample_Init(GameState* thisx); void Inventory_ChangeEquipment(s16 equipment, u16 value); +void *Item_GetIcon(s16 item); u8 Inventory_DeleteEquipment(PlayState* play, s16 equipment); void Inventory_ChangeUpgrade(s16 upgrade, s16 value); void Object_InitBank(PlayState* play, ObjectContext* objectCtx); diff --git a/soh/src/code/z_inventory.c b/soh/src/code/z_inventory.c index 8a51c6263..71d2e94a0 100644 --- a/soh/src/code/z_inventory.c +++ b/soh/src/code/z_inventory.c @@ -169,6 +169,18 @@ void* gItemIcons[] = { gOcarinaBtnIconATex, }; + +// Safe wrapper around gItemIcons, so we don't crash with bad data when we try +// to do weird things like assign non-button items to buttons +void *Item_GetIcon(s16 itemId) +{ + if (itemId > ARRAY_COUNT(gItemIcons)-1) { + return gItemIconSoldOutTex; + } + + return gItemIcons[itemId]; +} + // Used to map item IDs to inventory slots u8 gItemSlots[] = { SLOT_STICK, SLOT_NUT, SLOT_BOMB, SLOT_BOW, SLOT_ARROW_FIRE, SLOT_DINS_FIRE, diff --git a/soh/src/code/z_message_PAL.c b/soh/src/code/z_message_PAL.c index fc63f96c8..678a89288 100644 --- a/soh/src/code/z_message_PAL.c +++ b/soh/src/code/z_message_PAL.c @@ -1652,16 +1652,16 @@ void Message_LoadItemIcon(PlayState* play, u16 itemId, s16 y) { R_TEXTBOX_ICON_XPOS = R_TEXT_INIT_XPOS - sIconItem32XOffsets[language]; R_TEXTBOX_ICON_YPOS = y + 6; R_TEXTBOX_ICON_SIZE = 32; - memcpy((uintptr_t)msgCtx->textboxSegment + MESSAGE_STATIC_TEX_SIZE, gItemIcons[itemId], - strlen(gItemIcons[itemId]) + 1); + memcpy((uintptr_t)msgCtx->textboxSegment + MESSAGE_STATIC_TEX_SIZE, Item_GetIcon(itemId), + strlen(Item_GetIcon(itemId)) + 1); // "Item 32-0" osSyncPrintf("アイテム32-0\n"); } else { R_TEXTBOX_ICON_XPOS = R_TEXT_INIT_XPOS - sIconItem24XOffsets[language]; R_TEXTBOX_ICON_YPOS = y + 10; R_TEXTBOX_ICON_SIZE = 24; - memcpy((uintptr_t)msgCtx->textboxSegment + MESSAGE_STATIC_TEX_SIZE, gItemIcons[itemId], - strlen(gItemIcons[itemId]) + 1); + memcpy((uintptr_t)msgCtx->textboxSegment + MESSAGE_STATIC_TEX_SIZE, Item_GetIcon(itemId), + strlen(Item_GetIcon(itemId)) + 1); // "Item 24" osSyncPrintf("アイテム24＝%d (%d) {%d}\n", itemId, itemId - ITEM_KOKIRI_EMERALD, 84); } diff --git a/soh/src/code/z_parameter.c b/soh/src/code/z_parameter.c index 0fc923070..13bd8ff99 100644 --- a/soh/src/code/z_parameter.c +++ b/soh/src/code/z_parameter.c @@ -5434,14 +5434,14 @@ void Interface_Draw(PlayState* play) { // B Button Icon & Ammo Count if (gSaveContext.equips.buttonItems[0] != ITEM_NONE) { if (fullUi) { - Interface_DrawItemIconTexture(play, gItemIcons[gSaveContext.equips.buttonItems[0]], 0); + Interface_DrawItemIconTexture(play, Item_GetIcon(gSaveContext.equips.buttonItems[0]), 0); } if ((player->stateFlags1 & PLAYER_STATE1_ON_HORSE) || (play->shootingGalleryStatus > 1) || ((play->sceneNum == SCENE_BOMBCHU_BOWLING_ALLEY) && Flags_GetSwitch(play, 0x38))) { if (!fullUi) { - Interface_DrawItemIconTexture(play, gItemIcons[gSaveContext.equips.buttonItems[0]], 0); + Interface_DrawItemIconTexture(play, Item_GetIcon(gSaveContext.equips.buttonItems[0]), 0); } gDPPipeSync(OVERLAY_DISP++); @@ -5522,7 +5522,7 @@ void Interface_Draw(PlayState* play) { if (gSaveContext.equips.buttonItems[1] < 0xF0) { gDPSetPrimColor(OVERLAY_DISP++, 0, 0, 255, 255, 255, interfaceCtx->cLeftAlpha); gDPSetCombineMode(OVERLAY_DISP++, G_CC_MODULATERGBA_PRIM, G_CC_MODULATERGBA_PRIM); - Interface_DrawItemIconTexture(play, gItemIcons[gSaveContext.equips.buttonItems[1]], 1); + Interface_DrawItemIconTexture(play, Item_GetIcon(gSaveContext.equips.buttonItems[1]), 1); gDPPipeSync(OVERLAY_DISP++); gDPSetCombineLERP(OVERLAY_DISP++, PRIMITIVE, ENVIRONMENT, TEXEL0, ENVIRONMENT, TEXEL0, 0, PRIMITIVE, 0, PRIMITIVE, ENVIRONMENT, TEXEL0, ENVIRONMENT, TEXEL0, 0, PRIMITIVE, 0); @@ -5535,7 +5535,7 @@ void Interface_Draw(PlayState* play) { if (gSaveContext.equips.buttonItems[2] < 0xF0) { gDPSetPrimColor(OVERLAY_DISP++, 0, 0, 255, 255, 255, interfaceCtx->cDownAlpha); gDPSetCombineMode(OVERLAY_DISP++, G_CC_MODULATERGBA_PRIM, G_CC_MODULATERGBA_PRIM); - Interface_DrawItemIconTexture(play, gItemIcons[gSaveContext.equips.buttonItems[2]], 2); + Interface_DrawItemIconTexture(play, Item_GetIcon(gSaveContext.equips.buttonItems[2]), 2); gDPPipeSync(OVERLAY_DISP++); gDPSetCombineLERP(OVERLAY_DISP++, PRIMITIVE, ENVIRONMENT, TEXEL0, ENVIRONMENT, TEXEL0, 0, PRIMITIVE, 0, PRIMITIVE, ENVIRONMENT, TEXEL0, ENVIRONMENT, TEXEL0, 0, PRIMITIVE, 0); @@ -5548,7 +5548,7 @@ void Interface_Draw(PlayState* play) { if (gSaveContext.equips.buttonItems[3] < 0xF0) { gDPSetPrimColor(OVERLAY_DISP++, 0, 0, 255, 255, 255, interfaceCtx->cRightAlpha); gDPSetCombineMode(OVERLAY_DISP++, G_CC_MODULATERGBA_PRIM, G_CC_MODULATERGBA_PRIM); - Interface_DrawItemIconTexture(play, gItemIcons[gSaveContext.equips.buttonItems[3]], 3); + Interface_DrawItemIconTexture(play, Item_GetIcon(gSaveContext.equips.buttonItems[3]), 3); gDPPipeSync(OVERLAY_DISP++); gDPSetCombineLERP(OVERLAY_DISP++, PRIMITIVE, ENVIRONMENT, TEXEL0, ENVIRONMENT, TEXEL0, 0, PRIMITIVE, 0, PRIMITIVE, ENVIRONMENT, TEXEL0, ENVIRONMENT, TEXEL0, 0, PRIMITIVE, 0); @@ -5614,7 +5614,7 @@ void Interface_Draw(PlayState* play) { if (gSaveContext.equips.buttonItems[4] < 0xF0) { gDPSetPrimColor(OVERLAY_DISP++, 0, 0, 255, 255, 255, interfaceCtx->dpadUpAlpha); gDPSetCombineMode(OVERLAY_DISP++, G_CC_MODULATERGBA_PRIM, G_CC_MODULATERGBA_PRIM); - Interface_DrawItemIconTexture(play, gItemIcons[gSaveContext.equips.buttonItems[4]], 4); + Interface_DrawItemIconTexture(play, Item_GetIcon(gSaveContext.equips.buttonItems[4]), 4); gDPPipeSync(OVERLAY_DISP++); gDPSetCombineLERP(OVERLAY_DISP++, PRIMITIVE, ENVIRONMENT, TEXEL0, ENVIRONMENT, TEXEL0, 0, PRIMITIVE, 0, PRIMITIVE, ENVIRONMENT, TEXEL0, ENVIRONMENT, TEXEL0, 0, PRIMITIVE, 0); @@ -5625,7 +5625,7 @@ void Interface_Draw(PlayState* play) { if (gSaveContext.equips.buttonItems[5] < 0xF0) { gDPSetPrimColor(OVERLAY_DISP++, 0, 0, 255, 255, 255, interfaceCtx->dpadDownAlpha); gDPSetCombineMode(OVERLAY_DISP++, G_CC_MODULATERGBA_PRIM, G_CC_MODULATERGBA_PRIM); - Interface_DrawItemIconTexture(play, gItemIcons[gSaveContext.equips.buttonItems[5]], 5); + Interface_DrawItemIconTexture(play, Item_GetIcon(gSaveContext.equips.buttonItems[5]), 5); gDPPipeSync(OVERLAY_DISP++); gDPSetCombineLERP(OVERLAY_DISP++, PRIMITIVE, ENVIRONMENT, TEXEL0, ENVIRONMENT, TEXEL0, 0, PRIMITIVE, 0, PRIMITIVE, ENVIRONMENT, TEXEL0, ENVIRONMENT, TEXEL0, 0, PRIMITIVE, 0); @@ -5636,7 +5636,7 @@ void Interface_Draw(PlayState* play) { if (gSaveContext.equips.buttonItems[6] < 0xF0) { gDPSetPrimColor(OVERLAY_DISP++, 0, 0, 255, 255, 255, interfaceCtx->dpadLeftAlpha); gDPSetCombineMode(OVERLAY_DISP++, G_CC_MODULATERGBA_PRIM, G_CC_MODULATERGBA_PRIM); - Interface_DrawItemIconTexture(play, gItemIcons[gSaveContext.equips.buttonItems[6]], 6); + Interface_DrawItemIconTexture(play, Item_GetIcon(gSaveContext.equips.buttonItems[6]), 6); gDPPipeSync(OVERLAY_DISP++); gDPSetCombineLERP(OVERLAY_DISP++, PRIMITIVE, ENVIRONMENT, TEXEL0, ENVIRONMENT, TEXEL0, 0, PRIMITIVE, 0, PRIMITIVE, ENVIRONMENT, TEXEL0, ENVIRONMENT, TEXEL0, 0, PRIMITIVE, 0); @@ -5647,7 +5647,7 @@ void Interface_Draw(PlayState* play) { if (gSaveContext.equips.buttonItems[7] < 0xF0) { gDPSetPrimColor(OVERLAY_DISP++, 0, 0, 255, 255, 255, interfaceCtx->dpadRightAlpha); gDPSetCombineMode(OVERLAY_DISP++, G_CC_MODULATERGBA_PRIM, G_CC_MODULATERGBA_PRIM); - Interface_DrawItemIconTexture(play, gItemIcons[gSaveContext.equips.buttonItems[7]], 7); + Interface_DrawItemIconTexture(play, Item_GetIcon(gSaveContext.equips.buttonItems[7]), 7); gDPPipeSync(OVERLAY_DISP++); gDPSetCombineLERP(OVERLAY_DISP++, PRIMITIVE, ENVIRONMENT, TEXEL0, ENVIRONMENT, TEXEL0, 0, PRIMITIVE, 0, PRIMITIVE, ENVIRONMENT, TEXEL0, ENVIRONMENT, TEXEL0, 0, PRIMITIVE, 0); @@ -5754,7 +5754,7 @@ void Interface_Draw(PlayState* play) { gDPSetPrimColor(OVERLAY_DISP++, 0, 0, 255, 255, 255, pauseCtx->equipAnimAlpha); gSPVertex(OVERLAY_DISP++, &pauseCtx->cursorVtx[16], 4, 0); - gDPLoadTextureBlock(OVERLAY_DISP++, gItemIcons[pauseCtx->equipTargetItem], G_IM_FMT_RGBA, G_IM_SIZ_32b, + gDPLoadTextureBlock(OVERLAY_DISP++, Item_GetIcon(pauseCtx->equipTargetItem), G_IM_FMT_RGBA, G_IM_SIZ_32b, 32, 32, 0, G_TX_NOMIRROR | G_TX_WRAP, G_TX_NOMIRROR | G_TX_WRAP, G_TX_NOMASK, G_TX_NOMASK, G_TX_NOLOD, G_TX_NOLOD); } else { diff --git a/soh/src/overlays/misc/ovl_kaleido_scope/z_kaleido_collect.c b/soh/src/overlays/misc/ovl_kaleido_scope/z_kaleido_collect.c index 78a072c6f..944983b4a 100644 --- a/soh/src/overlays/misc/ovl_kaleido_scope/z_kaleido_collect.c +++ b/soh/src/overlays/misc/ovl_kaleido_scope/z_kaleido_collect.c @@ -396,7 +396,7 @@ void KaleidoScope_DrawQuestStatus(PlayState* play, GraphicsContext* gfxCtx) { gDPSetEnvColor(POLY_OPA_DISP++, D_8082A0D8[sp218], D_8082A0E4[sp218], D_8082A0F0[sp218], 0); gSPVertex(POLY_OPA_DISP++, &pauseCtx->questVtx[sp21A], 4, 0); - KaleidoScope_DrawQuadTextureRGBA32(gfxCtx, gItemIcons[ITEM_MEDALLION_FOREST + sp218], 24, 24, 0); + KaleidoScope_DrawQuadTextureRGBA32(gfxCtx, Item_GetIcon(ITEM_MEDALLION_FOREST + sp218), 24, 24, 0); } } @@ -444,7 +444,7 @@ void KaleidoScope_DrawQuestStatus(PlayState* play, GraphicsContext* gfxCtx) { for (sp218 = 0; sp218 < 3; sp218++, sp21A += 4) { if (CHECK_QUEST_ITEM(sp218 + 0x12)) { gSPVertex(POLY_OPA_DISP++, &pauseCtx->questVtx[sp21A], 4, 0); - KaleidoScope_DrawQuadTextureRGBA32(gfxCtx, gItemIcons[ITEM_KOKIRI_EMERALD + sp218], 24, 24, 0); + KaleidoScope_DrawQuadTextureRGBA32(gfxCtx, Item_GetIcon(ITEM_KOKIRI_EMERALD + sp218), 24, 24, 0); } } @@ -455,7 +455,7 @@ void KaleidoScope_DrawQuestStatus(PlayState* play, GraphicsContext* gfxCtx) { if (CHECK_QUEST_ITEM(sp218 + 0x15)) { gSPVertex(POLY_OPA_DISP++, &pauseCtx->questVtx[sp21A], 4, 0); gDPSetPrimColor(POLY_OPA_DISP++, 0, 0, 255, 255, 255, pauseCtx->alpha); - KaleidoScope_DrawQuadTextureRGBA32(gfxCtx, gItemIcons[ITEM_STONE_OF_AGONY + sp218], 24, 24, 0); + KaleidoScope_DrawQuadTextureRGBA32(gfxCtx, Item_GetIcon(ITEM_STONE_OF_AGONY + sp218), 24, 24, 0); } } @@ -511,7 +511,7 @@ void KaleidoScope_DrawQuestStatus(PlayState* play, GraphicsContext* gfxCtx) { gSPVertex(POLY_OPA_DISP++, &pauseCtx->questVtx[sp21A], 4, 0); POLY_OPA_DISP = KaleidoScope_QuadTextureIA8( - POLY_OPA_DISP, gItemIcons[0x79 + (((gSaveContext.inventory.questItems & 0xF0000000) & 0xF0000000) >> 0x1C)], + POLY_OPA_DISP, Item_GetIcon(0x79 + (((gSaveContext.inventory.questItems & 0xF0000000) & 0xF0000000) >> 0x1C)), 48, 48, 0); } diff --git a/soh/src/overlays/misc/ovl_kaleido_scope/z_kaleido_equipment.c b/soh/src/overlays/misc/ovl_kaleido_scope/z_kaleido_equipment.c index aa18912ea..dd6d8672c 100644 --- a/soh/src/overlays/misc/ovl_kaleido_scope/z_kaleido_equipment.c +++ b/soh/src/overlays/misc/ovl_kaleido_scope/z_kaleido_equipment.c @@ -775,7 +775,7 @@ void KaleidoScope_DrawEquipment(PlayState* play) { gSPGrayscale(POLY_OPA_DISP++, true); } KaleidoScope_DrawQuadTextureRGBA32(play->state.gfxCtx, - gItemIcons[sChildUpgradeItemBases[i] + point - 1], 32, 32, 0); + Item_GetIcon(sChildUpgradeItemBases[i] + point - 1), 32, 32, 0); gSPGrayscale(POLY_OPA_DISP++, false); } } else { @@ -787,7 +787,7 @@ void KaleidoScope_DrawEquipment(PlayState* play) { gSPGrayscale(POLY_OPA_DISP++, true); } KaleidoScope_DrawQuadTextureRGBA32( - play->state.gfxCtx, gItemIcons[sChildUpgradeItemBases[i] + CUR_UPG_VALUE(sChildUpgrades[i]) - 1], + play->state.gfxCtx, Item_GetIcon(sChildUpgradeItemBases[i] + CUR_UPG_VALUE(sChildUpgrades[i]) - 1), 32, 32, 0); gSPGrayscale(POLY_OPA_DISP++, false); } else if (CUR_UPG_VALUE(sAdultUpgrades[i]) != 0) { @@ -802,7 +802,7 @@ void KaleidoScope_DrawEquipment(PlayState* play) { gSPGrayscale(POLY_OPA_DISP++, true); } KaleidoScope_DrawQuadTextureRGBA32( - play->state.gfxCtx, gItemIcons[sAdultUpgradeItemBases[i] + CUR_UPG_VALUE(sAdultUpgrades[i]) - 1], + play->state.gfxCtx, Item_GetIcon(sAdultUpgradeItemBases[i] + CUR_UPG_VALUE(sAdultUpgrades[i]) - 1), 32, 32, 0); gSPGrayscale(POLY_OPA_DISP++, false); } @@ -821,7 +821,7 @@ void KaleidoScope_DrawEquipment(PlayState* play) { } else if ((i == 0) && (k == 2) && (gBitFlags[bit + 1] & gSaveContext.inventory.equipment)) { KaleidoScope_DrawQuadTextureRGBA32(play->state.gfxCtx, gItemIconBrokenGiantsKnifeTex, 32, 32, point); } else if (gBitFlags[bit] & gSaveContext.inventory.equipment) { - KaleidoScope_DrawQuadTextureRGBA32(play->state.gfxCtx, gItemIcons[itemId], 32, 32, point); + KaleidoScope_DrawQuadTextureRGBA32(play->state.gfxCtx, Item_GetIcon(itemId), 32, 32, point); } gSPGrayscale(POLY_OPA_DISP++, false); } diff --git a/soh/src/overlays/misc/ovl_kaleido_scope/z_kaleido_item.c b/soh/src/overlays/misc/ovl_kaleido_scope/z_kaleido_item.c index beef301c4..c092f2b30 100644 --- a/soh/src/overlays/misc/ovl_kaleido_scope/z_kaleido_item.c +++ b/soh/src/overlays/misc/ovl_kaleido_scope/z_kaleido_item.c @@ -245,7 +245,7 @@ void KaleidoScope_DrawItemCycleExtras(PlayState* play, u8 slot, u8 canCycle, u8 gDPSetGrayscaleColor(POLY_OPA_DISP++, 109, 109, 109, 255); gSPGrayscale(POLY_OPA_DISP++, true); } - KaleidoScope_DrawQuadTextureRGBA32(play->state.gfxCtx, gItemIcons[leftItem], 32, 32, 0); + KaleidoScope_DrawQuadTextureRGBA32(play->state.gfxCtx, Item_GetIcon(leftItem), 32, 32, 0); gSPGrayscale(POLY_OPA_DISP++, false); } if (showRightItem) { @@ -253,7 +253,7 @@ void KaleidoScope_DrawItemCycleExtras(PlayState* play, u8 slot, u8 canCycle, u8 gDPSetGrayscaleColor(POLY_OPA_DISP++, 109, 109, 109, 255); gSPGrayscale(POLY_OPA_DISP++, true); } - KaleidoScope_DrawQuadTextureRGBA32(play->state.gfxCtx, gItemIcons[rightItem], 32, 32, 4); + KaleidoScope_DrawQuadTextureRGBA32(play->state.gfxCtx, Item_GetIcon(rightItem), 32, 32, 4); gSPGrayscale(POLY_OPA_DISP++, false); } @@ -782,7 +782,7 @@ void KaleidoScope_DrawItemSelect(PlayState* play) { gDPSetGrayscaleColor(POLY_OPA_DISP++, 109, 109, 109, 255); gSPGrayscale(POLY_OPA_DISP++, true); } - KaleidoScope_DrawQuadTextureRGBA32(play->state.gfxCtx, gItemIcons[itemId], 32, 32, 0); + KaleidoScope_DrawQuadTextureRGBA32(play->state.gfxCtx, Item_GetIcon(itemId), 32, 32, 0); gSPGrayscale(POLY_OPA_DISP++, false); } } From 82fff58062e8694d583e45816b4422d80eebcc44 Mon Sep 17 00:00:00 2001 From: William Casarin Date: Sat, 12 Jul 2025 10:05:07 -0700 Subject: [PATCH 3/4] item: introduce Item_GetAction When creating custom items, let's not immediately crash if trying to lookup an unknown item action for a new id Signed-off-by: William Casarin --- soh/include/functions.h | 1 + soh/src/overlays/actors/ovl_player_actor/z_player.c | 13 +++++++++++-- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/soh/include/functions.h b/soh/include/functions.h index b322b5095..07604cef7 100644 --- a/soh/include/functions.h +++ b/soh/include/functions.h @@ -1239,6 +1239,7 @@ void Sample_Destroy(GameState* thisx); void Sample_Init(GameState* thisx); void Inventory_ChangeEquipment(s16 equipment, u16 value); void *Item_GetIcon(s16 item); +s32 Item_GetAction(s16 item); u8 Inventory_DeleteEquipment(PlayState* play, s16 equipment); void Inventory_ChangeUpgrade(s16 upgrade, s16 value); void Object_InitBank(PlayState* play, ObjectContext* objectCtx); diff --git a/soh/src/overlays/actors/ovl_player_actor/z_player.c b/soh/src/overlays/actors/ovl_player_actor/z_player.c index 205079576..5c384b735 100644 --- a/soh/src/overlays/actors/ovl_player_actor/z_player.c +++ b/soh/src/overlays/actors/ovl_player_actor/z_player.c @@ -2253,7 +2253,7 @@ s8 Player_ItemToItemAction(s32 item) { } else if (item == ITEM_FISHING_POLE) { return PLAYER_IA_FISHING_POLE; } else { - return sItemActions[item]; + return Item_GetAction(item); } } @@ -10657,9 +10657,18 @@ void Player_StartMode_BlueWarp(PlayState* play, Player* this) { static u8 D_808546F0[] = { ITEM_SWORD_MASTER, ITEM_SWORD_KOKIRI }; +// Helper so that we don't buffer overrun item actions with custom items +s32 Item_GetAction(s16 item) { + if (item > ARRAY_COUNT(sItemActions)-1) { + return PLAYER_IA_POCKET_EGG; + } + + return sItemActions[item]; +} + void func_80846720(PlayState* play, Player* this, s32 arg2) { s32 item = D_808546F0[(void)0, gSaveContext.linkAge]; - s32 itemAction = sItemActions[item]; + s32 itemAction = Item_GetAction(item); Player_DestroyHookshot(this); Player_DetachHeldActor(play, this); From dd53ad8cd4cac1bbd9cf8896cbc4a27a00e4e369 Mon Sep 17 00:00:00 2001 From: William Casarin Date: Sat, 12 Jul 2025 10:05:38 -0700 Subject: [PATCH 4/4] item: make SLOT macro safe for custom items When creating custom items, have a fallback slot for custom item ids that don't have an entry in the itemId -> slot mapping table Signed-off-by: William Casarin --- soh/include/macros.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/soh/include/macros.h b/soh/include/macros.h index 325fb91b6..fe7770da4 100644 --- a/soh/include/macros.h +++ b/soh/include/macros.h @@ -65,7 +65,7 @@ #define IS_DAY (gSaveContext.nightFlag == 0) #define IS_NIGHT (gSaveContext.nightFlag == 1) -#define SLOT(item) gItemSlots[item] +#define SLOT(item) (item > ARRAY_COUNT(gItemSlots)-1 ? SLOT_BOTTLE_1 : gItemSlots[item]) #define INV_CONTENT(item) gSaveContext.inventory.items[SLOT(item)] #define AMMO(item) gSaveContext.inventory.ammo[SLOT(item)] #define BEANS_BOUGHT AMMO(ITEM_BEAN + 1)