improve ModelItemID to ModelTreeElment map to not require rewriting when locel model ids are assigned values

assignment-client/src/models/ModelServer.cpp

@@ -66,18 +66,6 @@ void ModelServer::modelCreated(const ModelItem& newModel, const SharedNodePointe
     copyAt += sizeof(modelID);
     packetLength += sizeof(modelID);
     
-    // This would be a good place to fix our modelID to element map
-    ModelTree* tree = static_cast<ModelTree*>(_tree);
-    
-    // find and clear the map element with the creatorToken
-    ModelItemID creatorTokenModelItemID(modelID, creatorTokenID, true);
-    ModelTreeElement* element = tree->getContainingElement(creatorTokenModelItemID);
-    tree->setContainingElement(creatorTokenModelItemID, NULL);
-    
-    // find and clear the map element with the creatorToken
-    ModelItemID justModelID(modelID);
-    tree->setContainingElement(justModelID, element);
-
     NodeList::getInstance()->writeDatagram((char*) outputBuffer, packetLength, senderNode);
 }
 

libraries/models/src/ModelItem.cpp

@@ -815,6 +815,7 @@ ModelItem ModelItem::fromEditPacket(const unsigned char* data, int length, int&
 
         newModelItem.setCreatorTokenID(creatorTokenID);
         newModelItem._newlyCreated = true;
+        valid = true;
     } else {
         // look up the existing modelItem
         const ModelItem* existingModelItem = tree->findModelByID(editID, true);
@@ -822,20 +823,21 @@ ModelItem ModelItem::fromEditPacket(const unsigned char* data, int length, int&
         // copy existing properties before over-writing with new properties
         if (existingModelItem) {
             newModelItem = *existingModelItem;
+            valid = true;
         } else {
             // the user attempted to edit a modelItem that doesn't exist
             qDebug() << "user attempted to edit a modelItem that doesn't exist... editID=" << editID;
+            tree->debugDumpMap();
             valid = false;
-            return newModelItem;
+            
+            // NOTE: Even though we know item is not valid, we still need to parse the rest
+            // of the edit packet so that we don't end up out of sync on our bitstream
+            // fall through....
         }
         newModelItem._id = editID;
         newModelItem._newlyCreated = false;
     }
     
-    // if we got this far, then our result will be valid
-    valid = true;
-    
-
     // lastEdited
     memcpy(&newModelItem._lastEdited, dataAt, sizeof(newModelItem._lastEdited));
     dataAt += sizeof(newModelItem._lastEdited);

libraries/models/src/ModelItem.h

@@ -202,6 +202,16 @@ inline bool operator==(const ModelItemID& a, const ModelItemID& b) {
     return a.id == b.id;
 }
 
+inline uint qHash(const ModelItemID& a, uint seed) {
+    qint64 temp;
+    if (a.id == UNKNOWN_MODEL_ID) {
+        temp = -a.creatorTokenID;
+    } else {
+        temp = a.id;
+    }
+    return qHash(temp, seed);
+}
+
 inline QDebug operator<<(QDebug debug, const ModelItemID& id) {
     debug << "[ id:" << id.id << ", creatorTokenID:" << id.creatorTokenID << "]";
     return debug;

libraries/models/src/ModelTree.cpp

@@ -347,8 +347,11 @@ bool ModelTree::findAndUpdateModelItemIDOperation(OctreeElement* element, void*
 }
 
 void ModelTree::handleAddModelResponse(const QByteArray& packet) {
+    const bool wantDebug = false;
 
-    qDebug() << "ModelTree::handleAddModelResponse()..."; 
+    if (wantDebug) {
+        qDebug() << "ModelTree::handleAddModelResponse()..."; 
+    }
 
     int numBytesPacketHeader = numBytesForPacketHeader(packet);
     
@@ -362,8 +365,10 @@ void ModelTree::handleAddModelResponse(const QByteArray& packet) {
     memcpy(&modelID, dataAt, sizeof(modelID));
     dataAt += sizeof(modelID);
 
-    qDebug() << "    creatorTokenID=" << creatorTokenID;
-    qDebug() << "    modelID=" << modelID;
+    if (wantDebug) {
+        qDebug() << "    creatorTokenID=" << creatorTokenID;
+        qDebug() << "    modelID=" << modelID;
+    }
 
     // update models in our tree
     bool assumeModelFound = !getIsViewing(); // if we're not a viewing tree, then we don't have to find the actual model
@@ -375,7 +380,6 @@ void ModelTree::handleAddModelResponse(const QByteArray& packet) {
         getIsViewing() 
     };
     
-    const bool wantDebug = true;
     if (wantDebug) {
         qDebug() << "looking for creatorTokenID=" << creatorTokenID << " modelID=" << modelID 
                 << " getIsViewing()=" << getIsViewing();
@@ -824,3 +828,10 @@ void ModelTree::setContainingElement(const ModelItemID& modelItemID, ModelTreeEl
     //qDebug() << "AFTER _modelToElementMap=" << _modelToElementMap;
 }
 
+void ModelTree::debugDumpMap() {
+    QHashIterator<ModelItemID, ModelTreeElement*> i(_modelToElementMap);
+    while (i.hasNext()) {
+        i.next();
+        qDebug() << i.key() << ": " << i.value();
+    }
+}

libraries/models/src/ModelTree.h

@@ -91,6 +91,7 @@ public:
     
     ModelTreeElement* getContainingElement(const ModelItemID& modelItemID) const;
     void setContainingElement(const ModelItemID& modelItemID, ModelTreeElement* element);
+    void debugDumpMap();
 
 private:
 
@@ -114,7 +115,7 @@ private:
     QMultiMap<quint64, uint32_t> _recentlyDeletedModelItemIDs;
     ModelItemFBXService* _fbxService;
 
-    QMap<ModelItemID, ModelTreeElement*> _modelToElementMap;
+    QHash<ModelItemID, ModelTreeElement*> _modelToElementMap;
 };
 
 #endif // hifi_ModelTree_h

libraries/models/src/ModelTreeElement.cpp

@@ -426,8 +426,6 @@ void ModelTreeElement::updateModelItemID(FindAndUpdateModelItemIDArgs* args) {
 
                 thisModel.setID(args->modelID);
                 args->creatorTokenFound = true;
-                
-                // TODO: We probably need to fix up the tree's map of ids to models!
             }
         }