Rework GDExtension interface from a struct to loading function pointers

[dsnopek]

At the GDExtension meeting on 2023-04-14, we were discussing (among other things) how to allow us to add new methods to the GDExtensionInterface struct without breaking the ABI. (There is at least one GDExtension bug that requires a new method to be added in order to fix it.)

We discussed a few possibilities, including just adding new pointers to the end of the struct, or adding a void *next pointer to the end of the struct (so that could point to a new struct with new methods, similar to how we did it in GDNative).

@vnen had the idea of having a "low-level ClassDB" where we could register these functions, rather than putting them in a big struct. And @BastiaanOlij brought up how many APIs (like OpenGL, OpenXR and Vulkan) do exactly that, where you're given a void *GetProcAddress(const char *p_name) function and then load each of the function pointers individually.

So, this PR is attempting to implement that!

Here's an overview:

• The GDExtensionInterface struct no longer exists in gdextension_interface.h, and is replaced by a bunch of function pointer typedefs, and the initialization function in each GDExtension is now passed a GDExtensionInterfaceGetProcAddress function pointer instead
• In the XXX.gdextension file, you can set a compatibility_minimum property (came from a discussion on RocketChat with @mhilbrunner in the gdextension channel) to denote the minimum Godot version that a given GDExtension needs
• Any GDExtension without comptability_minimum or where it's set to 4.0 will still get passed the "legacy" structure, so GDExtensions made for Godot 4.0 should still work with Godot 4.1+, but extensions made for Godot 4.1+ won't work with Godot 4.0 (Its on the agenda for the next GDExtension meeting to discuss if this sort of forward-but-not-backward compatibility is OK)
• The companion PR for godot-cpp (Update to load function pointers for GDExtension interface godot-cpp#1095) also does some work to ensure that a newer GDExtension that uses this new interface won't be loaded by Godot 4.0, with a nice message rather than a crash

This is currently a draft for discussion! If the overall approach looks good, there's a bunch of little bits of clean-up I'd like to do (in addition to whatever comes up in review)

UPDATE: No longer a draft!

[BastiaanOlij]

This is looking really good David,

One other thing we discussed that we need to take along here is documenting what these functions do and basically create a document about the GDExtension C API.

One suggestion I had was to add this documentation alongside the function definitions in gdextension_interface.h because that is the file that ends up being distributed to any extension being created.

Alternatively we can make this work the same way all documentation works in Godot and look into feeding the function definitions into our documentation XML. Especially since you've implemented a function register system and we thus have an array with all function pointers, we can use this to generate our documentation as well.
This may mean that we also need to add some meta data here such as the function name and parameters. A bit of smart macro work could automate that for us.

[dsnopek]

Thanks!

One other thing we discussed that we need to take along here is documenting what these functions do and basically create a document about the GDExtension C API.

Yep! That's one of the "clean up" things I was planning to do once I was sure that folks were OK with the overall approach.

Alternatively we can make this work the same way all documentation works in Godot and look into feeding the function definitions into our documentation XML.

That's a really interesting idea! It'd certainly be nicer to point to well formatted docs online, as opposed to just comments in a header file. I'll look into it.

[mhilbrunner]

This indeed looks good! 🎉 And thanks for getting the compatibility_minimum in!

FWIW,

so GDExtensions made for Godot 4.0 should still work with Godot 4.1+, but extensions made for Godot 4.1+ won't work with Godot 4.0 (Its on the agenda for the next GDExtension meeting to discuss if this sort of forward-but-not-backward compatibility is OK)

This is perfectly OK IMO, lots of stuff works like this.

[dsnopek]

One suggestion I had was to add this documentation alongside the function definitions in gdextension_interface.h because that is the file that ends up being distributed to any extension being created.

Alternatively we can make this work the same way all documentation works in Godot and look into feeding the function definitions into our documentation XML. Especially since you've implemented a function register system and we thus have an array with all function pointers, we can use this to generate our documentation as well.

I've been struggling with how to move forward on documentation.

I really like the idea of generating XML that gets transformed into something nice for the docs. OpenGL also works like this - the "source" of the API is XML, and then the HTML docs are generated from that. This seems like a good fit for an API that is based on loading function pointers, because you're actually documenting sort of "imaginary functions".

My first thought was that we could generate a @GDExtensionInterface.xml from the registered interface functions, and list each of the functions with <method name="..."> (where 'name' is the name it's registered with, that you need to pass to the "GetProcAddress()" function) in the same XML format that we're using for rest of the class reference.

However, looking at Godot's make_rst.py and the godot-docs repo, I think this would require a ton of work to do well, and without creating some confusion. First of all, the types on the parameters would need to refer to C types, not normal Godot types. And we'd need a way to provide definitions for those C types too, and there isn't a nice way to make that all self-contained in a single @GDExtensionInterface.xml. All the while keeping this stuff separate from the other docs so we don't confuse users into thinking that these functions can be called from GDScript or C#.

So, I'm thinking it might be best to:

1. Just put the documentation into the header file. :-) This will be unstructured, but maybe we could use some adhoc structure that we could possibly parse later, like @param, @name, @return etc?
2. Punt on the documentation for now, with the plan to come back to this later, and then do all the work necessary to setup a system that generates XML from the function registrations and transforms that into something nice for the docs?

@BastiaanOlij What do you think?

[dsnopek]

My latest push shows a proposed way to include the docs in the gdextension_interface.h header:

https://github.com/godotengine/godot/compare/cd75ea62fcf067c865047beb2396a6529c61c442..0aa52e104d55ba056657746e1bc28d9c0040eb50

Does this look like an acceptable way to handle it?

If so, I can go through and add docs like this to all of the interface functions!

[BastiaanOlij]

@dsnopek looks good!

Indeed if it's too much work to get the documentation working through the XML approach it shouldn't be something we hold this back on. This really is only interesting to people who are building the bindings themselves and they would be using the header file.

It may be worth talking to some of the guys who know more about the in build documentation system like George (vnen) or Hugo (Calinou) if you haven't already.

But I'm happy if we continue with having the descriptions in the header file.

[Calinou]

The docs aspect looks good to me; most IDEs will display those on hover. Of course, there should be a corresponding manual page to tell you to check the interface header to know those details in the first place 🙂

[dsnopek]

Great, thanks!!

[dsnopek]

I've finished adding documentation for all the registered GDExtension interface functions. There's no other changes I was planning on this PR, so I'm taking this one out of draft - it's ready for review!

(I've still got some changes I want to make on the companion PR for godot-cpp, so I'm leaving that one as a draft for now. Of course, they will need to be merged at the same time so that tests pass on both ends. Updates coming soon on that one.)

[dsnopek]

Discussed at the GDExtension meeting, and approved!

Needs to be merged at the same time as godotengine/godot-cpp#1095 so that the tests pass (they are only failing here currently because the godot-cpp changes are required).

[dsnopek]

This PR conflicts with PR #35813, so one of them would need to be rebased on the other. To be fair to @touilleMan whose PR was approved at an earlier GDExtension meeting than mine, I've rebased this PR on his.

[dsnopek]

Now that PR #35813 has been merged, I've rebased on master - this should be ready to merge!

[akien-mga]

Seems like the godot-cpp change needs to be merged first, then this one rebased.

The best would be for you to do the merges when you're around, so you can merge the godot-cpp one, rebase this PR to force a new build against the new godot-cpp, and merge it once ready.

[akien-mga]

Thanks!