Skip to content

Conversation

@DedeHai
Copy link

@DedeHai DedeHai commented Oct 27, 2025

  • some renaming and simpler #defines
  • if not enough FX memory is available, reduces number of particles and retries -> makes PS effects run on larger setups / low heap situations

Summary by CodeRabbit

Release Notes

  • Bug Fixes

    • Corrected particle effect color saturation handling to prevent exceeding palette-derived values.
  • Improvements

    • Enhanced particle system memory allocation with fallback logic—reduces particle count gracefully when memory is constrained, ensuring effects remain stable on resource-limited devices.

@coderabbitai
Copy link

coderabbitai bot commented Oct 27, 2025

Walkthrough

The particle system is refactored to consolidate architecture-specific memory-allocation macros into unified public constants while maintaining per-architecture values. Memory allocation retry logic gracefully reduces particle counts on failure, and saturation handling is updated to prevent over-saturation via a min() guard.

Changes

Cohort / File(s) Summary
Header constant consolidation
wled00/FXparticleSystem.h
Replaced per-architecture macros (ESP8266_MAXPARTICLES, ESP32_MAXPARTICLES, etc.) with unified names (MAXPARTICLES_2D, MAXSOURCES_2D, MAXPARTICLES_1D, MAXSOURCES_1D); introduced architecture-specific conditional blocks and SOURCEREDUCTIONFACTOR constant for consistent semantic interface across all boards.
Implementation updates
wled00/FXparticleSystem.cpp
Added memory allocation retry loops in initParticleSystem2D and initParticleSystem1D to gracefully reduce particle count on allocation failure; updated saturation handling to use min(baseHSV.s, saturation) preventing over-saturation; replaced architecture-specific defines with generalized MAXPARTICLES_2D, MAXSOURCES_2D constants and SOURCEREDUCTIONFACTOR-based source calculation.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Memory allocation retry logic: Verify loop termination conditions (minimum particle thresholds of 4 for 2D, 10 for 1D) and alignment/halving arithmetic
  • Saturation guard: Confirm min() logic correctly prevents palette-derived saturation from being exceeded
  • Macro consolidation: Cross-check that all per-architecture constant values align with original definitions and that source count calculation using SOURCEREDUCTIONFACTOR produces equivalent results

Possibly related PRs

Poem

🐰 With constants now unified and neat,
Allocation retries make the system complete,
Saturation guards keep the colors just right,
While particles dance in memory's light! 🌟

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Title Check ❓ Inconclusive The title "ParticleFX: adding fixes and improvements from AC" refers to the correct component (ParticleFX) and acknowledges that the PR contains fixes and improvements, which are indeed present in the changeset. However, the title is quite generic and vague—it uses non-descriptive phrasing that doesn't convey what was specifically fixed or improved. The phrase "from AC" adds unexplained context without clarity. Most critically, the title fails to capture the primary technical changes: memory allocation retry logic that allows particle effects to run in low-heap situations, macro constant generalization across architectures, and saturation handling adjustments. A teammate scanning the history would not understand the nature or significance of the changes from this title alone. Consider revising the title to be more specific and descriptive. A stronger title might highlight the primary change, such as "ParticleFX: add memory allocation retry logic for low-heap situations" or "ParticleFX: generalize memory constants and add allocation retry fallback," which would immediately communicate the significance and intent of the changes to reviewers.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ 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.

@softhack007 softhack007 self-assigned this Oct 28, 2025
@softhack007 softhack007 changed the title adding fixes and improvements from AC ParticleFX: adding fixes and improvements from AC Oct 28, 2025
@softhack007 softhack007 merged commit 96bfd56 into MoonModules:mdev Nov 1, 2025
29 checks passed
softhack007 added a commit that referenced this pull request Nov 1, 2025
ParticleFX: adding fixes and improvements from AC
@softhack007 softhack007 added this to the 14.7.1 milestone Dec 23, 2025
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.

2 participants