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

There should be a way to register classes without exposing them (usually editor plugin internals) #1179

Closed
Zylann opened this issue Jul 16, 2023 · 2 comments
Labels
discussion topic:gdextension This relates to the new Godot 4 extension implementation
Milestone

Comments

@Zylann
Copy link
Collaborator

Zylann commented Jul 16, 2023

Godot version

Godot 4.0, 4.1

godot-cpp version

4.1

System information

Windows 10 64 bits NVIDIA GeForce GTX 1060

Issue description

I maintain a module that can also compile as an extension, and it needs to register EditorPlugins. The problem is that in order to work, I have to register all the classes that take part in these plugins:

https://github.com/Zylann/godot_voxel/blob/3f4b0b20cd6e9a093c60a3116decd63710c168e0/register_types.cpp#L342-L386

But I don't intend these classes to be accessible by users... just like a plethora of editor classes in Godot core.

This causes a number of issues:

  • It pollutes the Add Node dialog with nodes that should never be created by the user.
  • It pollutes the Editor Help with classes that should not be accessible.
  • It crashes Godot on startup because in order to get the default values of properties, Editor Help uses ClassDB to make temporary instances of every class, which then involves running the constructor of one of these plugin classes, which can want to access EditorNode or other editor-only singletons that may not exist at that time (this is another serious problem which needs its own issue!).
  • In cases it doesn't crash, it slows down editor startup, because some editors can be quite heavy in UIs or viewports.

Ideally, there must be a way to register a class for Godot to be able to instantiate an extension, but without exposing it, which means none of these issues would occur in the first place.

I assume #970 would fix it.

Example of crash:

godot.windows.editor.dev.x86_64.exe!EditorNode::get_gui_base() Line 882 (godot4\editor\editor_node.h:882)
godot.windows.editor.dev.x86_64.exe!EditorInterface::get_base_control() Line 200 (godot4\editor\editor_interface.cpp:200)
godot.windows.editor.dev.x86_64.exe!call_with_ptr_args_retc_helper<EditorInterface,Control *>(EditorInterface * p_instance, Control *(const EditorInterface::*)() p_method, const void * * p_args, void * r_ret, IndexSequence<> __formal) Line 339 (godot4\core\variant\binder_common.h:339)
godot.windows.editor.dev.x86_64.exe!call_with_ptr_args_retc<EditorInterface,Control *>(EditorInterface * p_instance, Control *(const EditorInterface::*)() p_method, const void * * p_args, void * r_ret) Line 588 (godot4\core\variant\binder_common.h:588)
godot.windows.editor.dev.x86_64.exe!MethodBindTRC<EditorInterface,Control *>::ptrcall(Object * p_object, const void * * p_args, void * r_ret) Line 598 (godot4\core\object\method_bind.h:598)
godot.windows.editor.dev.x86_64.exe!gdextension_object_method_bind_ptrcall(const void * p_method_bind, void * p_instance, const void * const * p_args, void * p_ret) Line 964 (godot4\core\extension\gdextension_interface.cpp:964)
libvoxel.windows.editor.dev.x86_64.dll!godot::internal::_call_native_mb_ret_obj<godot::Control>(const void * const mb, void * instance) Line 51 (godot_cpp_fork\include\godot_cpp\core\engine_ptrcall.hpp:51)
libvoxel.windows.editor.dev.x86_64.dll!godot::EditorInterface::get_base_control() Line 149 (godot_cpp_fork\gen\src\classes\editor_interface.cpp:149)
libvoxel.windows.editor.dev.x86_64.dll!zylann::voxel::VoxelBlockyLibraryEditorPlugin::VoxelBlockyLibraryEditorPlugin() Line 32 (godot4_fork\modules\voxel\editor\blocky_library\voxel_blocky_library_editor_plugin.cpp:32)
libvoxel.windows.editor.dev.x86_64.dll!zylann::voxel::VoxelBlockyLibraryEditorPlugin::create(void * data) Line 11 (godot4_fork\modules\voxel\editor\blocky_library\voxel_blocky_library_editor_plugin.h:11)
godot.windows.editor.dev.x86_64.exe!ClassDB::instantiate(const StringName & p_class) Line 350 (godot4\core\object\class_db.cpp:350)
godot.windows.editor.dev.x86_64.exe!ClassDB::class_get_default_property_value(const StringName & p_class, const StringName & p_property, bool * r_valid) Line 1589 (godot4\core\object\class_db.cpp:1589)
godot.windows.editor.dev.x86_64.exe!get_documentation_default_value(const StringName & p_class_name, const StringName & p_property_name, bool & r_default_value_valid) Line 340 (godot4\editor\doc_tools.cpp:340)
godot.windows.editor.dev.x86_64.exe!DocTools::generate(bool p_basic_types) Line 471 (godot4\editor\doc_tools.cpp:471)
godot.windows.editor.dev.x86_64.exe!EditorHelp::generate_doc(bool p_use_cache) Line 2339 (godot4\editor\editor_help.cpp:2339)
godot.windows.editor.dev.x86_64.exe!EditorNode::EditorNode() Line 6744 (godot4\editor\editor_node.cpp:6744)
godot.windows.editor.dev.x86_64.exe!Main::start() Line 3074 (godot4\main\main.cpp:3074)
godot.windows.editor.dev.x86_64.exe!widechar_main(int argc, wchar_t * * argv) Line 179 (godot4\platform\windows\godot_windows.cpp:179)
godot.windows.editor.dev.x86_64.exe!_main() Line 204 (godot4\platform\windows\godot_windows.cpp:204)
godot.windows.editor.dev.x86_64.exe!main(int argc, char * * argv) Line 218 (godot4\platform\windows\godot_windows.cpp:218)
godot.windows.editor.dev.x86_64.exe!WinMain(HINSTANCE__ * hInstance, HINSTANCE__ * hPrevInstance, char * lpCmdLine, int nCmdShow) Line 232 (godot4\platform\windows\godot_windows.cpp:232)
godot.windows.editor.dev.x86_64.exe!invoke_main() Line 107 (d:\a01\_work\12\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:107)
godot.windows.editor.dev.x86_64.exe!__scrt_common_main_seh() Line 288 (d:\a01\_work\12\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288)
godot.windows.editor.dev.x86_64.exe!__scrt_common_main() Line 331 (d:\a01\_work\12\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:331)
godot.windows.editor.dev.x86_64.exe!WinMainCRTStartup(void * __formal) Line 17 (d:\a01\_work\12\s\src\vctools\crt\vcstartup\src\startup\exe_winmain.cpp:17)
kernel32.dll!00007ffdcb657614() (Unknown Source:0)
ntdll.dll!00007ffdcb8c26b1() (Unknown Source:0)

