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

[TRACKER] Error macros with cryptic messages due to not having proper custom messages defined #42719

Open
4 of 8 tasks
Calinou opened this issue Oct 11, 2020 · 53 comments
Open
4 of 8 tasks

Comments

@Calinou
Copy link
Member

Calinou commented Oct 11, 2020

If you stumble upon difficult-to-understand error messages (regardless of your skill level with Godot), please report them here. In your comment, make sure to state:

  • Your operating system and Godot version.
  • What you were doing when you got the error message. Always include code samples!
  • Ideally, a minimal reproduction project that returns the error message.

Note that this issue isn't about reporting bugs, but reporting cryptic error messages. This issue is about improving error messages so users can figure things out by themselves more easily. If you stumble upon a bug, please open a dedicated issue instead of commenting in this tracker issue.

See #24199 for background.

Issues

For contributors

Even if you're a beginner in C++, you should be able to add error macros. This is why this issue has a junior job label 🙂

To add error explanations to existing errors, search for their origin in the source code (by reading the automatically-generated error message), suffix the error macro with _MSG and add a parameter with a custom message at the end.

We recommend adding context (such as provided arguments versus expected ones) whenever possible. Some examples:

# Best, do this whenever possible:
"Can't listen server on port 420000. The port number must be between 1 and 65535."

# OK:
"Can't listen server on port 420000."

# Bad, this doesn't give much context:
"Can't listen server."

Here are some real-world examples of improving error explanations: https://github.com/godotengine/godot/pull/41352/files, https://github.com/godotengine/godot/pull/35862/files

See Error macros in the documentation for more information.

@theoway
Copy link
Contributor

theoway commented Oct 11, 2020

Add #41493 . I've also made a PR for the same

@Anutrix
Copy link
Contributor

Anutrix commented Oct 25, 2020

File.eof_reached()` causes infinite while-loop on Android when read/write permissions aren't set #42959

Any idea where the error macro call should be? Is it at

ERR_FAIL_COND_V_MSG(!f, String(), "File must be opened before use.");
replaced by ERR_FAIL_COND_V_MSG(!f, String(), "File must be opened before use. In case you are on the Android, check if the app has file read-write permission.");.
Or should the error be somewhere in Android platform-specific part of codebase?

@Calinou
Copy link
Member Author

Calinou commented Oct 25, 2020

@Anutrix Maybe we should change the error message with an #ifdef depending on the target platform?

@Anutrix
Copy link
Contributor

Anutrix commented Oct 25, 2020

What's the difference between __ANDROID__ or ANDROID_ENABLED? I'm assuming one of these must be used.

@Calinou
Copy link
Member Author

Calinou commented Oct 25, 2020

@Anutrix We probably want to use ANDROID_ENABLED as we use <platform>_ENABLED for other platform defines.

@kyleguarco
Copy link

kyleguarco commented Nov 13, 2020

I would just like to point out that the links at the bottom of the error macros page in the master branch docs are dead as of #43385

@akien-mga
Copy link
Member

Yes, most references to core/ files in https://docs.godotengine.org/en/latest/development/cpp/index.html will need to be updated.

@AaronRecord
Copy link
Contributor

AaronRecord commented Mar 7, 2021

Godot 3.2.3, Windows 10, GLES 2

image

@Calinou
Copy link
Member Author

Calinou commented Mar 7, 2021

@LightningAA "high-end platform" means GLES3. We intentionally use a generic term as there may be several high-end platforms in the future (such as Metal or Direct3D 12).

That said, "high-end renderer" or "high-end backend" would be a more fitting term here. Also, since 3.2.x won't get any additional renderers, it makes sense to reference GLES3 there instead. I just don't think this will apply to 4.x and later 🙂

@AaronRecord
Copy link
Contributor

AaronRecord commented Mar 7, 2021

I think it would make a lot more sense if it said something like GLES2 does not support Sampler3D.

@Calinou
Copy link
Member Author

Calinou commented Mar 7, 2021

I think it would make a lot more sense if it said something like GLES2 does not support Sampler3D.

The new error message I added today should fit the bill 🙂

@Flarkk

This comment has been minimized.

@yankscally
Copy link

@Calinou

JacobianEntrySW: Condition "mAdiag <= realt(0.0)" is true.

I'm getting hundreds of these errors per second. I'm using lots of connected RigidBodys, with (mostly) 6DOA joints. GodotPhysics mode.

I have absolutely no idea what this means. I know its something to do with joints, but no idea what. The message is too abstract to fix the issue. This clogs up memory - after 10 mins my project will crash.

Here is the entire paste from the debugger

E 0:00:00.967 JacobianEntrySW: Condition "mAdiag <= realt(0.0)" is true.
<C++ Source> ./servers/physics/joints/jacobianentrysw.h:91 @ JacobianEntrySW()

@akien-mga
Copy link
Member

Closing as there's not much actionable anymore in this old issue, so it's misleading as a good first issue.

@Arecher
Copy link

Arecher commented Jun 18, 2024

image

  1. What node made this rpc? What function is being called? What script?

@Arecher
Copy link

Arecher commented Jun 18, 2024

This error is already fairly usable in comparison, but it has an issue with not being specific enough about WHY it appears. The fact that it gives the nodepath AND the RPC function name makes it super helpful already.

image
This error happens when the function doesn't exist on this id.

image
And this error happens when the function exists, but doesn't have the master keyword or the puppet keyword on one of the two sides.

These two should always clearly state the exact reason that they are failing, rather than a generalization. Right now it requires looking at the functions on the client and server, and checking whether it's on the right node, the right function, whether they all have keywords. It can fail in multiple difference places with the same error, which makes this one more effort to fix than it realistically should.

'mode is 0, master is 1' means almost nothing to me. I've been working with rpcs for years now, but I still do not understand what it's trying to tell me. It always says the exact same thing, without clearly explaining where things went wrong or what the issue is. All I know is that there is an issue with the setup, probably a wrong keyword on one of the two ends. It would be extremely helpful if this was more readable and understandable to the average user, and if the errors were segmented a little more for specific common issues.

@Arecher
Copy link

Arecher commented Jun 18, 2024

image

  1. In what scene?

And since this is basically always caused by changes in an edited scene, it should probably mention this. Telling a node has just vanished is pretty scary to a new user. I believe it was also changed to move the node to the root of the scene, so it should probably mention this instead.

@Calinou
Copy link
Member Author

Calinou commented Jul 8, 2024

I don't even know what this means, or what it relates to.

Regarding this "Response header too big." message, were you using an external script editor when it happened? If so, which one (and which version of their respective extension were you using)? Were you doing anything specific in the external editor when the editor printed the message?

@Arecher
Copy link

Arecher commented Jul 9, 2024

Regarding this "Response header too big." message, were you using an external script editor when it happened?

Nope, we only use the Godot Editor for our scripts. This error was something my co-developer had in Godot, and when I asked him if he had any clue what could be causing it, he said he had no idea. So I'm afraid I don't currently have any other info. If we ever figure it out, I'll let you know.

@Arecher
Copy link

Arecher commented Aug 2, 2024

Godot 4.2.2

image

This seems to mean that you're trying to set a Node's owner to it's current owner. Basically, you're doing something inefficient. It should just say something along the lines of:

Trying to set Node as the owner of Node2, but Node is already the owner of Node2

`Although frankly, it would be better if it just:

