Rewrite the ToGodot and FromGodot derive macros

[lilizoey]

The current ToGodot and FromGodot derive macros are as of #421 just a hacky adaptation of the previous ToVariant and FromVariant derive macros.

We should rewrite this and possibly merge it with the Property derive macro to create conversions that use more than just Variant as the backing type.

We might want to wait with this until #451 is done.

[lilizoey]

Current behavior:

Every type has Via = Variant.

• struct Name; becomes {"Name": null}
• struct Name(Type); becomes {"Name": Type}
• struct Name(Type1, Type2,..); becomes {"Name": [Type1, Type2, ..]}
• struct Name { field_1: Ty1, field_2: Ty2, .. } becomes {"Name": {"field_1": Ty1, "field_2": Ty2, ..}}
• enum Name { Variant1, ..} becomes {"Name": {"Variant1": .. }} where each variant id translated like above

Some notes.

We could use a #[godot_repr(..)] to let the user decide how their type should be translated. keeping in mind we'll likely use this for Property too so it'd be useful to have a way of representing various Property metadata too.

The default would likely be a dictionary, like we do currently.

If the type also derives GodotClass, then theTo/FromGodot impls should convert to/from the Gd version of that type instead.

Some ideas for reprs, names up for bikeshedding

• dictionary, for structs, become a dictionary as described above.
• array, for structs, become an array with each field in order.
• enum(int), become a c-style enum represented by an int
• enum(GodotString) become a c-style enum represented by a string
• bitflag(value ..) become a bitset
• transparent, for structs, convert to a type directly. Useful for newtype structs.

[Bromeon]

Maybe a general design question, how much configurability should the #[derive(ToGodot, FromGodot)] proc-macros really have? I want to avoid rebuilding an entire serde here, as this is just a small (although important) part of gdext's functionality. It's always possible for the user to implement those traits manually -- so we should maybe directly place the hand-written and #[derive] code side-by-side, to see whether it's worth to write macros for it.

Some features like choosing int/string for enums seem to be quite common, but others like representing a struct with named fields as an array are very niche. I would stay we start with opinionated defaults (no configurability beyond enum) and then add extra features one at a time, individually motivated by use cases.

• struct Name; becomes {"Name": null}
• struct Name(Type); becomes {"Name": Type}
• struct Name(Type1, Type2,..); becomes {"Name": [Type1, Type2, ..]}
• struct Name { field_1: Ty1, field_2: Ty2, .. } becomes {"Name": {"field_1": Ty1, "field_2": Ty2, ..}}
• enum Name { Variant1, ..} becomes {"Name": {"Variant1": .. }} where each variant id translated like above

I think those are a good start, but we might need more data to see whether these are good defaults (e.g. whether the type name is often required).

[vortexofdoom]

I think implementation could guide usage, there's a pretty solid case imo for not including the name by default. For instance if you were to nest structs to create a nested dictionary, you'd probably want the key names (ie. struct fields) to correspond directly to the underlying dict, instead of a dict containing it's name and fields.

If the name is included by default, you'd have to opt out of that either at the field level (which I would deem unacceptable) or at the struct level, unless we want to dynamically determine whether any given instance is inside another struct. To me, passing out a dict with one member listed by name seems like a less common pattern than using them in a way that actually emulates a struct.

Enums and tuple structs are where I'd expect to see names used on occasion, but even then, I think common usage would be using them as variables and using the enum variants or inner field of a tuple struct directly.

In fact, we should consider the usecase for ToGodot and FromGodot when there's a derive macro for GodotConvert already. How often would users want ToGodot and FromGodot without GodotConvert? Users should maybe just implement the traits themselves if the default for GodotConvert is insufficient.

[Bromeon]

Fully agree, I think including the name by default is the wrong choice.

Ideally we approach a new design in a pragmatic way:

