Add PropertyListHelper to PopupMenu

scene/gui/item_list.cpp

@@ -35,8 +35,6 @@
 #include "core/string/translation.h"
 #include "scene/theme/theme_db.h"
 
-PropertyListHelper ItemList::base_property_helper;
-
 void ItemList::_shape_text(int p_idx) {
 	Item &item = items.write[p_idx];
 
@@ -1705,22 +1703,6 @@ bool ItemList::_set(const StringName &p_name, const Variant &p_value) {
 	return false;
 }
 
-bool ItemList::_get(const StringName &p_name, Variant &r_ret) const {
-	return property_helper.property_get_value(p_name, r_ret);
-}
-
-void ItemList::_get_property_list(List<PropertyInfo> *p_list) const {
-	property_helper.get_property_list(p_list, items.size());
-}
-
-bool ItemList::_property_can_revert(const StringName &p_name) const {
-	return property_helper.property_can_revert(p_name);
-}
-
-bool ItemList::_property_get_revert(const StringName &p_name, Variant &r_property) const {
-	return property_helper.property_get_revert(p_name, r_property);
-}
-
 void ItemList::_bind_methods() {
 	ClassDB::bind_method(D_METHOD("add_item", "text", "icon", "selectable"), &ItemList::add_item, DEFVAL(Variant()), DEFVAL(true));
 	ClassDB::bind_method(D_METHOD("add_icon_item", "icon", "selectable"), &ItemList::add_icon_item, DEFVAL(true));
@@ -1889,10 +1871,10 @@ void ItemList::_bind_methods() {
 	Item defaults(true);
 
 	base_property_helper.set_prefix("item_");
-	base_property_helper.register_property(PropertyInfo(Variant::STRING, "text"), defaults.text, "set_item_text", "get_item_text");
-	base_property_helper.register_property(PropertyInfo(Variant::OBJECT, "icon", PROPERTY_HINT_RESOURCE_TYPE, "Texture2D"), defaults.icon, "set_item_icon", "get_item_icon");
-	base_property_helper.register_property(PropertyInfo(Variant::BOOL, "selectable"), defaults.selectable, "set_item_selectable", "is_item_selectable");
-	base_property_helper.register_property(PropertyInfo(Variant::BOOL, "disabled"), defaults.disabled, "set_item_disabled", "is_item_disabled");
+	base_property_helper.register_property(PropertyInfo(Variant::STRING, "text"), defaults.text, &ItemList::set_item_text, &ItemList::get_item_text);
+	base_property_helper.register_property(PropertyInfo(Variant::OBJECT, "icon", PROPERTY_HINT_RESOURCE_TYPE, "Texture2D"), defaults.icon, &ItemList::set_item_icon, &ItemList::get_item_icon);
+	base_property_helper.register_property(PropertyInfo(Variant::BOOL, "selectable"), defaults.selectable, &ItemList::set_item_selectable, &ItemList::is_item_selectable);
+	base_property_helper.register_property(PropertyInfo(Variant::BOOL, "disabled"), defaults.disabled, &ItemList::set_item_disabled, &ItemList::is_item_disabled);
 }
 
 ItemList::ItemList() {

scene/gui/item_list.h

@@ -87,7 +87,7 @@ private:
 		Item(bool p_dummy) {}
 	};
 
-	static PropertyListHelper base_property_helper;
+	static inline PropertyListHelper base_property_helper;
 	PropertyListHelper property_helper;
 
 	int current = -1;
@@ -161,10 +161,10 @@ private:
 protected:
 	void _notification(int p_what);
 	bool _set(const StringName &p_name, const Variant &p_value);
-	bool _get(const StringName &p_name, Variant &r_ret) const;
-	void _get_property_list(List<PropertyInfo> *p_list) const;
-	bool _property_can_revert(const StringName &p_name) const;
-	bool _property_get_revert(const StringName &p_name, Variant &r_property) const;
+	bool _get(const StringName &p_name, Variant &r_ret) const { return property_helper.property_get_value(p_name, r_ret); }
+	void _get_property_list(List<PropertyInfo> *p_list) const { property_helper.get_property_list(p_list, items.size()); }
+	bool _property_can_revert(const StringName &p_name) const { return property_helper.property_can_revert(p_name); }
+	bool _property_get_revert(const StringName &p_name, Variant &r_property) const { return property_helper.property_get_revert(p_name, r_property); }
 	static void _bind_methods();
 
 public:

scene/gui/popup_menu.cpp

@@ -57,6 +57,25 @@ bool PopupMenu::_set_item_accelerator(int p_index, const Ref<InputEventKey> &p_i
 	return false;
 }
 
+void PopupMenu::_set_item_checkable_type(int p_index, int p_checkable_type) {
+	switch (p_checkable_type) {
+		case Item::CHECKABLE_TYPE_NONE: {
+			set_item_as_checkable(p_index, false);
+		} break;
+		case Item::CHECKABLE_TYPE_CHECK_BOX: {
+			set_item_as_checkable(p_index, true);
+		} break;
+		case Item::CHECKABLE_TYPE_RADIO_BUTTON: {
+			set_item_as_radio_checkable(p_index, true);
+		} break;
+	}
+}
+
+int PopupMenu::_get_item_checkable_type(int p_index) const {
+	ERR_FAIL_INDEX_V(p_index, items.size(), Item::CHECKABLE_TYPE_NONE);
+	return items[p_index].checkable_type;
+}
+
 String PopupMenu::bind_global_menu() {
 #ifdef TOOLS_ENABLED
 	if (is_part_of_edited_scene()) {
@@ -2561,39 +2580,10 @@ void PopupMenu::take_mouse_focus() {
 }
 
 bool PopupMenu::_set(const StringName &p_name, const Variant &p_value) {
-	Vector<String> components = String(p_name).split("/", true, 2);
-	if (components.size() >= 2 && components[0].begins_with("item_") && components[0].trim_prefix("item_").is_valid_int()) {
-		int item_index = components[0].trim_prefix("item_").to_int();
-		const String &property = components[1];
-		if (property == "text") {
-			set_item_text(item_index, p_value);
-			return true;
-		} else if (property == "icon") {
-			set_item_icon(item_index, p_value);
-			return true;
-		} else if (property == "checkable") {
-			bool radio_checkable = (int)p_value == Item::CHECKABLE_TYPE_RADIO_BUTTON;
-			if (radio_checkable) {
-				set_item_as_radio_checkable(item_index, true);
-			} else {
-				bool checkable = p_value;
-				set_item_as_checkable(item_index, checkable);
-			}
-			return true;
-		} else if (property == "checked") {
-			set_item_checked(item_index, p_value);
-			return true;
-		} else if (property == "id") {
-			set_item_id(item_index, p_value);
-			return true;
-		} else if (property == "disabled") {
-			set_item_disabled(item_index, p_value);
-			return true;
-		} else if (property == "separator") {
-			set_item_as_separator(item_index, p_value);
-			return true;
-		}
+	if (property_helper.property_set_value(p_name, p_value)) {
+		return true;
 	}
+
 #ifndef DISABLE_DEPRECATED
 	// Compatibility.
 	if (p_name == "items") {
@@ -2639,71 +2629,6 @@ bool PopupMenu::_set(const StringName &p_name, const Variant &p_value) {
 	return false;
 }
 
-bool PopupMenu::_get(const StringName &p_name, Variant &r_ret) const {
-	Vector<String> components = String(p_name).split("/", true, 2);
-	if (components.size() >= 2 && components[0].begins_with("item_") && components[0].trim_prefix("item_").is_valid_int()) {
-		int item_index = components[0].trim_prefix("item_").to_int();
-		const String &property = components[1];
-		if (property == "text") {
-			r_ret = get_item_text(item_index);
-			return true;
-		} else if (property == "icon") {
-			r_ret = get_item_icon(item_index);
-			return true;
-		} else if (property == "checkable") {
-			if (item_index >= 0 && item_index < items.size()) {
-				r_ret = items[item_index].checkable_type;
-				return true;
-			} else {
-				r_ret = Item::CHECKABLE_TYPE_NONE;
-				ERR_FAIL_V(true);
-			}
-		} else if (property == "checked") {
-			r_ret = is_item_checked(item_index);
-			return true;
-		} else if (property == "id") {
-			r_ret = get_item_id(item_index);
-			return true;
-		} else if (property == "disabled") {
-			r_ret = is_item_disabled(item_index);
-			return true;
-		} else if (property == "separator") {
-			r_ret = is_item_separator(item_index);
-			return true;
-		}
-	}
-	return false;
-}
-
-void PopupMenu::_get_property_list(List<PropertyInfo> *p_list) const {
-	for (int i = 0; i < items.size(); i++) {
-		p_list->push_back(PropertyInfo(Variant::STRING, vformat("item_%d/text", i)));
-
-		PropertyInfo pi = PropertyInfo(Variant::OBJECT, vformat("item_%d/icon", i), PROPERTY_HINT_RESOURCE_TYPE, "Texture2D");
-		pi.usage &= ~(get_item_icon(i).is_null() ? PROPERTY_USAGE_STORAGE : 0);
-		p_list->push_back(pi);
-
-		pi = PropertyInfo(Variant::INT, vformat("item_%d/checkable", i), PROPERTY_HINT_ENUM, "No,As checkbox,As radio button");
-		pi.usage &= ~(!is_item_checkable(i) ? PROPERTY_USAGE_STORAGE : 0);
-		p_list->push_back(pi);
-
-		pi = PropertyInfo(Variant::BOOL, vformat("item_%d/checked", i));
-		pi.usage &= ~(!is_item_checked(i) ? PROPERTY_USAGE_STORAGE : 0);
-		p_list->push_back(pi);
-
-		pi = PropertyInfo(Variant::INT, vformat("item_%d/id", i), PROPERTY_HINT_RANGE, "0,10,1,or_greater");
-		p_list->push_back(pi);
-
-		pi = PropertyInfo(Variant::BOOL, vformat("item_%d/disabled", i));
-		pi.usage &= ~(!is_item_disabled(i) ? PROPERTY_USAGE_STORAGE : 0);
-		p_list->push_back(pi);
-
-		pi = PropertyInfo(Variant::BOOL, vformat("item_%d/separator", i));
-		pi.usage &= ~(!is_item_separator(i) ? PROPERTY_USAGE_STORAGE : 0);
-		p_list->push_back(pi);
-	}
-}
-
 void PopupMenu::_bind_methods() {
 	ClassDB::bind_method(D_METHOD("activate_item_by_event", "event", "for_global_only"), &PopupMenu::activate_item_by_event, DEFVAL(false));
 
@@ -2860,6 +2785,17 @@ void PopupMenu::_bind_methods() {
 	BIND_THEME_ITEM(Theme::DATA_TYPE_COLOR, PopupMenu, font_separator_color);
 	BIND_THEME_ITEM_CUSTOM(Theme::DATA_TYPE_CONSTANT, PopupMenu, font_separator_outline_size, "separator_outline_size");
 	BIND_THEME_ITEM(Theme::DATA_TYPE_COLOR, PopupMenu, font_separator_outline_color);
+
+	Item defaults(true);
+
+	base_property_helper.set_prefix("item_");
+	base_property_helper.register_property(PropertyInfo(Variant::STRING, "text"), defaults.text, &PopupMenu::set_item_text, &PopupMenu::get_item_text);
+	base_property_helper.register_property(PropertyInfo(Variant::OBJECT, "icon", PROPERTY_HINT_RESOURCE_TYPE, "Texture2D"), defaults.icon, &PopupMenu::set_item_icon, &PopupMenu::get_item_icon);
+	base_property_helper.register_property(PropertyInfo(Variant::INT, "checkable", PROPERTY_HINT_ENUM, "No,As checkbox,As radio button"), defaults.checkable_type, &PopupMenu::_set_item_checkable_type, &PopupMenu::_get_item_checkable_type);
+	base_property_helper.register_property(PropertyInfo(Variant::BOOL, "checked"), defaults.checked, &PopupMenu::set_item_checked, &PopupMenu::is_item_checked);
+	base_property_helper.register_property(PropertyInfo(Variant::INT, "id", PROPERTY_HINT_RANGE, "0,10,1,or_greater"), defaults.id, &PopupMenu::set_item_id, &PopupMenu::get_item_id);
+	base_property_helper.register_property(PropertyInfo(Variant::BOOL, "disabled"), defaults.disabled, &PopupMenu::set_item_disabled, &PopupMenu::is_item_disabled);
+	base_property_helper.register_property(PropertyInfo(Variant::BOOL, "separator"), defaults.separator, &PopupMenu::set_item_as_separator, &PopupMenu::is_item_separator);
 }
 
 void PopupMenu::popup(const Rect2i &p_bounds) {
@@ -2900,6 +2836,8 @@ PopupMenu::PopupMenu() {
 	minimum_lifetime_timer->set_one_shot(true);
 	minimum_lifetime_timer->connect("timeout", callable_mp(this, &PopupMenu::_minimum_lifetime_timeout));
 	add_child(minimum_lifetime_timer, false, INTERNAL_MODE_FRONT);
+
+	property_helper.setup_for_instance(base_property_helper, this);
 }
 
 PopupMenu::~PopupMenu() {

scene/gui/popup_menu.h

@@ -35,6 +35,7 @@
 #include "scene/gui/margin_container.h"
 #include "scene/gui/popup.h"
 #include "scene/gui/scroll_container.h"
+#include "scene/property_list_helper.h"
 #include "scene/resources/text_line.h"
 
 class PopupMenu : public Popup {
@@ -59,7 +60,7 @@ class PopupMenu : public Popup {
 			CHECKABLE_TYPE_NONE,
 			CHECKABLE_TYPE_CHECK_BOX,
 			CHECKABLE_TYPE_RADIO_BUTTON,
-		} checkable_type;
+		} checkable_type = CHECKABLE_TYPE_NONE;
 		int max_states = 0;
 		int state = 0;
 		bool separator = false;
@@ -89,8 +90,13 @@ class PopupMenu : public Popup {
 			accel_text_buf.instantiate();
 			checkable_type = CHECKABLE_TYPE_NONE;
 		}
+
+		Item(bool p_dummy) {}
 	};
 
+	static inline PropertyListHelper base_property_helper;
+	PropertyListHelper property_helper;
+
 	String global_menu_name;
 	String system_menu_name;
 
@@ -195,6 +201,8 @@ class PopupMenu : public Popup {
 	void _menu_changed();
 	void _input_from_window_internal(const Ref<InputEvent> &p_event);
 	bool _set_item_accelerator(int p_index, const Ref<InputEventKey> &p_ie);
+	void _set_item_checkable_type(int p_index, int p_checkable_type);
+	int _get_item_checkable_type(int p_index) const;
 
 protected:
 	virtual void add_child_notify(Node *p_child) override;
@@ -203,8 +211,10 @@ protected:
 
 	void _notification(int p_what);
 	bool _set(const StringName &p_name, const Variant &p_value);
-	bool _get(const StringName &p_name, Variant &r_ret) const;
-	void _get_property_list(List<PropertyInfo> *p_list) const;
+	bool _get(const StringName &p_name, Variant &r_ret) const { return property_helper.property_get_value(p_name, r_ret); }
+	void _get_property_list(List<PropertyInfo> *p_list) const { property_helper.get_property_list(p_list, items.size()); }
+	bool _property_can_revert(const StringName &p_name) const { return property_helper.property_can_revert(p_name); }
+	bool _property_get_revert(const StringName &p_name, Variant &r_property) const { return property_helper.property_get_revert(p_name, r_property); }
 	static void _bind_methods();
 
 #ifndef DISABLE_DEPRECATED

scene/property_list_helper.cpp

@@ -47,35 +47,28 @@ const PropertyListHelper::Property *PropertyListHelper::_get_property(const Stri
 	return property_list.getptr(components[1]);
 }
 
-void PropertyListHelper::_bind_property(const Property &p_property, const Object *p_object) {
-	Property property = p_property;
-	property.info = p_property.info;
-	property.default_value = p_property.default_value;
-	property.setter = Callable(p_object, p_property.setter_name);
-	property.getter = Callable(p_object, p_property.getter_name);
-
-	property_list[property.info.name] = property;
+void PropertyListHelper::_call_setter(const MethodBind *p_setter, int p_index, const Variant &p_value) const {
+	Variant args[] = { p_index, p_value };
+	const Variant *argptrs[] = { &args[0], &args[1] };
+	Callable::CallError ce;
+	p_setter->call(object, argptrs, 2, ce);
 }
 
-void PropertyListHelper::set_prefix(const String &p_prefix) {
-	prefix = p_prefix;
+Variant PropertyListHelper::_call_getter(const MethodBind *p_getter, int p_index) const {
+	Callable::CallError ce;
+	Variant args[] = { p_index };
+	const Variant *argptrs[] = { &args[0] };
+	return p_getter->call(object, argptrs, 1, ce);
 }
 
-void PropertyListHelper::register_property(const PropertyInfo &p_info, const Variant &p_default, const StringName &p_setter, const StringName &p_getter) {
-	Property property;
-	property.info = p_info;
-	property.default_value = p_default;
-	property.setter_name = p_setter;
-	property.getter_name = p_getter;
-
-	property_list[p_info.name] = property;
+void PropertyListHelper::set_prefix(const String &p_prefix) {
+	prefix = p_prefix;
 }
 
-void PropertyListHelper::setup_for_instance(const PropertyListHelper &p_base, const Object *p_object) {
+void PropertyListHelper::setup_for_instance(const PropertyListHelper &p_base, Object *p_object) {
 	prefix = p_base.prefix;
-	for (const KeyValue<String, Property> &E : p_base.property_list) {
-		_bind_property(E.value, p_object);
-	}
+	property_list = p_base.property_list;
+	object = p_object;
 }
 
 void PropertyListHelper::get_property_list(List<PropertyInfo> *p_list, int p_count) const {
@@ -84,7 +77,7 @@ void PropertyListHelper::get_property_list(List<PropertyInfo> *p_list, int p_cou
 			const Property &property = E.value;
 
 			PropertyInfo info = property.info;
-			if (property.getter.call(i) == property.default_value) {
+			if (_call_getter(property.getter, i) == property.default_value) {
 				info.usage &= (~PROPERTY_USAGE_STORAGE);
 			}
 
@@ -99,7 +92,7 @@ bool PropertyListHelper::property_get_value(const String &p_property, Variant &r
 	const Property *property = _get_property(p_property, &index);
 
 	if (property) {
-		r_ret = property->getter.call(index);
+		r_ret = _call_getter(property->getter, index);
 		return true;
 	}
 	return false;
@@ -110,7 +103,7 @@ bool PropertyListHelper::property_set_value(const String &p_property, const Vari
 	const Property *property = _get_property(p_property, &index);
 
 	if (property) {
-		property->setter.call(index, p_value);
+		_call_setter(property->setter, index, p_value);
 		return true;
 	}
 	return false;
@@ -121,7 +114,7 @@ bool PropertyListHelper::property_can_revert(const String &p_property) const {
 	const Property *property = _get_property(p_property, &index);
 
 	if (property) {
-		return property->getter.call(index) != property->default_value;
+		return _call_getter(property->getter, index) != property->default_value;
 	}
 	return false;
 }
@@ -136,3 +129,13 @@ bool PropertyListHelper::property_get_revert(const String &p_property, Variant &
 	}
 	return false;
 }
+
+PropertyListHelper::~PropertyListHelper() {
+	// No object = it's the main helper. Do a cleanup.
+	if (!object) {
+		for (const KeyValue<String, Property> &E : property_list) {
+			memdelete(E.value.setter);
+			memdelete(E.value.getter);
+		}
+	}
+}

scene/property_list_helper.h

@@ -31,34 +31,48 @@
 #ifndef PROPERTY_LIST_HELPER_H
 #define PROPERTY_LIST_HELPER_H
 
+#include "core/object/method_bind.h"
 #include "core/object/object.h"
 
 class PropertyListHelper {
 	struct Property {
 		PropertyInfo info;
 		Variant default_value;
-		StringName setter_name;
-		StringName getter_name;
-		Callable setter;
-		Callable getter;
+		MethodBind *setter = nullptr;
+		MethodBind *getter = nullptr;
 	};
 
 	String prefix;
 	HashMap<String, Property> property_list;
+	Object *object = nullptr;
 
 	const Property *_get_property(const String &p_property, int *r_index) const;
-	void _bind_property(const Property &p_property, const Object *p_object);
+	void _call_setter(const MethodBind *p_setter, int p_index, const Variant &p_value) const;
+	Variant _call_getter(const MethodBind *p_getter, int p_index) const;
 
 public:
 	void set_prefix(const String &p_prefix);
-	void register_property(const PropertyInfo &p_info, const Variant &p_default, const StringName &p_setter, const StringName &p_getter);
-	void setup_for_instance(const PropertyListHelper &p_base, const Object *p_object);
+
+	template <typename S, typename G>
+	void register_property(const PropertyInfo &p_info, const Variant &p_default, S p_setter, G p_getter) {
+		Property property;
+		property.info = p_info;
+		property.default_value = p_default;
+		property.setter = create_method_bind(p_setter);
+		property.getter = create_method_bind(p_getter);
+
+		property_list[p_info.name] = property;
+	}
+
+	void setup_for_instance(const PropertyListHelper &p_base, Object *p_object);
 
 	void get_property_list(List<PropertyInfo> *p_list, int p_count) const;
 	bool property_get_value(const String &p_property, Variant &r_ret) const;
 	bool property_set_value(const String &p_property, const Variant &p_value) const;
 	bool property_can_revert(const String &p_property) const;
 	bool property_get_revert(const String &p_property, Variant &r_value) const;
+
+	~PropertyListHelper();
 };
 
 #endif // PROPERTY_LIST_HELPER_H