Implement an explicit virtual method mark for GDScript

[Lazy-Rabbit-2001]

Implements and closes godotengine/godot-proposals#1631
Relative PRs: #103768, #67777

GDScript is a language where all methods are born with overridability, which brings confusion when it comes to user-defined _-prefixed methods (private methods). This pr implements an explicit way to mark a method, in GDScript, defined by user as a virtual method that can be overridden by subclasses. All old-fashioned overriding behaviors will be warned by the editor to maintain the compatibility with previous versions.

Why not making it a key word?

Honestly, making it a key word sounds better than turning it into an annotation. However, being a key word means forced checking and forced error if you don't follow the rule, which greatly breaks compatibility with previous versions of Godot. To maintain the compatibility at the most extent, annotation doesn't sound a disaster for old scripts, so I selected making it an annotation instead of a key word with an error.

Does it mean that we cannot directly override an method without this mark?

You can still do this, as long as you welcome warnings to live with you 😏

Is this conflict with PR #67777 that introduces abstract?

No, since you cannot annotate an abstract method as virtual because it is born to be overridden; while virtual methods are ones that are optional to be overridden, which means that you can also omit overriding the method in subclasses. A virutal mark cannot be used for an abstract method.

Is there any difference between an abstract method and a virtual one?

An abstract method must be overridden in a subclass, with any kind of implementation inside the function body, while virtual methods (or we can say all custom methods defined in GDScript) are optional to be overridden in subclasses.

Methods are all overridable already, so isn't this pointless?

No, this is not a pointless pr. As said before, this is a pr that helps make overriding a method explicit. Especially if you have checked #103768, you may realize that _-prefixed methods will be confusing when it comes to whether it is virtual+private or private only. This mark clarifies this and separate them apart from each other.

Preview and demo

class A:
    func test():
        pass

class B extends A:
    func test(): # Warning: trying to override a non-virtual method, which will lead to unexpected behaviors.
        pass

If you mark test() as virtual

class A:
    @virtual func test():
        pass

class B extends A:
    @override func test(): # Safe without the warning.
        pass

Notice that you are also supposed to add @override to explicitly annotate that the method overrides the virtual base method. Otherwise, a warning will be thrown.

[Jesusemora]

This would be complete if the overriden methods were highlighted in a different color

[Lazy-Rabbit-2001]

This would be complete if the overriden methods were highlighted in a different color

I don't think this is necessary, as we already have an icon that hints the current method has overridden one from the base class - a blue arrow at the left of the line (line gutter, iirc)

[geekley]

I think having abstract as keyword but @virtual as annotation is very inconsistent, as these 2 concepts are very closely related.

Honestly, making it a key word sounds better than turning it into an annotation. However, being a key word means forced checking and forced error if you don't follow the rule, which greatly breaks compatibility with previous versions of Godot. To maintain the compatibility at the most extent, annotation doesn't sound a disaster for old scripts, so I selected making it an annotation instead of a key word with an error.

I don't understand this. Won't compatibility break either way? GDScript (unfortunately) raises error on unrecognized annotations, so annotations are already not forward-compatible. How is it any different than a contextual keyword? Why does making it a keyword mean forced error? Why can't it just be a keyword that raises a warning by default?

[Lazy-Rabbit-2001]

I don't understand this. Won't compatibility break either way? GDScript (unfortunately) raises error on unrecognized annotations, so annotations are already not forward-compatible. How is it any different than a contextual keyword? Why does making it a keyword mean forced error? Why can't it just be a keyword that raises a warning by default?

Being a key word means that virtual is highly grammar-related, i.e. closely syntactic, which means that any unintended way against the grammar of GDScript should be an error rather than a simple warning. Btw have you ever seen any key word in GDScript, even in other languages, that throws warning when an unintended behavior is detected? Annotation, however, is not that syntactic as being a key word; it only modifies the detail of the grammar and any unintended way can be a warning or an error. Meanwhile, annotation + warning allows users to define how overriding a non-virtual method should behave. Users can shift the warning to an error if they do so, or ignore the warning completely, via Project Settings.

If you set this to a key word, yep it's literally compatibility-breaking, but we don't like to see that, unless we have something that upgrades your codes to fit the new grammar, but who prefers that? What's more, all methods, as I've said, are born with ability to be overridden by subclasses, without explicit annotations. So if the annotation becomes the key word, this won't accord with the nature of GDScript. Being an annotation, however, doesn't break the nature as you can still do that, except that you will receive a warning which doesn't break runtime and compilation.

