Godot-docs: Creating a more complete GDScript Style Guide

[Scony]

order of class definitions should be agreed and mentioned in the proper docs section.

It may be either fine-grained like e.g.:

- tools
- extends
- classnames
- enums
- consts
- signals
- exports
- onreadypubvars
- pubvars
- onreadyprvvars
- prvvars
- others

or coarse-grained like e.g.:

headers (tool, extend, classname)
definitions (enums, exports, const etc.)
publics
privates

but debate is needed

[winston-yallow]

I am in favor of the coarse grained order. It would allow code like this:

enum COLOR {BLUE, RED, GREEN}
export(COLOR) var outline_color

enum SHAPE {TRIANGLE, SQUARE}
export(SHAPE) var debug_shape

I think when an enum is used only for one (not multiple) exports it helps to have it in the line above the export. I feel like it is much more readable to group them by usecase rather than by type.

(just adding my point I made on discord here for reference)

[TheDuriel]

https://gist.github.com/TheDuriel/e34278a84ea163858d2d5e578b666dbc

[NathanLovato]

We've worked on a GDScript style-guide based on the GDScript styleguide from the docs this year with the team. It puts readability and accessibility first, and the information users of a given class need first.
We've been using it and improving throughout the year.

Below is the quick outline. Here is the complete style guide. In short, it puts properties at the top, public before private, node dependencies before variables, and initialization functions and callbacks before other built-in callbacks:

01. extends and class_name
02. """docstring"""

03. signals
04. `onready` variable (node dependencies)
05. enums
06. constants
07. exported variables
08. public variables
09. private variables

10. optional built-in virtual `_init` method
11. built-in virtual `_ready` method
12. signal callbacks
13. remaining built-in virtual methods
14. public methods
15. private methods
16. static functions

We'd like to suggest it as:

1. It's designed to make the code easy to read and understand what's happening, so it works well for teaching.
2. It's the result of teamwork. We've discussed it a lot, we use it with students, got feedback from professional developers as well. We've used it in our recent beginner platformer Godot course and it doesn't seem to cause issues even for programming beginners.
3. It's designed for teamwork but works well for individual work as well.

We discussed each decision thoroughly, so @razcore-art and I are here to clear any doubts or explain every single detail you might have questions about.

[NathanLovato]

In general, I'd be in favor of having polished and detailed docs for an official style guide so we can also have a precise code formatter, higher quality standards for code examples in the docs and in official demos, and help save everyone time.

I don't necessarily want our style to be the standard, I'm open to any style that people put a lot of thought into. Regardless, we're there to help write the actual documents anytime.

[winston-yallow]

The biggest problem I have with your styleguide is with the onready variables. I prefer to put them to the bottom directly before the first method. My reasons are:

1. It allows me to use other variables (for example exports) to initialise the onready vars.
2. It groups them visually closer together with _init() and _ready(). This way it is easy to see what happens on ready in one place.
3. I see onready vars mostly as private. This way they would be grouped together with the other private vars.

What are the reasons why you propose them to follow the signals? I currently do not see many reasons for this so I would be interested in your thoughts.

[winston-yallow]

Another point:
As enums can be used as export hints I would prefer them to be directly above the exports instead of below.

[NathanLovato]

Our styleguide is a living document, it's open for improvements! The enum makes sense, I'll update that, thanks.

What are the reasons why you propose them to follow the signals? I currently do not see many reasons for this so I would be interested in your thoughts.

The thing we called "onready variables" in our code are node dependencies that we cache upfront for the most part:

onready var cooldown: Timer = $Cooldown
onready var flash: Particles = $FlashEffect

Things like these. These are nodes the class depends on to function properly, so we put them at the top. A bit like in other languages, you would have imports from other modules or files.

Onready being a keyword that has several uses, I think we should rename that point to cached node dependencies.

[NathanLovato]

When it comes to private and public, you can use onready for both. It depends on how you want to design your APIs. We give public access to some nodes and values that need to be initialized in our code. It depends a lot on the context, but we expose properties in general over using methods to encapsulate data.

I edited the order in the list above, and fixed the style guide

[razcore-art]

The biggest problem I have with your styleguide is with the onready variables. I prefer to put them to the bottom directly before the first method. My reasons are:

1. It allows me to use other variables (for example exports) to initialise the onready vars.

As Nathan points out, we mostly use onready vars with setting up node dependencies, although not always. My recommendation (for our team) is to initialize any private vars inside the _ready() function instead of using onready for this so that we don't break onready var grouping which visually makes a bit of a mess, in my opinion. onready being used mostly for setting up these node dependencies, we decided to only use them with "public" vars since you can always do child_node.get_node("Timer") for example, which could just be accessed via child_node.timer if onready var timer: Timer = $Timer is there. The reason they're at the very top is to give a quick first-glance of the node dependencies when opening a script, but I see your point about having them above _ready() to make it easy to see what's happening. More on this below.

I'm not sure how many people know this but, this:

extends Node2D


onready var a: int = b

export var b: = 7

func _ready() -> void:
    print(a, " ", b)

"just" works, because onready is really syntax sugar for running the initialization in _ready() so you can place it before/after any other declarations, doesn't matter if it's before or after export.

1. It groups them visually closer together with _init() and _ready(). This way it is easy to see what happens on ready in one place.

I can see this being a good argument. At the moment I'm not really convinced one way or the other: first thing after signals - for quick glance at node dependencies VS. right above _ready() to group them visually closer. I can see the merit in both of these arguments.

1. I see onready vars mostly as private. This way they would be grouped together with the other private vars.

Yeah, I guess I don't see them this way as explained in the first part. I see them mostly as glue-code, the rest goes in _ready(). I avoid placing too much functionality inside the onready var declaration & initialization, which you'd need if referencing some functions or other private vars form the code.

What are the reasons why you propose them to follow the signals? I currently do not see many reasons for this so I would be interested in your thoughts.

Becuase we use signals mostly through code (as opposed to declaring them via UI), it's our experience that, when designing signal connections, we need to have quick visual access to them since the connections themselves don't appear anywhere in the Node panel. The Node panel is by default grouped with the Inspector panel, hidden behind a tab and we all know that people rarely change defaults. Being hidden by default means some extra work to change between Inspector <-> Node panels to visually tell the signals and the connections so it made sense for us to have them declared at the very top.

Another point:
As enums can be used as export hints I would prefer them to be directly above the exports instead of below.

Yeah, that's a good point, I'm pretty sure I noticed this at some point, but I forgot to update the docs.

[NathanLovato]

Agreed with Razvan, and I agree with

It groups them visually closer together with _init() and _ready(). This way it is easy to see what happens on ready in one place.

I can see this being a good argument.

[winston-yallow]

Thank you for your detailed answers!

I did not know you could use all variables in the onready variables, even if defined below. It makes sense from a technical side. However I think it can be confusing for reading the code. The brain would be confronted with a variable that it does not know about (yet). Maybe I am just too used to other languages than gdscript as I prefer to only use variables that are already known to me (when reading it from top to bottom).

Apart from that I really like the idea of seeing onready vars like node dependencies and therefore putting them on top. I think I can get used to writing only simple node dependencies as onready and doing other var initializations in _ready(). I can see how this leads to better understandable code. For a styleguide this sounds good to me :+1:

[Feniks-Gaming]

I have been using @NathanLovato guide for a while now but exactly as @winston-yallow says I modified it to have onready var as last just before _ready() function I feel apart of that one change it works really well creates consistent looking code and will enhance cooperation on other projects as more people adapt to one style.

What we really need is PEP-8 of Godot as many of our developers come from many different backgrounds and there is general lack of consistency even across tutorial scene for new users furthering the divide and confusion

[aaronfranke]

@NathanLovato: The thing we called "onready variables" in our code are node dependencies that we cache upfront for the most part:

It would be bad to put anything in the style guide that conflict with the official demo projects. For example, they use $Whatever everywhere with no caching. If caching node references is the preferred style, it should be used in the official demo projects first.

[razcore-art]

That's not how real life works. The demo projects aren't, by definition, the source of truth. We're actually in the process of updating the demos because they have such poor quality code in them. See this for example: https://github.com/GDQuest/godot-kickstarter-2019/tree/master/platformer-2d-rework. We already discussed this with Remi & Juan.

The truth is that the demos are just bad examples of code and apart form demonstrating Godot features they are in need of some better programming. And better explanations since 99% of the code isn't commented in any way.

[Feniks-Gaming]

I agree issue here isn't that Demo projects conflict with style but that Demo projects don't have any style at all. Style should be agreed and then added to all demo projects using random $NodeName out of a blue inside a function is just teaching people bad habits.

[NathanLovato]

@aaronfranke I second what @razcore-art and @Feniks-Gaming say. The official demos don't have a unified style and teach poor programming practices for the most part. They were written by different persons over the years, including back when there weren't any guidelines or peer review.

More importantly, remaking all the demos and all examples in the official docs to fit a new code style is a job for multiple developers. We couldn't expect contributors to follow the same style doing so without a reference document and before having agreed upon a style.

Also, you need peer review on the guidelines first so you don't have people discussing the smallest decision in every PR because they personally prefer another syntax.

[aaronfranke]

My main concern is with something becoming a "standard" that nobody follows or maybe doesn't know about because it's not used anywhere in practice, or create a "do as I say, not as I do" situation.

Overall I disagree with it not being "how real life works", standards are best defined by what works well in practice, and it's useful to see whether or not style changes actually improve the readability of a given project. Since y'all have already been using your style for GDQuest, you know it works in your own use cases, but maybe others disagree. I would recommend that you submit pull requests to the demo projects repository, and ideally get feedback on them from the community.

I agree that the demo projects not currently having a consistent style is really bad, and I'm not saying that we should base a style guide off of their current style, they just need to be improved through specific changes and eventually moved toward uniformity and improved readability. If a guide is drafted, I would suggest at least some of the demo projects be updated first, which would allow users to give feedback on specific tangible changes to projects that would be caused by such a guide.

