Standardize names for boolean properties, getters and setters

[madmiraal]

Describe the project you are working on

Godot engine

Describe the problem or limitation you are having in your project

Godot properties have underlying methods that can also be used to retrieve and change the property value called the getter and setter. Assuming a property is well named i.e. appropriately describes the value it holds, the property_name has a standard getter and setter: get_property_name() and set_property_name().

However, with bool (true or false) properties this structure is awkward and complicated by the standard for retrieving bool values using is_* or has_*, etc. rather than get_*. For example, for a property named enabled, the getter should be is_enabled() not get_enabled(). The problem is, what should the setter be called: the natural enable() or the more awkward set_enabled()? This is further complicated by methods that have natural opposites eg. pause() and resume() to control the property paused.

Furthermore, although it's already been agreed (https://github.com/godotengine/godot/issues/16863) that bool properties should be named positively to avoid double negatives, there are a number of bool properties that determine what the Object will do when it's created e.g. loop. This creates an awkward getter: is_loop(). Instead it would be better if the property were named for what it will be doing when it's created: looping, which has the better getter: is_looping().

However, there is a further complication. The property may be read-only once it is created i.e. there shouldn't be a setter. In other words, there is sometimes a need for two properties: one for controlling what the object will be doing and one for what it is doing. For example, start has a natural property called started that is read-only i.e. it only has a getter: is_started(). The question is, what should the property to determine whether or not the Object should automatically start when created be called: start, auto_start, auto_started or, as is often done, something completely different e.g. playing?

Describe the feature / enhancement and how it helps to overcome the problem or limitation

1. Ensure bool properties have names that describe what the Object will be or is doing e.g. use looping and not loop.
2. Ensure bool properties don't use negative names (which can be hard to spot) e.g. use looping and not not one_shot. (Yes, the double negative is deliberate.)
3. Use the standard set_*() and get_*() for the properties in addition to is_*() which maps to get_*().
4. If the property needs two values: one to describe what it will be doing when it's created and one to describe what it is doing, but is read only, use auto_* with an action verb for the former and the gerund i.e. *ing for the latter e.g. auto_play and playing respectively. Note: since playing is read-only it will not be exposed as a property, but only have the method is_playing().
5. If the bool property has a natural setter and "unsetter", they can be created as helper functions that map to set_*(true) and set_*(false). For example enabled should have enable() mapped to set_enabled(true) and and disable() mapped to set_enabled(false), paused should have pause mapped to set_paused(true) and resume() mapped to set_paused(false).

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

Described above.

If this enhancement will not be used often, can it be worked around with a few lines of script?

N/A

Is there a reason why this should be core and not an add-on in the asset library?

N/A

[pouleyKetchoupp]

See the original discussion on the main repository: https://github.com/godotengine/godot/issues/33026

And here's my feedback:

1. Yes
2. Yes
3. I would rather make a choice and have just one standard getter, to avoid having several functions doing the exact same thing. It still probably needs some degree of variation, so it could be: Getter: is_property_enabled() (or is_property() when it makes more sense, e.g. paused or enabled) Setter: set_property_enabled() (or set_property() when it makes more sense)
4. Yes
5. This should be really considered optional and specific to some cases. I wouldn't say any property should have this kind of setter on top of the standard one. Even the cases with enabled and paused are questionable, the standard setters seems to work fine to me.

[me2beats]

I just name getters/setters like getter_a, setter_a

[akien-mga]

I agree with most points in the proposal and with @pouleyKetchoupp's feedback above. In particular, adding more ways to do the same thing with slightly different semantic doesn't strike me as a good idea (e.g. duplicating get_ and is_ getters in 3.).

Similarly for 5., I'd also keep it on a case by case basis for the handful of places where it makes sense. So I wouldn't make it a rule, just a note that, where relevant, this can be done after discussion.

For 1., I'm a bit concerned about the scope of changes that this would imply. The loop/looping example makes sense, but there are tons of properties which are named with imperative forms such as CanvasItem's clip_children, show_behind_parent, show_on_top. IIUC this rule properly they'd have to become clipping_children, showing_behind_parent, showing_on_top, and I'm not sure this is better. I do agree about using the gerund form for the getter though: is_clipping_children(), is_showing_behind_parent, is_showing_on_top, which informs the user about the current state of the node which was defined when the property was toggled.

---

To reach a consensus on what the APIs should be like, I think we should take a few concrete examples from the existing API, and see what changes would be implied by the proposed name changes to make things more consistent. Then we can easily see if we like those new APIs, and thus validate the rules.