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

ERR_FAIL_COND_MSG sometimes doesn't print message #32276

Closed
KoBeWi opened this issue Sep 23, 2019 · 5 comments · Fixed by #33517
Closed

ERR_FAIL_COND_MSG sometimes doesn't print message #32276

KoBeWi opened this issue Sep 23, 2019 · 5 comments · Fixed by #33517
Milestone

Comments

@KoBeWi
Copy link
Member

KoBeWi commented Sep 23, 2019

Godot version:
fb12f54

Issue description:
So, uh, this happened
image

I'm not sure if it's possible to reliably reproduce it with minimal project.

@KoBeWi
Copy link
Member Author

KoBeWi commented Sep 23, 2019

Okay, it's possible:
ReproductionProject.zip

Just run the project and spam Space. Sometimes there's wrong message.

@qarmin
Copy link
Contributor

qarmin commented Sep 23, 2019

I see these messages all the time.
Screenshot_20190923_151859

@profan
Copy link
Contributor

profan commented Sep 23, 2019

Also seems to have been noted in #32143 (that it sometimes does one, and sometimes the other), this might be in part because the current implementation:

godot/core/error_macros.h

Lines 276 to 284 in f1146c2

#define ERR_FAIL_COND_MSG(m_cond, m_msg) \
{ \
if (unlikely(m_cond)) { \
ERR_EXPLAIN(m_msg); \
_err_print_error(FUNCTION_STR, __FILE__, __LINE__, "Condition ' " _STR(m_cond) " ' is true."); \
return; \
} \
_err_error_exists = false; \
}

... of the various _MSG macros might be kind of silly, I can go back and make them not use ERR_EXPLAIN as an in-between and directly set the error string instead of using _err_set_last_error indirectly through ERR_EXPLAIN.

(at the time I figured getting this change through at all so people would start employing it everywhere in godot mattered most, but I can refine it now to not do unnecessary work)

but also if this implementation doesn't work, it must mean that ERR_EXPLAIN + the normal error macros before also didn't work sometimes, as it's essentially the exact same concept here, only condensed into a single macro.

@Xrayez
Copy link
Contributor

Xrayez commented Sep 23, 2019

Might be related: #31832 (comment).

@akien-mga akien-mga added this to the 3.2 milestone Sep 23, 2019
@profan
Copy link
Contributor

profan commented Sep 23, 2019

Also is anyone aware of how long ago the C Error: etc, C Function: etc part was stripped from godot? (or is it still there somewhere..?)
image (from a build where it's still around, admittedly almost a year old)

If we don't care about that part fixing this current problem isn't hard, but it seems like a weird thing to have removed when having both the error cond and the message itself is quite useful in the debugger?

Found the commit: #21884

I guess I'll PR the simple fix for now and we can consider if maybe adding back the condition and function metadata is useful :)

edit: (scratch all this, need to debug the error message stuff I guess 🤔, new PR later)

akien-mga added a commit to akien-mga/godot that referenced this issue Sep 25, 2019
This was removed by @RyanStein in godotengine#21884 in the case where an error
message is provided, but this is actually useful information to have
even when there is a custom error message.

This PR makes it so that the "C++ Error" is shown whenever there is
a custom error message provided.

Also adds method name to the error item title, and re-adds the most
relevant info in the tooltip for quick error checks without expanding.

Renames C Error/Source to C++ Error/Source, since that's what it is.
And fix untranslatable entry due to misuse of TTR().

And some more cleanup for readability.

Cf. godotengine#32276 (comment)
pchasco pushed a commit to pchasco/godot that referenced this issue Oct 23, 2019
This was removed by @RyanStein in godotengine#21884 in the case where an error
message is provided, but this is actually useful information to have
even when there is a custom error message.

This PR makes it so that the "C++ Error" is shown whenever there is
a custom error message provided.

Also adds method name to the error item title, and re-adds the most
relevant info in the tooltip for quick error checks without expanding.

Renames C Error/Source to C++ Error/Source, since that's what it is.
And fix untranslatable entry due to misuse of TTR().

And some more cleanup for readability.

Cf. godotengine#32276 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants