godot icon indicating copy to clipboard operation
godot copied to clipboard

GDScript: Add support for variadic functions

Open dalexeev opened this issue 2 years ago • 20 comments

  • Closes godotengine/godot-proposals#1034.

Production edit: closes godotengine/godot-roadmap#65

dalexeev avatar Oct 04 '23 18:10 dalexeev

Does this support typed arrays as well?

JoNax97 avatar Oct 04 '23 19:10 JoNax97

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.

dalexeev avatar Oct 04 '23 19:10 dalexeev

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.

JoNax97 avatar Oct 04 '23 19:10 JoNax97

How I missed this! I just wanted to suggest this today

JekSun97 avatar Oct 04 '23 21:10 JekSun97

Would love to have this. I can't say how many times I abstracted some collection of features away, just to be forced to pass named arguments at specific positions, or abstract the names and use those variably. Can't wait to have this.

Blade67 avatar Oct 11 '23 14:10 Blade67

@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.

dalexeev avatar Oct 11 '23 14:10 dalexeev

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, ...)

NatGazer avatar Jan 10 '24 12:01 NatGazer

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]).

dalexeev avatar Feb 06 '24 13:02 dalexeev

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.

nlupugla avatar Feb 06 '24 15:02 nlupugla

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.)

koyae avatar Feb 08 '24 07:02 koyae

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

adamscott avatar Aug 27 '24 14:08 adamscott

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: image image

HolonProduction avatar Aug 27 '24 16:08 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:

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).

vnen avatar Aug 29 '24 14:08 vnen

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.

Mickeon avatar Sep 21 '24 18:09 Mickeon

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):

dalexeev avatar Oct 16 '24 07:10 dalexeev

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.

Mickeon avatar Oct 16 '24 20:10 Mickeon

  • 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.

Ivorforce avatar Dec 05 '24 10:12 Ivorforce

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.

dalexeev avatar Dec 05 '24 10:12 dalexeev

The Milestone says 4.4, is this still planned for 4.4?

peterhoglund avatar Feb 23 '25 20:02 peterhoglund

The Milestone says 4.4, is this still planned for 4.4?

Nope, the window for new features was closed with the 4.4 beta. The milestone just hasn't been updated yet.

Ivorforce avatar Feb 23 '25 20:02 Ivorforce

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.

I would like to add that I'm still in the opinion to omit the array parameter's name for non-virtual, variadic functions.

There's no point showing it because the user cannot override it or directly make use of it. There's already a lot of hints in the class reference. The ellipsis, the varang that can be hovered on, and the description oftentimes mentions the variadic nature of the function of interest.

Mickeon avatar Apr 11 '25 10:04 Mickeon

Is there any chance for this to be added to 4.5?

vvvvvvitor avatar May 02 '25 16:05 vvvvvvitor

That would genuinely be the idea, yes. There's not much else blocking this, other than appearances in the class reference. This is desired

Mickeon avatar May 02 '25 16:05 Mickeon

Rebased just in case.

dalexeev avatar Jun 09 '25 16:06 dalexeev

The comedic timing of this needing another rebase lmao

Repiteo avatar Jun 09 '25 17:06 Repiteo

Thanks! Excellent work on this PR; thrilled to see this make it in before the feature freeze!

Repiteo avatar Jun 09 '25 22:06 Repiteo

@dalexeev how to call overwritten variadic functions?

class Foo extends RefCounted:
	func vararg(...args: Array) -> void:
		prints(args)

class Bar extends Foo:
	func vararg(...args: Array) -> void:
		super(args)



func test_foo() -> void:
	Bar.new().vararg(1,2,3)
	
``

using `callv` is not an option

MikeSchulze avatar Sep 23 '25 17:09 MikeSchulze

Indeed, when using super(args) the entire argument array from Bar::vararg is passed as a single argument to Foo::vararg. Maybe this was overlooked while implementing the feature? It would be nice if Callable had a member property super or base referencing the method of the base class, so that we could use vararg.super.callv(args) or vararg.base.callv(args).

Gnumaru avatar Sep 23 '25 18:09 Gnumaru

@dalexeev how to call overwritten variadic functions?

class Foo extends RefCounted:
	func vararg(...args: Array) -> void:
		prints(args)

class Bar extends Foo:
	func vararg(...args: Array) -> void:
		super(args)



func test_foo() -> void:
	Bar.new().vararg(1,2,3)
	
``

using `callv` is not an option

Maybe this is where we need a spread operator to pass unpacked args array to the super? @dalexeev mentioned they have some draft working in progress https://github.com/godotengine/godot/issues/89916#issuecomment-3322367524

lethefrost avatar Sep 24 '25 01:09 lethefrost

Unfortunately, we've completely missed this case. We can fix it either by introducing spead syntax or by making it possible to get the parent method as a Callable using the super.func_name syntax (without parentheses). Then you'll be able to use callv().

I'll try to finalize my local draft of the spread syntax and look into the feasibility of implementing the second option soon. But we probably won't see a fix until 4.6. In the case of spread syntax, it's a large and complex feature that definitely can't be included in a patch release. The second option, which introduces getting the super method as a Callable, could potentially be considered for a 4.5.x patch, but it's still technically a new feature, even if it's much simpler than the first option.

I'm very sorry for the inconvenience, I completely forgot about this case, assuming that you can use callv() everywhere (native and custom methods of objects, methods of built-in Variant types, global utility functions, lambdas, and other Callable types).

dalexeev avatar Sep 24 '25 05:09 dalexeev