1. Set it anyway, and didn't complain.
2. Skipped it.
3. Gave a warning instead.

I see no reason for this to throw an 'must fix' error, since setting any other variable to it's current value is allowed throughout the Engine.

@AThousandShips
Copy link
Member

AThousandShips commented Aug 2, 2024

This seems to mean that you're trying to set a Node's owner to it's current owner

No it means trying to set the owner of the node to itself, not its owner, it should have a description though saying that, it's equivalent to the GDScript owner = self (this one now has a PR improving the message #95063)

@Arecher
Copy link

Arecher commented Aug 2, 2024

No it means trying to set the owner of the node to itself

You're completely right, we read the script wrong. Setting a node as it's own owner was the first thing we suspected, but then we wrongly ruled that out. In that case it's definitely error worthy, but should more clearly state the mistake.

Cannot set Node as it's own owner, at path: NodePath

Or something along those lines!

@AThousandShips
Copy link
Member

AThousandShips commented Aug 2, 2024

Didn't add paths to the error messages in this PR, that's a major change in general to error messages so should be handled in a dedicated PR IMO (it's also not always helpful, or available)

I think for adding node paths or names to errors it might be useful to instead add some helper macros or functions to make it standardized, rather than adding paths in various places, that would help clarify things and make it robust (the existing internal get_description method could be useful for this)

@Arecher
Copy link

Arecher commented Aug 2, 2024

I think for adding node paths or names to errors it might be useful to instead add some helper macros or functions to make it standardized, rather than adding paths in various places, that would help clarify things and make it robust (the existing internal get_description method could be useful for this)

I completely agree, although such a large change will still take quite a while to be implemented, and would probably require a revisit to nearly all errors in the Engine. So for the meantime, if the information such as NodePaths or Node Names, etc are available, I would definitely add them. The manual additions can always be removed once a PR for such a standardized function/macro is implemented.

Especially for extremely generic functions such as move_child()/get_child() this type of information can be extremely valuable, since they can crop up and be caused by one of thousands of calls.

@Arecher
Copy link

Arecher commented Aug 2, 2024

Godot 4.2.2
image

Unsure what caused this one, or what it's referring to.

@Arecher
Copy link

Arecher commented Aug 2, 2024

Godot 4.2.2
image

When calling replace_by() with as argument get_parent().
Didn't actually encounter this one in the wild, I was looking at the C++ code and doing some testing, but it's a great example of something that might be hard to figure out unless a readable message is supplied. Frankly all the ERR_FAIL_COND_ errors could probably do with a custom message to make them easier to understand, even if additional clarification via node names/node paths/additional arguments is something that can only be added in the future.

Failed to replace node [NodeName / at NodePath] with [NodeName / NodePath] . Replacement/new node cannot be an ancestor of the original node.

@AThousandShips
Copy link
Member

AThousandShips commented Aug 2, 2024

That's not what the error checks, the node replacing with cannot have a parent, will add it to my PR

@Arecher
Copy link

Arecher commented Aug 2, 2024

Godot 4.2.2
image

Somewhere along the line something failed to instantiate. Would love information on what resource or scene failed, so I can make sure to reimport/restart, etc.

Pretty sure somewhere a script failed to parse temporarily, which caused multiple scenes to become invalid. While fixing the script would solve the errors, it would still be helpful to know which scenes failed and why. Otherwise these errors don't add much, apart from snowing under the real culprit.

@AThousandShips
Copy link
Member

Please show the surrounding errors, they might say what's going on

@akien-mga
Copy link
Member

Parameter "version" is null. on shader compilation errors: #95443 (comment)

@Arecher
Copy link

Arecher commented Aug 16, 2024

Godot 3.5
image

Ran an OS.execute() command, probably with some wrong arguments. Hard to tell from this message!
Turns out I was actually looking for OS.shell_open(). If this error is specifically for trying to execute a non-executable file, pointing to this function as an alternative in the error message might be useful.

@Arecher
Copy link

Arecher commented Aug 19, 2024

Godot 3.5
image

Attempted to execute Godot through the command line, while opening a specific scene. Guess the scene path was wrong, or something like that.

@Arecher
Copy link

Arecher commented Sep 11, 2024

Godot 3.5
image

The Object was deleted while awaiting a callback message should really be a warning or error, and could use more information on what the 'object' and the 'callback' are/were. Right now the only way to figure out what caused this is by looking at every Container in the entire project.

@Calinou
Copy link
Member Author

Calinou commented Sep 18, 2024

Ran an OS.execute() command, probably with some wrong arguments. Hard to tell from this message! Turns out I was actually looking for OS.shell_open(). If this error is specifically for trying to execute a non-executable file, pointing to this function as an alternative in the error message might be useful.

The error message was improved in 4.x. I just backported it to 3.x: #97171

Note that no error message is present on Linux (and possibly macOS) when running unknown commands on both 4.x and 3.x currently. We should aim to fix that for consistency.

Attempted to execute Godot through the command line, while opening a specific scene. Guess the scene path was wrong, or something like that.

load_default_certificates() is a method that is meant to be called only once per engine session:

void CryptoMbedTLS::load_default_certificates(const String &p_path) {
ERR_FAIL_COND(default_certs != nullptr);
default_certs = memnew(X509CertificateMbedTLS);
ERR_FAIL_NULL(default_certs);

main.cpp calls load_default_certificates() twice, but in mutually exclusive conditions. This means it should never be called twice in a single session in practice. This hints at a bug somewhere in the main.cpp logic when using an invalid scene path (which has been known to cause some variables in that file to go haywire).

The Object was deleted while awaiting a callback message should really be a warning or error, and could use more information on what the 'object' and the 'callback' are/were. Right now the only way to figure out what caused this is by looking at every Container in the entire project.

For context, this was originally a warning, but it's always been commented out:

image

However, we can't use Godot's error/warning system here to avoid infinite recursion, so we'll need to manually replicate the error/warning formatting using ANSI escape codes (which is feasible, but slightly tedious).

I'm not sure if we can call the Object's _to_string() method here as it's already been deleted when the message is printed. We might be able to print something about the callback, but that's about it.

@SpockBauru
Copy link

Got this while messing with visibility range (begin/end margin and fade) of several objects scattered across the map:

E 0:00:11:0729   texture_create: Too many mipmaps requested for texture format and dimensions (5), maximum allowed: (4).
  <C++ Error>    Condition "required_mipmaps < format.mipmaps" is true. Returning: RID()
  <C++ Source>   servers/rendering/rendering_device.cpp:712 @ texture_create()

image

Which texture? The name would help greatly.
If possible, the faulty mesh and world position would also help, but just the name of the texture would help by a lot.

@AThousandShips
Copy link
Member

This is a known limitation, the rendering server doesn't know the texture file or anything else like that (nor is it guaranteed to even be known), see:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Development

No branches or pull requests