Add CallableCustom that devs can use in their GDExtensions

[dsnopek]

In PR #1155, we added the ability to use callable_mp() and callable_mp_static() to create custom callables for C++ function pointers.

However, it would also be great to allow developers to create their own sort of custom callables. For example, imagine a GDExtension that adds a scripting language with lambda's, and being able to create callables out of those lambdas. (See this discussion on the Godot Lua API issue queue.)

This PR aims to add a CallableCustom class, similar to the CallableCustom class in the engine, which developers can extend to create a new kind of custom callable.

It renames some of the functions added in PR #1155 to make it easier to distinguish which go with which kind of custom callable.

It's a draft because I haven't actually tested it yet :-)

[dsnopek]

I just added a automated test that tests the functionality this PR adds! So, this should now be ready for review :-)

[akien-mga]

Looks good to me.

[Trey2k]

I am in the process of testing these changes for my extension now. So far it seems great with a couple of notes that make using the same code base for module and GDExtension builds slightly annoying.

• call method
  This one is not too big of a deal as I think this happens a lot with the error type in godot-cpp, but the type of r_call_error in a module would be a Callable::CallError but GDExtensionCallError is used here instead.
• is_valid
  For some reason currently is_valid is a virtual in godot-cpp. This is not the case in the base engine.
• get_object
  The godot-cpp version of this method returns an actual Object* where in the base engine we are only returning the instance ID of the object.
• Callable class its self
  Another very important thing for my extension is the ability to get a CallableCustom from a callable. In module builds we use the Callable::get_custom method but this is not present in godot-cpp currently

Besides these small annoyances overall the changes seem great so far.

Edit:
One I forget to mention was the compare_less and compare_equal. In godot-cpp these are currently returning a GDExtensionBool instead of a bool. Causing some extra ifdefs.

[dsnopek]

@Trey2k Thanks for testing and the feedback!

Yeah, it's tricky to match the API exactly to what's in the engine.

• call method
  This one is not too big of a deal as I think this happens a lot with the error type in godot-cpp, but the type of r_call_error in a module would be a Callable::CallError but GDExtensionCallError is used here instead.

This one we may be able to fix in the future. We don't have Callable::CallError in godot-cpp, but we could certainly add it! This should probably be its own PR, though, because we'd have to update all the other places we're using GDExtensionCallError.

• is_valid
  For some reason currently is_valid is a virtual in godot-cpp. This is not the case in the base engine.

It is virtual in the engine too, it's just not pure virtual, it has a default implementation. I suppose we could add a similar default implementation? I'll see what I can do.

• get_object
  The godot-cpp version of this method returns an actual Object* where in the base engine we are only returning the instance ID of the object.

This was a judgement call I made: the API in gdextension_interface.h takes an Object *, and so we could have get_object() return ObjectID, but then we'd have to look up the Object * anyway, and I know that on the other side Godot is going to turn that back into an ObjectID, so it'd be a useless lookup. The only good fix here is to change the API in gdextension_interface.h to take an ObjectID, which maybe we could try to sneak in before Godot 4.2?

• Callable class its self
  Another very important thing for my extension is the ability to get a CallableCustom from a callable. In module builds we use the Callable::get_custom method but this is not present in godot-cpp currently

Ah, we could add this! We have a function to do it in gdextension_interface.h. I'll add it when I have a chance.

One I forget to mention was the compare_less and compare_equal. In godot-cpp these are currently returning a GDExtensionBool instead of a bool. Causing some extra ifdefs.

I think this one is unavoidable. Those function pointers are passed to the Godot side, and we can't pass around function pointers that use bool because it's size is undefined - it could be different depending on the compiler, which is why we have GDExtensionBool.

[maiself]

Some of this has already been mentioned, but here are my findings and thoughts. (please forgive formatting and abbreviations, on mobile, may edit later)

• typedef for compare funcs have different return types

  • solvable by implementing cmp funcs as static funcs which call the virtual ones
  • maybe also some template metaprograming to selectively do the above, skipping if types are same at abi level

• core has default impl for is_valid()

• call() has different error out arg type, Callable::CallError in core, PR has GDExtensionCallError

  • should probably add a Callable::CallError to godot-cpp

• get_object() returns ObjectID in core, PR has Object*

  • core also has some wierdness around this, Callable::get_object() returns Object*, CallableCustom::get_object() returns ObjectID

  • core Callable also has a get_object_id()

  • core Callable::get_object() always calls ObjectDB::get_instance() on the ObjectID...

  • core uses ObjectID as its safer, if the id is valid then you get nullptr when converting, but if you were to use Object* you have no way of knowing if the object is still valid (dangling pointer)

  • gde interface uses object only to get its ObjectID

  • object_get_instance_id() exposed to gde interface directly, so it shouldnt be much harder to create custom callables if the type is changed...

  • core callable_mp converts ptr -> id alot...

  • my python bindings dont even use object, its set to nullptr in all cases...

  • given the above I don't really see a reason against changing the type from object ptr to object id, as long as we can do it now before 4.2...

• if get_custom() is called on a callable_mp any calls to its members will lead to a crash. either callable_mp needs to inherit CallableCustom or a different token should be used to exclude callable_mp from get_custom

[dsnopek]

Alright! The latest version of this PR has the following updates:

• get_object() now returns ObjectID
• There's a default implementation of is_valid(), although, I have mixed feelings about it. It's the same default implementation as in Godot, but I feel like it's not as good a default from GDExtension, since in most cases you don't care about the object.
• Following @maiself's suggestion, the CompareEqualFunc and CompareLessFunc now return bool, by having an intermediate function that redoes some of the logic from Callable::operator==() and Callable::operator<() from Godot.
• The two types of custom callables possible in godot-cpp (ones from callable_mp() or descending CallableCustom) now share a base class, so that Callable::get_custom() can safely return only valid CallableCustom * objects
• I also needed to work around some cyclic includes to get ObjectID in all the right places, so its definition has moved to a new file, and there's some code to prevent generating a duplicate struct ObjectID from the "native_structures" in extension_api.json

[Trey2k]

There's a default implementation of is_valid(), although, I have mixed feelings about it. It's the same default implementation as in Godot, but I feel like it's not as good a default from GDExtension, since in most cases you don't care about the object.

I think this default is fine in my opinion, my extension the object is the lua VM its self, if its valid the method will still be valid as methods pulled as a Callable get copied. And if the object is not the determining factor then it can always be overridden.

[dsnopek]

@Trey2k Can you confirm if the latest version of the PR works for you? Looking PR WeaselGames/godot_luaAPI#180 for your bindings, it seems you're still using the older version, before it got rewritten to more closely resemble the Godot API.

[Trey2k]

@Trey2k Can you confirm if the latest version of the PR works for you? Looking PR WeaselGames/godot_luaAPI#180 for your bindings, it seems you're still using the older version, before it got rewritten to more closely resemble the Godot API.

Just updated the PR. New changes seem to work great! Thanks for the work you have done on this.

[dsnopek]

@Trey2k Thanks so much for testing!

I think I'm going to "live dangerously" and go ahead and finally merge this one :-)