Last, I don't think this breaks the forward-compatibility, since as I've said before, warnings DOES NOT break runtime and compilation. As long as users can still compile the scripts and run the game as expected, I think that's ok and enough.

[HolonProduction]

I will add this here, since the linked proposal doesn't seem to reflect this exact proposed implementation. If we are adding an explicit @virtual annotation IMO we have to add an explicit @override annotation as well, for readabilities sake. Not everyone is paying attention to the gutter when reading code, and the information on whether something is overriden is quite important. I don't see any reason to require it, or make it part of the warning system at all, but I think it is required as a cosmetic option to keep code readable.

[Lazy-Rabbit-2001]

I will add this here, since the linked proposal doesn't seem to reflect this exact proposed implementation. If we are adding an explicit @virtual annotation IMO we have to add an explicit @override annotation as well, for readabilities sake. Not everyone is paying attention to the gutter when reading code, and the information on whether something is overriden is quite important. I don't see any reason to require it, or make it part of the warning system at all, but I think it is required as a cosmetic option to keep code readable.

what about a method that overrides base virtual one without explicit @override?
P.S. Ok, we can maybe add a new warning (set as error by default) for this. And if a method is overriding a virtual one, without explicitly declare @override, a warning will be thrown to tip you to add this.

[Lazy-Rabbit-2001]

Ok, I found a situation where the gutter doesn't help while @override works better:

extends RefCounted

class A:
    var _t=1

    @virtual func _test() -> void:
        _t=2

class B extends A:
    @override func _test() -> void: # No gutter at the left side. Believe it or not.
        pass

Inner classes in the same script with one overriding the base method whose owner is also one of the inner classes will not have any gutter at the left side. In this case, @override helps better.

[geekley]

@Lazy-Rabbit-2001

have you ever seen any key word in GDScript, even in other languages, that throws warning when an unintended behavior is detected

Dunno about GDScript, but it's a common pattern in e.g. C# analyzers to do exactly this: requiring code be written a certain way, with warnings if it isn't, like when a language keyword is optional but you want to make it mandatory for code consistency. For example, in C# private is optional (it's the default) but you could make it "mandatory" with a warning, if you want, in the analyzer (EDIT: I thought this specific case was in the default analyzer, but apparently not?). Another example is forcing let instead of var in JS. This kind of thing seems precisely the intended use case of this PR: code clarity.

I don't think this breaks the forward-compatibility

You misunderstood what I said. I'm not saying this specific PR breaks BACKWARDS compatibility. Regardless of whether it's annotation or keyword, old code will run on the new version just fine.

I'm saying annotations in general (in GDScript) are, like keywords, not FORWARD compatible -- meaning new code with an unrecognized new annotation/keyword won't run in older versions, it raises an error. This is different from e.g. #pragma directives in C/C++/GLSL, which are just silently ignored if unrecognized (allowing to introduce a different kind of compatible optional behavior without ugly hacks like "special comments" or "use strict"-style dangling strings).

Because of this, I'd argue the difference between annotations and keywords is not really a matter of warning vs error, they are basically the same, as in they work the same (so much so many keywords in Godot 3 became annotations in 4).

What to me seems to be the main difference is more that most/all annotations refer to Godot-specific features (e.g. @tool @rpc @export* @onready @icon even @static_unload) whereas modifier keywords are more about features of programming languages in general (think static abstract protected override virtual readonly).
This must be why some keywords in Godot 3 were moved to annotations but not others.
Of course, there's also the @warning_ignore* exceptions but this makes sense as an annotation for a different reason: it needs to fit in many different places syntactically, plus it takes a parameter.

[dalexeev]

We've discussed the differences between keywords and annotations in the past, trying to find a universal criterion for whether a particular language construct should be a keyword or an annotation. Various options were proposed:

• Keywords for common programming language features, annotations for Godot-specific things.
• Keywords for declarators, annotations for their modifiers.
• Keywords for things that are meaningful from a parser and/or runtime perspective, annotations for editor metadata.

In the end, we couldn't find such a universal criterion, and decided to decide on a case-by-case basis. Some notes from the discussion are documented here.

In this case, I would favor @virtual and @override annotations over keywords. Even though we decided that abstract should be a keyword. Because abstract makes class instantiation impossible and can also affect the syntax of abstract methods (bodyless functions). Whereas @virtual and @override do not significantly affect either the parser or the runtime, they only help you avoid semantic errors. Unlike C++, where virtual and override affect method call dispatch.

[Lazy-Rabbit-2001]

For those who want to know what's the difference between an abstract method (#67777) and a virtual method, I updated the top post and you can check it for better understanding to these two, especially for those who have never touched with or dug in much deeper in this field.