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

Improve description of annotations #9188

Merged
merged 1 commit into from
Apr 30, 2024

Conversation

allenwp
Copy link
Contributor

@allenwp allenwp commented Apr 5, 2024

This change uses wording from discussions in the RocketChat GDscript channel.

@allenwp allenwp force-pushed the annotation-improvements branch from 33a4907 to 25e6104 Compare April 5, 2024 15:04
@AThousandShips
Copy link
Member

I still think the specifics of what annotations are isn't relevant to this article, I don't think it should even mention keywords because it's irrelevant

@AThousandShips AThousandShips requested a review from a team April 5, 2024 15:06
@AThousandShips AThousandShips added enhancement discussion topic:gdscript area:manual Issues and PRs related to the Manual/Tutorials section of the documentation labels Apr 5, 2024
@allenwp
Copy link
Contributor Author

allenwp commented Apr 5, 2024

I’m intentionally leaving this as a draft PR until after the Tuesday GDScript meeting, in case that meeting highlights any other ways annotations could be described in documentation.

@dalexeev
Copy link
Member

dalexeev commented Apr 5, 2024

I don't agree that the difference between annotations and keywords is how the script is compiled. In fact, annotations affect the script behavior and the bytecode generated (for example, @onready moves member variable initialization to @implicit_ready() function). But in any case, these are unnecessary technical details.

I think annotations in GDScript are not much different from keywords. Annotations are not just metadata like attributes in PHP and not a clear language feature like decorators in Python.

Differences between annotations and keywords:

  1. Adding a new annotation is guaranteed not to conflict with any identifiers.
  2. Adding a new annotation will likely require fewer changes to the parser than adding a keyword.
  3. The presence of annotations reduces the number of keywords. This is especially suitable for modifiers on declarations ("annotate" in the sense of "precede"), since modifiers are not used on their own. The only exception is static.

@nlupugla
Copy link

nlupugla commented Apr 5, 2024

Is static really the only exception? Wouldn't extends and perhaps class_name also be considered modifiers?

If not for those counterexamples, it would be tempting to think of keywords as nouns and annotations as adjectives.

@allenwp allenwp force-pushed the annotation-improvements branch from 25e6104 to 180271f Compare April 5, 2024 15:48
@AThousandShips AThousandShips changed the title Improved description of annotations Improve description of annotations Apr 5, 2024
@nlupugla
Copy link

nlupugla commented Apr 5, 2024

Thanks for starting on this allenwp!

I'm kind of wondering whether a change like this would be better suited for the "contributing" section of the docs. The user doesn't really care whether static is a keyword or an annotation and why, they just want to know what to type to make their function static. On the other hand, when contributing to GDScript, you'd want some guidelines as to whether to make your new feature of keyword or annotation.

@dalexeev
Copy link
Member

dalexeev commented Apr 5, 2024

Is static really the only exception? Wouldn't extends and perhaps class_name also be considered modifiers?

class_name declares a global class identifier, just classes in GDScript can be unnamed. If class_name is a modifier, then class is too, as well as var, const, func, etc.

extends serves as a separator between the class identifier and the base class name/path. Like in separates the iterable expression from the iterator variable identifier/type in for loop (here token in has a different meaning, not the content test operator). And like when separates the guard expression from the pattern list in match. Also, because class_name is optional, extends can act on its own, so it's not a modifier.

In principle, any keyword could be an annotation (and then you could do something like @var var = 1). But it would be awkward to type. Another approach: keywords are used more often than annotations, especially in function bodies (which is why I would prefer to have let rather than @onready var).

@allenwp
Copy link
Contributor Author

allenwp commented Apr 5, 2024

I'm kind of wondering whether a change like this would be better suited for the "contributing" section of the docs.

💯 Yep, a new Contributing page should be added with guidelines that go into detail on keywords vs annotations.

Maybe I could include that new page in this PR, but I feel I am not the person to write it, as the nuance goes beyond my experience and understanding...

@allenwp
Copy link
Contributor Author

allenwp commented Apr 5, 2024

Actually, maybe I can take stab at this… Here is a thought on how a Contributing section could be written on this subject. It would pair nicely with this current PR draft for the user-facing documentation:

Keywords vs Annotations

If the primary purpose is to affect the way that the engine, editor, or external tools treat or interact with the script, it should be implemented as an annotation. Otherwise, it should be implemented as a keyword.

Edit: I've pushed a new page to the Contributing section of the docs that adds some details on annotations vs. keywords. Please let me know if I've done this correctly, as this is my first time making this sort of change.

@allenwp allenwp force-pushed the annotation-improvements branch 3 times, most recently from d455033 to 3c476b7 Compare April 7, 2024 23:48
@allenwp
Copy link
Contributor Author

allenwp commented Apr 9, 2024

One important note that was brought up during the meeting was that Contributing guidelines should be written based on what we want future tokens to be implemented as, not based on describing all existing annotations and keywords perfectly.

@nlupugla
Copy link

nlupugla commented Apr 9, 2024

