cleanup of AvatarManger API

interface/src/avatar/AvatarManager.cpp

@@ -92,22 +92,18 @@ void AvatarManager::updateOtherAvatars(float deltaTime) {
     // simulate avatars
     AvatarHash::iterator avatarIterator = _avatarHash.begin();
     while (avatarIterator != _avatarHash.end()) {
-        AvatarSharedPointer sharedAvatar = avatarIterator.value();
+        Avatar* avatar = reinterpret_cast<Avatar*>(avatarIterator.value().data());
-        Avatar* avatar = reinterpret_cast<Avatar*>(sharedAvatar.data());
         
-        if (sharedAvatar == _myAvatar || !avatar->isInitialized()) {
+        if (avatar == _myAvatar || !avatar->isInitialized()) {
             // DO NOT update _myAvatar!  Its update has already been done earlier in the main loop.
-            // DO NOT update uninitialized Avatars
+            // DO NOT update or fade out uninitialized Avatars
             ++avatarIterator;
-            continue;
+        } else if (avatar->shouldDie()) {
-        }
+            _avatarFades.push_back(avatarIterator.value());
-        if (!shouldKillAvatar(sharedAvatar)) {
+            avatarIterator = _avatarHash.erase(avatarIterator);
-            // this avatar's mixer is still around, go ahead and simulate it
+        } else {
             avatar->simulate(deltaTime);
             ++avatarIterator;
-        } else {
-            // the mixer that owned this avatar is gone, give it to the vector of fades and kill it
-            avatarIterator = erase(avatarIterator);
         }
     }
     
@@ -175,24 +171,37 @@ AvatarSharedPointer AvatarManager::newSharedAvatar() {
     return AvatarSharedPointer(new Avatar());
 }
 
-AvatarHash::iterator AvatarManager::erase(const AvatarHash::iterator& iterator) {
+// virtual 
-    if (iterator.key() != MY_AVATAR_KEY) {
+AvatarSharedPointer AvatarManager::addAvatar(const QUuid& sessionUUID, const QWeakPointer<Node>& mixerWeakPointer) {
-        if (reinterpret_cast<Avatar*>(iterator.value().data())->isInitialized()) {
+    AvatarSharedPointer avatar = AvatarHashMap::addAvatar(sessionUUID, mixerWeakPointer);
-            _avatarFades.push_back(iterator.value());
+    // TODO: create MotionState for avatar and add to internal lists
+    return avatar;
+}
+
+// virtual 
+void AvatarManager::removeAvatar(const QUuid& sessionUUID) {
+    AvatarHash::iterator avatarIterator = _avatarHash.find(sessionUUID);
+    if (avatarIterator != _avatarHash.end()) {
+        Avatar* avatar = reinterpret_cast<Avatar*>(avatarIterator.value().data());
+        if (avatar != _myAvatar && avatar->isInitialized()) {
+            _avatarFades.push_back(avatarIterator.value());
+            _avatarHash.erase(avatarIterator);
         }
-        return AvatarHashMap::erase(iterator);
-    } else {
-        // never remove _myAvatar from the list
-        AvatarHash::iterator returnIterator = iterator;
-        return ++returnIterator;
     }
 }
 
 void AvatarManager::clearOtherAvatars() {
     // clear any avatars that came from an avatar-mixer
-    AvatarHash::iterator removeAvatar =  _avatarHash.begin();
+    AvatarHash::iterator avatarIterator =  _avatarHash.begin();
-    while (removeAvatar != _avatarHash.end()) {
+    while (avatarIterator != _avatarHash.end()) {
-        removeAvatar = erase(removeAvatar);
+        Avatar* avatar = reinterpret_cast<Avatar*>(avatarIterator.value().data());
+        if (avatar == _myAvatar || !avatar->isInitialized()) {
+            // don't remove myAvatar or uninitialized avatars from the list
+            ++avatarIterator;
+        } else {
+            _avatarFades.push_back(avatarIterator.value());
+            avatarIterator = _avatarHash.erase(avatarIterator);
+        }
     }
     _myAvatar->clearLookAtTargetAvatar();
 }

interface/src/avatar/AvatarManager.h

@@ -70,10 +70,10 @@ private:
     void simulateAvatarFades(float deltaTime);
     void renderAvatarFades(const glm::vec3& cameraPosition, RenderArgs::RenderMode renderMode);
     
-    virtual AvatarSharedPointer newSharedAvatar();
-    
     // virtual overrides
-    AvatarHash::iterator erase(const AvatarHash::iterator& iterator);
+    virtual AvatarSharedPointer newSharedAvatar();
+    virtual AvatarSharedPointer addAvatar(const QUuid& sessionUUID, const QWeakPointer<Node>& mixerWeakPointer);
+    virtual void removeAvatar(const QUuid& sessionUUID);
     
     QVector<AvatarSharedPointer> _avatarFades;
     QSharedPointer<MyAvatar> _myAvatar;

libraries/avatars/src/AvatarData.h

@@ -69,6 +69,7 @@ const quint32 AVATAR_MOTION_DEFAULTS =
 const quint32 AVATAR_MOTION_SCRIPTABLE_BITS = 
         AVATAR_MOTION_SCRIPTED_MOTOR_ENABLED;
 
+const qint64 AVATAR_SILENCE_THRESHOLD_USECS = 5 * USECS_PER_SECOND;
 
 // Bitset of state flags - we store the key state, hand state, faceshift, chat circling, and existance of
 // referential data in this bit set. The hand state is an octal, but is split into two sections to maintain
@@ -290,7 +291,6 @@ public:
     QString getSkeletonModelURLFromScript() const { return _skeletonModelURL.toString(); }
     void setSkeletonModelURLFromScript(const QString& skeletonModelString) { setSkeletonModelURL(QUrl(skeletonModelString)); }
     
-    Node* getOwningAvatarMixer() { return _owningAvatarMixer.data(); }
     void setOwningAvatarMixer(const QWeakPointer<Node>& owningAvatarMixer) { _owningAvatarMixer = owningAvatarMixer; }
     
     const AABox& getLocalAABox() const { return _localAABox; }
@@ -304,6 +304,8 @@ public:
     Q_INVOKABLE const glm::vec3& getVelocity() const { return _velocity; }
     const glm::vec3& getTargetVelocity() const { return _targetVelocity; }
 
+    bool shouldDie() const { return _owningAvatarMixer.isNull() || getUsecsSinceLastUpdate() > AVATAR_SILENCE_THRESHOLD_USECS; }
+
 public slots:
     void sendAvatarDataPacket();
     void sendIdentityPacket();

libraries/avatars/src/AvatarHashMap.cpp

@@ -20,19 +20,6 @@ AvatarHashMap::AvatarHashMap() {
     connect(DependencyManager::get<NodeList>().data(), &NodeList::uuidChanged, this, &AvatarHashMap::sessionUUIDChanged);
 }
 
-
-AvatarHash::iterator AvatarHashMap::erase(const AvatarHash::iterator& iterator) {
-    qCDebug(avatars) << "Removing Avatar with UUID" << iterator.key() << "from AvatarHashMap.";
-    return _avatarHash.erase(iterator);
-}
-
-const qint64 AVATAR_SILENCE_THRESHOLD_USECS = 5 * USECS_PER_SECOND;
-
-bool AvatarHashMap::shouldKillAvatar(const AvatarSharedPointer& sharedAvatar) {
-    return (sharedAvatar->getOwningAvatarMixer() == NULL
-            || sharedAvatar->getUsecsSinceLastUpdate() > AVATAR_SILENCE_THRESHOLD_USECS);
-}
-
 void AvatarHashMap::processAvatarMixerDatagram(const QByteArray& datagram, const QWeakPointer<Node>& mixerWeakPointer) {
     switch (packetTypeForPacket(datagram)) {
         case PacketTypeBulkAvatarData:
@@ -52,10 +39,6 @@ void AvatarHashMap::processAvatarMixerDatagram(const QByteArray& datagram, const
     }
 }
 
-bool AvatarHashMap::containsAvatarWithDisplayName(const QString& displayName) {
-    return !avatarWithDisplayName(displayName).isNull();
-}
-
 bool AvatarHashMap::isAvatarInRange(const glm::vec3& position, const float range) {
     foreach(const AvatarSharedPointer& sharedAvatar, _avatarHash) {
         glm::vec3 avatarPosition = sharedAvatar->getPosition();
@@ -67,45 +50,19 @@ bool AvatarHashMap::isAvatarInRange(const glm::vec3& position, const float range
     return false;
 }
 
-AvatarWeakPointer AvatarHashMap::avatarWithDisplayName(const QString& displayName) {
-    foreach(const AvatarSharedPointer& sharedAvatar, _avatarHash) {
-        if (sharedAvatar->getDisplayName() == displayName) {
-            // this is a match
-            // check if this avatar should still be around
-            if (!shouldKillAvatar(sharedAvatar)) {
-                // we have a match, return the AvatarData
-                return sharedAvatar;
-            } else {
-                // we should remove this avatar, but we might not be on a thread that is allowed
-                // so we just return NULL to the caller
-                return AvatarWeakPointer();
-            }
-        }
-    }
-        
-    return AvatarWeakPointer();
-}
-
 AvatarSharedPointer AvatarHashMap::newSharedAvatar() {
     return AvatarSharedPointer(new AvatarData());
 }
 
-AvatarSharedPointer AvatarHashMap::matchingOrNewAvatar(const QUuid& sessionUUID, const QWeakPointer<Node>& mixerWeakPointer) {
+AvatarSharedPointer AvatarHashMap::addAvatar(const QUuid& sessionUUID, const QWeakPointer<Node>& mixerWeakPointer) {
-    AvatarSharedPointer matchingAvatar = _avatarHash.value(sessionUUID);
+    qCDebug(avatars) << "Adding avatar with sessionUUID " << sessionUUID << "to AvatarHashMap.";
-    
+
-    if (!matchingAvatar) {
+    AvatarSharedPointer avatar = newSharedAvatar();
-        // insert the new avatar into our hash
+    avatar->setSessionUUID(sessionUUID);
-        matchingAvatar = newSharedAvatar();
+    avatar->setOwningAvatarMixer(mixerWeakPointer);
-    
+    _avatarHash.insert(sessionUUID, avatar);
-        qCDebug(avatars) << "Adding avatar with sessionUUID " << sessionUUID << "to AvatarHashMap.";
+
-        
+    return avatar;
-        matchingAvatar->setSessionUUID(sessionUUID);
-        matchingAvatar->setOwningAvatarMixer(mixerWeakPointer);
-        
-        _avatarHash.insert(sessionUUID, matchingAvatar);
-    }
-    
-    return matchingAvatar;
 }
 
 void AvatarHashMap::processAvatarDataPacket(const QByteArray &datagram, const QWeakPointer<Node> &mixerWeakPointer) {
@@ -118,10 +75,13 @@ void AvatarHashMap::processAvatarDataPacket(const QByteArray &datagram, const QW
         bytesRead += NUM_BYTES_RFC4122_UUID;
         
         if (sessionUUID != _lastOwnerSessionUUID) {
-            AvatarSharedPointer matchingAvatarData = matchingOrNewAvatar(sessionUUID, mixerWeakPointer);
+            AvatarSharedPointer avatar = _avatarHash.value(sessionUUID);
+            if (!avatar) {
+                avatar = addAvatar(sessionUUID, mixerWeakPointer);
+            }
             
             // have the matching (or new) avatar parse the data from the packet
-            bytesRead += matchingAvatarData->parseDataAtOffset(datagram, bytesRead);
+            bytesRead += avatar->parseDataAtOffset(datagram, bytesRead);
         } else {
             // create a dummy AvatarData class to throw this data on the ground
             AvatarData dummyData;
@@ -145,24 +105,24 @@ void AvatarHashMap::processAvatarIdentityPacket(const QByteArray &packet, const
         identityStream >> sessionUUID >> faceMeshURL >> skeletonURL >> attachmentData >> displayName;
         
         // mesh URL for a UUID, find avatar in our list
-        AvatarSharedPointer matchingAvatar = matchingOrNewAvatar(sessionUUID, mixerWeakPointer);
+        AvatarSharedPointer avatar = _avatarHash.value(sessionUUID);
-        if (matchingAvatar) {
+        if (!avatar) {
-            
+            avatar = addAvatar(sessionUUID, mixerWeakPointer);
-            if (matchingAvatar->getFaceModelURL() != faceMeshURL) {
+        }
-                matchingAvatar->setFaceModelURL(faceMeshURL);
+        if (avatar->getFaceModelURL() != faceMeshURL) {
-            }
+            avatar->setFaceModelURL(faceMeshURL);
-            
+        }
-            if (matchingAvatar->getSkeletonModelURL() != skeletonURL) {
+        
-                matchingAvatar->setSkeletonModelURL(skeletonURL);
+        if (avatar->getSkeletonModelURL() != skeletonURL) {
-            }
+            avatar->setSkeletonModelURL(skeletonURL);
-            
+        }
-            if (matchingAvatar->getAttachmentData() != attachmentData) {
+        
-                matchingAvatar->setAttachmentData(attachmentData);
+        if (avatar->getAttachmentData() != attachmentData) {
-            }
+            avatar->setAttachmentData(attachmentData);
-            
+        }
-            if (matchingAvatar->getDisplayName() != displayName) {
+        
-                matchingAvatar->setDisplayName(displayName);
+        if (avatar->getDisplayName() != displayName) {
-            }
+            avatar->setDisplayName(displayName);
         }
     }
 }
@@ -171,24 +131,25 @@ void AvatarHashMap::processAvatarBillboardPacket(const QByteArray& packet, const
     int headerSize = numBytesForPacketHeader(packet);
     QUuid sessionUUID = QUuid::fromRfc4122(QByteArray::fromRawData(packet.constData() + headerSize, NUM_BYTES_RFC4122_UUID));
     
-    AvatarSharedPointer matchingAvatar = matchingOrNewAvatar(sessionUUID, mixerWeakPointer);
+    AvatarSharedPointer avatar = _avatarHash.value(sessionUUID);
-    if (matchingAvatar) {
+    if (!avatar) {
-        QByteArray billboard = packet.mid(headerSize + NUM_BYTES_RFC4122_UUID);
+        avatar = addAvatar(sessionUUID, mixerWeakPointer);
-        if (matchingAvatar->getBillboard() != billboard) {
+    }
-            matchingAvatar->setBillboard(billboard);
+
-        }
+    QByteArray billboard = packet.mid(headerSize + NUM_BYTES_RFC4122_UUID);
+    if (avatar->getBillboard() != billboard) {
+        avatar->setBillboard(billboard);
     }
 }
 
 void AvatarHashMap::processKillAvatar(const QByteArray& datagram) {
     // read the node id
     QUuid sessionUUID = QUuid::fromRfc4122(datagram.mid(numBytesForPacketHeader(datagram), NUM_BYTES_RFC4122_UUID));
-    
+    removeAvatar(sessionUUID);
-    // remove the avatar with that UUID from our hash, if it exists
+}
-    AvatarHash::iterator matchedAvatar = _avatarHash.find(sessionUUID);
+
-    if (matchedAvatar != _avatarHash.end()) {
+void AvatarHashMap::removeAvatar(const QUuid& sessionUUID) {
-        erase(matchedAvatar);
+    _avatarHash.remove(sessionUUID);
-    }
 }
 
 void AvatarHashMap::sessionUUIDChanged(const QUuid& sessionUUID, const QUuid& oldUUID) {

libraries/avatars/src/AvatarHashMap.h

@@ -36,28 +36,26 @@ public:
     
 public slots:
     void processAvatarMixerDatagram(const QByteArray& datagram, const QWeakPointer<Node>& mixerWeakPointer);
-    bool containsAvatarWithDisplayName(const QString& displayName);
     bool isAvatarInRange(const glm::vec3 & position, const float range);
-    AvatarWeakPointer avatarWithDisplayName(const QString& displayname);
     
 private slots:
     void sessionUUIDChanged(const QUuid& sessionUUID, const QUuid& oldUUID);
     
 protected:
     AvatarHashMap();
-    virtual AvatarHash::iterator erase(const AvatarHash::iterator& iterator);
-    
-    bool shouldKillAvatar(const AvatarSharedPointer& sharedAvatar);
     
     virtual AvatarSharedPointer newSharedAvatar();
-    AvatarSharedPointer matchingOrNewAvatar(const QUuid& nodeUUID, const QWeakPointer<Node>& mixerWeakPointer);
+    virtual AvatarSharedPointer addAvatar(const QUuid& sessionUUID, const QWeakPointer<Node>& mixerWeakPointer);
+    virtual void removeAvatar(const QUuid& sessionUUID);
     
+    AvatarHash _avatarHash;
+
+private:
     void processAvatarDataPacket(const QByteArray& packet, const QWeakPointer<Node>& mixerWeakPointer);
     void processAvatarIdentityPacket(const QByteArray& packet, const QWeakPointer<Node>& mixerWeakPointer);
     void processAvatarBillboardPacket(const QByteArray& packet, const QWeakPointer<Node>& mixerWeakPointer);
     void processKillAvatar(const QByteArray& datagram);
 
-    AvatarHash _avatarHash;
     QUuid _lastOwnerSessionUUID;
 };