Godot: has_method does not work on class

[DavidSichma]

Godot version:
current master

OS/device including version:
Windows, don't think it matters

Issue description:
Investigating #22833 I found out that has_method does not work with classes.
Consider the following script:

extends Node

class Test:
    static func foo():
        print("foo")

func _ready():
    Test.foo() # works as expected
    print(Test.has_method("foo")) # false although Test has this method

In #22833 this bug makes it impossible to use a static func of a class in array.sort_custom().

Steps to reproduce:
Call has_method on a class.

Minimal reproduction project:
sfhm.zip

[DavidSichma]

Changing this line in object.h
https://github.com/godotengine/godot/blob/b17e71b6e5e035f49b5b3b5b55b9cdac80215d72/core/object.h#L652
and making this bool virtual seems to (partially) fix the issue. The Script class has its own has_method so it should work if correctly implemented.

As I didn't test it with VisualScript and C# I am not comfortable committing this change.

[akien-mga]

CC @reduz @vnen @neikeq

[neikeq]

The scripting API doesn't support static method. As such, has_method is not static. You need an instance to access it.

Changing this line in object.h
https://github.com/godotengine/godot/blob/b17e71b6e5e035f49b5b3b5b55b9cdac80215d72/core/object.h#L652
and making this bool virtual seems to fix the issue. The Script class has its own has_method so it should work if correctly implemented.

I don't understand how that would change anything. Could you give more details?

[DavidSichma]

In Objects there is this method defined:
https://github.com/godotengine/godot/blob/b17e71b6e5e035f49b5b3b5b55b9cdac80215d72/core/object.h#L652
If I get it right this method checks if the method is either specified in an atached script or in the class.

However there is also a has_method() defined in the Script class which inherits Object. The implementations of the derived classes seem work for static functions (tested only for GDScript).
https://github.com/godotengine/godot/blob/b17e71b6e5e035f49b5b3b5b55b9cdac80215d72/core/script_language.h#L127
This already caused duplicate issues: #10785

If I understand c++ correctly, correct my if I'm wrong, the following happens:

• Object has has_method which is not virtual and can not be overridden.
• Script classes have their own has_method which causes the Object's implementation to be hidden but not overriden.
• Calling has_method() on any object whose type isn't specified will call the Object's version.
• Changing to virtual will essentialy make the Object's version possible to be overridden. This way the correct method will be called.

This makes it possible to check for static method in inner classes. The inner classes are of type GDScript and their has_method works for both static and non static functions (I don't know if this is intended behavior but it just works in this context).

Note that I only tested this with GDScript. I have not testet it in VisualScript and C# nor do I plan learning C# just for that. Also I don't know if this could potentialy create other issues with has_method I can't foresee.

[DavidSichma]

Ok, changing has_method to virtual won't solve all problems.

The biggest one is that just overriding will get rid of the functionality in object's implementation. With this "fix" has_method on any script will not find

• free()
• methods in an attached script (though attaching a script to a script might be very rare)
• methods defined not in the script itself. For example has_method can not be found.

Furthermore static functions of parents can not be found, although they are callable.

Advice would be appreciated. Or if someone wants to try it themself, go for it.

[DavidSichma]

If someone is interested: Here are the test cases I could think of:

extends Node

class Foo:
    static func foo():
        return "foo"
    func foo_non_static():
        return "foo-non-static"

class Bar extends Foo:
    static func bar():
        return "bar"
    func bar_non_static():
        return "bar-non-static"

class scrpt:
    func script_func():
        return "script_func"

static func fun():
    return "fun"

func fun_non_static():
    return "fun_non_static"

func _ready():
    print("Test on self")
    _test(self, self.fun(), "fun")
    _test(self, self.fun_non_static(), "fun_non_static")

    print("Test on inner class")
    _test(Foo, Foo.foo(), "foo")
    _test(Foo, Foo.foo_non_static(), "foo_non_static")

    print("Test on instance of inner class")
    var fo = Foo.new()
    _test(fo, fo.foo(), "foo")
    _test(fo, fo.foo_non_static(), "foo_non_static")

    print("Test on inherited inner class")
    _test(Bar, Bar.foo(), "foo")
    _test(Bar, Bar.foo_non_static(), "foo_non_static")

    print("Test on instance of inherited inner class")
    var ba = Bar.new()
    _test(ba, ba.foo(), "foo")
    _test(ba, ba.foo_non_static(), "foo_non_static")

    print("Adding script to instance of inner class")
    fo.set_script(scrpt)
    _test(fo, fo.script_func(), "script_func")
    print("Adding script to inner class")
    Foo.set_script(scrpt)
    _test(Foo, Foo.script_func(), "script_func")

    print("Foo has " + ("" if Foo.has_method("has_method") else "no ") + "class methods")

func _test(var obj, var call, var method_name):
    var res = "Calling %s() on %s: \"%s\" and method can%s be found" % [method_name, obj, call, "" if obj.has_method(method_name) else " not"]
    print(res)

[neikeq]

First of all, Script::has_method is very likely unrelated to Object::has_method. If I were to go the virtual has_method way, I would rename the method in Script to something else first and then add the has_method override.

The biggest one is that just overriding will get rid of the functionality in object's implementation

This can be fixed by calling the object's implementation if the method was not found.

Furthermore static functions of parents can not be found, although they are callable.

This should be simple as well. Just calling base->has_method(p_method) as well should be enough.

This won't work in any other language (except VisualScript perhaps), unless they have a way to get the Script from a type.
The reason it works in GDScript is likely because a Type identifier seems to represent the Script itself (the language is designed to be well integrated with Godot). I would be surprised if it wasn't the same for VisualScript.
Theorically we could provide a method GetScriptFromType(typeof(Foo)) or GetScriptFromType<Foo>() in C# which would return the Script so you can call HasMethod on it. Not sure about other languages.

[akien-mga]

Note: I've reverted 6415454 with c730957 due to this bug. Once we have a solution for this issue, we could re-add 6415454.

[DavidSichma]

Thanks for the tip! I made the changes and it works fine.

I have renamed Script::has_method to Script::defines_method as this method just lists methods defined in this very script. I also updated the calls, so no Object::has_method will be called by accident.

[reduz]

This will be properly fixable when methods become actual members. @vnen is going to work on this for Godot 3.2, so kicking there.

[pycbouh]

Will this be a part of 4.0 or was it just postponed to the latest available version? I guess it is related to the introduction of a Callable type? Also, what is going to be the solution? Will has_method work with both static and instanced methods or will we get a separate method for static checks?

[willnationsdev]

Will this be a part of 4.0 or was it just postponed to the latest available version? I guess it is related to the introduction of a Callable type?

Yes, I believe this is being postponed till 4.0 (or, if not then, 4.1) since reduz's "when methods become actual members" comment refers to the introduction of the Callable type happening in 4.0.

I suspect the solution will just be changing the implementation of has_method() to support it. It would involve having the Script check static functions on a would-be ScriptInstance that hasn't yet been created, so the way that data is stored will likely be updated (probably by virtue of the Callable changes).

Will has_method work with both static and instanced methods or will we get a separate method for static checks?

My guess would be that it would be the same method for both, but it would just be updated so that instance and static methods work on ScriptInstances and static methods work on Scripts.