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

GDExtension issues tracker #442

Open
40 of 79 tasks
Zylann opened this issue Sep 22, 2022 · 2 comments
Open
40 of 79 tasks

GDExtension issues tracker #442

Zylann opened this issue Sep 22, 2022 · 2 comments
Labels
GDExtension Relates to the GDExtension version of this project tracker An issue to summarize other issues

Comments

@Zylann
Copy link
Owner

Zylann commented Sep 22, 2022

This tracks issues with GDExtension, as work is ongoing to port this module to it, and there are too many to be listed there. These issues are tracked because they currently prevent the module from working properly and reliably, or affect usability. It is a large module so it hits a lot of them. Once enough of them are solved to allow production use, this tracker will be closed.

So far the module has mostly been ported and compiled, but not tested much as an extension, therefore more issues might come up.

GDExtension/GodotCpp

Missing features (or documentation needed)

  • C# support: classes defined by extensions seem to not be accessible natively with C#. Same state as in GDNative, where you have to use slow Call, Get and Set with function names as string and no strong typing at all. So far only a module can expose an API. This is important as many users want to use C#, which is good candidate for heavier operations in games with voxel terrains.
  • No separation of editor and release libraries: just like the editor comes in two versions (editor and export templates), libraries must be able to do this too. However I don't recall GDNative having that feature. This may need a second check in GDExtension.
  • Pointer access to typed arrays: this was possible in GDNative, but got lost during the rewrite of GDExtension.
  • Singleton support: C++ modules can now register singletons, but there is an issue to solve about their cleanup on exit.
  • Method typing checks: this seemed to not work in GDNative. Calling a GDNative function from GDScript did not seem to check arguments types, or argument count. They were casted silently for the better or for the worse. This may need a second check in GDExtension because I haven't tried it yet.
  • WebAssembly support: this is relatively low priority but as far as I remember, there doesnt seem to be an easy documented way to make web libraries.
  • Documentation support: the classes appear in the editor documentation pages but they don't have descriptions. That feature should be planned but appears to not be implemented yet.
  • Exposing a method to change the Node icon godotengine/godot-cpp#863: In GDScript, you can set custom icons to custom types either as a second argument to class_name, or by using add_custom_type from an editor plugin. This seems to be missing from GDExtension.
  • ClassDB::add_compatibility_class is missing from GDExtension.
  • TO DOUBLE CHECK: someone raised that libraries compiled with the release target from GodotCpp cannot be used with a Godot Editor compiled with release_debug (aka official editor builds). If that's true, it prevents shipping plugins to users with GDExtension. Needs to be tested to confirm.
  • Add support for registering virtual and abstract classes godotengine/godot-cpp#883: ClassDB::register_abstract_class doesn't seem to have an equivalent. It isn't possible to bind an abstract class (unrelated to the C++ idiom, this is an existing Godot-specific feature), which means classes that don't work as-is will be shown to users in dropdowns and creation dialogs.
  • Implement register unexposed classes godotengine/godot-cpp#970: There doesn't seem to be a way to register a non-exposed class. That means editor tools will pollute the Add Node dialog (TODO I'm assuming we always have to register classes for them to work in GDExtension, contrary to core. Investigate if that really is true, maybe they can simply be not registered, but I have a doubt). Same concern was raised in Allow more than one EditorPlugin per addon, support GDExtension classes godotengine/godot#65592. It's also a source of crashes, see There should be a way to register classes without exposing them (usually editor plugin internals) godotengine/godot-cpp#1179
  • No idea how to bind virtual methods that can be implemented by scripts? GDVIRTUAL* method binding appears to be missing. This is required for users to implement custom generators and implement some functions. There is no example of it in the official repo. ClassDB::bind_virtual_method exists but it is actually used to register implementations of Godot classes virtual methods instead. According to Reduz, it's not exposed yet. Need to expose ClassDB::add_virtual_method, expose ObjectNativeExtension somehow and re-implement all the GDVIRTUAL_* features in GodotCpp. It's tricky though, because it's not just about using scripts, in theory an extension can derive from another extension...
  • Update error macros to match core godotengine/godot-cpp#1011: _err_flush_stdout is missing, so error macros generating a crash are unable to tell Godot to flush all pending log messages, so such messages often don't appear at all. Mentionned in CRASH_COND should interrupt execution godotengine/godot-cpp#521 (comment)

Compile-time issues

Runtime issues

