fix(wetness-effects): raindrop timing#2069
Merged
alandtse merged 8 commits intoApr 8, 2026
Merged
Conversation
raindrops fade\out with actual particles
Automated formatting by clang-format, prettier, and other hooks. See https://pre-commit.ci for details.
|
No actionable suggestions for changed features. |
….com/InTheBottle/skyrim-community-shaders into fix(wetness-effects)-raindrop-timing
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR normalizes line endings in an upscaling shader INI, bumps the WetnessEffects shader feature version, and changes WetnessEffects rain blending to use weather-specific, threshold-aware fade-in/fade-out weights when composing final raining intensity. Changes
Sequence Diagram(s)sequenceDiagram
participant Sky
participant CurrentWeather
participant LastWeather
participant Wetness
participant Shader
Sky ->> Wetness: provide currentWeatherPct, pointers to current/last weather
Wetness ->> CurrentWeather: read isRainy?, precipitationBeginFadeIn
Wetness ->> LastWeather: read isRainy?, precipitationEndFadeOut
Wetness ->> Wetness: compute linearstep = clamp((pct - edge0)/(edge1 - edge0))
Wetness ->> Wetness: currentWeight = isRainy? linearstep(pBegin, pEnd, pct) : 0
Wetness ->> Wetness: lastWeight = isRainy? linearstep(pBegin, pEnd, pct) : 0
Wetness ->> Wetness: data.Raining = currentRaining*currentWeight + lastRaining*lastWeight
Wetness ->> Shader: upload CommonBuffer (includes data.Raining)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/Features/WetnessEffects.cpp (1)
738-740: Consider extracting thelinearstephelper to reduce duplication.The same
linearsteplambda is defined both here and inCalculateWeatherWetness(lines 600-602). Extracting to a file-level helper or utility namespace would reduce duplication.♻️ Optional: Extract to file-level helper
Add near the top of the file (after includes):
namespace { inline float linearstep(float edge0, float edge1, float x) { return std::clamp((x - edge0) / (edge1 - edge0), 0.0f, 1.0f); } }Then remove the inline lambda definitions in both functions.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Features/WetnessEffects.cpp` around lines 738 - 740, Duplicate linearstep lambda exists in WetnessEffects.cpp (inside the local block near the function using it and also in CalculateWeatherWetness); extract a single helper by adding an internal-linkage function named linearstep (e.g., inline float linearstep(float edge0, float edge1, float x) in an anonymous namespace near the top of the file) and replace/remove the local lambda definitions in both the function that currently defines it and in CalculateWeatherWetness so both call the shared linearstep helper.
🤖 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/WetnessEffects.cpp`:
- Around line 738-752: The linearstep lambda can divide by zero when edge0 ==
edge1; update uses where linearstep is called (computing currentWeight and
lastWeight) to guard against that: before calling linearstep for fadeInThreshold
(used to set currentWeight) and fadeOutThreshold (used to set lastWeight) check
for equality (or near-equality) of edge0 and edge1 and handle those cases
explicitly (e.g., set linear step result to 0.0f or 1.0f as appropriate) so you
never call linearstep(1.0f,1.0f,...) or linearstep(0.0f,0.0f,...); reference the
linearstep lambda, fadeInThreshold, fadeOutThreshold, currentWeight, lastWeight,
and sky->currentWeatherPct when making the change (mirror the guard already used
in CalculateWeatherWetness).
---
Nitpick comments:
In `@src/Features/WetnessEffects.cpp`:
- Around line 738-740: Duplicate linearstep lambda exists in WetnessEffects.cpp
(inside the local block near the function using it and also in
CalculateWeatherWetness); extract a single helper by adding an internal-linkage
function named linearstep (e.g., inline float linearstep(float edge0, float
edge1, float x) in an anonymous namespace near the top of the file) and
replace/remove the local lambda definitions in both the function that currently
defines it and in CalculateWeatherWetness so both call the shared linearstep
helper.
🪄 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: ee79a08a-d931-42f5-80df-a7a8d5bcf23b
📒 Files selected for processing (3)
features/Upscaling/Shaders/Features/Upscaling.inifeatures/Wetness Effects/Shaders/Features/WetnessEffects.inisrc/Features/WetnessEffects.cpp
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/Features/WetnessEffects.cpp (1)
738-757:⚠️ Potential issue | 🟠 Major
precipitationBeginFadeIn == 255should step on at full transition, not stay at zero.The divide-by-zero is gone, but this fallback hard-codes
currentWeightto0.0ffor the maximum fade-in value. That makes a rainy weather never contribute todata.Raining, even after it becomes fully active withcurrentWeatherPct == 1.0f.🔧 Proposed fix
auto linearstep = [](float edge0, float edge1, float x) { - return std::clamp((x - edge0) / (edge1 - edge0), 0.0f, 1.0f); + if (edge0 >= edge1) + return x >= edge1 ? 1.0f : 0.0f; + return std::clamp((x - edge0) / (edge1 - edge0), 0.0f, 1.0f); }; @@ if (sky->currentWeather && sky->currentWeather->data.flags.any(RE::TESWeather::WeatherDataFlag::kRainy)) { float fadeInThreshold = sky->currentWeather->data.precipitationBeginFadeIn * (1.0f / 255.0f); - if (fadeInThreshold >= 1.0f) - currentWeight = 0.0f; // fade-in at end of transition, never rains - else - currentWeight = linearstep(fadeInThreshold, 1.0f, sky->currentWeatherPct); + currentWeight = linearstep(fadeInThreshold, 1.0f, sky->currentWeatherPct); } @@ if (sky->lastWeather && sky->lastWeather->data.flags.any(RE::TESWeather::WeatherDataFlag::kRainy)) { float fadeOutThreshold = sky->lastWeather->data.precipitationEndFadeOut * (1.0f / 255.0f); - if (fadeOutThreshold <= 0.0f) - lastWeight = 0.0f; // immediate cutoff, no fade-out period - else - lastWeight = 1.0f - linearstep(0.0f, fadeOutThreshold, sky->currentWeatherPct); + lastWeight = 1.0f - linearstep(0.0f, fadeOutThreshold, sky->currentWeatherPct); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Features/WetnessEffects.cpp` around lines 738 - 757, The code treats precipitationBeginFadeIn == 255 (fadeInThreshold >= 1.0f) as always setting currentWeight to 0.0f, which prevents a rainy weather from contributing when fully active; update the branch in the currentWeight calculation to set currentWeight = 1.0f when sky->currentWeatherPct is fully transitioned (>= 1.0f) and 0.0f otherwise (e.g., replace the hard-coded currentWeight = 0.0f with a ternary using sky->currentWeatherPct), keeping the existing linearstep(path) for the normal case; locate this change around variables/function names: precipitationBeginFadeIn, fadeInThreshold, currentWeight, linearstep, and sky->currentWeatherPct.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/Features/WetnessEffects.cpp`:
- Around line 738-757: The code treats precipitationBeginFadeIn == 255
(fadeInThreshold >= 1.0f) as always setting currentWeight to 0.0f, which
prevents a rainy weather from contributing when fully active; update the branch
in the currentWeight calculation to set currentWeight = 1.0f when
sky->currentWeatherPct is fully transitioned (>= 1.0f) and 0.0f otherwise (e.g.,
replace the hard-coded currentWeight = 0.0f with a ternary using
sky->currentWeatherPct), keeping the existing linearstep(path) for the normal
case; locate this change around variables/function names:
precipitationBeginFadeIn, fadeInThreshold, currentWeight, linearstep, and
sky->currentWeatherPct.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4ddc4b0a-4af7-4f6e-bb45-f4d3ee8d979c
📒 Files selected for processing (1)
src/Features/WetnessEffects.cpp
|
✅ A pre-release build is available for this PR: |
alandtse
requested changes
Apr 7, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/WetnessEffects.cpp`:
- Around line 740-744: The guarded linearstep lambda defined near line 740
should be reused by WetnessEffects::CalculateWeatherWetness to avoid the
zero-width threshold bug: replace the unguarded linearstep calls in
WetnessEffects::CalculateWeatherWetness (which currently call
linearstep(precipitationEndFadeOut, precipitationEndFadeOut, ...)) with the
guarded helper (or move the lambda to a file-local function) so that the
edge0>=edge1 case returns the correct 0/1 behavior; update references to use the
single file-local linearstep and ensure data.Raining and precipitationEndFadeOut
logic now calls that guarded version.
🪄 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: afd96d58-c619-41dd-8c0c-b3b74e0504dc
📒 Files selected for processing (1)
src/Features/WetnessEffects.cpp
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/Features/WetnessEffects.cpp (1)
739-753: PR metadata nit: add an issue keyword if one exists.Title already follows Conventional Commits. If this fix is tracked, consider adding
Fixes #<id>in the PR description for traceability.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Features/WetnessEffects.cpp` around lines 739 - 753, This PR needs an issue reference for traceability—update the PR description (or the commit message that introduced changes in src/Features/WetnessEffects.cpp) to include the issue keyword like "Fixes #<id>" so the change affecting symbols such as currentWeight/lastWeight and the linearstep logic in WetnessEffects.cpp is linked to the tracker; add the correct issue number and ensure the PR title or description contains "Fixes #<id>".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/Features/WetnessEffects.cpp`:
- Around line 739-753: This PR needs an issue reference for traceability—update
the PR description (or the commit message that introduced changes in
src/Features/WetnessEffects.cpp) to include the issue keyword like "Fixes #<id>"
so the change affecting symbols such as currentWeight/lastWeight and the
linearstep logic in WetnessEffects.cpp is linked to the tracker; add the correct
issue number and ensure the PR title or description contains "Fixes #<id>".
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6786672e-d687-4ec9-80ad-92d079fc1c93
📒 Files selected for processing (1)
src/Features/WetnessEffects.cpp
alandtse
approved these changes
Apr 8, 2026
ParticleTroned
pushed a commit
to ParticleTroned/skyrim-community-shaders
that referenced
this pull request
May 2, 2026
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> (cherry picked from commit ae325ac)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
raindrops fade in\out with actual particles
Summary by CodeRabbit