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

Node.GetChildren should return an array of Node, not Object in C#. #18773

Closed
PJB3005 opened this issue May 10, 2018 · 6 comments · Fixed by #57824
Closed

Node.GetChildren should return an array of Node, not Object in C#. #18773

PJB3005 opened this issue May 10, 2018 · 6 comments · Fixed by #57824

Comments

@PJB3005
Copy link
Contributor

PJB3005 commented May 10, 2018

Issue description:
Right now, it returns an object[] (it'd be an Array<object> if #15880 went through), which isn't amazing because the objects are always Nodes

Yes, this would require some changes to the binding generator.

@neikeq
Copy link
Contributor

neikeq commented May 10, 2018

I wonder if we could create a type safe Array template class that wrapped Array. It would only wrap it, pretty much the same as the generic Array in #15880 does.
This way the C++ method would be Array<Node> _get_children() instead, and the bindings generator would know about the type of the elements.
@reduz WDYT?

@neikeq
Copy link
Contributor

neikeq commented May 10, 2018

I must add that once #15880 goes through, you will be able to do var children = (Array<Node>)GetChildren();, which improves the situation. However, it would be better if the method signature returned a generic Array in the first place.

@akien-mga akien-mga modified the milestones: 3.1, 3.2 Jan 9, 2019
@akien-mga
Copy link
Member

Is this still relevant in the current master branch?

@mysticfall
Copy link
Contributor

Seems to be so, albeit in a slightly different form:

public Godot.Collections.Array GetChildren()

@akien-mga akien-mga modified the milestones: 3.2, 4.0 Nov 14, 2019
@akien-mga
Copy link
Member

akien-mga commented Jan 24, 2022

In 4.0 alpha 1, the C++ method is:

TypedArray<Node> Node::_get_children(bool p_include_internal) const;

This seems to be what would be needed to solve this issue. Is it handled properly on the Mono side too or does it still require dedicated development work? @godotengine/mono

@raulsntos
Copy link
Member

raulsntos commented Jan 25, 2022

This is still not solved. The bindings_generator still generates a method that returns a Godot.Collections.Array in that case, instead of a Godot.Collections.Array<Node>.

I don't think this can be fixed by the mono module since ClassDB does not give us enough information, currently the extension_api.json contains this data about the get_children method:

{
	"name": "get_children",
	"is_const": true,
	"is_vararg": false,
	"is_virtual": false,
	"hash": 172413545,
	"return_value": {
		"type": "Array"
	},
	"arguments": [
		{
			"name": "include_internal",
			"type": "bool",
			"default_value": "false"
		}
	]
}

Related: #26029

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants