[Suggestion] Notify added observer if observable was triggered.

[RaananW]

This is done to avoid the pattern of adding an observer after checking if the state requires that.
As an example let's take the scene's onReadyObservable which has a simple state - it will only notify once when the scene is ready. If a new observer will be added after the observable was triggered (and has notified existing observers) it will not be possible to trigger new observers that were added to is.
If notifyIfTriggered is set to true and the observable has already triggered it will automatically trigger newly added observers whenever they are added.

It's also recommended to add that to onDispose observables, which are the same.
A different solution would be to define an onObserverAdded function, but that would require setting this function to every observable that requires it. Instead - this is a more generic solution.

[bjsplat]

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

[bjsplat]

Snapshot stored with reference name:
refs/pull/13565/merge

Test environment:
https://babylonsnapshots.z22.web.core.windows.net/refs/pull/13565/merge/index.html

To test a playground add it to the URL, for example:

https://babylonsnapshots.z22.web.core.windows.net/refs/pull/13565/merge/index.html#WGZLGJ#4600

Links to test babylon tools with this snapshot:

https://playground.babylonjs.com/?snapshot=refs/pull/13565/merge
https://sandbox.babylonjs.com/?snapshot=refs/pull/13565/merge
https://gui.babylonjs.com/?snapshot=refs/pull/13565/merge
https://nme.babylonjs.com/?snapshot=refs/pull/13565/merge

To test the snapshot in the playground with a playground ID add it after the snapshot query string:

https://playground.babylonjs.com/?snapshot=refs/pull/13565/merge#BCU1XR#0

[Popov72]

It's a breaking change, so I'm not sure we should update our code to use the new feature (although, having the new feature is great!).

For the onDispose case, I think we can discuss it, but for Scene.onReadyObservable we shouldn't make the change, because it will break code that waits for the scene to be ready at the beginning and later adds meshes/materials/whatever and still wants to wait for new resources to be ready. With the new code, the user will be notified immediately because the scene has been ready in the past, even if it is not ready now. That's a perfectly valid use (and used!) case.

[bjsplat]

Visualization tests for webgl2 have failed. If some tests failed because the snapshots do not match, the report can be found at

https://babylonsnapshots.z22.web.core.windows.net/refs/pull/13565/merge/testResults/webgl2/index.html

If tests were successful afterwards, this report might not be available anymore.

[RaananW]

It's a breaking change, so I'm not sure we should update our code to use the new feature (although, having the new feature is great!).

For the onDispose case, I think we can discuss it, but for Scene.onReadyObservable we shouldn't make the change, because it will break code that waits for the scene to be ready at the beginning and later adds meshes/materials/whatever and still wants to wait for new resources to be ready. With the new code, the user will be notified immediately because the scene has been ready in the past, even if it is not ready now. That's a perfectly valid use (and used!) case.

Thanks for the review :-)

The scene onReady is actually doing roughly that under the hood, so it is not entirely a breaking change. The only difference is at what point would the observer be triggered.
I believe that a single-state-change observable (like onReady or onDispose - there must be a better name though :-) ) should always trigger if already notified, but it is really a matter of taste.
Anyhow - have removed the changed made to scene and node, as I think it is more important to have it as an option than to fix something that is not broken.

[sebavan]

Maybe we could have a reset functionnality in ?

I was thinking in Texture, onLoaded is called once and we keep the value until somebody calls updateURL for instance ?

[RaananW]

This grew a bit bigger than expected.

Adding this feature has surfaced memory leaks that we had (mainly undisposed objects and uncleared observables).
There is also an issue with the engine's render loop - after the engine stops we don't clear the next call for rendering, causing some observables to trigger even after clearing them. One solution would be to call clearANimationFrame, but that would have implications on different systems (as we apparent in the QueueNewFrame function - we use different mechanism s to queue the render loop function). What I did was add an isDisposed flag on the engine and not render if the engine is disposed.