fix(terrain-helper): restore parallax vs TruePBR

[SkrubbySkrubInAShrub]

Summary

• TerrainHelper's TESObjectLAND hook was installed in PostPostLoad(), which
  runs after TruePBR (feature index 0) had already installed its own hook. With
  stl::detour_thunk, last-installed is outermost, so the call chain was
  engine → TH_thunk → PBR_thunk → vanilla. TH called func(land) first,
  which let PBR replace the geometry's shaderProperty with a new
  BSLightingShaderMaterialPBRLandscape and set OBJ_LAND::Flag(8). TH's
  flag-8 guard then skipped the cell, leaving extendedSlots empty and
  parallax never activating — even on vanilla land — whenever a mod supplies
  the DefaultPBRLand EDID (which causes TruePBR to claim all default-textured
  quads as PBR).
• Fix: move TH's TESObjectLAND hook install to Load(), which runs at
  DLL-load time before any feature's PostPostLoad(). TH is now the inner
  hook (engine → PBR_thunk → TH_thunk → vanilla). TH reads the vanilla
  material's hashKey before TruePBR replaces the shaderProperty, stores
  parallax data for that key, and returns. TruePBR then runs and may replace
  the material with a PBR one. For PBR land, BSLightingShader::SetupMaterial
  is called with the PBR material's hashKey, which doesn't match TH's stored
  vanilla key → TH does nothing. For vanilla land, the key matches → parallax
  activates correctly.
• Remove the now-dead OBJ_LAND::Flag(8) guard from the hook thunk — flag 8
  is never set at the point TH runs in the new order.

Summary by CodeRabbit

• Bug Fixes

  • Fixed terrain material processing so all terrain types—including those previously skipped by special flags—consistently receive proper material updates; ensured initialization/hook ordering is preserved so material setup runs at the correct time.

• Chores

  • Bumped Terrain Helper shader configuration version to 1-0-1.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: d25c26c2-54ea-4169-9781-b15567fcf8c7

📥 Commits

Reviewing files that changed from the base of the PR and between 10c8a6a and e9a50b8.

📒 Files selected for processing (1)

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

---

📝 Walkthrough

Walkthrough

TerrainHelper reorders hook installation by adding a new Load() lifecycle method to install the TESObjectLAND detour earlier (moved from PostPostLoad). PostPostLoad now only installs the BSLightingShader::SetupMaterial vfunc hook. The TESObjectLAND thunk now invokes TerrainHelper::TESObjectLAND_SetupMaterial when the original call succeeds and terrainHelper.loaded is true (removing the TruePBR flag check).

Changes

TerrainHelper Hook Installation and Timing

Layer / File(s)	Summary
Load() lifecycle method and hook installation timing src/Features/TerrainHelper.h, src/Features/TerrainHelper.cpp, features/Terrain Helper/Shaders/Features/TerrainHelper.ini	TerrainHelper::Load() is declared and implemented to install the TESObjectLAND_SetupMaterial detour earlier; PostPostLoad() no longer installs the TESObjectLAND hook and retains only the BSLightingShader::SetupMaterial vfunc hook. INI Version bumped to 1-0-1.
TESObjectLAND thunk terrain processing condition src/Features/TerrainHelper.cpp	TH_TESObjectLAND_SetupMaterial::thunk now calls TerrainHelper::TESObjectLAND_SetupMaterial(land) when the original call succeeds and globals::features::terrainHelper.loaded is true, removing the prior TruePBR flag check.

Sequence Diagram(s)

sequenceDiagram
  participant TerrainHelper
  participant HookInstaller
  participant BSLightingShader
  TerrainHelper->>HookInstaller: TerrainHelper::Load() installs TESObjectLAND_SetupMaterial detour
  TerrainHelper->>BSLightingShader: PostPostLoad installs BSLightingShader::SetupMaterial vfunc hook

Loading

sequenceDiagram
  participant TH_TESObjectLAND_SetupMaterial
  participant TerrainHelper
  TH_TESObjectLAND_SetupMaterial->>TerrainHelper: call TerrainHelper::TESObjectLAND_SetupMaterial(land) when result==true && terrainHelper.loaded

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• community-shaders/skyrim-community-shaders#1176: Both PRs modify TerrainHelper’s TESObjectLAND_SetupMaterial hook behavior by changing/adding gating conditions around when TerrainHelper::TESObjectLAND_SetupMaterial(land) runs.
• community-shaders/skyrim-community-shaders#1545: Both PRs change src/Features/TerrainHelper.cpp behavior so terrain material setup/detection runs correctly under TRUE_PBR; they touch related flags and ordering.
• community-shaders/skyrim-community-shaders#1099: Both PRs modify the Hooks::TESObjectLAND_SetupMaterial::thunk / TerrainHelper::TESObjectLAND_SetupMaterial execution path and invocation ordering.

Suggested reviewers

• alandtse
• doodlum

Poem

🐰 I hopped in code at break of day,
Installed a hook to clear the way,
Load wakes the land before the rest,
No PBR skip—let helpers test,
A tiny hop, the shaders play.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix(terrain-helper): restore parallax vs TruePBR' accurately describes the main change: restoring parallax functionality in the context of TruePBR interactions by adjusting hook ordering.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[github-actions]

No actionable suggestions for changed features.

[coderabbitai]

🧹 Nitpick comments (1)

src/Features/TerrainHelper.cpp (1)

227-233: ⚡ Quick win

Rework: TerrainHelper::Load TESObjectLAND hook VR gating is likely consistent with TruePBR

• TerrainHelper::Load() unconditionally installs stl::detour_thunk<TH_TESObjectLAND_SetupMaterial>(REL::RelocationID(18368, 18791)) (src/Features/TerrainHelper.cpp:227-233), and the feature is included in VR via TerrainHelper::SupportsVR() == true + VR feature filtering (src/Features/TerrainHelper.h:43, src/Feature.cpp:260).
• The same TESObjectLAND detour target (REL::RelocationID(18368, 18791)) is already used unconditionally by TruePBR::TESObjectLAND_SetupMaterial (src/TruePBR.cpp:1508-1510), so the relocation pair is already treated as VR-compatible by the codebase.
• Optional: add a short comment in TerrainHelper::Load() pointing out this intentional alignment with TruePBR (only add REL::Module::IsVR() gating if VR offsets are known to diverge for this callsite).

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/Features/TerrainHelper.cpp` around lines 227 - 233, TerrainHelper::Load
currently installs the TESObjectLAND detour unconditionally using
stl::detour_thunk<TH_TESObjectLAND_SetupMaterial>(REL::RelocationID(18368,
18791)), which mirrors the same relocation used by
TruePBR::TESObjectLAND_SetupMaterial; update TerrainHelper::Load to keep the
existing unconditional detour but add a short clarifying comment that this is
intentionally aligned with TruePBR's detour (same REL::RelocationID(18368,
18791)) and therefore does not require additional REL::Module::IsVR() gating
here unless VR offsets diverge in the future.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/Features/TerrainHelper.cpp`:
- Around line 227-233: TerrainHelper::Load currently installs the TESObjectLAND
detour unconditionally using
stl::detour_thunk<TH_TESObjectLAND_SetupMaterial>(REL::RelocationID(18368,
18791)), which mirrors the same relocation used by
TruePBR::TESObjectLAND_SetupMaterial; update TerrainHelper::Load to keep the
existing unconditional detour but add a short clarifying comment that this is
intentionally aligned with TruePBR's detour (same REL::RelocationID(18368,
18791)) and therefore does not require additional REL::Module::IsVR() gating
here unless VR offsets diverge in the future.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 32c1d4b2-4a65-49af-9ce5-8d97b9a60b47

📥 Commits

Reviewing files that changed from the base of the PR and between 377da5f and 087f0f6.

📒 Files selected for processing (2)

• src/Features/TerrainHelper.cpp
• src/Features/TerrainHelper.h

[github-actions]

✅ A pre-release build is available for this PR:
Download