fix(terrain helper): fix terrain helper hook

features/Terrain Helper/Shaders/Features/TerrainHelper.ini

@@ -1,5 +1,5 @@
 [Info]
-Version = 1-0-0
+Version = 1-0-1
 
 [Nexus]
 nexusmodid = 143149

src/Features/TerrainHelper.cpp

@@ -1,8 +1,10 @@
 #include "TerrainHelper.h"
 
 #include "Globals.h"
+#include "Hooks.h"
 #include "ShaderCache.h"
 #include "State.h"
+#include "TruePBR.h"
 
 void TerrainHelper::DataLoaded()
 {
@@ -194,44 +196,25 @@ void TerrainHelper::BSLightingShader_SetupMaterial(RE::BSLightingShaderMaterialB
 	}
 }
 
-struct TH_TESObjectLAND_SetupMaterial
-{
-	static bool thunk(RE::TESObjectLAND* land)
-	{
-		bool result = func(land);
-
-		// TruePBR sets flag 8 on land cells it processes as PBR; skip TerrainHelper for those.
-		if (!land->data.flags.any(static_cast<RE::OBJ_LAND::Flag>(8))) {
-			auto& terrainHelper = globals::features::terrainHelper;
-			if (result && terrainHelper.loaded) {
-				terrainHelper.TESObjectLAND_SetupMaterial(land);
-			}
-		}
-
-		return result;
-	}
-	static inline REL::Relocation<decltype(thunk)> func;
-};
-
-struct TH_BSLightingShader_SetupMaterial
-{
-	static void thunk(RE::BSLightingShader* shader, RE::BSLightingShaderMaterialBase const* material)
-	{
-		func(shader, material);
-
-		auto& terrainHelper = globals::features::terrainHelper;
-		if (terrainHelper.loaded) {
-			terrainHelper.BSLightingShader_SetupMaterial(material);
-		}
-	}
-	static inline REL::Relocation<decltype(thunk)> func;
-};
-
 void TerrainHelper::PostPostLoad()
 {
-	logger::info("[Terrain Helper] Hooking TESObjectLAND");
-	stl::detour_thunk<TH_TESObjectLAND_SetupMaterial>(REL::RelocationID(18368, 18791));
-
-	logger::info("[Terrain Helper] Hooking BSLightingShader::SetupMaterial");
-	stl::write_vfunc<0x4, TH_BSLightingShader_SetupMaterial>(RE::VTABLE_BSLightingShader[0]);
+	logger::info("[Terrain Helper] Registering material hook post callbacks");
+	// TerrainHelper augments vanilla land setup; it never claims, so PBR (registered earlier) wins
+	// when both flag the cell.
+	Hooks::MaterialHooks::TESObjectLANDPost().Register(
+		+[](RE::TESObjectLAND* land, bool vanillaResult) -> bool {
+			if (!vanillaResult) {
+				return false;
+			}
+			// TruePBR tags land cells it processes; skip TerrainHelper for those.
+			if (land->data.flags.any(kPBRProcessedLandFlag)) {
+				return false;
+			}
+			globals::features::terrainHelper.TESObjectLAND_SetupMaterial(land);
+			return false;
+		});
+	Hooks::MaterialHooks::BSLightingShaderPost().Register(
+		+[](RE::BSLightingShader*, RE::BSLightingShaderMaterialBase const* material) {
+			globals::features::terrainHelper.BSLightingShader_SetupMaterial(material);
+		});
 }

src/Hooks.cpp

@@ -893,6 +893,57 @@ namespace Hooks
 		static inline REL::Relocation<decltype(thunk)> func;
 	};
 
+	namespace MaterialHooks
+	{
+		HookChain<TESObjectLANDPostCallback>& TESObjectLANDPost()
+		{
+			static HookChain<TESObjectLANDPostCallback> chain;
+			return chain;
+		}
+		HookChain<BSLightingShaderInterceptor>& BSLightingShaderInterceptors()
+		{
+			static HookChain<BSLightingShaderInterceptor> chain;
+			return chain;
+		}
+		HookChain<BSLightingShaderPostCallback>& BSLightingShaderPost()
+		{
+			static HookChain<BSLightingShaderPostCallback> chain;
+			return chain;
+		}
+	}
+
+	struct TESObjectLAND_SetupMaterial
+	{
+		static bool thunk(RE::TESObjectLAND* land)
+		{
+			bool vanillaResult = func(land);
+			for (auto cb : MaterialHooks::TESObjectLANDPost().callbacks) {
+				if (cb(land, vanillaResult)) {
+					return true;
+				}
+			}
+			return vanillaResult;
+		}
+		static inline REL::Relocation<decltype(thunk)> func;
+	};
+
+	struct BSLightingShader_SetupMaterial
+	{
+		static void thunk(RE::BSLightingShader* shader, RE::BSLightingShaderMaterialBase const* material)
+		{
+			for (auto cb : MaterialHooks::BSLightingShaderInterceptors().callbacks) {
+				if (cb(shader, material)) {
+					return;
+				}
+			}
+			func(shader, material);
+			for (auto cb : MaterialHooks::BSLightingShaderPost().callbacks) {
+				cb(shader, material);
+			}
+		}
+		static inline REL::Relocation<decltype(thunk)> func;
+	};
+
 	/**
 	 * @brief Installs hooks, detours, and memory patches for graphics, input, and rendering subsystems.
 	 *
@@ -905,6 +956,12 @@ namespace Hooks
 			stl::detour_thunk<BSImageSpace_Init_IBLF>(REL::RelocationID(100480, 107198));
 		}
 
+		logger::info("Hooking TESObjectLAND::SetupMaterial");
+		stl::detour_thunk<TESObjectLAND_SetupMaterial>(REL::RelocationID(18368, 18791));
+
+		logger::info("Hooking BSLightingShader::SetupMaterial");
+		stl::write_vfunc<0x4, BSLightingShader_SetupMaterial>(RE::VTABLE_BSLightingShader[0]);
+
 		// This input hook also drives per-frame Reflex update (see BSInputDeviceManager_PollInputDevices::thunk).
 		logger::info("Hooking BSInputDeviceManager::PollInputDevices");
 		stl::write_thunk_call<BSInputDeviceManager_PollInputDevices>(REL::RelocationID(67315, 68617).address() + REL::Relocate(0x7B, 0x7B, 0x81));

src/Hooks.h

@@ -19,6 +19,52 @@ namespace Hooks
 		static inline REL::Relocation<decltype(thunk)> func;
 	};
 
+	/**
+	 * @brief Append-only registration list of function pointers for a single hook dispatch slot.
+	 *
+	 * Registration happens during `PostPostLoad` (single-threaded); dispatch is render-thread
+	 * read-only, so no synchronization is needed. Function pointers (not `std::function`) keep the
+	 * hot path free of allocation and type erasure.
+	 */
+	template <typename Fn>
+	struct HookChain
+	{
+		std::vector<Fn> callbacks;
+		void Register(Fn cb) { callbacks.push_back(cb); }
+	};
+
+	/**
+	 * @brief Multi-subscriber dispatchers for engine functions that more than one feature wants to wrap.
+	 *
+	 * Both `TESObjectLAND::SetupMaterial` and `BSLightingShader::SetupMaterial` have multiple
+	 * subscribers (currently TruePBR and TerrainHelper). Installing a `detour_thunk` per feature
+	 * on the same address corrupts the first trampoline; installing competing `write_vfunc<0x4>`
+	 * writes is last-writer-wins. Hooks.cpp owns a single install of each and dispatches through
+	 * these slots so features stay decoupled from each other and from Hooks.cpp.
+	 *
+	 * Semantics (intentionally different between the two functions to preserve historical behavior):
+	 *  - `TESObjectLAND::SetupMaterial`: vanilla always runs first; then post callbacks run in
+	 *    registration order. A callback returning `true` claims the result (the hook returns `true`
+	 *    and any later post callbacks are skipped). TruePBR uses claim to tag PBR land cells (see
+	 *    `kPBRProcessedLandFlag` in `TruePBR.h`); TerrainHelper observes only and never claims.
+	 *  - `BSLightingShader::SetupMaterial`: interceptors run before vanilla. Returning `true`
+	 *    short-circuits — vanilla is skipped, no post callback runs. Post callbacks then run if
+	 *    no interceptor claimed. TruePBR is an interceptor (replaces vanilla for PBR materials);
+	 *    TerrainHelper is post.
+	 */
+	namespace MaterialHooks
+	{
+		// Return `true` to claim the result; the hook returns `true` and the rest of the chain is skipped.
+		using TESObjectLANDPostCallback = bool (*)(RE::TESObjectLAND* land, bool vanillaResult);
+		// Return `true` to skip vanilla (and the post chain).
+		using BSLightingShaderInterceptor = bool (*)(RE::BSLightingShader* shader, RE::BSLightingShaderMaterialBase const* material);
+		using BSLightingShaderPostCallback = void (*)(RE::BSLightingShader* shader, RE::BSLightingShaderMaterialBase const* material);
+
+		HookChain<TESObjectLANDPostCallback>& TESObjectLANDPost();
+		HookChain<BSLightingShaderInterceptor>& BSLightingShaderInterceptors();
+		HookChain<BSLightingShaderPostCallback>& BSLightingShaderPost();
+	}
+
 	void Install();
 	void InstallEarlyHooks();
 }

