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

Use static typing in scripts #868

Closed
Calinou opened this issue Mar 1, 2023 · 4 comments · Fixed by #1063
Closed

Use static typing in scripts #868

Calinou opened this issue Mar 1, 2023 · 4 comments · Fixed by #1063

Comments

@Calinou
Copy link
Member

Calinou commented Mar 1, 2023

Which demo project is affected: All demos

Description: Now that typed scripts have a tangible performance improvement over untyped scripts, we should start using type hints where relevant. This should also make demos easier to maintain in the long run. Some demos added in 4.0 have type hints already, but we should extend this to all demos for consistency.

One open question is whether we should use type inference when possible (i.e. using var x := y instead of writing type names explicitly). Currently, some of the demo projects use it, but it's not particularly beginner-friendly. Some people don't know this syntax exists, and it makes online code reviews more difficult too.

@jtnicholl
Copy link
Contributor

One open question is whether we should use type inference when possible (i.e. using var x := y instead of writing type names explicitly). Currently, some of the demo projects use it, but it's not particularly beginner-friendly. Some people don't know this syntax exists, and it makes online code reviews more difficult too.

The style guide recommends to use inferred typing when the type is immediately obvious from the assignment, which I think is a good policy.

@dalexeev
Copy link
Member

dalexeev commented Jan 8, 2024

Even though the Writing guidelines say not to use static typing for documentation, I find static typing useful and more readable for demo projects (especially if you're reading the code on GitHub).

I agree that consistency is important for documentation. But demo projects are not a collection of code fragments through all the documentation. Each demo is a self-contained completed project. We added and are constantly improving static typing in GDScript because we believe it offers users a more convenient and reliable experience in developing and maintaining projects.

So, I propose to add the following rules:

  1. Each demo must adhere to the same style: either static or dynamic typing, mixing is prohibited.
  2. The Getting Started demos should use dynamic typing as they are explained step by step in the docs and static typing is still prohibited in the docs (except for examples explaining static typing).
  3. I'm not sure whether to require static types in all new demos and force updating old demos. But, if there is a clear case for static typing for a demo, then we should prefer static typing (some demos contain little or no GDScript, there is no point in requiring static typing for them).

However, there are questions about using static typing:

  1. Should we prohibit the use type inference for variables (:=), if the initializer does not contain the type name? (Discussed above.)
  2. Should we require the type of a for loop variable to be specified, even if it's inferred? (A new feature in 4.2.)
  3. Should we require the type of a constant to be specified, if the initializer does not contain the type name? (Constants always infer a hard type, unlike variables.)

@MatrixFrog
Copy link
Contributor

What can we do to help reach a consensus on this? The rules sound good to me (though I'm a godot newbie) and I would say it's probably good to include types explicitly in demos if they're not immediately obvious from context, perhaps more than you would in a "real" project, just to make things super clear and explicit.

@Calinou
Copy link
Member Author

Calinou commented Jan 27, 2024

I think adding static typing to demo projects following #868 (comment) should be fine, with the exception of Dodge the Creeps and Squash the Creeps as these are the "Your first game" projects for 2D and 3D respectively.

To avoid merge conflicts, I'll go through demo projects first to ensure their files are fully up-to-date for 4.2.1 first (such as the mesh formats).

Edit: Pull request opened: #1013

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

Successfully merging a pull request may close this issue.

4 participants