-
-
Notifications
You must be signed in to change notification settings - Fork 97
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add ability to pass arguments to functions by name #902
Comments
Previous discussion about this proposal can be found here: godotengine/godot#6866 (comment) |
Just to summarize the technical issues:
Potential solution: To not be only negative, there's one thing feasible to implement: If the function is known at compile time then I could use the compiler to de-mangle the arguments and push them in correct order. With a few natural caveats:
I'm not sure if it's worth it adding with these restrictions. If it is, I can consider adding once the main features are working in the new GDScript. |
Is there a deterministic way go know whether the function is or isn't known at compile time? If so, are the "rules" simple enough to understand at a glance? I'm skeptical here.
This would cause so much confusion. I really don't see how this proposal is really even that useful. In fact, in languages this is supported, its use is often frowned upon because it negatively affects readability and maintainability for the average programmer. At the very least, I don't think the bebefits here (if any even exist) outweigh the plethora of cons. |
I haven't seen many criticisms of named parameters in Python or Lua, and for functions taking many parameters, their use increases readability and reduces occurrence of bugs. |
I've never heard of any criticism of named parameters in Kotlin either, and it's generally regarded as an improvement over positional parameters only in Java, where developers have to rely on the IDE to keep track of parameters. Also, no one is suggesting that named parameters replace positional parameters; only that it's offered as an alternative, like in Python, Lua and Kotlin, so there are no cons, unless performance is affected negatively. |
This would be nice. Currently I am using an optional func format_time_(time: float, options: Dictionary = {}) -> String: But that has a lot of drawbacks. First, just from a function definition, one cannot tell which options the function supports, so I have to not forget to document them (docs are not great currently): ## Format time.
## @param time {float} Time in seconds
## @param options {Dictionary}
## Options:
## * `format` - default `TimeFormat.DEFAULT` (hours + minutes + seconds)
## * `delim` - delimiter, default `":"`
## * `digit_format` - default `"%02d"`
## * `hours_format` - default is `digit_format`
## * `minutes_format` - default is `digit_format`
## * `seconds_format` - default is `digit_format` (or `"%05.2f"` when TimeFormat.MILISECONDS is in `format`)
...
func format_time_(time: float, options: Dictionary = {}) -> String:
... And a second drawback, I have to extract them from the dictionary and fill in the default values (performance-wise: for every field in options that's one call to Reading code is pretty good (compared to named args, it's just few characters longer): func format_time(time: float) -> String:
return GG.format_time_(time, {format = GG.TimeFormat.MINSEC, minutes_format = "%d"}) But using such function (writing code calling it) has, in my opinion, pretty bad UX and performance is most surely worse than anything you could come up with in the engine. Edit: Totally forgot, names of named args could be suggested by the editor (relatively easily I think). |
Another drawback is that these arguments don't get parsed, so you get failures at runtime. |
@vnen Assume there's a singleton class called Or if that would kill performance, maybe when running the game, if a script uses function calls with named args, a new (duplicate) script is created, and the func call is replaced in-line (like a C++ macro) with a func call with ordered default/named arguments. Example Project: FuncArg.zip
extends Sprite
var thing
var health := 100
var alias := "Character"
var color := Color.gray
func set_vars(__thing, __health := 250, __alias := "Wizard", __color := Color.blue) -> void:
thing = __thing
health = __health
alias = __alias
color = __color
func print_vars() -> void:
for __var_name in ["thing", "health", "alias", "color"]:
print(__var_name, ": ", get(__var_name))
func _ready() -> void:
print_vars()
print("-----")
# should be called by: set_vars(__color = Color.red, __thing = Image.new())
FuncArg.call_using_named_args(self, "set_vars", {"__color": Color.red, "__thing": Image.new()})
print("Default args for set_vars(): ", FuncArg.get_arg_defaults(self, "set_vars"))
print("-----")
print_vars()
The output is:
extends Node
const NO_DEFAULT := "(NO_DEFAULT)"
const NO_TYPE := "(NO_TYPE)"
# after saving 'Player.gd', 'data' should be:
var data := {
"res://Scenes/Player/Player.gd": {
"set_vars": {
"__thing": {"default": NO_DEFAULT, "type_name": NO_TYPE},
"__health": {"default": 250, "type_name": "int"},
"__alias": {"default": "Wizard", "type_name": "String"},
"__color": {"default": Color.blue, "type_name": "Color"},
}
},
}
func get_func_dict(__object: Object, __func_name: String) -> Dictionary:
if !__object:
push_error("Can't call on null object")
return {}
if !__object.has_method(__func_name):
push_error(str("No method named '", __func_name, "' in object: ", __object))
return {}
var __script: Script = __object.get_script()
if !__script:
push_error(str("No script attached to object: ", __object))
return {}
var __script_path: String = __script.resource_path
if !data.has(__script_path):
push_error(str("Invalid script path:", __script_path))
return {}
else:
return data[__script_path]
func get_arg_dict(__object: Object, __func_name: String) -> Dictionary:
var __func_dict := get_func_dict(__object, __func_name)
if __func_dict.empty():
return {}
elif !__func_name in __func_dict:
push_error(str("Invalid func name:", __func_name))
return {}
else:
return __func_dict[__func_name]
func get_arg_names(__object: Object, __func_name: String) -> Array:
var __arg_dict: Dictionary = get_arg_dict(__object, __func_name)
if __arg_dict.empty():
return []
else:
return __arg_dict.keys()
func get_arg_defaults(__object: Object, __func_name: String) -> Array:
var __arg_dict: Dictionary = get_arg_dict(__object, __func_name)
var __arg_defaults := []
for __subdict in __arg_dict.values():
__arg_defaults.append(__subdict["default"])
return __arg_defaults
func get_arg_default(__object: Object, __func_name: String, __arg_name: String):
var __arg_names: Array = get_arg_names(__object, __func_name)
var __arg_defaults := get_arg_defaults(__object, __func_name)
if !__arg_name in __arg_names:
push_error(str("Invalid argument name: ", __arg_name))
return
else:
return __arg_defaults[__arg_names.find(__arg_name)]
func call_using_named_args(__object: Object, __func_name: String, __passed_arg_data: Dictionary):
var __arg_names := get_arg_names(__object, __func_name)
var __arg_defaults := get_arg_defaults(__object, __func_name)
var __passed_arg_names := __passed_arg_data.keys()
var __passed_arg_values := __passed_arg_data.values()
for __passed_arg_name in __passed_arg_names:
if !__passed_arg_name in __arg_names:
push_error(str("Invalid argument name: ", __passed_arg_name, ". Can't call '", __func_name, "'"))
return
var __ordered_args := []
for __i in __arg_names.size():
var __arg_name: String = __arg_names[__i]
var __arg_default = __arg_defaults[__i]
var __required_argument_supplied := true
if str(__arg_default) == NO_DEFAULT:
if !__passed_arg_names.has(__arg_name):
push_error(str("Value not supplied for argument '", __arg_name, "'"))
__required_argument_supplied = false
if __required_argument_supplied:
if __passed_arg_names.has(__arg_name):
__ordered_args.append(__passed_arg_data[__arg_name])
else:
__ordered_args.append(__arg_defaults[__i])
if __ordered_args.size() == __arg_names.size():
return __object.callv(__func_name, __ordered_args) |
I would say benefits of named arguments outway the risks/performance hit. Would help make the code more readable and easier to understand and reduce incidence of bugs. This also would fall in line with all the other Godot 4 changes aimed to make the engine more accessible and easier to use and understand (such as class/member renames, 2D/3D node renames, removing && || etc). |
@Error7Studios the problem is that we don't always know the actual function at compile time, so anything like that would have to happen at runtime (and every time the function is called). The other issue is that release builds strip the argument names, meaning it would be impossible to tell the order of the arguments. @filipworksdev assuming we would add argument names on release builds (which would increase the binary size), the performance hit is very likely not worthy for this feature, because it would make function calls much slower than they are now. In many cases this wouldn't be noticeable, but when calling things inside a loop this can become a problem very easily. While we value usability over performance, in this case I believe it's not worth the trade-off. |
Visual Studio for C# now has an option where the editor shows the parameter names inline as you type or review the file while they aren't really there. Not sure how they do it, but then it doesn't really change the file, it's just a rendering in the editor concern rather than interpreter concern. I think it is called inline parameter hints and is optional. |
@jtrack3d I agree, that's handy and it would be great if we could get this feature in the editor. But named parameters are much more than that as language feature, in that they allow arbitrary ordering of parameters and explicit parameter assignment, where the code is much more self documenting. For me, the easy work around for now is:
I only use option 1 and 3 so far |
For example right now I need to only intersect with areas, so I swap the last two boolean parameters:
Of course that adds a lot of copies of these parameters, and it's not clear which ones are default and which ones are changed. Regarding the technical issues, I'd imagine these parameters should be resolved during "compile time" (when |
Any update on this? It makes the code difficult to read having to count the arguments in the function signature every time I am looking for an argument in particular (especially for functions with several arguments). It looks like a small thing but after a while it definitely starts becoming noticeable. |
If the technical problems in implementing this are too much to be worth contending with, perhaps an alternative could be to have a Not a perfect solution to what named arguments solve as you still have to worry about updating code when you add or rearrange arguments, but it removes one level of overhead in manually passing in the default value so you don't have to track it in a variable or hardcode it to match and manually update it everywhere if the default value changes |
There isn't anything to update, the technical roadblocks I mentioned are still in place and I see no way of overcoming them. I'm not particularly looking for solutions so it'll probably stay like it is. If anyone has a solution in mind I would like to know. In general: There's no way to implement this that is complete and don't affect runtime performance.
I can see some other issues with this, but it's better to be discussed as its own proposal. It is more feasible than this one though. |
Given the current technical limitations, there's simply no feasible way to implement this, so I'm finally closing the proposal. If there's a valid implementation solution we can reassess and reopen (or such implementation project could be sent as a new proposal). |
Not really invested, but to check that I understand correctly: there is a way to make it work at compile time without runtime penalties and the condition for that is that the code should be typed (at least at the place of calling). Why is it considered to be a blocker? Seems like a reasonable requirement and one that adds motivation to use types. |
@vonagam Since you're making the case for when the function signature is known in compile time due to strong typing... in that case, it seems to me that the IDE could be modified to show function arguments in the way modern IDEs do, such as how this plugin does it: Seems to me the same could be done for the built-in script editor, too... and since it's editor time there's no worry about runtime script performance overhead from the perspective of a programmer, what is the difference if the parameter names shown by the IDE, versus if you type them yourself? I imagine that a proposal for improving the script editor in this way could be accepted. |
The proposal isn't about showing names for ordered parameters. It is about ability to pass in any order and skip some default ones. Annotation usage of passing arguments by names is secondary one. In all examples and snippets here people use typed code and it was told that there is no problem to do it at compile time if the code is typed. So I'm trying to understand why was the proposal closed. |
@vonagam there is still the problem that argument names are stripped in release builds, so even with everything typed it couldn't work, at least not for native classes. |
I know this is just a very tangential solution, but it would be easy to implement.
would print
|
Sorry if that's too naive but when argument names are stripped, couldn't named parameters be transformed into positional parameters? |
Requesting a second look at this since keyword arguments are a barebones essential for clean code. Throwing a compiler error on messing up the order of keyword arguments or mixing would be an insanely good compromise for getting this feature in. Like take this function for example: func test(x, y):
... These calls would be legal: test(1, 2)
test(x=1, y=2) These calls would be illegal test(y=2, x=1)
test(x=1, 2)
test(y=2, 1)
|
@Walter-o This would only work if the function is known at compile time. If the argument list is not known in advance, this cannot be solved without a runtime penalty: untyped_var.test(x=1, y=2)
$Node.test(x=1, y=2) |
@dalexeev Ow that's really difficult then. |
In an attempt to get this re-opened, I want to propose a feasible and reasonable way to implement it. I believe in the importance of named arguments for clean and understandable code. I think closing a proposal as important as this one because of implementation feasability is a mistake, especially since many proposals are already lying around, waiting possibly for years until they can finally be picked up with a non-disruptive implementation. But yes, I do have an implementation proposal, too. non kwargs callRegular calls stay as-is (without a kwargs overhead). kwargs call
def map_args_with_kwargs(interface, call_args, call_arg_names):
args = list(interface.params.size())
non_kwargs_size = args.size() - call_arg_names.size()
# Copy over normal args
args[:non_kwargs_size] = call_args[:non_kwargs_size]
args_mask = [True] * interface.params.size()
for name, value in zip(call_arg_names , args[non_kwargs_size:]):
idx = interface.params.get_idx(name)
if idx < 0:
raise "Unknown parameter " + name
if args_mask[idx]:
raise "Multiple values for key " + name
args_mask[idx] = true
args[idx] = value
for idx in range(interface.params.size() - args.size(), None):
if args_mask[idx]:
continue
if !params.has_default_arg[idx]:
raise "Too few arguments for call"
args[i] = params.default_arg[idx]
return args Static analysis optimizationUnlike python, the godot type annotation system ensures that variant types are known. Known-type if kwargs.empty()
produce_non_kwargs_call(args)
if type_annotation is not None:
args = map_args(args, kwargs)
produce_non_kwargs_call(args)
else:
produce_kwargs_call(args, kwargs) Given people interested in Argument Name StrippingAbove, concerns were raised because argument names are currently stripped for release builds. There are 3 ways to go about this (in order of implementation complexity):
ConclusionRuntime costs of regular non-kwargs calls are unaffected. I hope this write-up illustrates that |
@Ivorforce Note that there is no single API for calling functions. GDScript functions are called in one way, utility functions in second way, native functions in third way (in fact there are even more). Also, almost any method/function can be represented as a So we can't solve this only for GDScript, it should also work properly for core, C# and GDExtension. Besides "don't strip argument names" we should make sure that all APIs that can be invoked in GDScript support named arguments. At least through "reflection" ( Regarding performance, note that in GDScript default arguments can be non-constant expressions, unlike in some other languages. The ability to pass arguments by name means that you can skip some optional arguments. This means that we can't simply patch the passed argument array. Instead, we have to replace the jump to the first non-specified default argument (or function body) with a loop or a check chain. This may not be a big deal, but it still affects all functions with optional arguments, regardless of whether you use named arguments or not, and whether the method and its arguments are known at compile time or not. |
Thank you for the considerations. I have some answers to your comments:
The answer, in this proposal, is of course "yes". If (Edit: Though an argument could be made that
Through argument names, they already do! As you pointed out yourself, existing argument names are enough to figure out how to map
This is certainly an interesting sub-problem. After some thinking I believe I have a decent response for that, inspired by ROP jumps. I assume the compiler is currently generating VM code for default parameters, and skipping past the ones supplied by entering into the function later. The code would have to be modified such that after each argument initialization, a jump is performed to the next uninitialized argument. Basically, the GDScript gdextension loads on the stack the entrypoints for each of the missing parameters. The function is then called at the first entrypoint, upon which the initializers are jumped through depending on which are needed for initialization. The current entrypoint can be kept as-is, essentially generating the parameter intializers twice - once for non |
So you're advocating the Python way? I was only thinking of something like JavaScript and PHP: you can pass existing arguments by name, and you can pass extra arguments (for variadic functions) packed into an array. But you can't pass non-existent arguments by name, that would be a compile-time or runtime error. Godot has functions with a Given the above, I think what you proposed can still be quite complex and is something more than what we discussed above (since your proposal affects not only GDScript, but also core, C#, GDExtension).
Well, I think we are sometimes inconsistent about complex proposals. This particular one we closed more because we have no hope of implementing it, not because we don't want to have the feature. While there are other popular proposals that are still open, but frankly have little chance of being implemented in the foreseeable future due to the complexity and amount of refactoring required. It is difficult to measure hope objectively. Maybe we should re-open this proposal, although this still does not mean that we plan to implement it. Or maybe we should close/archive other proposals that have too little chance of being implemented in the foreseeable future. Or use a GitHub project, labels or milestones to categorize proposals by their achievability. But I think you could open a new proposal with new technical information if you think it is possible to address all the concerns. In my opinion, what you are proposing goes beyond the original idea. Footnotes
|
Describe the project you are working on:
Any project.
Describe the problem or limitation you are having in your project:
Each time I add/remove function parameters, change their order, or change which are mandatory, I have to manually hunt for all function calls and update them.
Especially in setup functions that have dependency injection, mode setting etc.
Describe the feature / enhancement and how it helps to overcome the problem or limitation:
The feature allows calling parameters by name.
The future makes changing parameters less of a problem. For instance if I have function calls that use only one parameter of all available ones by name, changing their order or adding new non-mandatory ones does not break the function call.
Describe how your proposal will work, with code, pseudocode, mockups, and/or diagrams:
The future is already existing in python, and works like this:
More detailed explanation: https://treyhunner.com/2018/04/keyword-arguments-in-python/
Here is a practical example of usage:
Same code without this function:
^ And adding new ship parts before the called ones means changing amount of zeroes in the function call every time.
If this enhancement will not be used often, can it be worked around with a few lines of script?:
Nope.
Is there a reason why this should be core and not an add-on in the asset library?:
It's would be part of GDScript.
The text was updated successfully, but these errors were encountered: