Extending engine classes in GDScript or C# not working

[mrimvo]

Godot version:
3.2.1

OS/device including version:
All platforms

Issue description:
Extending engine classes is essentially broken.

No control over call of super functions
In the current state, godot will call all the super classes' implementations of a function and the user has no way to opt out of that behavior, as discussed in this issue. I completely understand the reasoning for this behavior and agree on that. While for user-written classes, there is a workaround:

# override this in your subclass
func _on_process(delta):
    pass

func _process(delta):
    _on_process(delta)

The problem is, we cannot apply this workaround on engine classes. As a result, extending engine classes in GDScript is barely supported in the moment, as it is not possible to override functions of engine classes properly.

Another problem is that some functions work like this, and others don't. There is a lack of consistency.

I thought quite a bit about it, and from the various options available, I think introducing the new keyword override might be the best option:

override func _process(delta):
  if we_want:
    ._process(delta)

So essentially, override will prevent the super classes' implementations of this function to be called. The user opts-in to that behavior and is then in full control if/when/how the super method is called. The "squirrel programmer" is still fine and everybody else used to OOP got his powers back.

More as a site note: the second reason why extending engine classes is essentially broken in godot in the moment is that it is impossible to extend virtual engine classes and provide a GDScript-implementation of these interfaces. This is discussed here and here, and affects all virtual engine classes exposed to GDScript.

How to fix
In an ideal world, I would love to have the override keyword and have full control on extending engine classes. Also, I would like to be able to implement virtual engine classes. Naturally, this would increase flexibility of GDScript a lot as discussed here. This would essentially fix inheritance of engine classes in godot.

If this is not possible for technical reasons, we should at least add a warning/errors to save the user from some frustration:

• a warning when a user tries to extend an engine class, because what he is trying to do might in fact not work out at all, because he will have no control over if and when super functions are called.
• an error when the user tries to extend virtual engine classes, because this is in fact not working at all
• a warning when the user tries to call super functions like ._process or ._ready, because this would call the super method twice, and this is most likely not intended

Thank you guys for your great work on this wonderful engine. I think it's normal there are flaws at some point, but it's important to save the user from endless frustrating hours trying figure out what's going wrong.

Thank you for considering, I appreciate your thoughts about this.

[Calinou]

I think this is more of a feature proposal than a bug report. This issue should be recreated on the Godot proposals repository while following its template.

[KoBeWi]

What do you mean by extending engine classes? They don't call any of the script methods (i.e. _ready(), _process() etc.), they are there only for script usage. They only have an internal process method, which you can't override. You can only disable it by using set_process_internal(false), which will e.g. stop AnimationPlayers.

[mrimvo]

With the term "engine classes" I refer to classes exposed to GDScript by the engine. Let's take WebSocketClient as an example, an engine class I tried to extend:

extends Node

class MyPeer extends WebSocketClient:
	# tried to override
	func get_available_packet_count() -> int:
		print("get_available_packet_count called")
		return 0
	# tried to override
	func get_packet():
		var packet = .get_packet()
		print("data from server: "+packet.get_string_from_utf8())
		return PoolByteArray()

var peer = MyPeer.new()

func _ready():
	peer.connect_to_url("ws://localhost:8080")
	get_tree().set_network_peer(peer)

The error log gets flooded with:

E 0:00:00.902   get_available_packet_count: Please use get_peer(ID).get_available_packet_count to get available packet count from peers when not using the MultiplayerAPI.
  <C++ Error>   Condition "!_is_multiplayer" is true. Returned: 0
  <C++ Source>  modules/websocket/websocket_multiplayer_peer.cpp:101 @ get_available_packet_count()

The print lines in the derived class get not triggert. Overriding this engine class (a native C++ class exposed to GDScript) is not working. The same is the case for NetworkedMultiplayerPeer a virtual engine class, which can't be extended in GDScript.

they are there only for script usage

If you meant to say "they are not meant to be extended in GDScript", then I suggest to add warnings/errors when the user is trying to do so, as I suggested in my original post.

Of course the preferable way to do it would be to allow extending all classes exposed to GDScript. It provides most flexibility and it would provide an easy way for the user to extend the engine's capabilities without having to recompile godot for every single platform.

[KoBeWi]

Well, any virtual method says in documentation that it's supposed to be overridden (or at least starts with _ to indicate that). We'd better make it more consistent if it isn't already, I don't think a warning is necessary.

btw there was a PR that allowed to override any method #15639

[mrimvo]

I think an error is necessary if the user tries to extend a virtual engine class, because it's not possible to create an instance of such a subclass implemented in GDScript.

Maybe I'm misunderstanding and the broken inheritance I discovered in this example is actually not related to the problem I tried to fix with the override keyword. There might be another problem causing this.

[aaronfranke]

Is this issue still relevant in the current master branch, after the merge of #40598?

[o01eg]

I got same issue with Translation class. Neither GDNative nor script allowed to use own get_message function.

[o01eg]

@aaronfranke Yes, I've tried to use extended Translation in script and it still fails.