GDScript cannot override Godot's native functions

[xsellier]

Godot version

4.0.dev 7791599

System information

Linux binogure 5.14.0-2-amd64 #1 SMP Debian 5.14.9-2 (2021-10-03) x86_64 GNU/Linux

Issue description

I don't know if it is normal, but I cannot override any godot's native functions using GDScript.
It never calls the gdscript function but the godot's core function instead.

Steps to reproduce

• Create a new project
• Attach this script to the main node:

extends Node

var __data = {}

func _ready():
    set('random_key', 'random value')

func set(key, value):
    print('Setting a new value')
    __data[key] = value

The set function is never called, and there is no warning/error in godot explaining that is prohibited.

Minimal reproduction project

No response

[xsellier]

Seems to be related to: #40288

[Zylann]

Shouldn't it be _set? I don't recall the engine expecting set to be overridable

[xsellier]

I expect to override any engine's function if I want to do it. Or at least the editor should tell me that I cannot override such functions and explain why.

Another solution would be, having an empty object which hasn't had any functions but can be extended as a Singleton.

[Zylann]

I expect to override any engine's function if I want to do it

I'm afraid that's not how things work, as it has probably been said in other issues, to achieve that the engine would have to use slow call on absolutely every function that ever faces the script API, which likely won't happen as it's too much work and downsides to it (unless maybe the engine code and scripts were both in Java?)

I remember that in Godot 3 so far, if you write a function with the same name as an existing function, Godot lets you "shadow" it, although it is not true overriding. For example if you write "get_position" in Node2D, GDScript might behave as if you overrode the function (because GDScript uses call everywhere), but the engine will not use your function when getting the position of the node. Because for this to work, the C++ code has to expect and handle the fact the object could have a script attached to it with a function on it.

the editor should tell me that I cannot override such functions and explain why.

So there should be a decision wether or not Godot should consider it an error. I personally think it should, but like many edge cases like this one, other people might have decided to exploit it and want it to remain maybe 🤷

Another solution would be, having an empty object which hasn't had any functions but can be extended as a Singleton.

I don't understand why you need an empty object or a singleton.

[xsellier]

I don't understand why you need an empty object or a singleton.

It would be great to have an empty class, that you can use as a singleton so you can have any functions you want. That's the bare minimum to me. For example:

extends EmptyObject

var __data = {}

func _ready():
    set('random_key', 'random value')

func set(key, value):
    print('Setting a new value')
    __data[key] = value

[Zylann]

It would be great to have an empty class, that you can use as a singleton so you can have any functions you want.

I still don't understand the relation with overriding functions, but you basically describe an utility class with static functions:

util.gd

class_name Util
static func foo(a, b) -> int:
    return a + b

bar.gd

func _ready():
    print(Util.foo(3, 4))

And I dont understand the example you give with it... the example sounds like just a dictionary.

var data = {}
data.set("hello", 42)
print(data.hello)

And wanting both in the same object can be done with a dictionary inside an autoload.

[xsellier]

It has to have some custom code on set and get, with warnings, exceptions and specific behavior. I simply put the simplest code there.

Here a more complete code

extends Node

var __global = {}

signal key_updated(name)

func _ready():
  reset()

func reset():
  # Load the default locale by default
  set('locale', OS.get_locale().left(2))

  set('soundfx_volume', 0.5)
  set('music_volume', 0.5)

func set(key, value):
  if __global.has(key):
    logger.debug('Overtwriting key %s', [key])

  else:
    logger.debug('Writing key %s', [key])

  __global[key] = value

  emit_signal('key_updated', key)

func get(key, default_value = null):
  return __global[key] if __global.has(key) else default_value

[Zylann]

And that example code doesn't work? It's basically still a fancy wrapped dictionary. You can put that in an autoload and it should work. Maybe the only difference is, with your code you'd have to call get and set (or call them something else if Godot prevents you), but if you use _get and _set you can do it, or even write with .varname syntax.

[xsellier]

It doesn't because of the set get functions that belongs to the Object class.

I understand, but prefixing any function with _ has a meaning to me. Since there is no private/public keywords, a function called _test will be private, where a function called test will be public.

[Zylann]

