Toggling the `sequenced` property of `VisualScriptFunction` doesn't redraw/refesh call nodes

[follower]

Godot version: v3.2.1.stable.official

OS/device including version: N/A

Issue description:

The sequenced property of VisualScriptFunction is (AFAICT) supposed to enable a function to act as a getter, however:

1. The functionality is undocumented (see: https://github.com/godotengine/godot-docs/issues/3287).

2. There is an apparent bug which makes the functionality--at a minimum--undiscoverable and potentially broken.

Steps to reproduce:

Here is an example of how the sequenced property is intended to be used:

Configuration:

• functionA has Sequenced set to true i.e. "On"/checked/ticked.
• functionB has Sequenced set to false i.e. "Off"/not-checked/not-ticked

In the above image, note the following:

• The call node for functionB() has no white triangle sequence ports, only an output data port.
• The call node for functionA() does have sequence ports in addition to the output data port.

Also, with this configuration, if the call to functionA() is not connected as part of a sequence then an error is generated by the Add node:

Add: Invalid arguments A: Nil B int

Minimal reproduction project:

The below image shows an example of what happens if sequenced is toggled on functionA and another call node is created by dragging the function name from the "Functions" list onto the graph:

Note:

• The two functionA() function call nodes have different appearances (the lower functionA() node was created after "Sequenced" was unchecked.

There is more detail in the documentation-related issue linked above. (Including discussion about whether there's an underlying feature design/exposure issue to consider and uncertainty about potential caching/consts issues.)

Potential solutions

My impression is that a bug fix would need to cover two parts:

1. Visual refresh of existing displayed function call nodes.

2. Handling of connections/disconnections of existing function call nodes when the referenced function's sequenced property is set to false--including potentially warning about unintended changes.

Related links

• Commit where functionality was added: https://github.com/godotengine/godot/commit/8a4bce6ebd843f9a8f482f74601f4933aae737a3

  • Issue marked as "Fixed" by above commit: "It's impossible to create custom function without sequence port in VS #6346 "

  • Semi-related issue: https://github.com/godotengine/godot/pull/9758

• Documentation of ports_changed_notify which seems like it might used to modify/update display of node after sequenced property is toggled.

• The Reddit /r/godot question about the Sequenced property of VisualScriptFunction that started me down this path. :)

[theoway]

So all the functions with sequence property set to false, will they be called at the same time? Or do they act as getters?

[follower]

So all the functions with sequence property set to false, will they be called at the same time? Or do they act as getters?

My understanding is that the functions act as getters.

[theoway]

A better way to solve it would be to refresh the nodes, like you mentioned. Because if the sequence property is changed, it would depend on the user on how to connect/reconnect the node.

[follower]

Because if the sequence property is changed, it would depend on the user on how to connect/reconnect the node.

From a "principle of least surprise" point of view:

• I wonder if there's any existing support in Visual Script for "disable this node/connection" rather than just disconnecting?

• Or, if it shouldn't be possible to toggle the sequenced property when there is an existing sequenced instance?

FWIW I do feel like the sequenced property being exposed is somewhat unintentional leaking of an implementation detail and that a better approach would be to just automatically treat a "non-sequenced" node as a getter when the output data port is only port connected.

[follower]

But, yes, at a minimum, visually refreshing the nodes would be an improvement on the existing behaviour, even if it may not be ideal.

[swarnimarun]

Yep I do agree, that sequenced is rather obtuse and probably should be removed in-favor of automatically detecting no sequence port connections. Or having grayed out sequence ports unless connected displaying capacity to work without the sequence ports.

As for the redrawing part, it is simple enough so maybe it's fine as a bandage.

[theoway]

But, yes, at a minimum, visually refreshing the nodes would be an improvement on the existing behaviour, even if it may not be ideal.

Yup, sure it will be. I'll be making a PR to refresh those nodes. This way there will be some consistency with the property change. Also, it's not only with sequence ports. The arguments also don't get updated when number of arguments are changed in function node.