search

tguerin / newton

An Easy To Use Particle Emitter
https://newton.7omtech.fr
MIT License
22 stars 7 forks source link

only re-setup effects if they are provided with the activeEffects Newton property #15

Closed Joshix-1 closed 8 months ago

Joshix-1 commented 9 months ago

this fixes a problem that an effect added with NewtonState.addEffect gets removed as soon as the child of the Newton changes. This is a really unexpected behaviour.

I'm not sure whether or not this is the correct solution. (it seems to work for my use-case)

I think a better solution could be to make _setupEffectsFromWidget ignore all effects added with addEffect. But I don't know what the best way to do that would be.

netlify[bot] commented 9 months ago

Deploy Preview for imaginative-palmier-d7ee2c canceled.

Name Link
Latest commit 43c1ef07b660eeaf7bbde7a40578f48520ccc337
Latest deploy log https://app.netlify.com/sites/imaginative-palmier-d7ee2c/deploys/651d35021352d30008185390
tguerin commented 9 months ago

For me it looks good, should have been like that at first.

tguerin commented 9 months ago

Honestly, i added the addEffect cause I had in mind to be able to do Newton.of(context).addEffect to easily add an effect on demand. I think I prefer to remove the addEffect and stick to a widget creation. Can you remove It in your PR?

Joshix-1 commented 9 months ago

my workflow was to have set the key of the Newton to a GlobalKey<NewtonState> which I then use to get the state and dynamically add a effect. I honestly like that more than having to keep track of a list on my own.

tguerin commented 9 months ago

my workflow was to have a GlobalKey which I then use to get the state and dynamically add a license. I honestly like that more than having to keep track of a list on my own.

Not sure it's related to this PR, but i see your point regarding the license.

tguerin commented 9 months ago

For this we could maintain a separate list for the runtime added effects, thus keeping them upon rebuild.

Joshix-1 commented 9 months ago

Not sure it's related to this PR, but i see your point regarding the license.

I'm tired I wrote license, when I meant effect

For this we could maintain a separate list for the runtime added effects, thus keeping them upon rebuild.

That seems to be a good option

Joshix-1 commented 9 months ago

I feel like the best solution is to only allow one of the two ways to add effects. Either use the activeEffects on Newton or use the addEffect of the NewtonState. Both together don't work well.

tguerin commented 9 months ago

Will try to take some time this week-end to take my head around this :)

tguerin commented 8 months ago

The behavior will be added with #16