feat(llf): restore JSON-scaled Particle Lights#45
Conversation
|
Warning Review limit reached
More reviews will be available in 12 minutes and 54 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (11)
📝 WalkthroughWalkthroughThis PR adds configurable particle-light support to the LightLimitFix shader feature. It includes shader-side light-type flagging and contact-shadow gating, INI-based particle configuration loading, CPU-side luminance computation and clustering, JSON-placed light intensity scaling, and render-pass culling hooks. ChangesParticle Lights Feature Addition
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
No actionable suggestions for changed features. |
There was a problem hiding this comment.
Pull request overview
This PR restores Light Limit Fix’s “Particle Lights” functionality by recognizing configured particle effects during rendering, emitting them as dynamic lights through LLF’s pipeline, and optionally feeding them into NPC light-level/stealth detection. It also adds JSON-placed light intensity scaling (gated by Inverse Square Lighting metadata) and extends LLF contact shadows to optionally include particle lights.
Changes:
- Add particle-light config loading (
Data\\ParticleLights\\*.ini+Gradients\\*.ini) and a render-pass hook path to queue/cull recognized particle emitters. - Extend LLF runtime lighting to cluster/emit particle lights, support particle-contact-shadows, and add JSON-placed light intensity scaling + settings sanitization.
- Update shader cbuffers and light-flag bits to support particle-specific contact shadow gating; bump LLF feature version to
3-2-0.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Hooks.h | Declares additional BSBatchRenderer::RenderPassImmediately hook thunks used for particle-light culling. |
| src/Hooks.cpp | Implements render-pass skip logic (SEH-guarded) and installs the new render-pass hooks. |
| src/Globals.h | Forward-declares ParticleLights and exposes it under globals::features::llf. |
| src/Globals.cpp | Instantiates the global ParticleLights manager. |
| src/Features/LightLimitFix/ParticleLights.h | Adds ParticleLights config structs and config maps. |
| src/Features/LightLimitFix/ParticleLights.cpp | Loads particle-light configs and gradient overrides from INI files. |
| src/Features/LightLimitFix.h | Adds LLF state for particle lights + JSON placed-light cache, new settings fields, and new hook declarations. |
| src/Features/LightLimitFix.cpp | Implements particle light detection/queueing, clustering/emission, NPC luminance injection, JSON placed-light scaling, and settings sanitization. |
| package/Shaders/Lighting.hlsl | Gates contact shadows for particle lights via a new Particle light-flag + setting. |
| package/Shaders/Common/SharedData.hlsli | Extends LLF settings cbuffer with EnableParticleContactShadows. |
| features/Light Limit Fix/Shaders/LightLimitFix/Common.hlsli | Adds LightFlags::Particle bit. |
| features/Light Limit Fix/Shaders/Features/LightLimitFix.ini | Bumps LLF shader feature version 3-1-0 → 3-2-0. |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/LightLimitFix.cpp`:
- Around line 1026-1035: The current block treats a non-empty
material->greyscaleTexturePath as fatal if ExtractTextureStem or a matching
entry in particleLights.particleLightGradientConfigs isn't found (calling
cacheInvalidReference(node)); change it so a missing/invalid gradient is treated
as an optional override instead: call
ExtractTextureStem(material->greyscaleTexturePath.c_str()), and if the stem is
empty or gradientConfigs.find(textureName) == gradientConfigs.end(), do NOT
return cacheInvalidReference(node) but simply leave hasGradientConfig false and
continue; only set hasGradientConfig = true when a matching itGradient is found.
In `@src/Features/LightLimitFix.h`:
- Around line 334-335: The hook installation for
AIProcess_CalculateLightValue_GetLuminance uses REL::Relocate with only two
offsets (SE/AE) but lacks the VR offset; update the stl::write_thunk_call
invocation in LightLimitFix.h to include the VR relocation (or follow the
pattern used by other hooks: use REL::Relocate with three offsets or a
VR-specific relocation branch) so the REL::RelocationID(...).address() +
REL::Relocate(...) resolves correctly for VR builds; reference the symbol
AIProcess_CalculateLightValue_GetLuminance and the existing
stl::write_thunk_call/REL::RelocationID call to add the third VR offset or
conditional path consistent with the rest of the codebase.
In `@src/Features/LightLimitFix/ParticleLights.cpp`:
- Around line 62-67: The INI numeric fields read into ParticleLightData
(data.colorMult.red/green/blue, data.radiusMult, data.saturationMult) are not
validated and can be NaN/inf or out-of-range; before assigning values returned
from ini.GetDoubleValue, validate with std::isfinite and clamp to safe ranges
(e.g., color multipliers >=0 and capped to a reasonable max like 100.0,
radiusMult >=0 and capped to a sane max, saturationMult within [0, 10] or
similar) and fall back to the default (1.0) when invalid; update the reads in
ParticleLights.cpp to sanitize each value and only store the validated/clamped
floats into data.* to avoid propagating malformed values into rendering math.
🪄 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
Run ID: be901d39-2499-466a-ba69-a7d18771de66
📒 Files selected for processing (12)
features/Light Limit Fix/Shaders/Features/LightLimitFix.inifeatures/Light Limit Fix/Shaders/LightLimitFix/Common.hlslipackage/Shaders/Common/SharedData.hlslipackage/Shaders/Lighting.hlslsrc/Features/LightLimitFix.cppsrc/Features/LightLimitFix.hsrc/Features/LightLimitFix/ParticleLights.cppsrc/Features/LightLimitFix/ParticleLights.hsrc/Globals.cppsrc/Globals.hsrc/Hooks.cppsrc/Hooks.h
|
✅ A pre-release build is available for this PR: |
0fcfaa1 to
8f00f64
Compare
d8b678d to
32c9bc0
Compare
Restores the Particle Lights feature removed from upstream Community Shaders, ported from ParticleTroned's cs-1.5-PL-VR branch and adapted to the current LLF / contact-shadows shape. Particle effects matching a Data\ParticleLights\*.ini entry are queued, clustered, and emitted as dynamic lights through LLF's pipeline, fed into AIProcess light-level detection for stealth, and optionally culled from their original draw. Adds JSON-placed light intensity scaling (gated on Inverse Square Lighting metadata) and an opt-in particle contact-shadow toggle. Co-authored-by: ParticleTroned <248299730+ParticleTroned@users.noreply.github.com>
32c9bc0 to
12de972
Compare
Restructures the Particle Lights / Placed Lights settings to match the SeparatorText + subsection-tree convention used elsewhere: - Adds an intro noting particle lights are additive emitters that don't cast shadow-map shadows (so they never appear in the caster table) and require a Data\ParticleLights\*.ini config pack to do anything. - De-duplicates the "Particle Lights" header (separator + identically named tree) and splits the controls into Performance / Appearance subsection trees, mirroring the Contact Shadows layout. - Promotes "Placed Lights (JSON)" to its own section with a short intro. - Fills in missing tooltips (Saturation, Particle/Billboard Brightness/Radius). UI text and layout only; no behavior change.
CommonLibVR 4.19.0 names BSEffectShaderProperty's previously-unknown member (PR alandtse/CommonLibVR#154). Switch the particle-light tint code from shaderProperty->unk88 to ->emittanceColor; no behavior change (same offset 0x88). Also trims the now-redundant explanatory comments and merges the duplicate anonymous namespace in ParticleLights.cpp. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The ParticleLights config holder was only ever read by LightLimitFix code (its own TUs), so it doesn't need to live in globals::features::llf. Move it to a LightLimitFix member and drop the Globals.h/Globals.cpp churn (forward decl, extern, include, definition). Unlike the depth-buffer globals in that namespace, no file-scope/cross-TU code consumes it. No behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…y-out GetCommonBufferData() never set perFrame.EnableContactShadows (lost in the dev merge), so the shader saw it as 0 and contact shadows were disabled outright. Restore the assignment. Also clear cachedParticleLights in UpdateLights()'s context/buffer early-out so AddParticleLightLuminance (gameplay thread) can't feed stale lights into NPC detection — matching the existing clearAndUpdate path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Summary
Restores the Particle Lights feature that was previously removed from upstream Community Shaders, ported from ParticleTroned/skyrim-community-shaders@cs-1.5-PL-VR and adapted to our current LLF / contact-shadows shape.
Data\ParticleLights\*.ini(with optionalGradients/*.inioverrides) are queued during render passes, clustered, and emitted as dynamic lights through LLF's existing pipeline.AIProcess::CalculateLightValuehook feeds particle lights into NPC light-level detection, so they affect stealth gameplay the way the legacy feature did.BSBatchRenderer::RenderPassImmediatelyhooks let an INI'sCull = truesuppress the original particle draw once it's been recognized as a light source.ClampFiniteOrDefault/SanitizeSettings) over all new tunables so malformed user JSON can't propagate NaN into the particle pipeline.EnableParticleContactShadowstoggle that routes through the newParticlelight-flag bit.Light Limit Fix/Shaders/Features/LightLimitFix.ini3-1-0 → 3-2-0.Files of note
src/Features/LightLimitFix/ParticleLights.{h,cpp}— INI loader, legacy first-win conflict policysrc/Features/LightLimitFix.{h,cpp}— queue/cluster/cache + queue, hooks, settings, sanitization, JSON-placed scalingsrc/Hooks.{h,cpp}—BSBatchRenderer_RenderPassImmediately1/2/3thunks + SEH-guardedShouldSkipRenderPassForParticleLightspackage/Shaders/Common/SharedData.hlsli—EnableParticleContactShadowsreplaces a pad slotpackage/Shaders/Lighting.hlsl— particle-aware contact-shadow gatefeatures/Light Limit Fix/Shaders/LightLimitFix/Common.hlsli—LightFlags::ParticlebitCo-author
cs-1.5-PL-VRTest plan
Data\ParticleLights\*.iniconfigs from a particle-light pack (Embers HD, Lanterns of Skyrim, etc.); verify candles/braziers/torches emit lightEnable Contact Shadowstoggle on non-particle point lights (VR-stable noise check from feat(llf): restore contact shadows with VR-aware noise #36 still holds)LightLimitFix.jsonround-trips with the new fields after Save/Load; malformed values (NaN, out-of-range) get clamped to defaultsHeads-up for reviewers
BSEffectShaderProperty::unk88and the two-argREL::Relocateoffsets on theBSBatchRenderer::RenderPassImmediatelyhooks are inherited verbatim from ParticleTroned'scs-1.5-PL-VRbranch — they're known to work there but ourextern/CommonLibSSE-NGrevision should be confirmed to exposeunk88.EffectExtensions::BSEffectShader_SetupGeometryhook insrc/Hooks.cppstill co-exists withLightLimitFix::Hooks::BSEffectShader_SetupGeometry, soExternalEmittance::UpdatePermutationruns twice per effect pass. Pre-existing condition, idempotent, not addressed here — worth a follow-up to delete the LLF-side call.https://claude.ai/code/session_01SoyystxDs66Via3eZreNfU
Generated by Claude Code
Summary by CodeRabbit
New Features
Improvements
Stability/Performance