Skip to content
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

Added new effect usermod - Diffusion Fire #4473

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

mryndzionek
Copy link

@mryndzionek mryndzionek commented Jan 9, 2025

Usermod adding new fire effect. Has more parameters than other fire effects and can be widely adjusted/tweaked to look good even on small matrices.
fire

@DedeHai
Copy link
Collaborator

DedeHai commented Jan 9, 2025

thanks for contributing!
it really looks a lot nicer on a small matrix.
There is a pending PR for my particle system which hopefully will be finished soon and it looks even better, almost independent of matrix size and it will most likely replace the current fire FX.
I will keep this PR open for now. You are welcome to check out the particle effects (if you do, use my dev branch)

@mryndzionek
Copy link
Author

thanks for contributing! it really looks a lot nicer on a small matrix. There is a pending PR for my particle system which hopefully will be finished soon and it looks even better, almost independent of matrix size and it will most likely replace the current fire FX. I will keep this PR open for now. You are welcome to check out the particle effects (if you do, use my dev branch)

The particle system seems to be a bit heavy. Is it even possible to compile it on 4MB ESP32? My effect is just a simple usermod, so I think it's independent/complementary to the particle system.

@mryndzionek
Copy link
Author

Okay, I managed to compile and flash the particle system branch and I think that my fire effect is different enough to warrant a merge, especially when it's a usermod, so conditionally compiled.

@DedeHai
Copy link
Collaborator

DedeHai commented Jan 15, 2025

The reason I did not yet approve this is the following:
the compile tools (discord bot, online compilers) need to be maintained, every usermod added increases the work. This FX may not be used by many (it only really looks good up to about 12x12 pixels IMHO), dedicating a usermod to simple effects complicates things.
So here is what we thought: for effects like yours it would be good to have a collection of "user FX" instead of having a usermod for each one. Since you are the first to commit such an effect: would you mind to rename the Usermod to "user FX" (I am not too sure about the name) where others can also contribute FX to and they can all be compiled in with a single UM addition?
I was even thinking of making a second UM with "legacy FX" for effects that were replaced or dropped out but users still may like.

@mryndzionek
Copy link
Author

The reason I did not yet approve this is the following: the compile tools (discord bot, online compilers) need to be maintained, every usermod added increases the work. This FX may not be used by many (it only really looks good up to about 12x12 pixels IMHO), dedicating a usermod to simple effects complicates things. So here is what we thought: for effects like yours it would be good to have a collection of "user FX" instead of having a usermod for each one. Since you are the first to commit such an effect: would you mind to rename the Usermod to "user FX" (I am not too sure about the name) where others can also contribute FX to and they can all be compiled in with a single UM addition? I was even thinking of making a second UM with "legacy FX" for effects that were replaced or dropped out but users still may like.

Okay, I renamed to user_fx.

@DedeHai
Copy link
Collaborator

DedeHai commented Jan 15, 2025

thanks!
can you add a readme file with a short description of what this UM is for (just one or two sentences that user FX go into this UM) and what your FX does.
Also:

  • can you update the formating to match the WLED standard (I think close to google standard) with opening curly brackets on the same line to make code more compact.
  • use fixed point math instead of floats, it helps a lot on all controllers without FPU (speed & code size)
  • the use of random8() is deprecated, use hw_random8() instead

- use hw_random8 instead of random8
 - changed diffusion calculation to fixed point math
 - adjusted general code formatting
@mryndzionek
Copy link
Author

thanks! can you add a readme file with a short description of what this UM is for (just one or two sentences that user FX go into this UM) and what your FX does. Also:

* can you update the formating to match the WLED standard (I think close to google standard) with opening curly brackets on the same line to make code more compact.

* use fixed point math instead of floats, it helps a lot on all controllers without FPU (speed & code size)

* the use of random8() is deprecated, use hw_random8() instead

Done.

@netmindz netmindz added effect usermod usermod related labels Jan 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effect usermod usermod related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants