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.

Sign up for GitHub

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

Jump to bottom

Fix deriving a custom class with virtual methods #855

Merged
merged 1 commit into from
Oct 5, 2022
Merged

Fix deriving a custom class with virtual methods #855

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions binding_generator.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1071,17 +1071,22 @@ def generate_engine_class_header(class_api, used_classes, fully_used_classes, us
result.append(method_signature + ";")

result.append("protected:")
result.append("\ttemplate <class T>")
# T is the custom class we want to register (from which the call initiates, going up the inheritance chain),
# B is its base class (can be a custom class too, that's why we pass it).
result.append("\ttemplate <class T, class B>")
result.append("\tstatic void register_virtuals() {")
if class_name != "Object":
result.append(f"\t\t{inherits}::register_virtuals<T>();")
result.append(f"\t\t{inherits}::register_virtuals<T, B>();")
if "methods" in class_api:
for method in class_api["methods"]:
if not method["is_virtual"]:
continue
method_name = escape_identifier(method["name"])
result.append(
f"\t\tif constexpr (!std::is_same_v<decltype(&{class_name}::{method_name}),decltype(&T::{method_name})>) {{"
# If the method is different from the base class, it means T overrides it, so it needs to be bound.
# Note that with an `if constexpr`, the code inside the `if` will not even be compiled if the
# condition returns false (in such cases it can't compile due to ambiguity).
f"\t\tif constexpr (!std::is_same_v<decltype(&B::{method_name}),decltype(&T::{method_name})>) {{"
)
result.append(f"\t\t\tBIND_VIRTUAL_METHOD(T, {method_name});")
result.append("\t\t}")
Expand Down
6 changes: 3 additions & 3 deletions include/godot_cpp/classes/wrapped.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,9 +145,9 @@ protected:
return (::godot::String(::godot::Wrapped::*)()) & m_class::_to_string; \
} \
\
template <class T> \
template <class T, class B> \
static void register_virtuals() { \
m_inherits::register_virtuals<T>(); \
m_inherits::register_virtuals<T, B>(); \
} \
\
public: \
Expand All @@ -159,7 +159,7 @@ public:
m_inherits::initialize_class(); \
if (m_class::_get_bind_methods() != m_inherits::_get_bind_methods()) { \
_bind_methods(); \
m_inherits::register_virtuals<m_class>(); \
m_inherits::register_virtuals<m_class, m_inherits>(); \
} \
initialized = true; \
} \
Expand Down
10 changes: 7 additions & 3 deletions include/godot_cpp/core/class_db.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,10 +89,12 @@ class ClassDB {
std::unordered_map<std::string, GDNativeExtensionClassCallVirtual> virtual_methods;
std::set<std::string> property_names;
std::set<std::string> constant_names;
// Pointer to the parent custom class, if any. Will be null if the parent class is a Godot class.
ClassInfo *parent_ptr = nullptr;
};

private:
// This may only contain custom classes, not Godot classes
static std::unordered_map<std::string, ClassInfo> classes;

static MethodBind *bind_methodfi(uint32_t p_flags, MethodBind *p_bind, const MethodDefinition &method_name, const void **p_defs, int p_defcount);
Expand Down Expand Up @@ -158,10 +160,12 @@ void ClassDB::register_class() {
cl.name = T::get_class_static();
cl.parent_name = T::get_parent_class_static();
cl.level = current_level;
classes[cl.name] = cl;
if (classes.find(cl.parent_name) != classes.end()) {
cl.parent_ptr = &classes[cl.parent_name];
std::unordered_map<std::string, ClassInfo>::iterator parent_it = classes.find(cl.parent_name);
if (parent_it != classes.end()) {
// Assign parent if it is also a custom class
cl.parent_ptr = &parent_it->second;
}
classes[cl.name] = cl;

// Register this class with Godot
GDNativeExtensionClassCreationInfo class_info = {
Expand Down
19 changes: 14 additions & 5 deletions src/core/class_db.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -277,20 +277,29 @@ void ClassDB::bind_integer_constant(const char *p_class_name, const char *p_enum
}

GDNativeExtensionClassCallVirtual ClassDB::get_virtual_func(void *p_userdata, const char *p_name) {
// This is called by Godot the first time it calls a virtual function, and it caches the result, per object instance.
// Because of this, it can happen from different threads at once.
// It should be ok not using any mutex as long as we only READ data.

const char *class_name = (const char *)p_userdata;

std::unordered_map<std::string, ClassInfo>::iterator type_it = classes.find(class_name);
ERR_FAIL_COND_V_MSG(type_it == classes.end(), nullptr, "Class doesn't exist.");

ClassInfo &type = type_it->second;
const ClassInfo *type = &type_it->second;

// Find method in current class, or any of its parent classes (Godot classes not included)
while (type != nullptr) {
std::unordered_map<std::string, GDNativeExtensionClassCallVirtual>::const_iterator method_it = type->virtual_methods.find(p_name);

std::unordered_map<std::string, GDNativeExtensionClassCallVirtual>::iterator method_it = type.virtual_methods.find(p_name);
if (method_it != type->virtual_methods.end()) {
return method_it->second;
}

if (method_it == type.virtual_methods.end()) {
return nullptr;
type = type->parent_ptr;
}

return method_it->second;
return nullptr;
}

void ClassDB::bind_virtual_method(const char *p_class, const char *p_method, GDNativeExtensionClassCallVirtual p_call) {
Expand Down