godotengine / godot-proposals

Godot Improvement Proposals (GIPs)
MIT License
1.1k stars 69 forks source link

Implement removing of class items (methods, properties, signals etc) #1996

Closed me2beats closed 3 years ago

me2beats commented 3 years ago

Describe the project you are working on

Editor Plugins

Describe the problem or limitation you are having in your project

[hereinafter (class) items are class properties, methods, signals, etc]

I noticed that with each new level of abstraction (inheritance), the size of unnecessary methods and properties grows like a snowball

These unnecessary class items

Problem example: a.gd:

extends Reference
class_name A
var a
var a1

b.gd:

extends A
class_name B
var b

c.gd:

extends B
class_name C
var b

Let's say one day I decide that I don't need a1 in C anymore. But I still would like C to extend B because I don't want to create C from scratch. But I can't do that. I need to create NewB (can't simply change B because it's used in some D.. But I don't want create NewB from scratch and I end up creating NewA (just without a1) and NewB (that extends A).

As you can see sooner or later such an approach will lead to the growth of the code base through duplication (copy-pasting) of classes and violation of the DRY principle.

Another example: I use utility classes often, they all extend Reference. In most cases, these classes have only static functions and I don't need to instantiate such classes. But this also means that I don't need many of the methods inherited from Object, such as connect(), disconnect(), etc. The problem is that I cannot create a class using gdscript that does not have these methods.

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

The ability to specify which class items should be removed when the script is created could (at least partially) solve the problem.

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

One way is that a new keyword could be created, for example remove:

class_name C
extends B #b.gd, see above
remove a1

This would mean that property a1 would be completely removed and an attempt to access it would cause an error.

It would be great if this could also work for godot API classes (Object, Node etc.):

class_name Util
extends Reference
remove connect, disconnect, emit_signal

As for dynamically deleting class items (like python del does), I believe that this is unnecessary (in gdscript) and often could be even harmful. This causes more problems than flexibility.

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

It can't

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

I don't think this feature can be implemented as an add-on properly.

Jummit commented 3 years ago

Is there any other language that has a similar feature or syntax?

kintrix007 commented 3 years ago

I kind of agree with this, maybe having a hidden keyword would work? I know people think there are already too many keywords, but for the sake of telling my point I'll just do it with a keyword, since it's the easiest.

hidden var a = 10 <- this var would not be inherited. However I think private does a pretty similar thing in other languages.

Edit: I'm aware this is a little different than what @me2beats wanted, but I think it's a pretty decent alterantive.

Calinou commented 3 years ago

I kind of agree with this, maybe having a hidden keyword would work? I know people think there are already too many keywords, but for the sake of telling my point I'll just do it with a keyword, since it's the easiest.

Now that GDScript supports annotations starting with @, annotations should be used instead of keywords for "supplemental" information like this. See https://github.com/godotengine/godot-proposals/issues/641.

me2beats commented 3 years ago

hidden var a = 10 <- this var would not be inherited. However I think private does a pretty similar thing in other languages.

It seems this is about having private modifiers, (https://github.com/godotengine/godot-proposals/issues/641) I think it would be great to have them as well. This is a bit similar but different. You can't always predict what properties and methods of class X you will need in class Y that will extend it.

The proposed feature would allow to solve this right in Y, no need to change parent X class or create a new one (NewX).

me2beats commented 3 years ago

@Jummit I'll try to find such examples, I'm sure such languages exist.

Python can remove properties and methods with del keyword. Tho as I said above imo this is too "dynamic" (and dangerous) thing.

A better way would be still having ability to remove some methods/properties but in more "static" manner (allowing to do this only when creating a script), which would be better for autocompletion, readability etc.

me2beats commented 3 years ago

The metaphor here is pretty simple: if you add something, there should be a way to remove it as well. Now inheritance in gdscript constantly adds new functionality with each new level (by adding methods and properties), or modifies the existing functionality (overriding methods; property overriding has not yet been implemented, there is https://github.com/godotengine/godot-proposals/issues/338). In this case, I think it would be flexible to have some kind of "reverse mechanism", the removal of some functionality.

KoBeWi commented 3 years ago

I'll try to find such examples, I'm sure such languages exist. Python can remove properties and methods with del keyword. Tho as I said above imo this is too "dynamic" (and dangerous) thing.

Ruby allows you to do pretty much everything with a class, including undefining methods (even for single instanced objects) or overriding behavior of core types. It's pretty dynamic too though.

dalexeev commented 3 years ago

Is there any other language that has a similar feature or syntax?

This is possible in the Eiffel language (the undefine keyword). However, unlike GDScript, Eiffel is strongly statically typed and supports multiple inheritance.

Personally, I am against adding this feature, because it violates the Liskov substitution principle.

class A:
    var x: int

class B extends A:
    undefine var x

func f(a: A) -> void:
    print(a.x)

func _ready() -> void:
    f(A.new()) # Ok
    f(B.new()) # Error
willnationsdev commented 3 years ago

I agree with dalexeev. Rather than undefining something from a base class, it seems more prudent to change the nature of the class definition in the first place. godotengine/godot#23101 originally introduced the idea of a GDScript trait system to enable you to define partial classes and merge them together. That kind of additive class definition would be a much smoother experience than having ability to randomly add and remove elements of definitions at any given time. vnen has mentioned plans for a trait system in GDScript, but it wouldn't come until 4.1+. He has yet to open a formal proposal detailing his thoughts on the implementation though (likely just because with everything else for 4.0).

me2beats commented 3 years ago

closing because implementing this proposal would violate OOP principles, and in favor of trait system