Introduce comet effect usermod with fire particle system#5347
Introduce comet effect usermod with fire particle system#5347gustebeast wants to merge 1 commit intowled:mainfrom
Conversation
WalkthroughIntroduces a new PS Comet usermod that implements a falling comet particle effect using 2D particle systems. The implementation includes initialization, per-frame updates, spawning logic, and particle lifecycle management with configurable parameters via UI sliders and palette selection. Includes documentation and library manifest. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 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}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@usermods/PS_Comet/PS_Comet.cpp`:
- Around line 23-60: Guard against division-by-zero when computing pixel mapping
and vertical shifts by using a safe denominator (e.g., uint32_t denomW = max(1u,
SEGMENT.vWidth() - 1) and uint32_t denomH = max(1u, SEGMENT.vHeight() - 1)) and
replace any uses of (SEGMENT.vWidth() - 1) / (SEGMENT.vHeight() - 1) with those
safe denominators; fix the 3x->column mapping in mode_pscomet by computing
baseCol = i / 3, offset = (int)(i % 3) - 1 (so center = 0, left = -1, right =
+1), then targetCol = clamp(baseCol + offset, 0u, SEGMENT.vWidth() - 1) and set
sourceParticle.x = targetCol * PartSys->maxX / denomW; similarly compute
sideCometVerticalShift using denomH (uint32_t sideCometVerticalShift = 2 *
PartSys->maxY / denomH) and ensure all divisions use the safe denominators and
clamp results to valid ranges to avoid out-of-bounds writes.
In `@usermods/PS_Comet/README.md`:
- Line 3: Fix the spelling mistake "palletes" to "palettes" in the README
sentence describing color support (the phrase "Works with custom color palletes,
defaulting to "Fire""). Update that phrase to read "Works with custom color
palettes, defaulting to 'Fire'".
🧹 Nitpick comments (1)
usermods/PS_Comet/PS_Comet.cpp (1)
4-4: Avoid global spawn timer to prevent cross‑segment coupling.
nextCometCreationTimeis global, so multiple segments running PS Comet will share a single spawn cadence. Consider storing this per segment (e.g., in SEGENV.aux0/aux1 or a small state struct tied to SEGENV.data) to keep animations independent.
| uint16_t mode_pscomet(void) { | ||
| ParticleSystem2D *PartSys = nullptr; | ||
| uint32_t i; | ||
| // Allocate three comets for every column to allow for large comets with side emitters | ||
| uint32_t numComets = SEGMENT.vWidth() * 3; | ||
|
|
||
| if (SEGMENT.call == 0) { // Initialization | ||
| if (!initParticleSystem2D(PartSys, numComets)) { | ||
| return mode_static(); // Allocation failed or not 2D | ||
| } | ||
| PartSys->setMotionBlur(170); // Enable motion blur | ||
| PartSys->setParticleSize(0); // Allow small comets to be a single pixel wide | ||
| } | ||
| else { | ||
| PartSys = reinterpret_cast<ParticleSystem2D *>(SEGENV.data); // If not first call, use existing data | ||
| } | ||
| if (PartSys == nullptr) { | ||
| return mode_static(); // Something went wrong, no data! | ||
| } | ||
|
|
||
| PartSys->updateSystem(); // Update system properties (dimensions and data pointers) | ||
|
|
||
| uint32_t sideCometVerticalShift = 2 * PartSys->maxY / (SEGMENT.vHeight() - 1); // Shift emitters on left and right of comet up by 1 pixel | ||
| uint32_t chosenIndex = hw_random(numComets); // Only allow one column to spawn a comet each frame | ||
| uint8_t fallingSpeed = 1 + (SEGMENT.speed >> 2); | ||
| uint16_t cometFrequencyDelay = 2040 - (SEGMENT.intensity << 3); | ||
|
|
||
| // Update the comets | ||
| for (i = 0; i < numComets; i++) { | ||
| auto& source = PartSys->sources[i]; | ||
| auto& sourceParticle = source.source; | ||
| // Large comets use three particle sources, with i indicating left, center or right: | ||
| // 0: left, 1: center, 2: right, 3: left, 4: center, 5: right, ... | ||
| // Small comets leave the left and right indices unused | ||
| bool isCometCenter = (i % 3) == 1; | ||
|
|
||
| // Map the 3x comet index into an output pixel index | ||
| sourceParticle.x = ((i / 3) + (i % 3)) * PartSys->maxX / (SEGMENT.vWidth() - 1); |
There was a problem hiding this comment.
Guard 1‑pixel dimensions and fix column mapping.
When vWidth or vHeight is 1, (vWidth - 1) / (vHeight - 1) becomes zero and will crash. Also ((i / 3) + (i % 3)) shifts center columns right and can exceed the last column, so some columns never receive center comets and x can go out of range.
🛠️ Proposed fix
uint16_t mode_pscomet(void) {
ParticleSystem2D *PartSys = nullptr;
uint32_t i;
- // Allocate three comets for every column to allow for large comets with side emitters
- uint32_t numComets = SEGMENT.vWidth() * 3;
+ const uint32_t vWidth = SEGMENT.vWidth();
+ const uint32_t vHeight = SEGMENT.vHeight();
+ if (vWidth < 2 || vHeight < 2) {
+ return mode_static();
+ }
+ // Allocate three comets for every column to allow for large comets with side emitters
+ uint32_t numComets = vWidth * 3;
@@
- uint32_t sideCometVerticalShift = 2 * PartSys->maxY / (SEGMENT.vHeight() - 1); // Shift emitters on left and right of comet up by 1 pixel
+ uint32_t sideCometVerticalShift = 2 * PartSys->maxY / (vHeight - 1); // Shift emitters on left and right of comet up by 1 pixel
@@
- sourceParticle.x = ((i / 3) + (i % 3)) * PartSys->maxX / (SEGMENT.vWidth() - 1);
+ const uint32_t baseCol = i / 3;
+ const int8_t offset = (int8_t)(i % 3) - 1; // left, center, right
+ const int32_t col = constrain((int32_t)baseCol + offset, 0, (int32_t)vWidth - 1);
+ sourceParticle.x = (uint32_t)col * PartSys->maxX / (vWidth - 1);🤖 Prompt for AI Agents
In `@usermods/PS_Comet/PS_Comet.cpp` around lines 23 - 60, Guard against
division-by-zero when computing pixel mapping and vertical shifts by using a
safe denominator (e.g., uint32_t denomW = max(1u, SEGMENT.vWidth() - 1) and
uint32_t denomH = max(1u, SEGMENT.vHeight() - 1)) and replace any uses of
(SEGMENT.vWidth() - 1) / (SEGMENT.vHeight() - 1) with those safe denominators;
fix the 3x->column mapping in mode_pscomet by computing baseCol = i / 3, offset
= (int)(i % 3) - 1 (so center = 0, left = -1, right = +1), then targetCol =
clamp(baseCol + offset, 0u, SEGMENT.vWidth() - 1) and set sourceParticle.x =
targetCol * PartSys->maxX / denomW; similarly compute sideCometVerticalShift
using denomH (uint32_t sideCometVerticalShift = 2 * PartSys->maxY / denomH) and
ensure all divisions use the safe denominators and clamp results to valid ranges
to avoid out-of-bounds writes.
| @@ -0,0 +1,25 @@ | |||
| ## Description | |||
|
|
|||
| A 2D falling comet effect similar to "Matrix" but with a fire particle simulation to enhance the comet trail visuals. Works with custom color palletes, defaulting to "Fire". Supports "small" and "large" comets which are 1px and 3px wide respectively. | |||
There was a problem hiding this comment.
Fix spelling: “palettes”.
User-facing docs should avoid typos.
📝 Proposed fix
-A 2D falling comet effect similar to "Matrix" but with a fire particle simulation to enhance the comet trail visuals. Works with custom color palletes, defaulting to "Fire". Supports "small" and "large" comets which are 1px and 3px wide respectively.
+A 2D falling comet effect similar to "Matrix" but with a fire particle simulation to enhance the comet trail visuals. Works with custom color palettes, defaulting to "Fire". Supports "small" and "large" comets which are 1px and 3px wide respectively.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| A 2D falling comet effect similar to "Matrix" but with a fire particle simulation to enhance the comet trail visuals. Works with custom color palletes, defaulting to "Fire". Supports "small" and "large" comets which are 1px and 3px wide respectively. | |
| A 2D falling comet effect similar to "Matrix" but with a fire particle simulation to enhance the comet trail visuals. Works with custom color palettes, defaulting to "Fire". Supports "small" and "large" comets which are 1px and 3px wide respectively. |
🧰 Tools
🪛 LanguageTool
[grammar] ~3-~3: Ensure spelling is correct
Context: ... trail visuals. Works with custom color palletes, defaulting to "Fire". Supports "small"...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 Prompt for AI Agents
In `@usermods/PS_Comet/README.md` at line 3, Fix the spelling mistake "palletes"
to "palettes" in the README sentence describing color support (the phrase "Works
with custom color palletes, defaulting to "Fire""). Update that phrase to read
"Works with custom color palettes, defaulting to 'Fire'".
|
looks good in general, I'll add some comments to consider |
|
|
||
| unsigned long nextCometCreationTime = 0; | ||
|
|
||
| static bool has_particle_fallen_off_screen(const ParticleSystem2D* PartSys, const PSparticle& particle) { |
There was a problem hiding this comment.
if you enable gravity and enable killoutofbounds and let the PS handle the movement of your sources, you do not need this function and can just check if TTL=0. you can set gravity to 1 for a very slight acceleration only if that is not disturbing your intentions.
another option would be to set the initial TTL accordingly so your sources die once off-screen.
or just do this check inside the FX, or add a lambda function.
|
|
||
| // Update the comets | ||
| for (i = 0; i < numComets; i++) { | ||
| auto& source = PartSys->sources[i]; |
There was a problem hiding this comment.
do not assume that requested sources = allocated sources. use numsources instead to avoid crashes.
|
|
||
| if (!hasFallenOffScreen) { | ||
| // Active comets fall downwards and emit flames | ||
| sourceParticle.y -= fallingSpeed; |
There was a problem hiding this comment.
if you want to you could also use vy=-fallingSpeed and let the PS handle the movement, that way you could add gravity as an option. see my PS FX for examples on that.
| for (i = 0; i < PartSys->usedParticles; i++) { | ||
| auto& particle = PartSys->particles[i]; | ||
| if (!has_particle_fallen_off_screen(PartSys, particle)) { | ||
| particle.y -= fallingSpeed; |
There was a problem hiding this comment.
why not make the emit speed handle this? the downside would be that the speed would not react immediately to to a slider change.
| ``` | ||
| Or if you are already using a usermod, append ps_comet to the list | ||
| ```ini | ||
| custom_usermods = audioreactive, ps_comet |
|
I gave it a quick spin. Parameters may need some tuning: set all sliders except speed to max and it breaks the effect. |
3 participants
A 2D falling comet effect similar to "Matrix" but with a fire particle simulation to enhance the comet trail visuals. Works with custom color palletes, defaulting to "Fire". Supports "small" and "large" comets which are 1px and 3px wide respectively.
Demo: https://imgur.com/a/i1v5WAy
Summary by CodeRabbit