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

#[export] infer VariantType #147

Merged
merged 1 commit into from
Mar 7, 2023

Conversation

mio991
Copy link
Contributor

@mio991 mio991 commented Mar 6, 2023

I removed the explicit VariantType declaration and replaced it with the ability to infer the same.

Also added an implementation for VariantMetadata for Option not sure if this is good but it was necessary.

@Bromeon Bromeon added quality-of-life No new functionality, but improves ergonomics/internals c: register Register classes, functions and other symbols to GDScript labels Mar 6, 2023
@Bromeon Bromeon mentioned this pull request Mar 6, 2023
Copy link
Member

@Bromeon Bromeon left a comment

Choose a reason for hiding this comment

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

This is a great simplification, thank you!

Sorry for the merge conflicts, I changed the proc-macro insides yesterday. Could you rebase onto master and squash into a single commit? Let me know if you need any help 🙂

Comment on lines +42 to +46
impl<T: VariantMetadata> VariantMetadata for Option<T> {
fn variant_type() -> VariantType {
T::variant_type()
}
}
Copy link
Member

Choose a reason for hiding this comment

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

We have to statically (at compile time) determine a variant type for a given Rust type.

Option::None however would typically correspond to NIL -- may that cause problems?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As far as I can see it, every time you recive a Variant it could be NIL even if you have declared another type.

For example if you reset a Property in the Inspector per default it gets set to NIL.

@mio991 mio991 force-pushed the faeture/export-infer-variant-type branch from c2c9ca0 to 67a68e4 Compare March 7, 2023 08:10
@Bromeon
Copy link
Member

Bromeon commented Mar 7, 2023

bors r+

@bors
Copy link
Contributor

bors bot commented Mar 7, 2023

Build succeeded:

  • full-ci

@bors bors bot merged commit fb15aef into godot-rust:master Mar 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: register Register classes, functions and other symbols to GDScript quality-of-life No new functionality, but improves ergonomics/internals
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants