Getters and Setters are both opt-in and opt-out, when they probably should be opt-out

[sylbeth]

The getters and setters when defining variables are opted-out of when specifying one of them, but then they're opt-in when you specify the second. This creates an error-prone framework when defining properties, as such, based on @Bromeon's comment: #1151 (comment), @ttencate proposed the following API (for getters, but it's symmetrical for setters, now both being independent of one another):

• No #[var]: No getter.
• #[var]: Default getter.
• #[var(get)]: Custom getter with default name.
• #[var(get = name_of_fn)]: Custom getter with custom name.
• #[var(no_get)]: No getter

To this, I want to bring again that if needed to change the default getter's name, the only way to do so is by making a custom one. For that I propose the following two:

• #[var(default_get)]: Default getter.
• #[var(default_get = name_of_fn)]: Default getter with custom name.

These last two are not necessary, but if they are deemed useful they should be easy to implement.

[Bromeon]

Do we need so many variants though?

Maybe we can keep set/get orthogonal, meaning using one in #[var] doesn't affect the other.

What about

#[var]
// Default getter + setter

#[var(get)]
// Custom getter 'get_field', default setter

#[var(get = my_getter)]
// Custom getter 'my_getter', default setter

#[var(no_get)]
// No getter, default setter

Then, the attribute key get always means "custom getter", and we have less syntax overall.

We can also forbid use of #[var(no_get, no_set)].

[sylbeth]

Do we need so many variants though?

Are you talking about the default ones, because the ones you wrote down are the same as the original proposal.

Maybe we can keep set/get orthogonal.

Yeah, that was the proposal, I'm confused.

Then, the attribute key get always means "custom getter", and we have less syntax overall.

So, like the original proposal?

We can also forbid use of #[var(no_get, no_set)].

Should we? It's just a more explicit way of not writing #[var] at all, is there anything wrong with it?

[Bromeon]

Yes, I meant that I don't see the need for default_get... Deviating from the convention is usually a bad idea and rarely needed anyway. Going through extra getter is fine in such cases.

---

We can also forbid use of #[var(no_get, no_set)].

Should we? It's just a more explicit way of not writing #[var] at all, is there anything wrong with it?

If you want to not register a property, then don't use #[var]. There's no reason to do this "more explicitly". Especially since you'd still register a property, it's just not accessible...

Forbidding it can prevent potential user errors rooted in misunderstandings.

[ttencate]

We can also forbid use of #[var(no_get, no_set)].

no_get might not even be fully supported by Godot. I found that a property without a getter can still be read from GDScript and just returns null. So this would need some additional investigation.

That aside, I think @Bromeon's proposal is good, because it's the same as mine, just better phrased :)

[sylbeth]

Deviating from the convention is usually a bad idea and rarely needed anyway.

It is kind of widely used by Godot itself, not all properties are named exactly as their getters/setters, as far as I remember, especially those set through enums.

It would be less painful having a macro to autogenerate it, needing to make the bloat every time is annoying.

Especially since you'd still register a property, it's just not accessible...

Can't we make it not do that instead? We still have to handle that specific case, whether it's panicking or skipping. But yeah, I think you're right, no one would go for both no_s unless it's an error.

no_get might not even be fully supported by Godot.

no_get should be able to be used when no export, as per the documentation on C#. I believe there is utility in supporting it internally, since it makes semantic sense. If the getter is going to be registered anyway, I believe people would prefer an explicit push_error than receiving whatever they got. My idea for it is this:

• If only export registers getter, then, only contemplate the export part of the next section.
• In Engine::singleton().is_editor_hint, return the default for the value or null if it's a node, with either a warning or no warning.
• In any other case, push error and return the default or null.

This could be made not to have any cost by only adding the branch when debug assertions.

[Bromeon]

That aside, I think @Bromeon's proposal is good, because it's the same as mine, just better phrased :)

Fun fact, it's also the same as @sylbeth's (apart from the default_get) 😁 glad we agree on these parts.

---

It is kind of widely used by Godot itself, not all properties are named exactly as their getters/setters, as far as I remember, especially those set through enums.

Godot's naming inconsistencies are one of the biggest pains we binding maintainers have to deal with (just check enums) 🙈

The reason why it's like this is that Godot's codebase has organically grown for over a decade, with dozens of maintainers and thousands of contributors -- and given that, the quality of the overall API is very high. But in many cases, different naming wasn't a deliberate choice.

May I ask if this comes from a genuine use case, or just curiosity? GDScript typically uses property syntax, not setter/getters. Why is the default naming important enough to warrant extra complexity to godot-rust's proc-macro API?

---

It would be less painful having a macro to autogenerate it, needing to make the bloat every time is annoying.

Please see that godot-rust needs to strike a balance between API complexity and feature set. IIRC you're the first one to request this. And there are already loads of little levers in godot-rust's proc-macro API. Requiring everyone to read about default_get/default_set when most users aren't going to need it just adds mental overhead.

What I would prefer instead is if we could somehow enable user customizations. Unfortunately Rust macros do not compose very well, but in an ideal world, users could write their own proc-macros that could be "stacked" with ours. This would allow extensions without everything needing to become core API.

In gdnative, someone did this: https://github.com/wyattjsmith1/gdrust. But our proc-macros nowadays are written very differently from gdnative. You might want to check if something like this is still feasible?

---

Can't we make it not do that instead? We still have to handle that specific case, whether it's panicking or skipping.

Not sure what you mean? We don't need to handle the case if we cause a compile error.

[sylbeth]

glad we agree on these parts.

Yeah, they neat

Godot's naming inconsistencies are one of the biggest pains we binding maintainers have to deal with

Can imagine.

May I ask if this comes from a genuine use case, or just curiosity?

Oh, it does come from moments I went: "Ugh, the getter and setter would fit best with this other name, but I don't want to copy and paste getter bloat". For example, you could have a property that is called inner_sprite and get_/set_sprite. So you're explicit you're editing some inner variable and not a different sprite, functions are much harder to collide with.

Requiring everyone to read about default_get/default_set when most users aren't going to need it just adds mental overhead.

That is fair, I'm the kind that would probably skip reading it until I actually need it.

their own proc-macros that could be "stacked" with ours.

don't declarative macros like paste achieve it though? One could always do trait PersonalizedGodotClass: GodotClass and extend the derive.

You might want to check if something like this is still feasible?

Will note is down to read sometime.

Not sure what you mean? We don't need to handle the case if we cause a compile error.

I meant that checking that case + error is already handling the case, at least in my definition of handling.