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

Implement native extension system (GDExtension) #49744

Merged
merged 1 commit into from
Jun 25, 2021

Conversation

reduz
Copy link
Member

@reduz reduz commented Jun 19, 2021

  • Deprecates GDNative in favor of a simpler, lower level interface.
  • New extension system allows registering core engine classes.
  • Per function signature hashing, so if an API changes, the old ones can still be made available.
  • Simple header interface in gdnative_interface.h

The API dump command line has also been re made, to invoke use:

$ godot --dump-extension-api

Example of how API dumps look now: api.json

This PR is a draft for review.

TODO:

  • JSON API dumping.
  • Function checksums to deprecate API Hash
  • Virtual Functions
  • Singletons
  • Automatic extension loading, editor extension loading.
  • Extension dependencies.
  • Hot reloading
  • Likely other features.

@reduz reduz requested a review from a team as a code owner June 19, 2021 16:03
@reduz reduz marked this pull request as draft June 19, 2021 16:04
core/extension/gdnative_interface.cpp Outdated Show resolved Hide resolved
core/extension/gdnative_interface.cpp Outdated Show resolved Hide resolved
core/extension/gdnative_interface.cpp Outdated Show resolved Hide resolved
core/extension/gdnative_interface.cpp Outdated Show resolved Hide resolved
core/extension/gdnative_interface.cpp Outdated Show resolved Hide resolved
core/extension/gdnative_interface.cpp Outdated Show resolved Hide resolved
@reduz reduz force-pushed the implement-native-extensions branch 2 times, most recently from 786193e to 5067206 Compare June 20, 2021 02:19
@akien-mga akien-mga requested review from a team and neikeq June 20, 2021 09:38
@CedNaru
Copy link
Contributor

CedNaru commented Jun 20, 2021

Correct me if I'm wrong but it seems the new API dump doesn't handle singletons.
The former "singleton" field in the JSON is gone and the "is_instantiable" is set to true.
Can we trust that the prefix "_" is enough to recognize a singleton?

image

@reduz
Copy link
Member Author

reduz commented Jun 20, 2021

@CedNaru Oh I just forgot about the singletons, have to add them back. I also dont think it should export the classes beginning with _ as such, I will change that.

};

typedef void *GDNativeVariantPtr;
typedef void *GDNativeStringNamePtr;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This and other functions use GDNativeStringNamePtr, so we should add helpers to convert from const char * to StringName and vice versa.

Or maybe these should just use const char * and let Godot do the conversion internally?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For this specific case, the only places where StringName is used is for the variant functions, for which I assume the binder has generated the right interfaces to use, so I am not sure its worth adding this. Generating a string from a CString and then a stringname from the right constructor is fast anyway.

Comment on lines +184 to +187
uint32_t hint;
const char *hint_string;
uint32_t usage;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we should also add PROPERTY_HINT and PROPERTY_USAGE enums to use with this. I know we can get this from the JSON but it's not very convenient when using this with pure C.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would probably avoid adding anything that is in the JSON to keep the API as simple as possible. If you want to actually use the Godot API from pure C for some strange reason, we could do pure C bindings too ala GTK.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we were also discussing with @groud about making the hints string only, although its a lot of work to change..

Comment on lines +380 to +392
void (*classdb_register_extension_class)(const GDNativeExtensionClassLibraryPtr p_library, const char *p_class_name, const char *p_parent_class_name, const GDNativeExtensionClassCreationInfo *p_extension_funcs);
void (*classdb_register_extension_class_method)(const GDNativeExtensionClassLibraryPtr p_library, const char *p_class_name, const GDNativeExtensionClassMethodInfo *p_method_info);
void (*classdb_register_extension_class_integer_constant)(const GDNativeExtensionClassLibraryPtr p_library, const char *p_enum_name, const char *p_class_name, const char *p_constant_name, uint32_t p_constant_value);
void (*classdb_register_extension_class_property)(const GDNativeExtensionClassLibraryPtr p_library, const char *p_class_name, const GDNativePropertyInfo *p_info, const char *p_setter, const char *p_getter);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing a way to register signals.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also: we should have a way of deregistering classes, for reloading purposes.

_FORCE_INLINE_ static void encode(const m_type &p_val, void *p_ptr) { \
*((m_type *)p_ptr) = p_val; \
} \
}

MAKE_PTRARG(bool);
MAKE_PTRARGCONV(bool, uint32_t);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason to use uint32_t instead of uint8_t? What do most compilers tend to use for bool?
Also does the compiler optimize this when the size is the same for both? If not we should probably use if constexpr.

Copy link
Member Author

@reduz reduz Jun 21, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

problem with bool is that, even if its uint8, most platforms will prefer 32 bit alignment (specially ARM) so I would rather leave it as 32.

Comment on lines 110 to 111
MAKE_PTRARGCONV(uint8_t, int64_t);
MAKE_PTRARGCONV(int8_t, int64_t);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the new extension API is a breaking change, can we take the opportunity to change this? GetTypeInfo has included info about int size and sign for quite some time now, so this conversion should not be needed anymore if the bindings use that info (as already do the C# bindings).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added this information in the bindings already, but this is mostly to create the right ones on the exposed API. The ptrcall API will still send everything through the biggest possible format (int64)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, the GDScript interpreter will use ptrcall if it knows the types now, and it always sends int64_t, its not possible to call the "right" one directly.

@neikeq
Copy link
Contributor

neikeq commented Jun 21, 2021

... I also dont think it should export the classes beginning with _ as such, I will change that.

Do you mean those should not be exposed? Classes beginning with _ should not be hidden. They only begin with _ to avoid conflicting with other classes in C++.

@vnen
Copy link
Member

vnen commented Jun 21, 2021

... I also dont think it should export the classes beginning with _ as such, I will change that.

Do you mean those should not be exposed? Classes beginning with _ should not be hidden. They only begin with _ to avoid conflicting with other classes in C++.

I think he means exposing them without the _. Which is the best IMO.

@reduz reduz force-pushed the implement-native-extensions branch from ecd2e19 to aebe779 Compare June 21, 2021 15:57
@reduz
Copy link
Member Author

reduz commented Jun 21, 2021

@CedNaru Added the singletons to the API json at the end of the file.

@reduz reduz force-pushed the implement-native-extensions branch from aebe779 to 94d1d91 Compare June 21, 2021 17:49
@reduz
Copy link
Member Author

reduz commented Jun 22, 2021

@arundilipan instead of creating the binary, the gdnativescript and manually assigning the script to the node, now you just create a new node type and it will appear directly in the editor for you to use.

@CedNaru
Copy link
Contributor

CedNaru commented Jun 22, 2021

From what I understand, extensions are directly registered into the ClassDB so when you want to create an "extended Object" in the editor instead of creating the base class and attaching a script to it, it directly behaves like if it was a regular native Object.
This way you don't have to use the scripting system of Godot.

Unlike scripts, you can't switch extensions at runtime dynamically without creating a new Object but the bright side is that you will still be able to modulate the behavior of extended Object with another script, giving 2 levels of controls. I see a strong potential for add ons. Developers can provide extensions as new usable objects for Godot that can still have custom behavior overwritten by a script written by the user. Currently, when I use an add-on and want to change the behavior of the new custom node provided I can only control it using a script on a parent node or whatever properties it has in the inspector.
Extensions will make this process much easier.

Also as a maintainer of Godot Kotlin, this will simplify our module a lot as we won't have to keep the .kt files around to assign them to object as scripts. (Currently forcing us to have a hashmap (path of the script -> actual Kotlin class). Only the final jar will be used.

Tell me if I got something wrong, it's the understanding I got by reading the commit changes.

@arunvickram
Copy link

So @reduz if I'm understanding this, there's no separate GDNative scripts needed, it's first class support for Kotlin or Python or Ruby classes in the Godot editor?

@reduz
Copy link
Member Author

reduz commented Jun 22, 2021

@CedNaru You got it perfectly right
@arundilipan For dynamically typed languages, you most likely don't want to use this route and instead provide a core extension that registers a script language. Again this process is simpler too, you no longer have to use PluginScript, just inherit ScriptLanguage like you would in a module.

@reduz reduz force-pushed the implement-native-extensions branch 3 times, most recently from 5753968 to 4e86a29 Compare June 23, 2021 14:19
@willnationsdev
Copy link
Contributor

willnationsdev commented Jun 25, 2021

For dynamically typed languages, you most likely don't want to use this route and instead provide a core extension that registers a script language. Again this process is simpler too, you no longer have to use PluginScript, just inherit ScriptLanguage like you would in a module.

So, what exactly would this look like?

From what I can understand of the changes, I will be able to write a C program that includes gdnative_interface.h and calls various gdnative extension functions to define an "extension" with one or more "classes" and their associated name, base type, properties, methods, signals, and integer constants. I can then mark when the classes should be registered (core/server/scene/editor). I then compile this into a dynamic library, e.g. *.dll on Windows, drop the file into my Godot project, and register it in the project settings or dynamically open the library using the NativeExtensionManager singleton. Is this right?

The thing that trips me up is that ScriptLanguage doesn't extend Object. It's effectively just an interface. So, if we are just registering things to the ClassDB as (I believe?) you said, we wouldn't be able to specify that the class we are introducing extends ScriptLanguage (we can only extend Object and its derivatives). Is this correct? If so, how does the dynamic language support work?

Also, wouldn't that mean we need to include the script_language.h file and all of its dependencies to be able to refer to the ScriptLanguage class? Are we effectively compiling our custom C code alongside the engine source code itself and then taking that library and just dropping it into a project to have a delayed runtime load? Cause, I seem to recall asking you previously on Twitter if the GDNative 2.0 code would have access to core data structures and the like, and you said that wasn't the case.

I'm a little confused as to what the capabilities and restrictions are with this.

@reduz
Copy link
Member Author

reduz commented Jun 25, 2021

@willnationsdev for the most part, virtual abstract classes like ScriptLanguage will have a special binder version (likely ScriptLanguageNative) like they do now, but it will be entirely defined in C++ using the new system for registering and native virtual functions. You can even pass pointers and other stuff to them.

Copy link
Member

@vnen vnen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are a few issues to iron out but the general idea looks good to me.

@reduz reduz force-pushed the implement-native-extensions branch 2 times, most recently from 06955b9 to 402d83a Compare June 25, 2021 18:42
@bruvzg
Copy link
Member

bruvzg commented Jun 25, 2021

What's the future plans for the low-level extensions (VideoDecoder, XR and TextServer), custom API or NativeExtension extended classes? If it's going to be NativeExtension what should be done with the functions using non-Variant arguments (raw char * pointers in the video decoder, and Vector<TextServer::Glyph> in the text server)?

@reduz reduz force-pushed the implement-native-extensions branch from 402d83a to 012b4db Compare June 25, 2021 19:37
@reduz
Copy link
Member Author

reduz commented Jun 25, 2021

@bruvzg New new system for native extension virtuals should allow for this, which had to be hardcoded in C in the past. It should also allow for custom function formats for the very specific types. The idea is to first test this system for regular development, then add these classes so we can have what we had previously in GDNative.

* Deprecates GDNative in favor of a simpler, lower level interface.
* New extension system allows registering core engine classes.
* Simple header interface in gdnative_interace.h
@reduz reduz force-pushed the implement-native-extensions branch from 012b4db to b1d15c5 Compare June 25, 2021 20:33
@reduz reduz marked this pull request as ready for review June 25, 2021 20:34
@reduz reduz requested review from a team as code owners June 25, 2021 20:34
@akien-mga akien-mga merged commit 56dafe9 into godotengine:master Jun 25, 2021
@akien-mga
Copy link
Member

Thanks!

@akien-mga akien-mga changed the title Implement native extension system Implement native extension system (GDExtension) Jun 30, 2021
@GeorgeS2019
Copy link

@neikeq can U suggest use cases how C# developers can take advantage of this exciting GDExtension.

@neikeq
Copy link
Contributor

neikeq commented Jul 23, 2021

@GeorgeS2019 It's not possible yet. I'll add support for it in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.