#[derive(GodotClass)] on tuple structs should fail before other macro failures

[fpdotmonkey]

Consider this snippet.

#[derive(GodotClass)]
#[class(rename = "Not an identifier")]
struct MyTupleStruct(String);

This is incorrect for two reasons. First, the rename key demands an identifier, but it's been given a string. Second, you cannot derive GodotClass on a tuple struct. Upon compilation, this yields only one error.

error: expected identifier
  --> itest\rust\src\object_tests\class_rename_test.rs:27:18
   |
27 | #[class(rename = "Not an identifier")]
   |                  ^^^^^^^^^^^^^^^^^^^

Upon fixing this and recompiling, you see a new error.

error: #[derive(GodotClass)] not supported for tuple structs
  --> itest\rust\src\object_tests\class_rename_test.rs:29:21
   |
29 | struct MyTupleStruct(String);
   |                     ^^^^^^^^

Fixing this issue requires some degree of redesign and would likely invalidate the work done to solve the first error. As such, this latter error should be encountered first before any other errors in related macros.

[PgBiel]

Maybe support for tuple structs could just be added instead, but without any support for #[var], #[export] etc - you'd just have an opaque object exported to Godot (which could be useful in certain scenarios).

Alternatively, support for #[var], #[export] etc could be eventually added using rename to give the fields proper names in the Godot side. But that could certainly be postponed in favor of just having basic support, in principle.

[Bromeon]

I'm not sure if tuple structs are useful in this context:

• you cannot have any exported properties
• you cannot have a #[base] object (unless you keep it unnamed, which is clumsy)
• the "opaque object" use case is trivially achievable with an empty named struct
• once you want to export fields, you need to refactor the whole code -- something that isn't necessary when starting with a named struct

I agree that the error message for tuple structs should be improved though. Thanks for reporting! 🙂

[PgBiel]

Thanks for the input. Personally, I think that those concerns are related to tuple structs themselves, such that it's a choice to be made by the end user - when they're using a tuple struct, they know that they'd have to refactor later if they wanted to actually not make it a tuple struct. What's more, we can actually make tuple structs easier to work with through our existing macros. Let me explain.

For one, I believe tuple structs are especially useful for the newtype pattern - you really don't want something named, you just want to wrap something (e.g. struct Meters(f32), struct CoolNode(Gd<Node>), ...), perhaps to implement a trait or something else. And this something doesn't have to be exported - it can be a Rust-only field, but sometimes you need to pass it around to Godot. So I just think that, by forbidding tuple structs, we are making a bit of an arbitrary choice for the user here - oftentimes #[derive(GodotClass)] is used only to be able to pass around a Gd<YourType>, and, if we can't do this for tuple structs, then we'd actually be forcing the user to refactor an existing tuple struct (in order to be able to use it within Godot) when it's not actually needed. That's not to mention the benefits of being able to expose methods and whatnot for your type to Godot.

(I actually saw a use case for this in the gdext Discord: someone worked around the lack of support for tuple structs by using fields such as _0 instead... :P)

Another relevant argument is that we wouldn't be able to distinguish between tuple and non-tuple structs in a future Builder API, so support for tuple structs would be added anyway.

Regarding the specific points:

• you cannot have any exported properties

You actually can through renaming, though, of course, it's better to use a named struct instead if you're going to have multiple exported properties and stuff. Either way, though, I believe it's valid in a newtype scenario where you just want to export the single field you have.

• you cannot have a #[base] object (unless you keep it unnamed, which is clumsy)

It's indeed a bit clumsy at the moment, but with #501 we'd just use .base() and .base_mut() anyway, so I believe it'd be fine that way.
And I believe this sort of reinforces the newtype idea: you'd be able to create a "newtype wrapper" for a Godot class by just doing struct Wrapper(#[base] Base<Class>). This seems like a way to make working with Godot "Rustier" in general.

• the "opaque object" use case is trivially achievable with an empty named struct

Sorry, I should have clarified: I meant an "opaque object" only for Godot - but we'd still have Rust-only (non-exported) fields we can mess with in the Rust side.

• once you want to export fields, you need to refactor the whole code -- something that isn't necessary when starting with a named struct

I believe that applies to tuple structs in general (not just in a Godot context), so that's generally a conscious choice (or, at least, it feels weird that we'd be imposing such a choice anyway). And changing a named struct's fields would also require significant refactor depending on the size of your code base anyway.

Though using renaming would alleviate this for the specific use case where you have only one field and that's what you want to export (you want to keep it "unnamed" in the Rust side though).

[Bromeon]

For one, I believe tuple structs are especially useful for the newtype pattern - you really don't want something named, you just want to wrap something (e.g. struct Meters(f32), struct CoolNode(Gd<Node>), ...), perhaps to implement a trait or something else. And this something doesn't have to be exported - it can be a Rust-only field, but sometimes you need to pass it around to Godot.

Fair point. But maybe we should then consider the newtype pattern in particular -- often you might want to expose the inner thing more or less transparently, maybe even saving some boilerplate code in the process. We already do such a distinction for #[derive(ToGodot, FromGodot)]:

gdext/itest/rust/src/register_tests/derive_variant_test.rs

Lines 62 to 66 in 5e18af8

	#[itest]
	fn new_type_struct() {
	roundtrip(StructNewType(String::from("five")));
	roundtrip(dict! { "StructNewType" : "five" })
	}

This doesn't extend naturally to all sorts of tuple structs. Especially if we add specific behavior for newtypes (i.e. tuple structs with a single field), it may be even more confusing if this behavior is different for those with multiple fields.

---

Another relevant argument is that we wouldn't be able to distinguish between tuple and non-tuple structs in a future Builder API, so support for tuple structs would be added anyway.

I don't think this is relevant -- the macro API will be a convenience layer on top of the programmatic API. As such it can be less powerful than the underlying one. But since it is essentially a DSL, it's good to have certain conventions, also for recognizability.

In other words, I think it's OK if #[derive] macros are opinionated. This already applies to ToGodot and FromGodot too, and my argument is the same there: instead of providing endless customizability à la serde, we should provide an easy way to implement the underlying trait directly. That is, our job is to implement a good builder API 🙂

---

TLDR: it would be good to learn more about the newtype use case. Do you know any ways how the current #[derive(GodotClass)] can make those nicer to use, beyond just allowing the syntax?

[lilizoey]

If this hasn't been fixed yet then just fixing the right error message to trigger first should be a good first issue. whether we should support tuple-structs themselves can be resolved in another issue.