PopupMenu: Fix missing text/xl_text when using add_shortcut

Use macros to ensure that `text`, `xl_text` and `id` are always set
using the same logic.

Fixes #25519.

Also fixes up #26914 when `p_id == -1` handling was only added for a
couple methods instead of all of them.

1 files changed, 49 insertions, 60 deletions

diff --git a/scene/gui/popup_menu.cpp b/scene/gui/popup_menu.cpp
index 9305b806c0..08faaf7d45 100644
--- a/scene/gui/popup_menu.cpp
+++ b/scene/gui/popup_menu.cpp
@@ -79,7 +79,7 @@ Size2 PopupMenu::get_minimum_size() const {
 		if (items[i].checkable_type)
 			has_check = true;
 
-		String text = items[i].shortcut.is_valid() ? String(tr(items[i].shortcut->get_name())) : items[i].xl_text;
+		String text = items[i].xl_text;
 		size.width += font->get_string_size(text).width;
 		if (i > 0)
 			size.height += vseparation;
@@ -519,7 +519,7 @@ void PopupMenu::_notification(int p_what) {
 					hover->draw(ci, Rect2(item_ofs + Point2(-hseparation, -vseparation / 2), Size2(get_size().width - style->get_minimum_size().width + hseparation * 2, h + vseparation)));
 				}
 
-				String text = items[i].shortcut.is_valid() ? String(tr(items[i].shortcut->get_name())) : items[i].xl_text;
+				String text = items[i].xl_text;
 
 				item_ofs.x += items[i].h_ofs;
 				if (items[i].separator) {
@@ -631,13 +631,16 @@ void PopupMenu::_notification(int p_what) {
  * Be sure to keep them in sync when adding new properties in the Item struct.
  */
 
+#define ITEM_SETUP_WITH_ACCEL(p_label, p_id, p_accel) \
+	item.text = p_label;                              \
+	item.xl_text = tr(p_label);                       \
+	item.id = p_id == -1 ? items.size() : p_id;       \
+	item.accel = p_accel;
+
 void PopupMenu::add_item(const String &p_label, int p_id, uint32_t p_accel) {
 
 	Item item;
-	item.text = p_label;
-	item.xl_text = tr(p_label);
-	item.accel = p_accel;
-	item.id = p_id == -1 ? items.size() : p_id;
+	ITEM_SETUP_WITH_ACCEL(p_label, p_id, p_accel);
 	items.push_back(item);
 	update();
 	minimum_size_changed();
@@ -646,11 +649,8 @@ void PopupMenu::add_item(const String &p_label, int p_id, uint32_t p_accel) {
 void PopupMenu::add_icon_item(const Ref<Texture> &p_icon, const String &p_label, int p_id, uint32_t p_accel) {
 
 	Item item;
+	ITEM_SETUP_WITH_ACCEL(p_label, p_id, p_accel);
 	item.icon = p_icon;
-	item.text = p_label;
-	item.xl_text = tr(p_label);
-	item.accel = p_accel;
-	item.id = p_id;
 	items.push_back(item);
 	update();
 	minimum_size_changed();
@@ -659,10 +659,7 @@ void PopupMenu::add_icon_item(const Ref<Texture> &p_icon, const String &p_label,
 void PopupMenu::add_check_item(const String &p_label, int p_id, uint32_t p_accel) {
 
 	Item item;
-	item.text = p_label;
-	item.xl_text = tr(p_label);
-	item.accel = p_accel;
-	item.id = p_id == -1 ? items.size() : p_id;
+	ITEM_SETUP_WITH_ACCEL(p_label, p_id, p_accel);
 	item.checkable_type = Item::CHECKABLE_TYPE_CHECK_BOX;
 	items.push_back(item);
 	update();
@@ -672,11 +669,8 @@ void PopupMenu::add_check_item(const String &p_label, int p_id, uint32_t p_accel
 void PopupMenu::add_icon_check_item(const Ref<Texture> &p_icon, const String &p_label, int p_id, uint32_t p_accel) {
 
 	Item item;
+	ITEM_SETUP_WITH_ACCEL(p_label, p_id, p_accel);
 	item.icon = p_icon;
-	item.text = p_label;
-	item.xl_text = tr(p_label);
-	item.accel = p_accel;
-	item.id = p_id;
 	item.checkable_type = Item::CHECKABLE_TYPE_CHECK_BOX;
 	items.push_back(item);
 	update();
@@ -685,16 +679,21 @@ void PopupMenu::add_icon_check_item(const Ref<Texture> &p_icon, const String &p_
 
 void PopupMenu::add_radio_check_item(const String &p_label, int p_id, uint32_t p_accel) {
 
-	add_check_item(p_label, p_id, p_accel);
-	items.write[items.size() - 1].checkable_type = Item::CHECKABLE_TYPE_RADIO_BUTTON;
+	Item item;
+	ITEM_SETUP_WITH_ACCEL(p_label, p_id, p_accel);
+	item.checkable_type = Item::CHECKABLE_TYPE_RADIO_BUTTON;
+	items.push_back(item);
 	update();
 	minimum_size_changed();
 }
 
 void PopupMenu::add_icon_radio_check_item(const Ref<Texture> &p_icon, const String &p_label, int p_id, uint32_t p_accel) {
 
-	add_icon_check_item(p_icon, p_label, p_id, p_accel);
-	items.write[items.size() - 1].checkable_type = Item::CHECKABLE_TYPE_RADIO_BUTTON;
+	Item item;
+	ITEM_SETUP_WITH_ACCEL(p_label, p_id, p_accel);
+	item.icon = p_icon;
+	item.checkable_type = Item::CHECKABLE_TYPE_RADIO_BUTTON;
+	items.push_back(item);
 	update();
 	minimum_size_changed();
 }
@@ -702,10 +701,7 @@ void PopupMenu::add_icon_radio_check_item(const Ref<Texture> &p_icon, const Stri
 void PopupMenu::add_multistate_item(const String &p_label, int p_max_states, int p_default_state, int p_id, uint32_t p_accel) {
 
 	Item item;
-	item.text = p_label;
-	item.xl_text = tr(p_label);
-	item.accel = p_accel;
-	item.id = p_id;
+	ITEM_SETUP_WITH_ACCEL(p_label, p_id, p_accel);
 	item.max_states = p_max_states;
 	item.state = p_default_state;
 	items.push_back(item);
@@ -713,16 +709,19 @@ void PopupMenu::add_multistate_item(const String &p_label, int p_max_states, int
 	minimum_size_changed();
 }
 
-void PopupMenu::add_shortcut(const Ref<ShortCut> &p_shortcut, int p_id, bool p_global) {
-
-	ERR_FAIL_COND(p_shortcut.is_null());
+#define ITEM_SETUP_WITH_SHORTCUT(p_shortcut, p_id, p_global)                           \
+	ERR_FAIL_COND_MSG(p_shortcut.is_null(), "Cannot add item with invalid ShortCut."); \
+	_ref_shortcut(p_shortcut);                                                         \
+	item.text = p_shortcut->get_name();                                                \
+	item.xl_text = tr(item.text);                                                      \
+	item.id = p_id == -1 ? items.size() : p_id;                                        \
+	item.shortcut = p_shortcut;                                                        \
+	item.shortcut_is_global = p_global;
 
-	_ref_shortcut(p_shortcut);
+void PopupMenu::add_shortcut(const Ref<ShortCut> &p_shortcut, int p_id, bool p_global) {
 
 	Item item;
-	item.id = p_id;
-	item.shortcut = p_shortcut;
-	item.shortcut_is_global = p_global;
+	ITEM_SETUP_WITH_SHORTCUT(p_shortcut, p_id, p_global);
 	items.push_back(item);
 	update();
 	minimum_size_changed();
@@ -730,15 +729,9 @@ void PopupMenu::add_shortcut(const Ref<ShortCut> &p_shortcut, int p_id, bool p_g
 
 void PopupMenu::add_icon_shortcut(const Ref<Texture> &p_icon, const Ref<ShortCut> &p_shortcut, int p_id, bool p_global) {
 
-	ERR_FAIL_COND(p_shortcut.is_null());
-
-	_ref_shortcut(p_shortcut);
-
 	Item item;
-	item.id = p_id;
+	ITEM_SETUP_WITH_SHORTCUT(p_shortcut, p_id, p_global);
 	item.icon = p_icon;
-	item.shortcut = p_shortcut;
-	item.shortcut_is_global = p_global;
 	items.push_back(item);
 	update();
 	minimum_size_changed();
@@ -746,14 +739,8 @@ void PopupMenu::add_icon_shortcut(const Ref<Texture> &p_icon, const Ref<ShortCut
 
 void PopupMenu::add_check_shortcut(const Ref<ShortCut> &p_shortcut, int p_id, bool p_global) {
 
-	ERR_FAIL_COND(p_shortcut.is_null());
-
-	_ref_shortcut(p_shortcut);
-
 	Item item;
-	item.id = p_id;
-	item.shortcut = p_shortcut;
-	item.shortcut_is_global = p_global;
+	ITEM_SETUP_WITH_SHORTCUT(p_shortcut, p_id, p_global);
 	item.checkable_type = Item::CHECKABLE_TYPE_CHECK_BOX;
 	items.push_back(item);
 	update();
@@ -762,16 +749,10 @@ void PopupMenu::add_check_shortcut(const Ref<ShortCut> &p_shortcut, int p_id, bo
 
 void PopupMenu::add_icon_check_shortcut(const Ref<Texture> &p_icon, const Ref<ShortCut> &p_shortcut, int p_id, bool p_global) {
 
-	ERR_FAIL_COND(p_shortcut.is_null());
-
-	_ref_shortcut(p_shortcut);
-
 	Item item;
-	item.id = p_id;
-	item.shortcut = p_shortcut;
-	item.checkable_type = Item::CHECKABLE_TYPE_CHECK_BOX;
+	ITEM_SETUP_WITH_SHORTCUT(p_shortcut, p_id, p_global);
 	item.icon = p_icon;
-	item.shortcut_is_global = p_global;
+	item.checkable_type = Item::CHECKABLE_TYPE_CHECK_BOX;
 	items.push_back(item);
 	update();
 	minimum_size_changed();
@@ -779,16 +760,21 @@ void PopupMenu::add_icon_check_shortcut(const Ref<Texture> &p_icon, const Ref<Sh
 
 void PopupMenu::add_radio_check_shortcut(const Ref<ShortCut> &p_shortcut, int p_id, bool p_global) {
 
-	add_check_shortcut(p_shortcut, p_id, p_global);
-	items.write[items.size() - 1].checkable_type = Item::CHECKABLE_TYPE_RADIO_BUTTON;
+	Item item;
+	ITEM_SETUP_WITH_SHORTCUT(p_shortcut, p_id, p_global);
+	item.checkable_type = Item::CHECKABLE_TYPE_RADIO_BUTTON;
+	items.push_back(item);
 	update();
 	minimum_size_changed();
 }
 
 void PopupMenu::add_icon_radio_check_shortcut(const Ref<Texture> &p_icon, const Ref<ShortCut> &p_shortcut, int p_id, bool p_global) {
 
-	add_icon_check_shortcut(p_icon, p_shortcut, p_id, p_global);
-	items.write[items.size() - 1].checkable_type = Item::CHECKABLE_TYPE_RADIO_BUTTON;
+	Item item;
+	ITEM_SETUP_WITH_SHORTCUT(p_shortcut, p_id, p_global);
+	item.icon = p_icon;
+	item.checkable_type = Item::CHECKABLE_TYPE_RADIO_BUTTON;
+	items.push_back(item);
 	update();
 	minimum_size_changed();
 }
@@ -798,13 +784,16 @@ void PopupMenu::add_submenu_item(const String &p_label, const String &p_submenu,
 	Item item;
 	item.text = p_label;
 	item.xl_text = tr(p_label);
-	item.id = p_id;
+	item.id = p_id == -1 ? items.size() : p_id;
 	item.submenu = p_submenu;
 	items.push_back(item);
 	update();
 	minimum_size_changed();
 }
 
+#undef ITEM_SETUP_WITH_ACCEL
+#undef ITEM_SETUP_WITH_SHORTCUT
+
 /* Methods to modify existing items. */
 
 void PopupMenu::set_item_text(int p_idx, const String &p_text) {