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

[Albert Astals Cid]

A Dimarts 03 Juny 2008, Alexander Smirnov va escriure:
> 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

Porting bug, negative index in qcombobox appended in Qt3

>
> 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)
> }

Porting bug, kde3 if condition is different

>
> 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.

Right, the only one that seems to use kchatbase is ksirk through kgamechat, 
probably never hits this problems or is broken too.

> Or may be i missed something?

Both problems seem correctly spotted for me.

> i'm going to submit the fixes soon, but afraid to broke something
> somethere :) I found that several games use KChat functionality. 

Really? Which ones?

Albert

> 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.
> _______________________________________________
> kde-games-devel mailing list
> kde-games-devel at kde.org
> https://mail.kde.org/mailman/listinfo/kde-games-devel