-
-
Notifications
You must be signed in to change notification settings - Fork 21.7k
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
Change find_node
to find_nodes
and Add an type
parameter.
#56032
Conversation
57578ed
to
d61c6b5
Compare
b08ca30
to
c74fcf9
Compare
Should it be named get_children_of_class() instead? |
|
All suggested changes made, and rebased to current Master. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still needs a review from core team probably.
Yes added PR meeting label as I think it needs broader review before going further, I think things like the naming and parameters are things that the reviewers like to bikeshed on and come to agreement. On the naming at least I would be considering something like "find_nodes_of_type" rather than "get_children_of_type". The difference between get and find seems already set by precedent in the And children could be confusing if you are using the function recursively, which may end up being a common use case. |
We discussed this in a PR review meeting, the added feature seems fine, but we were wondering if it wouldn't be worth refactoring this API to unify the existing |
@akien-mga: Maybe as another PR? This is a feature that would be welcome sooner rather than later |
Godot 4.0 is in alpha, and the further changes required mean breaking compatibility. There's no point merging a feature now in a pre-release branch to break compatibility right after. |
Ah, good point. |
14f0f30
to
b90ce18
Compare
Node.get_children_of_type()
find_node
to find_nodes
and Add an type
parameter.
Suggested changes from the PR Review Meeting have been implemented. I will wait for confirmation that the changes are correct/desired, before pushing the same changes to the 3.x PR. |
Changed 'find_node' to 'find_nodes' which now returns an 'TypedArray<Node>', as well as Added a 'type' parameter to match against specific node types, which supports inheritance.
b90ce18
to
78dc608
Compare
Thanks! And congrats for your first merged Godot contribution 🎉 |
I'm seeing the recursive option not working as intended on 4.0.alpha3. For example, Should I open a new issue for this? |
Why isn't there also a |
You can benefit from type hints with your own function, although I recognize the use case. func get_map(n: Node = self) -> Map:
if n == null:
return null
if n is Map:
return n
return get_map(n.get_parent()) Any other use cases that really shine? |
Changed
find_node
tofind_nodes
which now returns anTypedArray<Node>
, and Added antype
parameter to match against specific node types, which supports inheritance.find_nodes
can be used with justmask
or justtype
or both.type
: Can be anynative
,class_name
, orresource_path
type, similarly to theextends
keyword. (supports inheritance)3.x PR: #56085
Closes: godotengine/godot-proposals#3661
Example Project: FindNodesTest.zip