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.

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

PopupMenu: Fix inconsistency setting text/xl_text in add_* methods #32638

Merged
merged 2 commits into from Oct 8, 2019

Conversation

@akien-mga
Copy link
Member

akien-mga commented Oct 8, 2019

  • Part 1, cleanup: Reorder add_* methods in more natural order
    Also adds add_icon_radio_check_shortcut matching add_icon_radio_check_item,
    binds them for scripting languages, and binds add_multistate_item.

  • Part 2, bugfix for #25519: 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. (upcoming)

For the reference the second commit is basically the same as #25841 (which was reverted), but together with a proper fix for the editor bug it exposed back then (#25900).

BTW, this PopupMenu API is a complete mess :o)
We shouldn't need to have so many duplicated methods to add items with specific configuration changes.

@akien-mga akien-mga added this to the 3.2 milestone Oct 8, 2019
@akien-mga akien-mga requested a review from godotengine/documentation as a code owner Oct 8, 2019
akien-mga added 2 commits Oct 8, 2019
Also adds `add_icon_radio_check_shortcut` matching `add_icon_radio_check_item`,
binds them for scripting languages, and binds `add_multistate_item`.
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.
@akien-mga akien-mga force-pushed the akien-mga:popupmenu-keep-name branch from c46cdb2 to 58dd5d0 Oct 8, 2019
@akien-mga akien-mga changed the title WIP: PopupMenu: Fix inconsistency setting text/xl_text in add_* methods PopupMenu: Fix inconsistency setting text/xl_text in add_* methods Oct 8, 2019
@akien-mga akien-mga requested a review from YeldhamDev Oct 8, 2019
#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(); \

This comment has been minimized.

Copy link
@akien-mga

akien-mga Oct 8, 2019

Author Member

This is the main logic change, where add_shortcut* methods will now properly set the Item's text and xl_text to the ShortCut's name.

@akien-mga akien-mga merged commit 76ef250 into godotengine:master Oct 8, 2019
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@akien-mga akien-mga deleted the akien-mga:popupmenu-keep-name branch Oct 8, 2019
@akien-mga

This comment has been minimized.

Copy link
Member Author

akien-mga commented Nov 8, 2019

Cherry-picked for 3.1.2. (Only commit 58dd5d0.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.