refactor(UI): move light limit visualization into debug

[brucenguyen]

A frequent support inquiry is about this toggle. As it exists right now, users may confuse the Enable Lights Visualization toggle with a toggle for the LLF feature. Some reading might clear that up, but reading is a big ask for regular users.

Changes:

• Moved Light Limit Visualization into a new debug section at the bottom of the page. Collapsed by default
• Made Enable Lights Visualization ephemeral, no longer saves to JSON. Kept other settings unchanged, including Lights Visualization Mode

While looking into this, I noticed that there's no standard way to implement a debug section for each feature. There appears to be various implementations (e.g., Screen Space GI, Terrain Shadows, Wetness Effects). I don't have a source of truth so I just went with the SSGI implementation. If a different design is preferred, I can make that change.

Summary by CodeRabbit

• New Features

  • Added Statistics section displaying clustered light count information in visualization settings.

• Improvements

  • Reorganized debug visualization UI structure for improved clarity and organization.

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

[coderabbitai]

Walkthrough

Removed a public settings field and reorganized the LightLimitFix ImGui layout: the top-level section is renamed to "Statistics" with a displayed clustered light count, a debug separator was added, and the existing "Light Limit Visualization" subsection remains.

Changes

Cohort / File(s)	Summary
UI & Settings src/Features/LightLimitFix.cpp	Removed public field EnableLightsVisualisation from LightLimitFix::Settings; renamed top-level ImGui section to "Statistics" and added "Clustered Light Count : {lightCount}"; inserted a debug separator; retained the existing "Light Limit Visualization" subsection and adjusted TreeNode/TreePop usage; minor EOF newline cleanup.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20–30 minutes

• Removal of a public setting requires checking all consumers/usages across the codebase.
• Review areas:
  • LightLimitFix::Settings declaration and any serialization/deserialization or config files.
  • Call sites that referenced EnableLightsVisualisation (search across repo).
  • ImGui layout changes for correct TreeNode/TreePop balancing and display text formatting.

Possibly related PRs

• fix(llf): fix some llf issues #1467 — touches LightLimitFix feature and settings/UI; likely overlaps on similar UI or settings edits.
• feat(llf): remove particle lights #1495 — directly modifies EnableLightsVisualisation handling and DrawSettings behavior; closely related.

Suggested reviewers

• alandtse
• doodlum

Poem

🐰 I nudged a setting, then hopped away,
Counting clustered lights in bright array.
A Debug line, a TreeNode's cheer,
Visualization still tucked near,
Hooray — the UI's in bloom today!

Pre-merge checks and finishing touches

❌ 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%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ 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 reflects the main change: moving light limit visualization UI into a debug section, which is the primary refactoring objective of the pull request.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

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: 39aa9d7
Using base ref: 39aa9d7
Base commit date: 2025-11-30T19:11:20-08:00 (Sunday, November 30, 2025 07:11 PM)
No actionable suggestions for changed features.

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

src/Features/LightLimitFix.cpp (1)

20-29: UI re-layout looks correct; consider simpler ImGui::Text formatting

The new "Statistics" tree node and "Debug" separator cleanly separate stats from the light-limit visualization and match the described debug-section behavior (stats visible, visualization under a collapsible node).

You could avoid an extra allocation and formatting layer by letting ImGui handle the formatting directly:

-	if (ImGui::TreeNodeEx("Statistics", ImGuiTreeNodeFlags_DefaultOpen)) {
-		ImGui::Text(std::format("Clustered Light Count : {}", lightCount).c_str());
+	if (ImGui::TreeNodeEx("Statistics", ImGuiTreeNodeFlags_DefaultOpen)) {
+		ImGui::Text("Clustered Light Count: %u", static_cast<unsigned>(lightCount));

This keeps the same output while being a bit cheaper and more in line with other ImGui text usages.

📜 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 39aa9d7 and c23e973.

📒 Files selected for processing (1)

• src/Features/LightLimitFix.cpp (2 hunks)

🧰 Additional context used

📓 Path-based instructions (3)

**/*.{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/Features/LightLimitFix.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/Features/LightLimitFix.cpp

**/*

⚙️ CodeRabbit configuration file

**/*: When reviewing PRs, please provide suggestions for:

1. Conventional Commit Titles (if not following https://www.conventionalcommits.org/ or
   if the existing title does not describe the code changes):
   Format: type(scope): description
   Length: 50 characters limit for title, 72 for body
   Style: lowercase description, no ending period
   Examples:

   • feat(vr): add cross-eye sampling
   • fix(water): resolve flowmap bug
   • docs: update shader documentation

2. Issue References (if PR fixes bugs or implements features):
   Suggest adding appropriate GitHub keywords:

   • "Fixes #123" or "Closes #123" for bug fixes
   • "Implements #123" or "Addresses #123" for features
   • "Related to #123" for partial implementations

Otherwise, use your standard review approach focusing on code quality.

Files:

• src/Features/LightLimitFix.cpp

🧠 Learnings (1)

📓 Common learnings

Learnt from: alandtse
Repo: doodlum/skyrim-community-shaders PR: 0
File: :0-0
Timestamp: 2025-06-24T07:17:36.604Z
Learning: When reviewing PRs, always clarify the scope if there are multiple related features or dependencies. WeatherPicker was a separate PR that was already merged, while this PR focuses specifically on WetnessEffects climate preset system enhancements.

Learnt from: alandtse
Repo: doodlum/skyrim-community-shaders PR: 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 (1)

src/Features/LightLimitFix.cpp (1)

569-569: Trailing semicolon after function definition is fine

Changing the closing brace here to }; is syntactically valid, has no behavioral impact, and matches the style already used for the BSEffectShader_SetupGeometry hook above. No further action needed.

[brucenguyen]

FWIW this may be a bandaid fix for a different issue. It's possible that this toggle is turning on by itself for some odd reason. Though I've yet to see any reproducible steps on getting that to happen, so until then I have to assume this is user error.

[github-actions]

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

[midona-rhel]

I mean looks correct seems to work, simple UI change, but could I suggest that we also make this setting ephemeral. Sometimes user might download an INI with this already enabled, I don't feel like we need to save this as a setting?

[brucenguyen]

@midona-rhel thanks for the suggestion. Was simpler than I thought.

[midona-rhel]

Nice! Will try tomorrow

[alandtse]

Please note the new title.