godotengine / godot-cpp

C++ bindings for the Godot script API
MIT License
1.74k stars 575 forks source link

Not declare _bind_methods() may be cause not call "_ready()" #1430

Closed pupil1337 closed 6 months ago

pupil1337 commented 7 months ago

Godot version

4.3.dev(e5b4ef8)

godot-cpp version

4.3.dev(b021245)

System information

win11

Issue description

This may be a problem, and I'm not sure. For safety reasons, I reported it.

issue:

When Base class not declare _ready(), and Derive class declare _ready() but not declare _bind_methods(), the virtual func _ready() in Derive class will not called.

// base.h
class Base : public Node {
    GDCLASS(Base, Node)

protected:
    static void _bind_methods() {}
};
// derive.h
class Derive : public Base {
    GDCLASS(Derive, Base)

public:
    virtual void _ready() override {
        Base::_ready();

        UtilityFunctions::print("Derive::_ready");
    }
};

register Base and Derive, and use Derive as root node in engine, run the game: Derive will not call _ready()

Causes:
// wrapped.hpp
static void initialize_class() {
    static bool initialized = false;
    if (initialized) {
        return;
    }
    m_inherits::initialize_class(); 
    if (m_class::_get_bind_methods() != m_inherits::_get_bind_methods()) { // derive class not declare _bind_methods() cause issue
        _bind_methods();
        m_inherits::register_virtuals<m_class, m_inherits>(); // register virtual (_ready,_process,...) in this line
    }
    initialized = true;
}

Steps to reproduce

Look Issue description.

Minimal reproduction project

N/A

dsnopek commented 6 months ago

Thanks for the report!

All classes that use GDCLASS() are supposed to define their own static void _bind_methods() - it weren't for the parent class, there would have been a compilation error. I'll see if I can come up with a way for their to be a compilation error when _bind_methods() is omitted, even when there is a parent class involved.

dsnopek commented 6 months ago

I just posted PR https://github.com/godotengine/godot-cpp/pull/1448 that attempts to make it a compile-time error if a class doesn't define its own _bind_methods() function.

pupil1337 commented 6 months ago

I just posted PR #1448 that attempts to make it a compile-time error if a class doesn't define its own _bind_methods() function.

Thank you. I think _ready() _input() called not affected by the definition of _bind_methods() or not. _bind_methods() is only to write ClassDB::bind_method() or ADD_PROPERTY() for editor. When declare virtual void _ready() override; in my class, it must can call from godot. declare the _bind_methods() function is optional(dont compilation error). Please check if my idea is reasonable.

dsnopek commented 6 months ago

declare the _bind_methods() function is optional(dont compilation error).

It isn't intended to be optional - you're supposed to always have a _bind_methods(), even if it's empty.

And, if you were defining a class that descended from a native type, not implementing _bind_methods() would already have been a compilation error. My PR is only making it also be a compilation error when defining a class that descends from another extension class (rather than from a native class).

pupil1337 commented 6 months ago

It isn't intended to be optional - you're supposed to always have a , even if it's empty._bind_methods()

And, if you were defining a class that descended from a native type, not implementing would already have been a compilation error. My PR is only making it also be a compilation error when defining a class that descends from another extension class (rather than from a native class)._bind_methods()

Ok, I understand now