1. We add the minimal feature set that's needed to make the traits useful at all.
   • Meaning precisely 1 mapping per type, no configurability.
   • We could set up tests beforehand, which act as a spec, so we don't need to deal with design doc paperwork but still communicate the intent.
2. We discuss where configurability makes sense..
   • In particular, we decide what is out of scope. As mentioned multiple times, I don't want to build a serde clone, so let's keep things simple.
   • We can however discuss if integration with serde (under the serde feature) can be done.
3. We adapt only based on user feedback, backed by actual use cases.
   • No "please add this because it would be nice" 😉

What makes these derive macros painful is to get generics right with all the syntax like where clauses etc. I would suggest we do not rewrite this by throwing away all the code, since quite a few of these problems have been solved already. Let's instead try to scope the implementation down and then re-add some flexibility.

[Bromeon]

In fact, we should consider the usecase for ToGodot and FromGodot when there's a derive macro for GodotConvert already. How often would users want ToGodot and FromGodot without GodotConvert? Users should maybe just implement the traits themselves if the default for GodotConvert is insufficient.

Maybe #[derive(GodotConvert)] could auto-implement ToGodot/FromGodot (with an opt-out).

It may not be the most idiomatic (what a surprise in godot-rust 😁) -- but it's likely what 95% of users need, and it's similar to what we already do with GodotClass. Specifying 3 traits all the time primarily adds noise.

[vortexofdoom]

LOL I was actually typing exactly the same sentiments (concerns about idiomatic rust and the futility of worrying too much about idiomatic rust in godot-rust included).

I'm wondering is what's the use case for FromGodot without ToGodot or vice versa, or the use case for them without GodotConvert? Hypothetically it weakens the type system for something that you may want to get but not send back under any circumstances, or vice versa, but what would that look like in practice?

The opt out would handle the case where you have a more optimized/customized implementation of one of the traits pretty nicely, and I think that the first error the engine gives you is about GodotConvert anyway, then it yells at you about To/FromGodot, so that fits at least one process of figuring out the engine.

[vortexofdoom]

If GodotConvert was the top level macro for conversions, how would people feel about a #[prop(get[="name"], set[="name"]] annotation style for properties? Or maybe #[get[="name"]] and #[set[="name"]] as separate annotations? First feels more rusty, second feels more gd-scripty to me. Almost feels like it should be part of the GodotClass impl but there's probably something I'm not understanding.

[lilizoey]

Honestly my most common use-case for manually implementing these is for making enums that i wanna use with godot, and have them either become strings or ints.

Almost feels like it should be part of the GodotClass impl but there's probably something I'm not understanding.

Im not sure how this should work. You should not implement both GodotConvert and GodotClass for the same struct, and if you do then GodotConver should probably just use Gd<Self> as the via type at which point you dont need any fancy inference since GodotClass handles all the fields and such for you.

GodotConvert types can't really have "properties" in that sense since they just become a godot type on the other end. They dont become a class like GodotClass types. In fact if your struct could be a GodotClass type then you should just do that instead of GodotConvert. GodotConvert is useful when you have something that already basically just works like one of the non-object godot types. like an enum that's just an integer, or some custom vector2 type that you just wanna translate to godot's vector2 type.

FromGodot is useful when you want something to be usable as a function argument, or just to easily translate stuff from godot.

ToGodot is useful for return types or to easily translate to godot.

most of the time you want both. but sometimes you can only really have one. like &str can implement ToGodot but not FromGodot because you can't put the lifetime bound in there.

[vortexofdoom]

I won't pretend I realized all of that, but I did realize that the traits were somewhat orthogonal, which is why I crossed it out. &str is a solid example I didn't think about.

It might be possible to error on anything that contained a lifetime or reference when trying to derive both, but I still think the auto-deriving idea has legs.

The string-int case for enums is honestly probably useful and common enough to be a feature straight away.

[lilizoey]

So initially i think we can support these cases:

