Skip to content
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

get_script_method_list is missing defaults in some cases #66218

Open
bitwes opened this issue Sep 21, 2022 · 6 comments
Open

get_script_method_list is missing defaults in some cases #66218

bitwes opened this issue Sep 21, 2022 · 6 comments

Comments

@bitwes
Copy link

bitwes commented Sep 21, 2022

Godot version

4.0.alpha17

System information

macOS 10.15.7

Issue description

Some default values do not appear in the script method metadata default_args. When they are missing, there is no way to know which parameters have defaults. For example, the defaults listed in the metadata for

func mixed_example(p1, p2 = {}, p3 = "a", p4 = 1, p5 = []):
	pass

Contains two entries

"default_args": ["a", 1 ],

From this information, you cannot determine how many parameters have been defaulted and which get the default values that are provided.

In order to reliably find defaulted parameters and associate values with them, you need to know

  • the total number of defaulted parameters
  • the indexes of the parameters that appear in default_args

Something like this would work

"default_arg_count": 4,
"default_arg_indexes":[2, 3],
"default_args": ["a", 1 ],

or

"default_args":{
  "count":4,
  "value_indexes":[2, 3],
  "values":["a",1],
}

For my purposes, simply knowing how many parameters are defaulted is extremely useful. I use this information to generate doubles of objects in the GUT testing framework addon.

Steps to reproduce

Given this script at res://demo_missing_defaults.gd

extends SceneTree


func mixed_example(p1, p2 = {}, p3 = "a", p4 = 1, p5 = []):
	pass


func _init():
	var script = load('res://demo_missing_defaults.gd')
	var methods  = script.get_script_method_list()
	for m in methods:
		print('-- ', m.name, ' (', m.default_args.size(), ')')
		print(JSON.stringify(m, '  '))

	quit()

You get the following output:

-- mixed_example (2)
{
  "args": [
    {
      "class_name": "",
      "hint": 0,
      "hint_string": "",
      "name": "p1",
      "type": 0,
      "usage": 262150
    },
    {
      "class_name": "",
      "hint": 0,
      "hint_string": "",
      "name": "p2",
      "type": 0,
      "usage": 262150
    },
    {
      "class_name": "",
      "hint": 0,
      "hint_string": "",
      "name": "p3",
      "type": 0,
      "usage": 262150
    },
    {
      "class_name": "",
      "hint": 0,
      "hint_string": "",
      "name": "p4",
      "type": 0,
      "usage": 262150
    },
    {
      "class_name": "",
      "hint": 0,
      "hint_string": "",
      "name": "p5",
      "type": 0,
      "usage": 262150
    }
  ],
  "default_args": [
    "a",
    1
  ],
  "flags": 1,
  "id": 0,
  "name": "mixed_example",
  "return": {
    "class_name": "",
    "hint": 0,
    "hint_string": "",
    "name": "",
    "type": 0,
    "usage": 6
  }
}
-- _init (0)
{
  "args": [

  ],
  "default_args": [

  ],
  "flags": 1,
  "id": 0,
  "name": "_init",
  "return": {
    "class_name": "",
    "hint": 0,
    "hint_string": "",
    "name": "",
    "type": 0,
    "usage": 6
  }
}

Minimal reproduction project

See Steps to reproduce.

@bitwes bitwes changed the title get_script_method_list includes missing defaults in some cases get_script_method_list is missing defaults in some cases Sep 21, 2022
@MikeSchulze
Copy link

MikeSchulze commented May 16, 2024

This bug is still valid!

func test_parameterized_int_values(a: int, b :int, c :int, expected :int = 1, test_parameters := [
	[1, 2, 3, 6],
	[3, 4, 5, 12],
	[6, 7, 8, 21] ]) -> void:

The default_args contains nullbut should be

  "default_args": [
    1,
    null
  ],
  "default_args": [
    1,
    [[1, 2, 3, 6], [3, 4, 5, 12], [6, 7, 8, 21] ]
  ],

