godot-cpp
godot-cpp copied to clipboard
[DRAFT] Add `ScriptInstance` class which mimicks the API of Godot's internal `ScriptInstance` class
Background
Godot's internal ScriptInstance class (which is used in implementing scripting languages, like GDScript) isn't exposed the normal way, because we don't want scripts (ex a user script written in GDScript) to be able to directly interact with it.
Instead, GDExtensions get a special API from gdextension_interface.h to create script instances, allowing GDExtensions to provide new scripting languages for Godot.
However, working directly with that C API from C++ isn't ideal. And it also means big differences between GDExtensions and modules.
A number of extension projects have made their own C++ wrapper classes over this API to make life easier:
- https://github.com/Fumohouse/godot-luau-script/blob/master/src/scripting/luau_script.h#L216
- https://gitlab.com/snopek-games/godot-gravity-lang/-/blob/main/src/gravity_script_instance.h?ref_type=heads#L40
Given that this is something frequently done, I personally think it makes sense to have a wrapper in godot-cpp, so each project doesn't need to recreate it.
Implementation
This PR has the implementation I used in 'godot-gravity-lang', updated for the latest gdextension_interface.h, but I haven't re-tested it.
~~The main thing I don't like about it, is that it doesn't exactly mimick the API of ScriptInstance because it uses some types from gdextension_interface.h, rather than the equivalent Godot types (most of which also exist in godot-cpp).~~
~~The reason for this is that we'd need to copy data back and forth between the Godot types and gdextension_interface.h types, but script instances need to aim to be fairly performant. However, complete API compatibility may be worth it? It's something we'll need to discuss.~~
UPDATE: We discussed and decided that it was more important to mimick the Godot API as much as possible, and not worry about the performance of copying data in some cases, so the PR has been updated to reflect that.
Making a DRAFT for now so that we can discuss whether or not we want this in godot-cpp, and if this implementation makes sense.
We discussed this PR at the last GDExtension meetup, and decided that it'd be best for ScriptInstanceExtension to exactly match the method signatures from Godot, even if that means allocating and copying a bunch of extra memory.
I've updated this PR with everything that I think we need to do in order to do that, however, I still haven't had a chance to actually test it. So, leaving as a draft until I do.
Do you think it is possible to add the same for ScriptInstancePlaceholder in this PR ?
Do you think it is possible to add the same for ScriptInstancePlaceholder in this PR ?
Yes, I think we could do a similar thing with PlaceHolderScriptInstance, but I don't know if it should be in this PR.
This one has already been sitting here for ~9 months because it's gotten insufficient testing. Adding a new thing means even more things to test :-)
If you had time to help test this PR, that would be much appreciated!
I've made some updates (a few of which were discussed on RocketChat):
- Renamed from
ScriptInstanceExtensiontoScriptInstanceto match the class name within Godot - Added a default implementation of
ScriptInstance::get_property_state()copied from the Godot version - Avoid
thread_localon MacOS when hot-reload is enabled (just like inwraped.hpp/.cpp) since there's issues withthread_localbreaking reload on that platform
I've added an implementation of PlaceHolderScriptInstance now too, but this is all untested code