If you use _set and _get for Godot it means adding extra code to get and set, this is one way you can achieve what you want to do here. This is how Godot expects you to "extend" get and set, basically (not override it, but that's not really what you need it seems?). You can write all the code you wanted in _get and _set, and Godot will still allow you to call get and set AND take the _get and _set into account.

basically:

func _get(key):
    if key == "foo":
        return 42

data.get("foo") # gets 42 because Godot will call `_get` internally
data.foo # gets 42 too

I understand why you'd expect to do that with get and set directly so badly in principle (as many other overridable functions existing under a different _ name), but it isn't how Godot's scripting API works. As of right now, a native function cannot both represent a regular function and a virtual one. This would need some work to have both, but in the meantime your use case can be solved, except for the internal naming, which is relatively minor (i.e code using your class can still be written as if the names were like you wanted). Not saying the issue shouldn't be investigated, but that there is an easy workaround.

[xsellier]

Still doesn't solve my naming issue.

[akien-mga]

This was changed in #54676. It's being reconsidered / further discussed in #54949.

[StraToN]

@akien-mga #54949 seems to concern member variables only, not functions, which are the problematic point here.

[KoBeWi]

You are supposed to override only virtual functions and they are all prefixed with _ (e.g. _ready(), _get_drag_data() etc.). We should give a clear error when trying to shadow a built-in function, but non-virtual overriding likely won't happen.

[xsellier]

You are supposed to override only virtual functions and they are all prefixed with _ (e.g. _ready(), _get_drag_data() etc.). We should give a clear error when trying to shadow a built-in function, but non-virtual overriding likely won't happen.

Thank you for those explanations. However it would be better that behavior is describe somewhere (since it didn't behave that way in Godot 2/3), and also Godot itself should raise an error to warn the user. atm, this is not user friendly.

[CsloudX]

I code like this:

class Test
    enum Type{Input,Output}

It will report error: Enum member Input has the same name as a global class
I think this was valid code, because I can use it like this: Test.Type.Input

[xsellier]

Another use case:
set_owner and get_owner, those functions are defined by godot so not usable in GDScript.
Wasted time because:

• There is no warning when we overwrite a godot's native function
• There is no way to @override such functions

[akien-mga]

To repeat what @Zylann was explaining, not all functions in Godot can be overridden. That's the same in any language, some functions are virtual, others are not. In Godot's scripting API there's another level of indirection as some functions might be virtual in C++ but that doesn't transfer to the scripting API. "Virtual" methods are implemented by calling methods via StringName, which is much less efficient than what a C++ compiler can do for you with the virtual function table.

So there's no intention to support overriding any method such as Object.set or Node.set_owner. These methods are used by the engine API internally and making them overridable would lead to terrible performance, and a very simple footgun for users to break the API by changing the way things are expected to behave internally (i.e. if you were to replace set_owner and expect it to be called internally, you'd have to make sure that you reimplement exactly what the C++ method does - which might not even be possible).

Now, we should indeed add explicit warnings whenever you shadow such a method unknowingly, as the way it behaves is not intuitive at all (and can still lead to bugs for internal engine calls using call or call_deferred):

extends Control

var test = 0

func set(property, value):
	print("Setting '%s' to '%s'." % [property, str(value)])
	set(property, value)

func _ready():
	test = 1
	set("test", 2)
	self.set("test", 3)
	$".".set("test", 4)
	call("set", "test", 5)
	$".".call("set", "test", 6)
	call_deferred("set", "test", 7)
	$".".call_deferred("set", "test", 8)
	get_script().set("test", 9)

This prints:

Setting 'test' to '3'.
Setting 'test' to '5'.
Setting 'test' to '6'.
Setting 'test' to '7'.
Setting 'test' to '8'.

IMO it's far from obvious, and not something we should aim to support.

So I would ensure that shadowing an internal function raises a warning, as done in #54949 for variables.
IMO this could even be an error, but given the reactions to #54676, I guess we can start with a warning and let people who still want to play with fire mark the warning as ignored and accept they're using an unsupported feature.

[dalexeev]

I think we have two options:

1. Prohibit "overriding" non-virtual methods (this should produce an error).
2. Allow non-virtual methods to be "overridden" (as in 3.x), but generate a warning about it. And also we can change the method override indicator (blue arrow) for this case.

[YuriSizov]

I can't think of a single use case where I want the engine to call my custom functionality.

Whether you generally want to or not, if it was true overriding then the expectation from a programmer would be that the engine would call the override and not the original. Of course, not every public/exposed method is called by the engine, but some do, and consistency is expected. Allowing just shadowing wouldn't be very OOP of us, would it?

And overrides in general are allowed, but we have strict rules that the method should be declared as virtual. It's just not all built-in methods are, for one reason or another (already explained in the discussion above). This is all completely within the OOP principles.

[tx350z]

Whether you generally want to or not, if it was true overriding then the expectation from a programmer would be that the engine would call the override and not the original. Of course, not every public/exposed method is called by the engine, but some do, and consistency is expected. Allowing just shadowing wouldn't be very OOP of us, would it?

And overrides in general are allowed, but we have strict rules that the method should be declared as virtual. It's just not all built-in methods are, for one reason or another (already explained in the discussion above). This is all completely within the OOP principles.

Four Pillars of OOP:
1 Inheritance: child classes inherit data and behaviors from the parent class with the ability to extend or replace parent behaviors
2 Encapsulation: containing information in an object, exposing only selected information
3 Abstraction: only exposing high-level public methods for accessing an object
4 Polymorphism: functions bearing the same name, but each may have different behaviors

[akien-mga]

Reopening for now as #72487 was reverted.

[tx350z]

Consequences:

● String get_class() const

Returns the object's built-in class name, as a String. See also is_class().

Note: This method ignores class_name declarations. If this object's script has defined a class_name, the base, built-in class name is returned instead.

SCRIPT ERROR: Parse Error: The method "get_class" overrides a method from native class "Object". This won't be called b)

[YuriSizov]

Yes, and this was never going to work correctly.

[tx350z]

Even though it did work as needed in 3.x. At this point it appears none of my 3.x apps, libraries, and tools are upgradable to 4.x.

[Zireael07]

@tx350z It was a bug/inconsistency that you exploited

[dalexeev]

Even though it did work as needed in 3.x.

In 3.x, this is also not overriding, but shadowing. You're just creating another method with the same name, which is definitely confusing. In 4.0, an additional problem is that even GDScript can't always distinguish between these methods. We could fix this, but it would hurt performance for no good reason, since the behavior in 3.x is still not correct and expected by users ("I've overridden queue_free(), why isn't it called, when the node is removed by the engine?").

Discussion in Rocket Chat

Duplicate another my comment:

---

Why it's done:

• The engine doesn't call your "overrides" in most cases (unless it calls them via call()). This behavior is also present in 3.x, it's not a bug, it can't be fixed without a huge performance hit, it would introduce a lot of new bugs due to the violation of the engine's expectations when the user overrides the method incorrectly.
• Unlike 3.x, the "override" call may not happen even in GDScript, due to type collisions (in some cases, GDScript calls a native method for optimization, unless you work around it with a cast or call()). For this reason, non-virtual native methods should never be "overridden", even if you know about the previous point and don't worry about it.

A few tips:

1. You can disable or ignore this warning, but correct behavior is not guaranteed.
2. This warning does not apply to native virtual methods and any custom methods. You can override them.
3. If you need to customize some engine method, you should create a wrapper with a different name, not "override" it.

[tx350z]

I think I found a solution for my needs in HxGodot. As much as I've come to love coding GDScript, the 4.0 update has too many new barriers to overcome.

[MikeSchulze]

@GodotDevs it an always upcomming topic and you can see the useres want to have the possibility to override a core function.

From technical perspective so far i anderstand, the core change in GdScript 2,0 is instead of calling functions by name you call it via function pointer.

Why you not provide a @override annotation and check when a class is loading to use a function table instead of function pointer to achive it?

[YuriSizov]

@MikeSchulze Again, this didn't work in Godot 3.x either. It is not related to GDScript, it's just how Godot is, and always was. If a method is not made to be overridable, you can't override it, neither from scripting, nor from extensions. The only change in GDScript that is relevant here is that we now tell the user that what they are trying to do is wrong and is not going to work.

There are no plans to allow overriding any core function, because it's impractical from the performance perspective. And once again, this is not a decision of the GDScript team, it's a core decision.

Furthermore, cases like get_class are a different kind of issue entirely. Users don't want to override this method, they were trying to override it because it doesn't work as expected with custom classes out of the box. We need to address the underlying issue here instead of pleading to make the entire core extensible.

[sancho2]

In 3.x, this is also not overriding, but shadowing. You're just creating another method with the same name,

It is further confusing since the error message you get in 3.5 when you don't match the function signature is:
"The function signature doesn't match the parent" (specifically the Timer method set_wait_time)

Since we can only override virtual native methods, how do we know which native methods are virtual?

[KoBeWi]

Virtual methods all start with underscore (_) and are also labelled as virtual in the documentation.