results in

{
  "args": [
    {
      "class_name": "",
      "hint": 0,
      "hint_string": "",
      "name": "a",
      "type": 2,
      "usage": 0
    },
    {
      "class_name": "",
      "hint": 0,
      "hint_string": "",
      "name": "b",
      "type": 2,
      "usage": 0
    },
    {
      "class_name": "",
      "hint": 0,
      "hint_string": "",
      "name": "c",
      "type": 2,
      "usage": 0
    },
    {
      "class_name": "",
      "hint": 0,
      "hint_string": "",
      "name": "expected",
      "type": 2,
      "usage": 0
    },
    {
      "class_name": "",
      "hint": 0,
      "hint_string": "",
      "name": "test_parameters",
      "type": 28,
      "usage": 0
    }
  ],
  "default_args": [
    1,
    null
  ],
  "flags": 1,
  "id": 0,
  "name": "test_parameterized_int_values",
  "return": {
    "class_name": "",
    "hint": 0,
    "hint_string": "",
    "name": "",
    "type": 0,
    "usage": 6
  }
}

@dalexeev
Copy link
Member

This is not exactly a bug. Arrays and dictionaries are passed by reference, and two equal arrays are not the same. So by default (as in the case of function parameters) they are not constant expressions. To be treated as a constant expression, an array must be in constant context, in which case the array will be made read-only.

In the following case it is impossible to get the full "default" value:

func f(x, y = [1, x, 3]): pass
#  Value unknown. ^ 

In the following case, this is potentially possible, but we would need to reduce the expression twice, in the normal context and in the constant context (without generating errors if the expression is not constant):

func f(x, y = [1, 2, 3]): pass

For this reason, in many languages, initializers for global variables, class fields, and function parameters are required to be constant expressions. This also solves the problem with the initialization order at runtime.

@MikeSchulze
Copy link

In the following case, this is potentially possible, but we would need to reduce the expression twice, in the normal context and in the constant context (without generating errors if the expression is not constant):

Ok ... ;), what this mean at the end?
Arrays and Dictionaries will be not supported as default arguments, unrelated is it a constant expression or not.

Is there another way to grab the default arguments without writing your own parsers?

@dalexeev
Copy link
Member

I think this can be implemented without significant refactoring. Perhaps we could reuse the function make_expression_reduced_value() that is used when resolving constants. I just draw your attention to the fact that this is impossible in general case. We don't have an unknown value, so we use null instead. The question is what should be instead of [1, x, 3]: [1, null, 3] or just null. I guess the latter because it's less confusing.

@MikeSchulze
Copy link

Ok thanks, unfortunately that would only partially help me, but better than nothing ;)
I don't think zero is so good, because you can't tell whether the value was initially zero or simply couldn't be resolved.

@jaredstorm
Copy link

jaredstorm commented Nov 4, 2024

Bump on this issue. Are there any updates?

Right now I'm implementing some custom mocking tools, and need to be able to get an accurate description of a function's signature, default args and all.

At least as of v4.4.dev2.official.97ef3c837 default values that are dictionaries and arrays are still treated as <null> which means the following two funcs are reported as having the same default args [<null>, <null>, <null>]

func _init(a=null, b=null, c=null):
  pass

...

func _init(a={}, b=[], c=null):
  pass

More full context:
I'm working on making it so that my mocking tools can keep around a hidden instance of the class they're mocking so they can reference default implementations when needed. I'm really hoping to be able to validate and push errors when a mock script's instantiated with the wrong arg types or wrong number of args (since the mock script will also be passing those _init args to the real script), then fall back and create a mock instance without default implementations.

Unfortunately, to do that, I need to know if there's a signature mismatch, and I can only do that if I can dynamically determine what the default args of the real class constructor are. In the case above, I'd end up passing in null, which creates issues for code expecting arrays and dicts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Up for grabs
Development

No branches or pull requests

5 participants