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

Allow users to choose if scripts should be forced to be statically typed (or not) #173

Closed
dreamsComeTrue opened this issue Oct 21, 2019 · 34 comments

Comments

@dreamsComeTrue
Copy link

Describe the project you are working on:

Godot source code itself / RPG-type game in 2D

Describe the problem or limitation you are having in your project:

Having lot of GDScript files, I am not forced anyhow to maintain consistency across them.
Thus - I sometimes use optional typing, sometimes not.
I would love to have some sort of switch (on Editor Settings for example) that will force me
to remember to put types (or not) in Script files.

Describe how this feature / enhancement will help you overcome this problem or limitation:

Having scripts checked for using optional typing across whole files would set harmony across whole project.

Show a mock up screenshots/video or a flow diagram explaining how your proposal will work:

image

Describe implementation detail for your proposal (in code), if possible:

Probably (apart from Editor Settings changes) that would need a lot of modifications of script_text_editor.

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

Only, if team/developers agree to have one appointed way of writing scripts

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

It should have access to Script Editor itself (or some kind of background checker/code linter) - and as long as script_text_editor is a plugin itself - it could be also considered as a addon.

@KoBeWi
Copy link
Member

KoBeWi commented Oct 21, 2019

It's basically like warning about unsafe lines, no?

@rainlizard
Copy link

When the planned speedboost for using typed-variables is implemented, I would definitely want to have this feature so everything can be consistently optimized.

@aaronfranke
Copy link
Member

aaronfranke commented Oct 22, 2019

One big issue with this currently is that there are some pieces of code you can't write with static typing, such as if you need nullable value types. EDIT: #162

@Calinou
Copy link
Member

Calinou commented Oct 22, 2019

If we want this to be implemented, we need to have an any type hint first, so that you can explicitly mark values as being dynamically typed.

Edit: This is now available in the master branch as the Variant type hint.

@bojidar-bg
Copy link

Note that such a setting should likely live in the Project Settings, since it should be the same for all team members working on a project.

@Jummit
Copy link

Jummit commented Oct 22, 2019

This looks like it could be a by default disabled warning.

@dreamsComeTrue
Copy link
Author

It's basically like warning about unsafe lines, no?

Am I missing something - is there a magic switch for that? What I would like to achieve is one big list of all places in scripts where there is no type information which is dynamically updated as I am editing them. Currently, there is an Errors tab on Debugger pane, and it populates kinda similar data, but that happens only after I run the game (more like a debug output of all code places which were executed and had some problems). It is not being updated 'on the fly'.

@KoBeWi
Copy link
Member

KoBeWi commented Oct 22, 2019

Am I missing something - is there a magic switch for that?

There's none. "Safe" lines are code with unambiguous and strictly static typing (so no dynamic stuff). Their numbers are marked in green-ish color. I guess if we implemented a warning about unsafe lines, it could be enough for this proposal. Although there are some cases where some lines are marked as "unsafe" when they shouldn't.

@fab918
Copy link

fab918 commented Oct 23, 2019

I don't know if I should post a dedicated proposal but I want to extend your proposal to resolve your issue: keep the best of the two-word and switch to a (semi) typed language.

