-
-
Notifications
You must be signed in to change notification settings - Fork 8
1D effects x to y-axis conversion #81
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Back end ======== - blur1d: add optional column - MoonLight effects: RingEffect, Audiorings effect - WLED effects: Bouncing balls, tetrix: swap x,y - WLED effects: Rain: y-axis + random over x-axis (still a bug ...) - WLED effects: Drip swap xy and inverse y - WLED effects: DJLightEffect swap xy - Modifier: circle, Pinwheel1D, RippleYZModifier takes y-axis!
|
Note
|
| Cohort / File(s) | Summary |
|---|---|
Configuration platformio.ini |
APP_VERSION -> 0.7.1, APP_DATE -> 20251223 (build_flag metadata bump). |
Core Layer src/MoonLight/Layers/VirtualLayer.h |
void blur1d(fract8) → void blur1d(fract8, uint16_t x = 0); iteration changed to walk size.y and use Coord3D(x,i) (column-scoped blur). |
Effects — MoonLight src/MoonLight/Nodes/Effects/E_MoonLight.h |
Ring effects remapped to y-axis (Coord3D(0, ring)); RingRandomFlowEffect uses layer->size.y for hue arrays/loops; AudioRingsEffect removes nrOfRings member and computes rings at runtime from layer->size.y. |
Effects — WLED suite src/MoonLight/Nodes/Effects/E_WLED.h |
Broad axis reorientation: many effects (BouncingBalls, Tetrix, Rain, Drip, DJLight, Flow, Fireworks, etc.) switched primary iteration to size.x, coordinate writes adapted to Coord3D(...), per-column state added (e.g., Spark* drops, nrOfDrops) with destructors and onSizeChanged() reallocations; mode_fireworks() signature gained uint16_t x parameter. |
Modifiers src/MoonLight/Nodes/Modifiers/M_MoonLight.h |
Axis swaps in modifiers: CircleModifier now maps distance to y, PinwheelModifier petal assignment flipped in 3D branch, RippleYZModifier loop order and source pixel swapped (x/y inversion); minor whitespace edits in TransposeModifier. |
Sequence Diagram(s)
(No sequence diagram generated — changes are axis/structure reorientations and per-column state updates without a new multi-component sequential flow that warrants visualization.)
Estimated code review effort
🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
- Lots of effects #77 — Overlaps on axis/orientation changes and VirtualLayer/effects updates; likely touches the same mapping and effect logic.
- V070 preps #79 — Related axis handling fixes for BouncingBalls/Tetrix onSizeChanged; directly relevant to per-column/axis reorientation.
- Board presets p4 #74 — Touched
VirtualLayerand build metadata; potentially related to theblur1dsignature and platformio.ini version/date changes.
Pre-merge checks and finishing touches
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 20.45% which is insufficient. The required threshold is 80.00%. | You can run @coderabbitai generate docstrings to improve docstring coverage. |
✅ Passed checks (2 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The pull request title accurately describes the main objective of the changeset, which involves converting 1D effects from x-axis to y-axis orientation across multiple files. |
✨ Finishing touches
- 📝 Generate docstrings
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Post copyable unit tests in a comment
- Commit unit tests in branch
1d-effect-y-axis
📜 Recent review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/MoonLight/Nodes/Effects/E_WLED.h
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-12-18T15:15:57.828Z
Learnt from: ewowi
Repo: MoonModules/MoonLight PR: 0
File: :0-0
Timestamp: 2025-12-18T15:15:57.828Z
Learning: In DripEffect (E_WLED.h), dropsSize must be set to layer->size.y (not layer->size.x) in onSizeChanged(), because the drops array is allocated as [layer->size.y][maxNumDrops]. Using layer->size.x causes out-of-bounds access when iterating over y-coordinates.
Applied to files:
src/MoonLight/Nodes/Effects/E_WLED.h
📚 Learning: 2025-12-18T15:59:30.556Z
Learnt from: ewowi
Repo: MoonModules/MoonLight PR: 0
File: :0-0
Timestamp: 2025-12-18T15:59:30.556Z
Learning: In DripEffect (E_WLED.h), the bounce fails because the damped velocity is too weak to keep the drop at pos > 0 for multiple frames. When the drop returns to pos <= 0 while still in the bouncing state (colIndex == bouncing), it immediately resets to init (line 1663-1664) before any visible bounce occurs. The bounce velocity must be strong enough (divide by 1.1 or less, or use minimal damping like *= 0.9) so the drop stays airborne long enough for a visible bounce arc before the eventual second ground hit triggers the reset.
Applied to files:
src/MoonLight/Nodes/Effects/E_WLED.h
📚 Learning: 2025-12-18T15:46:48.319Z
Learnt from: ewowi
Repo: MoonModules/MoonLight PR: 0
File: :0-0
Timestamp: 2025-12-18T15:46:48.319Z
Learning: In DripEffect (E_WLED.h), the bounce damping at line 1669 needs to be `/1.5` or less (not `/4` or `/2`) to create a visible bounce effect. The continuous gravity application every frame (line 1652) requires a strong initial bounce velocity to overcome it. Using `/1.5` gives approximately 67% energy return, making the bounce visible and lasting for ~33 frames.
Applied to files:
src/MoonLight/Nodes/Effects/E_WLED.h
📚 Learning: 2025-12-18T15:30:02.486Z
Learnt from: ewowi
Repo: MoonModules/MoonLight PR: 0
File: :0-0
Timestamp: 2025-12-18T15:30:02.486Z
Learning: In DripEffect (E_WLED.h), the bounce at line 1669 uses heavy damping (`vel = -vel / 4`), which makes the bounce too weak to be visible. Reducing the damping factor from 4 to 2 (or lower) makes the bounce more pronounced and visible.
Applied to files:
src/MoonLight/Nodes/Effects/E_WLED.h
📚 Learning: 2025-12-07T14:16:03.228Z
Learnt from: ewowi
Repo: MoonModules/MoonLight PR: 0
File: :0-0
Timestamp: 2025-12-07T14:16:03.228Z
Learning: VirtualLayer mappingTableSize is uint16_t with a maximum of 65535 LEDs. If a user defines a space where size.x * size.y * size.z exceeds 65535, the code accepts the overflow and performs mapping partially. This is an explicit design decision balancing performance and memory constraints.
Applied to files:
src/MoonLight/Nodes/Effects/E_WLED.h
⏰ 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). (1)
- GitHub Check: build
🔇 Additional comments (8)
src/MoonLight/Nodes/Effects/E_WLED.h (8)
28-95: LGTM: BouncingBallsEffect correctly converted to per-column structure.The effect now allocates one ball array per column (layer->size.x), with proper bounds checking at line 59 using
MIN(layer->size.x, ballsSize). Each column's balls bounce independently using the correct coordinate mappingCoord3D(x, pos).
1017-1062: Verify: Past review comment about bounds checking may still apply.The past review comment suggested using
MIN(layer->size.x, nrOfDrops)in the loop condition (similar to BouncingBallsEffect at line 59). Currently line 1017 usesx < nrOfDropswithout the MIN check. WhilenrOfDropsis set tolayer->size.xon successful allocation (line 997), using MIN would be more defensive against edge cases.🔎 Optional defensive improvement
- for (int x = 0; x < nrOfDrops; x++) { + for (int x = 0; x < MIN(layer->size.x, nrOfDrops); x++) { if (drops[x].step == 0) { // init brick
1735-1754: LGTM: RainEffect correctly converted to per-column state.The effect now allocates per-column drop state (
Spark* drops) sized tolayer->size.xwith proper destructor cleanup and initialization. The dimension change from 1D to column-major 2D is well-implemented.
1811-1830: Note: DripEffect dimension changed from learning context.The retrieved learning mentions
dropsSizeshould belayer->size.y, but after the y-axis conversion in this PR, drops are now correctly allocated per-column (layer->size.x). This is intentional - each column now has its own set of drops falling along the y-axis. The loop at line 1842 correctly iteratesx < nrOfDropswherenrOfDrops == layer->size.x.Based on learnings, but noting the axis conversion changes the expected dimension.
1641-1657: LGTM: FlowEffect correctly converted to vertical (y-axis) flow.The effect now calculates zones along
layer->size.y(lines 1641-1646) and renders atCoord3D(0, pos + led), making it a single-column vertical effect. This aligns with the PR's goal of y-axis orientation.
1661-1719: LGTM: mode_fireworks correctly updated for per-column processing.The function signature now accepts an
xparameter (line 1661), and all coordinate accesses correctly useCoord3D(x, ...)(lines 1671-1672, 1702-1703, 1710-1712). The past review concern about hardcoded column 0 has been resolved.
1964-2010: LGTM: DJLightEffect correctly converted to vertical single-column effect.The effect now calculates the midpoint from
layer->size.y(line 1964) and renders/shifts pixels vertically atCoord3D(0, ...)(lines 2004-2007). The conversion to y-axis orientation is correct.
1938-1941: LGTM: HeartBeatEffect correctly converted to vertical single-column effect.The effect now iterates over
layer->size.y(line 1938) and renders atCoord3D(0, i)(line 1939), correctly implementing vertical heartbeat visualization.
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 help to get the list of available commands and usage tips.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/MoonLight/Nodes/Effects/E_WLED.h (1)
1844-1899: DripEffect: Verify all Coord3D usage and boundary conditions.The outer loop (line 1844) iterates over columns, and the inner loop (line 1845) iterates over drops per column. All LED access uses
Coord3D(x, ...)which is correct for per-column state.However, line 1878 has a concerning comment and constraint:
uint16_t pos = constrain(drops[x][j].pos + i, layer->size.y - 1, 0); // this is BAD, returns a pos >= layer->size.x occasionallyThe constrain call has swapped min/max parameters! It should be
constrain(value, min, max)but this hasconstrain(value, max, min). This will always return eitherlayer->size.y - 1or0, which is likely not the intent.🔎 Proposed fix
for (int i = 1; i < 7 - drops[x][j].colIndex; i++) { // some minor math so we don't expand bouncing droplets - uint16_t pos = constrain(drops[x][j].pos + i, layer->size.y - 1, 0); // this is BAD, returns a pos >= layer->size.x occasionally + uint16_t pos = constrain(drops[x][j].pos + i, 0, layer->size.y - 1); // constrain to valid y range layer->setRGB(Coord3D(x, pos), blend(CRGB::Black, dropColor, drops[x][j].col / i)); // spread pixel with fade while falling }
🧹 Nitpick comments (1)
src/MoonLight/Nodes/Effects/E_MoonLight.h (1)
1609-1615: AudioRingsEffect: nrOfRings now dynamically computed, user control removed.The nrOfRings slider control is commented out (line 1611), and the value is now computed dynamically as
MAX(layer->size.y, 2)in the loop (line 1615). This makes the number of rings adapt to the layer height automatically but removes user configurability.Is this removal of user control intentional? If not, consider keeping the slider but using it as a maximum/multiplier rather than commenting it out entirely.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
platformio.inisrc/MoonLight/Layers/VirtualLayer.hsrc/MoonLight/Nodes/Effects/E_MoonLight.hsrc/MoonLight/Nodes/Effects/E_WLED.hsrc/MoonLight/Nodes/Modifiers/M_MoonLight.h
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-12-07T14:16:03.228Z
Learnt from: ewowi
Repo: MoonModules/MoonLight PR: 0
File: :0-0
Timestamp: 2025-12-07T14:16:03.228Z
Learning: VirtualLayer mappingTableSize is uint16_t with a maximum of 65535 LEDs. If a user defines a space where size.x * size.y * size.z exceeds 65535, the code accepts the overflow and performs mapping partially. This is an explicit design decision balancing performance and memory constraints.
Applied to files:
src/MoonLight/Layers/VirtualLayer.hsrc/MoonLight/Nodes/Effects/E_MoonLight.h
📚 Learning: 2025-12-18T15:15:57.828Z
Learnt from: ewowi
Repo: MoonModules/MoonLight PR: 0
File: :0-0
Timestamp: 2025-12-18T15:15:57.828Z
Learning: In DripEffect (E_WLED.h), dropsSize must be set to layer->size.y (not layer->size.x) in onSizeChanged(), because the drops array is allocated as [layer->size.y][maxNumDrops]. Using layer->size.x causes out-of-bounds access when iterating over y-coordinates.
Applied to files:
src/MoonLight/Nodes/Modifiers/M_MoonLight.hsrc/MoonLight/Nodes/Effects/E_MoonLight.hsrc/MoonLight/Nodes/Effects/E_WLED.h
📚 Learning: 2025-12-18T15:59:30.556Z
Learnt from: ewowi
Repo: MoonModules/MoonLight PR: 0
File: :0-0
Timestamp: 2025-12-18T15:59:30.556Z
Learning: In DripEffect (E_WLED.h), the bounce fails because the damped velocity is too weak to keep the drop at pos > 0 for multiple frames. When the drop returns to pos <= 0 while still in the bouncing state (colIndex == bouncing), it immediately resets to init (line 1663-1664) before any visible bounce occurs. The bounce velocity must be strong enough (divide by 1.1 or less, or use minimal damping like *= 0.9) so the drop stays airborne long enough for a visible bounce arc before the eventual second ground hit triggers the reset.
Applied to files:
src/MoonLight/Nodes/Effects/E_WLED.h
📚 Learning: 2025-12-18T15:46:48.319Z
Learnt from: ewowi
Repo: MoonModules/MoonLight PR: 0
File: :0-0
Timestamp: 2025-12-18T15:46:48.319Z
Learning: In DripEffect (E_WLED.h), the bounce damping at line 1669 needs to be `/1.5` or less (not `/4` or `/2`) to create a visible bounce effect. The continuous gravity application every frame (line 1652) requires a strong initial bounce velocity to overcome it. Using `/1.5` gives approximately 67% energy return, making the bounce visible and lasting for ~33 frames.
Applied to files:
src/MoonLight/Nodes/Effects/E_WLED.h
📚 Learning: 2025-12-18T15:30:02.486Z
Learnt from: ewowi
Repo: MoonModules/MoonLight PR: 0
File: :0-0
Timestamp: 2025-12-18T15:30:02.486Z
Learning: In DripEffect (E_WLED.h), the bounce at line 1669 uses heavy damping (`vel = -vel / 4`), which makes the bounce too weak to be visible. Reducing the damping factor from 4 to 2 (or lower) makes the bounce more pronounced and visible.
Applied to files:
src/MoonLight/Nodes/Effects/E_WLED.h
🧬 Code graph analysis (2)
src/MoonLight/Nodes/Modifiers/M_MoonLight.h (1)
src/MoonBase/Utilities.h (1)
Coord3D(151-163)
src/MoonLight/Nodes/Effects/E_WLED.h (2)
src/MoonLight/Nodes/Effects/E_MoonLight.h (18)
dim(17-17)dim(45-45)dim(108-108)dim(141-141)dim(155-155)dim(191-191)dim(307-307)dim(338-338)dim(376-376)dim(450-450)dim(498-498)dim(567-567)dim(663-663)dim(966-966)dim(1202-1202)dim(1267-1267)onSizeChanged(1210-1216)onSizeChanged(1578-1584)src/MoonLight/Nodes/Effects/E_MoonModules.h (1)
onSizeChanged(183-199)
⏰ 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). (1)
- GitHub Check: build
🔇 Additional comments (13)
platformio.ini (1)
58-59: Version and date bump looks good.The patch version increment (0.7.0 → 0.7.1) is appropriate for the refactoring changes across effects and modifiers described in the PR. Date format is consistent with the existing convention.
src/MoonLight/Layers/VirtualLayer.h (1)
214-229: LGTM! Column-scoped blur implementation is correct.The blur1d function now operates on a vertical column (specified by x parameter with default 0), iterating over size.y and using Coord3D(x, i) for all LED access. The carryover logic and blur calculations are preserved correctly from the original implementation.
src/MoonLight/Nodes/Modifiers/M_MoonLight.h (2)
236-242: RippleYZModifier loop order inverted - verify correctness.The 1D->2D ripple traversal changed from iterating y-outer/x-inner with source
(x, y-1)to iterating x-outer/y-inner with source(x-1, y). This fundamentally changes the ripple direction from vertical-to-horizontal to horizontal-to-vertical propagation. The comment update reflects this, but ensure this matches the intended behavior for effects using this modifier.The axis inversion appears intentional and consistent with the PR's goal of reorienting 1D effects to the y-axis.
149-150: The PinwheelModifier 1D axis assignment is correct and aligns with the codebase. The git history confirms this was an intentional refactor in v0.7.1 ("1D effects x to y-axis conversion"), where 1D effects were standardized to use the y-axis dimension. The size assignment (size.x = 1; size.y = petals) is consistent with how other 1D-to-2D/3D modifiers like CircleModifier and RippleYZModifier handle axis expansion, and themodifyPosition()logic correctly remaps the petal index to the x-axis with y fixed at 0, matching the VirtualLayer rendering pattern for 1D effects.src/MoonLight/Nodes/Effects/E_MoonLight.h (2)
1561-1562: LGTM! Ring addressing correctly updated for 1D-on-y-axis.The setRing method now uses
Coord3D(0, ring)to place rings along the y-axis in a 1D-y interpretation, which is consistent with the PR's goal of converting 1D effects to use the y-axis.
1578-1592: LGTM! Hue allocation and loop bounds correctly aligned with size.y.The changes correctly allocate hue memory based on
layer->size.yand update all loop bounds to iterate oversize.yinstead ofsize.x. This matches the learned constraint that arrays must be allocated according to the dimension they index.src/MoonLight/Nodes/Effects/E_WLED.h (7)
34-42: LGTM! BouncingBallsEffect correctly updated for per-column state.The balls array is now allocated per column (
layer->size.x) andballsSizeis set accordingly. This aligns with the new column-based approach where each x-coordinate maintains its own ball state.
59-94: Verify loop bounds match ballsSize to prevent out-of-bounds access.The outer loop iterates
x < MIN(layer->size.x, ballsSize)(line 59), which correctly guards against accessing unallocated memory if reallocation failed. The ball position calculation and rendering logic look correct for the per-column approach.
993-1006: LGTM! TetrixEffect drops allocation aligned with per-column state.The drops array is correctly allocated for
layer->size.xcolumns (line 994) andnrOfDropsis set to match (line 997). Each column will maintain its own drop state, which is consistent with the 2D axis realignment.
1737-1756: RainEffect: Per-column drops allocation is correct for new axis orientation.The drops array is now allocated per column (
layer->size.xat line 1743) instead of per row, which aligns with the PR's goal of having each column maintain its own drop state. The initialization loop (lines 1748-1752) correctly iterates over x and initializes each column's drop.Note: The retrieved learning about
dropsSize = layer->size.yapplies to the OLD code structure. The NEW per-column approach usingsize.xis correct for this refactoring.
1818-1832: DripEffect: Per-column drops allocation is correct.The drops array is correctly allocated as
[layer->size.x][maxNumDrops](line 1819), withnrOfDropsset tolayer->size.x(line 1823). Each column maintains up tomaxNumDropsdrops independently. The initialization loop (lines 1824-1828) correctly iterates over both dimensions.
1965-2010: DJLightEffect: mid calculation uses size.y - verify this aligns with effect intent.Line 1966 calculates
mid = layer->size.y / 2, and the effect usesCoord3D(0, mid)for pixel access (line 2006) and shifts within column 0 (lines 2008-2009). This changes the effect from operating horizontally across x to operating vertically along y. Verify this is the intended behavior for the "DJLight" effect.The change appears consistent with the PR's goal of reorienting 1D effects to the y-axis.
1663-1721: mode_fireworks signature verified—all callers correctly pass x parameter.The function signature change to include the x parameter is correctly implemented. RainEffect (the only caller at line 1789) properly passes x as the loop variable from the
for (int x = 0; x < nrOfDrops; x++)loop at line 1762, and all LED access usingCoord3D(x, ...)is correct.
There was a problem hiding this 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
♻️ Duplicate comments (2)
src/MoonLight/Nodes/Effects/E_WLED.h (2)
1774-1783: Critical: Column-shift logic reads from wrong column and resets wrong field.This is the same issue flagged in the previous review. Two problems remain:
- Lines 1774-1777: The pixel shift reads from column 0 (
Coord3D(0, ...)) instead of the current columnx, causing all columns to display the same shifted pixels.- Line 1783: Resets
drops[x].posinstead ofdrops[x].colwhendrops[x].col <= 0.🔎 Proposed fix
{ // shift all leds down - CRGB ctemp = layer->getRGB(Coord3D(x, 0)); + CRGB ctemp = layer->getRGB(Coord3D(x, 0)); for (int i = 0; i < layer->size.y - 1; i++) { - layer->setRGB(Coord3D(x, layer->size.y - 1 - i), layer->getRGB(Coord3D(0, layer->size.y - 1 - (i + 1)))); + layer->setRGB(Coord3D(x, layer->size.y - 1 - i), layer->getRGB(Coord3D(x, layer->size.y - 1 - (i + 1)))); } layer->setRGB(Coord3D(x, 0), ctemp); // wrap around drops[x].pos++; // increase spark index drops[x].col++; } if (drops[x].pos <= 0) drops[x].pos = UINT16_MAX; // reset previous spark position - if (drops[x].col <= 0) drops[x].pos = UINT16_MAX; // reset previous spark position + if (drops[x].col <= 0) drops[x].col = UINT16_MAX; // reset previous spark position if (drops[x].pos >= layer->size.y) drops[x].pos = 0; // ignore
1017-1019: Verify drops array bounds in TetrixEffect loop.This issue was flagged in the previous review. The loop iterates over
nrOfDropswhich equalslayer->size.xafter successful allocation, but if reallocation fails or is incomplete, accesses todrops[x]could exceed the allocated size.Consider using the defensive pattern from BouncingBallsEffect at line 59:
for (int x = 0; x < MIN(layer->size.x, nrOfDrops); x++) {🔎 Proposed fix
- for (int x = 0; x < nrOfDrops; x++) { + for (int x = 0; x < MIN(layer->size.x, nrOfDrops); x++) { if (drops[x].step == 0) { // init brick
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/MoonLight/Nodes/Effects/E_WLED.h
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-12-18T15:15:57.828Z
Learnt from: ewowi
Repo: MoonModules/MoonLight PR: 0
File: :0-0
Timestamp: 2025-12-18T15:15:57.828Z
Learning: In DripEffect (E_WLED.h), dropsSize must be set to layer->size.y (not layer->size.x) in onSizeChanged(), because the drops array is allocated as [layer->size.y][maxNumDrops]. Using layer->size.x causes out-of-bounds access when iterating over y-coordinates.
Applied to files:
src/MoonLight/Nodes/Effects/E_WLED.h
📚 Learning: 2025-12-18T15:59:30.556Z
Learnt from: ewowi
Repo: MoonModules/MoonLight PR: 0
File: :0-0
Timestamp: 2025-12-18T15:59:30.556Z
Learning: In DripEffect (E_WLED.h), the bounce fails because the damped velocity is too weak to keep the drop at pos > 0 for multiple frames. When the drop returns to pos <= 0 while still in the bouncing state (colIndex == bouncing), it immediately resets to init (line 1663-1664) before any visible bounce occurs. The bounce velocity must be strong enough (divide by 1.1 or less, or use minimal damping like *= 0.9) so the drop stays airborne long enough for a visible bounce arc before the eventual second ground hit triggers the reset.
Applied to files:
src/MoonLight/Nodes/Effects/E_WLED.h
📚 Learning: 2025-12-18T15:46:48.319Z
Learnt from: ewowi
Repo: MoonModules/MoonLight PR: 0
File: :0-0
Timestamp: 2025-12-18T15:46:48.319Z
Learning: In DripEffect (E_WLED.h), the bounce damping at line 1669 needs to be `/1.5` or less (not `/4` or `/2`) to create a visible bounce effect. The continuous gravity application every frame (line 1652) requires a strong initial bounce velocity to overcome it. Using `/1.5` gives approximately 67% energy return, making the bounce visible and lasting for ~33 frames.
Applied to files:
src/MoonLight/Nodes/Effects/E_WLED.h
📚 Learning: 2025-12-18T15:30:02.486Z
Learnt from: ewowi
Repo: MoonModules/MoonLight PR: 0
File: :0-0
Timestamp: 2025-12-18T15:30:02.486Z
Learning: In DripEffect (E_WLED.h), the bounce at line 1669 uses heavy damping (`vel = -vel / 4`), which makes the bounce too weak to be visible. Reducing the damping factor from 4 to 2 (or lower) makes the bounce more pronounced and visible.
Applied to files:
src/MoonLight/Nodes/Effects/E_WLED.h
⏰ 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). (1)
- GitHub Check: build
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Summary by CodeRabbit
Version Update
Improvements
Behavior Changes
✏️ Tip: You can customize this high-level summary in your review settings.