When disposing a particle system with subEmitters, I found some issues:
The attached subEmitters are not disposed.
The existing end type subEmitters are not disposed, and there is not a option to do so.
This is a playground:
When you click the button to dispose the particle system, you could notice that the attached subEmitters still exist. The end type subEmitters are still there until they end. If you uncomment the code I added below, you will see that the issued are fixed.
I think the first question is a bug needs to be fixed by default.
As for the second, we could add an option to choose whether to dispose existing end type sub emitters if I want. Also for the purpose of backwards compatibility. Or maybe we could just make it default also.
This is what I think is a disposing, dispose everything, lol.
I noticed that you are in ācode-freezeā stage right now, so I donāt know if itās appropriate to open a pull request. You could just close it if it bothers you.
I agree thereās a bug, but from a user standpoint, I would expect that dispose would simply remove the particle system, meaning all particles would disappear right awayā¦ Having some particles still alive while the particle system has been removed/destroyed feels wrong to me.
However, itās how things work currently, so we must keep it for backward compatibility. But I think the new flag passed to dispose should trigger the right behavior: remove everything.
Yes, they are. Itās only your āfirstā (particleSystem1) that isnāt. Sorry though, I rushed the PG a little. This one should dispose of all. https://playground.babylonjs.com/#9NHBCC#33
Honestly, I was thinking about this and thinking about replyingā¦ but Iām not sure yet. Eventually, I could imagine a use case where keeping with the subemitter for a while (later removing it on a timeout or depending on context) could be something usefulā¦ although I admit I do not have this use case in mind at this moment Still, I appreciate the versatility of the framework (itās one of the thing I like most) even if it means some additional operations or some āweirdā coding/instructions at time. I think it will be good if other people will also give it a second thought before we proceed Of course, my opinion only Meanwhile, have a great day
@mawa Actually I agree with you that there might be some situations the existing particles need to be last a while. In the other hand, I also think the function that disposing all existing particles is an important feature. (But the fact that the attached subEmitters exist forever is definitely a bug. )
I also agree with you that the versatility is great. But I think having rich features and ease of use are also important. Of course, my opinion only
I agree with @Evgeni_Popov that while the current behavior does feel like a bug, we canāt remove it for back compat sake. There may be someone who saw the sub emitters still alive when the parent emitter is disposed and used that to their advantage. If it were a bug that broke the render loop, I would say to fix it entirely, but this is more of a āshouldnāt work like thisā type of bug that we canāt confirm if anyone is using or expects to work this way. Adding a flag to dispose all when the parent emitter is disposed and kill all particles immediately seems like the right move here. Users would want to make sure that their sub emitters have eclipsed their lifecycle and have waited long enough for all particles to die before disposing to make sure particles donāt snap out of existence, but this seems like a logical workflow to dispose the complex system by disposing the parent.
So I did not dispose the subEmitters on purpose:) the goal was to let the user decide but I totally agree that it should at least be an option.
Why letting the user decide? Initially I was imaging an use case where the main system could be a bullet and when the bullet hits a wall an explosion subsystem would be triggered (so the bullet is disposed but not the subsystems)
@PatrickRyan@Deltakosh Agree with both of you, too. Backward compatibility is important.
I made this playground that have an attached sub-emitter as a firework trailing. When I dispose the main particle system, the trailing will not be disposed and will exist forever.
I think itās because our code just make the sub-emitters disposeOnStop=true and stop them only when the particle they attach is dead:
if (particle.age >= particle.lifeTime) {
// Recycle by swapping with last particle
this._emitFromParticle(particle);
if (particle._attachedSubEmitters) {
particle._attachedSubEmitters.forEach((subEmitter) => {
subEmitter.particleSystem.disposeOnStop = true;
subEmitter.particleSystem.stop();
});
.....
Clearly it will not work when we dispose a particle system, all the particles in the particle system are disposed. The code above will not be executed. So all the attached sub-emitter are remained and will never be disposed. This is what I think is a bug.
You could try in the play ground by using stop or dispose in line 132 respectively. You will see the difference.
As for the existing end type sub-emitters, I totally agree with both of you that there must be some situations that need them exist until dead. So I add a option to the dispose function in my pull request. Let the user choose. Itās also a good prompt to users that how we handle the particles, by telling them they could choose whether to dispose the end type sub-emitters. The solution to dispose of them may not be easy to find (The loop must iterate in reverse):
if (this.activeSubSystems) {
for (let i = this.activeSubSystems.length - 1; i >= 0; i -= 1) {
this.activeSubSystems[i].dispose();
}
}
So I think itās a good choice to add it to our code.
We could have a property like ādispose the particle system when there is no active particles anymoreā?Or, maybe, instead of having a new property, it could be a new parameter to the stop method?
When set, we dispose all sub-emitters so that they canāt kick in anymore, and we āwaitā (in fact, we continue to render the particle system) for the active number of particles to reach 0 before calling dispose?
This would account for the case where we want the particle system to continue working as long as there are particles to display. Would that work in your case @Luke_Lee?
In that case, we would update dispose to really dispose of everything. It would be a breaking change, but the current behavior is a bug in my opinion. Or we add a flag to dispose to trigger this behavior.
Iām not going to add my two cents on top of it. Pretty much everything has been said, so I guess I just wanted to say how much I appreciate how things are handled in my favorite toy
I trust you Guys will make the right decision.
In summary, there might be 3 situations that we need to deal with:
The sub emitters:
Have a manualCount and still exist when dispose. They will dispose themselves when they are dead.
Have a manualCount and already are disposed themselves when dispose.
No manualCount. They will exist forever cause we handle the disposal only when the particle they link (attached and end type) is dead, which isnāt gonna happen, like I said above.
What we could do respectively:
Situation 1:
Do nothing, then they will dispose themselves when they are dead.
Dispose them immediately when dispose the system.
Situation 2: There is no need to do anything.
Situation 3:
Do nothing, then they will exist forever until the users do it manually.
Dispose them immediately when dispose the system.
So there are two options we have to handle the problem:
Do nothing.
Dispose them immediately.
Then we could add two options for the dispose function
Hi, I donāt think they are the same thing. As I mentioned above, there are some situations (linke 3 above) that the particles will not trigger the stop moment when dispose. Then the disposeOnStop will not work.
I beg to differ, If all the systems are flagged like in my PG (disposeOnstop and targetStopDuration set) then all the particle systems will be disposed automatically
But again Iām not pushing back on your intended change. I think it is a good one (the last one you mentioned with ādispose(disposeTexture = true, disposeAttachedSubEmitters = false, disposeEndSubEmitters = false)ā)
evgeni:
Of course, my opinion only 
Meanwhile, have a great day 
)
