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

GDScript 2.0 (again) #40598

Merged
merged 11 commits into from
Jul 23, 2020
Merged

GDScript 2.0 (again) #40598

merged 11 commits into from
Jul 23, 2020

Conversation

vnen
Copy link
Member

@vnen vnen commented Jul 22, 2020

This one includes and complements the other PR: #39093. Most changes are outlined on that description.

All the things marked as "broken" there are now implemented. This includes type checks, code completion, warnings, language server (although I could not test this), and the mentioned optimizations (constant folding and avoiding allocating an array when range is used as a for loop iterator).

setget is now replaced by properties, implemented as outlined in the proposal: godotengine/godot-proposals#844.

I've tried to do a bunch of tests to find potential issues. I solved a lot of crashes and incorrect behavior along the way and this should be mostly stable. However I won't be surprised if someone can find other crash cases, or infinite loops (especially on completion), or wrong behavior. If you do, please report an issue and I'll solve it when possible.

There are still a lot to improve but this should be at the same level of what it was before (although I know completion is not super great yet, I intend to revisit the code and overall improve the detection and listing).

vnen added 10 commits July 20, 2020 11:38
Sometimes to fix something you have to break it first.

This get GDScript mostly working with the new tokenizer and parser but
a lot of things isn't working yet. It compiles and it's usable, and that
should be enough for now.

Don't worry: other huge commits will come after this.
Also improve error for unknown characters.
Also store Variant operator to avoid needing to do it repeatedly in
later compiling stages.
Reenable checking those when validating code.
@vnen vnen added this to the 4.0 milestone Jul 22, 2020
@vnen vnen requested a review from bojidar-bg as a code owner July 22, 2020 13:39
@vnen vnen mentioned this pull request Jul 22, 2020
@akien-mga akien-mga changed the title Gdscript 2.0 (again) GDScript 2.0 (again) Jul 22, 2020
@akien-mga
Copy link
Member

This naturally needs extensive testing and possibly some more code review, but let's merge already to get a wide testing coverage from users of the master branch.

@akien-mga akien-mga merged commit 3811fb9 into godotengine:master Jul 23, 2020
@akien-mga
Copy link
Member

Thanks!

@Anutrix
Copy link
Contributor

Anutrix commented Jul 24, 2020

Weren't we going rename this to GDScript 4.0 to better signify that it's for 4.0? Or was it just some suggestion by someone and was rejected?

@vnen
Copy link
Member Author

vnen commented Jul 24, 2020

Weren't we going rename this to GDScript 4.0 to better signify that it's for 4.0? Or was it just some suggestion by someone and was rejected?

This is just a title for this PR, we don't have an explicit GDScript version number (or you can say that it follows the Godot version).

@Sslaxx
Copy link

Sslaxx commented Aug 5, 2020

Weren't we going rename this to GDScript 4.0 to better signify that it's for 4.0? Or was it just some suggestion by someone and was rejected?

This is just a title for this PR, we don't have an explicit GDScript version number (or you can say that it follows the Godot version).

Yet we have NativeScript 1.1.

@strank
Copy link
Contributor

strank commented Aug 28, 2020

I have two questions/suggestions regarding the expanded use of StringName:

  • Regarding the new literal notation for &"This is StringName" - I think there are good reasons why python chooses to use characters as the prefixes for string literals such as b"bytestring". It looks less busy compared to a symbol and it avoids clashing with the use of existing operators something & &"stringname". How about i"interned string" or n"named string"? (And since all the prefix literals are changing, I would suggest the same for ^ and $.)

  • The interning of strings in python is actually fully transparent to the user of python. Almost all strings known at compile time are interned, based on certain rules, see for example http://guilload.com/python-string-interning/ and there's no separate class at all. Maybe we could consider that approach for its ease of use? Might still be good to specifically mark strings to be (not) interned.

@gvekan
Copy link
Contributor

gvekan commented Nov 6, 2020

@vnen I'm working on completion for calls and I don't understand the purpose of:

COMPLETION_METHOD, // List available methods in scope.

It's the completion_context type when the cursor is in this spot print(|), which makes it weird that it should "List available methods in scope". No completion is implemented for COMPLETION_METHOD right now. Can you explain?

I hope it's okay that I ask here, I did because of git blame.

@vnen
Copy link
Member Author

vnen commented Nov 6, 2020

@gvekan if the comment contradicts the code then trust the code. This was likely an oversight when I added it since I started with the previous code and it might've worked that way before. Feel free to change it (I don't think that listing methods in scope is relevant because those are usually part of the list of identifiers, so there's not really much use for this kind of completion).

Also, you can open issues and mention me to avoid cluttering this merged PR.

@elenakrittik
Copy link
Contributor

I'm implementing a GDScript lexer, and for reference looked at Godot's own tokenizer. In gdscript_tokenizer.cpp there's two keywords that appear to be totally unusable: namespace and trait. After digging a bit into commit history i found this PR which actually introduced both of these, and after a bit of additional search i found two GIPs assosiated with them: #1566 and #6416. I wonder if they are kind of "reserved" in case GIPs will get approved and someone gets around to actually make use of these keywords?

@vnen
Copy link
Member Author

vnen commented Jul 24, 2023

I wonder if they are kind of "reserved" in case GIPs will get approved and someone gets around to actually make use of these keywords?

Yes, they were reserved beforehand with the plans of (eventually) implementing them. This just haven't been done yet.

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.

7 participants