fix(unified water): restore vanilla LOD ownership after attach

[Dlizzio]

Summary by CodeRabbit

• Bug Fixes
  • Fixed duplicated water meshes appearing during displacement updates
• Performance Improvements
  • Improved water culling and attachment for more efficient rendering
  • Streamlined water management to reduce overhead and improve stability
• Configuration
  • Shader feature version bumped to 1-0-2

[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: 19142146-ec38-4e17-a5b2-efe3528f6f2f

📥 Commits

Reviewing files that changed from the base of the PR and between 308cb4a and bed60be.

📒 Files selected for processing (1)

• features/Unified Water/Shaders/Features/UnifiedWater.ini

✅ Files skipped from review due to trivial changes (1)

• features/Unified Water/Shaders/Features/UnifiedWater.ini

---

📝 Walkthrough

Walkthrough

Refactors unified water: simplifies cell culling, centralizes terrain-block water build into BuildWaterForBlock, adds duplicate water detection/removal (quantized by position/scale), removes deferred worldspace-hook thunks, and drops shader runtime plane-property corrections.

Changes

Unified Water Refactoring

Layer / File(s)	Summary
Header Declarations and Interface Updates src/Features/UnifiedWater.h	Changes standard include from <atomic> to <vector>, removes nested relocation-thunk structs TES_SetWorldSpace and TES_DestroySkyCell, removes deferred culling atomic state members, and declares new BuildWaterForBlock(RE::BGSTerrainBlock*, RE::TESWaterSystem*) helper.
Simplified Cell Culling API src/Features/UnifiedWater.cpp	Simplifies ShouldCullAtCell by removing the "in grid" output parameter, reimplements logic to compute grid coordinates and query cell state, and rewrites CullWaterParentByGridCells to call the simplified helper.
Water Operation Helper Utilities src/Features/UnifiedWater.cpp	Adds relocation-wrapper helpers for vanilla LOD water add/remove behavior, recursive helpers for clearing water-node children and detaching child occurrences, and a child-node duplicate-detach utility.
Duplicate Water Detection Infrastructure src/Features/UnifiedWater.cpp	Introduces WaterPositionKey plus quantization and key-extraction helpers, and implements duplicate-water removal for TESWaterSystem by hashing shapes by quantized position/scale and removing duplicates.
Centralized Water Attachment Helper src/Features/UnifiedWater.cpp	Implements BuildWaterForBlock to consolidate water attachment: detach existing water, clone/attach cached shapes per LOD, set shader flowmap/normals flags, unwind vanilla LOD state, reattach under gWaterLOD, enable the water system, and return whether attachment occurred.
Terrain Block Attachment and Detachment Updates src/Features/UnifiedWater.cpp	Updates BGSTerrainBlock_Attach::thunk to delegate to BuildWaterForBlock and simplifies BGSTerrainBlock_Detach::thunk cleanup by detaching children and using DetachAllChildOccurrences on the water LOD root.
Worldspace Hook Removal and Cleanup src/Features/UnifiedWater.cpp	Removes TES_SetWorldSpace / TES_DestroySkyCell detour installation and thunk implementations in PostPostLoad, drops runtime shader plane-property correction logic, and calls duplicate-water removal in TESWaterSystem_UpdateDisplacementMeshPosition.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

• community-shaders/skyrim-community-shaders#1763: Overlaps UnifiedWater.cpp changes to water add/remove and build path.
• community-shaders/skyrim-community-shaders#2019: Introduced the deferred child-worldspace culling hooks that this PR removes.
• community-shaders/skyrim-community-shaders#1998: Related to runtime BSWaterShaderProperty plane validation/correction changes.

Suggested reviewers

• alandtse
• sicsix
• doodlum

Poem

🐰
With grids aligned and hooks untied,
I hop where water blocks collide,
Duplicates removed with care,
One mesh remains to glide and glare,
A rabbit cheers the clearer tide.

🚥 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(unified water): restore vanilla LOD ownership after attach' clearly summarizes the main change: restoring vanilla LOD water ownership after attachment. This directly relates to the primary refactoring in UnifiedWater.cpp where vanilla LOD water state is unwound/reattached via relocation wrappers and the gWaterLOD child relationships are managed.
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.

[github-actions]

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