force empty bottle OI to use the milk effect to avoid UB (#7247)

3 files changed, 33 insertions, 9 deletions

diff --git a/soh/soh/Enhancements/AlwaysOnFixes.cpp b/soh/soh/Enhancements/AlwaysOnFixes.cpp
index bfc1e05a8..cf1cc758e 100644
--- a/soh/soh/Enhancements/AlwaysOnFixes.cpp
+++ b/soh/soh/Enhancements/AlwaysOnFixes.cpp
@@ -1,6 +1,7 @@
 #include <libultraship/bridge/consolevariablebridge.h>
 
 #include "soh/Enhancements/game-interactor/GameInteractor_Hooks.h"
+#include "soh/Enhancements/game-interactor/vanilla-behavior/GIVanillaBehavior.h"
 #include "soh/ShipInit.hpp"
 
 extern "C" {
@@ -156,6 +157,17 @@ void RegisterAlwaysOnFixes() {
             }
         }
     });
+
+    // Empty Bottle OI (aka "Drinking Bugs") causes the bottle to read the potion effect array out of bounds
+    // The exact effect this had on N64 depended on the game version, but on ship it seems to depend on the compiler or
+    // build type. To resolve this UB, this hook forces the effect of milk, which is the most useful vanilla effect
+    // (other versions do nothing). A setting can be trivially added if the option for it to do something else is
+    // desired, but it should still be in this hook to avoid UB.
+    COND_VB_SHOULD(VB_EMPTY_BOTTLE_OI, true, {
+        if (!*should) {
+            gSaveContext.healthAccumulator = 0x50;
+        }
+    });
 }
 
 static RegisterShipInitFunc initAlwaysOnFixes(RegisterAlwaysOnFixes, { "" });
diff --git a/soh/soh/Enhancements/game-interactor/vanilla-behavior/GIVanillaBehavior.h b/soh/soh/Enhancements/game-interactor/vanilla-behavior/GIVanillaBehavior.h
index 146e5fd36..f6275bd94 100644
--- a/soh/soh/Enhancements/game-interactor/vanilla-behavior/GIVanillaBehavior.h
+++ b/soh/soh/Enhancements/game-interactor/vanilla-behavior/GIVanillaBehavior.h
@@ -4080,4 +4080,13 @@ typedef enum {
     // #### `args`
     // - `*Player`
     VB_PLAYER_UNEQUIP_MASK_WITHOUT_BUTTON,
+
+    // #### `result`
+    // ```c
+    // this->itemAction != PLAYER_IA_BOTTLE
+    // ```
+    // #### `args`
+    // - none
+    VB_EMPTY_BOTTLE_OI,
+
 } GIVanillaBehavior;
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 b5bbc6c58..6dc2e125d 100644
--- a/soh/src/overlays/actors/ovl_player_actor/z_player.c
+++ b/soh/src/overlays/actors/ovl_player_actor/z_player.c
@@ -21,6 +21,7 @@
 #include "objects/gameplay_keep/gameplay_keep.h"
 #include "objects/object_link_child/object_link_child.h"
 #include <soh/Enhancements/custom-message/CustomMessageTypes.h>
+#include "soh/Enhancements/game-interactor/vanilla-behavior/GIVanillaBehavior.h"
 #include "soh/Enhancements/item-tables/ItemTableTypes.h"
 #include "soh/Enhancements/cosmetics/cosmeticsTypes.h"
 #include "soh/Enhancements/enhancementTypes.h"
@@ -14380,18 +14381,20 @@ void Player_Action_8084EAC0(Player* this, PlayState* play) {
                     gSaveContext.healthAccumulator = rand * FULL_HEART_HEALTH;
                 }
             } else {
-                s32 sp28 = D_808549FC[this->itemAction - PLAYER_IA_BOTTLE_POTION_RED];
+                if (GameInteractor_Should(VB_EMPTY_BOTTLE_OI, this->itemAction != PLAYER_IA_BOTTLE)) {
+                    s32 sp28 = D_808549FC[this->itemAction - PLAYER_IA_BOTTLE_POTION_RED];
 
-                if (sp28 & 1) {
-                    gSaveContext.healthAccumulator = MAX_HEALTH;
-                }
+                    if (sp28 & 1) {
+                        gSaveContext.healthAccumulator = MAX_HEALTH;
+                    }
 
-                if (sp28 & 2) {
-                    Magic_Fill(play);
-                }
+                    if (sp28 & 2) {
+                        Magic_Fill(play);
+                    }
 
-                if (sp28 & 4) {
-                    gSaveContext.healthAccumulator = 0x50;
+                    if (sp28 & 4) {
+                        gSaveContext.healthAccumulator = 0x50;
+                    }
                 }
             }