godotengine / godot-proposals

Godot Improvement Proposals (GIPs)
MIT License
1.26k stars 101 forks source link

Add `sealed` for GDScript #12591

Closed Lazy-Rabbit-2001 closed 4 weeks ago

Lazy-Rabbit-2001 commented 4 weeks ago

Describe the project you are working on

Godot programming

Describe the problem or limitation you are having in your project

The topic has been talked for periods, and eventually lit by the chat

During GDScript programming, we often expect some classes unable to be extended, which is also called sealed classes in some languages like Java and C#. Sealed classes guarantees that a class is unextendable and trying to inherit a sealed class will lead to compiler error.

As we have non-inheritable abstract native classes in Godot, why not having non-inheritable classes for GDScript?

Describe the feature / enhancement and how it helps to overcome the problem or limitation

Add a keyword sealed, which must and can only modify extends, class_name or class, following abstract if there is the latter.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

# my_sealed_class.gd
sealed class_name MySealedClass
...

# test.gd
class_name Test extends MySealedClass
# Error: The class 'MySealedClass' is sealed and should not be extended.

Note that this does not require changes on the core of engine, nor on vm (runtime check). This is done only in analyzer and parser.

Unlike sealed in C#, you can define a sealed abstract class as a lib class, but you have to ensure that all methods in the abstract class are concrete and static.

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

No.

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

Addons cannot get access to scripting language module.

Lazy-Rabbit-2001 commented 4 weeks ago

Note that I don't plan to add this keyword for functions, as this requires one of my pr to be synchronized and tweaked here and there. Therefore, there is only support for sealed classes rn.

AThousandShips commented 4 weeks ago

Related:

Given the rest of the naming syntax I'd say final is better than sealed, based on c++

Lazy-Rabbit-2001 commented 4 weeks ago

Given the rest of the naming syntax I'd say final is better than sealed, based on c++

I looked up the Java syntax and, yk, sealed also supports determining several classes that are allowed to be inherited. Maybe this would be a potential advantage compared with final which does not support this.

vnen commented 4 weeks ago

The idea of sealed for core comes mostly from Java/Kotlin, where it means it can be extended with some limited scope (all inheritors are known at compile time). So it relates how it works in core because you can extend it just fine internally in C++, but you cannot add a script or extension.

For GDScript this concept does not really make sense because scripts are compiled individually when loaded. There's not a notion of "compile time" in the broader sense because it can happen at any time during a running game.

We could have it work like Java in which you can list which classes can inherit from it. I just don't see the point. If it's not solving an actual problem, it should not be added.

Now, final is a bit more feasible because it fully forbids inheritance in all cases (and yes, we can solve that in the analyzer). Still, I don't see a particular use case for it, so not sure why bother adding it.

Remember that in Godot we implement something because it's a solution to a problem, not just because it's nice. You mention that "during GDScript programming, we often expect some classes unable to be extended", but you haven't provided any concrete example. I honestly doubt it happens "often". If you do have use cases for it, please include them in the proposal.

AThousandShips commented 4 weeks ago

I'd say restricting individual methods makes a lot more sense, but I feel that this restrictive approach in general is a bit alien to GDScript

As a hard feature, rather than an annotation with a warning, this would I'd say only really be useful if you ship an add-on, but then I don't see that it's all that useful either, what is the goal?