fix(avatar): set friend to identicon on empty avatar receipt

*All friend avatar changes and removals go through Profile, so that Profile can manage identicons
*Split the concept of "changed" and "removed" into "changed", "set", and "removed"

Fixes #4724

src/core/core.h

@@ -139,8 +139,7 @@ public slots:
     void disconnected();
 
     void friendRequestReceived(const ToxPk& friendPk, const QString& message);
-    void friendAvatarChanged(const ToxPk& friendPk, const QPixmap& pic);
-    void friendAvatarData(const ToxPk& friendPk, const QByteArray& data);
+    void friendAvatarChanged(const ToxPk& friendPk, const QByteArray& pic);
     void friendAvatarRemoved(const ToxPk& friendPk);
 
     void requestSent(const ToxPk& friendPk, const QString& message);
@@ -187,7 +186,6 @@ public slots:
     void friendUsernameChanged(uint32_t friendId, const QString& username);
     void friendTypingChanged(uint32_t friendId, bool isTyping);
 
-    void friendAvatarChangedDeprecated(uint32_t friendId, const QPixmap& pic);
     void friendRemoved(uint32_t friendId);
     void friendLastSeenChanged(uint32_t friendId, const QDateTime& dateTime);
 

src/core/corefile.cpp

@@ -470,10 +470,7 @@ void CoreFile::onFileRecvChunkCallback(Tox* tox, uint32_t friendId, uint32_t fil
             pic.loadFromData(file->avatarData);
             if (!pic.isNull()) {
                 qDebug() << "Got" << file->avatarData.size() << "bytes of avatar data from" << friendId;
-                emit core->friendAvatarData(core->getFriendPublicKey(friendId), file->avatarData);
-                emit core->friendAvatarChanged(core->getFriendPublicKey(friendId), pic);
-                // TODO(sudden6): signal below is deprecated
-                emit core->friendAvatarChangedDeprecated(friendId, pic);
+                emit core->friendAvatarChanged(core->getFriendPublicKey(friendId), file->avatarData);
             }
         } else {
             emit core->fileTransferFinished(*file);

src/model/about/aboutfriend.cpp

@@ -34,9 +34,9 @@ QString AboutFriend::getStatusMessage() const
     return f->getStatusMessage();
 }
 
-QString AboutFriend::getPublicKey() const
+ToxPk AboutFriend::getPublicKey() const
 {
-    return f->getPublicKey().toString();
+    return f->getPublicKey();
 }
 
 QPixmap AboutFriend::getAvatar() const

src/model/about/aboutfriend.h

@@ -19,7 +19,7 @@ class AboutFriend : public IAboutFriend
 
     QString getName() const override;
     QString getStatusMessage() const override;
-    QString getPublicKey() const override;
+    ToxPk getPublicKey() const override;
 
     QPixmap getAvatar() const override;
 

src/model/about/iaboutfriend.h

@@ -12,7 +12,7 @@ class IAboutFriend : public QObject
 public:
     virtual QString getName() const = 0;
     virtual QString getStatusMessage() const = 0;
-    virtual QString getPublicKey() const = 0;
+    virtual ToxPk getPublicKey() const = 0;
 
     virtual QPixmap getAvatar() const = 0;
 

src/persistence/profile.cpp

@@ -76,7 +76,7 @@ void Profile::initCore(const QByteArray& toxsave, ICoreSettings& s)
     connect(core.get(), &Core::saveRequest, this, &Profile::onSaveToxSave);
     // react to avatar changes
     connect(core.get(), &Core::friendAvatarRemoved, this, &Profile::removeAvatar);
-    connect(core.get(), &Core::friendAvatarData, this, &Profile::saveAvatar);
+    connect(core.get(), &Core::friendAvatarChanged, this, &Profile::setFriendAvatar);
     connect(core.get(), &Core::fileAvatarOfferReceived, this, &Profile::onAvatarOfferReceived,
             Qt::ConnectionType::QueuedConnection);
 }
