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

Support generic types in the extension API dump #64469

Closed

Conversation

raulsntos
Copy link
Member

@raulsntos raulsntos commented Aug 15, 2022

Summary

Currently we're using {"type: "typedarray::Node"} to represent a typed array.
This PR changes this to: {"type": "Array", "generic_type_args": [{"type": "Node"}]}.

Changing the value of type breaks compatiblity.

Details

This is an alternative PR to #64249 suggested by @Bromeon in #64249 (comment)

Adds optional array_type field to specify the type of the array element when type is Array (for example in Node::get_children).

The JSON for Node::get_children with this PR is now:

{
	"name": "get_children",
	"is_const": true,
	"is_vararg": false,
	"is_static": false,
	"is_virtual": false,
	"hash": 873284517,
	"return_value": {
		"type": "Array", // <-- This is modified with this PR
		"generic_type_args": [  // <-- This is added with this PR
			{
				"type": "Node"
			}
		]
	},
	"arguments": [
		{
			"name": "include_internal",
			"type": "bool",
			"default_value": "false"
		}
	]
},

If the generic_type_args is not present the type does not support or does not have generic type arguments, otherwise it contains an array with a dictionary object for each generic type argument. The dictionary object has a type field with the type name for the generic type argument and can optionally also include a generic_type_args field if the type is generic as well.

Currently only Arrays are supported so the generic_type_args will never be present for Arrays of Variants and will always have one generic type argument when the element type is specified. If Dictionaries support key/value types in the future they should have two generic type arguments and the order is important, if we only want to specify one argument (key or type) the other could have "Variant" in the type field or be an empty dictionary object.

Alternative design

Adds support for generic typed arrays since they are the only type with
generic type arguments supported at the moment in Godot.
@raulsntos
Copy link
Member Author

Closing for now:

  • The changes in this PR break compat and we'd rather not do that unnecessarily.
  • Generic typed arrays are already supported, although it uses a different format (see [GDExtension] Implement support for typed arrays. #65817). So this PR doesn't allow anything that isn't already possible.
  • If/when we need to add additional information to the API dump, we can re-asses the format.
  • The current format (typedarray::Node) needs to be documented.

@raulsntos raulsntos closed this Aug 4, 2023
@raulsntos raulsntos removed this from the 4.x milestone Aug 4, 2023
@raulsntos raulsntos deleted the extension_api_typed_arrays_alt branch August 4, 2023 15:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant