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

Improve wording for "Invalid get index" error #46214

Closed
ttencate opened this issue Feb 19, 2021 · 2 comments
Closed

Improve wording for "Invalid get index" error #46214

ttencate opened this issue Feb 19, 2021 · 2 comments

Comments

@ttencate
Copy link
Contributor

ttencate commented Feb 19, 2021

Godot version:
3.2, but the same wording is used in master.

Issue description:
I lurk in the Godot subreddit, and one of the (if not the) errors that beginners encounter most frequently is of the form "Invalid get index 'X' (on base 'Y')". Now, I'm an experienced programmer with many languages, but without any context this message would make little sense to me either! I think everyone would be helped if we could make this error clearer.

First improvement:

Simply rephrase the error as Value of type 'Y' does not have attribute 'X'. This is similar to Python. It also puts the arguments in the same order as they appear in the code.

If the GDScript interpreter can tell whether it's a property access or method call, it could be specified as Value of type 'Y' does not have (property|method) 'X'.

Second improvement:

Add a dedicated message for the common case of Y == null, such as Tried to access attribute 'X' on a value that is null.

Again, if we know the type of access, say access property or call method instead.

Third improvement: (moonshot)

This would be icing on the cake, but might be taking it too far. One of the most frequent causes of X being null is that a particular node was not found. So if X is either $node or a get_node call with a literal path, the error could be reported as Node 'N' not found relative to 'P', where N is the NodePath of the node that doesn't exist, and P is the NodePath of self.

I'd be happy to write PRs for this if the Godot devs agree. Particularly interested in input from GDScript guru @vnen of course!

@Calinou
Copy link
Member

Calinou commented Feb 20, 2021

This would be icing on the cake, but might be taking it too far. One of the most frequent causes of X being null is that a particular node was not found. So if X is either $node or a get_node call with a literal path, the error could be reported as Node 'N' not found relative to 'P', where N is the NodePath of the node that doesn't exist, and P is the NodePath of self.

This is definitely a good idea as per the advice of #42719 🙂

@ttencate
Copy link
Contributor Author

Wow, you just went ahead and did something even better. Many thanks, LGTM! 😀

Calinou added a commit to Calinou/godot that referenced this issue Feb 22, 2021
- Mention the origin of the `get_node()` call.
- Mention whether the attempted path is absolute or relative.

See godotengine#46214.
akien-mga pushed a commit that referenced this issue Feb 22, 2021
- Mention the origin of the `get_node()` call.
- Mention whether the attempted path is absolute or relative.

See #46214.

(cherry picked from commit e6abdc9)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants