godotengine / godot

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

Porting ERR_EXPLAIN uses to new ERR_FAIL_COND_MSG error macros #31207

Closed akien-mga closed 5 years ago

akien-mga commented 5 years ago

Godot version: 3.2 master (22b42c331)

OS/device including version: All

Issue description: After the merge of #30893, we have new error macros to raise an error conditionally (or directly) with a defined error message.

These should be used in place of all existing ERR_EXPLAIN() calls which define the error message to be used by a later ERR_FAIL() or similar error macro.

As there's potential for overlap of several contributors working on it at the same time, please mention it if you're actively working on this issue, and ideally focus on a specific part of the codebase at a time (e.g. all scene/ or all editor/).

akien-mga commented 5 years ago

For the reference:

$ rg ERR_EXPLAIN | wc -l
693
$ rg ERR_EXPLAIN -l | sort
core/array.cpp
core/bind/core_bind.cpp
core/class_db.cpp
core/class_db.h
core/color.cpp
core/engine.cpp
core/error_macros.h
core/hash_map.h
core/image.cpp
core/input_map.cpp
core/io/config_file.cpp
core/io/file_access_buffered.cpp
core/io/file_access_buffered_fa.h
core/io/file_access_encrypted.cpp
core/io/file_access_pack.cpp
core/io/ip_address.cpp
core/io/marshalls.cpp
core/io/multiplayer_api.cpp
core/io/packet_peer.cpp
core/io/pck_packer.cpp
core/io/resource_format_binary.cpp
core/io/resource_loader.cpp
core/io/resource_saver.cpp
core/io/translation_loader_po.cpp
core/io/xml_parser.cpp
core/math/basis.cpp
core/math/expression.cpp
core/math/geometry.h
core/math/octree.h
core/message_queue.cpp
core/node_path.cpp
core/object.cpp
core/os/file_access.cpp
core/os/memory.cpp
core/os/os.cpp
core/pool_allocator.cpp
core/pool_vector.cpp
core/pool_vector.h
core/project_settings.cpp
core/resource.cpp
core/script_debugger_remote.cpp
core/translation.cpp
core/variant_call.cpp
core/variant.cpp
drivers/gles2/rasterizer_canvas_gles2.cpp
drivers/gles2/rasterizer_scene_gles2.cpp
drivers/gles2/rasterizer_storage_gles2.cpp
drivers/gles3/rasterizer_canvas_gles3.cpp
drivers/gles3/rasterizer_scene_gles3.cpp
drivers/gles3/rasterizer_storage_gles3.cpp
drivers/png/png_driver_common.cpp
drivers/png/resource_saver_png.cpp
drivers/unix/file_access_unix.cpp
drivers/unix/ip_unix.cpp
drivers/unix/os_unix.cpp
drivers/wasapi/audio_driver_wasapi.cpp
drivers/windows/file_access_windows.cpp
drivers/xaudio2/audio_driver_xaudio2.cpp
editor/collada/collada.cpp
editor/doc/doc_data.cpp
editor/editor_autoload_settings.cpp
editor/editor_export.cpp
editor/editor_file_system.cpp
editor/editor_node.cpp
editor/editor_resource_preview.cpp
editor/editor_settings.cpp
editor/export_template_manager.cpp
editor/filesystem_dock.cpp
editor/file_type_cache.cpp
editor/import/editor_import_collada.cpp
editor/import/editor_scene_importer_gltf.cpp
editor/import/resource_importer_csv_translation.cpp
editor/import/resource_importer_obj.cpp
editor/import/resource_importer_wav.cpp
editor/plugins/cpu_particles_2d_editor_plugin.cpp
editor/plugins/multimesh_editor_plugin.cpp
editor/plugins/particles_2d_editor_plugin.cpp
editor/plugins/theme_editor_plugin.cpp
editor/plugins/tile_set_editor_plugin.cpp
editor/pvrtc_compress.cpp
editor/scene_tree_dock.cpp
main/main.cpp
main/tests/test_gdscript.cpp
main/tests/test_math.cpp
modules/assimp/editor_scene_importer_assimp.cpp
modules/bmp/image_loader_bmp.cpp
modules/bullet/bullet_physics_server.cpp
modules/bullet/cone_twist_joint_bullet.cpp
modules/bullet/generic_6dof_joint_bullet.cpp
modules/bullet/hinge_joint_bullet.cpp
modules/bullet/pin_joint_bullet.cpp
modules/bullet/shape_bullet.cpp
modules/bullet/space_bullet.cpp
modules/dds/texture_loader_dds.cpp
modules/enet/networked_multiplayer_enet.cpp
modules/etc/texture_loader_pkm.cpp
modules/gdnative/arvr/arvr_interface_gdnative.cpp
modules/gdnative/gdnative.cpp
modules/gdnative/nativescript/godot_nativescript.cpp
modules/gdnative/nativescript/nativescript.cpp
modules/gdnative/pluginscript/pluginscript_script.cpp
modules/gdscript/gdscript_compiler.cpp
modules/gdscript/gdscript.cpp
modules/gdscript/gdscript_editor.cpp
modules/gdscript/gdscript_function.cpp
modules/gdscript/gdscript_functions.cpp
modules/gdscript/gdscript_parser.cpp
modules/gdscript/gdscript_tokenizer.cpp
modules/gridmap/grid_map.cpp
modules/hdr/image_loader_hdr.cpp
modules/mono/csharp_script.cpp
modules/mono/editor/bindings_generator.cpp
modules/mono/editor/csharp_project.cpp
modules/mono/editor/godotsharp_export.cpp
modules/mono/editor/script_class_parser.cpp
modules/mono/glue/gd_glue.cpp
modules/mono/mono_gd/gd_mono.cpp
modules/mono/mono_gd/gd_mono_field.cpp
modules/mono/mono_gd/gd_mono_log.cpp
modules/mono/mono_gd/gd_mono_marshal.cpp
modules/mono/mono_gd/gd_mono_utils.cpp
modules/mono/signal_awaiter_utils.cpp
modules/opus/audio_stream_opus.cpp
modules/pvr/texture_loader_pvr.cpp
modules/squish/image_compress_squish.cpp
modules/svg/image_loader_svg.cpp
modules/visual_script/visual_script.cpp
modules/vorbis/audio_stream_ogg_vorbis.cpp
modules/webm/video_stream_webm.cpp
modules/webp/image_loader_webp.cpp
modules/webrtc/webrtc_data_channel_js.cpp
modules/webrtc/webrtc_multiplayer.cpp
modules/webrtc/webrtc_peer_connection_gdnative.cpp
modules/websocket/emws_peer.cpp
modules/websocket/websocket_multiplayer_peer.cpp
modules/websocket/wsl_client.cpp
modules/websocket/wsl_server.cpp
modules/xatlas_unwrap/register_types.cpp
platform/android/audio_driver_opensl.cpp
platform/android/export/export.cpp
platform/android/java_godot_lib_jni.cpp
platform/android/os_android.cpp
platform/iphone/export/export.cpp
platform/iphone/os_iphone.cpp
platform/javascript/http_client_javascript.cpp
platform/javascript/os_javascript.cpp
platform/osx/os_osx.mm
platform/uwp/export/export.cpp
platform/uwp/os_uwp.cpp
platform/windows/os_windows.cpp
platform/x11/export/export.cpp
scene/2d/animated_sprite.cpp
scene/2d/area_2d.cpp
scene/2d/canvas_item.cpp
scene/2d/physics_body_2d.cpp
scene/3d/area.cpp
scene/3d/arvr_nodes.cpp
scene/3d/camera.cpp
scene/3d/physics_body.cpp
scene/3d/soft_body.cpp
scene/3d/spatial.cpp
scene/animation/animation_cache.cpp
scene/animation/animation_node_state_machine.cpp
scene/animation/animation_player.cpp
scene/animation/animation_tree_player.cpp
scene/animation/tween.cpp
scene/gui/control.cpp
scene/gui/graph_node.cpp
scene/gui/line_edit.cpp
scene/gui/popup_menu.cpp
scene/gui/tree.cpp
scene/main/http_request.cpp
scene/main/node.cpp
scene/main/scene_tree.cpp
scene/main/timer.cpp
scene/main/viewport.cpp
scene/resources/curve.cpp
scene/resources/dynamic_font.cpp
scene/resources/font.cpp
scene/resources/material.cpp
scene/resources/mesh.cpp
scene/resources/packed_scene.cpp
scene/resources/resource_format_text.cpp
scene/resources/text_file.cpp
scene/resources/texture.cpp
scene/resources/tile_set.cpp
scene/resources/visual_shader.cpp
servers/audio/voice_rb_sw.h
servers/physics_2d/physics_2d_server_sw.cpp
servers/physics/collision_object_sw.h
servers/physics/physics_server_sw.cpp
servers/visual_server.cpp
servers/visual/visual_server_canvas.cpp
Unholydeath commented 5 years ago

