Clarify emitting behavior of GPU particles

[Gatada]

Clarified the finished signal, as it is incorrect to say that the signal is issued once processing of all active particles have finished (at least according to the GitHub thread that explained this to me).

This time, modifying the XML file.

[YuriSizov]

When you address the feedback, please make sure to amend the commit message to be more descriptive about the nature of changes.

[Gatada]

Here is where I am explained that the particle emitter will emit the finished signal once the particles are no longer visible: #76859

What I see in my test project concurs: the finished signal is received before the emitter is ready to emit (i.e. there are still particles being processed), yet there are no particles visible on the screen.

[HolonProduction]

Here is where I am explained that the particle emitter will emit the finished signal once the particles are no longer visible: #76859

What I meant when I said that, is that I can guarantee you that the particles will no longer be visible. But that doesn't mean that they are necessarily still processed.

[Gatada]

Here is where I am explained that the particle emitter will emit the finished signal once the particles are no longer visible: #76859

What I meant when I said that, is that I can guarantee you that the particles will no longer be visible. But that doesn't mean that they are necessarily still processed.

But that is, in my opinion, not relevant for the game developer. The only thing that is relevant is that when the finished signal is received, the game developer can call restart() with no risk of popping particles off screen. In other words, when finished is received, the particles may or may not be processing, but are either way no longer visible.

This is why I suggest the wording in my PR.

[HolonProduction]

To clarify: I am in favor of adding clarification about this to the docs.

But we need to make a difference between intended behavior and the current situation.

We should not change the part that says Emitted when all active particles have finished processing IMO. It is the intended behavior and we can't remove the intended behavior from the docs. (It's especially important here, to keep consistent with the CPU particle nodes.)

That the node thinks it is processing for longer than the particles are visible does not sound like intended and defined behavior on the other hand. It's a quirk in the current code which would better be placed inside a Note:.

[HolonProduction]

Also when we find a suited modification for the docs. You should copy it over to the 2D GPU particles as well.

[Gatada]

To clarify: I am in favor of adding clarification about this to the docs.

But we need to make a difference between intended behavior and the current situation.

We should not change the part that says Emitted when all active particles have finished processing IMO. It is the intended behavior and we can't remove the intended behavior from the docs. (It's especially important here, to keep consistent with the CPU particle nodes.)

Well, I thought we were documenting how things work, and not how we would like it to work? I would assume a future release that changes this, will be responsible for updating the docs? Surely this is how it should be?

That the node thinks it is processing for longer than the particles are visible does not sound like intended and defined behavior on the other hand. It's a quirk in the current code which would better be placed inside a Note:.

I just want the docs to be actual helpful. As it is written, I wasted hours messaging to understand why the node was still emitting (i.e. true) even after having received finished.

[HolonProduction]

Well, I thought we were documenting how things work, and not how we would like it to work? I would assume a future release that changes this, will be responsible for updating the docs? Surely this is how it should be?

If a future release were to change things it would change nothing about the implementation of this signal. The signal is working as it should be. So I see no ground for changing it's behavior description based on, what I would call, undefined behavior at an other point.

Side Note:
I am no core dev so arguing with me about the philosophy of writing docs won't help the situation. It's my personal opinion on how to write docs that help both developers and users.

I just want the docs to be actual helpful.

👍

As it is written, I wasted hours messaging to understand why the node was still emitting (i.e. true) even after having received finished.

So first of all emitting is false when the signal finished (except you set it yourself). That's just how it is implemented. (Put simple: Emitting will be set to false after half of the lifetime)

The thing that needs clarification is that setting emitting to true won't guarantee the system to start even when emitting was false before. This fact has little to do with this signal. It was true in 4.1 as well.

emitting is just not the right way to start a one shot particle system. And something like this could go into the docs (preferably for the emitting properties description).

[Gatada]

I believe this corrects my earlier misunderstandings, and clarifies things—while helping developers be more efficient.

[Gatada]

I believe the edit is now finally covering all feedback.

Sorry for taking up so much of your time. This has been a great learning experience, and I am very grateful for all comments! Thank you so much to everyone that took part!

If Godot is developed with the same level of care as this doc-change, then that makes me very happy. Godot will keep getting better, and I'll do everything I can to help out. Albeit more efficiently next time.

Any more comments or feedback?

[Gatada]

Now all it needs is a review. Do I ask for the review somewhere?

[Gatada]

I think this should be pretty straight forward to merge? And adding this info, will make it easier for users.

[YuriSizov]

Still needs an approving review, but also you need to squash commits together into one. You can wait for an approval to make sure you don't need to make further edits, but you will need to do this eventually before we can merge.

Make sure that the final commit has a short but descriptive message (the title of this PR is a good option). See this documentation, if you need help with squashing.

[Gatada]

OK, so I squashed and give the commit a succinct description.

[akien-mga]

Thanks! And congrats for your first merged Godot contribution 🎉

[Gatada]

Thanks, though far from my first. I had one in Oct 2022 as well ;-)

[akien-mga]

My bad! Might have been from a different GitHub account as GitHub doesn't seem to remember it: https://github.com/godotengine/godot/commits?author=Gatada

[Gatada]

I am not sure how these repos work, but here it is:

https://github.com/godotengine/godot-docs/commits/master?author=gatada

Err.. the link returns different results in the browser..?

Why does the above work in browser, but opens in the app (unlike you link)-which doesn't work?

This works though?

https://github.com/godotengine/godot-docs/commits?author=gatada