fix(unified-water): LOD water cache mismatch

[davo0411]

Current code only calls SetCurrentWorldSpace when currentCache is null.

The problem scenario:

Player is in worldspace A → currentCache points to A's cache
Player loads into worldspace B
BGSTerrainBlock_Attach fires for worldspace B before TES_SetWorldSpace updates the cache
currentCache is NOT null (still points to A's cache), so SetCurrentWorldSpace is skipped
currentCache->GetInstructions() runs on A's cache with B's coordinates → returns nullptr or garbage
Falls back to vanilla LOD water

The fix ensures SetCurrentWorldSpace is always called, which has a fast early-return if already correct:
The fix just removes a micro-optimization. Worst case, it's slightly slower (one extra string comparison per terrain block attachment). Best case, it fixes the race condition probably causing the lod issue

Summary by CodeRabbit

• Bug Fixes
  • Improved reliability of water system caching by ensuring consistent initialization on each operation.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

📝 Walkthrough

Walkthrough

The GetInstructions method in WaterCache now unconditionally calls SetCurrentWorldSpace(worldSpace) on every invocation, rather than only when no cached data exists. Previously, the method would skip reinitialization if currentCache was already populated; it now attempts reinitialization regardless and returns nullptr only if that call fails.

Changes

Cohort / File(s)	Summary
WaterCache Control Flow src/Features/UnifiedWater/WaterCache.cpp	Modified GetInstructions to always attempt SetCurrentWorldSpace reinitialization instead of conditionally skipping when currentCache exists; maintains consistency in world space updates.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• build: fix unified water #1763: Modifies GetInstructions call sites and method signature regarding baseCell coordinates, directly related to this PR's control flow changes in GetInstructions.

Poem

🐰 A single line, but oh so wise,
SetWorldSpace now never skips or lies,
Each call ensures the waters flow,
With worlds aligned, both high and low. ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 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 (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: fixing a mismatch between LOD water cache and the current world space by always calling SetCurrentWorldSpace.

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

✨ Finishing touches

• 📝 Generate docstrings

---

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]

Using provided base ref: c29d8e2
Using base ref: c29d8e2
Base commit date: 2026-01-29T13:57:09+10:00 (Thursday, January 29, 2026 01:57 PM)
No actionable suggestions for changed features.

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@src/Features/UnifiedWater/WaterCache.cpp`:
- Around line 44-46: The error log dereferences worldSpace when
SetCurrentWorldSpace fails, which crashes if worldSpace is null; before calling
SetCurrentWorldSpace (and before logging) add a nullptr guard for worldSpace (or
compute a safe editorID string via a ternary like editorID = worldSpace ?
worldSpace->GetFormEditorID() : "<null>") and use that safe value in the
logger::error call so you never call worldSpace->GetFormEditorID() when
worldSpace is null; update the code around SetCurrentWorldSpace and the logger
call in WaterCache.cpp accordingly.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

Guard against nullptr worldSpace before logging.

SetCurrentWorldSpace(nullptr) returns false, and the error log then dereferences worldSpace, which is a crash path. Add an early-null guard (or a safe editorID string) before calling SetCurrentWorldSpace.

🔧 Proposed fix

 std::vector<WaterCache::Instruction>* WaterCache::GetInstructions(const RE::TESWorldSpace* worldSpace, const uint32_t lodLevel, const uint32_t x, const uint32_t y)
 {
-	if (!SetCurrentWorldSpace(worldSpace)) {
+	if (!worldSpace) {
+		logger::error("[Unified Water] [Cache] GetInstructions called with null worldSpace");
+		return nullptr;
+	}
+	if (!SetCurrentWorldSpace(worldSpace)) {
 		logger::error("[Unified Water] [Cache] Failed to set current cache to {} while getting instructions", worldSpace->GetFormEditorID());
 		return nullptr;
 	}

🤖 Prompt for AI Agents

In `@src/Features/UnifiedWater/WaterCache.cpp` around lines 44 - 46, The error log
dereferences worldSpace when SetCurrentWorldSpace fails, which crashes if
worldSpace is null; before calling SetCurrentWorldSpace (and before logging) add
a nullptr guard for worldSpace (or compute a safe editorID string via a ternary
like editorID = worldSpace ? worldSpace->GetFormEditorID() : "<null>") and use
that safe value in the logger::error call so you never call
worldSpace->GetFormEditorID() when worldSpace is null; update the code around
SetCurrentWorldSpace and the logger call in WaterCache.cpp accordingly.

[github-actions]

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