(Side note: We really need a dedicated maintainer for the demo projects repository. Most of the projects are still optimized for Godot 3.0, and there are PRs improving the projects and making them conform to the current style guide, but right now they're just sitting there... https://github.com/godotengine/godot-proposals/issues/172)

[NathanLovato]

My main concern is with something becoming a "standard" that nobody follows or maybe doesn't know about because it's not used anywhere in practice.

Making people follow the same guidelines is a job for the reviewers or leads in a team. It's the same problem with every project.

Since y'all have already been using your style for GDQuest, you know it works in your own use cases, but maybe others disagree

That's why we shared them with the community and we're having a discussion here. And no, it's not about our use cases only. We didn't open sourced them from the start to make them just for ourselves.

they just need to be improved through specific changes and eventually moved toward uniformity and improved readability

Again, how do you do that without people agreeing upon said changes? Do you really find the PRs in the godot-demo-projects repository consistent and moving the demos towards uniformity?

What happened since the start of the project is that the code was all over the place in the demos and the docs until we put up the GDScript style guide: http://docs.godotengine.org/en/latest/getting_started/scripting/gdscript/gdscript_styleguide.html

The "standard" came first, then we started to have some more consistent code formatting.

If a guide is drafted, I would suggest at least some of the demo projects be updated first, which would allow users to give feedback on specific tangible changes to projects that would be caused by such a guide.

Sure, that's what we did: https://github.com/GDQuest/godot-kickstarter-2019/tree/master/platformer-2d-rework

You can see the style in action in foss demo projects for review here:

• https://github.com/GDQuest/godot-3d-mannequin
• https://github.com/GDQuest/godot-platformer-2d

And in the projects of some community members who are unrelated to @razcore-art and me. We're the only 2 persons from GDQuest here.

The reason we don't PR to the demos directly are:

• As you said, there's no maintainer, nobody to review them on the demos repository.
• The demos need complete remakes for the most part, not just tweaking the code. You couldn't use the diff to see the improvement - you need to explore both projects separately. Right now, you can download the demo as a zip file from GitHub and look at them side-by-side.
• I'm a reviewer on the demos repo but
  
  
  
  1. There's not even a clear intention or agreement on what these demos should do. How do you review changes productively without a clear goal, scope, and code style for the demos? We all have different viewpoints, my reviews will be quite different from your reviews, Juan's reviews, Rémi's reviews unless we agree on some base principles. You have them on the godot engine, on the manual, but not on the demos right now.
  
  
  2. If I were the one doing the reviews and vouching for our style guide at the same time, even if our goal is just to help improve the code quality, its consistency and make reviews productive, I'd bet it'd be a source of arguments.
  
  

[NathanLovato]

As I've said above, we can detail every point in our guidelines, we've spent time discussing all of them. Then as I also said above, I don't want our guidelines to become standard, I'd like a standard like Python has PEP-8 and pythonic guidelines so that:

• We can have a solid reference to give consistent and productive reviews (less room for arguments and personal preferences).
• We can help everyone coming to Godot learn and teach good programming practices. That's one of the strengths of the Python community.

And to do so I'd:

1. Write and discuss a draft for the guidelines.
2. Use them in practice, sure, have some reference projects that people can explore and critique.
3. Have people critique the actual guidelines, possibly one by one, contribute some more examples or counter-examples.
4. Put a proposal doc up for more review, like the Python PEPs.

And moving forward, keep improving and modifying it as Godot changes and as people discover better ways of doing things.

We got started with 1. and 2. already, and I'm offering to do more work there to get this done, based on the community's discussion and critiques.

Then, I do want feedback from people who release actual games, from fellow developers, and guidelines that are better than what we have now.

[Feniks-Gaming]

I agree with @NathanLovato you can't update demos until you agree style guide to follow. Discussion here should bring some eyes to agree what style guide should look like and only there we can have a clear direction in maintaining demos and clear guide for what projects to accept and reject.

I think I it would be good idea to maybe make a video Nathan that this discussion is taking place and highlight it on twitter as you seem to have the biggest platform to do so. Comunity could then discuss here what exact style should be used and we could move forward. If you are in discussions with main developers maybe twitter update that opinions are needed here from them would also help. Here. Docs discussions aren't followed by many and small changes are fine to be done without wider involvement but big change like agreed style should see more eyes here. What do you think?

[NathanLovato]

Yup, we should bring people in!

I don't know if I should tweet or release a video about that because now GDQuest is relatively well-known in the Godot community, it may bring fans who would come with a bias towards our work or the opposite, or it could make some people feel like I'm trying to bias the discussion. So I'd rather let other people spread the word first, and maybe share the discussion where there's been some debate already.

[NathanLovato]

@akien-mga we're talking about having something like PEP8 for GDScript here, and a much more detailed style guide than what we have in the docs now, with the order of properties, methods, etc.

If you have a moment, could you give us your thoughts on the discussion, on the way to proceed, and invite devs who would be interested in participating?

[NathanLovato]

@Scony we've derailed the discussion, sorry for that. Do you mind if we rename this issue to say it's a discussion about having a more complete GDScript styleguide in general? Or should we open a new issue for that?

[Scony]

@NathanLovato yeah, feel free to set more general title.

Regarding PEP8, please note that most of the actual PEP8 may be used in Godot. And since it was discussed in a much bigger community, we should really consider starting from that.

Yet of course - we need to be careful - python is a general-purpose language and GDScript is rather a DSL for game development - that has to be taken into account.

Nevertheless, someone has to be a Thomas Jefferson of Godot and draft an initial proposal. I think PR will be a good place to discuss concrete things.

[NathanLovato]

I'm all for starting off PEP8 for everything that's relevant in GDScript, e.g. the details of the syntax, like where you put spaces or carriage returns, which is important to build e.g. a code formatter.

But we still have to thoroughly discuss and agree on guidelines for the order of properties, classes etc. (the topic of this issue), and other suggested practices.

You're right that PRs could help focus the discussion, as people can then discuss specific lines and points. I'll put up a PR for that. Or maybe do it in several passes to separate concerns.

[Feniks-Gaming]

When it comes to order of properties I think discussion so far resulted in this order correct me if I am wrong?

00. tool # optional for tool scripts
01. extends 
02. class_name
03. """docstring"""
04. signals 
05. constants
06. enums  # enums can be exported variables so should be near exported vars
07. exported variables
08. public variables
09. private variables
10.`onready` variable (node dependencies) # are initiated in _ready() so should be near it
11. optional built-in virtual `_init` method
12. built-in virtual `_ready` method
13. signal callbacks
14. remaining built-in virtual methods
15. public methods
16. private methods
17. static functions

My proposal for 03 """docstring""" longer than one line that it starts with one indent something like

    long description goes here
    if it's multiline

Here is example

thanks to that any long documentation can be folded away so keep code base readable to users familiar with it without need for scrolling down tones

[razcore-art]

I think the discussion related to the docstring can be its own topic actually. Since it should _indeed_ have its own style guideline. I haven't looked at PEP-8 for a long time now, but I don't remember the docs being discussed in that guideline. Most of the scientific/data-analysis community went with numpy's docstring styleguide which I think works pretty well: https://numpydoc.readthedocs.io/en/latest/format.html. It's human readable-first with no fancy "features". I think it was mostly made to be compatible with reST since they were using Sphinx to generate the docs. We could start with something like that. I'd personally go for something focused on Markdown rather than reST, but that's just a preference I guess.

We'll think about it, in any case, the idea we discussed at GDQuest is to have minimal to now docstrings because they tend to get out of sync with the code quite fast. It would be fine to get a workflow going where we'd complete the docstrings once the code gets into a beta stage or something like that, where we don't have breaking changes cause otherwise it's just a hassle to update it and no one wants to do it. We also believe that code should be self-documenting as much as possible so apart from an introductory docstring for the entire class we wouldn't really try to go too granular with the variables & functions. If you write declarative code more often (eg. make functions with meaningful names instead of "opaque" assignments of calculations) then it makes the code much simpler follow. This is kind of at odds with the way Godot works (preferring usage of "properties", e.g . variables and setter/gettters), but I think we can work something out. In general I'd say prefer to use meaningful functions if the computation assigned to a variable is complex enough or even if it's too "symbolical", meaning after 2 days you don't remember what the symbols meant cause we usually don't spell out stuff and instead go for abbreviations. This style of preferring functions over assignments should cut the need for docstrings drastically.

P.S.: the reason why for numpy it makes sense to get into complex docstrings like that is because it's a library so it needs proper documentation in order for users to know what the heck is going on. So this would be analogous to our demo & example projects or any other assets out there that need documentation for developers.

[NathanLovato]

@razcore-art There's a PEP dedicated to docstrings: PEP 257.

[razcore-art]

I never studied that, but at first superficial look I much prefer numpy's style of docstrings. That PEP 257 is from 2001, before numpy's time and much of the community has adopted the numpy guide since it's much more precise. Habbit of working as a scientist of sorts :)

[Feniks-Gaming]

I think we should keep discussion of which style of doc strings for separate issue and we should narrow this down here to just agreeing order of properties. Otherwise we will get to distracted here. I shouldn't even start this discussion so my apologies.

Is there any opposition to this order proposed and what is rationale behind it so we can discuss different orders?

00. tool # optional for tool scripts
01. extends 
02. class_name
03. """docstring"""
04. signals 
05. constants
06. enums  # enums can be exported variables so should be near exported vars
07. exported variables
08. public variables
09. private variables
10.`onready` variable (node dependencies) # are initiated in _ready() so should be near it
11. optional built-in virtual `_init` method
12. built-in virtual `_ready` method
13. signal callbacks
14. remaining built-in virtual methods
15. public methods
16. private methods
17. static functions

[razcore-art]

I think this is the order we're using, right @NathanLovato? So :+1: from me

[Feniks-Gaming]

There are 2 changes vs what GDQuest uses

First constants and enums switched places in order to have enums closer to export vars as enums can be exported it made sense to have them just before export variables.

Other change is bringing onready var closer to the _ready() method as they make more logical sense as stated here https://github.com/godotengine/godot-docs/issues/2997#issuecomment-568279298

[NathanLovato]

I'm all good with that, including the changes. I already updated our style guide the other day regarding the enums, and certainly don't mind changing the onready variables.

[Scony]

@Feniks-Gaming since enum may be used in e.g. const I'd prefer to keep the GDQuest order for the natural reading order.

Consider snippet from my game's Config.gd:

enum Car { CLASSIC, SEDAN, COUPE, HATCHBACK, PICKUP, TRUCK, AMBULANCE, POLICE, TAXI, GARBAGE }
# (...)

const CARS = {
    Car.CLASSIC: {
        'name': 'Classic',
        'layouts': {
            str(Color.black): 'res://vehicle-layouts/ClassicBlack.tscn',
            str(Color.red): 'res://vehicle-layouts/ClassicRed.tscn',
            str(Color.gray): 'res://vehicle-layouts/ClassicGray.tscn',
            str(Color.blue): 'res://vehicle-layouts/ClassicBlue.tscn',
            str(Color.green): 'res://vehicle-layouts/ClassicGreen.tscn',
        },
        'speed': 150,
        'price': 150,
        'tank': 100.0,
        'fuel_consumption': 0.01,
        'capacity': 20,
    },
# (...)

[Feniks-Gaming]

It looks like a solid argument I can't really argue with it.

[ntsik]

I'd make the argument that class_name should come before extends for two reasons:

1. It's the same as most other languages (or at least ones I'm familiar with). C++ has Derived : Base, Python has Derived(Base), Java has Derived extends Base, etc.

2. It's also consistent with the syntax for inner classes, e.g. class Derived extends Base.

[Duroxxigar]

If we do make a more complete standard - I'm voting against having 2 spaces between functions. There should only be 1 in my opinion. Just wanted to throw that out there. I also agree that class_name should come before extends. It is weird seeing the inherited class before the actual class.

[NathanLovato]

I'm voting against having 2 spaces between functions.

It's already in the gdscript guidelines since ~2+ years ago, based on Python's pep8: http://docs.godotengine.org/en/latest/getting_started/scripting/gdscript/gdscript_styleguide.html#blank-lines

Please give arguments for your choices, we don't want to fall into personal preferences there.

The case for 2 lines between functions is that it clearly distinguishes functions from one another, and from methods of an inner-class, that are separated by one line. This also distinguishes the separation between functions from the separation of logical blocks inside functions, that are also separated by one line.

As for the rest of the guidelines, it improves readability - you can understand the code's structure at a glance that way.

[Feniks-Gaming]

I strongly disagree. 2 spaces is a must. That is how Python and any other language that doesn't have {} does it to separate their functions. Blank line is not very readable. Especially as Nathan pointed out you already use single line to separate blocks of code inside function this would make those 2 indistinguishable and harder to read.

[Duroxxigar]

Code structure is all personal preference anyway. It's just what is agreed upon collectively. I know that both PEP8 and GDScript say to include 2 spaces, but that doesn't mean it can't change.

I disagree with it being more or less readable as well. There is a very clear break between two different functions. With the func keyword for GDScript and the def keyword for Python. As well as parameters and a colon. To me, it just adds a longer file to scroll through when reviewing it.

[Feniks-Gaming]

Of course it is but when we agree unified style we must go on something better than it's personal style.

I can be convinced that 1 line is better than 2 lines but we need some good arguments in favour of 1 line.

How many functions will you get pee script 50 maybe 100 on a larger file that's only extra 100 lines of code in several 1000s long file it's hardly much added effort especially that very rarely you will be reading entire file rather than jumping to specific functions you are reviewing. Issue is that if you scroll a little to fast it's easy to skip 1 break line and suddenly you are reading func2 thinking you are still reading func1 and it all becomes confusing. 2 lines mitigate this and reduce risk of errors. It also makes it easier to read outside of editor when code snippets are shared online on wiki, blogs and reddit.

[NathanLovato]

Code structure is all personal preference anyway. It's just what is agreed upon collectively.

That's contradictory. You're saying it's a personal preference, that is to say, the preference of a single individual, but at the same time agreed-upon collectively, i.e. as opposed to chosen individually.

If we discuss guidelines, that is precisely so that they are not the result of a single person's personal preferences but bring a relative consensus.

Also, if we provide arguments, it's so that we move a little more away from personal taste and towards reasoning.

Ideally, I'd love to have scientific papers on the subject to help us make the best decisions for the largest amount of users. Unfortunately, I don't know any. So we're likely going to be there with empirical evidence and experience.

---

I disagree with it being more or less readable as well. There is a very clear break between two different functions. With the func keyword for GDScript and the def keyword for Python. As well as parameters and a colon.

One thing I learned when I worked in UX is that the brain first processes big shapes, like bounding boxes, and that you naturally process what's in that shape as being related content.

With GDScript files being generally short, with a few functions, and code foldable, I'm not sure it makes that big of a difference when it comes to scrolling. It may make files what, 5-10% longer?

A side-by-side comparison of two long classes might be best but in the meantime, here's an example so people can compare.

One line:

extends Node
class_name StateMachine
"""
Hierarchical State machine for the player.
Initializes states and delegates engine callbacks (_physics_process, _unhandled_input) to the state.
"""

signal state_changed(previous, new)

onready var state: State = get_node(initial_state) setget set_state
onready var _state_name: = state.name

export var initial_state: = NodePath()

func _init() -> void:
    add_to_group("state_machine")

func _ready() -> void:
    connect("state_changed", self, "_on_state_changed")
    state.enter()

func _on_state_changed(previous: Node, new: Node) -> void:
    print("state changed")
    emit_signal("state_changed")

func _unhandled_input(event: InputEvent) -> void:
    state.unhandled_input(event)

func _physics_process(delta: float) -> void:
    state.physics_process(delta)

func transition_to(target_state_path: String, msg: Dictionary = {}) -> void:
    if not has_node(target_state_path):
        return

    var target_state: = get_node(target_state_path)
    assert target_state.is_composite == false

    state.exit()
    self.state = target_state
    state.enter(msg)
    Events.emit_signal("player_state_changed", state.name)

Two lines:

extends Node
class_name StateMachine
"""
Hierarchical State machine for the player.
Initializes states and delegates engine callbacks (_physics_process, _unhandled_input) to the state.
"""


signal state_changed(previous, new)

onready var state: State = get_node(initial_state) setget set_state
onready var _state_name: = state.name

export var initial_state: = NodePath()


func _init() -> void:
    add_to_group("state_machine")


func _ready() -> void:
    connect("state_changed", self, "_on_state_changed")
    state.enter()


func _on_state_changed(previous: Node, new: Node) -> void:
    print("state changed")
    emit_signal("state_changed")


func _unhandled_input(event: InputEvent) -> void:
    state.unhandled_input(event)


func _physics_process(delta: float) -> void:
    state.physics_process(delta)


func transition_to(target_state_path: String, msg: Dictionary = {}) -> void:
    if not has_node(target_state_path):
        return

    var target_state: = get_node(target_state_path)
    assert target_state.is_composite == false

    state.exit()
    self.state = target_state
    state.enter(msg)
    Events.emit_signal("player_state_changed", state.name)

[Duroxxigar]

Ignoring the debate about my statement being contradictory or not (as it doesn't provide much to the discussion at hand honestly) - the snippets you posted, both are readable.
I wonder if there are even any actual scientific papers out there about code structure though 🤔 ...

If we discuss guidelines, that is precisely so that they are not the result of a single person's personal preferences but bring a relative consensus.

Which is exactly why I threw in my vote on something. I already know the consensus is 2 spaces.

[NathanLovato]

Ignoring the debate about my statement being contradictory or not (as it doesn't provide much to the discussion at hand honestly)

My bad if I misinterpreted you. I thought you were dismissing other people's arguments by saying it's all personal preference anyway. So I was answering that no, it's not a matter of personal preference, that we're trying to reach consensus, and pointed out that you wrote it yourself.

I'll let others share their thoughts. The second example, with two lines, is easier on my eyes. Not that I can't read both - it's just a little easier to see the separation.

[Feniks-Gaming]

As I am viewing this discussion on mobile phone 2nd example requires slightly more scrolling but it's significantly easier to read on my smaller screen. If that is of any value to discussion.

I agree with Nathan here it's not that first example is unreadable but a 2nd exame has a structure that is easy to see at glance. I could zoom far enough that I would not be able to read a font any more and I would still know just from amount of what spaces where functions ends and start. In first example that is not possible.

I am not saying that 1 line break is bad it's just that 2 lines are better with almost no drawbacks to them.

It creates a patter that is easy to recognise and as you get accustomed to 2 lines meaning end of function you notice it easier and your ability to read code becomes faster and less mistake prone.

If I was quickly scrolling down in search of a function I need 2 line break would help me find it faster and made it clearer at glance to see how long the function is that I have to deal with.

I too also would love to read some papers behind 2 line vs 1 line readability if anyone has access

Edit: I took liberty of screenshoting both spinets side by side to demonstrate length increase in my opinion it is insignificant to matter

[YeldhamDev]

I personally think that signal callbacks should be put dead last. On my personal experience, they generally are very small, with trivial coding that generally just calls other functions, and if you're programming a rather complex UI, the script will accumulate a lot of signal callbacks, that will get in the way of the actual meat of the code.

[Feniks-Gaming]

Main reason for callbacks on front is that many of them are connected from inside _ready() function so they may make logical sense to put near _ready() function. However it's worth discussing as when you connect callbacks from editor they are automatically added to the end of a file which would suggest that is Godots preferred place to put them in.

[YeldhamDev]

@Feniks-Gaming I think most people rarely connect things from script, generally only doing it in case of Nodes instantiated on the fly.

[golddotasksquestions]

I agree with @Duroxxigar and would vote for one space instead of two between functions. The reason is not everyone is developing on large high-res desktop screens. Two spaces is elongating the scripts, making even basic scripts with only a few functions hard to fit on a single screen, even on 17" laptop monitors.
Reading and understanding the code is much easier if the user has to scroll less, provided things are still distinctively grouped, which they are.

[NathanLovato]

[signals] generally are very small, with trivial coding that generally just calls other functions

If you're mainly calling another function, you can directly connect to the target function, and from Godot 3.2 the script editor shows you the signal connections, on top of the icon in the scene tree that will take you to the method you connected to iirc.

I'd need to see how you end up with many small signals, as that's something we don't get in the team. Maybe there are ways to simplify your code if you're finding yourself in that situation. I find that using signals a lot tends to produce spaghetti code. We tend to use them sparingly now.

when you connect callbacks from editor they are automatically added to the end of a file

I would guess it's because it's simpler to append text to a file than to find the end of a function. You'll have to reorganize code anyway if you keep working with the code.

I think most people rarely connect things from script

I wouldn't be able to say anything about that without usage data.

All I could say is that connecting through the editor has its drawbacks. As a developer, it's more efficient to connect via code: autocompletion, staying in the script editor, you can find connections in the project with Ctrl Shift F, and Ctrl click the method you connect to jump to it.

---

And I'm done for this year. Time to turn off the computer. Happy new year everyone!

[YeldhamDev]

@NathanLovato

If you're mainly calling another function, you can directly connect to the target function

Lots of signals related to UI return things, generally indexes of items selected, with prevent directly calling functions. There are also times where functions need to be called in a specific order, or to call function that are from singletons.

I'd need to see how you end up with many small signals, as that's something we don't get in the team.

If your projects don't need more than just basic UIs, then yes, it would be rare to use a lot of signals.

All I could say is that connecting through the editor has its drawbacks. As a developer, it's more efficient to connect via code

It really depends, the editor itself steers the developer to connect things via the editor, and I find it more comfortable doing it so. Also, connecting things via code can create a mess, specially that you have to silence the return value warnings for each connection.

Happy new year everyone!

Happy 2020 for you as well! :wink:

---

@golddotasksquestions As someone with a small screen size (1366x768), two spaces are completely fine, as they help a lot to separate things out, and the space added when scrolling is negligible.

[NathanLovato]

It might be worth mentioning at this point that we're creating guidelines, so it's not a strict rulebook that we want everyone to follow to the T.

In the case of signals, I can see both guidelines work. @razcore-art and others, what do you think of having signal callback functions at the bottom when the user connects lots of them from the editor? We could have two recommendations to pick from there depending on the person's code. If you connect signals mostly through the editor or have a lot of them, you can put them at the bottom of the class so they don't get in the way of the class's interface?

If your projects don't need more than just basic UIs, then yes, it would be rare to use a lot of signals.

It's also that we use other approaches to avoid using signal connections. The event bus singleton Razvan talked about at the last godotcon, function calls in the animation player, coroutines... we found a few ways around wiring signals to extra callback functions. We generally try to avoid features that make the flow of the code less linear, when it's possible.

[Scony]

@NathanLovato @YeldhamDev why do you prefer distinguishing signal callback functions anyway? To me, they are just a private functions - nothing more nothing less. This way everyone is free to put them anywhere among private functions. And this freedom is essential as there are various situations:

• sometimes callback functions are very simple and e.g. added from the editor at the end
  
  
  
  • sometimes callback functions do a lot of things by delegating work to smaller, private functions (hence are placed just before them)
  
  
  • sometimes callback functions are called by other methods (public or private) and hence are placed somewhere nearby
  
  

[NathanLovato]

@Scony The signal callback functions we're talking about, like _on_Node_did_something(), are the ones that are only called through signal connections. They're often important to understand how a class works as they can call other functions and mutate the state of the object, while in the rest of your class's interface you can avoid mutations.

That's why for instance we put them close to the calls to connect, right before Godot's process and input callbacks: that way, we group all or most of the code that mutates the object's state in one place.

I think of signal callback functions like event listeners in Javascript. You generally want to group them together because they mess with your objects. And they follow a specific naming convention. In the case of signals:

_on_SomeNode_did_something()

[Scony]

@NathanLovato

all but static functions usually mutate the state. I don't see this point relevant.

Regarding special grouping for callback functions - it may be confusing if we force it. Let's consider the code below:

# called directly by parent and through signals by children
func handle_something(...):
    # (...)

# called only through signals by child
func _on_Button_pressed(...):
    # (...)

func _add_new_list_item():
    # add item
    # connect some list item's event to _on_LI_event()

# called only through signals
func _on_LI_event(...):
    # (...)

It's totally fine to have functions called not only by signals. It is also quite common to connect signals in runtime - by e.g. private functions. How would you order the functions in the code above? Would you force them all to be under _ready() or so? And if you claim "only _on_(...) functions are the real callback functions", then why they should come before handle_something which is much more meaningful than them?

To me, handle_something is just a regular public function. And _on_LI_event should be next to _add_new_list_item as they are very closely related.

[NathanLovato]

Got it, a convention there wouldn't make sense for some or most people's code. We can drop that.

all but static functions usually mutate the state.

Not for us at GDQuest, we take some inspiration from functional programming. And that explains some decisions in our own style guide, but anyway, that'd be off-topic as it goes into the broader realm of code architecture.

[winston-yallow]

Just my 2ct regarding two topics from above:

Convention for signal callbacks

I agree that a convention here does not make sense. I use signals a lot when I write editor plugins. Some methods are called only through signals, some are called directly too. I often decide based on the context where to place them.
(On a side note here: I almost never use the editor to connect signals, I always connect them via code. Besides that I still put the methods to the bottom of the script when they are small self explaining callbacks)

Separation of Functions

I am in favour of two lines spacing between methods. I use a 1024x768 display on a small laptop. The blank lines help me separating things. It helps me quickly finding the code blocks I am looking for. Most of my methods have roughly 20 lines. So an additional blank line just makes up 5% for me (which is no big deal considering the easier navigation in the code/better readability of the code).
Maybe I am a bit biased here as my day-to-day work in python requires me to stick to PEP-8 (which I love!), so maybe I am just more used to two lines spacing.

---

Additional things that could be discussed (I encountered these when I adopted the style guide for my current project):

Quote character

Should ' or " be preferred? I think it does not make sense to recommend either one. It depends on the users keyboard which one is easier to type in my opinion.
However consistency within the styleguide, docs and examples would be nice.

Type hints with inferred type

I personally place the whitespace before the colon:

var foo := "bar"

In the style guide from gdquest the following style is used:

var foo: = "bar"

I am used to := as this reads, for me personally, as "is defined as". We used this symbol in math always to define things so this seemed logical to me. I've also seen := multiple times while learning go so it seems to be somewhat common for assignments.

This is the reasoning behind gdquests style (quoted for reference):

Use : = with a space between the colon and the equal sign, not :=. : = is easier to spot compared to =, in case someone forgets to use the colon.

I personally could agree with that and don't mind to change my own way of doing this. However I think this might be worth discussing as people coming from other languages (go for example) may already know := from short variable declarations.

[NathanLovato]

Quote character

The docs' style guide already follows python's guideline: http://docs.godotengine.org/en/latest/getting_started/scripting/gdscript/gdscript_styleguide.html#quotes

In the style guide from gdquest the following style is used [for type hints]

That's actually the python and gdscript style for type hints: http://docs.godotengine.org/en/latest/getting_started/scripting/gdscript/gdscript_styleguide.html#static-typing

We already had a discussion about this on this repository. It's for readability, it's like natural english to have the colon at the end of the symbol.

var name: type
var name :type

So for consistency, the colon is always next to the name

var name: = value

[NathanLovato]

The first pass of the code order PR is up: https://github.com/godotengine/godot-docs/pull/3022

[aaronfranke]

In the style guide from gdquest the following style is used [for type hints]

That's actually the python and gdscript style for type hints: http://docs.godotengine.org/en/latest/getting_started/scripting/gdscript/gdscript_styleguide.html#static-typing

Actually it says:

When you let the compiler infer the type hint, write the colon and equal signs together: :=.

I prefer := since it's easier to look at. Placing : next to the word can be misread as an extension of the word, but : on the = makes the := "symbol" twice as big compared to =.

[Feniks-Gaming]

As I said in PR discussion I don't believe example code should use static typic considering that docs refer to it as "optional static typing" and what is optional shouldn't become official way of doing things just because this is our preference. GDScript remains primarily dynamic language and this is what examples should reflect

[NathanLovato]

When you let the compiler infer the type hint, write the colon and equal signs together: :=.

My bad! The worse part is that I wrote that part. 😄

[willnationsdev]

I agree with @aaronfranke that the := should be together, not separate.

name: type
name: type = value
name := value

@Feniks-Gaming

As I said in PR discussion I don't believe example code should use static typic considering that docs refer to it as "optional static typing" and what is optional shouldn't become official way of doing things just because this is our preference.

Well, I agree that the majority of examples that appear in the Godot docs should use the dynamic syntax, but whenever we do show code that is statically typed, say, for the static typing explanations in the documentation or in the style guide that tells the community how they should be writing their statically typed code, you still need it to be defined somewhere. So, not detailing a rule for it isn't a good idea, if that's what you were indicating (maybe I'm missing something?).

[Feniks-Gaming]

N9 I didn't mean we shouldn't define the rule. I meant that we shouldn't use static typing in general examples that are jot about static typing to avoid confusing the reader.

[willnationsdev]

Ah, yes. I agree with that.

[NathanLovato]

I agree with @aaronfranke that the := should be together, not separate.

As we said, it's already the guideline: http://docs.godotengine.org/en/latest/getting_started/scripting/gdscript/gdscript_styleguide.html#static-typing

[aaronfranke]

I opened an issue about project organization style: #3104. While it's not relevant to the OP, it is relevant to the discussion in this thread about project consistency, so I think it's worth linking here in case anyone would like to continue this discussion.

[NathanLovato]

Thanks, actually we can close this issue now