A bit of cleanup on multiple hooks (#5879)

* A bit of cleanup on BGS hook

* Cleanup on a few more hooks, fix itemId ref

* Use direct pointer to params

* Revert item receive ID hook setup

* Remove callbacks from menu GUI

* Add comments explaining conditions of permanent loss methods

* Move custom skeletons hook to subfolder

* Clang format

* Shorten comment

* Remove unnecessary re-register function call

soh/soh/Enhancements/Difficulty/PermanentLosses.cpp

@@ -1,5 +1,4 @@
 #include "soh/Enhancements/game-interactor/GameInteractor_Hooks.h"
-#include "soh/Enhancements/mods.h"
 #include "soh/OTRGlobals.h"
 #include "soh/SaveManager.h"
 #include "soh/ShipInit.hpp"
@@ -23,9 +22,14 @@ static constexpr int32_t CVAR_DELETE_FILE_DEFAULT = 0;
 
 static bool hasAffectedHealth = false;
 
-void UpdatePermanentHeartLossState() {
-    if (!GameInteractor::IsSaveLoaded() || !hasAffectedHealth || CVAR_PERM_HEART_LOSS_VALUE)
+static void UpdatePermanentHeartLossState() {
+    // Reset Link's hearts to the normal value without permanent losses. Only applies if all of the following are true:
+    // - A saved game is playing
+    // - The "Permanent Heart Loss" setting is turned off
+    // - The player has lost at least one Heart Container
+    if (!GameInteractor::IsSaveLoaded() || !hasAffectedHealth || CVAR_PERM_HEART_LOSS_VALUE) {
         return;
+    }
 
     uint8_t heartContainers = gSaveContext.ship.stats.heartContainers; // each worth 16 health
     uint8_t heartPieces = gSaveContext.ship.stats.heartPieces;         // each worth 4 health, but only in groups of 4
@@ -39,8 +43,10 @@ void UpdatePermanentHeartLossState() {
 }
 
 static void UpdateHealthCapacity() {
-    if (!GameInteractor::IsSaveLoaded())
+    // Applies permanent losses of Heart Containers to Link's health. Only applies when a saved game is playing.
+    if (!GameInteractor::IsSaveLoaded()) {
         return;
+    }
 
     if (gSaveContext.healthCapacity > 16 && gSaveContext.healthCapacity - gSaveContext.health >= 16) {
         gSaveContext.healthCapacity -= 16;
@@ -50,8 +56,9 @@ static void UpdateHealthCapacity() {
 }
 
 static void DeleteFileOnDeath() {
-    if (!GameInteractor::IsSaveLoaded() || gPlayState == NULL)
+    if (!GameInteractor::IsSaveLoaded() || gPlayState == NULL) {
         return;
+    }
 
     if (gPlayState->gameOverCtx.state == GAMEOVER_DEATH_MENU && gPlayState->pauseCtx.state == 9) {
         SaveManager::Instance->DeleteZeldaFile(gSaveContext.fileNum);
@@ -63,6 +70,7 @@ static void DeleteFileOnDeath() {
 }
 
 static void RegisterPermanentHeartLoss() {
+    UpdatePermanentHeartLossState();
     COND_HOOK(OnPlayerUpdate, CVAR_PERM_HEART_LOSS_VALUE, UpdateHealthCapacity);
 }
 

soh/soh/Enhancements/ExtraModes/MirroredWorld.cpp

@@ -3,13 +3,12 @@
 #include "soh/Enhancements/randomizer/3drando/random.hpp"
 #include "soh/Enhancements/randomizer/context.h"
 #include "soh/Enhancements/enhancementTypes.h"
-#include "soh/Enhancements/mods.h"
 #include "soh/ResourceManagerHelpers.h"
 #include "soh/ShipInit.hpp"
 
 extern "C" {
-#include "variables.h"
 extern SaveContext gSaveContext;
+extern PlayState* gPlayState;
 }
 
 static constexpr MirroredWorldMode CVAR_MIRRORED_WORLD_DEFAULT = MIRRORED_WORLD_OFF;
@@ -55,13 +54,7 @@ static bool MirroredWorld_ShouldApply(int32_t sceneNum) {
     }
 }
 
-static void RegisterMirroredWorld() {
-    COND_HOOK(OnSceneInit, CVAR_MIRRORED_WORLD_MODE_VALUE, UpdateMirrorModeState);
-}
-
-static RegisterShipInitFunc initFunc(RegisterMirroredWorld, { CVAR_MIRRORED_WORLD_MODE_NAME });
-
-void UpdateMirrorModeState(int32_t sceneNum) {
+static void UpdateMirrorModeState(int32_t sceneNum) {
     bool nextMirroredWorld = MirroredWorld_ShouldApply(sceneNum);
 
     if (prevMirroredWorld == nextMirroredWorld) {
@@ -73,8 +66,17 @@ void UpdateMirrorModeState(int32_t sceneNum) {
         CVarSetInteger(CVAR_MIRRORED_WORLD_NAME, 1);
     } else {
         CVarClear(CVAR_MIRRORED_WORLD_NAME);
-        RegisterMirroredWorld();
     }
 
     ApplyMirrorWorldGfxPatches();
 }
+
+static void RegisterMirroredWorld() {
+    if (gPlayState != NULL) {
+        UpdateMirrorModeState(gPlayState->sceneNum);
+    }
+
+    COND_HOOK(OnSceneInit, CVAR_MIRRORED_WORLD_MODE_VALUE, UpdateMirrorModeState);
+}
+
+static RegisterShipInitFunc initFunc(RegisterMirroredWorld, { CVAR_MIRRORED_WORLD_MODE_NAME });

soh/soh/Enhancements/ExtraModes/RupeeDash.cpp

@@ -17,7 +17,7 @@ static constexpr int32_t CVAR_RUPEE_DASH_INTERVAL_DEFAULT = 5;
 #define CVAR_RUPEE_DASH_INTERVAL_TIME \
     CVarGetInteger(CVAR_RUPEE_DASH_INTERVAL_NAME, CVAR_RUPEE_DASH_INTERVAL_DEFAULT) * 20
 
-void UpdateRupeeDash() {
+static void UpdateRupeeDash() {
     // Initialize Timer
     static uint16_t rupeeDashTimer = 0;
 
@@ -36,7 +36,7 @@ void UpdateRupeeDash() {
     }
 }
 
-void RegisterRupeeDash() {
+static void RegisterRupeeDash() {
     COND_HOOK(OnPlayerUpdate, CVAR_RUPEE_DASH_VALUE, UpdateRupeeDash);
 }
 

soh/soh/Enhancements/ExtraModes/ShadowTag.cpp

@@ -16,7 +16,7 @@ static constexpr s8 ROOM_GREEN_POE = 16;
 static constexpr s8 ROOM_BLUE_POE = 13;
 static constexpr s8 ROOM_RED_POE = 12;
 
-void OnPlayerUpdateShadowTag() {
+static void OnPlayerUpdateShadowTag() {
     if (gPlayState->sceneNum == SCENE_FOREST_TEMPLE) {
         switch (gPlayState->roomCtx.curRoom.num) {
             case ROOM_GREEN_POE:
@@ -36,12 +36,12 @@ void OnPlayerUpdateShadowTag() {
     }
 }
 
-void ResetShadowTagSpawnTimer() {
+static void ResetShadowTagSpawnTimer() {
     shouldSpawn = true;
     delayTimer = 60;
 }
 
-void RegisterShadowTag() {
+static void RegisterShadowTag() {
     COND_HOOK(OnPlayerUpdate, CVAR_SHADOW_TAG_VALUE, OnPlayerUpdateShadowTag);
     COND_HOOK(OnSceneSpawnActors, true, []() { ResetShadowTagSpawnTimer(); });
     COND_HOOK(OnSceneInit, true, [](int16_t) { ResetShadowTagSpawnTimer(); });

soh/soh/Enhancements/Fixes/BrokenGiantsKnife.cpp

@@ -13,7 +13,7 @@ static constexpr int32_t CVAR_BGS_FIX_DEFAULT = 0;
 #define CVAR_BGS_FIX_NAME CVAR_ENHANCEMENT("FixBrokenGiantsKnife")
 #define CVAR_BGS_FIX_VALUE CVarGetInteger(CVAR_BGS_FIX_NAME, CVAR_BGS_FIX_DEFAULT)
 
-void OnReceiveBrokenGiantsKnife(GetItemEntry itemEntry) {
+static void OnReceiveBrokenGiantsKnife(GetItemEntry itemEntry) {
     if (itemEntry.itemId != ITEM_SWORD_BGS) {
         return;
     }
@@ -39,7 +39,7 @@ void OnReceiveBrokenGiantsKnife(GetItemEntry itemEntry) {
     }
 }
 
-void RegisterBrokenGiantsKnifeFix() {
+static void RegisterBrokenGiantsKnifeFix() {
     // If enhancement is off, flag should be handled exclusively by vanilla behaviour
     COND_HOOK(OnItemReceive, CVAR_BGS_FIX_VALUE || IS_RANDO, OnReceiveBrokenGiantsKnife);
 }

soh/soh/Enhancements/Fixes/DekuNutUpgradeFix.cpp

@@ -13,7 +13,7 @@ static constexpr int32_t CVAR_NUT_UPGRADE_FIX_DEFAULT = 0;
 #define CVAR_NUT_UPGRADE_FIX_NAME CVAR_ENHANCEMENT("DekuNutUpgradeFix")
 #define CVAR_NUT_UPGRADE_FIX_VALUE CVarGetInteger(CVAR_NUT_UPGRADE_FIX_NAME, CVAR_NUT_UPGRADE_FIX_DEFAULT)
 
-void DekuNutUpgradeFixAtForestStage(bool* should) {
+static void DekuNutUpgradeFixAtForestStage(bool* should) {
     // This check is needed because of an intentional fallthrough at the source
     if (Player_GetMask(gPlayState) == PLAYER_MASK_SKULL) {
         return;
@@ -30,11 +30,11 @@ void DekuNutUpgradeFixAtForestStage(bool* should) {
     }
 }
 
-void DekuNutUpgradeSetByPoachersSaw(bool* should) {
+static void DekuNutUpgradeSetByPoachersSaw(bool* should) {
     *should = false;
 }
 
-void RegisterDekuNutUpgradeFix() {
+static void RegisterDekuNutUpgradeFix() {
     COND_VB_SHOULD(VB_POACHERS_SAW_SET_DEKU_NUT_UPGRADE_FLAG, CVAR_NUT_UPGRADE_FIX_VALUE || IS_RANDO,
                    { DekuNutUpgradeSetByPoachersSaw(should); });
     COND_VB_SHOULD(VB_DEKU_SCRUBS_REACT_TO_MASK_OF_TRUTH, CVAR_NUT_UPGRADE_FIX_VALUE && !IS_RANDO,

soh/soh/Enhancements/Fixes/DirtPathFix.cpp

@@ -1,13 +1,14 @@
 #include "soh/Enhancements/game-interactor/GameInteractor_Hooks.h"
 #include "soh/Enhancements/enhancementTypes.h"
-#include "soh/Enhancements/mods.h"
 #include "soh/ShipInit.hpp"
 
+extern "C" PlayState* gPlayState;
+
 static constexpr ZFightingFixType CVAR_DIRT_PATH_DEFAULT = ZFIGHT_FIX_DISABLED;
 #define CVAR_DIRT_PATH_NAME CVAR_ENHANCEMENT("SceneSpecificDirtPathFix")
 #define CVAR_DIRT_PATH_VALUE CVarGetInteger(CVAR_DIRT_PATH_NAME, CVAR_DIRT_PATH_DEFAULT)
 
-void DirtPathFix_UpdateZFightingMode(int32_t sceneNum) {
+static void DirtPathFix_UpdateZFightingMode(int32_t sceneNum) {
     switch (sceneNum) {
         case SCENE_HYRULE_FIELD:
         case SCENE_KOKIRI_FOREST:
@@ -20,6 +21,10 @@ void DirtPathFix_UpdateZFightingMode(int32_t sceneNum) {
 }
 
 static void RegisterDirtPathFix() {
+    if (gPlayState != NULL) {
+        DirtPathFix_UpdateZFightingMode(gPlayState->sceneNum);
+    }
+
     COND_HOOK(OnTransitionEnd, CVAR_DIRT_PATH_VALUE, DirtPathFix_UpdateZFightingMode);
 }
 

soh/soh/Enhancements/Graphics/ToTMedallions.cpp

@@ -1,5 +1,4 @@
 #include "soh/Enhancements/game-interactor/GameInteractor_Hooks.h"
-#include "soh/Enhancements/mods.h"
 #include "soh/ShipInit.hpp"
 
 extern "C" {
@@ -102,14 +101,6 @@ static void ResetToTMedallions() {
     endGrayscale.RevertPatch();
 }
 
-void UpdateToTMedallions() {
-    if (CVAR_TOT_MEDALLION_COLORS_VALUE) {
-        PatchToTMedallions();
-    } else {
-        ResetToTMedallions();
-    }
-}
-
 static void CheckTempleOfTime(int16_t sceneNum) {
     if (sceneNum != SCENE_TEMPLE_OF_TIME) {
         return;
@@ -118,6 +109,12 @@ static void CheckTempleOfTime(int16_t sceneNum) {
 }
 
 static void RegisterToTMedallions() {
+    if (CVAR_TOT_MEDALLION_COLORS_VALUE) {
+        PatchToTMedallions();
+    } else {
+        ResetToTMedallions();
+    }
+
     COND_HOOK(OnItemReceive, CVAR_TOT_MEDALLION_COLORS_VALUE, [](GetItemEntry) {
         if (gPlayState) {
             CheckTempleOfTime(gPlayState->sceneNum);

soh/soh/Enhancements/QoL/DaytimeGS.cpp

@@ -21,7 +21,7 @@ struct DayTimeGoldSkulltulas {
 
 using DayTimeGoldSkulltulasList = std::vector<DayTimeGoldSkulltulas>;
 
-void OnSpawnNighttimeGoldSkulltula() {
+static void OnSpawnNighttimeGoldSkulltula() {
     // Gold Skulltulas that are not part of the scene actor list during the day
     // Actor values copied from the night time scene actor list
     static const DayTimeGoldSkulltulasList dayTimeGoldSkulltulas = {
@@ -62,7 +62,7 @@ void OnSpawnNighttimeGoldSkulltula() {
     }
 }
 
-void RegisterDaytimeGoldSkultullas() {
+static void RegisterDaytimeGoldSkultullas() {
     COND_HOOK(OnSceneSpawnActors, CVAR_DAYTIME_GS_VALUE, OnSpawnNighttimeGoldSkulltula);
 }
 

soh/soh/Enhancements/QoL/OpenAllHours.cpp

@@ -24,12 +24,10 @@ static constexpr int32_t DOOR_NIGHT_KAK_POTION_SHOP = 7822;
 static constexpr int32_t DOOR_NIGHT_KAK_POTION_SHOP_BACK = 8846;
 
 static void OpenAllHours(void* refActor) {
-    Actor* actor = static_cast<Actor*>(refActor);
-    if (actor->id != ACTOR_EN_DOOR) {
-        return;
-    }
+    EnDoor* enDoor = static_cast<EnDoor*>(refActor);
+    s16* params = &enDoor->actor.params;
 
-    switch (actor->params) {
+    switch (*params) {
         case DOOR_DAY_CHEST_GAME:
         case DOOR_DAY_BOMBCHU_SHOP:
         case DOOR_NIGHT_POTION_SHOP:
@@ -40,8 +38,7 @@ static void OpenAllHours(void* refActor) {
         case DOOR_NIGHT_KAK_BAZAAR:
         case DOOR_NIGHT_KAK_POTION_SHOP:
         case DOOR_NIGHT_KAK_POTION_SHOP_BACK: {
-            actor->params = (actor->params & 0xFC00) | (DOOR_SCENEEXIT << 7) | 0x3F;
-            EnDoor* enDoor = static_cast<EnDoor*>(refActor);
+            *params = (*params & 0xFC00) | (DOOR_SCENEEXIT << 7) | 0x3F;
             EnDoor_SetupType(enDoor, gPlayState);
             break;
         }
@@ -54,7 +51,7 @@ static void RegisterOpenAllHours() {
     bool overworldDoorsOpen =
         !IS_RANDO || !OTRGlobals::Instance->gRandomizer->GetRandoSettingValue(RSK_LOCK_OVERWORLD_DOORS);
 
-    COND_HOOK(OnActorInit, CVAR_OPEN_ALL_HOURS_VALUE && overworldDoorsOpen, OpenAllHours);
+    COND_ID_HOOK(OnActorInit, ACTOR_EN_DOOR, CVAR_OPEN_ALL_HOURS_VALUE && overworldDoorsOpen, OpenAllHours);
 }
 
 static RegisterShipInitFunc initFunc(RegisterOpenAllHours, { CVAR_OPEN_ALL_HOURS_NAME, "IS_RANDO" });

soh/soh/Enhancements/mods.h

@@ -7,10 +7,6 @@
 extern "C" {
 #endif
 
-void DirtPathFix_UpdateZFightingMode(int32_t sceneNum);
-void UpdateMirrorModeState(int32_t sceneNum);
-void UpdateToTMedallions();
-void UpdatePermanentHeartLossState();
 void UpdateHyperEnemiesState();
 void UpdateHyperBossesState();
 void InitMods();

soh/soh/SohGui/SohMenuEnhancements.cpp

@@ -601,7 +601,6 @@ void SohMenu::AddMenuEnhancements() {
     AddWidget(path, "Color Temple of Time's Medallions", WIDGET_CVAR_CHECKBOX)
         .CVar(CVAR_ENHANCEMENT("ToTMedallionsColors"))
         .RaceDisable(false)
-        .Callback([](WidgetInfo& info) { UpdateToTMedallions(); })
         .Options(CheckboxOptions().Tooltip(
             "When Medallions are collected, the Medallion imprints around the Master Sword Pedestal in the Temple "
             "of Time will become colored-in."));
@@ -1078,11 +1077,6 @@ void SohMenu::AddMenuEnhancements() {
     AddWidget(path, "Fix Vanishing Paths", WIDGET_CVAR_COMBOBOX)
         .CVar(CVAR_ENHANCEMENT("SceneSpecificDirtPathFix"))
         .RaceDisable(false)
-        .Callback([](WidgetInfo& info) {
-            if (gPlayState != NULL) {
-                DirtPathFix_UpdateZFightingMode(gPlayState->sceneNum);
-            }
-        })
         .Options(
             ComboboxOptions()
                 .ComboMap(zFightingOptions)
@@ -1179,7 +1173,6 @@ void SohMenu::AddMenuEnhancements() {
     AddWidget(path, "Health", WIDGET_SEPARATOR_TEXT);
     AddWidget(path, "Permanent Heart Loss", WIDGET_CVAR_CHECKBOX)
         .CVar(CVAR_ENHANCEMENT("PermanentHeartLoss"))
-        .Callback([](WidgetInfo& info) { UpdatePermanentHeartLossState(); })
         .Options(CheckboxOptions().Tooltip(
             "When you lose 4 quarters of a heart you will permanently lose that Heart Container.\n\n"
             "Disabling this after the fact will restore your Heart Containers."));
@@ -1522,11 +1515,6 @@ void SohMenu::AddMenuEnhancements() {
 
     AddWidget(path, "Mirrored World", WIDGET_CVAR_COMBOBOX)
         .CVar(CVAR_ENHANCEMENT("MirroredWorldMode"))
-        .Callback([](WidgetInfo& info) {
-            if (gPlayState != NULL) {
-                UpdateMirrorModeState(gPlayState->sceneNum);
-            }
-        })
         .Options(
             ComboboxOptions()
                 .DefaultIndex(MIRRORED_WORLD_OFF)