From f50395f8db74dbc6bdeda28823562b0eaecd8d67 Mon Sep 17 00:00:00 2001 From: ZappoMan Date: Thu, 19 Jun 2014 22:11:10 -0700 Subject: [PATCH] improve ModelItemID to ModelTreeElment map to not require rewriting when locel model ids are assigned values --- assignment-client/src/models/ModelServer.cpp | 12 ------------ libraries/models/src/ModelItem.cpp | 12 +++++++----- libraries/models/src/ModelItem.h | 10 ++++++++++ libraries/models/src/ModelTree.cpp | 19 +++++++++++++++---- libraries/models/src/ModelTree.h | 3 ++- libraries/models/src/ModelTreeElement.cpp | 2 -- 6 files changed, 34 insertions(+), 24 deletions(-) diff --git a/assignment-client/src/models/ModelServer.cpp b/assignment-client/src/models/ModelServer.cpp index 642c2f71db..80dea78623 100644 --- a/assignment-client/src/models/ModelServer.cpp +++ b/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(_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); } diff --git a/libraries/models/src/ModelItem.cpp b/libraries/models/src/ModelItem.cpp index 6c31920b06..be39f01f83 100644 --- a/libraries/models/src/ModelItem.cpp +++ b/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); diff --git a/libraries/models/src/ModelItem.h b/libraries/models/src/ModelItem.h index 5ca3fd390e..b2be5af10f 100644 --- a/libraries/models/src/ModelItem.h +++ b/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; diff --git a/libraries/models/src/ModelTree.cpp b/libraries/models/src/ModelTree.cpp index dce0dda27c..cbc89d79fc 100644 --- a/libraries/models/src/ModelTree.cpp +++ b/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 i(_modelToElementMap); + while (i.hasNext()) { + i.next(); + qDebug() << i.key() << ": " << i.value(); + } +} diff --git a/libraries/models/src/ModelTree.h b/libraries/models/src/ModelTree.h index 16390a26e6..9ad3b6594d 100644 --- a/libraries/models/src/ModelTree.h +++ b/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 _recentlyDeletedModelItemIDs; ModelItemFBXService* _fbxService; - QMap _modelToElementMap; + QHash _modelToElementMap; }; #endif // hifi_ModelTree_h diff --git a/libraries/models/src/ModelTreeElement.cpp b/libraries/models/src/ModelTreeElement.cpp index ac12b6a81b..5f7c50f704 100644 --- a/libraries/models/src/ModelTreeElement.cpp +++ b/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! } }