-
-
Notifications
You must be signed in to change notification settings - Fork 21.5k
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
Expose _validate_property()
for scripting
#75778
Conversation
I'm pretty sure this is why I didn't expose it in #64339. I may even have an old branch somewhere that does the same as you do, but the problem with passing property info to extensions remains. |
Pretty sure it can be done the same way as in |
For me the problem was that with validate it's a two way communication, whereas property list only returns a list of properties. For validate we need to construct |
I changed the calling order. Now the script I also added a C# method stub. |
e80af6a
to
ae0a291
Compare
virtual void validate_property(PropertyInfo &p_property) const override { | ||
// TODO | ||
} |
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.
Would be good to have this implemented before merging. CC @godotengine/gdextension
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.
This'll require adding a new function pointer to GDExtensionScriptInstanceInfo
, which (per the pattern being established in PR #78634) actually means making a new struct with the new property and a new version of the script_instance_create
function in gdextension_interface.h
. We've been holding off on these sort of changes until PR #78634 is merged, wanting to get at least the first one in before doubling down on this pattern.
In any case, I'd be happy to either write the code to do this (not sure the best way to provide it with this unmerged, though?) or to provide guidance on how to do it.
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.
Makes sense. I guess let's wait for #78634, we could merge this PR with the TODO in the meantime, and make note / open an issue about implementing the GDExtension counterpart before the 4.2 release.
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.
Thanks! |
_validate_property()
has (at least) 2 uses in scripting:_get_property_list()
+_get()
+_set()
)This PR exposes it to users. I did it only for GDScript, as I'm not sure how to do it in other languages.
I also added a call to
_validate_property()
for exported properties. This is now possible: