Derive From Variant To Variant

[astrale-sharp]

Closes #245

[astrale-sharp]

I have very little interest for deriving something other than a struct or enum, I will not have time to work on it.

The only opinionated bit of this PR (imo)

is that

struct Struct(i32) ;

Struct(4).to_variant() == dict!{"Struct" : 4}
// instead of 
Struct(4).to_variant() == dict!{"Struct" : array![4]}

// and same for 
enum Enum {
        // this variant
        Variant(i32)
        // but not this one
        Variant2(i32,i32)
}

[Bromeon]

Thanks a lot for this! 👍

This only seems to implement ToVariant. Given the PR title, do you plan to also add FromVariant (could also be separate)?

Please avoid prelude for internal implementation and use the original modules, e.g. godot::builtin.

The Godot representation of tuple-structs/enums with only 1 element can be discussed. While a bit inconsistent with multiple elements, those are often specifically used for "newtypes", as such it can make sense to avoid the array. It means of course it's not easy to version them, but maybe we should recommend using named structs/enums if that is a concern.

CI is currently failing, would be nice if you could address those 🙂

[astrale-sharp]

Thank you very much for your comments! This is still a draft! I will take them into account! (Note that support and testing for FromVariant and generic structs is almost done)

[Bromeon]

You haven't addressed my comments with quote syntax correctly. Also, could you fix CI (make sure every file has a license header)?

For both issues, have a look at how existing code does it 😉

[astrale-sharp]

I wasn't totally sure what you meant about quote!, if the change still doesn't correspond to what you want, please provide an example or a more thorough explanation ! (but honestly i think I got it now)

[astrale-sharp]

forgot godot::builtin::
sorry about that
Edit :: removed the import of the prelude in test to make it very apparent when I forgot godot::builtin somewhere

[Bromeon]

Thanks a lot, this is really great work! ❤️

Added some inline comments, mostly stylistic ones.

You seem to use a lot of mut when modifying token streams, and slowly build them via appending to a previous quote! expression. Maybe as a hint, you can use interpolations on quote! to repeat patterns, without needing a for loop and procedural element pushing/popping. This also works on iterators, so you can use #( #expr )* if expr is the result of iter().map() for example; you don't need an actual collection backing it.

Interpolations are a nice pattern similar to declarative macros, and having less mut variables in the code can make things a bit easier to understand.

[astrale-sharp]

Thank you very much for your comments/review/positivity, it's been a pleasure to contribute!

Time for another review (with one question pending) we should be close!

[Bromeon]

Thanks a lot for the update!

A few comments left (mostly formatting-related), then should be good. A few suggestions from previous review you overlooked, I marked them again so they're not lost in this huge changeset 🙂

---

A side note, the struct MyClass { i: 33, b: true } can theoretically map to two dictionaries:

• { "MyClass": { i: 77, b: true } } -- as done here
• { i: 77, b: true }

And both have their use cases. It's definitely great to already have an initial implementation, so no need to change anything now! This could always be done later, or maybe even customizable.

[GodotRust]

API docs are being generated and will be shortly available at: https://godot-rust.github.io/docs/gdext/pr-246

[astrale-sharp]

Hi! I'm not sick anymore! ;)
Hopefully I didn't forget anything this time 👼

[Bromeon]

Great to hear! And thanks for the update, it's mostly good now.
You missed a few smaller comments, I wrote them down again 🙂

Could you squash all comments to a single one?

[astrale-sharp]

Thanks for the quick review! Good to go?

[astrale-sharp]

It looked good to go but more testing showed that it was relying on types being copy, I modified the test with !Copy types and modified the code_gen to use clone instead.

[Bromeon]

bors try

[Bromeon]

bors p=9
bors r+

[bors]

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

• full-ci

[Bromeon]

Thanks a lot for this great addition! 🚀