Skip to content

Split ToVariant and FromVariant traits #187

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

Merged
merged 1 commit into from Aug 29, 2019
Merged

Split ToVariant and FromVariant traits #187

merged 1 commit into from Aug 29, 2019

Conversation

ghost
Copy link

@ghost ghost commented Aug 29, 2019

Cherry-picked this off #181 because I think it would still be useful out of that PR's context, and can be reviewed individually, for use cases like #185 (comment), where you only really need a one-way conversion.

Related sections from the original PR comment:

Explanation

This allows types to be separately ToVariant and FromVariant. ToVariant no longer require Sized. Bounds on export function arguments, return values, and property getter / setters are relaxed to require only one of these traits. As such, it's now possible to:

  • Use a type that is only FromVariant as an argument. e.g. a custom "options" struct that reads from a Dictionary.
  • Use a type that is only ToVariant as a return value. e.g. Instance<T>.
  • Make ToVariant types into trait objects and send them through Rust channels: Box<dyn ToVariant + Send + 'static>.

All types that previously implement ToVariant are now also FromVariant.

Drawbacks

  • Compatibility break: all previous ToVariant implementations on custom types are void, although a fix is very easy.
  • Compatibility break: Dependent code that use ToVariant as a bound and call from_variant need to do use FromVariant (or ToVariant + FromVariant) instead.

This allows types to be separately `ToVariant` and `FromVariant`, enabling
`ToVariant`-only types to be returned by exported methods, or the other way
around. Also relaxes the `Sized` bound on `ToVariant`, so it can be made into
trait objects, and passed across threads in boxes.
Copy link
Member

@karroffel karroffel left a comment

Choose a reason for hiding this comment

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

That looks good to me, thanks a lot! :)

@karroffel karroffel merged commit 4330a6c into godot-rust:master Aug 29, 2019
@ghost ghost deleted the feature/split-tovariant-fromvariant branch August 29, 2019 11:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant