Skip to content

fix: replace original shaders off and not saveable#1835

Merged
alandtse merged 4 commits into
community-shaders:devfrom
alandtse:fix_panic
Feb 7, 2026
Merged

fix: replace original shaders off and not saveable#1835
alandtse merged 4 commits into
community-shaders:devfrom
alandtse:fix_panic

Conversation

@alandtse
Copy link
Copy Markdown
Collaborator

@alandtse alandtse commented Feb 7, 2026

Note this should be REBASED. Everyone will need to pull --rebase.

Summary by CodeRabbit

  • New Features

    • Added Contact Shadows configuration option to Light Limit Fix
    • Settings now persist and automatically restore when relaunching the application
    • Shader modification preferences are saved between sessions
  • Chores

    • Updated version to 1.4.11

Copilot AI review requested due to automatic review settings February 7, 2026 08:10
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Feb 7, 2026

📝 Walkthrough

Walkthrough

This PR bumps the project version to 1.4.11 and adds JSON serialization support for LightLimitFix settings, including a new EnableContactShadows flag, plus persistent storage for per-shader "Replace Original Shaders" state in the State class.

Changes

Cohort / File(s) Summary
Version Bump
CMakeLists.txt
Project version incremented from 1.4.10 to 1.4.11.
LightLimitFix Settings Serialization
src/Features/LightLimitFix.h, src/Features/LightLimitFix.cpp
Introduces LightLimitFix::Settings with new EnableContactShadows boolean field and implements JSON-based LoadSettings/SaveSettings with default initialization via RestoreDefaultSettings.
State Shader Replacement Persistence
src/State.h, src/State.cpp
Adds JSON serialization for per-shader "Replace Original Shaders" state; SaveToJson captures enabled flags for all shader types, LoadFromJson restores with validation and warning logging for missing/invalid entries. Initializes enabledClasses to true in constructor.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • doodlum
  • davo0411

Poem

🐰 ✨ A version bump so neat and clean,
Settings saved, a serializer's dream,
JSON whispers hold the shader's light,
One point four point one shines bright! 🌙

🚥 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 directly references the main functional change: making 'replace original shaders' off by default and persistently saveable.

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

✨ 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

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Feb 7, 2026

Using provided base ref: b3db5a7
Using base ref: b3db5a7
Base commit date: 2026-02-07T13:47:32+10:00 (Saturday, February 07, 2026 01:47 PM)
No actionable suggestions for changed features.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates Community Shaders’ settings persistence so shader replacement toggles (“Replace Original Shaders”) default to enabled and can be saved/loaded via JSON, and bumps the project version.

Changes:

  • Initialize State::enabledClasses to true by default.
  • Persist “Replace Original Shaders” per-shader-class enablement into the JSON config and restore it on load.
  • Add JSON (de)serialization for LightLimitFix::Settings and bump version to 1.4.11.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/State.h Defaults shader-class enable toggles to enabled at startup.
src/State.cpp Saves/loads per-shader-class “Replace Original Shaders” settings to/from JSON.
src/Features/LightLimitFix.h Adds a new settings field (EnableContactShadows).
src/Features/LightLimitFix.cpp Introduces nlohmann JSON serialization and enables saving/loading LLF settings.
CMakeLists.txt Version bump to 1.4.11.

Comment thread src/State.cpp
Comment thread src/Features/LightLimitFix.cpp
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

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

⚠️ Outside diff range comments (1)
src/State.cpp (1)

590-597: ⚠️ Potential issue | 🔴 Critical

Fix index mismatch in ShaderEnabled: change + 1 to - 1 to match save/load and UI code.

The ShaderEnabled function uses index = enum_integer(a_type) + 1 to read from enabledClasses, but ForEachShaderTypeWithIndex (used by save/load at lines 411-412, 484-487) computes classIndex = typeIndex - 1. For the same shader type with enum value v, this creates an offset of 2: ShaderEnabled reads enabledClasses[v+1] while save/load accesses enabledClasses[v-1]. This mismatch also affects the UI (AdvancedSettingsRenderer.cpp) and performance overlay, which all use the -1 scheme. Toggle state persisted in config files will not control actual shader enablement.

Change line 592 from:

auto index = magic_enum::enum_integer(a_type) + 1;

to:

auto index = magic_enum::enum_integer(a_type) - 1;
🤖 Fix all issues with AI agents
In `@src/Features/LightLimitFix.h`:
- Around line 177-182: The new Settings::EnableContactShadows field is declared
and serialized via NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE_WITH_DEFAULT in
LightLimitFix.cpp but never used; either drop the field or implement end-to-end
support: add a DrawSettings checkbox bound to Settings::EnableContactShadows
(same UI area that contains EnableLightsVisualisation), read the setting in the
module's runtime update/apply method (e.g., LightLimitFix::ApplySettings or
UpdateLights), and propagate it into the rendering pipeline by setting the
appropriate shader define/uniform or toggling the contact-shadow rendering path
so the serialized flag actually controls runtime behavior. Ensure the symbol
names to change include Settings::EnableContactShadows, the JSON macro in
LightLimitFix.cpp, the DrawSettings UI function, and the runtime method that
updates shader defines or light rendering.

Comment thread src/Features/LightLimitFix.h
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Feb 7, 2026

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

@alandtse alandtse merged commit 41dc178 into community-shaders:dev Feb 7, 2026
24 checks passed
ParticleTroned added a commit to ParticleTroned/skyrim-community-shaders that referenced this pull request Feb 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants