From 58dd5d0c788a3334c48076456ceff1e414ede986 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9mi=20Verschelde?= Date: Tue, 8 Oct 2019 09:33:22 +0200 Subject: [PATCH] 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. --- scene/gui/popup_menu.cpp | 109 ++++++++++++++++++--------------------- 1 file changed, 49 insertions(+), 60 deletions(-) diff --git a/scene/gui/popup_menu.cpp b/scene/gui/popup_menu.cpp index 9305b806c01..08faaf7d45f 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 &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 &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 &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 &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 &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 &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(); } +#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; + void PopupMenu::add_shortcut(const Ref &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); items.push_back(item); update(); minimum_size_changed(); @@ -730,15 +729,9 @@ void PopupMenu::add_shortcut(const Ref &p_shortcut, int p_id, bool p_g void PopupMenu::add_icon_shortcut(const Ref &p_icon, const Ref &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 &p_icon, const Ref &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 &p_shortcut, int p_id, bo void PopupMenu::add_icon_check_shortcut(const Ref &p_icon, const Ref &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 &p_icon, const Ref &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 &p_icon, const Ref &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) {