-
-
Notifications
You must be signed in to change notification settings - Fork 21.6k
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
Improve some cryptic error messages in Node
#95063
base: master
Are you sure you want to change the base?
Improve some cryptic error messages in Node
#95063
Conversation
@@ -1685,13 +1685,13 @@ Node *Node::get_child(int p_index, bool p_include_internal) const { | |||
if (p_index < 0) { | |||
p_index += data.children_cache.size(); | |||
} | |||
ERR_FAIL_INDEX_V(p_index, (int)data.children_cache.size(), nullptr); | |||
ERR_FAIL_INDEX_V_MSG(p_index, (int)data.children_cache.size(), nullptr, vformat("Invalid child index: %d.", p_index)); |
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.
Taken slightly modified from move_child
, both could be improved but unsure what to add further
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.
Perhaps Failed to get_child() at index %d. The node (preferably a nodename if available), only has [data.children_cache.size()] children.
@@ -2092,7 +2092,7 @@ void Node::set_owner(Node *p_owner) { | |||
_clean_up_owner(); | |||
} | |||
|
|||
ERR_FAIL_COND(p_owner == this); | |||
ERR_FAIL_COND_MSG(p_owner == this, "Invalid owner. Owner cannot be self."); |
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.
I'd suggest Attempted to set invalid owner (to [NodeName] if available). Owner cannot be self
.
The difference is very slight, but I think using attempt
indicates that although you tried, it didn't actually change the owner.
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.
This matches the error blow, so leaving it for consistency
d0294e4
to
fe38795
Compare
8d5a569
to
77d0709
Compare
77d0709
to
673dc3d
Compare
Will look for more potential cases but these are some that stuck out or were mentioned
See: