fix: fix some internal errors when debugging

[doodlum]

Summary by CodeRabbit

• Refactor
  • Centralized engine state and memory management via singleton accessors.
  • Unified interior/exterior detection across shadows, terrain, volumetric lighting, and VR prepass for more consistent behavior.
  • Reduced reliance on legacy globals to improve stability and maintainability.
• Chores
  • Removed obsolete global variables and their initialization paths.
  • Minor internal null-check adjustments.
  • No changes to public APIs or user-facing settings.

[coderabbitai]

Walkthrough

Centralizes access to TES and MemoryManager by replacing globals::game pointers with RE::TES::GetSingleton() and RE::MemoryManager::GetSingleton(). Removes the global tes and memoryManager variables and their externs/initializations. Touches multiple features (InteriorSun, TerrainShadows, VR, VolumetricLighting), utility functions, and TruePBR allocations.

Changes

Cohort / File(s)	Summary of edits
TES singleton access refactor src/Features/InteriorSun.cpp, src/Features/TerrainShadows.cpp, src/Features/VR.cpp, src/Features/VolumetricLighting.cpp, src/Utils/Game.cpp, src/TruePBR.cpp	Replaced globals::game::tes reads with RE::TES::GetSingleton() for interior/worldspace queries and related checks.
Remove global TES/MemoryManager pointers src/Globals.h, src/Globals.cpp	Deleted extern RE::TES* tes and extern RE::MemoryManager* memoryManager declarations and their definitions/initializations; removed related usage in init paths.
MemoryManager singleton usage src/TruePBR.cpp	Replaced globals::game::memoryManager->Allocate with RE::MemoryManager::GetSingleton()->Allocate in shader/geometry allocation paths.

Sequence Diagram(s)

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• feat(VR): separate interior/exterior options for depth buffer culling #1362 — Also modifies VR::EarlyPrepass depth-culling logic in the same area touched here.
• refactor(interior sun): rename interior sun shadows #1387 — Refactors InteriorSun component where this PR updates TES access.
• fix: support seasons swaps in PBR and TerrainHelper #1099 — Edits the same TruePBR TESObjectLAND_SetupMaterial paths affected by allocation/TES access changes.

Suggested reviewers

• alandtse

Poem

I hop through code where globals slept,
Now singletons keep their vigil kept.
Shadows, sun, and VR skies,
Terrain wakes up, with bunny eyes.
Memory burrows neat and small—
GetSingleton answers every call.
boop boop, bug fixed for all! 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.
Title Check	❓ Inconclusive	The title "fix: fix some internal errors when debugging" is vague and redundant; it does not clearly summarize the PR's main change. The actual changes center on replacing globals::game pointers (TES and MemoryManager) with RE::GetSingleton() accessors and removing those global declarations. As written, a teammate scanning history would not immediately understand the primary modification.	Rename the PR to a concise, specific title that highlights the core change, for example "Replace globals::game::tes and memoryManager with RE singletons" or "Remove globals::game pointers and use RE::GetSingleton()". Use present-tense imperative phrasing, avoid redundant words like the repeated "fix", and mention the key elements (TES/memoryManager → RE singletons) so reviewers immediately see the intent.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

✨ Finishing touches

• 📝 Generate Docstrings

🧪 Generate unit tests

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch debugging-fixes

---

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: a4ad64d
Using base ref: a4ad64d
Base commit date: 2025-09-19T17:18:59+01:00 (Friday, September 19, 2025 05:18 PM)
No actionable suggestions for changed features.

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (8)

src/Utils/Game.cpp (1)

169-175: Bug: dynamic resolution result is ignored.

ConvertToDynamic(resolution); doesn’t assign the returned value, so dynamic scaling never applies.

Apply this diff:

-    if (a_dynamic)
-      ConvertToDynamic(resolution);
+    if (a_dynamic)
+      resolution = ConvertToDynamic(resolution, /*a_ignoreLock=*/false);

src/Features/VolumetricLighting.cpp (1)

208-218: Guard GetSingleton() before dereference.

RE::TES::GetSingleton() can be null early; dereferencing to read interiorCell risks a crash during initialization.

Apply this diff:

-  const auto interiorCell = RE::TES::GetSingleton()->interiorCell;
+  const auto tes = RE::TES::GetSingleton();
+  const auto interiorCell = tes ? tes->interiorCell : nullptr;

src/Features/VR.cpp (1)

119-120: Null-check TES before deciding culling mode.

Avoid dereferencing RE::TES::GetSingleton() without verifying it’s valid.

Apply this diff:

-  *gDepthBufferCulling = RE::TES::GetSingleton()->interiorCell ? settings.EnableDepthBufferCullingInterior : settings.EnableDepthBufferCullingExterior;
+  if (auto tes = RE::TES::GetSingleton()) {
+    *gDepthBufferCulling = tes->interiorCell ? settings.EnableDepthBufferCullingInterior
+                                             : settings.EnableDepthBufferCullingExterior;
+  } else {
+    // Default to exterior setting if TES not ready
+    *gDepthBufferCulling = settings.EnableDepthBufferCullingExterior;
+  }

src/Features/InteriorSun.cpp (2)

76-77: Null-check TES before reading interiorCell.

Prevent early-init crash when TES may be unavailable.

Apply this diff:

-  isInteriorWithSun = IsInteriorWithSun(RE::TES::GetSingleton()->interiorCell);
+  if (auto tes = RE::TES::GetSingleton()) {
+    isInteriorWithSun = IsInteriorWithSun(tes->interiorCell);
+  } else {
+    isInteriorWithSun = false;
+  }