An alternative to godot-specific vs general programming is whether the token is more important at run time or compile time. Or maybe another way to say it would be, whether the token operates within the code or on the code itself.

For example, one can think of @onready as acting on the source code (although that's not how it's implemented): creating a _ready function if it doesn't exist and performing the assignment inside it.

Similarly, you can think of @export as modifying the source code to add boilerplate to the _set and _get methods of the script (although again, that is not how it is actually implemented).

In contrast, keywords like func or await don't really "act on the source code" in the same way.

This is also maybe a more helpful guideline for new keywords/annotations going forward. Whereas the distinction between game/Godot-specific and general programming can be fuzzy, I think the distinction between run time/compile time is more clear and might have technical implications about how the feature is implemented. For example, one could imagine designing the GDScript compiler/VM in such a way that all annotations are resolved before non-annotations. This would essentially make annotations an avenue for metaprogramming (e.g. @serializable, @observable, @evaluate(PI / 4)) whereas keywords would be reserved for core language features (e.g. Trait, Interface, maybe, readonly).

@allenwp
Copy link
Contributor Author

allenwp commented Apr 10, 2024

An alternative to godot-specific vs general programming is whether the token is more important at run time or compile time. Or maybe another way to say it would be, whether the token operates within the code or on the code itself.

I think there is some promise with the concept of "annotations operate on the script's code" (as in, the annotations are not considered the "code of the script". I think this is a little different than the historical/previous definitions of GDScript annotations, but again this could be a good thing.

The wording of "the token is more important at runtime" might be confusing to some people, as I interpreted that to mean that annotations like @tool and @export are important at runtime because they change how (existing) code behaves at runtime, whereas access modifier keywords are a traditional compile-time (not runtime) token in pre-compiled languages.

I will try to make some time in the next week or two to write out a new draft with this sort of wording instead, so we can compare it.

@allenwp allenwp force-pushed the annotation-improvements branch 2 times, most recently from 4599b24 to a6f3e63 Compare April 22, 2024 19:41
@allenwp
Copy link
Contributor Author

allenwp commented Apr 22, 2024

I just pushed a commit that reworks some of the PR. I think this basically the best I can do with annotation documentation for users and contributors based on the comments and discussion that I've seen in RocketChat, the GDScript meeting, and here in this PR. The line I added was:

Create annotations for modifiers that act on the script or its code.

Unfortunately, I don't entirely believe that this new documentation actually helps all that much in guiding whether abstract should be implemented as an annotation or as a keyword. The way I would interpret it is that abstract should be implemented as an annotation because permission modifiers are "modifiers that act on the script's code". It still seems a little vague to me.

These new changes remove the suggestion from @akien-mga:

So I think I like the proposal that annotations are used for engine specific behavior, and keywords are for purely language specific modifiers, which could be used if GDScript was theoretically spinned to a Godot-agnostic language (it won't, but it can be a way to reason about it).

With this new approach, some language features that act on the code, such as metaprogramming, would be implemented as annotations instead of keywords.

compilation details because these are often inconsistent between annotations

Create annotations for modifiers that act on the script or its code.
Addionally, create annotations for behaviour that is specific to the Godot
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Addionally, create annotations for behaviour that is specific to the Godot
Addionally, create annotations for behavior that is specific to the Godot

Copy link
Member

Choose a reason for hiding this comment

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

I unmarked this as resolved since the PR still have the previous word.

@allenwp allenwp force-pushed the annotation-improvements branch from 63bf2c2 to 71e4e2c Compare April 25, 2024 17:17
@allenwp
Copy link
Contributor Author

allenwp commented Apr 25, 2024

In yesterday's GDScript meeting, the decision was made to implemented abstract as a keyword, rather than an annotation. This is somewhat in line with with the previous idea of "Create keywords for modifiers that are, theoretically, equally useful in contexts outside of Godot.". Instead of bringing back this specific wording into the documentation, I've rewrote it slightly. I think this PR is now ready for formal review and merge, so I'm taking it out of draft.

@allenwp allenwp marked this pull request as ready for review April 25, 2024 17:17
Copy link
Member

@vnen vnen left a comment

Choose a reason for hiding this comment

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

I believe the description of annotations is a good fit for what has been discussed with the GDScript team and it's an improvement of what was there before.

Removes an unnecessary and potentially confusing comparison of annotations to keywords and generally improves wording in the GDScript Basics tutorial. A new page to the Contributing section has been added for scripting development guidelines. This new page hosts suggestions on when annotations should be implemented in GDScript.
@allenwp allenwp force-pushed the annotation-improvements branch from 71e4e2c to 8dd09aa Compare April 29, 2024 17:18
@mhilbrunner mhilbrunner merged commit 160034a into godotengine:master Apr 30, 2024
1 check passed
@mhilbrunner
Copy link
Member

mhilbrunner commented Apr 30, 2024

Merged. Thanks! Good idea to document the results of this discussion. :)

@allenwp allenwp deleted the annotation-improvements branch April 30, 2024 03:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:manual Issues and PRs related to the Manual/Tutorials section of the documentation discussion enhancement topic:gdscript
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants