fix(unified water): reduce transition flicker

[FIocker]

Small follow-up to the recent Unified Water LOD ownership change.

On my setup, LOD water no longer disappeared, but after going into an interior and back out some water surfaces could flicker heavily until the area was reloaded. This keeps the current vanilla LOD ownership path, but cleans up some stale water state around terrain block transitions.

Changes:

• clear detached water children through TESWaterSystem instead of only detaching the scene children
• run the duplicate water cleanup after the AddLODWater / RemoveLODWater handoff
• refresh BSWaterShaderProperty::plane during water setup so stale transition data does not survive on one of the overlapping surfaces

Testing:

• built ALL-VS2022 locally
• shader tests passed, 166 assertions
• tested an equivalent v1.5.2 build in game by repeating exterior -> interior -> exterior transitions; the water stayed visible and the transition flicker stopped

Summary by CodeRabbit

• Bug Fixes
  • Resolved water surface flicker during terrain transitions and after LOD changes by re-stabilizing surface calculations each draw.
  • Reduced visual glitches and duplicated water artifacts by cleaning up redundant water instances, ensuring proper detachment of water nodes, and tightening flowmap/displacement setup for more consistent rendering.

[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: ad3c81e9-5a74-4111-b032-51a3b5a75581

📥 Commits

Reviewing files that changed from the base of the PR and between 3333c5e and 8d36b63.

📒 Files selected for processing (1)

• src/Features/UnifiedWater.cpp

🚧 Files skipped from review as they are similar to previous changes (1)

• src/Features/UnifiedWater.cpp

---

📝 Walkthrough

Walkthrough

Three targeted fixes in UnifiedWater.cpp: deduplicate water-system objects during BuildWaterForBlock, refactor terrain detach child cleanup with null guards, and update BSWaterShader_SetupGeometry to refresh the shader plane per draw and tighten flowmap/displacement guards.

Changes

Unified Water System Fixes

Layer / File(s)	Summary
Water deduplication in build src/Features/UnifiedWater.cpp	BuildWaterForBlock calls RemoveDuplicateWaterSystemObjects after LOD water shape attachment and before the water-parent detach/reattach sequence.
Terrain block detach and cleanup src/Features/UnifiedWater.cpp	BGSTerrainBlock_Detach::thunk replaces manual child detachment with ClearWaterNodeChildren, adds guards around gWaterLOD before detaching occurrences, and clears block->waterAttached only when block is non-null.
Water shader plane re-stabilization src/Features/UnifiedWater.cpp	BSWaterShader_SetupGeometry::thunk updates BSWaterShaderProperty plane normal and constant each draw when pass and pass->geometry are present, and tightens flowmap/displacement setup to require uw.flowmap && pass && pass->geometry.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• community-shaders/skyrim-community-shaders#1763: Overlaps BuildWaterForBlock and water LOD object management changes.
• community-shaders/skyrim-community-shaders#1998: Related BSWaterShader_SetupGeometry plane stabilization edits.
• community-shaders/skyrim-community-shaders#2019: Touches BSWaterShader_SetupGeometry draw-path logic and LOD behavior related to shader stability.

Suggested reviewers

• alandtse
• Dlizzio
• SkrubbySkrubInAShrub

Poem

🐰 I hopped through code by moonlit streams,
I chased the duplicates from your dreams,
Cleared stray children, fixed the plane,
Now water shimmers without strain,
A rabbit's patch to mend your beams.

🚥 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): reduce transition flicker' directly and clearly describes the main objective of the PR, which is to fix Unified Water flicker after transitions.
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

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 OpenGrep (1.20.0)

OpenGrep fatal error (exit code 2): [00.12][ERROR]: Error: exception Unix_error: No such file or directory stat src/Features/UnifiedWater.cpp
Raised by primitive operation at UTmp.replace_named_pipe_by_regular_file_if_needed in file "libs/commons/UTmp.ml", line 145, characters 8-27
Called from Scan_CLI.replace_target_roots_by_regular_files_where_needed.(fun) in file "src/osemgrep/cli_scan/Scan_CLI.ml", lines 1086-1087, characters 19-65
Called from List_.fast_map in file "libs/commons/List_.ml", line 81, characters 17-20
Called fro

---

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]

Actionable comments posted: 1

🤖 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.

Inline comments:
In `@src/Features/UnifiedWater.cpp`:
- Around line 649-654: The code currently only clears block->waterAttached
inside the water-present branch, leaving stale state when block->water is null;
update the logic in the ClearWaterNodeChildren/DetachAllChildOccurrences section
so that block->waterAttached = false is executed unconditionally (i.e., move or
duplicate the assignment out of the if (water) branch) while keeping the
existing ClearWaterNodeChildren(water, globals::game::waterSystem) and
DetachAllChildOccurrences(*uw.gWaterLOD, water) calls intact and conditioned on
uw.gWaterLOD/water as before.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 58f724db-a1f3-4414-a470-0e9f8bcf87dc

📥 Commits

Reviewing files that changed from the base of the PR and between 8c9b18c and 0d991a7.

📒 Files selected for processing (1)

• src/Features/UnifiedWater.cpp

[github-actions]

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

[SkrubbySkrubInAShrub]

Not seeing any regressions, the obvious issues are still present. I guess it doesn't hurt.