[Addon][GUI] Multi-select dialog does not appear to report Cancel vs. OK

[djp952]

Bug report

Describe the bug

Here is a clear and concise description of what the problem is:

I've been working on a PVR radio addon that requires the use of the addon GUI Multi-Select dialog to allow the user to choose what DAB / HD Radio subchannels should be added / updated as part of adding/updating an ensemble and ran into an unexpected problem with this dialog. I expected the call to kodi::gui::dialogs::Select::ShowMultiSelect() to return false if the user selects Cancel, this does not appear to be the case.

I believe that the current Interface_GUIDialogSelect::open_multi_select function may be faulty in this regard?

xbmc/xbmc/addons/interfaces/gui/dialogs/Select.cpp

Lines 79 to 85 in 3868401

	bool Interface_GUIDialogSelect::open_multi_select(KODI_HANDLE kodiBase,
	const char* heading,
	const char* entryIDs[],
	const char* entryNames[],
	bool entriesSelected[],
	unsigned int size,
	unsigned int autoclose)

The as-is function will seemingly return true regardless of the user choosing OK or Cancel in response to the dialog:

xbmc/xbmc/addons/interfaces/gui/dialogs/Select.cpp

Lines 125 to 140 in 3868401

	dialog->Open();
	if (dialog->IsConfirmed())
	{
	for (unsigned int i = 0; i < size; ++i)
	entriesSelected[i] = false;
	selectedIndexes = dialog->GetSelectedItems();
	for (unsigned int i = 0; i < selectedIndexes.size(); ++i)
	{
	if (selectedIndexes[i])
	entriesSelected[selectedIndexes[i]] = true;
	}
	}
	return true;

To me, this seems like an impossible situation to resolve for the addon? If the user didn't change the default selections but tried to Cancel, I'm not sure how the addon can determine this condition?

Expected Behavior

Here is a clear and concise description of what was expected to happen:

I feel that Interface_GUIDialogSelect::open_multi_select should return false on a Cancel operation.

Actual Behavior

Unless something goes horribly wrong, this function always returns true.

Possible Fix

I think that moving the return true condition should be moved up into the dialog->IsConfirmed() block ...

xbmc/xbmc/addons/interfaces/gui/dialogs/Select.cpp

Line 126 in 3868401

if (dialog->IsConfirmed())

... and the default return should be false:

xbmc/xbmc/addons/interfaces/gui/dialogs/Select.cpp

Line 140 in 3868401

return true;

To Reproduce

Steps to reproduce the behavior:

Author a Binary Addon that tries to use the multi-select dialog and wonder how a "Cancel" operation can be detected if the user didn't change anything?

Debuglog

There is zero value in this log, it's only being provided to legitimatize the Issue: https://paste.kodi.tv/agaxonigoz.kodi. Nothing to see here.

Screenshots

Here are some links or screenshots to help explain the problem:

N/A

Additional context or screenshots (if appropriate)

Here is some additional context or explanation that might help:

I think the issue is fairly easy to solve, but I have not had the best luck with Kodi PRs of late and thought that opening an Issue was better this time around to allow for better commentary and/or concerns with the proposed change.

If there is any agreement on my proposed change, I am more than happy to issue a PR to that effect, for both Nexus and Matrix. Again, my luck has been "bad" on this of late, and I felt that an Issue was better than a PR in case I'm just being obtuse and overlooking something dumb again.

Your Environment

Used Operating system:

• Android

• iOS

• tvOS

• Linux

• OSX

• Windows

• Windows UWP

• Operating system version/name:

• Kodi version: Nexus Alpha 1, Windows 11 x64 (Desktop)

[enen92]

maybe @AlwinEsch can take a look as this seems related to the binary addon interface

[djp952]

Actually, I'm going to pull this back. I appreciate the look-over. I understand why it behaves this way, the use case for the dialog assumes that all the entries are going to be FALSE on initialization, and it subsequently toggles the first item in the list.

My need is for a dialog that allows all the entries to be TRUE on initialization, with a need for a Cancel notification. I can work with the as-is dialog as a stop gap, but I think I need to implement a custom dialog to do what I really want to, which is fine.

It's cool, and again I sincerely appreciate you having a look through the description, @enen92! @AlwinEsch, if for some reason you think having the dialog behaving differently to allow for an all-TRUE entry default, please just let me know. I doubt it, however :)