-
-
Notifications
You must be signed in to change notification settings - Fork 214
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
fix: ambiguous virtual method calls #1020
base: master
Are you sure you want to change the base?
fix: ambiguous virtual method calls #1020
Conversation
API docs are being generated and will be shortly available at: https://godot-rust.github.io/docs/gdext/pr-1020 |
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.
Thanks a lot! The implementation looks good from what I can see 🙂
So I am not 100% of if this fix is entirely proper or not.
Yep, I think the case you're handling is the important one. To verify that it works, could you construct an #[itest]
with a simple example like in #858? Maybe func_test.rs
is a good location; you can possibly reuse the existing classes and just add extra traits.
Some(api_trait) => { | ||
let instance_ref = match signature_info.receiver_type { | ||
ReceiverType::Mut => quote! { &mut instance }, | ||
_ => quote! { &instance }, |
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.
do we want to just silently fail here if an incorrect receiver is passed somehow? it should generate a compile error either way, but maybe we'd wanna explicitly pass a compile_error!
instead?
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 is inside of a match arm against receiver_type
that matches ReceiverType::Mut
and ReceiverType::Ref
, so it is impossible in this case for it to be anything else.
917858c
to
ddf30e6
Compare
Fixes #858
This issue was a great way to start getting introduced to the internals of this library. I'm still figuring out exactly what does what, and some of the libraries in use are also new to me. So I am not 100% of if this fix is entirely proper or not. For example, as far as I can tell, the
__before_<method>
function is really only ever__before_ready
and is never a member of any Godot API trait or any type that would need it to be qualified. I'm also still wrapping my head around the web of different modules and functions and types. I would appreciate some guidance.Pre-macro example
Expanded code before this change
Expanded Rust code after change