[Kde-games-devel] KChatBase. Let's discuss

[Alexander Smirnov]

Hello kdegamers!

I work on kbackgammon and faced with strange pieces of code in 
KChatBase. Since i'm very new to kdegames development and have doubts i 
would ask you for help.

I want to discuss two functions: sendingEntry() and insertSendingEntry.

bool KChatBase::insertSendingEntry(const QString& text, int id, int index)
{
 if (!d->mCombo) {
    kWarning(11000) << "KChatBase: Cannot add an entry to the combo box";
    return false;
 }
 if (d->mIndex2Id.indexOf(id) != -1) {
    kError(11000) << "KChatBase: Cannot add more than one entry with the 
same ID! ";
    kError(11000) << "KChatBase: Text="<<text;
    return false;
 }
 d->mCombo->insertItem(index, text); //(1)
 if (index < 0) {
    d->mIndex2Id.append(id); // (2)
 } else {
    d->mIndex2Id.insert(d->mIndex2Id.at(index), id);
 }
 if (d->mIndex2Id.count() != d->mCombo->count()) {
    kError(11000) << "KChatBase: internal ERROR - local IDs do not match 
combo box entries!";
 }
 return true;
}

this function is intended to add combo box items and map them with 
player's id. Mapping is done in QList<String> mIndex2Id. Function 
arguments are: combo text, user's id and index of item in combo box 
which has default value = -1.
In all cases i've seen this function uses default value for last 
argument. When inserting item in combo box (1) with negative index, the 
item becomes first in drop-down list. But by some reason id is stored at 
the end of list mIndex2Id, that causes broken mapping index->user's id.
So i would rather prefer _prepending_ to _appending_ here:
    d->mIndex2Id.prepend(id); //2

the second magic function is sendingEntry:

int KChatBase::sendingEntry() const
{
 if (!d->mCombo) {
    kWarning(11001) << "Cannot retrieve index from NULL combo box";
    return -1;
 }
 int index = d->mCombo->currentIndex(); //(1)

 if ( index > 0 && index <  d->mIndex2Id.size()) { // (2)
    kWarning(11000) << "could not find the selected sending entry! 2nd 
user id=" << d->mIndex2Id[index];
    return -1;
 }
 return d->mIndex2Id[index]; // (3)
}

it is intended to return user's id for current combo box selection. (1) 
retrieves current combo box index and then, using index->user_id map 
function tries to return id.
But actually we have strange behavior here(2): if current index is in 
allowed bounds, then return -1!!! otherwise (incorrect bounds) try to 
return value from map. (3) should cause index out of bounds error.

I wonder how it works :) I suspect that nobody except kbackgammon uses 
this code, and that's why it hasn't been noticed earlier.
Or may be i missed something?
i'm going to submit the fixes soon, but afraid to broke something 
somethere :) I found that several games use KChat functionality. How 
they live with such bugs?
i would be glad to comments on this stuff from you, especially if you 
use KChat in your games.

Thank you,
-Alexander.