godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
91.12k stars 21.19k forks source link

Ability to pass arguments to functions by name #6866

Closed PLyczkowski closed 4 years ago

PLyczkowski commented 8 years ago

Ability to pass arguments like this:

func foo(val1:String = "c", val2:String = "c"):
    print(val1)
    print(val2)

foo(val2 = "a") # prints "c a"
foo(val2 = "a", val1 = "b") # prints "b a"

Here is the equivalent in python explained: https://treyhunner.com/2018/04/keyword-arguments-in-python/

bojidar-bg commented 8 years ago

What about the skipped values, that don't have a default? Would it raise an error, or use some "type" default? E.g.:

func test(x, y = 2):
func main():
    test(x = 6) # (6, 2)
    test() # Error, not enough arguments
    test(y = 5) # Uhhh, whats?

Note: if this gets implemented, the docteam would have to go over all classes and check if all parameter names are "self-evident" and "well-picked"...

PLyczkowski commented 8 years ago

Skipped values that don't have a default would rise an error. Here is more info on this, it's an existing feature in python: https://treyhunner.com/2018/04/keyword-arguments-in-python/

Sslaxx commented 8 years ago

First time I saw this was in Ren'Py. It's a nice feature, one GDScript should adopt I think.

rgrams commented 8 years ago

Can you do default values in gdscript? Or I guess you're just talking about built-in functions here?

bojidar-bg commented 8 years ago

@rgrams Yeah, you can, I just had to make sure what's the expected behavior.

neikeq commented 8 years ago

I would also like to see this implemented. Named arguments are very helpful to avoid boolean traps.

PLyczkowski commented 7 years ago

Can this maybe be considered for 3.0?

Zireael07 commented 7 years ago

3.0 is right around the corner, so probably not. But maybe 3.1?

Zephilinox commented 6 years ago

I'm not sure I see the value in it myself, but it seems well received so I thought I'd bump this so it might get a milestone and not be left forgotten.

Calinou commented 6 years ago

How does this compare to passing a dictionary as a parameter?

vnen commented 6 years ago

How does this compare to passing a dictionary as a parameter?

They are quite different from each other. A dictionary is good if you don't have a fixed list of arguments, otherwise relying on the engine parser to do the validation is much better. For instance:

func my_func(arg1, arg2, arg3):
    # Do stuff with args
    # You can assume they are all set

func _ready():
    my_func(arg1 = 2, arg3 = "string", arg2 = true)

With a dictionary:

func my_func(args):
    # First check if all args are there, and you can't really raise an error
    # (unless you do something crazy like null dereference)
    # Do stuff with args, but have to use args.arg1 instead of just arg1

func _ready():
    # not much difference here
    # but you may also need to check for returned errors
    # which can appear if you mistype something
    my_func({ arg1 = 2, arg3 = "string", arg2 = true })

Also, it makes really well for boolean parameters, since you can clarify what they are in the caller:

func do_stuff(data, skip_unique = false):
    if skip_unique:
    # ...

func _ready():
    # What does "true" here do?
    do_stuff(get_data(), true)

    # Here you know it's about unique values
    do_stuff(get_data(), skip_unique = true)
PLyczkowski commented 6 years ago

Candidate for 3.1 maybe?

JayStringer commented 6 years ago

Coming from Python, this would be a really handy feature, it increases the readability of functions dramatically which makes future you much happier.

vnen commented 6 years ago

For what I understand of the engine and GDScript, there's a couple of problems to implement this:

Eoin-ONeill-Yokai commented 6 years ago

Would it be possible to change our way of thinking about this? Solution could be something similar to this:

func my_func({arg1, arg2, arg3}):
    #Access to arg1, arg2 and arg3 as dictionary members. 

func _ready():
    my_func({arg1 = 2, arg3="SomeString", arg2 = false})

Where instead of defining arguments in any order, you are basically asking for any dictionary that has the keys "arg1, arg2, arg3" filled? It could be a runtime error if one of the designated keys are missing from a dictionary being passed into the function. I'm not sure about my syntax (because the dictionary wouldn't have a name in this case to reference it by) but I feel like something like this could at least be a workable band-aid solution. Basically, having it so that dictionaries can be checked to fulfill a "key" requirement in order to be a valid argument for a function.

vnen commented 6 years ago

@TheYokai you mean to use a special syntax? I find it pointless because then you cannot use this regularly, only the functions you know that were declared in this way (so nothing from plugins, for instance) and not on any engine function, which is what you use the most. Also, you can do that already by just asking for one argument that is a dictionary and do the check in the function itself. And you are forced to always name all the arguments, which is a bit counterproductive. Unless I misunderstood what you meant.

Runtime checks are really bad for performance, this feature is something that needs to be solved at parse/compile time so it doesn't add any overhead.

Eoin-ONeill-Yokai commented 6 years ago

No, that all makes sense, and I think you understood what I meant. Was just interested to see if there was a compromise that could be made.

The performance impact is a pretty good point though.

neikeq commented 6 years ago

I want to add that introducing named parameters also has its negative side. It makes it easier to break API compatibility. If the name of a parameter changes, it will break existing code that specified the old name. This applies to the methods in the Godot scripting API as well, but that was already the case since the addition of C#.

Mochnant commented 5 years ago

This would be a useful feature and it really does help readability.

ArdaE commented 5 years ago

named parameters also has its negative side. It makes it easier to break API compatibility. If the name of a parameter changes, it will break existing code that specified the old name.

