godotengine / godot-docs

Godot Engine official documentation
https://docs.godotengine.org
Other
3.89k stars 3.18k forks source link

The style guide should explicitly give guidelines for setter getter #5651

Open ajreckof opened 2 years ago

ajreckof commented 2 years ago

Godot version 3.4

There is no explicit guidelines on the seteget variables in the style guide page of the documentation. In particular it would be good to explicitly say where setters and getters should be place (either nearest to the variable or after the built-in functions as every other function does). maybe having naming guidelines for setters and getters would be good i don't know

https://docs.godotengine.org/en/stable/tutorials/scripting/gdscript/gdscript_styleguide.html

Calinou commented 2 years ago

@NathanLovato Do you have any opinions on where setter/getter method definitions should be in the script?

NathanLovato commented 2 years ago

They're regular public or private methods in Godot 3. That's the guideline we collectively decided to stick to a couple of years back. If your setter is public, it goes with the public methods, if it's pseudo private, with the pseudo private methods.

That's why there's no extra case for them or signal callbacks in the style guide: they're no exception.

If you'd like to be explicit about the fact setters and getters should be treated like other methods, please go ahead, there's no harm in that.

ajreckof commented 2 years ago

Thanks that was what i thought but i was unsure after a discussion on reddit as a lot of person put them in various places. I will modify it to make it explicit

NathanLovato commented 2 years ago

People out there don't necessarily follow the style guide, they may not remember it or do some bits differently. We have a teammate working on the gdscript code formatter for Godot 4, then we could consider a tool that automatically reorders your code to not have to worry about learning the guidelines by heart.

Proggle commented 2 years ago

People out there don't necessarily follow the style guide, they may not remember it or do some bits differently. We have a teammate working on the gdscript code formatter for Godot 4, then we could consider a tool that automatically reorders your code to not have to worry about learning the guidelines by heart.

Definitely needs to spell out practices for Godot 4, since in the current alpha, writing a normal external setter function that would work in Godot 3 will instead cause infinite recursion ( https://github.com/godotengine/godot/issues/48437 )

cbscribe commented 1 year ago

The style guide hasn't yet been updated for 4.x syntax, and now we have two separate options for declaring setters/getters. We need to reach a consensus on which is the preferred style:

var my_prop:
    get:
        return my_prop
    set(value):
        my_prop = value
var my_prop:
    get = get_my_prop, set = set_my_prop

I lean toward the latter, as the former can get messy when you have several variables with setters at the top of the script. The argument for the first option is that having everything in one place makes it easier to reason about how that particular property works.

ajreckof commented 1 year ago

I think there is more nuance to it and we can't just say that one is better than the other. I think each one as his advantages and that is what we should show in the documentation. we should the common use cases for each of them and clarify what are the pros and cons. But independently of which is preferred, I think we should add guideline as to where to put them (if it is not already done). I was think putting them after all variables and before functions.

PS : Here are one examples for each where one is better (in my opinion) than the other

In this one it makes you write more line and the code is less readable as they might be far appart

var my_prop:
    set(value):
        do something one line
        my_prop = value

var my_prop:
    set = my_set

func my_set(value):
    do something one line
    my_prop = value

And here is one where keeping it near the variable will only make it messy

var my_prop:
    set(value):
        something
        very
        very
        very
        very
        very
        long
        that 
        makes
        it 
        really 
        messy
        whenever
        there 
        are
        multiple
        variables
        my_prop = value

var my_prop:
    set = my_set

func my_set(value):
    something
    very
    very
    very
    very
    very
    long
    that 
    makes
    it 
    really 
    messy
    whenever
    there 
    are
    multiple
    variables
    my_prop = value