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

Provide error message for every single ERR_FAIL, ERR_FAIL_COND etc. #31849

Closed
qarmin opened this issue Sep 1, 2019 · 1 comment
Closed

Provide error message for every single ERR_FAIL, ERR_FAIL_COND etc. #31849

qarmin opened this issue Sep 1, 2019 · 1 comment

Comments

@qarmin
Copy link
Contributor

qarmin commented Sep 1, 2019

Issue description:
There is a lot of errors, that shows when using Godot. Some of this errors are not really clear:

core/node_path.cpp:215 - Condition ' !data ' is true. returned: StringName()

ERROR: affine_invert: Condition ' det == 0 ' is true.

At: ./core/cowdata.h:252:resize() - Condition ' psize < 0 ' is true. returned: ERRINVALIDPARAMETER

ERROR: ~List: Condition ' _first != __null ' is true.

Condition ' !id_map.has(p_rid.get_data()) ' is true. returned: __null

editor/plugins/asset_library_editor_plugin.cpp:1206 - Condition 'category_map.has(r["category_id"]) ' is true.

core/class_db.h:186:register_virtual_class() - Condition ' !t ' is true.

So I think, that every ERR_* macro (maybe except ERR_CRASH, ERR_FAIL_INDEX), should provide clear error message.

A lot of ERR_FAIL can be easy to describe, like

ERR_FAIL_COND(p_src.is_null());

ERR_FAIL_COND_MSG(p_src.is_null(), "It isn't valid Image resource.");
ERR_FAIL_COND_V(alloc->lock > 0, ERR_LOCKED); //can't resize if locked!

ERR_FAIL_COND_V_MSG(alloc->lock > 0, ERR_LOCKED," Can't resize Pool Vector if locked!");

People creating PR should always use ERR_*_MSG because, they should know best what the message should be.

Current number of macros without error message (Godot 3.2 00aabec)

rg "ERR_FAIL[_V]{0,2}\(" | wc -l
228

rg "ERR_FAIL_COND[_V]{0,2}\(" | wc -l
5091

rg "ERR_CONTINUE\(" | wc -l
274

rg "ERR_BREAK\(" | wc -l
50

rg "ERR_FAIL_NULL[_V]{0,2}\(" | wc -l
162

rg "CRASH_COND\(" | wc -l
124

@Calinou
Copy link
Member

Calinou commented Oct 21, 2020

Closing in favor of #42719, which should hopefully get more discussion going.

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