Other minor issues

  • GDCLASS defines create, a common name, which prevents anyone from binding a create function directly. A wrapper is necessary.
  • Math::is_nan, Math::is_inf, Math::deg_to_rad, etc. are missing. They are found in UtilityFunctions instead, but in GDExtension it is desirable to have an inline version in Math, for performance.
  • Threading hooks are not exposed godotengine/godot-cpp#964: ThreadWorkPool is missing Godot thread hooks. I have no use for it, but noticed it uses std::thread instead of godot::Thread. That means Godot hooks will not work here. This is not obvious and should really be indicated. Alternatively, Godot thread hooks should be exposed.
  • Adding custom build options causes warnings. Following the example's SConstruct file (which nests GodotCpp's SConstruct file by calling it), adding custom command line variables causes warnings in GodotCpp's SConstruct file: "WARNING: Unknown SCons variables were passed and will be ignored". This is a false-alarm though. The check GodotCpp does should not be where it is in a setup like this, it should be in the library's SConstruct (once all options were added), unless a better solution exists?
  • varray is not available. It is useful to create an Array in using a single expression. There is Array::make, but it is unnecessarily different from core, making porting and supporting both module and extension target annoying.
  • vformat is not available. It is useful to create formatted strings with variables in a single expression.
  • Expose some low level functions and String operators. godotengine/godot-cpp#939: String::operator+= is missing for characters and strings
  • String::operator+ cause an ambiguous compiler error when adding a const char* like some_string + "hello", requires to wrap them inside String("hello"). That doesnt happen in core.
  • Dictionary is very inefficient to iterate, worse than GDScript. The only way apparently is to get a copy of all the keys with .keys(), iterate keys and then get each element this way. I don't know if a solution exists.
  • Implement support for typed arrays. godotengine/godot-cpp#841: TypedArray<T> is missing. It is used in modules to expose an Array with a type that isn't necessarily a built-in type (like a class), but in GodotCpp it's not available. Workaround is to use Array.
  • SNAME for inline-registered StringNames is missing.
  • TTR and RTR are missing (tool-translate and runtime-translate). It is necessary for showing localized messages to users.
  • Rename Transform2D and Basis elements to columns and rows respectively godotengine/godot-cpp#856: godot::Basis.elements is a different name than Basis.rows in core. After a quick comparison of the get_column method, it seems elements is the same as rows (transposed matrix), but it would be better to fix the naming to make it explicit that this is exactly the same struct as in core.
  • GDCLASS requires the presence of _bind_methods(), while in modules, this is optional.
  • Add Object::cast_to for const Object* godotengine/godot-cpp#849: Missing version of Object::cast_to for const T*, cannot cast constant pointers
  • Add Transform3D translated_local, rotated_local, scaled_local godotengine/godot-cpp#850: Transform3D::translated_local() is missing
  • Including object.hpp when defining a custom class inheriting Object does not compile. The GDCLASS macro requires ClassDB, which is not available in this specific case. core/class_db.hpp has to be included too. Although, this problem seems to also exist in core. So maybe there is no possible fix.
  • Enum types in method arguments and return values aren't typed as they should be, if the type comes from another class. Example: in EditorInspectorPlugin._parse_property, type should be Variant::Type but instead is exposed as int64_t. That makes the function signature needlessly different in strongly typed languages, and it becomes more annoying when it has to be overriden.
  • Any object type cannot be forward-declared when used in virtual method override declarations, even if taken by reference &. This is due to how register_virtuals is implemented in parent classes, which requires the full definition of T. That means we have to include the full classes for arguments like Ref<InputEvent>& or Camera3D* for example, slowing down compilation a bit.
  • Use quaternion instead of quat in method names godotengine/godot-cpp#851: Basis::get_rotation_quat should be renamed get_rotation_quaternion.
  • EditorImportPlugin::_import needlessly uses TypedArray<String> (an array of Variant with a hint), when PackedStringArray exists.
  • Global enums are exposed differently, making them unnecessarily different from core, which in turn cause trouble when porting or supporting both module and extension targets. For example, MouseButton is used like MouseButton::NONE in core, but in GodotCpp it is MOUSE_BUTTON_NONE (the enum is not an enum class).

Godot issues

These are unrelated to GDExtension but come up because extensions dont have the same level of API access than modules.

  • Thread is not exposing the static method to set the name of the thread. The voxel engine uses threads heavily. It makes them harder to spot with C++ debuggers.
  • EditorSceneFormatImporter is badly exposed. A lot of its methods have problems, making it barely usable. Fix wrong get_import_options glue signature in some cases godotengine/godot#84701 (this is only one of the methods though, get_option_visibility is also not usable)
  • Resource::duplicate cannot be overriden (scripts can't do it either). But modules can override it, and the function is virtual in core. It prevents from porting classes using it in the voxel engine. This mostly leads to performance degradation, and possibly unexpected behavior (until I re-investigate all existing code that was using it).
  • All extension class method arguments in editor docs show up as unnamed, despite being provided in the C++ code.
  • Methods returning an object instance always show Object in editor docs, instead of the actual type, unlike methods from core classes.
  • Expose ProjectSettings.set_restart_if_changed(name, restart) godotengine/godot#66079: ProjectSettings lacks a function or parameter to tell that a custom setting requires an editor restart. This could be specified with PROPERTY_USAGE_RESTART_IF_CHANGED but usage is not handled by add_property_info.
  • RenderingServer::is_low_end() is not exposed. It is therefore not possible to know automatically if the renderer supports creating resources from different threads.
  • Rename and expose RefCounted::get_reference_count() godotengine/godot#66110: RefCounted::reference_get_count() is not exposed. It is then not possible to run debug checks in case we expect a place to hold the last ownership.
  • Use Vector2i when returning atlas size in Geometry2D::make_atlas godotengine/godot#66097: Geometry2D::make_atlas returns the size of the resulting atlas as a Vector2, but should be Vector2i. points should be Vector2i too, but PackedVector2iArray was never added to Variant so we have to keep converting from float to int, despite make_atlas working with int internally.
  • Change return type of get_configuration_warnings to PackedStringArray godotengine/godot#66112: Unnecessary return type difference in get_configuration_warnings. In the script API and GDExtension, _get_configuration_warnings returns a PackedStringArray. But in Godot core, the return type is TypedArray<String> (an array of Variant with a type hint). This difference is unnecessary because String is a builtin type for which PackedStringArray exists. This comes up due to compiling both as a module and an extension.
  • Expose Node3D NOTIFICATION_LOCAL_TRANSFORM_CHANGED godotengine/godot#66104: Node3D::NOTIFICATION_LOCAL_TRANSFORM_CHANGED is not exposed in the API.
  • Expose PROPERTY_USAGE_READ_ONLY godotengine/godot#66103: PROPERTY_USAGE_READ_ONLY is not exposed in the API.
  • Expose EditorProperty._set_read_only virtual method godotengine/godot#66080: EditorProperty is not exposing the virtual method _set_read_only. It prevents from making custom read-only property controls.
  • Change _can_handle and _edit virtual methods to take Object* godotengine/godot#66121: Some virtual methods that previously took an Object* (or derived) now take a Variant. They weren't taking Variant in Godot 3, and still take Object* internally, so this looks like a regression. Found so far: EditorPlugin._handles, EditorPlugin._edit, EditorInspectorPlugin._handles... It is annoying in strongly typed languages, and even more annoying when compiling both as module and extension. There is a valid reason to use Variant, but it is not consistently applied, and is actually irrelevant considering the callee handles referencing.
  • In EditorPlugin, _forward_3d_gui_input returns an AfterGUIInput enum that belongs to the same class, but the enum is actually not present in GodotCpp, or the docs.
  • Expose EditorInspector.get_selected_path godotengine/godot#66108: EditorInspector::get_selected_path is not exposed. Currently used in VoxelInstanceLibrary to identify which item is selected, similar to Godot's MeshLibraryEditorPlugin.
  • Make EditorInterface accessible as a singleton godotengine/godot#75694: EDSCALE is not available in GodotCpp. This is required to implement DPI-aware editor tools. It is available in EditorInterface.get_editor_scale(), but it's not a static method. That means it needs to be passed around as parameter everywhere that needs it. This is annoying in custom controls that cannot have a constructor with parameters, and porting modules in general.
  • RenderingDevice has a singleton used by RenderingServer, but it cannot be accessed. Therefore it seems not possible to query its limits. It was done deliberately to prevents scripters from messing with the renderer, considered dangerous. Workaround?
@SneaK1ng
Copy link
Contributor

It seems to take a long time

@Zylann Zylann added the GDExtension Relates to the GDExtension version of this project label Nov 26, 2023
@xkisu
Copy link

xkisu commented May 20, 2024

EditorImportPlugin::_import is passing platform_variants and gen_files as const arrays. They are supposed to be modifiable, for the method to fill them in. It makes the method unusable and confusing. Workaround is to const_cast them, which is ugly.

I found this issue thread upon discovering the const issue myself in a plugin and thought I'd update you - I've raised it as an issue in the gotdot-cpp repository (godotengine/godot-cpp#1471) and a pull request draft has been started to fix it!

@Zylann Zylann added the tracker An issue to summarize other issues label May 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GDExtension Relates to the GDExtension version of this project tracker An issue to summarize other issues
Projects
None yet
Development

No branches or pull requests

3 participants