Skip to content

Conversation

@DedeHai
Copy link
Collaborator

@DedeHai DedeHai commented Nov 19, 2025

fixes #5102

edit:
also adds glitter, solid glitter, multi comet and sparkle FX back as default FX as they are light-weight in code size (1k total)

Summary by CodeRabbit

  • Refactor
    • Made effect compilation more granular so 1D and 2D effects can be included or excluded individually, improving build modularity and flexibility.
    • Updated how effects are registered so only the relevant effect sets are compiled and initialized.
    • No changes to public APIs or runtime behavior — user-facing features and settings remain unchanged.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 19, 2025

Walkthrough

Replaced a single broad FX compile-time guard with per-system guards: WLED_PS_DONT_REPLACE_1D_FX and WLED_PS_DONT_REPLACE_2D_FX. Adjusted conditional includes and effect registration so 1D and 2D FX compile and register based on their respective guards only. No public APIs changed.

Changes

Cohort / File(s) Summary
FX guard refactoring
wled00/FX.cpp
Replaced the global WLED_PS_DONT_REPLACE_FX guard with per-system guards (WLED_PS_DONT_REPLACE_1D_FX, WLED_PS_DONT_REPLACE_2D_FX); added conditional pathways and helper defines to gate 1D vs 2D FX compilation; updated setupEffectData registrations and ESP-specific initialization guard logic.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Extra attention areas:
    • Ensure all 1D FX are guarded by WLED_PS_DONT_REPLACE_1D_FX and all 2D FX by WLED_PS_DONT_REPLACE_2D_FX
    • Verify setupEffectData registration conditions avoid missing or duplicate registrations
    • Check ESP8266/ESP32 branching and helper-define propagation for disabled/enabled particle-system builds

Possibly related PRs

Suggested reviewers

  • blazoncek
  • softhack007

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 52.94% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'refined PS replacement ifdefs' is concise and directly related to the main change, which involves refining preprocessor conditional guards for particle system replacement effects.
Linked Issues check ✅ Passed The PR addresses issue #5102 by implementing per-FX guards instead of broad global guards, enabling original 1D effects to compile when particle system is disabled without requiring the broad WLED_PS_DONT_REPLACE_FX guard.
Out of Scope Changes check ✅ Passed All changes are confined to conditional compilation guards and FX registration logic in FX.cpp, directly addressing the issue without introducing unrelated modifications.
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.

Copy link
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: 0

🧹 Nitpick comments (2)
wled00/FX.cpp (2)

2128-2215: Align Fire2012 / Exploding Fireworks guards between implementations and addEffect

mode_fire_2012 and mode_exploding_fireworks are compiled when either WLED_PS_DONT_REPLACE_1D_FX or WLED_PS_DONT_REPLACE_2D_FX is defined:

#if defined(WLED_PS_DONT_REPLACE_1D_FX) || defined(WLED_PS_DONT_REPLACE_2D_FX)
…
#endif

but the corresponding addEffect() registrations are only gated by WLED_PS_DONT_REPLACE_1D_FX. In a configuration where only WLED_PS_DONT_REPLACE_2D_FX is defined (e.g. 2D PS disabled but 1D PS enabled), these effects will be compiled but never registered, becoming dead code.

If the intent is to expose these FX whenever either dimension isn’t PS‑replaced, consider mirroring the OR condition in the registration block, or narrowing the implementation guards to 1D only. For example:

-#ifdef WLED_PS_DONT_REPLACE_1D_FX
+ #if defined(WLED_PS_DONT_REPLACE_1D_FX) || defined(WLED_PS_DONT_REPLACE_2D_FX)
   addEffect(FX_MODE_FIRE_2012, &mode_fire_2012, _data_FX_MODE_FIRE_2012);
   addEffect(FX_MODE_EXPLODING_FIREWORKS, &mode_exploding_fireworks, _data_FX_MODE_EXPLODING_FIREWORKS);
-#endif
+ #endif

This keeps behavior consistent across 1D/2D PS disable combinations and avoids compiled‑but‑unused implementations.

Also applies to: 3563-3701, 10890-10891


11016-11031: Indentation of PS‑1D addEffect block deviates from 2‑space style

Within setupEffectData(), the PS‑1D addEffect calls under #ifndef WLED_DISABLE_PARTICLESYSTEM1D are flush‑left, whereas the surrounding blocks use 2‑space indentation per wled00 C++ style. For consistency and readability, consider indenting these lines like the adjacent addEffect blocks:

-#ifndef WLED_DISABLE_PARTICLESYSTEM1D
-addEffect(FX_MODE_PSDRIP, &mode_particleDrip, _data_FX_MODE_PARTICLEDRIP);
-
-addEffect(FX_MODE_PS1DSPRINGY, &mode_particleSpringy, _data_FX_MODE_PS_SPRINGY);
-#endif // WLED_DISABLE_PARTICLESYSTEM1D
+ #ifndef WLED_DISABLE_PARTICLESYSTEM1D
+   addEffect(FX_MODE_PSDRIP, &mode_particleDrip, _data_FX_MODE_PARTICLEDRIP);
+
+   addEffect(FX_MODE_PS1DSPRINGY, &mode_particleSpringy, _data_FX_MODE_PS_SPRINGY);
+ #endif // WLED_DISABLE_PARTICLESYSTEM1D
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 54b7dfe and 7cc53e8.

📒 Files selected for processing (1)
  • wled00/FX.cpp (20 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
wled00/**/*.cpp

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use 2-space indentation for C++ source files (.cpp)

Files:

  • wled00/FX.cpp
🧠 Learnings (23)
📓 Common learnings
Learnt from: DedeHai
Repo: wled/WLED PR: 4798
File: wled00/FX.cpp:7531-7533
Timestamp: 2025-08-26T11:51:21.817Z
Learning: In WLED PR #4798, DedeHai confirmed that certain gamma-related calls in FX.cpp/FX_fcn.cpp/particle systems are intentional for effect-level shaping (e.g., brightness curves, TV sim, Pride 2015 pre-mix), distinct from final output gamma. Do not flag or remove these in future reviews; add comments when feasible to clarify intent.
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-11-14T13:37:11.994Z
Learning: In WLED code reviews, when code is modified or added, look for "magic numbers" (hardcoded numeric literals) and suggest replacing them with appropriate defined constants when those constants are meaningful in the context of the PR. For example, the hardcoded value 32 should be replaced with WLED_MAX_SEGNAME_LEN when it represents a segment name length limit. This improves code maintainability and reduces the risk of inconsistencies.
Learnt from: BobLoeffler68
Repo: wled/WLED PR: 4891
File: wled00/FX.cpp:3333-3349
Timestamp: 2025-08-31T03:38:14.114Z
Learning: WLED PacMan effect (wled00/FX.cpp): Keep pacmancharacters_t position fields as signed int (not int16_t). Maintainer preference (blazoncek) prioritizes avoiding potential overhead/regressions over minor RAM savings. Avoid type shrinking here unless memory pressure is demonstrated.
Learnt from: netmindz
Repo: wled/WLED PR: 4728
File: wled00/FX.h:378-378
Timestamp: 2025-06-15T09:59:52.720Z
Learning: In WLED's FX.h, MODE_COUNT represents the highest FX_MODE_ ID + 1, not the total count of FX_MODE_ definitions. This is because some effects may have been removed but their IDs are not reused, creating gaps in the numbering sequence. MODE_COUNT is used for array bounds and range checking.
Learnt from: netmindz
Repo: wled/WLED PR: 4728
File: wled00/FX.h:378-378
Timestamp: 2025-06-15T09:59:52.720Z
Learning: In WLED's FX.h, MODE_COUNT represents the highest FX_MODE_ ID + 1, not the total count of FX_MODE_ definitions. This is because some effects may have been removed but their IDs are not reused, creating gaps in the numbering sequence. MODE_COUNT is used for array bounds and range checking.
Learnt from: DedeHai
Repo: wled/WLED PR: 4926
File: wled00/FX.cpp:4727-4730
Timestamp: 2025-11-16T19:40:46.241Z
Learning: WLED AuroraWave (wled00/FX.cpp): wave_start and wave_end intentionally use int16_t; segments longer than 32k LEDs are not supported (bounded by MAX_LEDS), so widening to 32-bit is unnecessary.
Learnt from: DedeHai
Repo: wled/WLED PR: 4997
File: wled00/e131.cpp:33-44
Timestamp: 2025-10-10T18:34:06.550Z
Learning: In WLED's DDP packet handling (ws.cpp and e131.cpp), only prevent out-of-bounds memory access rather than enforcing DDP spec compliance. Don't check the 1440-byte spec limit—accept out-of-spec packets assuming correct encoding. The bounds check `maxDataIndex = c + numLeds * ddpChannelsPerLed; if (maxDataIndex > dataLen) reject` is sufficient and already covers the timecode flag case (when c=4) without needing separate validation.
Learnt from: mval-sg
Repo: wled/WLED PR: 4876
File: wled00/xml.cpp:0-0
Timestamp: 2025-08-28T08:09:20.630Z
Learning: The WLED codebase has opportunities for refactoring hardcoded array bounds (like the "15" used for DMX channels) to use sizeof(array)/sizeof(array[0]) for more maintainable code, but such changes should be done consistently across the entire codebase in a dedicated refactoring effort.
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-11-14T13:37:30.955Z
Learning: In WLED code reviews, when code is modified or added, look for "magic numbers" (hardcoded numeric literals) and suggest replacing them with defined constants when meaningful constants exist in the codebase. For example, suggest replacing hardcoded "32" with WLED_MAX_SEGNAME_LEN if the context relates to segment name length limits.
Learnt from: netmindz
Repo: wled/WLED PR: 4819
File: wled00/FX.cpp:10654-10655
Timestamp: 2025-08-08T17:31:47.330Z
Learning: In WLED core, avoid introducing AR-specific helper wrappers for um_data access. Keep um_data untyped and, when reading samplePeak, prefer `(*(uint8_t*)um_data->u_data[3]) != 0` over `*(bool*)` to avoid alignment/aliasing issues, while staying decoupled from the AudioReactive usermod.
📚 Learning: 2025-08-26T11:51:21.817Z
Learnt from: DedeHai
Repo: wled/WLED PR: 4798
File: wled00/FX.cpp:7531-7533
Timestamp: 2025-08-26T11:51:21.817Z
Learning: In WLED PR #4798, DedeHai confirmed that certain gamma-related calls in FX.cpp/FX_fcn.cpp/particle systems are intentional for effect-level shaping (e.g., brightness curves, TV sim, Pride 2015 pre-mix), distinct from final output gamma. Do not flag or remove these in future reviews; add comments when feasible to clarify intent.

Applied to files:

  • wled00/FX.cpp
📚 Learning: 2025-11-16T19:40:46.241Z
Learnt from: DedeHai
Repo: wled/WLED PR: 4926
File: wled00/FX.cpp:4727-4730
Timestamp: 2025-11-16T19:40:46.241Z
Learning: WLED AuroraWave (wled00/FX.cpp): wave_start and wave_end intentionally use int16_t; segments longer than 32k LEDs are not supported (bounded by MAX_LEDS), so widening to 32-bit is unnecessary.

Applied to files:

  • wled00/FX.cpp
📚 Learning: 2025-08-31T03:38:14.114Z
Learnt from: BobLoeffler68
Repo: wled/WLED PR: 4891
File: wled00/FX.cpp:3333-3349
Timestamp: 2025-08-31T03:38:14.114Z
Learning: WLED PacMan effect (wled00/FX.cpp): Keep pacmancharacters_t position fields as signed int (not int16_t). Maintainer preference (blazoncek) prioritizes avoiding potential overhead/regressions over minor RAM savings. Avoid type shrinking here unless memory pressure is demonstrated.

Applied to files:

  • wled00/FX.cpp
📚 Learning: 2025-06-15T09:59:52.720Z
Learnt from: netmindz
Repo: wled/WLED PR: 4728
File: wled00/FX.h:378-378
Timestamp: 2025-06-15T09:59:52.720Z
Learning: In WLED's FX.h, MODE_COUNT represents the highest FX_MODE_ ID + 1, not the total count of FX_MODE_ definitions. This is because some effects may have been removed but their IDs are not reused, creating gaps in the numbering sequence. MODE_COUNT is used for array bounds and range checking.

Applied to files:

  • wled00/FX.cpp
📚 Learning: 2025-11-14T13:37:11.994Z
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-11-14T13:37:11.994Z
Learning: In WLED code reviews, when code is modified or added, look for "magic numbers" (hardcoded numeric literals) and suggest replacing them with appropriate defined constants when those constants are meaningful in the context of the PR. For example, the hardcoded value 32 should be replaced with WLED_MAX_SEGNAME_LEN when it represents a segment name length limit. This improves code maintainability and reduces the risk of inconsistencies.

Applied to files:

  • wled00/FX.cpp
📚 Learning: 2025-11-14T13:37:30.955Z
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-11-14T13:37:30.955Z
Learning: In WLED code reviews, when code is modified or added, look for "magic numbers" (hardcoded numeric literals) and suggest replacing them with defined constants when meaningful constants exist in the codebase. For example, suggest replacing hardcoded "32" with WLED_MAX_SEGNAME_LEN if the context relates to segment name length limits.

Applied to files:

  • wled00/FX.cpp
📚 Learning: 2025-08-08T17:22:37.374Z
Learnt from: netmindz
Repo: wled/WLED PR: 4819
File: wled00/FX.cpp:10654-10655
Timestamp: 2025-08-08T17:22:37.374Z
Learning: In WLED FX.cpp, um_data returned by getAudioData() is intentionally untyped because different usermods provide different payloads, and simulateSound() may supply dummy data when the AudioReactive usermod isn’t built. Strongly typed accessors are not feasible; prefer explicit casts (e.g., samplePeak as uint8_t at u_data[3]) or small helpers that encapsulate these casts.

Applied to files:

  • wled00/FX.cpp
📚 Learning: 2025-09-16T18:08:42.848Z
Learnt from: DedeHai
Repo: wled/WLED PR: 4939
File: wled00/FX_fcn.cpp:1176-1187
Timestamp: 2025-09-16T18:08:42.848Z
Learning: In WLED finalizeInit() bus creation (wled00/FX_fcn.cpp), intentionally allowing memory overruns when bus configurations exceed MAX_LED_MEMORY is a deliberate design choice. The trade-off prioritizes creating buses with reduced LED counts over completely failing to create buses, which would cause no LED output and UI failures. This approach forces users to update configurations after migrating to version 0.16 while maintaining basic functionality.

Applied to files:

  • wled00/FX.cpp
📚 Learning: 2025-10-10T18:34:06.550Z
Learnt from: DedeHai
Repo: wled/WLED PR: 4997
File: wled00/e131.cpp:33-44
Timestamp: 2025-10-10T18:34:06.550Z
Learning: In WLED's DDP packet handling (ws.cpp and e131.cpp), only prevent out-of-bounds memory access rather than enforcing DDP spec compliance. Don't check the 1440-byte spec limit—accept out-of-spec packets assuming correct encoding. The bounds check `maxDataIndex = c + numLeds * ddpChannelsPerLed; if (maxDataIndex > dataLen) reject` is sufficient and already covers the timecode flag case (when c=4) without needing separate validation.

Applied to files:

  • wled00/FX.cpp
📚 Learning: 2025-08-29T00:26:15.808Z
Learnt from: ksedgwic
Repo: wled/WLED PR: 4883
File: usermods/usermod_v2_skystrip/rest_json_client.h:6-14
Timestamp: 2025-08-29T00:26:15.808Z
Learning: In WLED projects, ArduinoJson.h is not directly included via #include <ArduinoJson.h> - the ArduinoJson symbols are made available through the WLED build system and wled.h transitive includes, so explicitly adding #include <ArduinoJson.h> is not necessary and may not work.

Applied to files:

  • wled00/FX.cpp
📚 Learning: 2025-08-29T00:26:15.808Z
Learnt from: ksedgwic
Repo: wled/WLED PR: 4883
File: usermods/usermod_v2_skystrip/rest_json_client.h:6-14
Timestamp: 2025-08-29T00:26:15.808Z
Learning: WLED uses a vendored ArduinoJson library (version 6) located at "src/dependencies/json/ArduinoJson-v6.h" which is included through wled.h. Usermods should not directly include ArduinoJson headers but instead rely on wled.h for ArduinoJson symbols. The standard pattern is to include wled.h and use JsonObject, JsonArray, DynamicJsonDocument, etc. without additional includes.

Applied to files:

  • wled00/FX.cpp
📚 Learning: 2025-10-05T15:24:05.545Z
Learnt from: CR
Repo: wled/WLED PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-05T15:24:05.545Z
Learning: Applies to wled00/html_*.h : DO NOT edit generated embedded web header files (wled00/html_*.h)

Applied to files:

  • wled00/FX.cpp
📚 Learning: 2025-09-02T19:42:57.475Z
Learnt from: freakintoddles2
Repo: wled/WLED PR: 4904
File: wled00/FX.cpp:3915-3930
Timestamp: 2025-09-02T19:42:57.475Z
Learning: WLED wled00/FX.cpp::mode_percent — For time-based options (% of Minute/Hour/Day), behavior is clock-like: at rollover (elapsed==0) intentionally show SEGLEN lit for one tick to represent completion, then move to 1 LED at the next second. This is by design (not a progress bar) and should not be “fixed”.

Applied to files:

  • wled00/FX.cpp
📚 Learning: 2025-04-27T09:37:28.415Z
Learnt from: KrX3D
Repo: wled/WLED PR: 4585
File: usermods/seven_segment_display_reloaded_v2/seven_segment_display_reloaded_v2.h:121-136
Timestamp: 2025-04-27T09:37:28.415Z
Learning: Using PROGMEM for the seven-segment font array (umSSDRNumbers) in the WLED SSDR usermod causes compilation problems, so it should be left as a regular array.

Applied to files:

  • wled00/FX.cpp
📚 Learning: 2025-05-09T18:48:21.296Z
Learnt from: DedeHai
Repo: wled/WLED PR: 4682
File: wled00/FX.cpp:9016-9039
Timestamp: 2025-05-09T18:48:21.296Z
Learning: WLED does not support matrices large enough (≥1024 px) for 32-bit overflow in dx*dx + dy*dy to be a practical concern in particle system effects.

Applied to files:

  • wled00/FX.cpp
📚 Learning: 2025-10-20T09:41:50.374Z
Learnt from: blazoncek
Repo: wled/WLED PR: 4995
File: wled00/FX.cpp:5209-5212
Timestamp: 2025-10-20T09:41:50.374Z
Learning: In WLED 2D effects, SEGMENT.length() returns the physical pixel count (upper bound), while SEG_W * SEG_H can be smaller depending on mapping (mirroring/grouping). For per-cell state buffers (e.g., mode_2Dgameoflife in wled00/FX.cpp), allocate using SEGMENT.length() to ensure sufficient capacity and avoid reallocations, while bounding indexing by cols*rows.

Applied to files:

  • wled00/FX.cpp
📚 Learning: 2025-09-02T19:36:49.513Z
Learnt from: freakintoddles2
Repo: wled/WLED PR: 4904
File: wled00/FX.cpp:3901-3913
Timestamp: 2025-09-02T19:36:49.513Z
Learning: In FX.cpp mode_percent time-based options (% of Minute/Hour/Day), do not guard against invalid RTC; showing a full bar with unsynced time is intentional to avoid confusing UX.

Applied to files:

  • wled00/FX.cpp
📚 Learning: 2025-10-20T09:38:51.997Z
Learnt from: blazoncek
Repo: wled/WLED PR: 4995
File: wled00/FX.cpp:5223-5226
Timestamp: 2025-10-20T09:38:51.997Z
Learning: WLED matrices: each dimension (SEG_W, SEG_H) is limited to ≤255; 256 or larger per side is not supported/feasible on ESP32, so effects should assume per-dimension max 255.

Applied to files:

  • wled00/FX.cpp
📚 Learning: 2025-09-11T04:36:42.869Z
Learnt from: DedeHai
Repo: wled/WLED PR: 4754
File: usermods/user_fx/README.md:169-179
Timestamp: 2025-09-11T04:36:42.869Z
Learning: In WLED, SEGENV.data is already typed as uint8_t* and can be accessed directly without casting when storing per-pixel state data.

Applied to files:

  • wled00/FX.cpp
📚 Learning: 2025-11-14T05:48:44.673Z
Learnt from: DedeHai
Repo: wled/WLED PR: 5040
File: wled00/image_loader.cpp:84-96
Timestamp: 2025-11-14T05:48:44.673Z
Learning: In WLED (wled00/FX_2Dfcn.cpp), the Segment::setPixelColorXY() function performs internal bounds checking against vWidth() and vHeight(), returning early if coordinates are out of bounds. No additional guards are needed when calling this function, even in upscaling loops where coordinates might exceed segment dimensions.

Applied to files:

  • wled00/FX.cpp
📚 Learning: 2025-08-28T08:09:20.630Z
Learnt from: mval-sg
Repo: wled/WLED PR: 4876
File: wled00/xml.cpp:0-0
Timestamp: 2025-08-28T08:09:20.630Z
Learning: The WLED codebase has opportunities for refactoring hardcoded array bounds (like the "15" used for DMX channels) to use sizeof(array)/sizeof(array[0]) for more maintainable code, but such changes should be done consistently across the entire codebase in a dedicated refactoring effort.

Applied to files:

  • wled00/FX.cpp
📚 Learning: 2025-04-26T19:19:07.600Z
Learnt from: blazoncek
Repo: wled/WLED PR: 4658
File: wled00/const.h:140-141
Timestamp: 2025-04-26T19:19:07.600Z
Learning: In WLED, the WLED_MAX_PANELS macro is intentionally defined as a fixed constant value (18) with no redefinition mechanism, making it "unoverridable" - there's no need for a static assertion to check its maximum value.

Applied to files:

  • wled00/FX.cpp
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (16)
  • GitHub Check: wled_build / Build Environments (esp32_eth)
  • GitHub Check: wled_build / Build Environments (esp32s3_4M_qspi)
  • GitHub Check: wled_build / Build Environments (esp32s3dev_16MB_opi)
  • GitHub Check: wled_build / Build Environments (usermods)
  • GitHub Check: wled_build / Build Environments (esp32s3dev_8MB_opi)
  • GitHub Check: wled_build / Build Environments (esp01_1m_full_compat)
  • GitHub Check: wled_build / Build Environments (esp32dev_debug)
  • GitHub Check: wled_build / Build Environments (esp32_wrover)
  • GitHub Check: wled_build / Build Environments (nodemcuv2_compat)
  • GitHub Check: wled_build / Build Environments (lolin_s2_mini)
  • GitHub Check: wled_build / Build Environments (esp32c3dev_qio)
  • GitHub Check: wled_build / Build Environments (esp32c3dev)
  • GitHub Check: wled_build / Build Environments (esp32dev)
  • GitHub Check: wled_build / Build Environments (esp8266_2m_160)
  • GitHub Check: wled_build / Build Environments (nodemcuv2_160)
  • GitHub Check: wled_build / Build Environments (nodemcuv2)
🔇 Additional comments (4)
wled00/FX.cpp (4)

17-33: PS include logic and per-dimension replacement flags look correct

The new #if !(defined(WLED_DISABLE_PARTICLESYSTEM2D) && defined(WLED_DISABLE_PARTICLESYSTEM1D)) plus the WLED_PS_DONT_REPLACE_{1D,2D}_FX defines give the expected behavior for all combinations of the disable macros: FXparticleSystem is only included when at least one PS is enabled, and the appropriate *_DONT_REPLACE_* flag is set whenever that dimension’s PS is disabled (or both are). That matches the goal of restoring original 1D FX when WLED_DISABLE_PARTICLESYSTEM1D is used.


765-786: Granular 1D “don’t replace” guards around legacy FX are coherent

Wrapping the legacy 1D FX (sparkle, multi comet, rolling balls, glitter/solid glitter, starburst, dancing shadows) in #ifdef WLED_PS_DONT_REPLACE_1D_FX (or the combined 1D/2D guard for rolling_balls) cleanly decouples their compilation from the PS-based versions. Together with the new top-level defines, this ensures these effects are present when 1D PS is disabled or explicitly configured not to replace them, and hidden when PS-1D should fully own those slots.

Also applies to: 1762-1801, 3067-3165, 3223-3260, 3430-3561, 4350-4483


6045-6232: 2D Ghost Rider / Blobs replacement flags are wired as expected

Guarding the 2D Ghost Rider and 2D Blobs implementations and their addEffect() registrations with WLED_PS_DONT_REPLACE_2D_FX cleanly mirrors the 1D handling: when 2D PS is disabled (or configured not to replace 2D FX), the legacy 2D effects are present; when PS‑2D owns those modes, the originals are excluded to avoid overlap.

Also applies to: 10955-10958


7891-9312: Separate WLED_DISABLE_PARTICLESYSTEM{2D,1D} guards for PS FX are consistent

The new #ifndef WLED_DISABLE_PARTICLESYSTEM2D and #ifndef WLED_DISABLE_PARTICLESYSTEM1D blocks around the particle‑system‑based 2D and 1D FX, plus matching guards on the PS addEffect() registrations, make it possible to independently compile out each particle system. In particular, disabling only 1D PS while keeping 2D PS now correctly drops PS‑1D effects and restores the legacy 1D FX via the WLED_PS_DONT_REPLACE_1D_FX path, which addresses the behavior described in issue #5102.

Also applies to: 9317-10757, 11016-11031

@DedeHai DedeHai mentioned this pull request Nov 21, 2025
1 task
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.

Disabling 1D particle system does not restore original/replaced 1D effects

1 participant