@@ -499,7 +499,6 @@ void Profile::loadDatabase(const ToxId& id, QString password)
 /**
  * @brief Sets our own avatar
  * @param pic Picture to use as avatar, if empty an Identicon will be used depending on settings
- * @param owner
  */
 void Profile::setAvatar(QByteArray pic)
 {
@@ -511,7 +510,6 @@ void Profile::setAvatar(QByteArray pic)
         avatarData = pic;
     } else {
         if (Settings::getInstance().getShowIdenticons()) {
-            // with IDENTICON_ROWS=5 this gives a 160x160 image file
             const QImage identicon = Identicon(selfPk.getKey()).toImage(32);
             pixmap = QPixmap::fromImage(identicon);
 
@@ -527,6 +525,32 @@ void Profile::setAvatar(QByteArray pic)
     AvatarBroadcaster::enableAutoBroadcast();
 }
 
+
+/**
+ * @brief Sets a friends avatar
+ * @param pic Picture to use as avatar, if empty an Identicon will be used depending on settings
+ * @param owner pk of friend
+ */
+void Profile::setFriendAvatar(const ToxPk& owner, QByteArray pic)
+{
+    QPixmap pixmap;
+    QByteArray avatarData;
+    if (!pic.isEmpty()) {
+        pixmap.loadFromData(pic);
+        avatarData = pic;
+        emit friendAvatarSet(owner, pixmap);
+    } else if (Settings::getInstance().getShowIdenticons()) {
+        const QImage identicon = Identicon(owner.getKey()).toImage(32);
+        pixmap = QPixmap::fromImage(identicon);
+        emit friendAvatarSet(owner, pixmap);
+    } else {
+        pixmap.load(":/img/contact_dark.svg");
+        emit friendAvatarRemoved(owner);
+    }
+    friendAvatarChanged(owner, pixmap);
+    saveAvatar(owner, avatarData);
+}
+
 /**
  * @brief Adds history message about friendship request attempt if history is enabled
  * @param friendPk Pk of a friend which request is destined to
@@ -591,6 +615,14 @@ void Profile::removeSelfAvatar()
     removeAvatar(core->getSelfId().getPublicKey());
 }
 
+/**
+ * @brief Removes friend avatar.
+ */
+void Profile::removeFriendAvatar(const ToxPk& owner)
+{
+    removeAvatar(owner);
+}
+
 /**
  * @brief Checks that the history is enabled in the settings, and loaded successfully for this
  * profile.
@@ -619,6 +651,8 @@ void Profile::removeAvatar(const ToxPk& owner)
     QFile::remove(avatarPath(owner));
     if (owner == core->getSelfId().getPublicKey()) {
         setAvatar({});
+    } else {
+        setFriendAvatar(owner, {});
     }
 }
 

src/persistence/profile.h

@@ -57,9 +57,10 @@ class Profile : public QObject
     QPixmap loadAvatar(const ToxPk& owner);
     QByteArray loadAvatarData(const ToxPk& owner);
     void setAvatar(QByteArray pic);
+    void setFriendAvatar(const ToxPk& owner, QByteArray pic);
     QByteArray getAvatarHash(const ToxPk& owner);
     void removeSelfAvatar();
-
+    void removeFriendAvatar(const ToxPk& owner);
     bool isHistoryEnabled();
     History* getHistory();
 
@@ -76,7 +77,12 @@ class Profile : public QObject
 
 signals:
     void selfAvatarChanged(const QPixmap& pixmap);
-
+    // emit on any change, including default avatar. Used by those that don't care about active on default avatar.
+    void friendAvatarChanged(const ToxPk& friendPk, const QPixmap& pixmap);
+    // emit on a set of avatar, including identicon, used by those two care about active for default, so can't use friendAvatarChanged
+    void friendAvatarSet(const ToxPk& friendPk, const QPixmap& pixmap);
+    // emit on set to default, used by those that modify on active
+    void friendAvatarRemoved(const ToxPk& friendPk);
     // TODO(sudden6): this doesn't seem to be the right place for Core errors
     void failedToStart();
     void badProxy();

src/video/groupnetcamview.cpp

@@ -20,6 +20,7 @@
 #include "groupnetcamview.h"
 #include "src/audio/audio.h"
 #include "src/core/core.h"
+#include "src/core/toxpk.h"
 #include "src/friendlist.h"
 #include "src/model/friend.h"
 #include "src/nexus.h"
@@ -154,7 +155,7 @@ GroupNetCamView::GroupNetCamView(int group, QWidget* parent)
         setActive();
     });
 
-    connect(Core::getInstance(), &Core::friendAvatarChangedDeprecated, this,
+    connect(Nexus::getProfile(), &Profile::friendAvatarChanged, this,
             &GroupNetCamView::friendAvatarChanged);
 
     selfVideoSurface->setText(Core::getInstance()->getUsername());
@@ -230,9 +231,8 @@ void GroupNetCamView::setActive(const ToxPk& peer)
 #endif
 }
 
-void GroupNetCamView::friendAvatarChanged(int friendId, const QPixmap& pixmap)
+void GroupNetCamView::friendAvatarChanged(ToxPk friendPk, const QPixmap& pixmap)
 {
-    const auto friendPk = Core::getInstance()->getFriendPublicKey(friendId);
     auto peerVideo = videoList.find(friendPk);
 
     if (peerVideo != videoList.end()) {

src/video/groupnetcamview.h

@@ -39,7 +39,7 @@ class GroupNetCamView : public GenericNetCamView
 
 private slots:
     void onUpdateActivePeer();
-    void friendAvatarChanged(int friendId, const QPixmap& pixmap);
+    void friendAvatarChanged(ToxPk friendPk, const QPixmap& pixmap);
 
 private:
     struct PeerVideo

src/video/netcamview.cpp

@@ -34,11 +34,10 @@
 NetCamView::NetCamView(int friendId, QWidget* parent)
     : GenericNetCamView(parent)
     , selfFrame{nullptr}
-    , friendId{friendId}
+    , friendPk{FriendList::findFriend(friendId)->getPublicKey()}
     , e(false)
 {
-    const ToxPk pk = FriendList::findFriend(friendId)->getPublicKey();
-    videoSurface = new VideoSurface(Nexus::getProfile()->loadAvatar(pk), this);
+    videoSurface = new VideoSurface(Nexus::getProfile()->loadAvatar(friendPk), this);
     videoSurface->setMinimumHeight(256);
 
     verLayout->insertWidget(0, videoSurface, 1);
@@ -75,9 +74,9 @@ NetCamView::NetCamView(int friendId, QWidget* parent)
     connections += connect(Nexus::getProfile(), &Profile::selfAvatarChanged,
                            [this](const QPixmap& pixmap) { selfVideoSurface->setAvatar(pixmap); });
 
-    connections += connect(Core::getInstance(), &Core::friendAvatarChangedDeprecated,
-                           [this](int FriendId, const QPixmap& pixmap) {
-                               if (this->friendId == FriendId)
+    connections += connect(Nexus::getProfile(), &Profile::friendAvatarChanged,
+                           [this](ToxPk friendPk, const QPixmap& pixmap) {
+                               if (this->friendPk == friendPk)
                                    videoSurface->setAvatar(pixmap);
                            });
 

src/video/netcamview.h

@@ -20,6 +20,7 @@
 #ifndef NETCAMVIEW_H
 #define NETCAMVIEW_H
 
+#include "src/core/toxpk.h"
 #include "genericnetcamview.h"
 #include <QVector>
 
@@ -55,7 +56,7 @@ private slots:
 
     VideoSurface* selfVideoSurface;
     MovableWidget* selfFrame;
-    int friendId;
+    ToxPk friendPk;
     bool e;
     QVector<QMetaObject::Connection> connections;
 };

src/widget/about/aboutfriendform.cpp

@@ -1,5 +1,6 @@
 #include "aboutfriendform.h"
 #include "ui_aboutfriendform.h"
+#include "src/core/core.h"
 
 #include <QFileDialog>
 #include <QMessageBox>
@@ -37,7 +38,7 @@ AboutFriendForm::AboutFriendForm(std::unique_ptr<IAboutFriend> _about, QWidget*
     const QString name = about->getName();
     setWindowTitle(name);
     ui->userName->setText(name);
-    ui->publicKey->setText(about->getPublicKey());
+    ui->publicKey->setText(about->getPublicKey().toString());
     ui->publicKey->setCursorPosition(0); // scroll textline to left
     ui->note->setPlainText(about->getNote());
     ui->statusMessage->setText(about->getStatusMessage());

src/widget/form/chatform.cpp

@@ -157,10 +157,9 @@ ChatForm::ChatForm(Friend* chatFriend, History* history)
         menu.addAction(QIcon::fromTheme("document-save"), QString(), this, SLOT(onExportChat()));
 
     const Core* core = Core::getInstance();
+    const Profile* profile = Nexus::getProfile();
     connect(core, &Core::fileReceiveRequested, this, &ChatForm::onFileRecvRequest);
-    // TODO(sudden6): update slot to new API
-    connect(core, &Core::friendAvatarChangedDeprecated, this, &ChatForm::onAvatarChange);
-    connect(core, &Core::friendAvatarRemoved, this, &ChatForm::onAvatarRemoved);
+    connect(profile, &Profile::friendAvatarChanged, this, &ChatForm::onAvatarChanged);
     connect(core, &Core::fileSendStarted, this, &ChatForm::startFileSend);
     connect(core, &Core::fileSendFailed, this, &ChatForm::onFileSendFailed);
     connect(core, &Core::receiptRecieved, this, &ChatForm::onReceiptReceived);
@@ -652,9 +651,9 @@ void ChatForm::onReceiptReceived(quint32 friendId, int receipt)
     }
 }
 
-void ChatForm::onAvatarChange(uint32_t friendId, const QPixmap& pic)
+void ChatForm::onAvatarChanged(const ToxPk &friendPk, const QPixmap& pic)
 {
-    if (friendId != f->getId()) {
+    if (friendPk != f->getPublicKey()) {
         return;
     }
 
@@ -726,15 +725,6 @@ void ChatForm::dropEvent(QDropEvent* ev)
     }
 }
 
-void ChatForm::onAvatarRemoved(const ToxPk& friendPk)
-{
-    if (friendPk != f->getPublicKey()) {
-        return;
-    }
-
-    headWidget->setAvatar(QPixmap(":/img/contact_dark.svg"));
-}
-
 void ChatForm::clearChatArea(bool notInForm)
 {
     GenericChatForm::clearChatArea(notInForm);

src/widget/form/chatform.h

@@ -72,8 +72,7 @@ public slots:
     void onAvInvite(uint32_t friendId, bool video);
     void onAvStart(uint32_t friendId, bool video);
     void onAvEnd(uint32_t friendId, bool error);
-    void onAvatarChange(uint32_t friendId, const QPixmap& pic);
-    void onAvatarRemoved(const ToxPk& friendPk);
+    void onAvatarChanged(const ToxPk &friendPk, const QPixmap& pic);
     void onFileNameChanged(const ToxPk& friendPk);
 
 protected slots:

src/widget/friendwidget.cpp

@@ -389,7 +389,7 @@ void FriendWidget::resetEventFlags()
     chatroom->resetEventFlags();
 }
 
-void FriendWidget::onAvatarChange(const ToxPk& friendPk, const QPixmap& pic)
+void FriendWidget::onAvatarSet(const ToxPk& friendPk, const QPixmap& pic)
 {
     const auto frnd = chatroom->getFriend();
     if (friendPk != frnd->getPublicKey()) {

src/widget/friendwidget.h

@@ -50,7 +50,7 @@ class FriendWidget : public GenericChatroomWidget
     void contextMenuCalled(QContextMenuEvent* event);
 
 public slots:
-    void onAvatarChange(const ToxPk& friendPk, const QPixmap& pic);
+    void onAvatarSet(const ToxPk& friendPk, const QPixmap& pic);
     void onAvatarRemoved(const ToxPk& friendPk);
     void onContextMenuCalled(QContextMenuEvent* event);
     void setActive(bool active);

src/widget/widget.cpp

@@ -1018,16 +1018,16 @@ void Widget::addFriend(uint32_t friendId, const ToxPk& friendPk)
     connect(widget, &FriendWidget::copyFriendIdToClipboard, this, &Widget::copyFriendIdToClipboard);
     connect(widget, &FriendWidget::contextMenuCalled, widget, &FriendWidget::onContextMenuCalled);
     connect(widget, SIGNAL(removeFriend(int)), this, SLOT(removeFriend(int)));
-
-    Core* core = Core::getInstance();
-    connect(core, &Core::friendAvatarChanged, widget, &FriendWidget::onAvatarChange);
-    connect(core, &Core::friendAvatarRemoved, widget, &FriendWidget::onAvatarRemoved);
+    
+    Profile* profile = Nexus::getProfile();
+    connect(profile, &Profile::friendAvatarSet, widget, &FriendWidget::onAvatarSet);
+    connect(profile, &Profile::friendAvatarRemoved, widget, &FriendWidget::onAvatarRemoved);
 
     // Try to get the avatar from the cache
     QPixmap avatar = Nexus::getProfile()->loadAvatar(friendPk);
     if (!avatar.isNull()) {
-        friendForm->onAvatarChange(friendId, avatar);
-        widget->onAvatarChange(friendPk, avatar);
+        friendForm->onAvatarChanged(friendPk, avatar);
+        widget->onAvatarSet(friendPk, avatar);
     }
 
     FilterCriteria filter = getFilterCriteria();
@@ -1281,13 +1281,13 @@ void Widget::addFriendDialog(const Friend* frnd, ContentDialog* dialog)
     // FIXME: emit should be removed
     emit widget->chatroomWidgetClicked(widget);
 
-    Core* core = Core::getInstance();
-    connect(core, &Core::friendAvatarChanged, friendWidget, &FriendWidget::onAvatarChange);
-    connect(core, &Core::friendAvatarRemoved, friendWidget, &FriendWidget::onAvatarRemoved);
+    Profile* profile = Nexus::getProfile();
+    connect(profile, &Profile::friendAvatarSet, friendWidget, &FriendWidget::onAvatarSet);
+    connect(profile, &Profile::friendAvatarRemoved, friendWidget, &FriendWidget::onAvatarRemoved);
 
     QPixmap avatar = Nexus::getProfile()->loadAvatar(frnd->getPublicKey());
     if (!avatar.isNull()) {
-        friendWidget->onAvatarChange(frnd->getPublicKey(), avatar);
+        friendWidget->onAvatarSet(frnd->getPublicKey(), avatar);
     }
 }