1. c-style enum with #[repr(i*/u*)] becomes an integer of the given type
2. c-style enum without #[repr(..)] becomes a GString
3. single-field struct with #[repr(transparent)] becomes the type of its single field

that should cover at least the main use-cases that i think have been brought up (at least the ones i face mostly). most other cases for converting between rust types and godot types are likely better covered by GodotClass so it may be better for us to just not provide derive macros for more complicated cases to encourage people to just use GodotClass.

[Bromeon]

Sounds good!

However, if we're changing this, we should probably move away from mixing Rust's #[repr] syntax -- which is used to influence memory layout -- with ToGodot/FromGodot (de-)serialization behavior.

We will anyway need customization beyond what's supported in #[repr] and thus require a differently named attribute. And memory layout and serialization are orthogonal, there may be cases where users wouldn't want to mix them.

I'm not sure what we could use; gdnative used #[variant]. Something like #[godot] might also be an option.

[lilizoey]

i dont think #[variant] would be the right name tbh, since some implementations may not even touch Variant at all
#[godot] probably works, so maybe:

#[godot(transparent)] // expect newtype-style struct and use the newtype field type as the `Via` type
#[godot(via = type)] // use `type` as the via type

initially we will only support:

1. transparent for structs with exactly one field
2. via = GString for c-style enums, here explicit discriminants are ignored
3. via = i8/i16/i32/i64/u8/u16/u32 for c-style enums, here we will use explicit discriminants, but infer the rest of the discriminants. but we'll only accept literals for now since trying to handle more complicated expressions there would be difficult

We can also require explicit discriminants in the last case, or that there's either only inferred or only explicit discriminants. I think doing some simple inference here is fine, it's not very complicated and is more convenient.

So some examples:

// -----------------------
// newtype structs

#[derive(GodotConvert, ToGodot, FromGodot)]
#[godot(transparent)]
// Via = Vector2
struct MyVector2(Vector2); 

#[derive(GodotConvert, ToGodot, FromGodot)]
#[godot(transparent)]
// Via = Dictionary
struct DictionaryWrapper {
  dictionary: Dictionary,
}

// this would error:
#[derive(GodotConvert, ToGodot, FromGodot)]
#[godot(transparent)]
struct DictionaryWrapper {
  dictionary: Dictionary,
  extra_info: SomeData,
}

// -----------------------
// stringy enums

#[derive(GodotConvert, ToGodot, FromGodot)]
#[godot(via = GString)]
pub enum MyEnum {
  A = 0, // becomes "A"
  B = 1 + 1, // becomes "B"
  SomeVariant, // becomes "SomeVariant"
  // would error:
  // C(u64),
  // C { value: u64 },
}

// -----------------------
// inty enums

#[derive(GodotConvert, ToGodot, FromGodot)]
#[godot(via = i32)]
pub enum MyEnum {
  A = 3,
  B, // inferred to be 1, since 0 is taken
  C, // inferred to be 2, since 0 and 1 are taken
  D = 0,
  E, // inferred to be 4, since 0, 1, and 2, are now taken
  // not allowed:
  // F = (10 * 2), 
  // F(u64),
  // F { value: u64 }
}

Future possibilities

• renaming of stringy enum variants (automatic or manual)
• allow extra zero sized fields in transparent newtype struct
• find a mapping for option and result-like enums (this should mirror whatever we end up doing with option and result)

[Bromeon]

That looks great! I like the #[godot(via = ...)] attribute 🙂

1. For enums, would the absence of #[godot(via = ...)] cause an error or be equivalent to #[godot(via = GString)]?

2. Do you think we should consider StringName for enumerators, as they are immutable and could be interned?

3. Also, I would consider #[derive(GodotConvert)] being enough in this case, as stated above. This could however be implemented separately/later. Possible syntax if I only want one direction:

   #[derive(GodotConvert)]
   #[godot(to, via = i64)] // or 'from'