godotengine / godot-proposals

Godot Improvement Proposals (GIPs)
MIT License
1.16k stars 97 forks source link

Allow stricter base types when extending a script #698

Open Dragoncraft89 opened 4 years ago

Dragoncraft89 commented 4 years ago

Describe the project you are working on: A Mario party clone, with plugin support

Describe the problem or limitation you are having in your project: I'd like to add an interface for plugins. To make this pleasant for plugin authors, I'd like to create a script that exposes a bunch of properties, which can then be extended to implement the plugin logic.

The problem is, that when inheriting from a script, the extended class is being determined by the script you're inheriting from. Therefore you can't restrict the type the script needs, to get access to specific properties.

For example if the base script extends from Node, then it can be used by any type that derives from Node, which is what I want. But any script that extends this base script will be limited to the functionality of Node, even if the Node it is attached to is, e.g. a Spatial.

Of course, I can let the base script extend from Spatial, which will give all scripts that extend from this the ability to use methods from Spatial, but limits what node types the script can be attached to, even though the base script doesn't need that specific node type.

Describe the feature / enhancement and how it helps to overcome the problem or limitation: Let scripts specify both a node type and a script to inherit from.

So I could define the node agnostic properties in the base script and specify the needed type in the extended script.

Describe how your proposal will work, with code, pseudocode, mockups, and/or diagrams: One could allow multiple uses of extends at the top of the script file, e.g.

a.gd:

extends Node

func _ready():
    print("do stuff...")

b.gd:

extends "res://a.gd"
extends Spatial

func _ready():
    self.translation = Vector3(0, 1, 0) # Here we can use properties of Spatial and are not limited to Node by a.gd

But because having multiple extends keywords in a single file may be confusing, a better option would be to deprecate "extends" for built in types and replace it with a base_class keyword.

So that it can be written like this:

a.gd:

base_class Node

func _ready():
    print("do stuff...")

b.gd:

extends "res://a.gd"
base_class Spatial

func _ready():
    self.translation = Vector3(0, 1, 0)

If a script does not specify a base_class, then it will automatically inherit the base_class of the script you're extending.

If this enhancement will not be used often, can it be worked around with a few lines of script?: It can be worked around by adding a node of the specific type below and attaching the script on that one, but it's not really a comfortable way of doing things

Another way would be to outsmart the type system:

var body = self
body.some_func_on_body()

But this is confusing to read and does not benefit from type checks

Is there a reason why this should be core and not an add-on in the asset library?: This is a gdscript language feature and therefore can't be made into an add-on

Dragoncraft89 commented 4 years ago

Another use case, found on godotengine QA

bojidar-bg commented 4 years ago

Related: https://github.com/godotengine/godot/issues/23101

Dragoncraft89 commented 4 years ago

@bojidar-bg I don't think the issues are related at all.

The linked issue enables a kind of "multi-inheritance", to compose game logic.

While this issue is about a type checking limitation. As it would already work, when you trick the type system like this:

var body = self
body.some_func_on_body()

And I haven't seen that being addressed in the linked issue.

Although I can see, that how those proposals would work together is necessary to think about if both are to be accepted.

Dragoncraft89 commented 4 years ago

Maybe I should explain the current limitation a bit more:

Currently, code like this fails the type check although it would be perfectly legal

a.gd:

extends Spatial

func _ready():
    pass # Do some generic setup here

b.gd (attached to a KinematicBody):

extends "res://a.gd"

var direction: Vector3 = Vector3(1, 0, 0)

func _process(delta):
    self.move_and_slide(direction) # Here the error is: `The method move_and_slide isn't declared in the current class`

The editor is right, as a.gd extends from Node and b.gd extends from a.gd and therefore transitively extends from Node as well.

But it is more limiting as it would necessarily be, because if you changed b.gd like this:

extends "res://a.gd"

var direction: Vector3 = Vector3(1, 0, 0)

func _process(delta):
    var body = self
    body.move_and_slide(direction)

It would work, as the editor looses the typing information necessary to deduct that error when assigning to body

And it would run without issue as well, because KinematicBody (which b.gd is attached to) has the method move_and_slide

bojidar-bg commented 4 years ago

I remember a discussion (not sure where now) about the fact that allowing scripts to be applied to subclasses of the base class is not really safe (as in type safety) and can prevent certain optimizations (namely, caching the function pointer to call). Consider:

### A.gd
extends Node2D

func _physics_process(_delta: float) -> void:
    (get_node("player") as KinematicBody2D).move_and_slide(Vector2(100, 0))

### B.gd
extends Node2D

func move_and_slide(amount: int) -> void:
    print(amount)

So far so good. Both scripts are valid as far as GDScript is concerned. Consider now what happens when someone makes a scene like this:

The "world" and "node_b" nodes are definitely correct, as their types match the base type of the script. However, the "player" node is not, since it breaks the so-called Liskov substitution principle. Namely, it does no longer function correctly as a KinematicBody2D, since its move_and_slide function does not implement the interface defined by KinematicBody2D.

(This might get worse if https://github.com/godotengine/godot/issues/29390 is resolved to completely disallow overriding methods not marked as "virtual".)

There are some ways to patch this issue, for example making it so that (x as Y).z() would always call the original Y::z unless explicitly overriden (e.g. by x having Y as a base class). Unfortunately, this can be quite breaking, impacting practically all places using call/call_deferred/etc for reflection in Godot.

That's where traits come in. Traits allow one to share a snippet of code (such as func move_and_slide(int) above) between different scripts, while validation of interfaces happens when the trait is included in the script with the correct base type.

### B_trait.gd
trait # or however it would be marked as trait
extends Node2D # Can still add a constraint that it extends some base type

func move_and_slide(amount: int) -> void:
    print(amount)

### B_node2d.gd
extends Node2D
trait "B_trait.gd" # or however it would include a trait

### B_kinematicbody2d.gd
extends Node2D
trait "B_trait.gd" # or however it would include a trait
# ^ Error: move_and_slide defined in "B_trait.gd" does not match parent function signature move_and_slide(Vector2, ...)

### B_spatial.gd
extends Spatial
trait "B_trait.gd" # or however it would include a trait
# ^ Error: "B_trait.gd" cannot be applied on a base of Spatial (needs Node2D)

Thinking about it, scripts like B_node2d.gd/B_kinematicbody2d.gd/B_spatial.gd do not really have to exist, if we allow applying a trait to a node directly. In this case, your proposal comes very close to this traits idea I just outlined by replacing extends by trait and base_class by extends. The main difference is that you can extend multiple traits, as opposed to one base script with changed base class.