src/TruePBR.cpp

@@ -1174,7 +1174,7 @@ bool TruePBR::TESObjectLAND_SetupMaterial(RE::TESObjectLAND* land)
 	auto memoryManager = RE::MemoryManager::GetSingleton();
 
 	if (land->loadedData != nullptr && land->loadedData->mesh[0] != nullptr) {
-		land->data.flags.set(static_cast<RE::OBJ_LAND::Flag>(8));
+		land->data.flags.set(kPBRProcessedLandFlag);
 		for (uint32_t quadIndex = 0; quadIndex < 4; ++quadIndex) {
 			auto shaderProperty = static_cast<RE::BSLightingShaderProperty*>(memoryManager->Allocate(REL::Module::IsVR() ? 0x178 : sizeof(RE::BSLightingShaderProperty), 0, false));
 			shaderProperty->Ctor();
@@ -1442,34 +1442,6 @@ struct BSLightingShaderProperty_OnLoadTextureSet
 	static inline REL::Relocation<decltype(thunk)> func;
 };
 
-struct PBR_TESObjectLAND_SetupMaterial
-{
-	static bool thunk(RE::TESObjectLAND* land)
-	{
-		bool vanillaResult = func(land);
-
-		if (globals::features::truePBR.TESObjectLAND_SetupMaterial(land)) {
-			return true;
-		}
-
-		return vanillaResult;
-	}
-	static inline REL::Relocation<decltype(thunk)> func;
-};
-
-struct PBR_BSLightingShader_SetupMaterial
-{
-	static void thunk(RE::BSLightingShader* shader, RE::BSLightingShaderMaterialBase const* material)
-	{
-		if (globals::features::truePBR.BSLightingShader_SetupMaterial(shader, material)) {
-			return;
-		}
-
-		func(shader, material);
-	}
-	static inline REL::Relocation<decltype(thunk)> func;
-};
-
 void TruePBR::PostPostLoad()
 {
 	logger::info("[TruePBR] Hooking BGSTextureSet");
@@ -1505,11 +1477,17 @@ void TruePBR::PostPostLoad()
 	logger::info("[TruePBR] Hooking TESObjectSTAT");
 	stl::write_vfunc<0x4A, TESBoundObject_Clone3D>(RE::VTABLE_TESObjectSTAT[0]);
 
-	logger::info("[TruePBR] Hooking TESObjectLAND");
-	stl::detour_thunk<PBR_TESObjectLAND_SetupMaterial>(REL::RelocationID(18368, 18791));
-
-	logger::info("[TruePBR] Hooking BSLightingShader::SetupMaterial");
-	stl::write_vfunc<0x4, PBR_BSLightingShader_SetupMaterial>(RE::VTABLE_BSLightingShader[0]);
+	logger::info("[TruePBR] Registering material hook callbacks");
+	// TruePBR claims LAND cells whose textures are PBR (after vanilla has run).
+	Hooks::MaterialHooks::TESObjectLANDPost().Register(
+		+[](RE::TESObjectLAND* land, bool) -> bool {
+			return globals::features::truePBR.TESObjectLAND_SetupMaterial(land);
+		});
+	// TruePBR intercepts BSLightingShader::SetupMaterial — replaces vanilla setup for PBR materials.
+	Hooks::MaterialHooks::BSLightingShaderInterceptors().Register(
+		+[](RE::BSLightingShader* shader, RE::BSLightingShaderMaterialBase const* material) -> bool {
+			return globals::features::truePBR.BSLightingShader_SetupMaterial(shader, material);
+		});
 }
 
 void TruePBR::DataLoaded()

src/TruePBR.h

@@ -11,6 +11,10 @@ struct GlintParameters
 	float densityRandomization = 2.f;
 };
 
+// TruePBR tags land cells it processes by setting this flag on TESObjectLAND::data.flags.
+// TerrainHelper reads it to skip cells that PBR has already claimed.
+inline constexpr auto kPBRProcessedLandFlag = static_cast<RE::OBJ_LAND::Flag>(8);
+
 struct TruePBR : Feature
 {
 public: