refactor: use ToxPk as identifier

fixes #5116 and #5117

src/core/toxpk.cpp

@@ -71,6 +71,16 @@ bool ToxPk::operator!=(const ToxPk& other) const
     return key != other.key;
 }
 
+/**
+ * @brief Compares two ToxPks
+ * @param other ToxPk to compare.
+ * @return True if this ToxPks is less than the other ToxPk, false otherwise.
+ */
+bool ToxPk::operator<(const ToxPk& other) const
+{
+    return key < other.key;
+}
+
 /**
  * @brief Converts the ToxPk to a uppercase hex string.
  * @return QString containing the hex representation of the key

src/core/toxpk.h

@@ -17,6 +17,7 @@ public:
 
     bool operator==(const ToxPk& other) const;
     bool operator!=(const ToxPk& other) const;
+    bool operator<(const ToxPk& other) const;
     QString toString() const;
     QByteArray getKey() const;
     const uint8_t* getBytes() const;

src/model/group.cpp

@@ -32,7 +32,6 @@ Group::Group(int groupId, const QString& name, bool isAvGroupchat, const QString
     : selfName{selfName}
     , title{name}
     , groupId(groupId)
-    , nPeers{0}
     , avGroupchat{isAvGroupchat}
 {
     // in groupchats, we only notify on messages containing your name <-- dumb
@@ -46,15 +45,14 @@ Group::Group(int groupId, const QString& name, bool isAvGroupchat, const QString
 void Group::updatePeer(int peerId, QString name)
 {
     ToxPk peerKey = Core::getInstance()->getGroupPeerPk(groupId, peerId);
-    QByteArray peerPk = peerKey.getKey();
+    toxpks[peerKey] = name;
-    toxids[peerPk] = name;
 
     Friend* f = FriendList::findFriend(peerKey);
     if (f != nullptr) {
         // use the displayed name from the friends list
-        toxids[peerPk] = f->getDisplayedName();
+        toxpks[peerKey] = f->getDisplayedName();
     } else {
-        emit userListChanged(groupId, toxids);
+        emit userListChanged(groupId, toxpks);
     }
 }
 
@@ -89,34 +87,42 @@ QString Group::getDisplayedName() const
     return getName();
 }
 
+/**
+ * @brief performs a peerId to ToxPk lookup
+ * @param peerId peerId to lookup
+ * @return ToxPk if peerId found
+ * @note should not be used, reference peers by their ToxPk instead
+ * @todo remove this function
+ */
+const ToxPk Group::resolvePeerId(int peerId) const
+{
+    const Core* core = Core::getInstance();
+    return core->getGroupPeerPk(groupId, peerId);
+}
+
 void Group::regeneratePeerList()
 {
     const Core* core = Core::getInstance();
-    const ToxPk self = core->getSelfId().getPublicKey();
 
     QStringList peers = core->getGroupPeerNames(groupId);
-    toxids.clear();
+    toxpks.clear();
-    nPeers = peers.size();
+    const int nPeers = peers.size();
     for (int i = 0; i < nPeers; ++i) {
-        ToxPk id = core->getGroupPeerPk(groupId, i);
+        const auto pk = core->getGroupPeerPk(groupId, i);
-        if (id == self) {
-            selfPeerNum = i;
-        }
 
-        QByteArray peerPk = id.getKey();
+        toxpks[pk] = peers[i];
-        toxids[peerPk] = peers[i];
+        if (toxpks[pk].isEmpty()) {
-        if (toxids[peerPk].isEmpty()) {
+            toxpks[pk] =
-            toxids[peerPk] =
                 tr("<Empty>", "Placeholder when someone's name in a group chat is empty");
         }
 
-        Friend* f = FriendList::findFriend(id);
+        Friend* f = FriendList::findFriend(pk);
         if (f != nullptr && f->hasAlias()) {
-            toxids[peerPk] = f->getDisplayedName();
+            toxpks[pk] = f->getDisplayedName();
         }
     }
 
-    emit userListChanged(groupId, toxids);
+    emit userListChanged(groupId, toxpks);
 }
 
 bool Group::isAvGroupchat() const
@@ -131,17 +137,16 @@ uint32_t Group::getId() const
 
 int Group::getPeersCount() const
 {
-    return nPeers;
+    return toxpks.size();
 }
 
-QStringList Group::getPeerList() const
+/**
+ * @brief Gets the PKs and names of all peers
+ * @return PKs and names of all peers, including our own PK and name
+ */
+const QMap<ToxPk, QString>& Group::getPeerList() const
 {
-    return toxids.values();
+    return toxpks;
-}
-
-bool Group::isSelfPeerNumber(int num) const
-{
-    return num == selfPeerNum;
 }
 
 void Group::setEventFlag(bool f)
@@ -166,10 +171,9 @@ bool Group::getMentionedFlag() const
 
 QString Group::resolveToxId(const ToxPk& id) const
 {
-    QByteArray key = id.getKey();
+    auto it = toxpks.find(id);
-    auto it = toxids.find(key);
 
-    if (it != toxids.end()) {
+    if (it != toxpks.end()) {
         return *it;
     }
 

src/model/group.h

@@ -21,14 +21,13 @@
 #define GROUP_H
 
 #include "contact.h"
+
+#include "src/core/toxpk.h"
+
 #include <QMap>
 #include <QObject>
 #include <QStringList>
 
-#define RETRY_PEER_INFO_INTERVAL 500
-
-class ToxPk;
-
 class Group : public Contact
 {
     Q_OBJECT
@@ -39,8 +38,7 @@ public:
     uint32_t getId() const override;
     int getPeersCount() const;
     void regeneratePeerList();
-    QStringList getPeerList() const;
+    const QMap<ToxPk, QString>& getPeerList() const;
-    bool isSelfPeerNumber(int peernumber) const;
 
     void setEventFlag(bool f) override;
     bool getEventFlag() const override;
@@ -54,23 +52,22 @@ public:
     QString getName() const;
     QString getDisplayedName() const override;
 
+    const ToxPk resolvePeerId(int peerId) const;
     QString resolveToxId(const ToxPk& id) const;
     void setSelfName(const QString& name);
 
 signals:
     void titleChangedByUser(uint32_t groupId, const QString& title);
     void titleChanged(uint32_t groupId, const QString& author, const QString& title);
-    void userListChanged(uint32_t groupId, const QMap<QByteArray, QString>& toxids);
+    void userListChanged(uint32_t groupId, const QMap<ToxPk, QString>& toxpks);
 
 private:
     QString selfName;
     QString title;
-    QMap<QByteArray, QString> toxids;
+    QMap<ToxPk, QString> toxpks;
     bool hasNewMessages;
     bool userWasMentioned;
     int groupId;
-    int nPeers;
-    int selfPeerNum = -1;
     bool avGroupchat;
 };
 

src/video/groupnetcamview.cpp

@@ -152,6 +152,7 @@ GroupNetCamView::GroupNetCamView(int group, QWidget* parent)
         selfVideoSurface->setText(username);
         setActive();
     });
+
     connect(Core::getInstance(), &Core::friendAvatarChanged, this,
             &GroupNetCamView::friendAvatarChanged);
 
@@ -160,16 +161,14 @@ GroupNetCamView::GroupNetCamView(int group, QWidget* parent)
 
 void GroupNetCamView::clearPeers()
 {
-    QList<int> keys = videoList.keys();
+    for (const auto& peerPk : videoList.keys()) {
-
+        removePeer(peerPk);
-    for (int& i : keys)
+    }
-        removePeer(i);
 }
 
-void GroupNetCamView::addPeer(int peer, const QString& name)
+void GroupNetCamView::addPeer(const ToxPk& peer, const QString& name)
 {
-    QPixmap groupAvatar =
+    QPixmap groupAvatar = Nexus::getProfile()->loadAvatar(peer);
-        Nexus::getProfile()->loadAvatar(Core::getInstance()->getGroupPeerPk(group, peer));
     LabeledVideo* labeledVideo = new LabeledVideo(groupAvatar, this);
     labeledVideo->setText(name);
     horLayout->insertWidget(horLayout->count() - 1, labeledVideo);
@@ -180,7 +179,7 @@ void GroupNetCamView::addPeer(int peer, const QString& name)
     setActive();
 }
 
-void GroupNetCamView::removePeer(int peer)
+void GroupNetCamView::removePeer(const ToxPk& peer)
 {
     auto peerVideo = videoList.find(peer);
 
@@ -199,14 +198,16 @@ void GroupNetCamView::onUpdateActivePeer()
     setActive();
 }
 
-void GroupNetCamView::setActive(int peer)
+void GroupNetCamView::setActive(const ToxPk& peer)
 {
-    if (peer == -1) {
+    if (peer.isEmpty()) {
         videoLabelSurface->setText(selfVideoSurface->getText());
         activePeer = -1;
         return;
     }
 
+    // TODO(sudden6): check if we can remove the code, it won't be reached right now
+#if 0
     auto peerVideo = videoList.find(peer);
 
     if (peerVideo != videoList.end()) {
@@ -225,22 +226,16 @@ void GroupNetCamView::setActive(int peer)
 
         activePeer = peer;
     }
+#endif
 }
 
-void GroupNetCamView::friendAvatarChanged(int FriendId, const QPixmap& pixmap)
+void GroupNetCamView::friendAvatarChanged(int friendId, const QPixmap& pixmap)
 {
-    Friend* f = FriendList::findFriend(FriendId);
+    const auto friendPk = Core::getInstance()->getFriendPublicKey(friendId);
+    auto peerVideo = videoList.find(friendPk);
 
-    for (uint32_t i = 0; i < Core::getInstance()->getGroupNumberPeers(group); ++i) {
+    if (peerVideo != videoList.end()) {
-        if (Core::getInstance()->getGroupPeerPk(group, i) == f->getPublicKey()) {
+        peerVideo.value().video->getVideoSurface()->setAvatar(pixmap);
-            auto peerVideo = videoList.find(i);
+        setActive();
-
-            if (peerVideo != videoList.end()) {
-                peerVideo.value().video->getVideoSurface()->setAvatar(pixmap);
-                setActive();
-            }
-
-            break;
-        }
     }
 }

src/video/groupnetcamview.h

@@ -21,6 +21,9 @@
 #define GROUPNETCAMVIEW_H
 
 #include "genericnetcamview.h"
+
+#include "src/core/toxpk.h"
+
 #include <QMap>
 
 class LabeledVideo;
@@ -31,15 +34,12 @@ class GroupNetCamView : public GenericNetCamView
 public:
     GroupNetCamView(int group, QWidget* parent = 0);
     void clearPeers();
-    void addPeer(int peer, const QString& name);
+    void addPeer(const ToxPk& peer, const QString& name);
-    void removePeer(int peer);
+    void removePeer(const ToxPk& peer);
-
-public slots:
-    void groupAudioPlayed(int group, int peer, unsigned short volume);
 
 private slots:
     void onUpdateActivePeer();
-    void friendAvatarChanged(int FriendId, const QPixmap& pixmap);
+    void friendAvatarChanged(int friendId, const QPixmap& pixmap);
 
 private:
     struct PeerVideo
@@ -47,10 +47,10 @@ private:
         LabeledVideo* video;
     };
 
-    void setActive(int peer = -1);
+    void setActive(const ToxPk& peer = ToxPk{});
 
     QHBoxLayout* horLayout;
-    QMap<int, PeerVideo> videoList;
+    QMap<ToxPk, PeerVideo> videoList;
     LabeledVideo* videoLabelSurface;
     LabeledVideo* selfVideoSurface;
     int activePeer;

src/widget/form/groupchatform.cpp

@@ -223,20 +223,19 @@ void GroupChatForm::updateUserNames()
     }
 
     peerLabels.clear();
-    const int peersCount = group->getPeersCount();
+    const auto peers = group->getPeerList();
 
-    // no need to do anything without a peer
+    // no need to do anything without any peers
-    if (peersCount == 0) {
+    if (peers.isEmpty()) {
         return;
     }
-    peerLabels.reserve(peersCount);
-    QVector<QLabel*> nickLabelList(peersCount);
 
-    /* the list needs peers in peernumber order, nameLayout needs alphabetical
+    /* we store the peer labels by their ToxPk, but the namelist layout
-     * first traverse in peer number order, storing the QLabels as necessary */
+     * needs it in alphabetical order, so we first create and store the labels
-    const QStringList names = group->getPeerList();
+     * and then sort them by their text and add them to the layout in that order */
-    int peerNumber = 0;
+    const auto selfPk = Core::getInstance()->getSelfPublicKey();
-    for (const QString& fullName : names) {
+    for (const auto& peerPk : peers.keys()) {
+        const QString fullName = peers.value(peerPk);
         const QString editedName = editName(fullName).append(QLatin1String(", "));
         QLabel* const label = new QLabel(editedName);
         if (editedName != fullName) {
@@ -245,24 +244,24 @@ void GroupChatForm::updateUserNames()
         label->setTextFormat(Qt::PlainText);
 
         const Settings& s = Settings::getInstance();
-        const Core* core = Core::getInstance();
-        const ToxPk peerPk = core->getGroupPeerPk(group->getId(), peerNumber);
 
-        if (group->isSelfPeerNumber(peerNumber)) {
+        if (peerPk == selfPk) {
             label->setStyleSheet(QStringLiteral("QLabel {color : green;}"));
         } else if (s.getBlackList().contains(peerPk.toString())) {
             label->setStyleSheet(QStringLiteral("QLabel {color : darkRed;}"));
         } else if (netcam != nullptr) {
-            static_cast<GroupNetCamView*>(netcam)->addPeer(peerNumber, fullName);
+            static_cast<GroupNetCamView*>(netcam)->addPeer(peerPk, fullName);
         }
-        peerLabels.append(label);
+        peerLabels.insert(peerPk, label);
-        nickLabelList[peerNumber++] = label;
     }
 
     if (netcam != nullptr) {
         static_cast<GroupNetCamView*>(netcam)->clearPeers();
     }
 
+    // add the labels in alphabetical order into the layout
+    auto nickLabelList = peerLabels.values();
+
     qSort(nickLabelList.begin(), nickLabelList.end(), [](const QLabel* a, const QLabel* b)
     {
         return a->text().toLower() < b->text().toLower();
@@ -278,30 +277,30 @@ void GroupChatForm::updateUserNames()
     }
 }
 
-void GroupChatForm::peerAudioPlaying(int peer)
+void GroupChatForm::peerAudioPlaying(ToxPk peerPk)
 {
-    peerLabels[peer]->setStyleSheet(QStringLiteral("QLabel {color : red;}"));
+    peerLabels[peerPk]->setStyleSheet(QStringLiteral("QLabel {color : red;}"));
-    if (!peerAudioTimers[peer]) {
+    // TODO(sudden6): check if this can ever be false, cause [] default constructs
-        peerAudioTimers[peer] = new QTimer(this);
+    if (!peerAudioTimers[peerPk]) {
-        peerAudioTimers[peer]->setSingleShot(true);
+        peerAudioTimers[peerPk] = new QTimer(this);
-        connect(peerAudioTimers[peer], &QTimer::timeout, [this, peer] {
+        peerAudioTimers[peerPk]->setSingleShot(true);
-            if (netcam)
+        connect(peerAudioTimers[peerPk], &QTimer::timeout, [this, peerPk] {
-                static_cast<GroupNetCamView*>(netcam)->removePeer(peer);
+            if (netcam) {
+                static_cast<GroupNetCamView*>(netcam)->removePeer(peerPk);
+            }
 
-            if (peer >= peerLabels.size())
+            peerLabels[peerPk]->setStyleSheet("");
-                return;
+            delete peerAudioTimers[peerPk];
-
+            peerAudioTimers[peerPk] = nullptr;
-            peerLabels[peer]->setStyleSheet("");
-            delete peerAudioTimers[peer];
-            peerAudioTimers[peer] = nullptr;
         });
 
         if (netcam) {
-            static_cast<GroupNetCamView*>(netcam)->removePeer(peer);
+            static_cast<GroupNetCamView*>(netcam)->removePeer(peerPk);
-            static_cast<GroupNetCamView*>(netcam)->addPeer(peer, group->getPeerList()[peer]);
+            const auto nameIt = group->getPeerList().find(peerPk);
+            static_cast<GroupNetCamView*>(netcam)->addPeer(peerPk, nameIt.value());
         }
     }
-    peerAudioTimers[peer]->start(500);
+    peerAudioTimers[peerPk]->start(500);
 }
 
 void GroupChatForm::dragEnterEvent(QDragEnterEvent* ev)
@@ -380,10 +379,12 @@ GenericNetCamView* GroupChatForm::createNetcam()
 {
     GroupNetCamView* view = new GroupNetCamView(group->getId(), this);
 
-    QStringList names = group->getPeerList();
+    const auto& names = group->getPeerList();
-    for (int i = 0; i < names.size(); ++i) {
+    const auto ownPk = Core::getInstance()->getSelfPublicKey();
-        if (!group->isSelfPeerNumber(i))
+    for (const auto& peerPk : names.keys()) {
-            static_cast<GroupNetCamView*>(view)->addPeer(i, names[i]);
+        if (peerPk != ownPk) {
+            static_cast<GroupNetCamView*>(view)->addPeer(peerPk, names.find(peerPk).value());
+        }
     }
 
     return view;

src/widget/form/groupchatform.h

@@ -21,6 +21,7 @@
 #define GROUPCHATFORM_H
 
 #include "genericchatform.h"
+#include "src/core/toxpk.h"
 #include <QMap>
 
 namespace Ui {
@@ -38,7 +39,7 @@ public:
     explicit GroupChatForm(Group* chatGroup);
     ~GroupChatForm();
 
-    void peerAudioPlaying(int peer);
+    void peerAudioPlaying(ToxPk peerPk);
 
 private slots:
     void onSendTriggered() override;
@@ -67,8 +68,8 @@ private:
 
 private:
     Group* group;
-    QVector<QLabel*> peerLabels;
+    QMap<ToxPk, QLabel*> peerLabels;
-    QMap<int, QTimer*> peerAudioTimers;
+    QMap<ToxPk, QTimer*> peerAudioTimers;
     FlowLayout* namesListLayout;
     QLabel* nusersLabel;
     TabCompleter* tabber;

src/widget/widget.cpp

@@ -1835,7 +1835,8 @@ void Widget::onGroupPeerAudioPlaying(int groupnumber, int peernumber)
     }
 
     auto form = groupChatForms[g->getId()];
-    form->peerAudioPlaying(peernumber);
+    // TODO(sudden6): switch to ToxPk here
+    form->peerAudioPlaying(g->resolvePeerId(peernumber));
 }
 
 void Widget::removeGroup(Group* g, bool fake)