Revert invalid PopupMenu breaking change from #61102

[akien-mga]

See #61179 (comment) for rationale.

• Fixes 3.5rc1 Clear Recent Files returns out of bounds error #61179.

CC @okla.

[timothyqiu]

I also spotted a documentation error.

godot/doc/classes/OptionButton.xml

Lines 97 to 101 in b7523a9

	<method name="get_selected_id" qualifiers="const">
	<return type="int" />
	<description>
	Returns the ID of the selected item, or [code]-1[/code] if no item is selected.
	</description>

The special return value is not changed on 3.x

godot/scene/gui/option_button.cpp

Lines 260 to 264 in b7523a9

	int OptionButton::get_selected_id() const {
	int idx = get_selected();
	if (idx < 0) {
	return 0;
	}

[akien-mga]

Hm maybe we should actually do that tiny compat breakage for 3.5, it feels more like a bugfix than an arbitrary behavior change. Since currently there would be no way to differentiate between "item 0 selected" and "no item selected".

[timothyqiu]

@akien-mga It's the ID of that item, not index.

When using the default ID (passing -1 to the parameter), the auto assigned ID starts from 1. selected / get_selected() returns -1 if nothing selected. It's kind of a more suitable way to check if anything is selected.

But yeah, -1 will never be a valid ID. So it's either the doc or the implementation that should be changed :P

[okla]

I also spotted a documentation error.

godot/doc/classes/OptionButton.xml

Lines 97 to 101 in b7523a9

	<method name="get_selected_id" qualifiers="const">
	<return type="int" />
	<description>
	Returns the ID of the selected item, or [code]-1[/code] if no item is selected.
	</description>

The special return value is not changed on 3.x

godot/scene/gui/option_button.cpp

Lines 260 to 264 in b7523a9

	int OptionButton::get_selected_id() const {
	int idx = get_selected();
	if (idx < 0) {
	return 0;
	}

Looks like I've missed get_selected_id() change in backport commit. It should contain only return get_item_id(current); now. So the documentation is right, code is wrong.

[okla]

Maybe backport #54647 instead of reverting?

Edit: or better not, since it depends on another PR...

[okla]

I also spotted a documentation error.

godot/doc/classes/OptionButton.xml

Lines 97 to 101 in b7523a9

	<method name="get_selected_id" qualifiers="const">
	<return type="int" />
	<description>
	Returns the ID of the selected item, or [code]-1[/code] if no item is selected.
	</description>

The special return value is not changed on 3.x

godot/scene/gui/option_button.cpp

Lines 260 to 264 in b7523a9

	int OptionButton::get_selected_id() const {
	int idx = get_selected();
	if (idx < 0) {
	return 0;
	}

Looks like I've missed get_selected_id() change in backport commit. It should contain only return get_item_id(current); now. So the documentation is right, code is wrong.

Corresponding change is here #61185

[akien-mga]

Maybe backport #54647 instead of reverting?

It seems to build upon #54533... the rabbit hole starts getting deep :) I prefer that we only backport what users actively need in their 3.x projects, not general API consistency improvements done in 4.0 while breaking compat.

[okla]

Maybe backport #54647 instead of reverting?

It seems to build upon #54533... the rabbit hole starts getting deep :) I prefer that we only backport what users actively need in their 3.x projects, not general API consistency improvements done in 4.0 while breaking compat.

Yeah, I've already edited my message)