Skip to content

[GDExtension] Missing methods, classes and API discrepancies. #633

Open

Description

@bruvzg

godot-cpp: ad11bbb5845a454551d490812631922c33b7601c
godot: godotengine/godot#52192

Some missing API functions and discrepancies found during attempt to port TextServerAdvanced to the GDExtension (as part of godotengine/godot#52192):

  • String:
    • missing ptr, ptrw methods.
    • missing +, += operators.
  • StringName do not expose < and > operators, and unusable as map key.
  • Packed*Arrray:
    • missing ptr, ptrw methods.
    • [] operator API difference with the engine ([] vs .write[]).
  • Dictionary:
    • missing [] operator.
    • seems to have no methods to write value at all.
  • Char*String:
    • no public constructors.
  • RID:
  • Mutex and Semaphore:
  • memnew, memdelete aren't working with non-Godot classes.
  • no memalloc, memfree macros defined (can be added locally):
    #define memalloc(m_size) Memory::alloc_static(m_size)
    #define memrealloc(m_mem, m_size) Memory::realloc_static(m_mem, m_size)
    #define memfree(m_mem) Memory::free_static(m_mem)
    
  • Enum type return values can't be bound on the engine side.
  • Enum type argument types can't be bound, unless conversion macros is added manually - MAKE_PTRARGCONV(TextServer::Direction, int64_t);.
  • Missing math defines, MAX, MIN, CLAMP etc.
  • No conversion for custom native pointer types, can be fixed locally by adding GDVIRTUAL_NATIVE_PTR(Glyph *); macro.
  • No templates for Vector, Map, List, HashMap, Set, not a critical issue, but would be convenient to have for better module <-> GDExtension portability. Included in Port a bunch of Godot container templates to GDExtension. #701.

Activity

  1. added
    bugThis has been identified as a bug
    topic:gdextensionThis relates to the new Godot 4 extension implementation
    on Sep 29, 2021
  2. added this to the 4.0 milestone on Sep 29, 2021
  3. akien-mga commented on Sep 29, 2021

    @akien-mga
    Member

    Most of the discrepancy in core classes comes from the fact that GDExtension uses the API registered in ClassDB, i.e. the proxy classes defined in core/core_bind.cpp. Some of the differences are probably oversights (core class refactored, but not the core_bind.cpp version), and others might be for technical reasons. For example Thread and core_bind::Thread have a somewhat different API (e.g. Thread::is_started() vs core_bind::Thread::is_active()).

    I ran into most of the above too when attempting to port the opensimplex module to an extension (just for testing purposes). Some others I ran into:

    • RS alias for RenderingServer is not defined (might be worth considering whether we should still use it at all upstream).
    • RenderingServer::free() is exposed as RenderingServer::free_rid(). That's a good candidate for renaming upstream to use free_rid consistently IMO (also PhysicsServer* and TextServer)
    • SNAME is not available to create and reference static stringnames.
    • callable_mp is not available either. It could likely be copied from core/object/callable_method_pointer.h and adapted to work in godot-cpp.
    • vformat not available
    • DEFVAL not defined. It's a no-op upstream but used for readability: Remove DEFVAL godot#46903. Either that PR should be reassessed (remove DEFVAL), or it should be added here for consistency.
    • Image constructors missing.
    • OBJ_SAVE_TYPE and GDREGISTER_CLASS missing.

    I also had issues with a class extending Texture2D. It's an abstract virtual class upstream, and my derived class is still seen as abstract in Godot so it can't be instantiated. That's probably the same cause as godotengine/godot#35484 for GDScript/C# custom nodes.

    Also related, using the override keyword in that class extending Texture2D didn't work, as apparently godot-cpp couldn't see that I'm indeed overriding virtual methods from the base class. That might be an expected limitation though.

  4. Zylann commented on Sep 29, 2021

    @Zylann
    Collaborator

    Missing math defines, MAX, MIN, CLAMP etc.

    For these ones I believe they should stop being used. In C++ max/min/clamp already exist as inline templates in Math:: and in STL so I see no reason to use macros for that stuff.

    RS alias for RenderingServer is not defined (might be worth considering whether we should still use it at all upstream).

    Such shortcuts shouldn't need to exist IMO, at least not globally. It's a 2-letter macro whose sole purpose is shortening something that isn't that long and not used often (depends of the area of course), and being explicit would be much better. If the singleton is repeated a lot then it can be placed in a local reference.

  5. Xrayez commented on Sep 29, 2021

    @Xrayez
    • No templates for Vector, Map, List, HashMap, Set, not a critical issue, but would be convenient to have for better module <-> GDExtension portability.

    I use those all the time in https://github.com/goostengine/goost, even templates like LocalVector.

    I also had issues with a class extending Texture2D. It's an abstract virtual class upstream, and my derived class is still seen as abstract in Godot so it can't be instantiated. That's probably the same cause as godotengine/godot#35484 for GDScript/C# custom nodes.

    See godotengine/godot#42830.

  6. Razoric480 commented on Dec 5, 2021

    @Razoric480

    The ClassDB object allows for registering but doesn't give access to any of its methods as defined in the version GDscript has access to: https://docs.godotengine.org/en/latest/classes/class_classdb.html

  7. 2shady4u commented on Jan 29, 2022

    @2shady4u
    Contributor

    Another method of String that went missing in GDExtension:

    alloc_c_string()

    EDIT: Also, I can't seem to find an equivalent to the String::num_int64()-method in the new GDExtension API.

  8. bruvzg commented on Feb 21, 2022

    @bruvzg
    MemberAuthor

    Another method of String that went missing in GDExtension: alloc_c_string()

    There's no such method in the Godot ether, what's it supposed to do?

    Also, I can't seem to find an equivalent to the String::num_int64()-method in the new GDExtension API.

    This is fixed.


    Updated the first comment, most of the stuff in the original issue is already fixed, or (in case of templates) will be added by #701

  9. 2shady4u commented on Feb 21, 2022

    @2shady4u
    Contributor

    It's supposed to be an easy way of converting a Godot String to a char*.
    This can be worked around by first going to CharString and then getting a char* from that.

    char *String::alloc_c_string() const {
    	godot_char_string contents = godot::api->godot_string_utf8(&_godot_string);
    
    	int length = godot::api->godot_char_string_length(&contents);
    
    	char *result = (char *)godot::api->godot_alloc(length + 1);
    
    	if (result) {
    		memcpy(result, godot::api->godot_char_string_get_data(&contents), length + 1);
    	}
    
    	godot::api->godot_char_string_destroy(&contents);
    
    	return result;
    }
  10. bruvzg commented on Feb 21, 2022

    @bruvzg
    MemberAuthor

    This can be worked around by first going to CharString and then getting a char* from that.

    With the new GDextension API you can do something like this instead (and CharString is essentially a wrapper which will do it for you and auto-free the char * array):

    	int size = internal::gdn_interface->string_to_utf8_chars(string, nullptr, 0);
    	char *cstr = memnew_arr(char, size + 1);
    	internal::gdn_interface->string_to_utf8_chars(string, cstr, size + 1);
  11. 2shady4u commented on Feb 26, 2022

    @2shady4u
    Contributor

    I'm getting warnings during compilation:
    image

    Is there any reason why the []-operator doesn't take a int64_t (like all other Array methods)?
    image

    All other methods of Array either return or accept int64_t as their output/input. For example the insert()-method which also deals with accessing an array's position is as follows:

    int64_t insert(int64_t position, const Variant &value);
    

    In Godot's source code this method takes an int instead.

    If no reason at all, would it be possible to change this inconsistency?
    (Either by making all methods take int or by making all methods take int64_t)

  12. Zylann commented on Feb 26, 2022

    @Zylann
    Collaborator

    In Godot's source code, that method also takes an int, and in practice if you need more than INT_MAX then you'd need a bit more than 64 Gb of RAM to hold such an array in memory (Variant is around 32 bytes).

    One reason I know int64_t is used in many places is because Variant stores integers as such. Otherwise, I don't know.

    For consistency however it would be valid to use int64_t here instead, or size_t. I dont think it's using int for any other reason than "it's the same in core".

  13. codecat commented on Mar 5, 2022

    @codecat
    Contributor

    This issue mentions String::operator+= but is stroked out, does that mean it won't be implemented, or is that just an oversight for now? I think += is kind of an important operator on strings, and it does exist on Godot itself.

  14. Zylann commented on Mar 5, 2022

    @Zylann
    Collaborator

    Dont the strokes mean "done"?

  15. codecat commented on Mar 5, 2022

    @codecat
    Contributor

    It it means "done", then it is wrongly crossed out:

    error C2676: binary '+=': 'godot::String' does not define this operator or a conversion to a type acceptable to the predefined operator

  16. bruvzg commented on Mar 5, 2022

    @bruvzg
    MemberAuthor

    It means "done", but it's done it the godotengine/godot#58233, which is not merged.

  17. 2shady4u commented on Jun 5, 2022

    @2shady4u
    Contributor

    I'm using the latest master commit (eaaf941) and using += to add two godot Strings together isn't accepted yet... :/ (Event though godotengine/godot#58233) has now been merged and been included in the latest Godot 4.0 Alpha 9.

  18. DmitriySalnikov commented on Jul 1, 2022

    @DmitriySalnikov
    Contributor

    Do I understand correctly that Callable in GDExtension cannot be used normally at the moment?
    For example, in Godot v4.0.alpha11.official [afdae67cc] Thread.start now does not accept user_data, but Callable.bind is not implemented. Callable.bind doesn't even get a pointer to the engine function.

    image

  19. Zylann commented on Sep 11, 2022

    @Zylann
    Collaborator

    @DmitriySalnikov None of Callable's vararg methods are implemented #802

  20. Daylily-Zeleen commented on Dec 4, 2022

    @Daylily-Zeleen
    Contributor

    The ClassDB object allows for registering but doesn't give access to any of its methods as defined in the version GDscript has access to: https://docs.godotengine.org/en/latest/classes/class_classdb.html

    Implement by #936.

  21. mihe commented on Dec 8, 2022

    @mihe
    Contributor

    I'm not sure if this should be considered a bug or an API discrepancy, but CharString, Char16String and Char32String all behave differently from core with regards to length() and (lack of) size().

    In core, size() returns the size with the null-terminator included and length() returns the size without the null-terminator included.

    In godot-cpp, there is no size() and length() returns the size with the null-terminator included.

    However, String::length() behaves as expected in godot-cpp, giving you the size without the null-terminator, which makes this discrepancy a bit jarring.

    EDIT: I ended up making a PR for it: #961

  22. DmitriySalnikov commented on May 31, 2023

    @DmitriySalnikov
    Contributor

    Resource::get_rid is not exposed as virtual method.
    For me, this makes it a bit difficult to create my own Texture2D and forces me to override the draw_* methods.
    Actually, this issue has already been discussed here #877, but for some reason it was archived.

  23. modified the milestones: 4.0, 4.x on Jul 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugThis has been identified as a bugconfirmeddiscussiontopic:gdextensionThis relates to the new Godot 4 extension implementation

    Type

    No type

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions