Flow graph control flow nodes - 1 #14327

carolhmj · 2023-09-20T17:54:10Z

Changes

Add the following Control Flow nodes (lifted from: https://docs.google.com/document/d/1MT7gL-IEn_PUw-4XGBazMxsyqsxqgAVGYcNeC4Cj_9Q/edit#heading=h.m04l33f79oe5)
Counter
DoN
MultiGate
Switch
Throttle
WaitAll
WhileLoop
Move some nodes to the Control Flow directory:
ForLoop
Timer
Conditional
To set a value on a FlowGraphDataConnection point, now you need to specify a context. This is because, if we have the same graph running with multiple contexts, the value of a data connection can be different between them. I wasn't differentiating them before, which was an oversight.
When a block executes, it now knows which of its input signals activated the block. This is needed for blocks that have a "reset" input, such as some of the conditional blocks added.

Example PGs

#RKQX23#10: DoN - Click on the sphere to log a message max 5 times, click on the box to reset the DoN counter
#BWRXG9#2: Throttle - Log a message every 3 seconds using onTick event and Throttle, click on the sphere to reset the throttle
#RKQX23#7: Multigate - without looping, click on the sphere to log the number of execution
#RKQX23#8: Multigate - with looping, click on the sphere to log the number of execution
#RKQX23#9: Multigate - with looping and randomizing, click on the sphere to log the number of execution
#RKQX23#6: Switch - Change the switch's value on the code
#J1N2GP#5: Wait all - Click on the 3 spheres to open the door, click on the box to reset the wait all count
#9C7VK6#1: Counter - Log the value on the counter after a delay, explore the value in two different contexts
#5JFH79#7: While loop - Logs from 1 to 5

…wNodes

bjsplat · 2023-09-20T17:55:34Z

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.

deltakosh · 2023-09-20T18:08:34Z

Yes I was surprised by the lack of context but now it makes total sense. Very close to NGE actually

Looks good to me

bjsplat · 2023-09-20T18:10:15Z

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

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

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

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

Links to test babylon tools with this snapshot:

https://playground.babylonjs.com/?snapshot=refs/pull/14327/merge
https://sandbox.babylonjs.com/?snapshot=refs/pull/14327/merge
https://gui.babylonjs.com/?snapshot=refs/pull/14327/merge
https://nme.babylonjs.com/?snapshot=refs/pull/14327/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/14327/merge#BCU1XR#0

packages/dev/core/src/FlowGraph/Blocks/Execution/ControlFlow/flowGraphCounterBlock.ts

packages/dev/core/src/FlowGraph/Blocks/Execution/ControlFlow/flowGraphForLoopBlock.ts

RaananW

A few general comments:

Codedoc is missing on different functions and classes
Reusing objects and avoiding GC should be implemented wherever possible.

Otherwise - looks great!

packages/dev/core/src/FlowGraph/flowGraphContext.ts

packages/dev/core/src/FlowGraph/Blocks/Execution/ControlFlow/flowGraphWaitAllBlock.ts

packages/dev/core/src/FlowGraph/Blocks/Execution/ControlFlow/flowGraphTimerBlock.ts

packages/dev/core/src/FlowGraph/Blocks/Execution/ControlFlow/flowGraphThrottleBlock.ts

packages/dev/core/src/FlowGraph/Blocks/Execution/ControlFlow/flowGraphMultiGateBlock.ts

packages/dev/core/src/FlowGraph/Blocks/Execution/ControlFlow/flowGraphCounterBlock.ts

packages/dev/core/src/FlowGraph/Blocks/Execution/index.ts

Co-authored-by: Raanan Weber <raananw+github@gmail.com>

RaananW

There are some conflicts to be resolved

…wNodes

sebavan

A lot of the public properties/functions do not have code comments.

carolhmj · 2023-09-25T11:54:54Z

A lot of the public properties/functions do not have code comments.

@sebavan do you mean the _updateOutputs and _execute functions? These are just overriding the existing functions in the base block, should I add this as a comment in all of them?

carolhmj · 2023-09-25T11:59:16Z

@sebavan added some more docstrings

sebavan · 2023-09-25T14:25:12Z

A lot of the public properties/functions do not have code comments.

@sebavan do you mean the _updateOutputs and _execute functions? These are just overriding the existing functions in the base block, should I add this as a comment in all of them?

Oh no sorry I mainly mean properties like

_functions being internal by conventions are a bit less important knowing they are defined in the base class.

carolhmj · 2023-09-25T19:29:28Z

A lot of the public properties/functions do not have code comments.

@sebavan do you mean the _updateOutputs and _execute functions? These are just overriding the existing functions in the base block, should I add this as a comment in all of them?

Oh no sorry I mainly mean properties like

_functions being internal by conventions are a bit less important knowing they are defined in the base class.

@sebavan oh that makes sense 😄 thanks for catching them, I added comments to where I found them 😸

bjsplat · 2023-09-25T19:46:57Z

Visualization tests for webgl1 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/14327/merge/testResults/webgl1/index.html

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

bjsplat · 2023-09-25T19:47:06Z

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/14327/merge/testResults/webgl2/index.html

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

bjsplat · 2023-09-25T20:34:32Z

Visualization tests for webgl1 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/14327/merge/testResults/webgl1/index.html

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

bjsplat · 2023-09-25T20:34:50Z

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/14327/merge/testResults/webgl2/index.html

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

carolhmj added 6 commits September 19, 2023 18:33

control flow nodes

d5d3d43

Merge remote-tracking branch 'origin/master' into flowGraphControlFlo…

701636f

…wNodes

fix throttle

f1794e9

multigate, switch, waitall

a1dc001

change for setValue to take context into account

ebff91e

fix imports

c349c60

carolhmj added new feature flow graph labels Sep 20, 2023

deltakosh reviewed Sep 20, 2023

View reviewed changes

packages/dev/core/src/FlowGraph/Blocks/Execution/ControlFlow/flowGraphCounterBlock.ts Show resolved Hide resolved

packages/dev/core/src/FlowGraph/Blocks/Execution/ControlFlow/flowGraphForLoopBlock.ts Outdated Show resolved Hide resolved

RaananW reviewed Sep 21, 2023

View reviewed changes

packages/dev/core/src/FlowGraph/Blocks/Execution/index.ts Outdated Show resolved Hide resolved

carolhmj and others added 2 commits September 21, 2023 09:12

Update packages/dev/core/src/FlowGraph/Blocks/Execution/index.ts

b10688b

Co-authored-by: Raanan Weber <raananw+github@gmail.com>

review changes

8e0c6a9

carolhmj requested review from deltakosh and RaananW September 21, 2023 18:56

RaananW approved these changes Sep 22, 2023

View reviewed changes

Merge remote-tracking branch 'origin/master' into flowGraphControlFlo…

1008a3c

…wNodes

sebavan reviewed Sep 22, 2023

View reviewed changes

docs

8dd59e2

more docs

883eb52

sebavan enabled auto-merge September 25, 2023 19:43

sebavan approved these changes Sep 25, 2023

View reviewed changes

fix while loop

07d0722

sebavan merged commit 861e1d6 into BabylonJS:master Sep 25, 2023
10 checks passed

carolhmj deleted the flowGraphControlFlowNodes branch October 3, 2023 13:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Flow graph control flow nodes - 1 #14327

Flow graph control flow nodes - 1 #14327

carolhmj commented Sep 20, 2023 •

edited

bjsplat commented Sep 20, 2023

deltakosh commented Sep 20, 2023

bjsplat commented Sep 20, 2023

RaananW left a comment

RaananW left a comment

sebavan left a comment

carolhmj commented Sep 25, 2023

carolhmj commented Sep 25, 2023

sebavan commented Sep 25, 2023 •

edited

carolhmj commented Sep 25, 2023

bjsplat commented Sep 25, 2023

bjsplat commented Sep 25, 2023

bjsplat commented Sep 25, 2023

bjsplat commented Sep 25, 2023

Flow graph control flow nodes - 1 #14327

Flow graph control flow nodes - 1 #14327

Conversation

carolhmj commented Sep 20, 2023 • edited

Changes

Example PGs

bjsplat commented Sep 20, 2023

deltakosh commented Sep 20, 2023

bjsplat commented Sep 20, 2023

RaananW left a comment

Choose a reason for hiding this comment

RaananW left a comment

Choose a reason for hiding this comment

sebavan left a comment

Choose a reason for hiding this comment

carolhmj commented Sep 25, 2023

carolhmj commented Sep 25, 2023

sebavan commented Sep 25, 2023 • edited

carolhmj commented Sep 25, 2023

bjsplat commented Sep 25, 2023

bjsplat commented Sep 25, 2023

bjsplat commented Sep 25, 2023

bjsplat commented Sep 25, 2023

carolhmj commented Sep 20, 2023 •

edited

sebavan commented Sep 25, 2023 •

edited