Open Zylann opened 2 years ago
It seems to take a long time
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 (https://github.com/godotengine/godot-cpp/issues/1471) and a pull request draft has been started to fix it!
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)
Call
,Get
andSet
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.class_name
, or by usingadd_custom_type
from an editor plugin. This seems to be missing from GDExtension.ClassDB::add_compatibility_class
is missing from GDExtension.release
target from GodotCpp cannot be used with a Godot Editor compiled withrelease_debug
(aka official editor builds). If that's true, it prevents shipping plugins to users with GDExtension. Needs to be tested to confirm.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.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 https://github.com/godotengine/godot/pull/65592. It's also a source of crashes, see https://github.com/godotengine/godot-cpp/issues/1179GDVIRTUAL*
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 exposeClassDB::add_virtual_method
, exposeObjectNativeExtension
somehow and re-implement all theGDVIRTUAL_*
features in GodotCpp. It's tricky though, because it's not just about using scripts, in theory an extension can derive from another extension..._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 https://github.com/godotengine/godot-cpp/issues/521#issuecomment-1302874090Compile-time issues
GodotCpp
only defines two targets:debug
(big unoptimized binary with debug checks) andrelease
(small optimized binary without debug checks), which it assumes being "Editor" and "Exported game". There is no equivalent preset forrelease_debug
(small optimized binary with debug checks), which is what Godot Editor official builds are using. This missing target is required for plugins. See https://github.com/godotengine/godot-cpp/pull/835 and https://github.com/godotengine/godot-proposals/issues/3371callable_mp
is not available. It allows to create aCallable
from a method pointer. Without it, all methods we want to connect to signals have to be registered manually, like in the pre-Callable era.Callable
cannot be called, andbind()
doesn't work.PropertyInfo
requires aconst char*
forname
andhint_string
(unlike modules, where they areString
). This makes it difficult to use formatted strings, which are very useful when the name or hint depends on a variable. In_bind_methods
it is relatively safe to format anstd::string
and send.c_str()
because Godot makes aString
copy internally in the call tobind_method
. However, in_get_property_list
that trick isn't possible because theconst char*
has to be returned to the caller, it would have to outlive the method.Node::NOTIFICATION_*
are not declared static, therefore they take space in every instance, and cannot be used inswitch
statements, amongst other things. They are supposed to be static, or better, be declared as enums. This can be fixed in https://github.com/godotengine/godot-cpp/pull/828/filesObject
(likeSceneTree*
orNode*
) cannot be bound. It causes a compiler error. This is supposed to be working. Workaround is to useObject*
and cast manually.A
implements_get_configuration_warnings
, andB
derives fromA
without overriding it.GDCLASS
callsregister_virtuals
, which in turn callsBIND_VIRTUAL_METHOD(B, _get_configuration_warnings)
, whereT
is the derived class. The problem might be thatB
does not actually define_get_configuration_warnings
, its base class does. Oddly enough, this situation works fine if the base class is a core class, but it does not work if the base is also a custom class. This issue becomes a nightmare when supporting both module and extension targets, as making wrappers isn't fully working (example https://github.com/Zylann/godot_voxel/blob/gdextension/editor/vox/vox_scene_importer.h).EditorImportPlugin::_import
is passingplatform_variants
andgen_files
asconst
arrays. They are supposed to be modifiable, for the method to fill them in. It makes the method unusable and confusing. Workaround is toconst_cast
them, which is ugly.File
have functions returning a global enum likeError
. However when compiling with the targettemplate_release
, it appears the enum is actually not defined, forcing to also include<godot_cpp/classes/global_constants.hpp>
.Runtime issues
RenderingServer
are unavailable when extensions initialize, even inMODULE_INITIALIZATION_LEVEL_SCENE
. Apparently it was a deliberate choice for reasons unknown to me. People workaround this by adding autoloads to their project... but it's messy and confusing.get_singleton
should really be reworked properly, just like in modules.RenderingServer
in their constructor will crash on startup (similar to earlier issue).EditorHelp
creates instances of EVERY class in ClassDB when the editor starts (to get properties default values), and it turns out extension classes SPECIFICALLY will crash in their constructor if they ever attempt to accessRenderingServer
. This does not happen with module classes. It prevents making custom nodes or resources using the renderer, and calls for painful workarounds deferring things to_enter_tree
(main thread) or functions the user has to call manually to initialize things._init()
, which is called after theowner
pointer is set. However this fix was lost during the rewrite for GDExtensions so the problem came back. No issue was tracking it though, and it seems to no longer happen.virtual
C++ methods, which were not present in GDNative before. But this also broke casting object types, and needs to be solved.release
builds.nullptr
to functions takingObject*
-derived types. That also mean we can't set a resource property tonullptr
._get_property_list
from base classes is never called when the inheritance hierarchy is deeper than 1. Certain resources of the module are not working because of properties made unavailable.Other minor issues
GDCLASS
definescreate
, a common name, which prevents anyone from binding acreate
function directly. A wrapper is necessary.Math::is_nan
,Math::is_inf
,Math::deg_to_rad
, etc. are missing. They are found inUtilityFunctions
instead, but in GDExtension it is desirable to have an inline version inMath
, for performance.ThreadWorkPool
is missing Godot thread hooks. I have no use for it, but noticed it usesstd::thread
instead ofgodot::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.SConstruct
file (which nestsGodotCpp
'sSConstruct
file by calling it), adding custom command line variables causes warnings inGodotCpp
'sSConstruct
file:"WARNING: Unknown SCons variables were passed and will be ignored"
. This is a false-alarm though. The checkGodotCpp
does should not be where it is in a setup like this, it should be in the library'sSConstruct
(once all options were added), unless a better solution exists?varray
is not available. It is useful to create anArray
in using a single expression. There isArray::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.String::operator+=
is missing for characters and stringsString::operator+
cause an ambiguous compiler error when adding a const char* likesome_string + "hello"
, requires to wrap them insideString("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.TypedArray<T>
is missing. It is used in modules to expose anArray
with a type that isn't necessarily a built-in type (like a class), but in GodotCpp it's not available. Workaround is to useArray
.SNAME
for inline-registeredStringNames
is missing.TTR
andRTR
are missing (tool-translate and runtime-translate). It is necessary for showing localized messages to users.godot::Basis.elements
is a different name thanBasis.rows
in core. After a quick comparison of theget_column
method, it seemselements
is the same asrows
(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.Object::cast_to
forconst T*
, cannot cast constant pointersTransform3D::translated_local()
is missingobject.hpp
when defining a custom class inheritingObject
does not compile. TheGDCLASS
macro requiresClassDB
, 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.EditorInspectorPlugin._parse_property
,type
should beVariant::Type
but instead is exposed asint64_t
. That makes the function signature needlessly different in strongly typed languages, and it becomes more annoying when it has to be overriden.&
. This is due to howregister_virtuals
is implemented in parent classes, which requires the full definition ofT
. That means we have to include the full classes for arguments likeRef<InputEvent>&
orCamera3D*
for example, slowing down compilation a bit.Basis::get_rotation_quat
should be renamedget_rotation_quaternion
.EditorImportPlugin::_import
needlessly usesTypedArray<String>
(an array of Variant with a hint), whenPackedStringArray
exists.MouseButton
is used likeMouseButton::NONE
in core, but in GodotCpp it isMOUSE_BUTTON_NONE
(the enum is not anenum 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. https://github.com/godotengine/godot/pull/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).unnamed
, despite being provided in the C++ code.Object
in editor docs, instead of the actual type, unlike methods from core classes.ProjectSettings
lacks a function or parameter to tell that a custom setting requires an editor restart. This could be specified withPROPERTY_USAGE_RESTART_IF_CHANGED
butusage
is not handled byadd_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.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.Geometry2D::make_atlas
returns thesize
of the resulting atlas as aVector2
, but should beVector2i
.points
should beVector2i
too, butPackedVector2iArray
was never added toVariant
so we have to keep converting fromfloat
toint
, despitemake_atlas
working withint
internally.get_configuration_warnings
. In the script API and GDExtension,_get_configuration_warnings
returns aPackedStringArray
. But in Godot core, the return type isTypedArray<String>
(an array ofVariant
with a type hint). This difference is unnecessary becauseString
is a builtin type for whichPackedStringArray
exists. This comes up due to compiling both as a module and an extension.Node3D::NOTIFICATION_LOCAL_TRANSFORM_CHANGED
is not exposed in the API.PROPERTY_USAGE_READ_ONLY
is not exposed in the API.EditorProperty
is not exposing the virtual method_set_read_only
. It prevents from making custom read-only property controls.Object*
(or derived) now take aVariant
. They weren't takingVariant
in Godot 3, and still takeObject*
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 useVariant
, but it is not consistently applied, and is actually irrelevant considering the callee handles referencing.EditorPlugin
,_forward_3d_gui_input
returns anAfterGUIInput
enum that belongs to the same class, but the enum is actually not present in GodotCpp, or the docs.EditorInspector::get_selected_path
is not exposed. Currently used inVoxelInstanceLibrary
to identify which item is selected, similar to Godot'sMeshLibraryEditorPlugin
.EDSCALE
is not available in GodotCpp. This is required to implement DPI-aware editor tools. It is available inEditorInterface.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 byRenderingServer
, 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?