GPUParticles2D Ignore Parent's "Disabled" Process Mode When Running Game

[floere]

Godot version

v4.0.1.stable.official [cacf499]

System information

macOS Ventura 13.0.1 (22A400)

Issue description

I expected GPUParticles2D to not run when in a process mode = disabled subtree when set to process mode = inherit. But the GPUParticles2D process material still emits particles (while running, editor is fine).

GPUParticles2DIgnoreProcessModeDisabled.mp4

Steps to reproduce

1. Add parent node, set to process mode disabled.
2. Add GPUParticles2D as a child, keep process mode inherit.
3. In the editor, the particles are stopped (or jittering back and forth)
4. When running the "game", particles are emitted and move normally.

Minimal reproduction project

BugDisabledGPUParticles2D.zip

[AThousandShips]

Strangely it works if you set the mode while running, but not if it is set at start, this is because when a child is added the paused notifications are not triggered

[newobj]

Hi, I found this issue as a "good first issue", and am trying to implement a fix.

What I've done locally and what works it to create a method, _update_speed_scale, which encapsulates the can_process check, and call it during both pause/unpause notifications as well as tree entry.

https://pastebin.com/A5J3cFzs

Do folks think this is a reasonable approach? If so, I'll submit a PR.

This would be my first contribution to Godot, so sorry in advance if there's something stupid about this approach :)

[AThousandShips]

I think this should work well, it's what I thought as a solution when seeing this, using a separate function is useful but could be easier to just copy the code

Remember to run clang-format on your code and follow the other guidelines! And nice to see new contributors, welcome!

[newobj]

Thanks! I wasn't sure what was "the norm" was in Godot re: when to copy vs. encapsulate. I notice the codebase is not overly-encapsulated so I wasn't sure. (btw; wow, the codebase is SO clean + elegant!!) So would you prefer duplication or helper function as I've done?

I'll read the guidelines and do my best to make a good patch. Thanks again for the prompt response!

[AThousandShips]

I don't think copy in this case increase the risk of making mistakes, and the usage is local within _notification so might be best to copy to not add a new function just for this local case, but I don't think there's any direct opposition to either approach

[AThousandShips]

And swapping approach is a trivial change so won't block the success of the PR assuming the basic approach is correct, so use your own judgement