godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
89.12k stars 20.2k forks source link

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

Open Calinou opened 3 years ago

Calinou commented 3 years ago

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:

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 https://github.com/godotengine/godot/issues/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 :slightly_smiling_face:

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 commented 3 years ago

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

Anutrix commented 3 years ago

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 https://github.com/godotengine/godot/blob/6c173e2f7ffdb972ec3754b3d78fe51bfbfb4f22/core/bind/core_bind.cpp#L1374 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 commented 3 years ago

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

Anutrix commented 3 years ago

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

Calinou commented 3 years ago

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

kyleguarco commented 3 years ago

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 commented 3 years ago

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

AaronRecord commented 3 years ago

Godot 3.2.3, Windows 10, GLES 2

image

Calinou commented 3 years ago

@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 :slightly_smiling_face:

AaronRecord commented 3 years ago

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

Calinou commented 3 years ago

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 :slightly_smiling_face:

Flarkk commented 3 years ago

Another one : Non explicit error message when setting active a Viewport that is already active #50382

yankscally commented 3 years ago

@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 commented 1 year ago

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

Calinou commented 3 months ago

Reopening, so that users can report additional cryptic error messages.

Arecher commented 3 months ago

Going through some recent development screenshots of errors and the like, so I'm not 100% sure what caused this. But this is a good example of errors that do not give enough information to be appropriately tracked down and solved by the average Godot user, without wasting hours debugging and trying to find the potential cause via process of elimination.

image

  1. Upon reading I'm not sure what this error means. Probably a text_edit is trying to set the caret to a position of -1? That'd be my best guess, but that's not actually what it says.
  2. What node? What location? What script tried to set this to begin with? What is the text in the lineedit? Even if I understand that somewhere in my code I'm setting the caret of a lineedit wrong, that doesn't mean I know which of my currently spawned 30 lineedits causes this particular issue!

Godot 3.5, Windows 10

Arecher commented 3 months ago

image

  1. What's the parent node that we're performing get_child() on? What script/function called this?

I completely understand what's going wrong, but the error doesn't lead me to being able to fix anything in it's current state. Especially since this could be caused by any of the MANY get_child() or $ references in an entire project.

This same error essentially happens for move_child() etc

Arecher commented 3 months ago

image

  1. What timer? What's it called, what is it's parent node? What tried to set this invalid Time?
Arecher commented 3 months ago

image

  1. What image (if saved)? Where (script, function) is it being used/created?

Haven't had this one be an issue as often, it's often fairly easy to track down, because you're unlikely to be working with multiple in-progress functions that create images. But it fits in the list nonetheless. If somewhere in your project a random condition appears that makes an image 0px in height, I'd love to know where that came from

Arecher commented 3 months ago

This same error essentially happens for move_child() etc

Specifically this one image

  1. What were we trying to add? To what parent node? What is that parents path? What script/function tried to add this?
Arecher commented 3 months ago

image

  1. What method did we attempt to call? What tried to call it? What node are we trying to call it in?
Arecher commented 3 months ago

image image image

This one is rarely an issue, due to the user made signal name making it fairly easy to track down. But it would still be very nice to know what node/script it is trying to find this signal in. Especially since this error can also show up for default Godot signals, in which case Nonexistent Signal: texture_changed doesn't give you any leads on where this might be going wrong.

  1. What node is this signal being called in? Paths are always great.

The second one saying "that object" really adds to the sadness

Arecher commented 3 months ago

image

  1. What node is this tween being started from?
Arecher commented 3 months ago

image

  1. What function/node called this? What's the name/type of the node that isn't inside the tree?

One of these cases where this could apply to any node in your project, and there is absolutely 0 indication of where to even start looking.

Arecher commented 3 months ago

image

  1. What is the path of the file we're trying to open? What's the directory this file is in?

Usually not an issue that takes long to find/fix, but it should just be part of the error message to help you track down the correct issue faster.

Arecher commented 3 months ago

image

  1. No clue what it even means. From the notes it seemed to be an issue with add_child() on a separate thread. Nothing here even hints at that being a potential causation.
  2. Where did this originate from? What function/script/node? I understand that's probably hard to pass, because this seems a very in-engine error, but as a user that receives this, I have no clue what I'm doing wrong, what even caused this to happen, what the consequences are, or how to fix it.
MYNAMEISRADU commented 3 months ago

Godot 4.2.1

image servers/rendering/renderer_rd/storage_rd/mesh_storage.cpp:777 - Parameter "mesh" is null. is the error that gets continuously thrown in my project. The errors get thrown in one specific scene whenever I open it or when I just switch scene and switch back to this scene, around 90ish of em each time.

I've pinpointed the error to be due to the GridMap nodes I am using, specifically the ones sitting under the NavigationRegion3D node image If I remove some of the nodes the amount of errors gets reduced, and if I delete them all the errors are gone. I have other GridMap nodes outside of the NavigationRegion3D node but they don't cause any problems. Moving the GridMap nodes outside of the NavigationRegion3D node and deleting the latter does not remove the errors, either.

https://github.com/godotengine/godot/assets/38227348/e991219c-68a2-479e-9228-b55261833e4b

The amount of errors seems quite arbitrary, I tried adding 7 new tiles to the scene and it only added 4 more errors upon refresh. Deleting these tiles also causes an error each time, but only if I delete them after refreshing the scene. But it gave an error upon deleting any of them, not just 4 of them.

This is about all the testing I've done, no mesh is missing from the Mesh Library, so I have no leads on what this could mean.

AThousandShips commented 3 months ago

Does it still happen in 4.2.2?

Arecher commented 3 months ago

image

  1. I don't even know what this means, or what it relates to.
Arecher commented 3 months ago

image

  1. A nice engine error. No clue where to even start looking on how to solve this one.
Arecher commented 3 months ago

image

  1. What node(path)? What was this RPC and the function it is calling? Where did the RPC originate from?
Arecher commented 3 months ago

image

  1. What node made this rpc? What function is being called? What script?
Arecher commented 3 months ago

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 masterkeyword 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 commented 3 months ago

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 commented 2 months ago

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 commented 2 months ago

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 commented 1 month ago

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 commented 1 month ago

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 https://github.com/godotengine/godot/pull/95063)

Arecher commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

Godot 4.2.2 image

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

Arecher commented 1 month ago

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 commented 1 month ago

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

Arecher commented 1 month ago

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 commented 1 month ago

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

akien-mga commented 1 month ago

Parameter "version" is null. on shader compilation errors: https://github.com/godotengine/godot/issues/95443#issuecomment-2284851316

Arecher commented 1 month ago

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 commented 1 month ago

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 commented 1 week ago

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.