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

Add get_script_instance() to GDExtension #7199

Closed
fuzzybinary opened this issue Jul 3, 2023 · 8 comments · Fixed by godotengine/godot#80040
Closed

Add get_script_instance() to GDExtension #7199

fuzzybinary opened this issue Jul 3, 2023 · 8 comments · Fixed by godotengine/godot#80040
Milestone

Comments

@fuzzybinary
Copy link

Describe the project you are working on

I'm working on binding Dart as both an extension and scripting language in Godot:
https://github.com/fuzzybinary/godot_dart/

Describe the problem or limitation you are having in your project

I need to be able to call Godot functions from GDExtension which return Objects, then cast these objects to defined by scripts.

For example from 3D tutorial

 var collision = getSlideCollision(i);
var mob = gde.cast<Mob>(collision.obj?.getCollider(0));
if (mob == null) continue;

var collisionNormal = collision.obj?.getNormal(0) ?? Vector3.up();
if (Vector3.up().dot(collisionNormal) > 0.1) {
  mob.squash();
  targetVelocity.y = bounceImpulse.toDouble();
}

Currently, cast is calling gdextension_object_cast_to which fails because the Mob type (defined only in scripts) does not have a class tag.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

Adding gdextension_get_script_instance would allow me to do the following in my cast functtion:

  • Get the classTag for the type I'm trying to cast to. If it exists, call gdextension_object_cast_to
  • If it doesn't, call gdextension_get_script_instance and check if the language is our language
  • If it is, get the object from the ScriptInstanceExtension and use the language to perform proper casting
  • If it is not, return null.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

Add gdextension_get_script_instance to the next version of GDExtension

If this enhancement will not be used often, can it be worked around with a few lines of script?

The only workaround I can see is to have my ScriptLanguageExtension or ScriptExtension store pointers to all ScriptInstances that are created, and use that to do casting.

Is there a reason why this should be core and not an add-on in the asset library?

Because it involves the interface for creating add-ons.

@Calinou Calinou changed the title GDExtension shoud support get_script_instance Add get_script_instance() to GDExtension Jul 3, 2023
@dsnopek
Copy link

dsnopek commented Jul 7, 2023

I'm not super familiar with how scripting languages are implemented in Godot (this is an area that I hope to spend some time learning soon!), but if you have a pointer to an Object, you should be able to call the Object's get_script() method registered in ClassDB.

Would that get you access to what you need?

@fuzzybinary
Copy link
Author

No, unfortunately get_script only returns the Script resource (the actual file that contains the script). The ScriptInstance is created from that script and then attached to the object. This is what holds the actual thing that I use to do interactions from the scripting language.

@dsnopek
Copy link

dsnopek commented Jul 24, 2023

Ah, ok, thanks! I've spent the weekend learning more about the scripting language APIs in Godot and GDExtension, and I do think it would be nice to have a function like you described added to the GDExtension interface. While it is possible to solve this problem without it (see below!), it is a bit of work to access a piece of information that is "right there" and should be easier to get. If you make a PR for this, I'd happily review it!

But if you were interested in a way to solve this without changing the GDExtension interface:

I looked at the Godot Luau bindings, and they've got a HashMap on their custom Script class (called LuauScript) which tracks the instances by the object id of the object they are attached to.

So, in their LuauScript::_instances_create(Object *p_for_object), they're creating the instance, and using the p_for_object->get_instance_id() as the key on the HashMap. Then later when they encounter any old Object, they can call get_script(), see if it's a LuauScript, and if so, try to look up the instance by the object id in the HashMap on the LuauScript object.

Hopefully, that makes sense! Trying to explain code in prose is always tricky :-)

@fuzzybinary
Copy link
Author

Yeup, that makes sense and I figured that would be the workaround.

I'm working on updating godot_dart to Godot 4.1, and once that's done I have an implementation of object_get_script_instance that I'll test out. Once I've tested I'll submit the PR. This won't get around the need to call get_script and check the language though.

@dsnopek
Copy link

dsnopek commented Jul 24, 2023

Thanks, sounds good!

This won't get around the need to call get_script and check the language though.

It'd be nice to bake the language check into the object_get_script_instance function somehow. Could we pass in an argument that would only return the script instance if it matches a given language? That would prevent GDExtensions from accidentally using the instance data from another extension.

@fuzzybinary
Copy link
Author

It'd be nice to bake the language check into the object_get_script_instance function somehow. Could we pass in an argument that would only return the script instance if it matches a given language?

Potentially. I think the only way available to us right now is to pass in the ScriptLanguage / ScriptLanguageExtension to object_get_script_instance, then call ScriptLanguage::get_language and compare the pointers. This would make the signature of the new function:

GDExtensionScriptInstanceDataPtr object_get_script_instance(GDExtensionConstObjectPtr p_language, GDExtensionConstObjectPtr p_object);

Not necessarily relevant for this conversation, but I'm wondering how / if ScriptLanguages can talk to each other in the future. Topic for another time.

@dsnopek
Copy link

dsnopek commented Jul 24, 2023

This would make the signature of the new function:

GDExtensionScriptInstanceDataPtr object_get_script_instance(GDExtensionConstObjectPtr p_language, GDExtensionConstObjectPtr p_object);

That signature seems good to me!

@dsnopek
Copy link

dsnopek commented Jul 25, 2023

Actually, the one thing I'd say about that signature is that the order of the arguments should probably be swapped. All the other object_* functions in gdextension_interface.h take the object they are operating on as the first argument, and so, for consistency, this one probably should do that as well.

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

Successfully merging a pull request may close this issue.

4 participants