Use GDExtensionCallableCustomInfo2 instead of deprecated GDExtensionCallableCustomInfo.
#952
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
277863e to
0af2f6d
Compare
|
API docs are being generated and will be shortly available at: https://godot-rust.github.io/docs/gdext/pr-952 |
Bromeon
reviewed
Nov 23, 2024
godot-core/src/builtin/callable.rs
Outdated
Comment on lines
19
to
21
| #[cfg(before_api = "4.3")] | ||
| #[cfg(since_api = "4.2")] | ||
| type GDExtensionCallableCustomInfo = sys::GDExtensionCallableCustomInfo; |
There was a problem hiding this comment.
You can use all(...) here 🙂
Suggested change
| #[cfg(before_api = "4.3")] | |
| #[cfg(since_api = "4.2")] | |
| type GDExtensionCallableCustomInfo = sys::GDExtensionCallableCustomInfo; | |
| #[cfg(all(since_api = "4.2", before_api = "4.3"))] | |
| type GDExtensionCallableCustomInfo = sys::GDExtensionCallableCustomInfo; |
godot-core/src/builtin/callable.rs
Outdated
Comment on lines
22
to
23
| #[cfg(since_api = "4.3")] | ||
| type GDExtensionCallableCustomInfo = sys::GDExtensionCallableCustomInfo2; |
There was a problem hiding this comment.
The naming is a bit confusing -- the use sites look like sys:: was forgotten and the code is referring to sys::GDExtensionCallableCustomInfo.
Maybe just like this, to make clear it's not a typedef exported by the GDExtension API itself?
Suggested change
| #[cfg(since_api = "4.3")] | |
| type GDExtensionCallableCustomInfo = sys::GDExtensionCallableCustomInfo2; | |
| #[cfg(since_api = "4.3")] | |
| type CallableCustomInfo = sys::GDExtensionCallableCustomInfo2; |
…onCallableCustomInfo`. ====== The only difference between old and new `GDExtensionCallableCustomInfo` is an addition of `get_argument_count_func`. In godot-cpp implementation the only way to get this count is overriding method in the question; For now we are leaving nullptr – consider implementing builder pattern for the Callable (`Callable::from_…ex(…).arg_count(n).done()`) if the need to do so arise.
0af2f6d to
6f21759
Compare
Bromeon
approved these changes
Nov 23, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The only difference between old and new
GDExtensionCallableCustomInfois an addition ofget_argument_count_func. In godot-cpp implementation the only way to provide an argument count is overriding the method in a question.For now I'm leaving the nullptr – consider implementing builder pattern for the Callable (
Callable::from_…ex(…).arg_count(n).done()) if the need to do so arise (would require changes to userdata stored with the callable).One could provide blanket implementation, but IMO it would be only more confusing. Gdext checks if given function is present https://github.com/godotengine/godot/blob/0c45ace151f25de2ca54fe7a46b6f077be32ba6f/core/extension/gdextension_interface.cpp#L149, executes it and returns the result if it is valid (
r_is_valid) – otherwise it returns "0" (in other words, the default implementation is already provided by the engine itself).