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

Clarify doc: Node.get_child returns null for invalid index #86349

Merged
merged 1 commit into from
Jan 8, 2024

Conversation

DSteve595
Copy link
Contributor

This adds a line to Node.get_child's doc explaining that if no child exists at the specified index, null will be returned.

I found myself repeatedly writing code that checks for get_child_count() >= (n + 1) before calling get_child(n). I had a wrong assumption that calling get_child with an out-of-bounds index would raise an error rather than returning null, as many collections do in other languages/frameworks. I think this would be helpful and relevant to mention in the doc.

@DSteve595 DSteve595 requested a review from a team as a code owner December 20, 2023 02:27
doc/classes/Node.xml Outdated Show resolved Hide resolved
@dalexeev dalexeev added enhancement documentation cherrypick:4.1 Considered for cherry-picking into a future 4.1.x release cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release labels Dec 20, 2023
@dalexeev dalexeev added this to the 4.3 milestone Dec 20, 2023
@dalexeev
Copy link
Member

I had a wrong assumption that calling get_child with an out-of-bounds index would raise an error rather than returning null

Both will happen. The debugger will not be stopped, but the error will be logged.


@DSteve595
Copy link
Contributor Author

You're right! Updated.

@dalexeev
Copy link
Member

Please squash commits into one. To do this you need a local git client, GitHub web interface doesn't support it.

@DSteve595
Copy link
Contributor Author

Squashed.

doc/classes/Node.xml Outdated Show resolved Hide resolved
@akien-mga akien-mga changed the title Clarify doc: Node.get_child returns null for invalid index Clarify doc: Node.get_child returns null for invalid index Jan 3, 2024
@akien-mga
Copy link
Member

Needs rebase after #68560, provided it's still needed after the changes in that PR. CC @Mickeon for review.

@Mickeon
Copy link
Contributor

Mickeon commented Jan 4, 2024

It's still good to have, yeah, even with the new example code. It would need to be reworded slighly to match the new sentence.

@akien-mga akien-mga removed the cherrypick:4.1 Considered for cherry-picking into a future 4.1.x release label Jan 4, 2024
@DSteve595
Copy link
Contributor Author

Rebased and reworded "error is logged" to "error is generated". Any other changes?

@Mickeon
Copy link
Contributor

Mickeon commented Jan 7, 2024

I like it, myself!

@akien-mga akien-mga merged commit ff79ec7 into godotengine:master Jan 8, 2024
15 checks passed
@akien-mga
Copy link
Member

Thanks!

@DSteve595 DSteve595 deleted the patch-1 branch January 8, 2024 14:11
@YuriSizov YuriSizov removed the cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release label Jan 25, 2024
@YuriSizov
Copy link
Contributor

Cherry-picked for 4.2.2.

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.

5 participants