Skip to content

Commit

Permalink
Don't call parent _get_property_list when a class doesn't define it.
Browse files Browse the repository at this point in the history 
Godot is already supposed to call _get_property_list of parent classes,
so this binding function must really only return procedural properties of
the class it belongs to, and not parent or child classes.
  • Loading branch information
@Zylann
Zylann committed Jul 26, 2023
1 parent 749b0b9 commit 2861611
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 30 deletions.
57 changes: 28 additions & 29 deletions include/godot_cpp/classes/wrapped.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,9 @@ class Wrapped {
static GDExtensionBool property_get_revert_bind(GDExtensionClassInstancePtr p_instance, GDExtensionConstStringNamePtr p_name, GDExtensionVariantPtr r_ret) { return false; }
static void to_string_bind(GDExtensionClassInstancePtr p_instance, GDExtensionBool *r_is_valid, GDExtensionStringPtr r_out) {}

// The only reason this has to be held here, is when we return results of `_get_property_list` to Godot, we pass
// pointers to strings in this list. They have to remain valid to pass the bridge, until the list is freed by Godot...
::godot::List<::godot::PropertyInfo> plist_owned;
GDExtensionPropertyInfo *plist = nullptr;
uint32_t plist_size = 0;

void _postinitialize();

Expand All @@ -95,9 +95,19 @@ class Wrapped {
GodotObject *_owner = nullptr;
};

namespace internal {

GDExtensionPropertyInfo *create_c_property_list(const ::godot::List<::godot::PropertyInfo> &plist_cpp, uint32_t *r_size);
void free_c_property_list(GDExtensionPropertyInfo *plist);

} // namespace internal

} // namespace godot

#define GDCLASS(m_class, m_inherits) \
// Use this on top of your own classes.
// Note: the trail of `***` is to keep sane diffs in PRs, because clang-format otherwise moves every `\` which makes
// every line of the macro different
#define GDCLASS(m_class, m_inherits) /***********************************************************************************************************************************************/ \
private: \
void operator=(const m_class &p_rval) {} \
friend class ::godot::ClassDB; \
Expand Down Expand Up @@ -209,40 +219,29 @@ public:
return false; \
} \
\
static inline bool has_get_property_list() { \
return m_class::_get_get_property_list() && m_class::_get_get_property_list() != m_inherits::_get_get_property_list(); \
} \
\
static const GDExtensionPropertyInfo *get_property_list_bind(GDExtensionClassInstancePtr p_instance, uint32_t *r_count) { \
if (p_instance && m_class::_get_get_property_list()) { \
if (m_class::_get_get_property_list() != m_inherits::_get_get_property_list()) { \
m_class *cls = reinterpret_cast<m_class *>(p_instance); \
ERR_FAIL_COND_V_MSG(!cls->plist_owned.is_empty() || cls->plist != nullptr || cls->plist_size != 0, nullptr, "Internal error, property list was not freed by engine!"); \
cls->_get_property_list(&cls->plist_owned); \
cls->plist = reinterpret_cast<GDExtensionPropertyInfo *>(memalloc(sizeof(GDExtensionPropertyInfo) * cls->plist_owned.size())); \
cls->plist_size = 0; \
for (const ::godot::PropertyInfo &E : cls->plist_owned) { \
cls->plist[cls->plist_size].type = static_cast<GDExtensionVariantType>(E.type); \
cls->plist[cls->plist_size].name = E.name._native_ptr(); \
cls->plist[cls->plist_size].hint = E.hint; \
cls->plist[cls->plist_size].hint_string = E.hint_string._native_ptr(); \
cls->plist[cls->plist_size].class_name = E.class_name._native_ptr(); \
cls->plist[cls->plist_size].usage = E.usage; \
cls->plist_size++; \
} \
if (r_count) \
*r_count = cls->plist_size; \
return cls->plist; \
} \
return m_inherits::get_property_list_bind(p_instance, r_count); \
if (!p_instance) { \
if (r_count) \
*r_count = 0; \
return nullptr; \
} \
return nullptr; \
m_class *cls = reinterpret_cast<m_class *>(p_instance); \
::godot::List<::godot::PropertyInfo> &plist_cpp = cls->plist_owned; \
ERR_FAIL_COND_V_MSG(!plist_cpp.is_empty(), nullptr, "Internal error, property list was not freed by engine!"); \
cls->_get_property_list(&plist_cpp); \
return ::godot::create_c_property_list(plist_cpp, r_count); \
} \
\
static void free_property_list_bind(GDExtensionClassInstancePtr p_instance, const GDExtensionPropertyInfo *p_list) { \
if (p_instance) { \
m_class *cls = reinterpret_cast<m_class *>(p_instance); \
ERR_FAIL_COND_MSG(cls->plist == nullptr, "Internal error, property list double free!"); \
memfree(cls->plist); \
cls->plist = nullptr; \
cls->plist_size = 0; \
cls->plist_owned.clear(); \
/* TODO `GDExtensionClassFreePropertyList` is ill-defined, we need a non-const pointer to free this. */ \
::godot::free_c_property_list(const_cast<GDExtensionPropertyInfo *>(p_list)); \
} \
} \
\
Expand Down
2 changes: 1 addition & 1 deletion include/godot_cpp/core/class_db.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ void ClassDB::_register_class(bool p_virtual) {
is_abstract, // GDExtensionBool is_abstract;
T::set_bind, // GDExtensionClassSet set_func;
T::get_bind, // GDExtensionClassGet get_func;
T::get_property_list_bind, // GDExtensionClassGetPropertyList get_property_list_func;
T::has_get_property_list() ? T::get_property_list_bind : nullptr, // GDExtensionClassGetPropertyList get_property_list_func;
T::free_property_list_bind, // GDExtensionClassFreePropertyList free_property_list_func;
T::property_can_revert_bind, // GDExtensionClassPropertyCanRevert property_can_revert_func;
T::property_get_revert_bind, // GDExtensionClassPropertyGetRevert property_get_revert_func;
Expand Down
29 changes: 29 additions & 0 deletions src/classes/wrapped.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,4 +60,33 @@ void postinitialize_handler(Wrapped *p_wrapped) {
p_wrapped->_postinitialize();
}

namespace internal {

GDExtensionPropertyInfo *create_c_property_list(const ::godot::List<::godot::PropertyInfo> &plist_cpp, uint32_t *r_size) {
GDExtensionPropertyInfo *plist = nullptr;
// Linked list size can be expensive to get so we cache it
const uint32_t plist_size = plist_cpp.size();
if (r_size != nullptr) {
*r_size = plist_size;
}
plist = reinterpret_cast<GDExtensionPropertyInfo *>(memalloc(sizeof(GDExtensionPropertyInfo) * plist_size));
unsigned int i = 0;
for (const ::godot::PropertyInfo &E : plist_cpp) {
plist[i].type = static_cast<GDExtensionVariantType>(E.type);
plist[i].name = E.name._native_ptr();
plist[i].hint = E.hint;
plist[i].hint_string = E.hint_string._native_ptr();
plist[i].class_name = E.class_name._native_ptr();
plist[i].usage = E.usage;
++i;
}
return plist;
}

void free_c_property_list(GDExtensionPropertyInfo *plist) {
memfree(plist);
}

} // namespace internal

} // namespace godot

0 comments on commit 2861611

Please sign in to comment.