---

106-114: Also guard TES in culling thunk.

Dereferencing GetSingleton() without a check can crash on startup.

Apply this diff:

-  const auto cell = RE::TES::GetSingleton()->interiorCell;
+  const auto tes = RE::TES::GetSingleton();
+  const auto cell = tes ? tes->interiorCell : nullptr;

src/TruePBR.cpp (3)

1112-1118: Defensive check for MemoryManager before Allocate.

Allocate through the singleton is correct; add a guard to avoid deref if not ready. On failure, bail out gracefully to avoid corrupt allocations.

Apply this diff:

-  auto memoryManager = RE::MemoryManager::GetSingleton();
+  auto memoryManager = RE::MemoryManager::GetSingleton();
+  if (!memoryManager) {
+    logger::error("TruePBR: MemoryManager not available; aborting TESObjectLAND material setup");
+    return false;
+  }
@@
-      auto shaderProperty = static_cast<RE::BSLightingShaderProperty*>(memoryManager->Allocate(REL::Module::IsVR() ? 0x178 : sizeof(RE::BSLightingShaderProperty), 0, false));
+      auto shaderProperty = static_cast<RE::BSLightingShaderProperty*>(
+          memoryManager->Allocate(REL::Module::IsVR() ? 0x178 : sizeof(RE::BSLightingShaderProperty), 0, false));

---

1154-1160: Guard TES before accessing worldSpace.

Avoid UB if TES is null in early frames.

Apply this diff:

-      auto tes = RE::TES::GetSingleton();
-      auto worldSpace = tes->GetRuntimeData2().worldSpace;
+      auto tes = RE::TES::GetSingleton();
+      auto worldSpace = tes ? tes->GetRuntimeData2().worldSpace : nullptr;

---

1254-1260: Repeat MemoryManager guard for decal path.

Same rationale as LAND material setup; prevent null deref.

Apply this diff:

-      auto shaderProperty = static_cast<RE::BSLightingShaderProperty*>(RE::MemoryManager::GetSingleton()->Allocate(sizeof(RE::BSLightingShaderProperty), 0, false));
+      auto mm = RE::MemoryManager::GetSingleton();
+      if (!mm) {
+        logger::error("TruePBR: MemoryManager not available; skipping PBR decal material");
+        return;
+      }
+      auto shaderProperty = static_cast<RE::BSLightingShaderProperty*>(mm->Allocate(sizeof(RE::BSLightingShaderProperty), 0, false));

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a4ad64d and 6248feb.

📒 Files selected for processing (8)

• src/Features/InteriorSun.cpp (3 hunks)
• src/Features/TerrainShadows.cpp (3 hunks)
• src/Features/VR.cpp (1 hunks)
• src/Features/VolumetricLighting.cpp (1 hunks)
• src/Globals.cpp (0 hunks)
• src/Globals.h (0 hunks)
• src/TruePBR.cpp (3 hunks)
• src/Utils/Game.cpp (2 hunks)

💤 Files with no reviewable changes (2)

• src/Globals.h
• src/Globals.cpp

🧰 Additional context used

📓 Path-based instructions (2)

**/*.{cpp,cxx,cc,c,h,hpp,hxx,hlsl,hlsli,fx,fxh,py}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Do not include TODO/FIXME placeholders; provide complete, working solutions

Files:

• src/TruePBR.cpp
• src/Utils/Game.cpp
• src/Features/TerrainShadows.cpp
• src/Features/InteriorSun.cpp
• src/Features/VR.cpp
• src/Features/VolumetricLighting.cpp

src/**/*.{cpp,cxx,cc,h,hpp,hxx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

src/**/*.{cpp,cxx,cc,h,hpp,hxx}: Ensure SE/AE/VR runtime compatibility; use runtime detection patterns (e.g., REL::RelocateMember())
Include robust error handling and resource management with graceful degradation in the plugin code

Files:

• src/TruePBR.cpp
• src/Utils/Game.cpp
• src/Features/TerrainShadows.cpp
• src/Features/InteriorSun.cpp
• src/Features/VR.cpp
• src/Features/VolumetricLighting.cpp

🧠 Learnings (1)

📓 Common learnings

Learnt from: alandtse
PR: doodlum/skyrim-community-shaders#0
File: :0-0
Timestamp: 2025-07-05T05:20:45.823Z
Learning: In the skyrim-community-shaders repository, file deletion error handling improvements that replace existence checks and try-catch blocks with std::filesystem::remove error-code-based approaches are considered bug fixes rather than refactoring, as they address inadequate error handling and misleading log messages.

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: Build plugin and addons

🔇 Additional comments (6)

src/Utils/Game.cpp (2)

34-41: Switch to TES singleton access looks good.

Null-check is in place before use. No functional change beyond the pointer source.

---

305-314: LGTM: Interior check uses TES singleton safely.

Falls back to true when TES is null, matching prior conservative behavior.

src/Features/TerrainShadows.cpp (3)

30-37: LGTM: Debug worldspace uses TES singleton with checks.

Consistent with the PR’s centralization pattern.

---

174-178: LGTM: Heightmap readiness check is safe.

Short-circuits on null TES/worldspace. No issues.

---

200-211: LGTM: LoadHeightmap null-checks TES and worldspace.

Matches expected defensive style; no behavior regressions.

src/Features/InteriorSun.cpp (1)

21-23: LGTM: Settings apply with safe interior check.

Uses TES singleton with null-aware boolean. Good.

[github-actions]

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