There are practically only advantages that will help beginners and advanced users:

  • auto-complete
  • keep consistency with tutorials and examples
  • avoid bugs
  • avoid bad designs
  • increase the maintainability (especially for a serious project)
  • performance (one of the main complain about GDScript, people don't want to learn GDNative)
  • etc.

The main drawback is the extra syntax, but that's why I say "semi typed", the magic of GDscript is to be able to make type inferences in most of the situation. In other words, the language can be "typed" without writing any types in code in most of the case, so why keep this inconsistency? The gain of performance and maintainability will be a game changer for (semi) pro-users and the compilation errors will helps especially beginners.

@aaronfranke
Copy link
Member

@fab918: The main drawback is the extra syntax

The main drawback is breaking backwards compatibility. This likely won't happen even though I personally think it's a good idea.

@fab918
Copy link

fab918 commented Oct 23, 2019

@aaronfranke Godot 4 will be a major release, major release can have breaking changes if the benefits worth it. Maybe a survey on this, explaining the stakes, can be a good option!?

Moreover, a typed language doesn't mean you can't have untyped variable (but not by default). If you set an any type like Typescript, you can probably convert old code to new one without breaking changes.

It's a shame to have to choose between fun and fast syntax (GDscript) or robust language (c++/c#) when GDscript can be both.

@Calinou
Copy link
Member

Calinou commented Oct 23, 2019

Godot 4 will be a major release, major release can have breaking changes if the benefits worth it. Maybe a survey on this, explaining the stakes, can be a good option!?

We already have some compatibility breakages planned for 4.0. We don't want to add too much of them to avoid ending up in a situation like Godot 3 where many people never migrated to it. This is especially true due to the new rendering engines coming in Godot 4, which most people will benefit from.

I also proposed adding an any keyword above. Combining it with a project setting that requires everything to have a type annotation is likely sufficient (like TypeScript's noImplicitAny).

  • performance (one of the main complain about GDScript, people don't want to learn GDNative)

Typed instructions can likely be implemented without requiring type hints to be used everywhere. Of course, the more type hints you add, the better the speedup will be 🙂

@fab918
Copy link

fab918 commented Oct 23, 2019

I also proposed adding an any keyword above. Combining it with a project setting that requires everything to have a type IF THE TYPE IS NOT INFERABLE annotation is likely sufficient (like TypeScript's noImplicitAny).

I added few words to your quote I think they are important. If your assertion on performance optimization is correct, it's a good balanced solution (I think it's important if this box is checked by the default to promote best practice for users, especially beginners).

@Calinou
Copy link
Member

Calinou commented Oct 23, 2019

@fab918 Of course, the point of the setting isn't to forbid type inference. I like my := assignments too 🙂

That said, I'd say it's a bad idea to make = automatically infer the type as it'd change how the language would behave depending on project settings.

@fab918
Copy link

fab918 commented Oct 23, 2019

That said, I'd say it's a bad idea to make = automatically infer the type as it'd change how the language would behave depending on project settings.

@Calinou we are near to be 100% agreed :)

I understand the principle, but forasmuch it's only for a hint, is it a really problem? Because the alternative is to be forced to use an heavy/useless syntax
:= (especially if this setting become the standard in the community, what I'm pretty sure it will be when the performance optimization will be available). And in term of principles, a useless syntax is against the spirit of GdScript / Python.

@Calinou
Copy link
Member

Calinou commented Oct 23, 2019

Because the alternative is to be forced to use an heavy/ugly/useless syntax

This syntax is established in many languages such as Go. I don't think it's so alien to people, especially as it's well-documented in the static typing tutorial.

And in term of principles, a useless syntax is against the spirit of GdScript / Python.

There's "Explicit is better than implicit." in the Zen of Python still 😉

@fab918
Copy link

fab918 commented Oct 23, 2019

This syntax is established in many languages such as Go. I don't think it's so alien to people, especially as it's well-docoumented in the static typing tutorial.

It makes totally sense in a dynamical context to said when you want to inferring a type := but this setting said that the context should be a typed one, and in a typed one, we want always try to do := if the type is not added, I am wrong?

@Calinou
Copy link
Member

Calinou commented Oct 23, 2019

@fab918 My point is that we should avoid making language behavior depend on project settings. There are many practical issues with this:

  • Third-party assets may stop working if you import them into your own project with different settings.
  • This breaks existing tutorials and demos that people can find online.
  • It makes it harder to find solutions to scripting problems, as different people may effectively be using a different version of GDScript without realizing it.

@fab918
Copy link

fab918 commented Oct 24, 2019

The setting will break all non-typed example/tutorial in any case. That's why I suggested making this setting a default, to promote this direction for the futur.

Maybe an option to erase/add types in the editor can help to switch easily.

Anyway, I can live with that and I don't want to extend too much the discussion for a detail, I will be more than happy with the any keyword and this setting

@Wavesonics
Copy link

I think doing it on a per file level makes more sense. Adding a keyword like strict to the top of a file enforces static typing for that file. That way you can mix and match. A non strict project can include strict code from the asset store, and receve benefit from it being strict, or visa versa.

This is some what similar to what JavaScript does: https://www.w3schools.com/js/js_strict.asp

The other advantage of per file, is this obviously impacts how the code is written and read, so putting it in the code to be read is nice. Having it as a project setting decouples it from the code, you have to go check some where else to have all of the information. And if you include code from the asset store you can't be sure it was intended to be used in strict mode or not.

@Jummit
Copy link

Jummit commented Dec 3, 2019

I think doing it on a per file level makes more sense

The problem OP is having is that he isn't consistent across files. This will not help solve that. Also, I don't like the idea of more magic keywords in GDscript. A project settings would complement "Treat warning as errors" nicely.

@Calinou
Copy link
Member

Calinou commented Dec 3, 2019

Ignoring warnings in third-party add-ons should be covered by godotengine/godot#33437, which is now merged and available in 3.2 beta2 and 3.1.2 alike.

@Shadowblitz16
Copy link

Shadowblitz16 commented Nov 23, 2020

What about statically typing nodes with cylindrical dependencies or errors?
Its currently not possible to do so.

That makes this this feature really hard to do

The ideal way would be to ignore cylindrical dependencies or errors when this feature is on because everything would be typed checked at compile time

Also we need a way to statically type all custom objects first this would include nodes

@Shadowblitz16
Copy link

Looks like gdscript is getting generics and its going to be <> styled which I am fine with since its from C# and c++

@Calinou Calinou changed the title Allow users to choose, if scripts should be force to be strongly-typed (or not) Allow users to choose if scripts should be force to be strongly-typed (or not) Jun 13, 2021
@Calinou Calinou changed the title Allow users to choose if scripts should be force to be strongly-typed (or not) Allow users to choose if scripts should be forced to be statically typed (or not) Sep 11, 2021
@mirrorlink
Copy link

This is the only reason I still havent migrated to godot. To be fair, a full linter would even be better. So you can decide in which cases you want typing or not, just like typescript.

Maybe godot should support typescript itself? Wouldnt it be much easier? Im pretty sure microsoft would help as they did with C#

@Calinou
Copy link
Member

Calinou commented Sep 18, 2021

Maybe godot should support typescript itself? Wouldnt it be much easier? Im pretty sure microsoft would help as they did with C#

There are no plans to support more languages officially in Godot, but nothing prevents third parties from creating integrations thanks to GDNative. There is a QuickJS integration here: https://github.com/GodotExplorer/ECMAScript

@akien-mga
Copy link
Member

This is probably superseded by #3284, but I guess we can keep it open until that one has been implemented and see if we want to take it further (e.g. by enforcing it on a per-file basis instead of a project-wide warning, so that e.g. some plugins could enforce it while the main project would not).

@Calinou
Copy link
Member

Calinou commented Jun 16, 2022

This is probably superseded by #3284

That proposal is actually entirely different and can be implemented separately. It's only about the usage of type inference syntax, not using type hints.

godotengine/godot#59428 only addresses this proposal, not #3284.

@TannerBluhm
Copy link

I'm new here, just curious what it would take to move this PR forward?

@Zireael07
Copy link

@TannerBluhm This is an issue, not a PR.

@Calinou
Copy link
Member

Calinou commented Jul 29, 2023

godotengine/godot#59428 needs to be rebased and reviewed again. cc @jordigcs

I can do a quick review, but GDScript isn't my area.

@TheColorRed
Copy link

TheColorRed commented Jul 30, 2023

I'm still new to Godot (so maybe it's already supported), but when adding strict types wouldn't it be helpful to support union types (#737) otherwise you will have to put Variant everywhere you want multiple types, and that isn't very helpful.

var val: int | float = 123

@Calinou
Copy link
Member

Calinou commented Jul 30, 2023

I'm still new to Godot (so maybe it's already supported), but when adding strict types wouldn't it be helpful to support union types (#737) otherwise you will have to put Variant everywhere you want multiple types, and that isn't very helpful.

var val: int | float = 123

Godot already allows implicit casts in many situations, such as float -> int or String -> StringName. As a result, union types aren't essential for getting this proposal working.

@akien-mga
Copy link
Member

Implemented as an optional warning (which can be enforced as error) in godotengine/godot#81355.

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

Successfully merging a pull request may close this issue.