This is my first post here. I think I have found a bug in CEGUI 0.7.7, that also affects lastest versions. I searched for the guilty method's name on this forum and CEGUI's Mantis, with no success. Please excuse me if this issue has already been adressed.
Also, the wiki said to directly post issues on Mantis, a tool I am familiar with, but I couldn't find a link to a page permitting to report a new issue.
The alleged bug I found is caused by these few lines of code in GEUIItemListbox.cpp (or ItemListbox.cpp in later versions) :
Code: Select all
/************************************************************************
Get a pointer to the first selected item starting from the given index
************************************************************************/
ItemEntry* ItemListbox::findSelectedItem(size_t start_index) const
{
size_t max = d_listItems.size();
if (start_index >= max)
{
return 0;
}
for (size_t i=start_index; i<max; ++i)
{
ItemEntry* li = d_listItems[i];
if (li->isSelected())
{
d_nextSelectionIndex = i; // <----- Issue caused by this line
return li;
}
}
return 0;
}
/************************************************************************
Get a pointer to the first selected item
************************************************************************/
ItemEntry* ItemListbox::getFirstSelectedItem(size_t start_index) const
{
if (!d_multiSelect)
{
return d_lastSelected;
}
return findSelectedItem(start_index);
}
/************************************************************************
Get a pointer to the next selected item using internal counter
************************************************************************/
ItemEntry* ItemListbox::getNextSelectedItem() const
{
if (!d_multiSelect)
{
return 0;
}
return findSelectedItem(d_nextSelectionIndex);
}From what I understand, getFirstSelectedItem and getNextSelectedItem call findSelectedItem to do their main job.
The latter method updates the d_nextSelectionIndex member variable in order to keep the ItemListbox's internal state coherent for the next call to getNextSelectedItem.
However, a quick review of the body of findSelectedItem makes me think that successive calls to it will always return the first selected item in the list, without ever proceeding to the next items.
That, in turn, not only causes getNextSelectedItem not to behave as written in the documentation, but moreover causes an infinite loop in my project's code : D
A quick fix would be to replace the commented line above by :
Code: Select all
d_nextSelectionIndex = i + 1;Moreover, I'd like to add two suggestions to this post :
1. I think the first if block in findSelectedItem item is useless. Without it, the method's behavior will be exactly the same. I might be wrong, but a modern compiler wouldn't even proceed with allocation of the loop variable before doing the check on the loop condition.
2. Keeping an internal state variable inside the ItemListbox looks bad to me. Shouldn't this state be conserved in an iterator object pointing to the listbox ?
One last question : is there a way to use an iterator to cycle through an ItemListbox's selected values ? I had to write one myself for my project.
Thanks for reading !