https://github.com/Zylann/godot_voxel/blob/3f4b0b20cd6e9a093c60a3116decd63710c168e0/editor/blocky_library/voxel_blocky_library_editor_plugin.cpp#L32

Steps to reproduce

Have an EditorPlugin that calls get_editor_interface()->get_base_control() in its constructor.

Minimal reproduction project

N.A

@Calinou Calinou added discussion topic:gdextension This relates to the new Godot 4 extension implementation labels Jul 16, 2023
Zylann added a commit to Zylann/godot_voxel that referenced this issue Jul 17, 2023
- Don't access EditorNode indirectly from EditorPlugin constructors,
see godotengine/godot-cpp#1179
- Don't access RenderingServer directly from any registered constructor,
see godotengine/godot-cpp#1180
- Register a few missing classes

A plethora of hidden issues remain. Terrains crash as soon as they
start generating stuff.
@Zylann Zylann changed the title No way to avoid exposing editor-only classes There should be a way to register classes without exposing them (usually editor plugin internals) Jul 19, 2023
@Naros
Copy link
Contributor

Naros commented Oct 26, 2023

  • It pollutes the Add Node dialog with nodes that should never be created by the user.
  • It pollutes the Editor Help with classes that should not be accessible.

Both can easily be solved by using the GDREGISTER_INTERNAL_CLASS macro. These classes won't appear in the Add Node dialog window nor in the editor help search window when using F1 or whatever you have bound search/help to.

@dsnopek
Copy link
Collaborator

dsnopek commented Oct 26, 2023

Yep, this issue predates GDREGISTER_INTERNAL_CLASS().

I think we can consider this issue fixed by PR #970

@dsnopek dsnopek closed this as completed Oct 26, 2023
@AThousandShips AThousandShips added this to the 4.2 milestone Oct 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion topic:gdextension This relates to the new Godot 4 extension implementation
Projects
None yet
Development

No branches or pull requests

5 participants