@bug sweep (#449)

* bug sweep round 1

* player anim frame bug already handled

* format

* stick tex and div by 0

* deku stick n64 and nicer swlift fix

10 files changed, 54 insertions, 18 deletions

diff --git a/mm/assets/xml/GC_US/objects/gameplay_keep.xml b/mm/assets/xml/GC_US/objects/gameplay_keep.xml
index afad6ec8e..5da957423 100644
--- a/mm/assets/xml/GC_US/objects/gameplay_keep.xml
+++ b/mm/assets/xml/GC_US/objects/gameplay_keep.xml
@@ -46,6 +46,7 @@
         <DList Name="gKokiriSwordDL" Offset="0x3050" />
         <DList Name="gRazorSwordDL" Offset="0x3060" />
 
+        <!-- 2S2H [Port] - Increase referenced texture's extraction size -->
         <DList Name="gDekuStickDL" Offset="0x32B0" /> <!-- @bug Supplies the wrong size to gsDPLoadTextureBlock, meaning it uses part of gLinkPauseChildJointTable as if it were a texture. -->
 
         <Array Name="gLinkMaskVtx" Count="157" Offset="0x3400"> <!-- Rendering the vertices shows the same shape as the Link Mask in object_stk -->
@@ -85,7 +86,8 @@
         <Texture Name="gameplay_keep_Tex_00C830" OutName="tex_00C830" Format="rgba16" Width="16" Height="16" Offset="0xC830" />
         <Texture Name="gameplay_keep_Tex_00CA30" OutName="tex_00CA30" Format="rgba16" Width="16" Height="16" Offset="0xCA30" />
 
-        <Texture Name="gDekuStickTex" OutName="deku_stick" Format="i8" Width="8" Height="8" Offset="0xCC30" />
+        <!-- 2S2H [Port] - gDekuStickDL loads this as a 16x16 texture, despite it being 8x8. We need to extract this extra 'garbage' data too -->
+        <Texture Name="gDekuStickTex" OutName="deku_stick" Format="i8" Width="16" Height="16" Offset="0xCC30" />
 
         <!-- Leftover joint tables from Ocarina of Time's Pause Menu. -->
         <Array Name="gLinkPauseChildJointTable" Count="24" Offset="0xCC70">
diff --git a/mm/assets/xml/N64_US/objects/gameplay_keep.xml b/mm/assets/xml/N64_US/objects/gameplay_keep.xml
index 5f4c43b58..eee79cdd1 100644
--- a/mm/assets/xml/N64_US/objects/gameplay_keep.xml
+++ b/mm/assets/xml/N64_US/objects/gameplay_keep.xml
@@ -46,6 +46,7 @@
         <DList Name="gKokiriSwordDL" Offset="0x3050" />
         <DList Name="gRazorSwordDL" Offset="0x3060" />
 
+        <!-- 2S2H [Port] - Increase referenced texture's extraction size -->
         <DList Name="gDekuStickDL" Offset="0x32B0" /> <!-- @bug Supplies the wrong size to gsDPLoadTextureBlock, meaning it uses part of gLinkPauseChildJointTable as if it were a texture. -->
 
         <Array Name="gLinkMaskVtx" Count="157" Offset="0x3400"> <!-- Rendering the vertices shows the same shape as the Link Mask in object_stk -->
@@ -85,7 +86,8 @@
         <Texture Name="gameplay_keep_Tex_00C830" OutName="tex_00C830" Format="rgba16" Width="16" Height="16" Offset="0xC830" />
         <Texture Name="gameplay_keep_Tex_00CA30" OutName="tex_00CA30" Format="rgba16" Width="16" Height="16" Offset="0xCA30" />
 
-        <Texture Name="gDekuStickTex" OutName="deku_stick" Format="i8" Width="8" Height="8" Offset="0xCC30" />
+        <!-- 2S2H [Port] - gDekuStickDL loads this as a 16x16 texture, despite it being 8x8. We need to extract this extra 'garbage' data too -->
+        <Texture Name="gDekuStickTex" OutName="deku_stick" Format="i8" Width="16" Height="16" Offset="0xCC30" />
 
         <!-- Leftover joint tables from Ocarina of Time's Pause Menu. -->
         <Array Name="gLinkPauseChildJointTable" Count="24" Offset="0xCC70">
diff --git a/mm/src/code/sys_math3d.c b/mm/src/code/sys_math3d.c
index 3710c1e1e..565dcd463 100644
--- a/mm/src/code/sys_math3d.c
+++ b/mm/src/code/sys_math3d.c
@@ -133,6 +133,8 @@ f32 Math3D_LineClosestToPoint(InfiniteLine* line, Vec3f* pos, Vec3f* closestPoin
     if (IS_ZERO(dirMagnitudeSq)) {
         Math_Vec3f_Copy(closestPoint, pos);
         //! @bug Missing early return
+        // 2S2H [Port] - return early to avoid div by 0!
+        return 0.0f;
     }
 
     t = (((pos->x - line->point.x) * line->dir.x) + ((pos->y - line->point.y) * line->dir.y) +
diff --git a/mm/src/overlays/actors/ovl_Bg_F40_Swlift/z_bg_f40_swlift.c b/mm/src/overlays/actors/ovl_Bg_F40_Swlift/z_bg_f40_swlift.c
index 6a68e9299..05078fc3e 100644
--- a/mm/src/overlays/actors/ovl_Bg_F40_Swlift/z_bg_f40_swlift.c
+++ b/mm/src/overlays/actors/ovl_Bg_F40_Swlift/z_bg_f40_swlift.c
@@ -45,7 +45,8 @@ void BgF40Swlift_Init(Actor* thisx, PlayState* play) {
     Actor_ProcessInitChain(&this->dyna.actor, sInitChain);
     DynaPolyActor_Init(&this->dyna, DYNA_TRANSFORM_POS);
     index = BG_F40_SWLIFT_GET_INDEX(thisx);
-    if ((index < 0) || (index >= 5)) { //! @bug An index greater than 3 will cause an out of bounds array access.
+    // #region 2S2H [Port] - Change index >= 5 to index >= 4 to avoid OOB array access
+    if ((index < 0) || (index >= 4)) { //! @bug An index greater than 3 will cause an out of bounds array access.
         Actor_Kill(&this->dyna.actor);
         return;
     }
diff --git a/mm/src/overlays/actors/ovl_En_S_Goro/z_en_s_goro.c b/mm/src/overlays/actors/ovl_En_S_Goro/z_en_s_goro.c
index 54876acee..8cf6a99dd 100644
--- a/mm/src/overlays/actors/ovl_En_S_Goro/z_en_s_goro.c
+++ b/mm/src/overlays/actors/ovl_En_S_Goro/z_en_s_goro.c
@@ -904,18 +904,19 @@ void EnSGoro_UpdateToIdleAnimation(EnSGoro* this) {
 }
 
 void EnSGoro_UpdateCollider(EnSGoro* this, PlayState* play) {
-    Vec3f world_pos = this->actor.world.pos;
+    Vec3f worldPos = this->actor.world.pos;
     f32 radius = 24.0f;
     f32 height = 62.0f;
 
-    this->collider.dim.pos.x = world_pos.x;
-    this->collider.dim.pos.y = world_pos.y;
-    this->collider.dim.pos.z = world_pos.z;
+    this->collider.dim.pos.x = worldPos.x;
+    this->collider.dim.pos.y = worldPos.y;
+    this->collider.dim.pos.z = worldPos.z;
     this->collider.dim.radius = radius;
     this->collider.dim.height = height;
 
-    //! @bug: It is not clear what this is for.
-    if ((s32)this != -0x190) {
+    //! @bug: The check is useless. If &this->collider somehow was NULL the above code would have already dereferenced
+    //! it.
+    if (&this->collider != NULL) {
         CollisionCheck_SetOC(play, &play->colChkCtx, &this->collider.base);
     }
 }
diff --git a/mm/src/overlays/actors/ovl_En_Ssh/z_en_ssh.c b/mm/src/overlays/actors/ovl_En_Ssh/z_en_ssh.c
index 3d7a0d605..8df5256d9 100644
--- a/mm/src/overlays/actors/ovl_En_Ssh/z_en_ssh.c
+++ b/mm/src/overlays/actors/ovl_En_Ssh/z_en_ssh.c
@@ -631,6 +631,7 @@ void EnSsh_Init(Actor* thisx, PlayState* play) {
     //! @bug: object_st_Anim_000304 is similar if not idential to object_ssh_Anim_001494.
     //! They also shared the same offset into their respective object files in OoT.
     //! However since object_ssh is the one loaded, this ends up reading garbage data from within object_ssh_Tex_000190.
+    // 2S2H [Port] - Due to the nature of the port, this ends up reading the correct data anyway
     f32 frameCount = Animation_GetLastFrame(&object_st_Anim_000304);
     s32 pad;
     EnSsh* this = THIS;
diff --git a/mm/src/overlays/actors/ovl_En_Trt/z_en_trt.c b/mm/src/overlays/actors/ovl_En_Trt/z_en_trt.c
index 188ff6705..08b977732 100644
--- a/mm/src/overlays/actors/ovl_En_Trt/z_en_trt.c
+++ b/mm/src/overlays/actors/ovl_En_Trt/z_en_trt.c
@@ -468,13 +468,15 @@ void EnTrt_GiveRedPotionForKoume(EnTrt* this, PlayState* play) {
 
 void EnTrt_GivenRedPotionForKoume(EnTrt* this, PlayState* play) {
     //! @bug: player is set to NULL not PLAYER
+    // 2S2H [Port] - Opt to comment out later reference of player to avoid bad access
     Player* player = NULL;
 
     if ((Message_GetState(&play->msgCtx) == TEXT_STATE_DONE) && Message_ShouldAdvance(play)) {
         if (this->cutsceneState == ENTRT_CUTSCENESTATE_STOPPED) {
             if (CutsceneManager_IsNext(this->csId)) {
                 CutsceneManager_StartWithPlayerCsAndSetFlag(this->csId, &this->actor);
-                player->stateFlags2 |= PLAYER_STATE2_20000000;
+                // 2S2H [Port] - player is always NULL by this point
+                // player->stateFlags2 |= PLAYER_STATE2_20000000;
                 //! @bug: EnTrt_ContinueShopping gets overwritten by EnTrt_ItemGiven
                 this->actionFunc = EnTrt_ContinueShopping;
                 this->cutsceneState = ENTRT_CUTSCENESTATE_PLAYING;
diff --git a/mm/src/overlays/actors/ovl_Obj_Nozoki/z_obj_nozoki.c b/mm/src/overlays/actors/ovl_Obj_Nozoki/z_obj_nozoki.c
index cf0191078..b6199f67e 100644
--- a/mm/src/overlays/actors/ovl_Obj_Nozoki/z_obj_nozoki.c
+++ b/mm/src/overlays/actors/ovl_Obj_Nozoki/z_obj_nozoki.c
@@ -6,6 +6,7 @@
 
 #include "z_obj_nozoki.h"
 #include "objects/object_secom_obj/object_secom_obj.h"
+#include "objects/object_gi_mssa/object_gi_mssa.h"
 
 #define FLAGS (ACTOR_FLAG_10)
 
@@ -454,8 +455,11 @@ void ObjNozoki_Update(Actor* thisx, PlayState* play) {
 
 Gfx* D_80BA34FC[] = {
     object_secom_obj_DL_000080,
-    (Gfx*)0x0A0001A0, //! @bug This dlist should point to a dlist in OBJECT_GI_MSSA, but the segment and the offset are
-                      //! wrong. This doesn't have side effects because of the extra check in the Draw function
+    //! @bug This dlist should point to a dlist in OBJECT_GI_MSSA, but the segment and the offset are
+    //! wrong. This doesn't have side effects because of the extra check in the Draw function
+    // (Gfx*)0x0A0001A0,
+    // 2S2H [Port] - Even though this is unreachable, putting the sun mask dl here
+    gGiSunMaskFaceDL,
     object_secom_obj_DL_001230,
     object_secom_obj_DL_001300,
 };
diff --git a/mm/src/overlays/actors/ovl_player_actor/z_player.c b/mm/src/overlays/actors/ovl_player_actor/z_player.c
index 9876999de..b45d6f5ed 100644
--- a/mm/src/overlays/actors/ovl_player_actor/z_player.c
+++ b/mm/src/overlays/actors/ovl_player_actor/z_player.c
@@ -10583,7 +10583,12 @@ void func_80841358(PlayState* play, Player* this, s32 arg2) {
     PlayerItemAction itemAction;
 
     //! @bug OoB read if player is goron, deku or human
-    item = D_8085D2B0[this->transformation];
+    // 2S2H [Port] - Set item to kokiri sword instead of OOB behaviour
+    if (this->transformation > 2) {
+        item = ITEM_SWORD_KOKIRI;
+    } else {
+        item = D_8085D2B0[this->transformation];
+    }
     itemAction = sItemItemActions[item];
     Player_DestroyHookshot(this);
     Player_DetachHeldActor(play, this);
diff --git a/mm/src/overlays/gamestates/ovl_file_choose/z_file_nameset_NES.c b/mm/src/overlays/gamestates/ovl_file_choose/z_file_nameset_NES.c
index e5b1d3a07..512bfc2b0 100644
--- a/mm/src/overlays/gamestates/ovl_file_choose/z_file_nameset_NES.c
+++ b/mm/src/overlays/gamestates/ovl_file_choose/z_file_nameset_NES.c
@@ -37,11 +37,17 @@ s16 D_80814280[] = {
     1, 1, 1, 2, 2, 2, 2, 2, 3, 2, 2, 4, 3, 2, 4, 1, 2, 2, 1, 1, 2, 2, 3, 2, 2, 0, 2, 2, 2, 0, 3, 1, 0,
 };
 
-s16 D_80814304[] = { 1, 2, 0, 1, 1, 2, 1, 1, 4, 2, 2, 2, 1, 1, 0, 2, 0, 1, 1, 1, 1, 1, 0, 1, 1, 1, 2, 2, 2, 2, 2, 3, 2,
-                     2, 4, 3, 2, 4, 1, 2, 2, 1, 1, 2, 2, 3, 2, 2, 0, 2, 2, 2, 3, 2, 2, 2, 2, 2, 2, 2, 2, 2, 1, 3, 0 };
+// 2S2H [Port] - Add halfword from following array to avoid OOB access
+s16 D_80814304[] = {
+    1, 2, 0, 1, 1, 2, 1, 1, 4, 2, 2, 2, 1, 1, 0, 2, 0, 1, 1, 1, 1, 1, 0, 1, 1, 1, 2, 2, 2, 2, 2, 3, 2,
+    2, 4, 3, 2, 4, 1, 2, 2, 1, 1, 2, 2, 3, 2, 2, 0, 2, 2, 2, 3, 2, 2, 2, 2, 2, 2, 2, 2, 2, 1, 3, 0, 0
+};
 
-s16 D_80814384[] = { 0, 1, 0, 1, 1, 1, 1, 1, 2, 1, 1, 1, 1, 1, 0, 1, 0, 1, 1, 1, 1, 1, 0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1,
-                     1, 2, 1, 1, 2, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0 };
+// 2S2H [Port] - Add halfword from following array to avoid OOB access
+s16 D_80814384[] = {
+    0, 1, 0, 1, 1, 1, 1, 1, 2, 1, 1, 1, 1, 1, 0, 1, 0, 1, 1, 1, 1, 1, 0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1,
+    1, 2, 1, 1, 2, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0, -94
+};
 
 s16 D_80814404[] = {
     -94, -96, -48, 0, 32, 64,
@@ -74,6 +80,7 @@ void FileSelect_SetKeyboardVtx(GameState* thisx) {
         for (phi_t1 = 0; phi_t1 < 13; phi_t1++, phi_t3 += 4, phi_t2++) {
             //! @bug D_80814304 is accessed out of bounds when drawing the empty space character (value of 64). Under
             //! normal circumstances it reads a halfword from D_80814384.
+            // 2S2H [Port] - increase D_80814304 size
             this->keyboardVtx[phi_t3].v.ob[0] = this->keyboardVtx[phi_t3 + 2].v.ob[0] = D_80814304[phi_t2] + phi_t0;
 
             this->keyboardVtx[phi_t3 + 1].v.ob[0] = this->keyboardVtx[phi_t3 + 3].v.ob[0] =
@@ -436,6 +443,7 @@ void FileSelect_DrawNameEntry(GameState* thisx) {
 
                     //! @bug D_80814384 is accessed out of bounds when drawing the empty space character (value of 64).
                     //! Under normal circumstances it reads a halfword from D_80814404.
+                    // 2S2H - increase D_80814384 size
                     this->keyboardVtx[(this->charIndex * 4) + 0].v.ob[0] =
                         this->keyboardVtx[(this->charIndex * 4) + 2].v.ob[0] =
                             this->keyboardVtx[(this->charIndex * 4) + 0].v.ob[0] + D_80814384[this->charIndex] - 2;
@@ -1254,8 +1262,16 @@ void FileSelect_DrawOptionsImpl(GameState* thisx) {
 
         //! @bug the gOptionsMenuHeaders usage here will produce an OoB read for i == 5. It reads the first element of
         //! `gOptionsMenuSettings`
+        // 2S2H [Port] - directly use first element of gOptionsMenuSettings when i == 5, note this is fixed in the GC-US
+        // version
+        u16 height;
+        if (i == 5) {
+            height = gOptionsMenuSettings[0].height;
+        } else {
+            height = gOptionsMenuHeaders[i].height;
+        }
         gDPLoadTextureBlock(POLY_OPA_DISP++, gOptionsMenuSettings[i].texture, G_IM_FMT_IA, G_IM_SIZ_8b,
-                            gOptionsMenuSettings[i].width, gOptionsMenuHeaders[i].height, 0, G_TX_NOMIRROR | G_TX_WRAP,
+                            gOptionsMenuSettings[i].width, height, 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);
         gSP1Quadrangle(POLY_OPA_DISP++, vtx, vtx + 2, vtx + 3, vtx + 1, 0);
     }