feat(ISL-editor): add light counters and fix filtering

[SkrubbySkrubInAShrub]

Small rework of the ISL editor, added the counting of shadowcasters and normal lights.
Changed some sorting logic to ensure no lights are skipped and added guards for spotlights so they can be viewed but not edited.
Gives more transparency to the lighting mod authors.

Summary by CodeRabbit

• New Features

  • Displays total light and active shadow-casting light counters; adds a "Shadows only" checkbox filter with hover tooltip; hiding non-shadow lights when enabled.

• Improvements

  • Refined light-type filtering so spot/spot-shadow lights are handled separately and excluded from certain lists.
  • More reliable position handling and guarded updates to avoid missing-parent issues.
  • Distance sorting now prioritizes lights with valid positions.
  • Inverse-square control is disabled when a spotlight is selected.

[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

Run ID: 63bbefa1-1861-452f-a4be-0efbdeb8e8cb

📥 Commits

Reviewing files that changed from the base of the PR and between 0342f36 and 808b83a.

📒 Files selected for processing (1)

• src/Features/InverseSquareLighting/LightEditor.cpp

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

• src/Features/InverseSquareLighting/LightEditor.cpp

---

📝 Walkthrough

Walkthrough

Adds a shadowsOnly filter and per-light flags (isSpotlight, hasPosition); introduces totalLightCount and activeShadowLightCount. Gathering, filtering, selection, and sorting logic updated to classify spotlights, count shadow lights, safely handle parent/position access, and prioritize items with positions.

Changes

Cohort / File(s)

Summary

Light Editor Core src/Features/InverseSquareLighting/LightEditor.h, src/Features/InverseSquareLighting/LightEditor.cpp

Added shadowsOnly flag and counters (totalLightCount, activeShadowLightCount); extended LightInfo with isSpotlight and hasPosition. Updated Gather/Draw to classify spotlights and shadow status, increment counters, early-filter when shadowsOnly is set, avoid unconditional parent access, set/refine position and name safely, guard original/display position updates, disable inverse-square UI for spotlights, and sort items so those with positions come first.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Suggested reviewers

• alandtse

Poem

🐰 I count the lights that cast a shade,
I mark the spots the shadows made.
I hop with care, avoid the fall,
Position first, I sort them all. ✨

🚥 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 'feat(ISL-editor): add light counters and fix filtering' accurately reflects the main changes: addition of light counters (totalLightCount, activeShadowLightCount) and filtering/guard fixes for spotlight lights.

_{✏️ 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.

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/Features/InverseSquareLighting/LightEditor.cpp (1)

302-311: ⚠️ Potential issue | 🔴 Critical

Potential null pointer dereference on niLight->parent.

selected.isAttached is set based on refr != nullptr (line 202), independent of whether niLight->parent exists. A light can have isAttached = true while parent is null, causing a crash at lines 303, 306, and 308.

Line 276 correctly guards this case; the same pattern should be applied here.

Proposed fix

 	} else if (selected.isAttached) {
+		if (!niLight->parent)
+			return;
 		const auto currentPos = niLight->parent->local.translate;
 		const auto newPos = original.pos + current.pos;
 		if (currentPos != newPos) {

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/Features/InverseSquareLighting/LightEditor.cpp` around lines 302 - 311,
The code assumes niLight->parent exists when selected.isAttached is true,
causing possible null dereference; update the attached branch in the LightEditor
logic to first check niLight and niLight->parent (e.g., if (selected.isAttached
&& niLight && niLight->parent)) before reading or writing
parent->local.translate, creating RE::NiUpdateData, calling parent->Update, or
setting waitFrames, and only then update displayInfo.pos = newPos; mirror the
same null-guard pattern used earlier in this file to prevent crashes.

🧹 Nitpick comments (2)

src/Features/InverseSquareLighting/LightEditor.h (1)

25-26: Consider adding default initialization for isSpotlight.

isSpotlight lacks default initialization, unlike the adjacent hasPosition = false and other bool members (isSelected = false). While aggregate initialization (LightInfo selected = {};) zero-initializes all members, adding an explicit default keeps the pattern consistent and guards against future misuse.

Suggested fix

-		bool isSpotlight;
+		bool isSpotlight = false;
		bool hasPosition = false;

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/Features/InverseSquareLighting/LightEditor.h` around lines 25 - 26, Add
an explicit default initializer for the boolean isSpotlight (e.g., set
isSpotlight = false) in the LightInfo/LightEditor.h declaration to match the
existing pattern used for hasPosition and isSelected; update the isSpotlight
member declaration so it is directly initialized, ensuring consistent defaults
and preventing future uninitialized-usage issues.

src/Features/InverseSquareLighting/LightEditor.cpp (1)

205-207: Counter reflects filtered state, not absolute totals.

The counters are incremented after the shadowsOnly early-return (lines 198-200), so when shadowsOnly is enabled, totalLightCount only counts shadow lights. If the intent is to always show absolute totals regardless of filter, move the counter increments before the early return.

If this behavior is intentional (showing filtered counts), consider renaming the UI label from "Total Lights" to "Displayed Lights" or similar for clarity.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/Features/InverseSquareLighting/LightEditor.cpp` around lines 205 - 207,
The counters totalLightCount and activeShadowLightCount are incremented after
the shadowsOnly early-return, so they reflect the filtered (displayed) set
instead of absolute totals; fix this by moving the increments for
totalLightCount++ (and activeShadowLightCount++ when isShadow is true) to occur
before the shadowsOnly check/early-return so they always count all lights, or if
the filtered behavior was intentional, rename the UI label "Total Lights" to
something like "Displayed Lights" to avoid confusion.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@src/Features/InverseSquareLighting/LightEditor.cpp`:
- Around line 302-311: The code assumes niLight->parent exists when
selected.isAttached is true, causing possible null dereference; update the
attached branch in the LightEditor logic to first check niLight and
niLight->parent (e.g., if (selected.isAttached && niLight && niLight->parent))
before reading or writing parent->local.translate, creating RE::NiUpdateData,
calling parent->Update, or setting waitFrames, and only then update
displayInfo.pos = newPos; mirror the same null-guard pattern used earlier in
this file to prevent crashes.

---

Nitpick comments:
In `@src/Features/InverseSquareLighting/LightEditor.cpp`:
- Around line 205-207: The counters totalLightCount and activeShadowLightCount
are incremented after the shadowsOnly early-return, so they reflect the filtered
(displayed) set instead of absolute totals; fix this by moving the increments
for totalLightCount++ (and activeShadowLightCount++ when isShadow is true) to
occur before the shadowsOnly check/early-return so they always count all lights,
or if the filtered behavior was intentional, rename the UI label "Total Lights"
to something like "Displayed Lights" to avoid confusion.

In `@src/Features/InverseSquareLighting/LightEditor.h`:
- Around line 25-26: Add an explicit default initializer for the boolean
isSpotlight (e.g., set isSpotlight = false) in the LightInfo/LightEditor.h
declaration to match the existing pattern used for hasPosition and isSelected;
update the isSpotlight member declaration so it is directly initialized,
ensuring consistent defaults and preventing future uninitialized-usage issues.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 16dbc4c9-4cc0-4a7a-975f-f1f57750efdd

📥 Commits

Reviewing files that changed from the base of the PR and between c8fe392 and 18f6b7b.

📒 Files selected for processing (2)

• src/Features/InverseSquareLighting/LightEditor.cpp
• src/Features/InverseSquareLighting/LightEditor.h

[coderabbitai]

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/Features/InverseSquareLighting/LightEditor.cpp`:
- Line 276: When computing position for attached lights, ensure the UI and
displayed values are cleared/hidden when the light has no parent: in the branch
that sets original.pos (the ternary involving selected.isRef,
refr->GetPosition(), and niLight->parent->local.translate) add explicit handling
for non-ref lights with a null niLight->parent by assigning original.pos =
RE::NiPoint3{} and also updating displayInfo.pos = RE::NiPoint3{} and toggling
the position-editor visibility/enable flag (the same flag the UI reads to show
the position editor) to false; apply the identical fix in the other block
mentioned (the code around lines 302-312) so the position controls are hidden
and show no stale coordinates whenever an attached niLight has no parent.
- Around line 190-197: current.isSpotlight is computed before the fallback
resolution of ligh, so non-ref lights resolved via runtimeData->lighFormId get
misclassified; move the spotlight classification to after the fallback so that
ligh is checked both when it is the original reference and after LookupByID,
i.e., keep setting current.isRef based on ligh, perform the
runtimeData->lighFormId lookup to set ligh if needed (runtimeData->lighFormId !=
0), then compute current.isSpotlight (and isShadow) using the possibly-updated
ligh.
- Around line 91-96: The spotlight UI currently only disables the ISL checkbox;
make spotlight selections fully read-only by wrapping all spotlight-related
widgets in ImGui::BeginDisabled(selected.isSpotlight)/ImGui::EndDisabled (i.e.,
move the BeginDisabled to cover the "Linear Light" checkbox, color/intensity
controls, radius/size/cutoff fields and any other inputs that mutate
current.data) and also prevent any state writeback by early-exiting or skipping
modifications in UpdateSelectedLight() when selected.isSpotlight is true so that
current.data and selection flags are not changed for spotlight entries.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 9bf02516-9526-4f54-afea-1c7dd5f65061

📥 Commits

Reviewing files that changed from the base of the PR and between 18f6b7b and 0342f36.

📒 Files selected for processing (2)

• src/Features/InverseSquareLighting/LightEditor.cpp
• src/Features/InverseSquareLighting/LightEditor.h

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

• src/Features/InverseSquareLighting/LightEditor.h

[github-actions]

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

[alandtse]

This will need to be integrated with the coming shadowmap fix.