Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Static method override fails when call site is base script #72973

Open
Gnumaru opened this issue Feb 9, 2023 · 3 comments
Open

Static method override fails when call site is base script #72973

Gnumaru opened this issue Feb 9, 2023 · 3 comments

Comments

@Gnumaru
Copy link
Contributor

Gnumaru commented Feb 9, 2023

Godot version

4.0.rc1

System information

windows 10

Issue description

When overriding a static method, it is possible to call the override "from outside", when the call site is using a reference to the derived script, and it is also possible to invoke the base implementation from within the overridden implementation using "super()". BUT, if a given method is not overridden and it's implementation calls another method which has an override in the derived script, then the overridden implementation does not get called at all, only the base implementation.

I think that, If static method override is desired in gdscript, then the static method call should be bound to the script reference lastly used to invoke the method, that is, the derived scripts which may override static methods, and this binding should propagate to other static calls within the method implementation unless a new script reference is used.

If this is not a bug, but an unwanted behavior, then we should warn the user that overriding static methods between scripts is not supported. But if this is indeed a bug, then it could be interesting consider doing something like in php: in php, when using the 'static' keyword inside a method, it refers always to the most derived script used to initiate the call stack, whereas the 'self' keyword refers always to the current script where the keyword was written. If similar keywords existed in gdscript, we could use static.myStaticMethod() to ensure the myStaticMethod() call is bound to the most derived script, whereas self.myStaticMethod() (obviously the keyword should be other since self is already used) would be bound always to the script where the keyword is written. Or maybe we could make get_script() work somehow in static contexts and always return the most derived script used in the call stack, so that we can use get_script().myStaticMethod() to ensure the overriden method is called and MyScriptName.myStaticMethod() to ensure the base implementation is called

Steps to reproduce

Create a Base class. Then create a Derived class which extends from base. In the Base class implement two methods, A and B, A will call B. In the Derived class, override method B but not A. Then, from a third script, call Derived.a() and notice how Derived.b() is never called

Screenshot (280)

Screenshot (281)

Screenshot (282)

Minimal reproduction project

StaticMethodOverrideFailsPropagatingBoundScript.zip

@kleonc
Copy link
Member

kleonc commented Feb 9, 2023

I'd say that's definitely a bug, for instance/non-static methods it'd call the overridden method just fine. Also if calling static method on an instance (which seems to be allowed) it does call the proper overridden static method. In the MRP:

print(v)
print(Derived.get_script_name_static())
print(v)
print(Derived.new().get_script_name_static())
print(v)

Output:

========================================
Base.get_script_name_static
Base.get_script_static
Base
========================================
Base.get_script_name_static
Derived.get_script_static
Base.get_script_static
Derived
========================================

@NotDaze
Copy link

NotDaze commented May 22, 2023

This is still present in 4.0.3

@dalexeev
Copy link
Member

I'm not sure if this can be called a 100% bug. The expected behavior is not documented anywhere. For example, PHP has Late Static Bindings, you can explicitly specify with the self:: or static:: prefix which method you want to call (of the current class or the inherited one). It probably requires a proposal.

See also:

Example
<?php

class A {
    static function overriden(): string {
        return "A::overriden";
    }

    static function not_overriden(): string {
        return "A::not_overriden";
    }	

    static function test_overriden(): void {
        echo "A::test_overriden\n";
        echo "=================\n";
        echo "overriden (self): " . self::overriden() . "\n";
        echo "overriden (static): " . static::overriden() . "\n";
        echo "not_overriden (self): " . self::not_overriden() . "\n";
        echo "not_overriden (static): " . static::not_overriden() . "\n";
        echo "\n";
    }

    static function test_not_overriden(): void {
        echo "A::test_not_overriden\n";
        echo "=====================\n";
        echo "overriden (self): " . self::overriden() . "\n";
        echo "overriden (static): " . static::overriden() . "\n";
        echo "not_overriden (self): " . self::not_overriden() . "\n";
        echo "not_overriden (static): " . static::not_overriden() . "\n";
        echo "\n";
    }
}

class B extends A {
    static function overriden(): string {
        return "B::overriden";
    }

    static function test_overriden(): void {
        echo "B::test_overriden\n";
        echo "=================\n";
        echo "overriden (self): " . self::overriden() . "\n";
        echo "overriden (static): " . static::overriden() . "\n";
        echo "not_overriden (self): " . self::not_overriden() . "\n";
        echo "not_overriden (static): " . static::not_overriden() . "\n";
        echo "\n";
    }
}

A::test_overriden();
A::test_not_overriden();
B::test_overriden();
B::test_not_overriden();
A::test_overriden
=================
overriden (self): A::overriden
overriden (static): A::overriden
not_overriden (self): A::not_overriden
not_overriden (static): A::not_overriden

A::test_not_overriden
=====================
overriden (self): A::overriden
overriden (static): A::overriden
not_overriden (self): A::not_overriden
not_overriden (static): A::not_overriden

B::test_overriden
=================
overriden (self): B::overriden
overriden (static): B::overriden
not_overriden (self): A::not_overriden
not_overriden (static): A::not_overriden

A::test_not_overriden
=====================
overriden (self): A::overriden
overriden (static): B::overriden
not_overriden (self): A::not_overriden
not_overriden (static): A::not_overriden

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants