godot: GDScript cannot override Godot's native functions
Godot version
4.0.dev 7791599d5bcfdbae0d2a96e5fc13d9021ec820be
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
About this issue
- Original URL
- State: closed
- Created 3 years ago
- Comments: 58 (39 by maintainers)
@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_classare 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.All methods in custom classes are virtual by default.
They are not special. You can extend them and override their virtual methods. Every single script extends some class, so making them non-extendable would make the engine literally unusable.
No, it was done for performance. Calling methods in GDScript 2.0 is much faster thanks to that.
And GDScript does support overriding virtual methods like every OOP language. The behavior in 3.x was a convenient bug that went against the OOP, because it allowed shadowing methods that aren’t virtual. The methods that you can override are marked as such in the documentation:
Because it’s bugged. It’s the only reason why this issue is still open.
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.
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
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
callon 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 usescalleverywhere), 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.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 🤷
I don’t understand why you need an empty object or a singleton.
Shouldn’t it be
_set? I don’t recall the engine expectingsetto be overridableHere are some numbers.
Script 1 (overwrites a function)
Script 2 (calls script 1)
Execution time in 3.x
release_debugbuild: 210msExecution time in 4.0
debugbuild: 140msIn 4.0 this executes 33% faster, despite I used a debug build which comes with severe speed handicap. If we compared official builds, the difference would be even greater. This kind of optimization would be impossible if functions were overwritable.
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.setorNode.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 replaceset_ownerand 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
callorcall_deferred):This prints:
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.
It doesn’t because of the
setgetfunctions that belongs to theObjectclass.I understand, but prefixing any function with
_has a meaning to me. Since there is noprivate/publickeywords, a function called_testwill be private, where a function calledtestwill be public.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:
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.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:
In 3.x you can “override” non-virtual methods, but only for GDScript, not C++. That is, if some method is called directly, the C++ original will be called, not your GDScript override (this does not apply to calling via
Object.call). This is especially noticeable when trying to override a property setter or getter.Just chiming in to say in my issue #69765 overriding would be really nice for plugin development. I wanted to have a
RandomTimerclass (see here for Godot 3 implementation) with the same methods as the build inTimerso that it can be used in the same way. I don’t really want to tell users “This is a timer, but make sure to call start_random instead of start, otherwise it won’t work as expected”. Lots of room for error there.I feel
@overridewould not be the right name for this though, the problem is to allow shadowing core class methods.@overridewould make it look like it makes it work like a virtual function override, when it doesn’t works in all cases, and has limitations. Maybe something like@overwriteor@shadowor@scriptonlyoverride, or whatever retains the fact it’s not a typical override like every other language.I know it is outstandingly difficult to reimplement, but it is worth reconsidering this stance, at least by adding way more overridable methods, since these “issues” are being reported by the userbase fairly frequently, as it often is limiting, compared to 3.x.
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.
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.@tx350z It was a bug/inconsistency that you exploited
Yes, and this was never going to work correctly.
I disagree with the claim “there is little reason to support it”. I can’t think of a single use case where I want the engine to call my custom functionality. So I’ll depart this discussion.
For my needs I’m now building an editor plugin that automates code generation of wrapper classes around built-in classes so I can extend functionality as needed. The result will no doubt be slower than having built-in support but it is what it is.
P.S. My hope was 4.0 would support polymorphism but that’s not likely to happen if simple method overrides will never be supported.
Thats not true! I still using override built-in methods on Godot 3.5.1 as build a mock/spy by fuction doubling and calling than the original implementation.
@tx350z It didn’t actually work in 3.x. Bringing it back, if it even was sensible, would only create an illusion that you can override built-in methods when in fact you cannot. And shadowing is not a feature of any language, it’s a bug or a code smell at the very least.
@tx350z It didn’t exist in 3.x. Unless the method is registered in a way that allows extending it with scripting, which few are, the engine cannot call into it, it has no idea that you’ve overridden the method. What might have worked is scripts calling methods of other scripts directly. But it wasn’t really overriding the method, it was indeed just shadowing it for the scripting side of things, while the engine was not aware that you might’ve overridden it.
So it was a bad idea to do it, even if it could be perceived as working in some practical scenarios.
3.x:
4.0-dev:
In both 3.x and 4.0-dev you can’t really override non-virtual methods (to have C++ call them), this only works for GDScript.
I’m not sure if the behavior in 4.0 should be the same as in 3.x, or if we should completely disallow overriding non-virtual native functions. But if we choose the first option, then we should add a warning and/or make another indicator (blue arrow).
Given that
call("native_func_name")andnative_func_name.call()call the GDScript method, can we classify this as a bug?No they don’t. Unless you say that C++, C#, Rust or any compiled language are not good. They require you to explicitly mark overrideable functions as virutal and most of their functions aren’t virtual.
Sorry, but any good programming language allows us to override methods to extend existing functionallities. Now all of a sudden not allowing it anymore will push many devs over the head. The argumentation to write an equivalent function with underscore that calls the core function is total nonsense. This just creates useless code noise and forces us to define pseudo names for existing core functions. A simple example, you want to log if a child node was added.
Godot 3:
for Godot4 it would look like this
now we have two functions with almost the same name
_add_childandadd_child. Furthermore it leads to confusion when providing such an API, should i call_add_childoradd_child.The GDScript implementations are entirely different between 3.x and 4.0, so it’s not possible to compare performance with this change alone.
This would result in bugs that only occur in projects exported in release mode, which are difficult to track down. (Remember the whole dangling Variant thing?)
We should aim to have fewer behavior differences between debug and release, not more 🙂
Hello I don’t understand why you want to restrict developers so much, there is no real reason for that. Whether shadowing or override this is blurring in my eyes.
Overriding methods/functions is still a valid option in many languages, and was possible in Godot 3.x. Instead of responding to the users, it is worsened, I would like to refer only to the sorry topic private/protected/public accessors there is stubbornly blocked. What speaks against it? It is only important that if you overwrite a function a warning, error (configurable) gets as feedback in the editor to avoid accidental overwrites.
So please don’t bring in such regressions! This makes me frustrating.
Here is my suggestion a) allow function overrides b) show a warning for overwritten functions c) introduce an annotation
@overrideto mark as allowed override and disables the warning d) an editor setting to configure accidental overrides as a warning or error e) finally respond to the users and introduce the long requested private/protected accessors @private, @protected This would also help to disallow overwrites if explicitly requested by the devI code like this:
It will report error:
Enum member Input has the same name as a global classI think this was valid code, because I can use it like this:Test.Type.InputThis was changed in #54676. It’s being reconsidered / further discussed in #54949.
If you use
_setand_getfor Godot it means adding extra code togetandset, this is one way you can achieve what you want to do here. This is how Godot expects you to “extend”getandset, basically (not override it, but that’s not really what you need it seems?). You can write all the code you wanted in_getand_set, and Godot will still allow you to callgetandsetAND take the_getand_setinto account.basically:
I understand why you’d expect to do that with
getandsetdirectly 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.