Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable Add Type Hints editor setting by default #88026

Merged

Conversation

Calinou
Copy link
Member

@Calinou Calinou commented Feb 6, 2024

Now that GDScript type hints improve performance since Godot 4.0 and the community is increasingly getting used to typed GDScript, it makes sense to add type hints by default in autocompletion and script templates.

Official demos will also be moving to type hints at some point in the future, further increasing the relevance of enabling type hints out of the box.

Additional context: https://twitter.com/reduzio/status/1754794206206239036 (although this has been a general trend since 4.0's release, and even before)

@Calinou Calinou requested a review from a team as a code owner February 6, 2024 18:40
@Calinou Calinou added this to the 4.x milestone Feb 6, 2024
Now that GDScript type hints improve performance since Godot 4.0
and the community is increasingly getting used to typed GDScript,
it makes sense to add type hints by default.

Official demos will also be moving to type hints at some point
in the future, further increasing the relevance of enabling type
hints out of the box.
@Calinou Calinou force-pushed the editor-default-enable-add-type-hints branch from 399ef49 to 345f09d Compare February 6, 2024 18:41
Copy link
Member

@AThousandShips AThousandShips left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm in favour as part of a broader push to prefer static typing, and phrasing is good

Unless I'm mistaken editor settings are all stored so this won't affect existing users, so anyone can just disable it if they dislike it

Copy link
Contributor

@Mickeon Mickeon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine with this. Although I have always felt like this option could use a bit more granularity to it.

@Mickeon
Copy link
Contributor

Mickeon commented Feb 6, 2024

Since this changes a default Editor Setting, I would encourage whoever is reading this to also check out #81906

@akien-mga akien-mga requested a review from a team February 7, 2024 09:11
@akien-mga akien-mga modified the milestones: 4.x, 4.3 Feb 7, 2024
@dalexeev
Copy link
Member

dalexeev commented Feb 7, 2024

I see little risk in this (unlike UNTYPED_DECLARATION, INFERRED_DECLARATION, and UNSAFE_* warnings). This only affects autocomplete methods like _ready(), users can still use untyped code. GDScript is a gradually typed language and should handle mixed code gracefully.

However, does this mean that our recommendations are shifting towards typed code, although we still use only untyped code in the documentation? Also, we recommend not mixing approaches, and changing the setting prevents users of untyped GDScript from sticking to one option.

@Mickeon
Copy link
Contributor

Mickeon commented Feb 7, 2024

"Not mixing approaches" feels like a weird rule. I get it's meant to encourage consistency, but GDScript works completely fine with a mixed approach.
In fact, most users probably have some form of "mixture" in their codebase. That's especially true if they start programming untyped, and gradually use types as they gain experience (I can relate).
Generally speaking hard-typing the codebase does take time, time that could be spent developing something else entirely. So, some parts of the codebase may be more "refined", while others not so much because they really do not need to in the moment.

In my personal opinion the documentation should still aim to be accessible and avoid static typing when possible, as it makes the code look more overwhelming than it actually is in practice.
A lot of times = can be substituted with := and experienced developers know this already.

@adamscott
Copy link
Member

adamscott commented Feb 7, 2024

cc. @GDQuest @Xananax @SanderVanhove

@SanderVanhove
Copy link

I'm a big fan of type hints and we use it nearly everywhere on Koira. We do have, as mentioned, a mixed approach though, because sometimes you are not 100% sure what type is coming in or you want to be able to support multiple types and switch within the function.

Although type hints can seem overwhelming, it helps beginners a lot with autocompletion and finding bugs before you run the game. Enabling this feature by default gives a nudge towards good programming practices.

@Xananax
Copy link

Xananax commented Feb 7, 2024

Funny, I just fished this old related article back for a conversation I was having with a friend.

From an educator perspective, my go-to recommendation is that dynamic typing should only be handled by experts, while beginners need explicit types.

My personal anecdotal experience is that types are scary for newcomers, but once the hurdle passed, they are easier on the slightly longer term1. The concept of "this keywords is a filter that validates other values are acceptable to put in that box" is easy. The syntax is complicated.

For that reason, I am all for types by default, but I am also in favor of leaving them out of at least the initial tutorials (:= is fine). Ideally, a switch above code boxes could add/remove types (similarly to GDScript/C# currently).

Footnotes

  1. Side note: This is what I've subjectively witnessed by teaching students IRL and online. In the literature though, there is no evidence of types being easier or harder for beginners (or for that matter, better or worse for anything in general, regardless of the metric chosen).

@akien-mga akien-mga merged commit f8020d1 into godotengine:master Feb 8, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants