GDScript: Add support for variadic functions

[dalexeev]

• Closes Add support for variadic functions (varargs) to GDScript godot-proposals#1034.

Production edit: closes godotengine/godot-roadmap#65

[JoNax97]

Does this support typed arrays as well?

[dalexeev]

Does this support typed arrays as well?

Currently no. I'm not sure if we should support this, as it would make it harder to check that the number and types of arguments are contravariant.

[JoNax97]

I understand.
I feel we should aim to support it eventually, as it will otherwise leave out users that seek to write type safe gdscript.

[dalexeev]

@Blade67 Please don't bump issues without contributing significant new information. Use reactions on the first post instead. We are currently focused on the upcoming 4.2 release, which will not include this feature. After that I'm going to continue working on this feature.

[NatGazer]

If you work just with GDScript, there are some easy workarounds regarding vararg functions. But if you're using GDExtension this is a big deal. I like to communicate with GDScript with the call function:

node->call("functionName");

If I want to pass a variable number of parameters, I can't unless I create an Array. But the most annoying thing in GDExtension is that you just can't initialize arrays or Dictionaries directly, So to achieve this I need to use "append" which is a pain:

Array args;
args.append(arg1);
args.append(arg2);
args.append(arg3);
...
node->call("functionName", args)

With this commit it will be like this:
node->call("functionName", arg1, arg2, arg3, ...)

[dalexeev]

I decided to split this PR into two parts:

1. rest parameter allows you to declare variadic functions (this PR);
2. spread syntax allows you to unpack arrays when calling functions and constructing arrays.

With this PR only you won't be able to do:

func vararg_func(x: int, y: int, ...args: Array) -> void: # Rest parameter.
    other_vararg_func(x, y, ...args) # Spread syntax.

But you will be able to do:

func vararg_func(x: int, y: int, ...args: Array) -> void: # Rest parameter.
    other_vararg_func.callv([x, y] + args)

Now I'm working on the spread syntax. I'm not sure if we could merge just rest parameter for 4.3 and leave the spread syntax for 4.4. But in any case, two PRs are more convenient for review.

As for supporting typed arrays, in addition to the problem mentioned above, there is an obstacle that MethodInfo does not support rest parameter (name and type), only the METHOD_FLAG_VARARG, so the type would not appear in the docs and in editor tooltips (in some cases). This is probably not critical, but I would prefer to leave it for the future.

Finally, there is the question: should we introduce a new token ... or could we reuse .., which is used in match patterns. They have a similar purpose (.. do not allow you to declare a variable for packing rest array elements and dictionary key/value pairs). For variadic functions languages usually use ... (C++, JavaScript, PHP), plus in the documentation we use ellipsis too. Also, match patterns are more like declarations, the question arises what will happen if we want to add spread syntax for patterns (like [var x, ...array, ..var rest]).

[nlupugla]

I would vote in favor of waiting to merge until the spread syntax is finished. In my opinion, varargs feels like an "incomplete" feature unless there is some sort of spreading syntax.

[koyae]

Regarding waiting for a spread (splat) operator before merging, I don't think a small amount of syntax-asymmetry here is a big problem given that this is a technical feature, which will have technical users. Though getting a spread operator merged at the same time as the ...args syntax for declaring variadic (varargs) functions might be nice, people have been waiting nearly 4 years at this point, and the ability to declare variadic functions is more important than just making Object.callv() calls prettier.

(I found this issue looking for a simple way to generically connect to signals, since they can be emitted with any number of objects passed along to provide extra info.)

[adamscott]

Talked about in a GDScript meeting. This kind of feature would nicely fit in a 4.4 release. We need to review it first, though. cc. @HolonProduction @vnen

[HolonProduction]

The editor and tooling related stuff looks good to me aside from some nitpicks, can't say anything about the VM though.

One thing that we probably should resolve before 4.4 is that when typing the three dots we get code suggestions after the first two of them. But that's stuff for a follow up.

[HolonProduction]

Do we really want "rest parameter" as user facing name? I feel like dropping this name without context does not really make it clear what is talked about. I think vararg parameter would be easier for users to understand. (For reference were is the name taken from?)

[dalexeev]

For reference were is the name taken from?

Rest parameters - JavaScript | MDN

[HolonProduction]

Every time you think you know javascript 😅

I still think vararg would be a better name, since it is already used in the docs for print.

[HolonProduction]

Also I'm a bit conflicted about how varargs show in the documentation. On one side it's in line with print, but on the other side not showing the param name could get confusing for users writing doc comments:

[vnen]

Also I'm a bit conflicted about how varargs show in the documentation. On one side it's in line with print, but on the other side not showing the param name could get confusing for users writing doc comments:

I think it should show the name. I would argue even the builtin functions should have a name attached to the rest parameter (but it's not a requirement for this PR).

[Mickeon]

I don't know how to feel about the additional vararg in the class reference. It's repetitive and it feels like args plus ellipsis conveys that information more intuitively.
I feel like you the tooltip should be put on args and/or ellipsis instead.

[dalexeev]

I think it should show the name. I would argue even the builtin functions should have a name attached to the rest parameter (but it's not a requirement for this PR).

Done:

func f(...test_args):
    pass

func g(...test_args: Array):
    pass

For native methods ...args: Array is used (but I'm not sure if that's right):

[Mickeon]

I do reiterate the sentiment that the vararg existing alongside ...args: Array feels needlessly repetitive, now.
I have multiple thoughts on how to address it. I'll use call()'s signature for the examples:

• call(method: StringName, ...args: Array)
  The tooltip is moved to ...args.
• call(method: StringName, ...args)
  Same as above, without Array. This makes args stand out more. It also prevents readers mistaking args as an actual array argument to pass into the function, which I foresee. The general convention and tooltip may make the intention clearer.
• call(method: StringName, ...) varargs
  Same as it is prior to this PR, but this format should only apply to built-in functions. Would be a problem if virtual, vararg functions are eventually implemented.

[Ivorforce]

• call(method: StringName, ...) varargs

In my opinion, ... without a name to the varargs should be avoided, because it may be unclear what the additionally supplied arguments will do. Example:

multiply(...)

This, in my opinion, does not sufficiently convey what is being passed into the function.

multiply(...factors)

This makes it much clearer.

However, I think this is a bit outside the scope of the PR to change.

[dalexeev]

In my opinion, ... without a name to the varargs should be avoided, because it may be unclear what the additionally supplied arguments will do.

The documentation has already been changed as you suggest, see the comment above. Note that core and extension methods do not support rest parameter name, so hardcoded args was used. Maybe we should leave the lone ... when the rest parameter name is unknown, but then there will be inconsistency between native and GDScript methods.