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

GDExtension: Allow directly getting ObjectID from Variant #97119

Merged

Conversation

dsnopek
Copy link
Contributor

@dsnopek dsnopek commented Sep 17, 2024

Presently, we don't have a way to directly get the ObjectID from a Variant that holds an Object.

In godot-cpp (and probably other bindings), we implement getting the ObjectID by first getting the Object *, however, this won't work for one common use of getting the ObjectID: checking if the Object is still valid.

This adds a new variant_get_object_instance_id() function to gdextension_interface.h.

See PR godotengine/godot-cpp#1591 for integrating this with godot-cpp.

@Bromeon
Copy link
Contributor

Bromeon commented Sep 17, 2024

Nice! 🙂

I think the behavior should be specified if:

  • the Variant has another type than OBJECT
  • the pointed-to object is dead

Can the two cases be differentiated by just this API? Or are follow-up calls (is_instance_id_valid, Variant::get_type) needed?

@dsnopek
Copy link
Contributor Author

dsnopek commented Sep 17, 2024

  • the Variant has another type than OBJECT

It's currently coded to return 0 if the type isn't object. This is different than the vanilla Variant::operator ObjectID() which will also return the integer value if the variant is an integer. However, I feel like what's in the current PR aligns better with what we actually need this function for. If bindings want to replicate the Variant::operator ObjectID() behavior, they can do it themselves.

I'll add a note about this to the docs.

the pointed-to object is dead

It returns the instance ID of the object, even if it's no longer valid. This matches Godot's internal behavior, and if we don't do that, we don't have a way replicate it.

This is also worth putting in the docs.

Or are follow-up calls (is_instance_id_valid, Variant::get_type) needed?

I personally think follow-up calls are the way to go.

@dsnopek dsnopek force-pushed the gdextension-object-instance-id branch from 393a3f9 to 971e154 Compare September 17, 2024 16:59
Copy link
Contributor

@Bromeon Bromeon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Docs look good now, thanks!

@akien-mga akien-mga modified the milestones: 4.x, 4.4 Oct 4, 2024
@akien-mga akien-mga merged commit 4dd812f into godotengine:master Oct 4, 2024
20 checks passed
@akien-mga
Copy link
Member

Thanks!

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

Successfully merging this pull request may close these issues.

3 participants