I would like to take the /core files. I'm interested in working on a lot of this problem, so I'll give updates on my progress and do a PR for as many as I can do until everyone involved is finished with it.

KoBeWi commented 5 years ago

I'm taking the /scene stuff.

Unholydeath commented 5 years ago

I'm mostly through /core and I'll finish it tonight. I would also like to claim /driver and /editor

YeldhamDev commented 5 years ago

I'll take "main/" and "servers/".

neikeq commented 5 years ago

Glad to see this change. I'll take "modules/mono".

profan commented 5 years ago

I'll take "platform/" and "modules/gdscript/" + "modules/gdnative".

akien-mga commented 5 years ago

Great work everyone :)

As discussed in https://github.com/godotengine/godot/pull/31213#issuecomment-519665658, I think we should take the opportunity to make the error style more consistent, especially ensuring that sentences start with a capitalized letter and end with a dot.

If you see error messages using TTR() for translation, that's a bug and it should be removed. We don't translate error strings (and TTR() doesn't work outside editor code anyway, so such strings would end up as empty strings in non-tools builds).

Unholydeath commented 5 years ago

I removed almost all the EXPLAIN from /core and /drivers #31244, but I had trouble with these files: core/error_macros.h (defines EXPLAIN and the *_MSG files so don't know if it needs changed) core/io/file_access_buffered_fa.h (couldn't find it in my files) core/os/memory.cpp (got errors when trying to combine CRASH_NOW and EXPLAINC) drivers/png/png_driver_common.cpp (wasn't sure how to replace just EXPLAINC) drivers/xaudio2/audio_driver_xaudio2.cpp (couldn't find it in my files)

If anyone would either like to let me know how to fix it or take a look at it, that'd be #great.

profan commented 5 years ago

I'll deal with the remaining folders under "modules/".

akien-mga commented 5 years ago

Remaining occurrences once #31244 will be merged:

$ rg ERR_EXPLAIN -l | sort
core/error_macros.h
core/io/file_access_buffered_fa.h
core/os/memory.cpp
drivers/png/png_driver_common.cpp
drivers/windows/file_access_windows.cpp
drivers/xaudio2/audio_driver_xaudio2.cpp
editor/import/editor_import_collada.cpp
modules/etc/texture_loader_pkm.cpp
modules/gdscript/gdscript_editor.cpp
modules/gdscript/gdscript_parser.cpp
modules/hdr/image_loader_hdr.cpp
modules/webp/image_loader_webp.cpp
modules/websocket/wsl_client.cpp
modules/websocket/wsl_server.cpp
modules/xatlas_unwrap/register_types.cpp
scene/resources/dynamic_font.cpp

I'll have a look at those.

akien-mga commented 5 years ago

All fixed, thanks everyone for your work!