-
-
Notifications
You must be signed in to change notification settings - Fork 586
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
Add CallableCustom
that devs can use in their GDExtensions
#1280
Conversation
c1f3235
to
b733a16
Compare
CallableCustom
that devs can use in their GDExtensionsCallableCustom
that devs can use in their GDExtensions
I just added a automated test that tests the functionality this PR adds! So, this should now be ready for review :-) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
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.
Besides these small annoyances overall the changes seem great so far. Edit: |
@Trey2k Thanks for testing and the feedback! Yeah, it's tricky to match the API exactly to what's in the engine.
This one we may be able to fix in the future. We don't have
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.
This was a judgement call I made: the API in
Ah, we could add this! We have a function to do it in
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 |
b733a16
to
73f7a8b
Compare
Some of this has already been mentioned, but here are my findings and thoughts. (please forgive formatting and abbreviations, on mobile, may edit later)
|
73f7a8b
to
e81a829
Compare
240543e
to
f15ec7d
Compare
Alright! The latest version of this PR has the following updates:
|
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. |
29bd975
to
91eea26
Compare
91eea26
to
d33bd47
Compare
@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. |
@Trey2k Thanks so much for testing! I think I'm going to "live dangerously" and go ahead and finally merge this one :-) |
In PR #1155, we added the ability to use
callable_mp()
andcallable_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 theCallableCustom
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 :-)