From 734b48eee0f02ad8a0b30f4ad6fd5908d2feaa44 Mon Sep 17 00:00:00 2001
From: Simon Walton <simon@highfidelity.io>
Date: Fri, 30 Mar 2018 17:27:52 -0700
Subject: [PATCH] Local IDs - Reviewer's suggested improvements

---
 assignment-client/src/audio/AudioMixer.cpp   |  2 +-
 libraries/networking/src/LimitedNodeList.cpp | 10 ++++----
 libraries/networking/src/NLPacket.h          | 24 +-------------------
 libraries/networking/src/PacketReceiver.cpp  |  4 +++-
 4 files changed, 11 insertions(+), 29 deletions(-)

diff --git a/assignment-client/src/audio/AudioMixer.cpp b/assignment-client/src/audio/AudioMixer.cpp
index a50304e1d5..a18df68f90 100644
--- a/assignment-client/src/audio/AudioMixer.cpp
+++ b/assignment-client/src/audio/AudioMixer.cpp
@@ -137,7 +137,7 @@ void AudioMixer::queueReplicatedAudioPacket(QSharedPointer<ReceivedMessage> mess
 
     auto replicatedMessage = QSharedPointer<ReceivedMessage>::create(audioData, rewrittenType,
                                                                      versionForPacketType(rewrittenType),
-                                                                     message->getSenderSockAddr(), nodeList->getSessionLocalID());
+                                                                     message->getSenderSockAddr(), message->getSourceID());
 
     getOrCreateClientData(replicatedNode.data())->queuePacket(replicatedMessage, replicatedNode);
 }
diff --git a/libraries/networking/src/LimitedNodeList.cpp b/libraries/networking/src/LimitedNodeList.cpp
index 177bc729cd..c30952c344 100644
--- a/libraries/networking/src/LimitedNodeList.cpp
+++ b/libraries/networking/src/LimitedNodeList.cpp
@@ -132,6 +132,7 @@ void LimitedNodeList::setSessionUUID(const QUuid& sessionUUID) {
 }
 
 Node::LocalID LimitedNodeList::getSessionLocalID() const {
+    QReadLocker readLock { &_sessionUUIDLock };
     return _sessionLocalID;
 }
 
@@ -578,7 +579,7 @@ void LimitedNodeList::eraseAllNodes() {
         // and then remove them from the hash
         QWriteLocker writeLocker(&_nodeMutex);
 
-        _localIDMap.erase(_localIDMap.begin(), _localIDMap.end());
+        _localIDMap.clear();
 
         if (_nodeHash.size() > 0) {
             qCDebug(networking) << "LimitedNodeList::eraseAllNodes() removing all nodes from NodeList.";
@@ -851,9 +852,10 @@ void LimitedNodeList::removeSilentNodes() {
     });
 
     foreach(const SharedNodePointer& killedNode, killedNodes) {
-        _nodeMutex.lockForWrite();
-        _localIDMap.erase(killedNode->getLocalID());
-        _nodeMutex.unlock();
+        {
+            QWriteLocker writeLock { &_nodeMutex };
+            _localIDMap.erase(killedNode->getLocalID());
+        }
         handleNodeKill(killedNode);
     }
 }
diff --git a/libraries/networking/src/NLPacket.h b/libraries/networking/src/NLPacket.h
index a994b7bde0..b9c598500b 100644
--- a/libraries/networking/src/NLPacket.h
+++ b/libraries/networking/src/NLPacket.h
@@ -22,7 +22,7 @@ class NLPacket : public udt::Packet {
     Q_OBJECT
 public:
     //
-    //    Current NLPacket format:
+    //    NLPacket format:
     //
     //    |      BYTE     |      BYTE     |      BYTE     |      BYTE     |
     //     0                   1                   2                   3
@@ -35,28 +35,6 @@ public:
     //    |                 (ONLY FOR VERIFIED PACKETS)                   |
     //    |                                                               |
     //    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
-
-    //    Old NLPacket format:
-    //
-    //    |      BYTE     |      BYTE     |      BYTE     |      BYTE     |
-    //
-    //     0                   1                   2                   3
-    //     0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
-    //    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
-    //    |  Packet Type  |    Version    |                               |
-    //    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+                               |
-    //    |                                                               |
-    //    |                       Node UUID - 16 bytes                    |
-    //    |                   (ONLY FOR SOURCED PACKETS)                  |
-    //    |                               +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
-    //    |                               |                               |
-    //    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+                               |
-    //    |                                                               |
-    //    |                   MD5 Verification - 16 bytes                 |
-    //    |                   (ONLY FOR VERIFIED PACKETS)                 |
-    //    |                               +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
-    //    |                               |                               |
-    //    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+                               |
     //
 
     using LocalID = NetworkLocalID;
diff --git a/libraries/networking/src/PacketReceiver.cpp b/libraries/networking/src/PacketReceiver.cpp
index 3d35424316..39a3800c4b 100644
--- a/libraries/networking/src/PacketReceiver.cpp
+++ b/libraries/networking/src/PacketReceiver.cpp
@@ -261,7 +261,9 @@ void PacketReceiver::handleVerifiedMessage(QSharedPointer<ReceivedMessage> recei
     
     SharedNodePointer matchingNode;
     
-    matchingNode = nodeList->nodeWithLocalID(receivedMessage->getSourceID());
+    if (receivedMessage->getSourceID() != 0) {
+        matchingNode = nodeList->nodeWithLocalID(receivedMessage->getSourceID());
+    }
     QMutexLocker packetListenerLocker(&_packetListenerLock);
     
     auto it = _messageListenerMap.find(receivedMessage->getType());