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

Allow implementing ScriptInstance::validate_property() from GDExtension #81261

Merged

Conversation

dsnopek
Copy link
Contributor

@dsnopek dsnopek commented Sep 2, 2023

Fixes #81119

I haven't had a chance to test this yet.

It adds this function pointer type:

typedef GDExtensionBool (*GDExtensionScriptInstanceValidateProperty)(GDExtensionScriptInstanceDataPtr p_instance, GDExtensionPropertyInfo *p_property);

I opted to have it return GDExtensionBool (the ScriptInstance::validate_property() method doesn't return anything) since we have to copy back and forth between the Godot and GDExtension version of PropertyInfo. I figure this could allow the GDExtension to check if the script even implements validate_property(), and if it doesn't, just return false in order to skip one of the copies. Does that make sense? I'd love some feedback on that.

@dsnopek dsnopek added this to the 4.2 milestone Sep 2, 2023
@dsnopek dsnopek requested review from a team as code owners September 2, 2023 22:41
@dsnopek dsnopek force-pushed the gdextension-validate-property branch from 83d73ad to 3e7a6e0 Compare September 4, 2023 18:55
@YuriSizov
Copy link
Contributor

This PR only allows scripting languages created by extensions to implement this method, so I assume that implementing an extension hook to Object::validate_property itself is a separate issue?

@dsnopek
Copy link
Contributor Author

dsnopek commented Sep 5, 2023

Ah, yes, I was focused on scripts because that's what the issue was about, but that would also need to be done. I've just added that to my TODO list as well :-)

Copy link
Contributor

@YuriSizov YuriSizov left a comment

Choose a reason for hiding this comment

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

The core part of this seems correct to me. If the Object hook is to be done in a separate PR, then this should be okay, I think.

@akien-mga akien-mga merged commit d1c94ee into godotengine:master Sep 7, 2023
15 checks passed
@akien-mga
Copy link
Member

Thanks!

@dsnopek dsnopek deleted the gdextension-validate-property branch July 22, 2024 15:27
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.

_validate_property() for GDExtension is not implemented
3 participants