How is that a bad thing? If the name of a parameter has changed, its meaning is likely to have changed as well. In that case, you would want an error message so you can fix the issue (as opposed to Godot silently accepting your broken call and your code working incorrectly leading to hours of debugging for something that could have been caught in an instant. Make that days of debugging if the issue slips through to a released version and you get reports from your customers).

In the rare cases that a parameter's name is modified because it had a typo or an ill-suited name, the developer would see the error and update the argument's name in a few seconds. Anyone who doesn't want to deal with code robustness achieved through compile-time checks wouldn't use named arguments anyway.

What @vnen brings up with regards to this being unimplementable at compile-time is the actual deal-breaker, but I also don't know why that would be so. I don't know Godot's internals, but it surely must be possible to do this check at compile time, especially with semi-strong typing in place now. Possible doesn't mean easy of course. I wouldn't want this feature if it's done at runtime.

menip commented 5 years ago

Could we take a look at this for 4.0? Would love to have this in GDScript, would increase code readability.

neikeq commented 5 years ago

@ArdaE I didn't say it was always a bad things. As you said, if the parameter is renamed, then the meaning is likely to have changed as well, but that's not always the case. BTW, I'm in favour of named parameters, just pointing out something that must be taken into account.

TheCire commented 5 years ago

@menip i 3rd your proposal for GD 4.0

fabriceci commented 4 years ago

Hope he will succeed to do this at compile time. For function like move_and_slide() with many parameters (better again, if you want to keep many default value) that's increase a lot the readability.

julian-a-avar-c commented 4 years ago

It's been a while since this was requested, and I think it would be great. I'm encountering problems with the autocomplete that doesn't work between files for some reason, and after the 6th argument in a constructor, you start wondering if there's a way to name them so that it stays consistent between files. This would help a lot. Thank you for your work as always :3

Caenhorn commented 4 years ago

I write a lot of code that looks like this: var recover_state = AnimState.new("Recover", $AnimationPlayer, "recover", false)

Would really improve readability if I could do this instead:

var recover_state = AnimState.new(name="Recover", 
  animation_player=$AnimationPlayer, 
  animation_name="recover"
  reset_player=false)
Calinou commented 4 years ago

While not as convenient, keep in mind you could also write the snippet above like this:

var recover_state = AnimState.new(
    "Recover", # name
    $AnimationPlayer, # animation_player
    "recover", # animation_name
    false # reset_player
)

This would be more convenient if trailing commas were allowed in method declarations and calls (there's an open issue for this already).

vnen commented 4 years ago

I do have an idea to bring named parameters when it's possible to figure out at compile time. If you use a lot of static types it's gonna work better. It's not possible to do anything at runtime, because argument names are not present on release (it's also unwanted overhead for function calls).

However, I don't think there's any convention to keep parameter names on overridden methods, so I don't how that would work (especially for engine classes).

akien-mga commented 4 years ago

Feature and improvement proposals for the Godot Engine are now being discussed and reviewed in a dedicated Godot Improvement Proposals (GIP) (godotengine/godot-proposals) issue tracker. The GIP tracker has a detailed issue template designed so that proposals include all the relevant information to start a productive discussion and help the community assess the validity of the proposal for the engine.

The main (godotengine/godot) tracker is now solely dedicated to bug reports and Pull Requests, enabling contributors to have a better focus on bug fixing work. Therefore, we are now closing all older feature proposals on the main issue tracker.

If you are interested in this feature proposal, please open a new proposal on the GIP tracker following the given issue template (after checking that it doesn't exist already). Be sure to reference this closed issue if it includes any relevant discussion (which you are also encouraged to summarize in the new proposal). Thanks in advance!


@vnen told me he'll try to write a technical proposal in coming days for this.

PLyczkowski commented 4 years ago

Added in godot-proposals: https://github.com/godotengine/godot-proposals/issues/902

merriam commented 3 years ago

I do have an idea to bring named parameters when it's possible to figure out at compile time. If you use a lot of static types it's gonna work better. It's not possible to do anything at runtime, because argument names are not present on release (it's also unwanted overhead for function calls).

However, I don't think there's any convention to keep parameter names on overridden methods, so I don't how that would work (especially for engine classes).

This should be doable at compile time if the compiler has an extra nonce value for USE_DEFAULT:

func f(a, b, c=true, d=4):
     ....

f(1,2, d=99)    # Complied as f(1, 2, USE_DEFAULT, 99)
f(1,2)               # Compiled as f(1, 2, USE_DEFAULT, USE_DEFAULT)
f(1,2, d=99, c=false)    # Compiled as f(1, 2, false, 99)

I don't know if Godot does an internal linking phase for default arguments or will need to do a bit twiddle to assign the arguments at run time. I'm still learning Godot internals so this may not apply; it might save a run-time step to create a 0's and 1's mask at compile time for the default object ids so that a quick memcpy and AND can set the default object ids.

vnen commented 3 years ago

@merriam that is really not the issue. The issue is knowing what the signature of f() is when you call it (that is, knowing that d is the 4th argument and c has a default value at the point of the call), which is impossible in a lot of cases if you're not using static typing (which is the default).

ArtyIF commented 1 year ago

@merriam that is really not the issue. The issue is knowing what the signature of f() is when you call it (that is, knowing that d is the 4th argument and c has a default value at the point of the call), which is impossible in a lot of cases if you're not using static typing (which is the default).

Python somehow does it, why can't GDScript?

AThousandShips commented 1 year ago

Please add your feedback to the proposal:

This has been closed