-
-
Notifications
You must be signed in to change notification settings - Fork 260
Support rename for #[var]
#1388
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
Conversation
|
API docs are being generated and will be shortly available at: https://godot-rust.github.io/docs/gdext/pr-1388 |
4c74fd7 to
729dd56
Compare
Bromeon
left a comment
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!
| assert!(prop_list | ||
| .iter_shared() | ||
| .any(|d| d.get("name") == Some("renamed_variable".to_variant()))); | ||
| assert!(prop_list | ||
| .iter_shared() | ||
| .any(|d| d.get("name") == Some("renamed_variable".to_variant()))); |
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.
Accidental duplicate?
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.
oh whoops yeah i meant to check that it doesn't contain the unused name
| #[itest] | ||
| fn test_renamed_var_getter_setter() { | ||
| let obj = HasProperty::new_alloc(); | ||
|
|
||
| assert!(obj.has_method("get_renamed_variable")); | ||
| assert!(obj.has_method("set_renamed_variable")); | ||
| assert!(!obj.has_method("get_unused_name")); | ||
| assert!(!obj.has_method("get_unused_name")); | ||
|
|
||
| obj.free(); | ||
| } |
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.
Are the generated getters in Rust (accessible at compile time) also renamed, or do they use the original field name?
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.
oh good question, i should test that
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.
it is renamed as well, but it should probably not be right? so that all the rust-accessible stuff uses the rust name
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.
I think it's OK to rename -- we should maybe discuss whether the auto-generated Rust getter/setters still make sense in general, in #1263 or so 🤔
729dd56 to
e45c0f3
Compare
e45c0f3 to
230547e
Compare
Bromeon
left a comment
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!
Also fix a broken link in the docs for the
GodotClassderive macro toPhantomVar.fixes #1262