From 431c032d8c02fcad8d0a43e2a38ce955f9b44dc9 Mon Sep 17 00:00:00 2001
From: Sergey Pershenkov <s.pershenkov@icloud.com>
Date: Mon, 16 May 2022 22:47:46 +0300
Subject: [PATCH] Make various improvements to OptionButton

- Allow OptionButton selection to be set to -1 to signify no selection, both via API and in the editor.
- Reset OptionButton selection to -1 when the selected item has been removed.
- Fully convert PopupMenu to a zero-based ID system, which improves an inconsistency in generated IDs when making new items in the editor.

(cherry picked from commit 3b146c5eaa06d9f6827c651802b9fb2a9a1e013d)
---
 doc/classes/OptionButton.xml |  3 ++-
 scene/gui/option_button.cpp  | 40 +++++++++++++++++++++++++-----------
 scene/gui/popup_menu.cpp     |  6 +++---
 3 files changed, 33 insertions(+), 16 deletions(-)

diff --git a/doc/classes/OptionButton.xml b/doc/classes/OptionButton.xml
index a0f9aaa1c24..84625038963 100644
--- a/doc/classes/OptionButton.xml
+++ b/doc/classes/OptionButton.xml
@@ -97,7 +97,7 @@
 		<method name="get_selected_id" qualifiers="const">
 			<return type="int" />
 			<description>
-				Returns the ID of the selected item, or [code]0[/code] if no item is selected.
+				Returns the ID of the selected item, or [code]-1[/code] if no item is selected.
 			</description>
 		</method>
 		<method name="get_selected_metadata" qualifiers="const">
@@ -125,6 +125,7 @@
 			<argument index="0" name="idx" type="int" />
 			<description>
 				Selects an item by index and makes it the current item. This will work even if the item is disabled.
+				Passing [code]-1[/code] as the index deselects any currently selected item.
 			</description>
 		</method>
 		<method name="set_item_disabled">
diff --git a/scene/gui/option_button.cpp b/scene/gui/option_button.cpp
index 6add6f3e157..88889069879 100644
--- a/scene/gui/option_button.cpp
+++ b/scene/gui/option_button.cpp
@@ -32,6 +32,8 @@
 #include "core/os/input.h"
 #include "core/print_string.h"
 
+static const int NONE_SELECTED = -1;
+
 Size2 OptionButton::get_minimum_size() const {
 	Size2 minsize = Button::get_minimum_size();
 
@@ -173,6 +175,10 @@ Ref<Texture> OptionButton::get_item_icon(int p_idx) const {
 }
 
 int OptionButton::get_item_id(int p_idx) const {
+	if (p_idx == NONE_SELECTED) {
+		return NONE_SELECTED;
+	}
+
 	return popup->get_item_id(p_idx);
 }
 
@@ -203,34 +209,41 @@ void OptionButton::add_separator() {
 void OptionButton::clear() {
 	popup->clear();
 	set_text("");
-	current = -1;
+	current = NONE_SELECTED;
 }
 
 void OptionButton::_select(int p_which, bool p_emit) {
-	if (p_which < 0) {
-		return;
-	}
 	if (p_which == current) {
 		return;
 	}
 
-	ERR_FAIL_INDEX(p_which, popup->get_item_count());
+	if (p_which == NONE_SELECTED) {
+		for (int i = 0; i < popup->get_item_count(); i++) {
+			popup->set_item_checked(i, false);
+		}
 
-	for (int i = 0; i < popup->get_item_count(); i++) {
-		popup->set_item_checked(i, i == p_which);
+		current = NONE_SELECTED;
+		set_text("");
+		set_icon(NULL);
+	} else {
+		ERR_FAIL_INDEX(p_which, popup->get_item_count());
+
+		for (int i = 0; i < popup->get_item_count(); i++) {
+			popup->set_item_checked(i, i == p_which);
+		}
+
+		current = p_which;
+		set_text(popup->get_item_text(current));
+		set_icon(popup->get_item_icon(current));
 	}
 
-	current = p_which;
-	set_text(popup->get_item_text(current));
-	set_icon(popup->get_item_icon(current));
-
 	if (is_inside_tree() && p_emit) {
 		emit_signal("item_selected", current);
 	}
 }
 
 void OptionButton::_select_int(int p_which) {
-	if (p_which < 0 || p_which >= popup->get_item_count()) {
+	if (p_which < NONE_SELECTED || p_which >= popup->get_item_count()) {
 		return;
 	}
 	_select(p_which, false);
@@ -261,6 +274,9 @@ Variant OptionButton::get_selected_metadata() const {
 
 void OptionButton::remove_item(int p_idx) {
 	popup->remove_item(p_idx);
+	if (current == p_idx) {
+		_select(NONE_SELECTED);
+	}
 }
 
 PopupMenu *OptionButton::get_popup() const {
diff --git a/scene/gui/popup_menu.cpp b/scene/gui/popup_menu.cpp
index f6bbf5c6153..67786958c0e 100644
--- a/scene/gui/popup_menu.cpp
+++ b/scene/gui/popup_menu.cpp
@@ -666,7 +666,7 @@ void PopupMenu::_notification(int p_what) {
 #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.id = p_id == -1 ? items.size() - 1 : p_id;   \
 	item.accel = p_accel;
 
 void PopupMenu::add_item(const String &p_label, int p_id, uint32_t p_accel) {
@@ -739,7 +739,7 @@ void PopupMenu::add_multistate_item(const String &p_label, int p_max_states, int
 	_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.id = p_id == -1 ? items.size() - 1 : p_id;                                    \
 	item.shortcut = p_shortcut;                                                        \
 	item.shortcut_is_global = p_global;
 
@@ -802,7 +802,7 @@ 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 == -1 ? items.size() : p_id;
+	item.id = p_id == -1 ? items.size() - 1 : p_id;
 	item.submenu = p_submenu;
 	items.push_back(item);
 	update();