From d3fea94540a618d3de276a54dbcc3402d248e044 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Mon, 3 Dec 2018 18:13:31 -0800 Subject: [PATCH 01/43] send AvatarEntities via raw data packet --- .../src/avatars-renderer/Avatar.cpp | 25 ++++++++------- .../entities/src/EntityEditPacketSender.cpp | 24 ++++++-------- libraries/entities/src/EntityItem.cpp | 5 +++ libraries/octree/src/OctreePacketData.cpp | 31 ++++++++++++------- libraries/octree/src/OctreePacketData.h | 4 +++ 5 files changed, 53 insertions(+), 36 deletions(-) diff --git a/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp b/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp index dbf5899386..2c52bc69ea 100644 --- a/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp +++ b/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp @@ -378,19 +378,22 @@ void Avatar::updateAvatarEntities() { } ++dataItr; - // see EntityEditPacketSender::queueEditEntityMessage for the other end of this. unpack properties - // and either add or update the entity. - QJsonDocument jsonProperties = QJsonDocument::fromBinaryData(data); - if (!jsonProperties.isObject()) { - qCDebug(avatars_renderer) << "got bad avatarEntity json" << QString(data.toHex()); - continue; + EntityItemProperties properties; + { + // create a temporary EntityItem to unpack the data + int32_t bytesLeftToRead = data.size(); + unsigned char* dataAt = (unsigned char*)(data.data()); + ReadBitstreamToTreeParams args; + EntityItemPointer tempEntity = EntityTypes::constructEntityItem(dataAt, bytesLeftToRead, args); + if (!tempEntity) { + continue; + } + tempEntity->readEntityDataFromBuffer(dataAt, bytesLeftToRead, args); + + // extract the properties from tempEntity + properties = tempEntity->getProperties(); } - QVariant variantProperties = jsonProperties.toVariant(); - QVariantMap asMap = variantProperties.toMap(); - QScriptValue scriptProperties = variantMapToScriptValue(asMap, scriptEngine); - EntityItemProperties properties; - EntityItemPropertiesFromScriptValueIgnoreReadOnly(scriptProperties, properties); properties.setEntityHostType(entity::HostType::AVATAR); properties.setOwningAvatarID(getID()); diff --git a/libraries/entities/src/EntityEditPacketSender.cpp b/libraries/entities/src/EntityEditPacketSender.cpp index c414a7a4ac..8d0348bae6 100644 --- a/libraries/entities/src/EntityEditPacketSender.cpp +++ b/libraries/entities/src/EntityEditPacketSender.cpp @@ -54,27 +54,23 @@ void EntityEditPacketSender::queueEditAvatarEntityMessage(PacketType type, return; } - // the properties that get serialized into the avatar identity packet should be the entire set + // serialize ALL properties in an "AvatarEntity" packet // rather than just the ones being edited. EntityItemProperties entityProperties = entity->getProperties(); entityProperties.merge(properties); - std::lock_guard lock(_mutex); - QScriptValue scriptProperties = EntityItemNonDefaultPropertiesToScriptValue(&_scriptEngine, entityProperties); - QVariant variantProperties = scriptProperties.toVariant(); - QJsonDocument jsonProperties = QJsonDocument::fromVariant(variantProperties); + OctreePacketData packetData(false, AvatarTraits::MAXIMUM_TRAIT_SIZE); + EncodeBitstreamParams params; + EntityTreeElementExtraEncodeDataPointer extra { nullptr }; + OctreeElement::AppendState appendState = entity->appendEntityData(&packetData, params, extra); - // the ID of the parent/avatar changes from session to session. use a special UUID to indicate the avatar - QJsonObject jsonObject = jsonProperties.object(); - if (jsonObject.contains("parentID")) { - if (QUuid(jsonObject["parentID"].toString()) == _myAvatar->getID()) { - jsonObject["parentID"] = AVATAR_SELF_ID.toString(); - } + if (appendState != OctreeElement::COMPLETED) { + // this entity is too big + return; } - jsonProperties = QJsonDocument(jsonObject); - QByteArray binaryProperties = jsonProperties.toBinaryData(); - _myAvatar->updateAvatarEntity(entityItemID, binaryProperties); + packetData.shrinkByteArrays(); + _myAvatar->updateAvatarEntity(entityItemID, packetData.getUncompressedByteArray()); entity->setLastBroadcast(usecTimestampNow()); } diff --git a/libraries/entities/src/EntityItem.cpp b/libraries/entities/src/EntityItem.cpp index 498f0ff066..5b7a152318 100644 --- a/libraries/entities/src/EntityItem.cpp +++ b/libraries/entities/src/EntityItem.cpp @@ -179,6 +179,11 @@ OctreeElement::AppendState EntityItem::appendEntityData(OctreePacketData* packet EntityPropertyFlags propertyFlags(PROP_LAST_ITEM); EntityPropertyFlags requestedProperties = getEntityProperties(params); + // the values of these two properties are known to the receiver by context + // therefore they don't need to be packed + requestedProperties -= PROP_ENTITY_HOST_TYPE; + requestedProperties -= PROP_OWNING_AVATAR_ID; + // If we are being called for a subsequent pass at appendEntityData() that failed to completely encode this item, // then our entityTreeElementExtraEncodeData should include data about which properties we need to append. if (entityTreeElementExtraEncodeData && entityTreeElementExtraEncodeData->entities.contains(getEntityItemID())) { diff --git a/libraries/octree/src/OctreePacketData.cpp b/libraries/octree/src/OctreePacketData.cpp index 6c0bba5ec6..60b0e6fbad 100644 --- a/libraries/octree/src/OctreePacketData.cpp +++ b/libraries/octree/src/OctreePacketData.cpp @@ -38,7 +38,11 @@ void OctreePacketData::changeSettings(bool enableCompression, unsigned int targe _enableCompression = enableCompression; _targetSize = targetSize; _uncompressedByteArray.resize(_targetSize); - _compressedByteArray.resize(_targetSize); + if (_enableCompression) { + _compressedByteArray.resize(_targetSize); + } else { + _compressedByteArray.resize(0); + } _uncompressed = (unsigned char*)_uncompressedByteArray.data(); _compressed = (unsigned char*)_compressedByteArray.data(); @@ -586,13 +590,10 @@ bool OctreePacketData::appendRawData(QByteArray data) { AtomicUIntStat OctreePacketData::_compressContentTime { 0 }; AtomicUIntStat OctreePacketData::_compressContentCalls { 0 }; -bool OctreePacketData::compressContent() { +bool OctreePacketData::compressContent() { PerformanceWarning warn(false, "OctreePacketData::compressContent()", false, &_compressContentTime, &_compressContentCalls); - - // without compression, we always pass... - if (!_enableCompression) { - return true; - } + assert(_dirty); + assert(_enableCompression); _bytesInUseLastCheck = _bytesInUse; @@ -605,13 +606,13 @@ bool OctreePacketData::compressContent() { QByteArray compressedData = qCompress(uncompressedData, uncompressedSize, MAX_COMPRESSION); - if (compressedData.size() < (int)MAX_OCTREE_PACKET_DATA_SIZE) { + if (compressedData.size() < _compressedByteArray.size()) { _compressedBytes = compressedData.size(); memcpy(_compressed, compressedData.constData(), _compressedBytes); _dirty = false; success = true; } else { - qCWarning(octree) << "OctreePacketData::compressContent -- compressedData.size >= MAX_OCTREE_PACKET_DATA_SIZE"; + qCWarning(octree) << "OctreePacketData::compressContent -- compressedData.size >= " << _compressedByteArray.size(); assert(false); } return success; @@ -644,8 +645,7 @@ void OctreePacketData::loadFinalizedContent(const unsigned char* data, int lengt memcpy(_uncompressed, uncompressedData.constData(), _bytesInUse); } else { memcpy(_uncompressed, data, length); - memcpy(_compressed, data, length); - _bytesInUse = _compressedBytes = length; + _bytesInUse = length; } } else { if (_debug) { @@ -654,6 +654,15 @@ void OctreePacketData::loadFinalizedContent(const unsigned char* data, int lengt } } +void OctreePacketData::shrinkByteArrays() { + _uncompressedByteArray.resize(_bytesInUse); + _compressedByteArray.resize(_compressedBytes); + // if you call this method then you are expected to be done packing to raw pointers + // and you just want the ByteArrays + // therefore we reset + reset(); +} + void OctreePacketData::debugContent() { qCDebug(octree, "OctreePacketData::debugContent()... COMPRESSED DATA.... size=%d",_compressedBytes); int perline=0; diff --git a/libraries/octree/src/OctreePacketData.h b/libraries/octree/src/OctreePacketData.h index bd1abf8744..cca78e19b3 100644 --- a/libraries/octree/src/OctreePacketData.h +++ b/libraries/octree/src/OctreePacketData.h @@ -220,6 +220,8 @@ public: /// get pointer to the uncompressed stream buffer at the byteOffset const unsigned char* getUncompressedData(int byteOffset = 0) { return &_uncompressed[byteOffset]; } + const QByteArray& getUncompressedByteArray() { return _uncompressedByteArray; } + /// the size of the packet in uncompressed form int getUncompressedSize() { return _bytesInUse; } @@ -243,6 +245,8 @@ public: int getBytesAvailable() { return _bytesAvailable; } + void shrinkByteArrays(); + /// displays contents for debugging void debugContent(); void debugBytes(); From d1927340f5a64d67c4e023418f73e9473c2ea448 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Tue, 4 Dec 2018 18:07:53 -0800 Subject: [PATCH 02/43] remove redundant addToNeedsParentFixupList() --- libraries/entities/src/EntityTree.cpp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index 0957b226e9..0590eac09c 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -531,10 +531,6 @@ EntityItemPointer EntityTree::addEntity(const EntityItemID& entityID, const Enti // Recurse the tree and store the entity in the correct tree element AddEntityOperator theOperator(getThisPointer(), result); recurseTreeWithOperator(&theOperator); - if (!result->getParentID().isNull()) { - addToNeedsParentFixupList(result); - } - postAddEntity(result); } return result; From 01fc4426955e6934476a5a0c7ad4b15dc69df25c Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Tue, 4 Dec 2018 18:08:54 -0800 Subject: [PATCH 03/43] save AvatarEntities settings as strings --- interface/src/avatar/MyAvatar.cpp | 72 ++++++++++++------- interface/src/avatar/MyAvatar.h | 4 +- .../src/avatars-renderer/Avatar.cpp | 10 +-- .../entities/src/EntityItemProperties.cpp | 10 +++ libraries/entities/src/EntityItemProperties.h | 2 + 5 files changed, 63 insertions(+), 35 deletions(-) diff --git a/interface/src/avatar/MyAvatar.cpp b/interface/src/avatar/MyAvatar.cpp index e40fc7f9dd..0e17fa8fbe 100755 --- a/interface/src/avatar/MyAvatar.cpp +++ b/interface/src/avatar/MyAvatar.cpp @@ -1263,11 +1263,11 @@ void MyAvatar::resizeAvatarEntitySettingHandles(unsigned int avatarEntityIndex) // Create Setting::Handles to mimic this. while (_avatarEntityIDSettings.size() <= avatarEntityIndex) { - Setting::Handle idHandle(QStringList() << AVATAR_SETTINGS_GROUP_NAME << "avatarEntityData" - << QString::number(avatarEntityIndex + 1) << "id", QUuid()); + Setting::Handle idHandle(QStringList() << AVATAR_SETTINGS_GROUP_NAME << "avatarEntityData" + << QString::number(avatarEntityIndex + 1) << "id", QUuid().toString()); _avatarEntityIDSettings.push_back(idHandle); - Setting::Handle dataHandle(QStringList() << AVATAR_SETTINGS_GROUP_NAME << "avatarEntityData" - << QString::number(avatarEntityIndex + 1) << "properties", QByteArray()); + Setting::Handle dataHandle(QStringList() << AVATAR_SETTINGS_GROUP_NAME << "avatarEntityData" + << QString::number(avatarEntityIndex + 1) << "properties", QString()); _avatarEntityDataSettings.push_back(dataHandle); } } @@ -1299,23 +1299,46 @@ void MyAvatar::saveData() { auto hmdInterface = DependencyManager::get(); _avatarEntitiesLock.withReadLock([&] { - QList avatarEntityIDs = _avatarEntityData.keys(); - unsigned int avatarEntityCount = avatarEntityIDs.size(); - unsigned int previousAvatarEntityCount = _avatarEntityCountSetting.get(0); - resizeAvatarEntitySettingHandles(std::max(avatarEntityCount, previousAvatarEntityCount)); - _avatarEntityCountSetting.set(avatarEntityCount); + uint32_t numEntities = _avatarEntityData.size(); + uint32_t prevNumEntities = _avatarEntityCountSetting.get(0); + resizeAvatarEntitySettingHandles(std::max(numEntities, prevNumEntities)); - unsigned int avatarEntityIndex = 0; - for (auto entityID : avatarEntityIDs) { - _avatarEntityIDSettings[avatarEntityIndex].set(entityID); - _avatarEntityDataSettings[avatarEntityIndex].set(_avatarEntityData.value(entityID)); - avatarEntityIndex++; + // Note: this roundabout path from AvatarEntityData to JSON string is NOT efficient + QScriptEngine engine; + QScriptValue toStringMethod = engine.evaluate("(function() { return JSON.stringify(this) })"); + AvatarEntityMap::const_iterator itr = _avatarEntityData.begin(); + numEntities = 0; + while (itr != _avatarEntityData.end()) { + EntityItemProperties properties; + QByteArray buffer = itr.value(); + if (properties.constructFromBuffer((uint8_t*)buffer.data(), buffer.size())) { + if (properties.getParentID() == getSessionUUID()) { + properties.setParentID(AVATAR_SELF_ID); + } + QScriptValue scriptValue = EntityItemPropertiesToScriptValue(&engine, properties); + scriptValue.setProperty("toString", toStringMethod); + _avatarEntityDataSettings[numEntities].set(scriptValue.toString()); + _avatarEntityIDSettings[numEntities].set(itr.key().toString()); + ++numEntities; + } else { + // buffer is corrupt --> skip it + } + ++itr; } + _avatarEntityCountSetting.set(numEntities); - // clean up any left-over (due to the list shrinking) slots - for (; avatarEntityIndex < previousAvatarEntityCount; avatarEntityIndex++) { - _avatarEntityIDSettings[avatarEntityIndex].remove(); - _avatarEntityDataSettings[avatarEntityIndex].remove(); + if (numEntities < prevNumEntities) { + uint32_t numEntitiesToRemove = prevNumEntities - numEntities; + for (uint32_t i = 0; i < numEntitiesToRemove; ++i) { + if (_avatarEntityIDSettings.size() > numEntities) { + _avatarEntityIDSettings.back().remove(); + _avatarEntityIDSettings.pop_back(); + } + if (_avatarEntityDataSettings.size() > numEntities) { + _avatarEntityDataSettings.back().remove(); + _avatarEntityDataSettings.pop_back(); + } + } } }); } @@ -1426,13 +1449,14 @@ void MyAvatar::loadData() { useFullAvatarURL(_fullAvatarURLFromPreferences, _fullAvatarModelName); - int avatarEntityCount = _avatarEntityCountSetting.get(0); - for (int i = 0; i < avatarEntityCount; i++) { + int numEntities = _avatarEntityCountSetting.get(0); + + for (int i = 0; i < numEntities; i++) { resizeAvatarEntitySettingHandles(i); - // QUuid entityID = QUuid::createUuid(); // generate a new ID - QUuid entityID = _avatarEntityIDSettings[i].get(QUuid()); - QByteArray properties = _avatarEntityDataSettings[i].get(); - updateAvatarEntity(entityID, properties); + QUuid entityID = QUuid::createUuid(); // generate a new ID + QString propertiesString = _avatarEntityDataSettings[i].get(); + // TODO: convert propertiesString to EntityItemProperties + //updateAvatarEntity(entityID, properties); } // Flying preferences must be loaded before calling setFlyingEnabled() diff --git a/interface/src/avatar/MyAvatar.h b/interface/src/avatar/MyAvatar.h index 17b71153ea..1984ea9766 100644 --- a/interface/src/avatar/MyAvatar.h +++ b/interface/src/avatar/MyAvatar.h @@ -1936,8 +1936,8 @@ private: Setting::Handle _flyingHMDSetting; Setting::Handle _avatarEntityCountSetting; Setting::Handle _allowTeleportingSetting { "allowTeleporting", true }; - std::vector> _avatarEntityIDSettings; - std::vector> _avatarEntityDataSettings; + std::vector> _avatarEntityIDSettings; + std::vector> _avatarEntityDataSettings; }; QScriptValue audioListenModeToScriptValue(QScriptEngine* engine, const AudioListenerMode& audioListenerMode); diff --git a/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp b/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp index 2c52bc69ea..23093028b1 100644 --- a/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp +++ b/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp @@ -383,15 +383,7 @@ void Avatar::updateAvatarEntities() { // create a temporary EntityItem to unpack the data int32_t bytesLeftToRead = data.size(); unsigned char* dataAt = (unsigned char*)(data.data()); - ReadBitstreamToTreeParams args; - EntityItemPointer tempEntity = EntityTypes::constructEntityItem(dataAt, bytesLeftToRead, args); - if (!tempEntity) { - continue; - } - tempEntity->readEntityDataFromBuffer(dataAt, bytesLeftToRead, args); - - // extract the properties from tempEntity - properties = tempEntity->getProperties(); + properties.constructFromBuffer(dataAt, bytesLeftToRead); } properties.setEntityHostType(entity::HostType::AVATAR); diff --git a/libraries/entities/src/EntityItemProperties.cpp b/libraries/entities/src/EntityItemProperties.cpp index 26bb450b2e..0badd00bc3 100644 --- a/libraries/entities/src/EntityItemProperties.cpp +++ b/libraries/entities/src/EntityItemProperties.cpp @@ -90,6 +90,16 @@ void EntityItemProperties::setLastEdited(quint64 usecTime) { _lastEdited = usecTime > _created ? usecTime : _created; } +bool EntityItemProperties::constructFromBuffer(const unsigned char* data, int dataLength) { + ReadBitstreamToTreeParams args; + EntityItemPointer tempEntity = EntityTypes::constructEntityItem(data, dataLength, args); + if (!tempEntity) { + return false; + } + tempEntity->readEntityDataFromBuffer(data, dataLength, args); + (*this) = tempEntity->getProperties(); + return true; +} QHash stringToShapeTypeLookup; diff --git a/libraries/entities/src/EntityItemProperties.h b/libraries/entities/src/EntityItemProperties.h index 17ec83be78..4a9729f5fe 100644 --- a/libraries/entities/src/EntityItemProperties.h +++ b/libraries/entities/src/EntityItemProperties.h @@ -135,6 +135,8 @@ public: EntityPropertyFlags getDesiredProperties() { return _desiredProperties; } void setDesiredProperties(EntityPropertyFlags properties) { _desiredProperties = properties; } + bool constructFromBuffer(const unsigned char* data, int dataLength); + // Note: DEFINE_PROPERTY(PROP_FOO, Foo, foo, type, value) creates the following methods and variables: // type getFoo() const; // void setFoo(type); From e4cc3fa976463c0969125198cd8ae018588beee6 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Wed, 5 Dec 2018 14:32:42 -0800 Subject: [PATCH 04/43] minor cleanup --- libraries/entities/src/EntityTree.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index 0590eac09c..3f0b5249ec 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -490,7 +490,6 @@ bool EntityTree::updateEntity(EntityItemPointer entity, const EntityItemProperti } EntityItemPointer EntityTree::addEntity(const EntityItemID& entityID, const EntityItemProperties& properties, bool isClone) { - EntityItemPointer result = NULL; EntityItemProperties props = properties; auto nodeList = DependencyManager::get(); @@ -517,12 +516,12 @@ EntityItemPointer EntityTree::addEntity(const EntityItemID& entityID, const Enti if (containingElement) { qCWarning(entities) << "EntityTree::addEntity() on existing entity item with entityID=" << entityID << "containingElement=" << containingElement.get(); - return result; + return nullptr; } // construct the instance of the entity EntityTypes::EntityType type = props.getType(); - result = EntityTypes::constructEntityItem(type, entityID, props); + EntityItemPointer result = EntityTypes::constructEntityItem(type, entityID, props); if (result) { if (recordCreationTime) { From 263d6cb7adf4b8cd308eb8c80c459644060bb662 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Wed, 5 Dec 2018 14:34:20 -0800 Subject: [PATCH 05/43] only change parentID=AVATAR_SELF_ID for non-clientOnly case --- libraries/entities/src/EntityItem.cpp | 30 +++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/libraries/entities/src/EntityItem.cpp b/libraries/entities/src/EntityItem.cpp index 5b7a152318..f402aae7d0 100644 --- a/libraries/entities/src/EntityItem.cpp +++ b/libraries/entities/src/EntityItem.cpp @@ -329,6 +329,36 @@ OctreeElement::AppendState EntityItem::appendEntityData(OctreePacketData* packet APPEND_ENTITY_PROPERTY(PROP_CERTIFICATE_ID, getCertificateID()); APPEND_ENTITY_PROPERTY(PROP_STATIC_CERTIFICATE_VERSION, getStaticCertificateVersion()); + APPEND_ENTITY_PROPERTY(PROP_NAME, getName()); + APPEND_ENTITY_PROPERTY(PROP_COLLISION_SOUND_URL, getCollisionSoundURL()); + APPEND_ENTITY_PROPERTY(PROP_HREF, getHref()); + APPEND_ENTITY_PROPERTY(PROP_DESCRIPTION, getDescription()); + APPEND_ENTITY_PROPERTY(PROP_ACTION_DATA, getDynamicData()); + + QUuid actualParentID = getParentID(); + if (!getClientOnly() && actualParentID == AVATAR_SELF_ID) { + // convert AVATAR_SELF_ID to actual sessionUUID. + auto nodeList = DependencyManager::get(); + actualParentID = nodeList->getSessionUUID(); + } + APPEND_ENTITY_PROPERTY(PROP_PARENT_ID, actualParentID); + + APPEND_ENTITY_PROPERTY(PROP_PARENT_JOINT_INDEX, getParentJointIndex()); + APPEND_ENTITY_PROPERTY(PROP_QUERY_AA_CUBE, getQueryAACube()); + APPEND_ENTITY_PROPERTY(PROP_LAST_EDITED_BY, getLastEditedBy()); + + APPEND_ENTITY_PROPERTY(PROP_CLONEABLE, getCloneable()); + APPEND_ENTITY_PROPERTY(PROP_CLONE_LIFETIME, getCloneLifetime()); + APPEND_ENTITY_PROPERTY(PROP_CLONE_LIMIT, getCloneLimit()); + APPEND_ENTITY_PROPERTY(PROP_CLONE_DYNAMIC, getCloneDynamic()); + APPEND_ENTITY_PROPERTY(PROP_CLONE_AVATAR_ENTITY, getCloneAvatarEntity()); + APPEND_ENTITY_PROPERTY(PROP_CLONE_ORIGIN_ID, getCloneOriginID()); + + withReadLock([&] { + _grabProperties.appendSubclassData(packetData, params, entityTreeElementExtraEncodeData, requestedProperties, + propertyFlags, propertiesDidntFit, propertyCount, appendState); + }); + appendSubclassData(packetData, params, entityTreeElementExtraEncodeData, requestedProperties, propertyFlags, From 026c6301a61cda663aa799958f116a0ca2f73ccf Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Wed, 5 Dec 2018 14:34:43 -0800 Subject: [PATCH 06/43] correctly load AvatarEntities from settings --- interface/src/avatar/MyAvatar.cpp | 61 +++++++++++++++++-- interface/src/avatar/MyAvatar.h | 3 + libraries/avatars/src/AvatarData.cpp | 15 +++-- libraries/avatars/src/AvatarData.h | 3 +- .../entities/src/EntityEditPacketSender.cpp | 13 ++-- .../entities/src/EntityEditPacketSender.h | 2 +- 6 files changed, 76 insertions(+), 21 deletions(-) diff --git a/interface/src/avatar/MyAvatar.cpp b/interface/src/avatar/MyAvatar.cpp index 0e17fa8fbe..197bfcdca5 100755 --- a/interface/src/avatar/MyAvatar.cpp +++ b/interface/src/avatar/MyAvatar.cpp @@ -50,6 +50,7 @@ #include #include #include +#include #include "MyHead.h" #include "MySkeletonModel.h" @@ -1437,6 +1438,38 @@ void MyAvatar::setEnableInverseKinematics(bool isEnabled) { _skeletonModel->getRig().setEnableInverseKinematics(isEnabled); } +void MyAvatar::updateAvatarEntity(const QUuid& entityID, const EntityItemProperties& properties) { + auto entityTreeRenderer = qApp->getEntities(); + EntityTreePointer entityTree = entityTreeRenderer ? entityTreeRenderer->getTree() : nullptr; + EntityItemPointer entity; + if (entityTree) { + entity = entityTree->findEntityByID(entityID); + if (!entity) { + entity = entityTree->addEntity(entityID, properties); + if (!entity) { + // unable to create entity + // TODO? handle this case? + return; + } + } else { + // TODO: propagate changes to entity + } + } + + OctreePacketData packetData(false, AvatarTraits::MAXIMUM_TRAIT_SIZE); + EncodeBitstreamParams params; + EntityTreeElementExtraEncodeDataPointer extra { nullptr }; + OctreeElement::AppendState appendState = entity->appendEntityData(&packetData, params, extra); + + if (appendState != OctreeElement::COMPLETED) { + // this entity's data is too big + return; + } + + packetData.shrinkByteArrays(); + storeAvatarEntityDataPayload(entity->getID(), packetData.getUncompressedByteArray()); +} + void MyAvatar::loadData() { getHead()->setBasePitch(_headPitchSetting.get()); @@ -1450,13 +1483,29 @@ void MyAvatar::loadData() { useFullAvatarURL(_fullAvatarURLFromPreferences, _fullAvatarModelName); int numEntities = _avatarEntityCountSetting.get(0); + auto entityTree = DependencyManager::get()->getTree(); - for (int i = 0; i < numEntities; i++) { - resizeAvatarEntitySettingHandles(i); - QUuid entityID = QUuid::createUuid(); // generate a new ID - QString propertiesString = _avatarEntityDataSettings[i].get(); - // TODO: convert propertiesString to EntityItemProperties - //updateAvatarEntity(entityID, properties); + if (numEntities > 0) { + QScriptEngine scriptEngine; + for (int i = 0; i < numEntities; i++) { + resizeAvatarEntitySettingHandles(i); + QUuid entityID = QUuid::createUuid(); // generate a new ID + + // NOTE: this path from EntityItemProperties JSON string to EntityItemProperties is NOT efficient + QString propertiesString = _avatarEntityDataSettings[i].get(); + QJsonDocument propertiesDoc = QJsonDocument::fromJson(propertiesString.toUtf8()); + QJsonObject propertiesObj = propertiesDoc.object(); + QVariant propertiesVariant(propertiesObj); + QVariantMap propertiesMap = propertiesVariant.toMap(); + QScriptValue propertiesScriptValue = variantMapToScriptValue(propertiesMap, scriptEngine); + EntityItemProperties properties; + EntityItemPropertiesFromScriptValueIgnoreReadOnly(propertiesScriptValue, properties); + + // the ClientOnly property can get stripped out elsewhere so we need to always set it true here + properties.setClientOnly(true); + + updateAvatarEntity(entityID, properties); + } } // Flying preferences must be loaded before calling setFlyingEnabled() diff --git a/interface/src/avatar/MyAvatar.h b/interface/src/avatar/MyAvatar.h index 1984ea9766..b1848a19b9 100644 --- a/interface/src/avatar/MyAvatar.h +++ b/interface/src/avatar/MyAvatar.h @@ -1402,6 +1402,9 @@ public slots: */ bool getEnableMeshVisible() const override; + // TODO: make this invokable, probably also move down to AvatarData + void updateAvatarEntity(const QUuid& entityID, const EntityItemProperties& properties); + /**jsdoc * Set whether or not your avatar mesh is visible. * @function MyAvatar.setEnableMeshVisible diff --git a/libraries/avatars/src/AvatarData.cpp b/libraries/avatars/src/AvatarData.cpp index 2f1c8d3d82..e015410582 100644 --- a/libraries/avatars/src/AvatarData.cpp +++ b/libraries/avatars/src/AvatarData.cpp @@ -2019,7 +2019,7 @@ void AvatarData::processTrait(AvatarTraits::TraitType traitType, QByteArray trai void AvatarData::processTraitInstance(AvatarTraits::TraitType traitType, AvatarTraits::TraitInstanceID instanceID, QByteArray traitBinaryData) { if (traitType == AvatarTraits::AvatarEntity) { - updateAvatarEntity(instanceID, traitBinaryData); + storeAvatarEntityDataPayload(instanceID, traitBinaryData); } else if (traitType == AvatarTraits::Grab) { updateAvatarGrabData(instanceID, traitBinaryData); } @@ -2569,8 +2569,9 @@ void AvatarData::fromJson(const QJsonObject& json, bool useFrameSkeleton) { if (attachmentJson.isObject()) { QVariantMap entityData = attachmentJson.toObject().toVariantMap(); QUuid entityID = entityData.value("id").toUuid(); + // ADEBUG TODO: fix this broken path QByteArray properties = QByteArray::fromBase64(entityData.value("properties").toByteArray()); - updateAvatarEntity(entityID, properties); + storeAvatarEntityDataPayload(entityID, properties); } } } @@ -2754,15 +2755,15 @@ void AvatarData::setAttachmentsVariant(const QVariantList& variant) { const int MAX_NUM_AVATAR_ENTITIES = 42; -void AvatarData::updateAvatarEntity(const QUuid& entityID, const QByteArray& entityData) { +void AvatarData::storeAvatarEntityDataPayload(const QUuid& entityID, const QByteArray& data) { _avatarEntitiesLock.withWriteLock([&] { AvatarEntityMap::iterator itr = _avatarEntityData.find(entityID); if (itr == _avatarEntityData.end()) { if (_avatarEntityData.size() < MAX_NUM_AVATAR_ENTITIES) { - _avatarEntityData.insert(entityID, entityData); + _avatarEntityData.insert(entityID, data); } } else { - itr.value() = entityData; + itr.value() = data; } }); @@ -2775,6 +2776,10 @@ void AvatarData::updateAvatarEntity(const QUuid& entityID, const QByteArray& ent } } +void AvatarData::updateAvatarEntity(const QUuid& entityID, const QString& entityPropertiesString) { + // TODO: implement this as API exposed to JS +} + void AvatarData::clearAvatarEntity(const QUuid& entityID, bool requiresRemovalFromTree) { bool removedEntity = false; diff --git a/libraries/avatars/src/AvatarData.h b/libraries/avatars/src/AvatarData.h index b42c387f61..1137cff869 100644 --- a/libraries/avatars/src/AvatarData.h +++ b/libraries/avatars/src/AvatarData.h @@ -952,13 +952,14 @@ public: // FIXME: Can this name be improved? Can it be deprecated? Q_INVOKABLE virtual void setAttachmentsVariant(const QVariantList& variant); + void storeAvatarEntityDataPayload(const QUuid& entityID, const QByteArray& payload); /**jsdoc * @function MyAvatar.updateAvatarEntity * @param {Uuid} entityID * @param {string} entityData */ - Q_INVOKABLE void updateAvatarEntity(const QUuid& entityID, const QByteArray& entityData); + Q_INVOKABLE void updateAvatarEntity(const QUuid& entityID, const QString& entityPropertiesString); /**jsdoc * @function MyAvatar.clearAvatarEntity diff --git a/libraries/entities/src/EntityEditPacketSender.cpp b/libraries/entities/src/EntityEditPacketSender.cpp index 8d0348bae6..dfc2d45d36 100644 --- a/libraries/entities/src/EntityEditPacketSender.cpp +++ b/libraries/entities/src/EntityEditPacketSender.cpp @@ -39,8 +39,7 @@ void EntityEditPacketSender::adjustEditPacketForClockSkew(PacketType type, QByte } } -void EntityEditPacketSender::queueEditAvatarEntityMessage(PacketType type, - EntityTreePointer entityTree, +void EntityEditPacketSender::queueEditAvatarEntityMessage(EntityTreePointer entityTree, EntityItemID entityItemID, const EntityItemProperties& properties) { assert(_myAvatar); @@ -53,6 +52,7 @@ void EntityEditPacketSender::queueEditAvatarEntityMessage(PacketType type, qCDebug(entities) << "EntityEditPacketSender::queueEditAvatarEntityMessage can't find entity: " << entityItemID; return; } + entity->setLastBroadcast(usecTimestampNow()); // serialize ALL properties in an "AvatarEntity" packet // rather than just the ones being edited. @@ -65,17 +65,14 @@ void EntityEditPacketSender::queueEditAvatarEntityMessage(PacketType type, OctreeElement::AppendState appendState = entity->appendEntityData(&packetData, params, extra); if (appendState != OctreeElement::COMPLETED) { - // this entity is too big + // this entity's payload is too big return; } packetData.shrinkByteArrays(); - _myAvatar->updateAvatarEntity(entityItemID, packetData.getUncompressedByteArray()); - - entity->setLastBroadcast(usecTimestampNow()); + _myAvatar->storeAvatarEntityDataPayload(entity->getID(), packetData.getUncompressedByteArray()); } - void EntityEditPacketSender::queueEditEntityMessage(PacketType type, EntityTreePointer entityTree, EntityItemID entityItemID, @@ -85,7 +82,7 @@ void EntityEditPacketSender::queueEditEntityMessage(PacketType type, qCWarning(entities) << "Suppressing entity edit message: cannot send avatar entity edit with no myAvatar"; } else if (properties.getOwningAvatarID() == _myAvatar->getID()) { // this is an avatar-based entity --> update our avatar-data rather than sending to the entity-server - queueEditAvatarEntityMessage(type, entityTree, entityItemID, properties); + queueEditAvatarEntityMessage(entityTree, entityItemID, properties); } else { qCWarning(entities) << "Suppressing entity edit message: cannot send avatar entity edit for another avatar"; } diff --git a/libraries/entities/src/EntityEditPacketSender.h b/libraries/entities/src/EntityEditPacketSender.h index 9bf9095f7f..a4ec2c45f9 100644 --- a/libraries/entities/src/EntityEditPacketSender.h +++ b/libraries/entities/src/EntityEditPacketSender.h @@ -50,7 +50,7 @@ public slots: void processEntityEditNackPacket(QSharedPointer message, SharedNodePointer sendingNode); private: - void queueEditAvatarEntityMessage(PacketType type, EntityTreePointer entityTree, + void queueEditAvatarEntityMessage(EntityTreePointer entityTree, EntityItemID entityItemID, const EntityItemProperties& properties); private: From e37b5b52f0c52ef2a70e5fd48bbab7fc44e5add5 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Fri, 7 Dec 2018 08:39:37 -0800 Subject: [PATCH 07/43] correctly load AvatarEntities from settings redux --- interface/src/avatar/MyAvatar.cpp | 180 +++++++++++++----- interface/src/avatar/MyAvatar.h | 4 + .../src/avatars-renderer/Avatar.cpp | 9 - .../src/avatars-renderer/Avatar.h | 2 +- 4 files changed, 139 insertions(+), 56 deletions(-) diff --git a/interface/src/avatar/MyAvatar.cpp b/interface/src/avatar/MyAvatar.cpp index 197bfcdca5..f822d107ad 100755 --- a/interface/src/avatar/MyAvatar.cpp +++ b/interface/src/avatar/MyAvatar.cpp @@ -1299,19 +1299,25 @@ void MyAvatar::saveData() { _flyingHMDSetting.set(getFlyingHMDPref()); auto hmdInterface = DependencyManager::get(); - _avatarEntitiesLock.withReadLock([&] { - uint32_t numEntities = _avatarEntityData.size(); - uint32_t prevNumEntities = _avatarEntityCountSetting.get(0); - resizeAvatarEntitySettingHandles(std::max(numEntities, prevNumEntities)); + saveAvatarEntityDataToSettings(); +} + +void MyAvatar::saveAvatarEntityDataToSettings() { + uint32_t numEntities = _avatarEntityData.size(); + uint32_t prevNumEntities = _avatarEntityCountSetting.get(0); + resizeAvatarEntitySettingHandles(std::max(numEntities, prevNumEntities)); + if (numEntities > 0) { + _avatarEntitiesLock.withReadLock([&] { // Note: this roundabout path from AvatarEntityData to JSON string is NOT efficient - QScriptEngine engine; - QScriptValue toStringMethod = engine.evaluate("(function() { return JSON.stringify(this) })"); + //QScriptEngine engine; + //QScriptValue toStringMethod = engine.evaluate("(function() { return JSON.stringify(this) })"); AvatarEntityMap::const_iterator itr = _avatarEntityData.begin(); numEntities = 0; while (itr != _avatarEntityData.end()) { EntityItemProperties properties; QByteArray buffer = itr.value(); + /* TODO: fix this to read data from elsewhere if (properties.constructFromBuffer((uint8_t*)buffer.data(), buffer.size())) { if (properties.getParentID() == getSessionUUID()) { properties.setParentID(AVATAR_SELF_ID); @@ -1324,24 +1330,27 @@ void MyAvatar::saveData() { } else { // buffer is corrupt --> skip it } + */ + ++numEntities; ++itr; } - _avatarEntityCountSetting.set(numEntities); + }); + } + _avatarEntityCountSetting.set(numEntities); - if (numEntities < prevNumEntities) { - uint32_t numEntitiesToRemove = prevNumEntities - numEntities; - for (uint32_t i = 0; i < numEntitiesToRemove; ++i) { - if (_avatarEntityIDSettings.size() > numEntities) { - _avatarEntityIDSettings.back().remove(); - _avatarEntityIDSettings.pop_back(); - } - if (_avatarEntityDataSettings.size() > numEntities) { - _avatarEntityDataSettings.back().remove(); - _avatarEntityDataSettings.pop_back(); - } + if (numEntities < prevNumEntities) { + uint32_t numEntitiesToRemove = prevNumEntities - numEntities; + for (uint32_t i = 0; i < numEntitiesToRemove; ++i) { + if (_avatarEntityIDSettings.size() > numEntities) { + _avatarEntityIDSettings.back().remove(); + _avatarEntityIDSettings.pop_back(); + } + if (_avatarEntityDataSettings.size() > numEntities) { + _avatarEntityDataSettings.back().remove(); + _avatarEntityDataSettings.pop_back(); } } - }); + } } float loadSetting(Settings& settings, const QString& name, float defaultValue) { @@ -1459,6 +1468,7 @@ void MyAvatar::updateAvatarEntity(const QUuid& entityID, const EntityItemPropert OctreePacketData packetData(false, AvatarTraits::MAXIMUM_TRAIT_SIZE); EncodeBitstreamParams params; EntityTreeElementExtraEncodeDataPointer extra { nullptr }; + QUuid parentID = entity->getParentID(); OctreeElement::AppendState appendState = entity->appendEntityData(&packetData, params, extra); if (appendState != OctreeElement::COMPLETED) { @@ -1466,10 +1476,56 @@ void MyAvatar::updateAvatarEntity(const QUuid& entityID, const EntityItemPropert return; } - packetData.shrinkByteArrays(); - storeAvatarEntityDataPayload(entity->getID(), packetData.getUncompressedByteArray()); + //QByteArray tempArray = QByteArray::fromRawData((const char*)packetData.getUncompressedData(), packetData.getUncompressedSize()); + QByteArray tempArray((const char*)packetData.getUncompressedData(), packetData.getUncompressedSize()); + for (int i = 0; i < 4; ++i) { + tempArray[i] = (uint8_t)(0xff); + } + storeAvatarEntityDataPayload(entity->getID(), tempArray); } +void MyAvatar::updateAvatarEntities() { + // TODO: modify this info for MyAvatar + // AVATAR ENTITY UPDATE FLOW + // - if queueEditEntityMessage sees clientOnly flag it does _myAvatar->updateAvatarEntity() + // - updateAvatarEntity saves the bytes and flags the trait instance for the entity as updated + // - ClientTraitsHandler::sendChangedTraitsToMixer sends the entity bytes to the mixer which relays them to other interfaces + // - AvatarHashMap::processBulkAvatarTraits on other interfaces calls avatar->processTraitInstace + // - AvatarData::processTraitInstance calls updateAvatarEntity, which sets _avatarEntityDataChanged = true + // - (My)Avatar::simulate notices _avatarEntityDataChanged and here we are... + + // AVATAR ENTITY DELETE FLOW + // - EntityScriptingInterface::deleteEntity calls _myAvatar->clearAvatarEntity() for deleted avatar entities + // - clearAvatarEntity removes the avatar entity and flags the trait instance for the entity as deleted + // - ClientTraitsHandler::sendChangedTraitsToMixer sends a deletion to the mixer which relays to other interfaces + // - AvatarHashMap::processBulkAvatarTraits on other interfaces calls avatar->processDeletedTraitInstace + // - AvatarData::processDeletedTraitInstance calls clearAvatarEntity + // - AvatarData::clearAvatarEntity sets _avatarEntityDataChanged = true and adds the ID to the detached list + // - Avatar::simulate notices _avatarEntityDataChanged and here we are... + + if (!_avatarEntityDataChanged) { + return; + } + + if (getID().isNull() || + getID() == AVATAR_SELF_ID || + DependencyManager::get()->getSessionUUID() == QUuid()) { + // wait until MyAvatar and this Node gets an ID before doing this. Otherwise, various things go wrong -- + // things get their parent fixed up from AVATAR_SELF_ID to a null uuid which means "no parent". + return; + } + + auto treeRenderer = DependencyManager::get(); + EntityTreePointer entityTree = treeRenderer ? treeRenderer->getTree() : nullptr; + if (!entityTree) { + return; + } + + loadAvatarEntityDataFromSettings(); + setAvatarEntityDataChanged(false); +} + + void MyAvatar::loadData() { getHead()->setBasePitch(_headPitchSetting.get()); @@ -1482,31 +1538,7 @@ void MyAvatar::loadData() { useFullAvatarURL(_fullAvatarURLFromPreferences, _fullAvatarModelName); - int numEntities = _avatarEntityCountSetting.get(0); - auto entityTree = DependencyManager::get()->getTree(); - - if (numEntities > 0) { - QScriptEngine scriptEngine; - for (int i = 0; i < numEntities; i++) { - resizeAvatarEntitySettingHandles(i); - QUuid entityID = QUuid::createUuid(); // generate a new ID - - // NOTE: this path from EntityItemProperties JSON string to EntityItemProperties is NOT efficient - QString propertiesString = _avatarEntityDataSettings[i].get(); - QJsonDocument propertiesDoc = QJsonDocument::fromJson(propertiesString.toUtf8()); - QJsonObject propertiesObj = propertiesDoc.object(); - QVariant propertiesVariant(propertiesObj); - QVariantMap propertiesMap = propertiesVariant.toMap(); - QScriptValue propertiesScriptValue = variantMapToScriptValue(propertiesMap, scriptEngine); - EntityItemProperties properties; - EntityItemPropertiesFromScriptValueIgnoreReadOnly(propertiesScriptValue, properties); - - // the ClientOnly property can get stripped out elsewhere so we need to always set it true here - properties.setClientOnly(true); - - updateAvatarEntity(entityID, properties); - } - } + loadAvatarEntityDataFromSettings(); // Flying preferences must be loaded before calling setFlyingEnabled() Setting::Handle firstRunVal { Settings::firstRun, true }; @@ -1528,6 +1560,62 @@ void MyAvatar::loadData() { setEnableDebugDrawPosition(Menu::getInstance()->isOptionChecked(MenuOption::AnimDebugDrawPosition)); } +void MyAvatar::loadAvatarEntityDataFromSettings() { + _avatarEntitiesLock.withReadLock([&] { + _avatarEntityData.clear(); + }); + + int numEntities = _avatarEntityCountSetting.get(0); + if (numEntities == 0) { + return; + } + + QScriptEngine scriptEngine; + + std::vector entitiesToLoad; + entitiesToLoad.resize(numEntities); + for (int i = 0; i < numEntities; i++) { + resizeAvatarEntitySettingHandles(i); + + // NOTE: this path from EntityItemProperties JSON string to EntityItemProperties is NOT efficient + QString propertiesString = _avatarEntityDataSettings[i].get(); + QJsonDocument propertiesDoc = QJsonDocument::fromJson(propertiesString.toUtf8()); + QJsonObject propertiesObj = propertiesDoc.object(); + QVariant propertiesVariant(propertiesObj); + QVariantMap propertiesMap = propertiesVariant.toMap(); + QScriptValue propertiesScriptValue = variantMapToScriptValue(propertiesMap, scriptEngine); + EntityItemProperties& properties = entitiesToLoad[i]; + EntityItemPropertiesFromScriptValueIgnoreReadOnly(propertiesScriptValue, properties); + + // the ClientOnly property can get stripped out elsewhere so we need to always set it true here + properties.setClientOnly(true); + } + + auto entityTreeRenderer = qApp->getEntities(); + EntityTreePointer entityTree = entityTreeRenderer ? entityTreeRenderer->getTree() : nullptr; + if (entityTree) { + OctreePacketData packetData(false, AvatarTraits::MAXIMUM_TRAIT_SIZE); + EncodeBitstreamParams params; + EntityTreeElementExtraEncodeDataPointer extra { nullptr }; + for (uint32_t i = 0; i < entitiesToLoad.size(); ++i) { + // try to create the entity + QUuid entityID = QUuid::createUuid(); // generate a new ID + entityTree->withWriteLock([&] { + EntityItemPointer entity = entityTree->addEntity(entityID, entitiesToLoad[i]); + if (entity) { + // use the entity to build the data payload + OctreeElement::AppendState appendState = entity->appendEntityData(&packetData, params, extra); + if (appendState == OctreeElement::COMPLETED) { + QByteArray tempArray = QByteArray::fromRawData((const char*)packetData.getUncompressedData(), packetData.getUncompressedSize()); + storeAvatarEntityDataPayload(entityID, tempArray); + } + packetData.reset(); + } + }); + } + } +} + void MyAvatar::saveAttachmentData(const AttachmentData& attachment) const { Settings settings; settings.beginGroup("savedAttachmentData"); diff --git a/interface/src/avatar/MyAvatar.h b/interface/src/avatar/MyAvatar.h index b1848a19b9..4568688d27 100644 --- a/interface/src/avatar/MyAvatar.h +++ b/interface/src/avatar/MyAvatar.h @@ -13,6 +13,7 @@ #define hifi_MyAvatar_h #include +#include // adebug #include @@ -577,7 +578,9 @@ public: // get/set avatar data void resizeAvatarEntitySettingHandles(unsigned int avatarEntityIndex); void saveData(); + void saveAvatarEntityDataToSettings(); void loadData(); + void loadAvatarEntityDataFromSettings(); void saveAttachmentData(const AttachmentData& attachment) const; AttachmentData loadAttachmentData(const QUrl& modelURL, const QString& jointName = QString()) const; @@ -1184,6 +1187,7 @@ public: virtual void setAttachmentsVariant(const QVariantList& variant) override; glm::vec3 getNextPosition() { return _goToPending ? _goToPosition : getWorldPosition(); } + void updateAvatarEntities() override; /**jsdoc * Create a new grab. diff --git a/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp b/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp index 23093028b1..722c3c8a5e 100644 --- a/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp +++ b/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp @@ -332,14 +332,6 @@ void Avatar::updateAvatarEntities() { return; } - if (getID().isNull() || - getID() == AVATAR_SELF_ID || - DependencyManager::get()->getSessionUUID() == QUuid()) { - // wait until MyAvatar and this Node gets an ID before doing this. Otherwise, various things go wrong -- - // things get their parent fixed up from AVATAR_SELF_ID to a null uuid which means "no parent". - return; - } - auto treeRenderer = DependencyManager::get(); EntityTreePointer entityTree = treeRenderer ? treeRenderer->getTree() : nullptr; if (!entityTree) { @@ -380,7 +372,6 @@ void Avatar::updateAvatarEntities() { EntityItemProperties properties; { - // create a temporary EntityItem to unpack the data int32_t bytesLeftToRead = data.size(); unsigned char* dataAt = (unsigned char*)(data.data()); properties.constructFromBuffer(dataAt, bytesLeftToRead); diff --git a/libraries/avatars-renderer/src/avatars-renderer/Avatar.h b/libraries/avatars-renderer/src/avatars-renderer/Avatar.h index fddd52a6dd..9a57e46f1a 100644 --- a/libraries/avatars-renderer/src/avatars-renderer/Avatar.h +++ b/libraries/avatars-renderer/src/avatars-renderer/Avatar.h @@ -139,7 +139,7 @@ public: typedef render::Payload Payload; void init(); - void updateAvatarEntities(); + virtual void updateAvatarEntities(); void removeAvatarEntitiesFromTree(); void simulate(float deltaTime, bool inView); virtual void simulateAttachments(float deltaTime); From 0ed936520dde55d08dca60798ec972abc5edb202 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Fri, 7 Dec 2018 10:44:09 -0800 Subject: [PATCH 08/43] MyAvatar saving to settings works again --- interface/src/avatar/MyAvatar.cpp | 62 ++++++++++++------------------- interface/src/avatar/MyAvatar.h | 3 +- 2 files changed, 26 insertions(+), 39 deletions(-) diff --git a/interface/src/avatar/MyAvatar.cpp b/interface/src/avatar/MyAvatar.cpp index f822d107ad..4a1fc1b740 100755 --- a/interface/src/avatar/MyAvatar.cpp +++ b/interface/src/avatar/MyAvatar.cpp @@ -1252,7 +1252,7 @@ void MyAvatar::saveAvatarUrl() { } } -void MyAvatar::resizeAvatarEntitySettingHandles(unsigned int avatarEntityIndex) { +void MyAvatar::resizeAvatarEntitySettingHandles(uint32_t maxIndex) { // The original Settings interface saved avatar-entity array data like this: // Avatar/avatarEntityData/size: 5 // Avatar/avatarEntityData/1/id: ... @@ -1262,14 +1262,15 @@ void MyAvatar::resizeAvatarEntitySettingHandles(unsigned int avatarEntityIndex) // Avatar/avatarEntityData/5/properties: ... // // Create Setting::Handles to mimic this. - - while (_avatarEntityIDSettings.size() <= avatarEntityIndex) { + uint32_t settingsIndex = _avatarEntityIDSettings.size() + 1; + while (settingsIndex <= maxIndex) { Setting::Handle idHandle(QStringList() << AVATAR_SETTINGS_GROUP_NAME << "avatarEntityData" - << QString::number(avatarEntityIndex + 1) << "id", QUuid().toString()); + << QString::number(settingsIndex) << "id", QUuid().toString()); _avatarEntityIDSettings.push_back(idHandle); Setting::Handle dataHandle(QStringList() << AVATAR_SETTINGS_GROUP_NAME << "avatarEntityData" - << QString::number(avatarEntityIndex + 1) << "properties", QString()); + << QString::number(settingsIndex) << "properties", QString()); _avatarEntityDataSettings.push_back(dataHandle); + settingsIndex++; } } @@ -1303,41 +1304,23 @@ void MyAvatar::saveData() { } void MyAvatar::saveAvatarEntityDataToSettings() { - uint32_t numEntities = _avatarEntityData.size(); + // save new settings + uint32_t numEntities = _avatarEntitiesAsPropertiesStrings.size(); + _avatarEntityCountSetting.set(numEntities); uint32_t prevNumEntities = _avatarEntityCountSetting.get(0); resizeAvatarEntitySettingHandles(std::max(numEntities, prevNumEntities)); if (numEntities > 0) { _avatarEntitiesLock.withReadLock([&] { - - // Note: this roundabout path from AvatarEntityData to JSON string is NOT efficient - //QScriptEngine engine; - //QScriptValue toStringMethod = engine.evaluate("(function() { return JSON.stringify(this) })"); - AvatarEntityMap::const_iterator itr = _avatarEntityData.begin(); - numEntities = 0; - while (itr != _avatarEntityData.end()) { - EntityItemProperties properties; - QByteArray buffer = itr.value(); - /* TODO: fix this to read data from elsewhere - if (properties.constructFromBuffer((uint8_t*)buffer.data(), buffer.size())) { - if (properties.getParentID() == getSessionUUID()) { - properties.setParentID(AVATAR_SELF_ID); - } - QScriptValue scriptValue = EntityItemPropertiesToScriptValue(&engine, properties); - scriptValue.setProperty("toString", toStringMethod); - _avatarEntityDataSettings[numEntities].set(scriptValue.toString()); - _avatarEntityIDSettings[numEntities].set(itr.key().toString()); - ++numEntities; - } else { - // buffer is corrupt --> skip it + uint32_t i = 0; + for (const auto& mapEntry : _avatarEntitiesAsPropertiesStrings) { + _avatarEntityDataSettings[i].set(mapEntry.second); + _avatarEntityIDSettings[i].set(mapEntry.first.toString()); + ++i; } - */ - ++numEntities; - ++itr; - } - }); + }); } - _avatarEntityCountSetting.set(numEntities); + // remove old settings if any if (numEntities < prevNumEntities) { uint32_t numEntitiesToRemove = prevNumEntities - numEntities; for (uint32_t i = 0; i < numEntitiesToRemove; ++i) { @@ -1468,7 +1451,6 @@ void MyAvatar::updateAvatarEntity(const QUuid& entityID, const EntityItemPropert OctreePacketData packetData(false, AvatarTraits::MAXIMUM_TRAIT_SIZE); EncodeBitstreamParams params; EntityTreeElementExtraEncodeDataPointer extra { nullptr }; - QUuid parentID = entity->getParentID(); OctreeElement::AppendState appendState = entity->appendEntityData(&packetData, params, extra); if (appendState != OctreeElement::COMPLETED) { @@ -1571,12 +1553,10 @@ void MyAvatar::loadAvatarEntityDataFromSettings() { } QScriptEngine scriptEngine; - std::vector entitiesToLoad; entitiesToLoad.resize(numEntities); + resizeAvatarEntitySettingHandles(numEntities); for (int i = 0; i < numEntities; i++) { - resizeAvatarEntitySettingHandles(i); - // NOTE: this path from EntityItemProperties JSON string to EntityItemProperties is NOT efficient QString propertiesString = _avatarEntityDataSettings[i].get(); QJsonDocument propertiesDoc = QJsonDocument::fromJson(propertiesString.toUtf8()); @@ -1584,13 +1564,15 @@ void MyAvatar::loadAvatarEntityDataFromSettings() { QVariant propertiesVariant(propertiesObj); QVariantMap propertiesMap = propertiesVariant.toMap(); QScriptValue propertiesScriptValue = variantMapToScriptValue(propertiesMap, scriptEngine); + + // NOTE: we grab properties by reference and wrangle it: EntityItemProperties& properties = entitiesToLoad[i]; EntityItemPropertiesFromScriptValueIgnoreReadOnly(propertiesScriptValue, properties); - // the ClientOnly property can get stripped out elsewhere so we need to always set it true here properties.setClientOnly(true); } + _avatarEntitiesAsPropertiesStrings.clear(); auto entityTreeRenderer = qApp->getEntities(); EntityTreePointer entityTree = entityTreeRenderer ? entityTreeRenderer->getTree() : nullptr; if (entityTree) { @@ -1606,13 +1588,17 @@ void MyAvatar::loadAvatarEntityDataFromSettings() { // use the entity to build the data payload OctreeElement::AppendState appendState = entity->appendEntityData(&packetData, params, extra); if (appendState == OctreeElement::COMPLETED) { + // only remember an AvatarEntity that successfully loads QByteArray tempArray = QByteArray::fromRawData((const char*)packetData.getUncompressedData(), packetData.getUncompressedSize()); storeAvatarEntityDataPayload(entityID, tempArray); + _avatarEntitiesAsPropertiesStrings[entityID] = _avatarEntityDataSettings[i].get(); } packetData.reset(); } }); } + } else { + // TODO? handle this case: try to load AvatatEntityData from settings again later? } } diff --git a/interface/src/avatar/MyAvatar.h b/interface/src/avatar/MyAvatar.h index 4568688d27..b343f9bf79 100644 --- a/interface/src/avatar/MyAvatar.h +++ b/interface/src/avatar/MyAvatar.h @@ -576,7 +576,7 @@ public: float getHMDRollControlRate() const { return _hmdRollControlRate; } // get/set avatar data - void resizeAvatarEntitySettingHandles(unsigned int avatarEntityIndex); + void resizeAvatarEntitySettingHandles(uint32_t maxIndex); void saveData(); void saveAvatarEntityDataToSettings(); void loadData(); @@ -1945,6 +1945,7 @@ private: Setting::Handle _allowTeleportingSetting { "allowTeleporting", true }; std::vector> _avatarEntityIDSettings; std::vector> _avatarEntityDataSettings; + std::map _avatarEntitiesAsPropertiesStrings; }; QScriptValue audioListenModeToScriptValue(QScriptEngine* engine, const AudioListenerMode& audioListenerMode); From ec384d7dbbf5097789810c93963030698ab6b1f8 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Fri, 7 Dec 2018 11:30:53 -0800 Subject: [PATCH 09/43] more correct reload AvatarEntityData from settings --- interface/src/Application.cpp | 3 +- interface/src/avatar/MyAvatar.cpp | 46 ++++++++++++++++--------------- interface/src/avatar/MyAvatar.h | 2 ++ 3 files changed, 28 insertions(+), 23 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 134c375b56..0fbe9fba94 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -6783,7 +6783,8 @@ void Application::clearDomainOctreeDetails() { DependencyManager::get()->clearUnusedResources(); DependencyManager::get()->clearUnusedResources(); - getMyAvatar()->setAvatarEntityDataChanged(true); + // we just deleted all of MyAvatar's AvatarEntities so we flag it to reload from settings + getMyAvatar()->rememberToReloadOfAvatarEntityDataFromSettings(); } void Application::domainURLChanged(QUrl domainURL) { diff --git a/interface/src/avatar/MyAvatar.cpp b/interface/src/avatar/MyAvatar.cpp index 4a1fc1b740..ce70d40c74 100755 --- a/interface/src/avatar/MyAvatar.cpp +++ b/interface/src/avatar/MyAvatar.cpp @@ -1443,8 +1443,10 @@ void MyAvatar::updateAvatarEntity(const QUuid& entityID, const EntityItemPropert // TODO? handle this case? return; } + // TODO: remember this entity and its properties, so we can save to settings } else { // TODO: propagate changes to entity + // TODO: and remember these changes so we can save to settings } } @@ -1474,7 +1476,7 @@ void MyAvatar::updateAvatarEntities() { // - ClientTraitsHandler::sendChangedTraitsToMixer sends the entity bytes to the mixer which relays them to other interfaces // - AvatarHashMap::processBulkAvatarTraits on other interfaces calls avatar->processTraitInstace // - AvatarData::processTraitInstance calls updateAvatarEntity, which sets _avatarEntityDataChanged = true - // - (My)Avatar::simulate notices _avatarEntityDataChanged and here we are... + // - (My)Avatar::simulate calls updateAvatarEntities every frame and here we are... // AVATAR ENTITY DELETE FLOW // - EntityScriptingInterface::deleteEntity calls _myAvatar->clearAvatarEntity() for deleted avatar entities @@ -1483,28 +1485,26 @@ void MyAvatar::updateAvatarEntities() { // - AvatarHashMap::processBulkAvatarTraits on other interfaces calls avatar->processDeletedTraitInstace // - AvatarData::processDeletedTraitInstance calls clearAvatarEntity // - AvatarData::clearAvatarEntity sets _avatarEntityDataChanged = true and adds the ID to the detached list - // - Avatar::simulate notices _avatarEntityDataChanged and here we are... + // - Avatar::simulate calls updateAvatarEntities every frame and here we are... - if (!_avatarEntityDataChanged) { - return; + if (_reloadAvatarEntityDataFromSettings) { + + if (getID().isNull() || + getID() == AVATAR_SELF_ID || + DependencyManager::get()->getSessionUUID() == QUuid()) { + // wait until MyAvatar and this Node gets an ID before doing this. Otherwise, various things go wrong -- + // things get their parent fixed up from AVATAR_SELF_ID to a null uuid which means "no parent". + return; + } + + auto treeRenderer = DependencyManager::get(); + EntityTreePointer entityTree = treeRenderer ? treeRenderer->getTree() : nullptr; + if (!entityTree) { + return; + } + + loadAvatarEntityDataFromSettings(); } - - if (getID().isNull() || - getID() == AVATAR_SELF_ID || - DependencyManager::get()->getSessionUUID() == QUuid()) { - // wait until MyAvatar and this Node gets an ID before doing this. Otherwise, various things go wrong -- - // things get their parent fixed up from AVATAR_SELF_ID to a null uuid which means "no parent". - return; - } - - auto treeRenderer = DependencyManager::get(); - EntityTreePointer entityTree = treeRenderer ? treeRenderer->getTree() : nullptr; - if (!entityTree) { - return; - } - - loadAvatarEntityDataFromSettings(); - setAvatarEntityDataChanged(false); } @@ -1547,6 +1547,7 @@ void MyAvatar::loadAvatarEntityDataFromSettings() { _avatarEntityData.clear(); }); + _reloadAvatarEntityDataFromSettings = false; int numEntities = _avatarEntityCountSetting.get(0); if (numEntities == 0) { return; @@ -1588,7 +1589,7 @@ void MyAvatar::loadAvatarEntityDataFromSettings() { // use the entity to build the data payload OctreeElement::AppendState appendState = entity->appendEntityData(&packetData, params, extra); if (appendState == OctreeElement::COMPLETED) { - // only remember an AvatarEntity that successfully loads + // only remember an AvatarEntity that successfully loads and can be packed QByteArray tempArray = QByteArray::fromRawData((const char*)packetData.getUncompressedData(), packetData.getUncompressedSize()); storeAvatarEntityDataPayload(entityID, tempArray); _avatarEntitiesAsPropertiesStrings[entityID] = _avatarEntityDataSettings[i].get(); @@ -2089,6 +2090,7 @@ void MyAvatar::removeWearableAvatarEntities() { } QVariantList MyAvatar::getAvatarEntitiesVariant() { + // NOTE: this method is NOT efficient QVariantList avatarEntitiesData; QScriptEngine scriptEngine; auto treeRenderer = DependencyManager::get(); diff --git a/interface/src/avatar/MyAvatar.h b/interface/src/avatar/MyAvatar.h index b343f9bf79..a6df4de7bf 100644 --- a/interface/src/avatar/MyAvatar.h +++ b/interface/src/avatar/MyAvatar.h @@ -1188,6 +1188,7 @@ public: glm::vec3 getNextPosition() { return _goToPending ? _goToPosition : getWorldPosition(); } void updateAvatarEntities() override; + void rememberToReloadOfAvatarEntityDataFromSettings() { _reloadAvatarEntityDataFromSettings = true; } /**jsdoc * Create a new grab. @@ -1927,6 +1928,7 @@ private: bool _haveReceivedHeightLimitsFromDomain { false }; int _disableHandTouchCount { 0 }; bool _skeletonModelLoaded { false }; + bool _reloadAvatarEntityDataFromSettings { true }; Setting::Handle _dominantHandSetting; Setting::Handle _headPitchSetting; From fe2ee68b79bcf9a4f53795cdc30ae9582305edd2 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Fri, 7 Dec 2018 13:49:48 -0800 Subject: [PATCH 10/43] add EntityItemProperties::copyFromJSONString() --- interface/src/avatar/MyAvatar.cpp | 36 ++++++++----------- interface/src/avatar/MyAvatar.h | 6 ++-- .../entities/src/EntityItemProperties.cpp | 14 +++++++- libraries/entities/src/EntityItemProperties.h | 1 + 4 files changed, 32 insertions(+), 25 deletions(-) diff --git a/interface/src/avatar/MyAvatar.cpp b/interface/src/avatar/MyAvatar.cpp index ce70d40c74..7d6318ddbb 100755 --- a/interface/src/avatar/MyAvatar.cpp +++ b/interface/src/avatar/MyAvatar.cpp @@ -50,7 +50,6 @@ #include #include #include -#include #include "MyHead.h" #include "MySkeletonModel.h" @@ -1305,14 +1304,14 @@ void MyAvatar::saveData() { void MyAvatar::saveAvatarEntityDataToSettings() { // save new settings - uint32_t numEntities = _avatarEntitiesAsPropertiesStrings.size(); + uint32_t numEntities = _avatarEntityStrings.size(); _avatarEntityCountSetting.set(numEntities); uint32_t prevNumEntities = _avatarEntityCountSetting.get(0); resizeAvatarEntitySettingHandles(std::max(numEntities, prevNumEntities)); if (numEntities > 0) { _avatarEntitiesLock.withReadLock([&] { uint32_t i = 0; - for (const auto& mapEntry : _avatarEntitiesAsPropertiesStrings) { + for (const auto& mapEntry : _avatarEntityStrings) { _avatarEntityDataSettings[i].set(mapEntry.second); _avatarEntityIDSettings[i].set(mapEntry.first.toString()); ++i; @@ -1430,7 +1429,13 @@ void MyAvatar::setEnableInverseKinematics(bool isEnabled) { _skeletonModel->getRig().setEnableInverseKinematics(isEnabled); } -void MyAvatar::updateAvatarEntity(const QUuid& entityID, const EntityItemProperties& properties) { +void MyAvatar::updateAvatarEntity(const QUuid& entityID, const QString& entityPropertiesString) { + /* TODO: implement this so JS can add/update (and delete?) AvatarEntitieskj:w + // convert string to properties + // NOTE: this path from EntityItemProperties JSON string to EntityItemProperties is NOT efficient + EntityItemProperties properties; + properties.copyFromJSONString(scriptEngine, entityPropertiesString); + auto entityTreeRenderer = qApp->getEntities(); EntityTreePointer entityTree = entityTreeRenderer ? entityTreeRenderer->getTree() : nullptr; EntityItemPointer entity; @@ -1460,12 +1465,9 @@ void MyAvatar::updateAvatarEntity(const QUuid& entityID, const EntityItemPropert return; } - //QByteArray tempArray = QByteArray::fromRawData((const char*)packetData.getUncompressedData(), packetData.getUncompressedSize()); - QByteArray tempArray((const char*)packetData.getUncompressedData(), packetData.getUncompressedSize()); - for (int i = 0; i < 4; ++i) { - tempArray[i] = (uint8_t)(0xff); - } + QByteArray tempArray = QByteArray::fromRawData((const char*)packetData.getUncompressedData(), packetData.getUncompressedSize()); storeAvatarEntityDataPayload(entity->getID(), tempArray); + */ } void MyAvatar::updateAvatarEntities() { @@ -1558,22 +1560,14 @@ void MyAvatar::loadAvatarEntityDataFromSettings() { entitiesToLoad.resize(numEntities); resizeAvatarEntitySettingHandles(numEntities); for (int i = 0; i < numEntities; i++) { - // NOTE: this path from EntityItemProperties JSON string to EntityItemProperties is NOT efficient - QString propertiesString = _avatarEntityDataSettings[i].get(); - QJsonDocument propertiesDoc = QJsonDocument::fromJson(propertiesString.toUtf8()); - QJsonObject propertiesObj = propertiesDoc.object(); - QVariant propertiesVariant(propertiesObj); - QVariantMap propertiesMap = propertiesVariant.toMap(); - QScriptValue propertiesScriptValue = variantMapToScriptValue(propertiesMap, scriptEngine); - - // NOTE: we grab properties by reference and wrangle it: + // DANGER: this JSONString --> EntityItemProperties operation is expensive EntityItemProperties& properties = entitiesToLoad[i]; - EntityItemPropertiesFromScriptValueIgnoreReadOnly(propertiesScriptValue, properties); + properties.copyFromJSONString(scriptEngine, _avatarEntityDataSettings[i].get()); // the ClientOnly property can get stripped out elsewhere so we need to always set it true here properties.setClientOnly(true); } - _avatarEntitiesAsPropertiesStrings.clear(); + _avatarEntityStrings.clear(); auto entityTreeRenderer = qApp->getEntities(); EntityTreePointer entityTree = entityTreeRenderer ? entityTreeRenderer->getTree() : nullptr; if (entityTree) { @@ -1592,7 +1586,7 @@ void MyAvatar::loadAvatarEntityDataFromSettings() { // only remember an AvatarEntity that successfully loads and can be packed QByteArray tempArray = QByteArray::fromRawData((const char*)packetData.getUncompressedData(), packetData.getUncompressedSize()); storeAvatarEntityDataPayload(entityID, tempArray); - _avatarEntitiesAsPropertiesStrings[entityID] = _avatarEntityDataSettings[i].get(); + _avatarEntityStrings[entityID] = _avatarEntityDataSettings[i].get(); } packetData.reset(); } diff --git a/interface/src/avatar/MyAvatar.h b/interface/src/avatar/MyAvatar.h index a6df4de7bf..b05d9ed875 100644 --- a/interface/src/avatar/MyAvatar.h +++ b/interface/src/avatar/MyAvatar.h @@ -1407,8 +1407,7 @@ public slots: */ bool getEnableMeshVisible() const override; - // TODO: make this invokable, probably also move down to AvatarData - void updateAvatarEntity(const QUuid& entityID, const EntityItemProperties& properties); + void updateAvatarEntity(const QUuid& entityID, const QString& entityPropertiesString) override; /**jsdoc * Set whether or not your avatar mesh is visible. @@ -1947,7 +1946,8 @@ private: Setting::Handle _allowTeleportingSetting { "allowTeleporting", true }; std::vector> _avatarEntityIDSettings; std::vector> _avatarEntityDataSettings; - std::map _avatarEntitiesAsPropertiesStrings; + std::map _avatarEntityStrings; + std::map _avatarEntityProperties; }; QScriptValue audioListenModeToScriptValue(QScriptEngine* engine, const AudioListenerMode& audioListenerMode); diff --git a/libraries/entities/src/EntityItemProperties.cpp b/libraries/entities/src/EntityItemProperties.cpp index 0badd00bc3..6ba4c84985 100644 --- a/libraries/entities/src/EntityItemProperties.cpp +++ b/libraries/entities/src/EntityItemProperties.cpp @@ -28,6 +28,7 @@ #include #include #include +#include #include "EntitiesLogging.h" #include "EntityItem.h" @@ -2033,6 +2034,18 @@ void EntityItemProperties::copyFromScriptValue(const QScriptValue& object, bool _lastEdited = usecTimestampNow(); } +void EntityItemProperties::copyFromJSONString(QScriptEngine& scriptEngine, const QString& jsonString) { + // DANGER: this method is expensive + QJsonDocument propertiesDoc = QJsonDocument::fromJson(jsonString.toUtf8()); + QJsonObject propertiesObj = propertiesDoc.object(); + QVariant propertiesVariant(propertiesObj); + QVariantMap propertiesMap = propertiesVariant.toMap(); + QScriptValue propertiesScriptValue = variantMapToScriptValue(propertiesMap, scriptEngine); + bool honorReadOnly = true; + copyFromScriptValue(propertiesScriptValue, honorReadOnly); +} + + void EntityItemProperties::merge(const EntityItemProperties& other) { // Core COPY_PROPERTY_IF_CHANGED(simulationOwner); @@ -2262,7 +2275,6 @@ void EntityItemPropertiesFromScriptValueHonorReadOnly(const QScriptValue &object properties.copyFromScriptValue(object, true); } - QScriptValue EntityPropertyFlagsToScriptValue(QScriptEngine* engine, const EntityPropertyFlags& flags) { return EntityItemProperties::entityPropertyFlagsToScriptValue(engine, flags); } diff --git a/libraries/entities/src/EntityItemProperties.h b/libraries/entities/src/EntityItemProperties.h index 4a9729f5fe..48f4aa333e 100644 --- a/libraries/entities/src/EntityItemProperties.h +++ b/libraries/entities/src/EntityItemProperties.h @@ -109,6 +109,7 @@ public: virtual QScriptValue copyToScriptValue(QScriptEngine* engine, bool skipDefaults, bool allowUnknownCreateTime = false, bool strictSemantics = false, EntityPsuedoPropertyFlags psueudoPropertyFlags = EntityPsuedoPropertyFlags()) const; virtual void copyFromScriptValue(const QScriptValue& object, bool honorReadOnly); + void copyFromJSONString(QScriptEngine& scriptEngine, const QString& jsonString); static QScriptValue entityPropertyFlagsToScriptValue(QScriptEngine* engine, const EntityPropertyFlags& flags); static void entityPropertyFlagsFromScriptValue(const QScriptValue& object, EntityPropertyFlags& flags); From 6c81e8845bc800f80fdff4d583f356490319cfdd Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Fri, 7 Dec 2018 14:43:44 -0800 Subject: [PATCH 11/43] cleanup --- interface/src/avatar/MyAvatar.cpp | 59 ++++++++++++------- interface/src/avatar/MyAvatar.h | 3 +- libraries/avatars/src/AvatarData.cpp | 2 +- libraries/avatars/src/AvatarData.h | 4 +- .../entities/src/EntityEditPacketSender.cpp | 4 +- libraries/octree/src/OctreePacketData.cpp | 9 --- libraries/octree/src/OctreePacketData.h | 2 - 7 files changed, 44 insertions(+), 39 deletions(-) diff --git a/interface/src/avatar/MyAvatar.cpp b/interface/src/avatar/MyAvatar.cpp index 7d6318ddbb..04f1cb599e 100755 --- a/interface/src/avatar/MyAvatar.cpp +++ b/interface/src/avatar/MyAvatar.cpp @@ -1303,6 +1303,10 @@ void MyAvatar::saveData() { } void MyAvatar::saveAvatarEntityDataToSettings() { + if (!_entitiesToSaveToSettings.empty()) { + // TODO: save these to settings. + _entitiesToSaveToSettings.clear(); + } // save new settings uint32_t numEntities = _avatarEntityStrings.size(); _avatarEntityCountSetting.set(numEntities); @@ -1429,17 +1433,28 @@ void MyAvatar::setEnableInverseKinematics(bool isEnabled) { _skeletonModel->getRig().setEnableInverseKinematics(isEnabled); } -void MyAvatar::updateAvatarEntity(const QUuid& entityID, const QString& entityPropertiesString) { - /* TODO: implement this so JS can add/update (and delete?) AvatarEntitieskj:w - // convert string to properties - // NOTE: this path from EntityItemProperties JSON string to EntityItemProperties is NOT efficient - EntityItemProperties properties; - properties.copyFromJSONString(scriptEngine, entityPropertiesString); +void MyAvatar::storeAvatarEntityDataPayload(const QUuid& entityID, const QByteArray& payload) { + AvatarData::storeAvatarEntityDataPayload(entityID, payload); + _entitiesToSaveToSettings.insert(entityID); +} +/* TODO: verify we don't need this +void MyAvatar::updateAvatarEntity(const QUuid& entityID, const QString& entityPropertiesString) { auto entityTreeRenderer = qApp->getEntities(); EntityTreePointer entityTree = entityTreeRenderer ? entityTreeRenderer->getTree() : nullptr; EntityItemPointer entity; if (entityTree) { + return; + } + // convert string to properties + EntityItemProperties properties; + { + // NOTE: this path from EntityItemProperties JSON string to EntityItemProperties is NOT efficient + QScriptEngine scriptEngine; + properties.copyFromJSONString(scriptEngine, entityPropertiesString); + } + + entityTree->withWriteLock([&] { entity = entityTree->findEntityByID(entityID); if (!entity) { entity = entityTree->addEntity(entityID, properties); @@ -1450,25 +1465,25 @@ void MyAvatar::updateAvatarEntity(const QUuid& entityID, const QString& entityPr } // TODO: remember this entity and its properties, so we can save to settings } else { - // TODO: propagate changes to entity - // TODO: and remember these changes so we can save to settings + entityTree->updateEntity(entityID, properties); } - } + if (entity) { + // build update packet for later + OctreePacketData packetData(false, AvatarTraits::MAXIMUM_TRAIT_SIZE); + EncodeBitstreamParams params; + EntityTreeElementExtraEncodeDataPointer extra { nullptr }; + OctreeElement::AppendState appendState = entity->appendEntityData(&packetData, params, extra); + if (appendState == OctreeElement::COMPLETED) { + QByteArray tempArray = QByteArray::fromRawData((const char*)packetData.getUncompressedData(), packetData.getUncompressedSize()); + storeAvatarEntityDataPayload(entity->getID(), tempArray); + properties = entity->getProperties(); + _entitiesToSaveToSettings.insert(entityID); + } - OctreePacketData packetData(false, AvatarTraits::MAXIMUM_TRAIT_SIZE); - EncodeBitstreamParams params; - EntityTreeElementExtraEncodeDataPointer extra { nullptr }; - OctreeElement::AppendState appendState = entity->appendEntityData(&packetData, params, extra); - - if (appendState != OctreeElement::COMPLETED) { - // this entity's data is too big - return; - } - - QByteArray tempArray = QByteArray::fromRawData((const char*)packetData.getUncompressedData(), packetData.getUncompressedSize()); - storeAvatarEntityDataPayload(entity->getID(), tempArray); - */ + } + }); } +*/ void MyAvatar::updateAvatarEntities() { // TODO: modify this info for MyAvatar diff --git a/interface/src/avatar/MyAvatar.h b/interface/src/avatar/MyAvatar.h index b05d9ed875..b100eebb73 100644 --- a/interface/src/avatar/MyAvatar.h +++ b/interface/src/avatar/MyAvatar.h @@ -1407,7 +1407,7 @@ public slots: */ bool getEnableMeshVisible() const override; - void updateAvatarEntity(const QUuid& entityID, const QString& entityPropertiesString) override; + void storeAvatarEntityDataPayload(const QUuid& entityID, const QByteArray& payload) override; /**jsdoc * Set whether or not your avatar mesh is visible. @@ -1948,6 +1948,7 @@ private: std::vector> _avatarEntityDataSettings; std::map _avatarEntityStrings; std::map _avatarEntityProperties; + std::set _entitiesToSaveToSettings; }; QScriptValue audioListenModeToScriptValue(QScriptEngine* engine, const AudioListenerMode& audioListenerMode); diff --git a/libraries/avatars/src/AvatarData.cpp b/libraries/avatars/src/AvatarData.cpp index e015410582..9380699504 100644 --- a/libraries/avatars/src/AvatarData.cpp +++ b/libraries/avatars/src/AvatarData.cpp @@ -2777,7 +2777,7 @@ void AvatarData::storeAvatarEntityDataPayload(const QUuid& entityID, const QByte } void AvatarData::updateAvatarEntity(const QUuid& entityID, const QString& entityPropertiesString) { - // TODO: implement this as API exposed to JS + // TODO: implement this to expose AvatarEntity to JS } void AvatarData::clearAvatarEntity(const QUuid& entityID, bool requiresRemovalFromTree) { diff --git a/libraries/avatars/src/AvatarData.h b/libraries/avatars/src/AvatarData.h index 1137cff869..f2f856678b 100644 --- a/libraries/avatars/src/AvatarData.h +++ b/libraries/avatars/src/AvatarData.h @@ -952,14 +952,14 @@ public: // FIXME: Can this name be improved? Can it be deprecated? Q_INVOKABLE virtual void setAttachmentsVariant(const QVariantList& variant); - void storeAvatarEntityDataPayload(const QUuid& entityID, const QByteArray& payload); + virtual void storeAvatarEntityDataPayload(const QUuid& entityID, const QByteArray& payload); /**jsdoc * @function MyAvatar.updateAvatarEntity * @param {Uuid} entityID * @param {string} entityData */ - Q_INVOKABLE void updateAvatarEntity(const QUuid& entityID, const QString& entityPropertiesString); + Q_INVOKABLE virtual void updateAvatarEntity(const QUuid& entityID, const QString& entityPropertiesString); /**jsdoc * @function MyAvatar.clearAvatarEntity diff --git a/libraries/entities/src/EntityEditPacketSender.cpp b/libraries/entities/src/EntityEditPacketSender.cpp index dfc2d45d36..40dd26a467 100644 --- a/libraries/entities/src/EntityEditPacketSender.cpp +++ b/libraries/entities/src/EntityEditPacketSender.cpp @@ -69,8 +69,8 @@ void EntityEditPacketSender::queueEditAvatarEntityMessage(EntityTreePointer enti return; } - packetData.shrinkByteArrays(); - _myAvatar->storeAvatarEntityDataPayload(entity->getID(), packetData.getUncompressedByteArray()); + QByteArray tempArray = QByteArray::fromRawData((const char*)packetData.getUncompressedData(), packetData.getUncompressedSize()); + _myAvatar->storeAvatarEntityDataPayload(entityItemID, tempArray); } void EntityEditPacketSender::queueEditEntityMessage(PacketType type, diff --git a/libraries/octree/src/OctreePacketData.cpp b/libraries/octree/src/OctreePacketData.cpp index 60b0e6fbad..fd57f2fa3a 100644 --- a/libraries/octree/src/OctreePacketData.cpp +++ b/libraries/octree/src/OctreePacketData.cpp @@ -654,15 +654,6 @@ void OctreePacketData::loadFinalizedContent(const unsigned char* data, int lengt } } -void OctreePacketData::shrinkByteArrays() { - _uncompressedByteArray.resize(_bytesInUse); - _compressedByteArray.resize(_compressedBytes); - // if you call this method then you are expected to be done packing to raw pointers - // and you just want the ByteArrays - // therefore we reset - reset(); -} - void OctreePacketData::debugContent() { qCDebug(octree, "OctreePacketData::debugContent()... COMPRESSED DATA.... size=%d",_compressedBytes); int perline=0; diff --git a/libraries/octree/src/OctreePacketData.h b/libraries/octree/src/OctreePacketData.h index cca78e19b3..01ed4977b0 100644 --- a/libraries/octree/src/OctreePacketData.h +++ b/libraries/octree/src/OctreePacketData.h @@ -245,8 +245,6 @@ public: int getBytesAvailable() { return _bytesAvailable; } - void shrinkByteArrays(); - /// displays contents for debugging void debugContent(); void debugBytes(); From 61b8d005b51362d2864f1d87cb7b0f8cd17a1d03 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Fri, 7 Dec 2018 16:39:07 -0800 Subject: [PATCH 12/43] scripts can edit AvatarEntities again --- interface/src/avatar/MyAvatar.cpp | 153 +++++++++--------- interface/src/avatar/MyAvatar.h | 2 + .../src/avatars-renderer/Avatar.cpp | 12 +- libraries/avatars/src/AvatarData.h | 2 +- 4 files changed, 82 insertions(+), 87 deletions(-) diff --git a/interface/src/avatar/MyAvatar.cpp b/interface/src/avatar/MyAvatar.cpp index 04f1cb599e..9ece7a0ac7 100755 --- a/interface/src/avatar/MyAvatar.cpp +++ b/interface/src/avatar/MyAvatar.cpp @@ -1303,25 +1303,74 @@ void MyAvatar::saveData() { } void MyAvatar::saveAvatarEntityDataToSettings() { - if (!_entitiesToSaveToSettings.empty()) { - // TODO: save these to settings. - _entitiesToSaveToSettings.clear(); + if (_entitiesToSaveToSettings.size() + _entitiesToRemoveFromSettings.size() == 0) { + // nothing to do + return; } - // save new settings - uint32_t numEntities = _avatarEntityStrings.size(); - _avatarEntityCountSetting.set(numEntities); + auto entityTreeRenderer = qApp->getEntities(); + EntityTreePointer entityTree = entityTreeRenderer ? entityTreeRenderer->getTree() : nullptr; + if (!entityTree) { + return; + } + + // find set of things that changed + std::set entitiesToSave; + _avatarEntitiesLock.withWriteLock([&] { + // TODO: save these to settings. + entitiesToSave = std::move(_entitiesToSaveToSettings); + for (const auto& id : _entitiesToRemoveFromSettings) { + // remove + entitiesToSave.erase(id); + std::map::iterator itr = _avatarEntityStrings.find(id); + if (itr != _avatarEntityStrings.end()) { + _avatarEntityStrings.erase(itr); + } + } + for (const auto& id : entitiesToSave) { + // remove old strings to be replaced by new saves + std::map::iterator itr = _avatarEntityStrings.find(id); + if (itr != _avatarEntityStrings.end()) { + _avatarEntityStrings.erase(itr); + } + } + _entitiesToRemoveFromSettings.clear(); + }); + + uint32_t numEntities = entitiesToSave.size() + _avatarEntityStrings.size(); uint32_t prevNumEntities = _avatarEntityCountSetting.get(0); resizeAvatarEntitySettingHandles(std::max(numEntities, prevNumEntities)); + + // save new settings if (numEntities > 0) { - _avatarEntitiesLock.withReadLock([&] { - uint32_t i = 0; - for (const auto& mapEntry : _avatarEntityStrings) { - _avatarEntityDataSettings[i].set(mapEntry.second); - _avatarEntityIDSettings[i].set(mapEntry.first.toString()); - ++i; + // get all properties to save + std::map allProperties; + EntityItemPointer entity; + entityTree->withWriteLock([&] { + for (auto& id : entitiesToSave) { + EntityItemPointer entity = entityTree->findEntityByID(id); + if (entity) { + allProperties[id] = entity->getProperties(); + } } }); + // convert properties to strings + QScriptEngine scriptEngine; + QScriptValue toStringMethod = scriptEngine.evaluate("(function() { return JSON.stringify(this) })"); + for (const auto& entry : allProperties) { + QScriptValue scriptValue = EntityItemPropertiesToScriptValue(&scriptEngine, entry.second); + scriptValue.setProperty("toString", toStringMethod); + _avatarEntityStrings[entry.first] = scriptValue.toString(); + } + // save all strings + uint32_t i = 0; + for (const auto& entry : _avatarEntityStrings) { + _avatarEntityIDSettings[i].set(entry.first.toString()); + _avatarEntityDataSettings[i].set(entry.second); + ++i; + } + numEntities = i; } + _avatarEntityCountSetting.set(numEntities); // remove old settings if any if (numEntities < prevNumEntities) { @@ -1435,91 +1484,32 @@ void MyAvatar::setEnableInverseKinematics(bool isEnabled) { void MyAvatar::storeAvatarEntityDataPayload(const QUuid& entityID, const QByteArray& payload) { AvatarData::storeAvatarEntityDataPayload(entityID, payload); - _entitiesToSaveToSettings.insert(entityID); -} - -/* TODO: verify we don't need this -void MyAvatar::updateAvatarEntity(const QUuid& entityID, const QString& entityPropertiesString) { - auto entityTreeRenderer = qApp->getEntities(); - EntityTreePointer entityTree = entityTreeRenderer ? entityTreeRenderer->getTree() : nullptr; - EntityItemPointer entity; - if (entityTree) { - return; - } - // convert string to properties - EntityItemProperties properties; - { - // NOTE: this path from EntityItemProperties JSON string to EntityItemProperties is NOT efficient - QScriptEngine scriptEngine; - properties.copyFromJSONString(scriptEngine, entityPropertiesString); - } - - entityTree->withWriteLock([&] { - entity = entityTree->findEntityByID(entityID); - if (!entity) { - entity = entityTree->addEntity(entityID, properties); - if (!entity) { - // unable to create entity - // TODO? handle this case? - return; - } - // TODO: remember this entity and its properties, so we can save to settings - } else { - entityTree->updateEntity(entityID, properties); - } - if (entity) { - // build update packet for later - OctreePacketData packetData(false, AvatarTraits::MAXIMUM_TRAIT_SIZE); - EncodeBitstreamParams params; - EntityTreeElementExtraEncodeDataPointer extra { nullptr }; - OctreeElement::AppendState appendState = entity->appendEntityData(&packetData, params, extra); - if (appendState == OctreeElement::COMPLETED) { - QByteArray tempArray = QByteArray::fromRawData((const char*)packetData.getUncompressedData(), packetData.getUncompressedSize()); - storeAvatarEntityDataPayload(entity->getID(), tempArray); - properties = entity->getProperties(); - _entitiesToSaveToSettings.insert(entityID); - } - - } + _avatarEntitiesLock.withWriteLock([&] { + _entitiesToSaveToSettings.insert(entityID); }); } -*/ + +void MyAvatar::clearAvatarEntity(const QUuid& entityID, bool requiresRemovalFromTree) { + _avatarEntitiesLock.withWriteLock([&] { + _entitiesToRemoveFromSettings.insert(entityID); + }); + AvatarData::clearAvatarEntity(entityID, requiresRemovalFromTree); +} void MyAvatar::updateAvatarEntities() { - // TODO: modify this info for MyAvatar - // AVATAR ENTITY UPDATE FLOW - // - if queueEditEntityMessage sees clientOnly flag it does _myAvatar->updateAvatarEntity() - // - updateAvatarEntity saves the bytes and flags the trait instance for the entity as updated - // - ClientTraitsHandler::sendChangedTraitsToMixer sends the entity bytes to the mixer which relays them to other interfaces - // - AvatarHashMap::processBulkAvatarTraits on other interfaces calls avatar->processTraitInstace - // - AvatarData::processTraitInstance calls updateAvatarEntity, which sets _avatarEntityDataChanged = true - // - (My)Avatar::simulate calls updateAvatarEntities every frame and here we are... - - // AVATAR ENTITY DELETE FLOW - // - EntityScriptingInterface::deleteEntity calls _myAvatar->clearAvatarEntity() for deleted avatar entities - // - clearAvatarEntity removes the avatar entity and flags the trait instance for the entity as deleted - // - ClientTraitsHandler::sendChangedTraitsToMixer sends a deletion to the mixer which relays to other interfaces - // - AvatarHashMap::processBulkAvatarTraits on other interfaces calls avatar->processDeletedTraitInstace - // - AvatarData::processDeletedTraitInstance calls clearAvatarEntity - // - AvatarData::clearAvatarEntity sets _avatarEntityDataChanged = true and adds the ID to the detached list - // - Avatar::simulate calls updateAvatarEntities every frame and here we are... - if (_reloadAvatarEntityDataFromSettings) { - if (getID().isNull() || getID() == AVATAR_SELF_ID || DependencyManager::get()->getSessionUUID() == QUuid()) { - // wait until MyAvatar and this Node gets an ID before doing this. Otherwise, various things go wrong -- + // wait until MyAvatar and this Node gets an ID before doing this. Otherwise, various things go wrong: // things get their parent fixed up from AVATAR_SELF_ID to a null uuid which means "no parent". return; } - auto treeRenderer = DependencyManager::get(); EntityTreePointer entityTree = treeRenderer ? treeRenderer->getTree() : nullptr; if (!entityTree) { return; } - loadAvatarEntityDataFromSettings(); } } @@ -1580,6 +1570,7 @@ void MyAvatar::loadAvatarEntityDataFromSettings() { properties.copyFromJSONString(scriptEngine, _avatarEntityDataSettings[i].get()); // the ClientOnly property can get stripped out elsewhere so we need to always set it true here properties.setClientOnly(true); + properties.setOwningAvatarID(getID()); } _avatarEntityStrings.clear(); diff --git a/interface/src/avatar/MyAvatar.h b/interface/src/avatar/MyAvatar.h index b100eebb73..b03c41fe9a 100644 --- a/interface/src/avatar/MyAvatar.h +++ b/interface/src/avatar/MyAvatar.h @@ -1408,6 +1408,7 @@ public slots: bool getEnableMeshVisible() const override; void storeAvatarEntityDataPayload(const QUuid& entityID, const QByteArray& payload) override; + void clearAvatarEntity(const QUuid& entityID, bool requiresRemovalFromTree = true) override; /**jsdoc * Set whether or not your avatar mesh is visible. @@ -1949,6 +1950,7 @@ private: std::map _avatarEntityStrings; std::map _avatarEntityProperties; std::set _entitiesToSaveToSettings; + std::set _entitiesToRemoveFromSettings; }; QScriptValue audioListenModeToScriptValue(QScriptEngine* engine, const AudioListenerMode& audioListenerMode); diff --git a/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp b/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp index 722c3c8a5e..0555f60742 100644 --- a/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp +++ b/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp @@ -312,12 +312,13 @@ void Avatar::updateAvatarEntities() { PerformanceTimer perfTimer("attachments"); // AVATAR ENTITY UPDATE FLOW - // - if queueEditEntityMessage sees avatarEntity flag it does _myAvatar->updateAvatarEntity() - // - updateAvatarEntity saves the bytes and flags the trait instance for the entity as updated + // - if queueEditEntityMessage sees clientOnly flag it calls _myAvatar->storeAvatarEntityDataPayload + // - storeAvatarEntityDataPayload saves the payload and flags the trait instance for the entity as updated, // - ClientTraitsHandler::sendChangedTraitsToMixer sends the entity bytes to the mixer which relays them to other interfaces // - AvatarHashMap::processBulkAvatarTraits on other interfaces calls avatar->processTraitInstace - // - AvatarData::processTraitInstance calls updateAvatarEntity, which sets _avatarEntityDataChanged = true - // - (My)Avatar::simulate notices _avatarEntityDataChanged and here we are... + // - AvatarData::processTraitInstance calls storeAvatarEntityDataPayload, which sets _avatarEntityDataChanged = true + // - (My)Avatar::simulate calls updateAvatarEntities every frame which checks _avatarEntityDataChanged + // and here we are... // AVATAR ENTITY DELETE FLOW // - EntityScriptingInterface::deleteEntity calls _myAvatar->clearAvatarEntity() for deleted avatar entities @@ -326,7 +327,8 @@ void Avatar::updateAvatarEntities() { // - AvatarHashMap::processBulkAvatarTraits on other interfaces calls avatar->processDeletedTraitInstace // - AvatarData::processDeletedTraitInstance calls clearAvatarEntity // - AvatarData::clearAvatarEntity sets _avatarEntityDataChanged = true and adds the ID to the detached list - // - Avatar::simulate notices _avatarEntityDataChanged and here we are... + // - (My)Avatar::simulate calls updateAvatarEntities every frame which checks _avatarEntityDataChanged + // and here we are... if (!_avatarEntityDataChanged) { return; diff --git a/libraries/avatars/src/AvatarData.h b/libraries/avatars/src/AvatarData.h index f2f856678b..65ea873502 100644 --- a/libraries/avatars/src/AvatarData.h +++ b/libraries/avatars/src/AvatarData.h @@ -965,7 +965,7 @@ public: * @function MyAvatar.clearAvatarEntity * @param {Uuid} entityID */ - Q_INVOKABLE void clearAvatarEntity(const QUuid& entityID, bool requiresRemovalFromTree = true); + Q_INVOKABLE virtual void clearAvatarEntity(const QUuid& entityID, bool requiresRemovalFromTree = true); /**jsdoc From ce660bee9b0df8332595a7619f1b3c0f119ceb47 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Fri, 7 Dec 2018 17:31:50 -0800 Subject: [PATCH 13/43] fix compile errors after rebase --- interface/src/avatar/MyAvatar.cpp | 2 +- libraries/entities/src/EntityItem.cpp | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/interface/src/avatar/MyAvatar.cpp b/interface/src/avatar/MyAvatar.cpp index 9ece7a0ac7..eeca8ec4f4 100755 --- a/interface/src/avatar/MyAvatar.cpp +++ b/interface/src/avatar/MyAvatar.cpp @@ -1569,7 +1569,7 @@ void MyAvatar::loadAvatarEntityDataFromSettings() { EntityItemProperties& properties = entitiesToLoad[i]; properties.copyFromJSONString(scriptEngine, _avatarEntityDataSettings[i].get()); // the ClientOnly property can get stripped out elsewhere so we need to always set it true here - properties.setClientOnly(true); + properties.setEntityHostType(entity::HostType::AVATAR); properties.setOwningAvatarID(getID()); } diff --git a/libraries/entities/src/EntityItem.cpp b/libraries/entities/src/EntityItem.cpp index f402aae7d0..b766485450 100644 --- a/libraries/entities/src/EntityItem.cpp +++ b/libraries/entities/src/EntityItem.cpp @@ -335,9 +335,9 @@ OctreeElement::AppendState EntityItem::appendEntityData(OctreePacketData* packet APPEND_ENTITY_PROPERTY(PROP_DESCRIPTION, getDescription()); APPEND_ENTITY_PROPERTY(PROP_ACTION_DATA, getDynamicData()); + // convert AVATAR_SELF_ID to actual sessionUUID. QUuid actualParentID = getParentID(); - if (!getClientOnly() && actualParentID == AVATAR_SELF_ID) { - // convert AVATAR_SELF_ID to actual sessionUUID. + if (actualParentID == AVATAR_SELF_ID) { auto nodeList = DependencyManager::get(); actualParentID = nodeList->getSessionUUID(); } From 6da8c5545e3d81f27e241faac4ac89081c899ffc Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Fri, 7 Dec 2018 18:01:13 -0800 Subject: [PATCH 14/43] remove debug code --- interface/src/avatar/MyAvatar.h | 1 - 1 file changed, 1 deletion(-) diff --git a/interface/src/avatar/MyAvatar.h b/interface/src/avatar/MyAvatar.h index b03c41fe9a..17edfc1a3a 100644 --- a/interface/src/avatar/MyAvatar.h +++ b/interface/src/avatar/MyAvatar.h @@ -13,7 +13,6 @@ #define hifi_MyAvatar_h #include -#include // adebug #include From 1815d711580d832a74f415f6f3559ed3db7bd39b Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Sat, 8 Dec 2018 09:13:13 -0800 Subject: [PATCH 15/43] fix data corruption bug --- interface/src/avatar/MyAvatar.cpp | 2 +- libraries/entities/src/EntityEditPacketSender.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/interface/src/avatar/MyAvatar.cpp b/interface/src/avatar/MyAvatar.cpp index eeca8ec4f4..af8df7fb71 100755 --- a/interface/src/avatar/MyAvatar.cpp +++ b/interface/src/avatar/MyAvatar.cpp @@ -1590,7 +1590,7 @@ void MyAvatar::loadAvatarEntityDataFromSettings() { OctreeElement::AppendState appendState = entity->appendEntityData(&packetData, params, extra); if (appendState == OctreeElement::COMPLETED) { // only remember an AvatarEntity that successfully loads and can be packed - QByteArray tempArray = QByteArray::fromRawData((const char*)packetData.getUncompressedData(), packetData.getUncompressedSize()); + QByteArray tempArray((const char*)packetData.getUncompressedData(), packetData.getUncompressedSize()); storeAvatarEntityDataPayload(entityID, tempArray); _avatarEntityStrings[entityID] = _avatarEntityDataSettings[i].get(); } diff --git a/libraries/entities/src/EntityEditPacketSender.cpp b/libraries/entities/src/EntityEditPacketSender.cpp index 40dd26a467..eca431b565 100644 --- a/libraries/entities/src/EntityEditPacketSender.cpp +++ b/libraries/entities/src/EntityEditPacketSender.cpp @@ -69,7 +69,7 @@ void EntityEditPacketSender::queueEditAvatarEntityMessage(EntityTreePointer enti return; } - QByteArray tempArray = QByteArray::fromRawData((const char*)packetData.getUncompressedData(), packetData.getUncompressedSize()); + QByteArray tempArray((const char*)packetData.getUncompressedData(), packetData.getUncompressedSize()); _myAvatar->storeAvatarEntityDataPayload(entityItemID, tempArray); } From 0083c7728065d57208198eb516f2504e716e006c Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Sat, 8 Dec 2018 09:58:39 -0800 Subject: [PATCH 16/43] add EntityTypes::extractEntityTypeAndID(), remove one spaghetti --- .../entities/src/EntityItemProperties.cpp | 2 +- libraries/entities/src/EntityTree.cpp | 2 +- libraries/entities/src/EntityTypes.cpp | 33 ++++++++++--------- libraries/entities/src/EntityTypes.h | 3 +- 4 files changed, 22 insertions(+), 18 deletions(-) diff --git a/libraries/entities/src/EntityItemProperties.cpp b/libraries/entities/src/EntityItemProperties.cpp index 6ba4c84985..2c8548033f 100644 --- a/libraries/entities/src/EntityItemProperties.cpp +++ b/libraries/entities/src/EntityItemProperties.cpp @@ -93,7 +93,7 @@ void EntityItemProperties::setLastEdited(quint64 usecTime) { bool EntityItemProperties::constructFromBuffer(const unsigned char* data, int dataLength) { ReadBitstreamToTreeParams args; - EntityItemPointer tempEntity = EntityTypes::constructEntityItem(data, dataLength, args); + EntityItemPointer tempEntity = EntityTypes::constructEntityItem(data, dataLength); if (!tempEntity) { return false; } diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index 3f0b5249ec..542e18fa6a 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -174,7 +174,7 @@ int EntityTree::readEntityDataFromBuffer(const unsigned char* data, int bytesLef addToNeedsParentFixupList(entity); } } else { - entity = EntityTypes::constructEntityItem(dataAt, bytesLeftToRead, args); + entity = EntityTypes::constructEntityItem(dataAt, bytesLeftToRead); if (entity) { bytesForThisEntity = entity->readEntityDataFromBuffer(dataAt, bytesLeftToRead, args); diff --git a/libraries/entities/src/EntityTypes.cpp b/libraries/entities/src/EntityTypes.cpp index e511af83b0..1544adca94 100644 --- a/libraries/entities/src/EntityTypes.cpp +++ b/libraries/entities/src/EntityTypes.cpp @@ -107,8 +107,7 @@ EntityItemPointer EntityTypes::constructEntityItem(EntityType entityType, const return newEntityItem; } -EntityItemPointer EntityTypes::constructEntityItem(const unsigned char* data, int bytesToRead, - ReadBitstreamToTreeParams& args) { +void EntityTypes::extractEntityTypeAndID(const unsigned char* data, int dataLength, EntityTypes::EntityType& typeOut, QUuid& idOut) { // Header bytes // object ID [16 bytes] @@ -119,28 +118,32 @@ EntityItemPointer EntityTypes::constructEntityItem(const unsigned char* data, in // ~27-35 bytes... const int MINIMUM_HEADER_BYTES = 27; - int bytesRead = 0; - if (bytesToRead >= MINIMUM_HEADER_BYTES) { - int originalLength = bytesToRead; - QByteArray originalDataBuffer((const char*)data, originalLength); + if (dataLength >= MINIMUM_HEADER_BYTES) { + int bytesRead = 0; + QByteArray originalDataBuffer = QByteArray::fromRawData((const char*)data, dataLength); // id QByteArray encodedID = originalDataBuffer.mid(bytesRead, NUM_BYTES_RFC4122_UUID); // maximum possible size - QUuid actualID = QUuid::fromRfc4122(encodedID); + idOut = QUuid::fromRfc4122(encodedID); bytesRead += encodedID.size(); // type QByteArray encodedType = originalDataBuffer.mid(bytesRead); // maximum possible size ByteCountCoded typeCoder = encodedType; encodedType = typeCoder; // determine true length - bytesRead += encodedType.size(); quint32 type = typeCoder; - EntityTypes::EntityType entityType = (EntityTypes::EntityType)type; - - EntityItemID tempEntityID(actualID); - EntityItemProperties tempProperties; - return constructEntityItem(entityType, tempEntityID, tempProperties); + typeOut = (EntityTypes::EntityType)type; } - - return NULL; +} + +EntityItemPointer EntityTypes::constructEntityItem(const unsigned char* data, int bytesToRead) { + QUuid id; + EntityTypes::EntityType type = EntityTypes::Unknown; + extractEntityTypeAndID(data, bytesToRead, type, id); + if (type > EntityTypes::Unknown && type <= EntityTypes::LAST) { + EntityItemID tempEntityID(id); + EntityItemProperties tempProperties; + return constructEntityItem(type, tempEntityID, tempProperties); + } + return nullptr; } diff --git a/libraries/entities/src/EntityTypes.h b/libraries/entities/src/EntityTypes.h index 29a695718e..dfb7779154 100644 --- a/libraries/entities/src/EntityTypes.h +++ b/libraries/entities/src/EntityTypes.h @@ -112,8 +112,9 @@ public: static const QString& getEntityTypeName(EntityType entityType); static EntityTypes::EntityType getEntityTypeFromName(const QString& name); static bool registerEntityType(EntityType entityType, const char* name, EntityTypeFactory factoryMethod); + static void extractEntityTypeAndID(const unsigned char* data, int dataLength, EntityTypes::EntityType& typeOut, QUuid& idOut); static EntityItemPointer constructEntityItem(EntityType entityType, const EntityItemID& entityID, const EntityItemProperties& properties); - static EntityItemPointer constructEntityItem(const unsigned char* data, int bytesToRead, ReadBitstreamToTreeParams& args); + static EntityItemPointer constructEntityItem(const unsigned char* data, int bytesToRead); private: static QMap _typeToNameMap; From 09f3b8f4857f65c8f434df86333e38f940ce77aa Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Sat, 8 Dec 2018 10:36:35 -0800 Subject: [PATCH 17/43] add bit of sanity checking for incomming AvatarEntities data --- .../avatars-renderer/src/avatars-renderer/Avatar.cpp | 9 +++++++++ libraries/entities/src/EntityTypes.cpp | 4 ++++ libraries/entities/src/EntityTypes.h | 1 + 3 files changed, 14 insertions(+) diff --git a/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp b/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp index 0555f60742..f583b41172 100644 --- a/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp +++ b/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp @@ -367,6 +367,15 @@ void Avatar::updateAvatarEntities() { // TODO? put a maximum number of tries on this? } } else { + // sanity check data + QUuid id; + EntityTypes::EntityType type; + EntityTypes::extractEntityTypeAndID((unsigned char*)(data.data()), data.size(), type, id); + if (id != entityID || !EntityTypes::typeIsValid(type)) { + // skip for corrupt + ++dataItr; + continue; + } // remember this hash for the future stateItr = _avatarEntityDataHashes.insert(entityID, AvatarEntityDataHash(newHash)); } diff --git a/libraries/entities/src/EntityTypes.cpp b/libraries/entities/src/EntityTypes.cpp index 1544adca94..6f05237b1e 100644 --- a/libraries/entities/src/EntityTypes.cpp +++ b/libraries/entities/src/EntityTypes.cpp @@ -58,6 +58,10 @@ REGISTER_ENTITY_TYPE(Light) REGISTER_ENTITY_TYPE(Zone) REGISTER_ENTITY_TYPE(Material) +bool EntityTypes::typeIsValid(EntityType type) { + return type > EntityType::Unknown && type <= EntityType::LAST; +} + const QString& EntityTypes::getEntityTypeName(EntityType entityType) { QMap::iterator matchedTypeName = _typeToNameMap.find(entityType); if (matchedTypeName != _typeToNameMap.end()) { diff --git a/libraries/entities/src/EntityTypes.h b/libraries/entities/src/EntityTypes.h index dfb7779154..9f4ba36df1 100644 --- a/libraries/entities/src/EntityTypes.h +++ b/libraries/entities/src/EntityTypes.h @@ -109,6 +109,7 @@ public: NUM_TYPES } EntityType; + static bool typeIsValid(EntityType type); static const QString& getEntityTypeName(EntityType entityType); static EntityTypes::EntityType getEntityTypeFromName(const QString& name); static bool registerEntityType(EntityType entityType, const char* name, EntityTypeFactory factoryMethod); From c4115bece381a2256866227702b8f8480146f082 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Mon, 10 Dec 2018 08:26:43 -0800 Subject: [PATCH 18/43] remove unused ScriptEngine instance --- libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp b/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp index f583b41172..c6f315fc59 100644 --- a/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp +++ b/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp @@ -340,7 +340,6 @@ void Avatar::updateAvatarEntities() { return; } - QScriptEngine scriptEngine; entityTree->withWriteLock([&] { AvatarEntityMap avatarEntities = getAvatarEntityData(); AvatarEntityMap::const_iterator dataItr = avatarEntities.begin(); From 121a5910238ad7a02f100a81892946d0a29e8f07 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Mon, 10 Dec 2018 11:52:56 -0800 Subject: [PATCH 19/43] delete old AvatarEntities on domain reset on login --- interface/src/Application.cpp | 2 +- interface/src/avatar/MyAvatar.cpp | 6 ++++++ interface/src/avatar/MyAvatar.h | 2 +- 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 0fbe9fba94..e0128d0ceb 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -6784,7 +6784,7 @@ void Application::clearDomainOctreeDetails() { DependencyManager::get()->clearUnusedResources(); // we just deleted all of MyAvatar's AvatarEntities so we flag it to reload from settings - getMyAvatar()->rememberToReloadOfAvatarEntityDataFromSettings(); + getMyAvatar()->rememberToReloadAvatarEntityDataFromSettings(); } void Application::domainURLChanged(QUrl domainURL) { diff --git a/interface/src/avatar/MyAvatar.cpp b/interface/src/avatar/MyAvatar.cpp index af8df7fb71..58b4d5e7d9 100755 --- a/interface/src/avatar/MyAvatar.cpp +++ b/interface/src/avatar/MyAvatar.cpp @@ -1515,6 +1515,12 @@ void MyAvatar::updateAvatarEntities() { } +void MyAvatar::rememberToReloadAvatarEntityDataFromSettings() { + AvatarEntityMap emptyMap; + setAvatarEntityData(emptyMap); + _reloadAvatarEntityDataFromSettings = true; +} + void MyAvatar::loadData() { getHead()->setBasePitch(_headPitchSetting.get()); diff --git a/interface/src/avatar/MyAvatar.h b/interface/src/avatar/MyAvatar.h index 17edfc1a3a..21517c5af9 100644 --- a/interface/src/avatar/MyAvatar.h +++ b/interface/src/avatar/MyAvatar.h @@ -1187,7 +1187,7 @@ public: glm::vec3 getNextPosition() { return _goToPending ? _goToPosition : getWorldPosition(); } void updateAvatarEntities() override; - void rememberToReloadOfAvatarEntityDataFromSettings() { _reloadAvatarEntityDataFromSettings = true; } + void rememberToReloadAvatarEntityDataFromSettings(); /**jsdoc * Create a new grab. From 320ba9177c9a846a56b22f10514bf0f75fd5d8eb Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Mon, 10 Dec 2018 13:29:13 -0800 Subject: [PATCH 20/43] remove cruft --- interface/src/avatar/MyAvatar.cpp | 1 - libraries/octree/src/OctreePacketData.h | 2 -- 2 files changed, 3 deletions(-) diff --git a/interface/src/avatar/MyAvatar.cpp b/interface/src/avatar/MyAvatar.cpp index 58b4d5e7d9..9a0787151d 100755 --- a/interface/src/avatar/MyAvatar.cpp +++ b/interface/src/avatar/MyAvatar.cpp @@ -1316,7 +1316,6 @@ void MyAvatar::saveAvatarEntityDataToSettings() { // find set of things that changed std::set entitiesToSave; _avatarEntitiesLock.withWriteLock([&] { - // TODO: save these to settings. entitiesToSave = std::move(_entitiesToSaveToSettings); for (const auto& id : _entitiesToRemoveFromSettings) { // remove diff --git a/libraries/octree/src/OctreePacketData.h b/libraries/octree/src/OctreePacketData.h index 01ed4977b0..bd1abf8744 100644 --- a/libraries/octree/src/OctreePacketData.h +++ b/libraries/octree/src/OctreePacketData.h @@ -220,8 +220,6 @@ public: /// get pointer to the uncompressed stream buffer at the byteOffset const unsigned char* getUncompressedData(int byteOffset = 0) { return &_uncompressed[byteOffset]; } - const QByteArray& getUncompressedByteArray() { return _uncompressedByteArray; } - /// the size of the packet in uncompressed form int getUncompressedSize() { return _bytesInUse; } From d622635a710c3548c1253a1d945c164ec4f12e28 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Mon, 10 Dec 2018 13:40:38 -0800 Subject: [PATCH 21/43] fix wording in comment to reflect new 'HostType' parameter --- libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp b/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp index c6f315fc59..59d9f13f04 100644 --- a/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp +++ b/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp @@ -312,7 +312,7 @@ void Avatar::updateAvatarEntities() { PerformanceTimer perfTimer("attachments"); // AVATAR ENTITY UPDATE FLOW - // - if queueEditEntityMessage sees clientOnly flag it calls _myAvatar->storeAvatarEntityDataPayload + // - if queueEditEntityMessage sees "AvatarEntity" HostType it calls _myAvatar->storeAvatarEntityDataPayload // - storeAvatarEntityDataPayload saves the payload and flags the trait instance for the entity as updated, // - ClientTraitsHandler::sendChangedTraitsToMixer sends the entity bytes to the mixer which relays them to other interfaces // - AvatarHashMap::processBulkAvatarTraits on other interfaces calls avatar->processTraitInstace From dedc14434b47db64b2a8a147c8847934a6596de9 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Mon, 10 Dec 2018 15:41:25 -0800 Subject: [PATCH 22/43] fix compile warnings on windows --- interface/src/avatar/MyAvatar.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/interface/src/avatar/MyAvatar.cpp b/interface/src/avatar/MyAvatar.cpp index 9a0787151d..26296355a6 100755 --- a/interface/src/avatar/MyAvatar.cpp +++ b/interface/src/avatar/MyAvatar.cpp @@ -1261,7 +1261,7 @@ void MyAvatar::resizeAvatarEntitySettingHandles(uint32_t maxIndex) { // Avatar/avatarEntityData/5/properties: ... // // Create Setting::Handles to mimic this. - uint32_t settingsIndex = _avatarEntityIDSettings.size() + 1; + uint32_t settingsIndex = (uint32_t)_avatarEntityIDSettings.size() + 1; while (settingsIndex <= maxIndex) { Setting::Handle idHandle(QStringList() << AVATAR_SETTINGS_GROUP_NAME << "avatarEntityData" << QString::number(settingsIndex) << "id", QUuid().toString()); @@ -1335,7 +1335,7 @@ void MyAvatar::saveAvatarEntityDataToSettings() { _entitiesToRemoveFromSettings.clear(); }); - uint32_t numEntities = entitiesToSave.size() + _avatarEntityStrings.size(); + uint32_t numEntities = (uint32_t)(entitiesToSave.size() + _avatarEntityStrings.size()); uint32_t prevNumEntities = _avatarEntityCountSetting.get(0); resizeAvatarEntitySettingHandles(std::max(numEntities, prevNumEntities)); From 43fc86fe54b84731497757b96d8cd0cacbca5b35 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Tue, 11 Dec 2018 09:07:58 -0800 Subject: [PATCH 23/43] restore non-human-readable avatarEntityDataSettings format --- interface/src/avatar/MyAvatar.cpp | 98 +++++++++++++++++++++++-------- interface/src/avatar/MyAvatar.h | 7 +-- 2 files changed, 78 insertions(+), 27 deletions(-) diff --git a/interface/src/avatar/MyAvatar.cpp b/interface/src/avatar/MyAvatar.cpp index 26296355a6..f808928709 100755 --- a/interface/src/avatar/MyAvatar.cpp +++ b/interface/src/avatar/MyAvatar.cpp @@ -50,6 +50,7 @@ #include #include #include +#include #include "MyHead.h" #include "MySkeletonModel.h" @@ -1263,11 +1264,11 @@ void MyAvatar::resizeAvatarEntitySettingHandles(uint32_t maxIndex) { // Create Setting::Handles to mimic this. uint32_t settingsIndex = (uint32_t)_avatarEntityIDSettings.size() + 1; while (settingsIndex <= maxIndex) { - Setting::Handle idHandle(QStringList() << AVATAR_SETTINGS_GROUP_NAME << "avatarEntityData" - << QString::number(settingsIndex) << "id", QUuid().toString()); + Setting::Handle idHandle(QStringList() << AVATAR_SETTINGS_GROUP_NAME << "avatarEntityData" + << QString::number(settingsIndex) << "id", QUuid()); _avatarEntityIDSettings.push_back(idHandle); - Setting::Handle dataHandle(QStringList() << AVATAR_SETTINGS_GROUP_NAME << "avatarEntityData" - << QString::number(settingsIndex) << "properties", QString()); + Setting::Handle dataHandle(QStringList() << AVATAR_SETTINGS_GROUP_NAME << "avatarEntityData" + << QString::number(settingsIndex) << "properties", QByteArray()); _avatarEntityDataSettings.push_back(dataHandle); settingsIndex++; } @@ -1320,22 +1321,22 @@ void MyAvatar::saveAvatarEntityDataToSettings() { for (const auto& id : _entitiesToRemoveFromSettings) { // remove entitiesToSave.erase(id); - std::map::iterator itr = _avatarEntityStrings.find(id); - if (itr != _avatarEntityStrings.end()) { - _avatarEntityStrings.erase(itr); + std::map::iterator itr = _poorlyFormattedAvatarEntityData.find(id); + if (itr != _poorlyFormattedAvatarEntityData.end()) { + _poorlyFormattedAvatarEntityData.erase(itr); } } for (const auto& id : entitiesToSave) { // remove old strings to be replaced by new saves - std::map::iterator itr = _avatarEntityStrings.find(id); - if (itr != _avatarEntityStrings.end()) { - _avatarEntityStrings.erase(itr); + std::map::iterator itr = _poorlyFormattedAvatarEntityData.find(id); + if (itr != _poorlyFormattedAvatarEntityData.end()) { + _poorlyFormattedAvatarEntityData.erase(itr); } } _entitiesToRemoveFromSettings.clear(); }); - uint32_t numEntities = (uint32_t)(entitiesToSave.size() + _avatarEntityStrings.size()); + uint32_t numEntities = (uint32_t)(entitiesToSave.size() + _poorlyFormattedAvatarEntityData.size()); uint32_t prevNumEntities = _avatarEntityCountSetting.get(0); resizeAvatarEntitySettingHandles(std::max(numEntities, prevNumEntities)); @@ -1356,14 +1357,27 @@ void MyAvatar::saveAvatarEntityDataToSettings() { QScriptEngine scriptEngine; QScriptValue toStringMethod = scriptEngine.evaluate("(function() { return JSON.stringify(this) })"); for (const auto& entry : allProperties) { - QScriptValue scriptValue = EntityItemPropertiesToScriptValue(&scriptEngine, entry.second); - scriptValue.setProperty("toString", toStringMethod); - _avatarEntityStrings[entry.first] = scriptValue.toString(); + // begin recipe our unfortunate legacy avatarEntityData format + QScriptValue scriptValue = EntityItemNonDefaultPropertiesToScriptValue(&scriptEngine, entry.second); + QVariant variantProperties = scriptValue.toVariant(); + QJsonDocument jsonProperties = QJsonDocument::fromVariant(variantProperties); + // the ID of the parent/avatar changes from session to session. use a special UUID to indicate the avatar + QJsonObject jsonObject = jsonProperties.object(); + if (jsonObject.contains("parentID")) { + if (QUuid(jsonObject["parentID"].toString()) == getID()) { + jsonObject["parentID"] = AVATAR_SELF_ID.toString(); + } + } + jsonProperties = QJsonDocument(jsonObject); + QByteArray binaryProperties = jsonProperties.toBinaryData(); + // end recipe + + _poorlyFormattedAvatarEntityData[entry.first] = binaryProperties; } // save all strings uint32_t i = 0; - for (const auto& entry : _avatarEntityStrings) { - _avatarEntityIDSettings[i].set(entry.first.toString()); + for (const auto& entry : _poorlyFormattedAvatarEntityData) { + _avatarEntityIDSettings[i].set(entry.first); _avatarEntityDataSettings[i].set(entry.second); ++i; } @@ -1567,18 +1581,53 @@ void MyAvatar::loadAvatarEntityDataFromSettings() { QScriptEngine scriptEngine; std::vector entitiesToLoad; - entitiesToLoad.resize(numEntities); resizeAvatarEntitySettingHandles(numEntities); for (int i = 0; i < numEntities; i++) { - // DANGER: this JSONString --> EntityItemProperties operation is expensive - EntityItemProperties& properties = entitiesToLoad[i]; - properties.copyFromJSONString(scriptEngine, _avatarEntityDataSettings[i].get()); - // the ClientOnly property can get stripped out elsewhere so we need to always set it true here + QUuid entityID = QUuid::createUuid(); // generate a new ID + QByteArray binaryData = _avatarEntityDataSettings[i].get(); + //updateAvatarEntity(entityID, binaryData); + + + _avatarEntityDataChanged = true; + if (_clientTraitsHandler) { + // we have a client traits handler, so we need to mark this instanced trait as changed + // so that changes will be sent next frame + _clientTraitsHandler->markInstancedTraitUpdated(AvatarTraits::AvatarEntity, entityID); + } + + QJsonDocument jsonProperties = QJsonDocument::fromBinaryData(binaryData); + if (!jsonProperties.isObject()) { + qCDebug(interfaceapp) << "bad avatarEntityData json" << QString(binaryData.toHex()); + continue; + } + + QVariant variantProperties = jsonProperties.toVariant(); + QVariantMap asMap = variantProperties.toMap(); + QScriptValue scriptProperties = variantMapToScriptValue(asMap, scriptEngine); + EntityItemProperties properties; + EntityItemPropertiesFromScriptValueHonorReadOnly(scriptProperties, properties); properties.setEntityHostType(entity::HostType::AVATAR); properties.setOwningAvatarID(getID()); + + // there's no entity-server to tell us we're the simulation owner, so always set the + // simulationOwner to the owningAvatarID and a high priority. + properties.setSimulationOwner(getID(), AVATAR_ENTITY_SIMULATION_PRIORITY); + + if (properties.getParentID() == AVATAR_SELF_ID) { + properties.setParentID(getID()); + } + + // When grabbing avatar entities, they are parented to the joint moving them, then when un-grabbed + // they go back to the default parent (null uuid). When un-gripped, others saw the entity disappear. + // The thinking here is the local position was noticed as changing, but not the parentID (since it is now + // back to the default), and the entity flew off somewhere. Marking all changed definitely fixes this, + // and seems safe (per Seth). + properties.markAllChanged(); + + entitiesToLoad.push_back(properties); } - _avatarEntityStrings.clear(); + _poorlyFormattedAvatarEntityData.clear(); auto entityTreeRenderer = qApp->getEntities(); EntityTreePointer entityTree = entityTreeRenderer ? entityTreeRenderer->getTree() : nullptr; if (entityTree) { @@ -1597,7 +1646,10 @@ void MyAvatar::loadAvatarEntityDataFromSettings() { // only remember an AvatarEntity that successfully loads and can be packed QByteArray tempArray((const char*)packetData.getUncompressedData(), packetData.getUncompressedSize()); storeAvatarEntityDataPayload(entityID, tempArray); - _avatarEntityStrings[entityID] = _avatarEntityDataSettings[i].get(); + + // we store these binaryProperties for later: when saving back to settings + // unfortunately we must use this non-human-readable format because reasons + _poorlyFormattedAvatarEntityData[entityID] = _avatarEntityDataSettings[i].get(); } packetData.reset(); } diff --git a/interface/src/avatar/MyAvatar.h b/interface/src/avatar/MyAvatar.h index 21517c5af9..4306db8242 100644 --- a/interface/src/avatar/MyAvatar.h +++ b/interface/src/avatar/MyAvatar.h @@ -1944,10 +1944,9 @@ private: Setting::Handle _flyingHMDSetting; Setting::Handle _avatarEntityCountSetting; Setting::Handle _allowTeleportingSetting { "allowTeleporting", true }; - std::vector> _avatarEntityIDSettings; - std::vector> _avatarEntityDataSettings; - std::map _avatarEntityStrings; - std::map _avatarEntityProperties; + std::vector> _avatarEntityIDSettings; + std::vector> _avatarEntityDataSettings; + std::map _poorlyFormattedAvatarEntityData; std::set _entitiesToSaveToSettings; std::set _entitiesToRemoveFromSettings; }; From 9b253690dba10aa4c9aa2c1541ddd8a5fbfe71e2 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Tue, 11 Dec 2018 10:57:49 -0800 Subject: [PATCH 24/43] bump packet version number --- libraries/networking/src/udt/PacketHeaders.cpp | 2 +- libraries/networking/src/udt/PacketHeaders.h | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/libraries/networking/src/udt/PacketHeaders.cpp b/libraries/networking/src/udt/PacketHeaders.cpp index aba3822883..642914cd56 100644 --- a/libraries/networking/src/udt/PacketHeaders.cpp +++ b/libraries/networking/src/udt/PacketHeaders.cpp @@ -41,7 +41,7 @@ PacketVersion versionForPacketType(PacketType packetType) { return static_cast(AvatarMixerPacketVersion::CollisionFlag); case PacketType::BulkAvatarData: case PacketType::KillAvatar: - return static_cast(AvatarMixerPacketVersion::GrabTraits); + return static_cast(AvatarMixerPacketVersion::FasterAvatarEntities); case PacketType::MessagesData: return static_cast(MessageDataVersion::TextOrBinaryData); // ICE packets diff --git a/libraries/networking/src/udt/PacketHeaders.h b/libraries/networking/src/udt/PacketHeaders.h index 6f019f39b6..f53a287d71 100644 --- a/libraries/networking/src/udt/PacketHeaders.h +++ b/libraries/networking/src/udt/PacketHeaders.h @@ -311,7 +311,8 @@ enum class AvatarMixerPacketVersion : PacketVersion { JointTransScaled, GrabTraits, CollisionFlag, - AvatarTraitsAck + AvatarTraitsAck, + FasterAvatarEntities }; enum class DomainConnectRequestVersion : PacketVersion { From 9f404ef006290c20f5d34c4dba1206584d228512 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Wed, 12 Dec 2018 13:49:44 -0800 Subject: [PATCH 25/43] Agent bots can manipulate AvatarEntities again --- .../src/avatars/ScriptableAvatar.cpp | 51 +++++++++++++++++++ .../src/avatars/ScriptableAvatar.h | 3 ++ interface/src/avatar/MyAvatar.cpp | 4 ++ interface/src/avatar/MyAvatar.h | 2 + .../src/avatars-renderer/Avatar.h | 4 +- libraries/avatars/src/AvatarData.cpp | 4 +- libraries/avatars/src/AvatarData.h | 7 +-- libraries/entities/src/EntityTypes.cpp | 4 ++ libraries/entities/src/EntityTypes.h | 1 + 9 files changed, 73 insertions(+), 7 deletions(-) diff --git a/assignment-client/src/avatars/ScriptableAvatar.cpp b/assignment-client/src/avatars/ScriptableAvatar.cpp index a2e13a03be..25cf702b22 100644 --- a/assignment-client/src/avatars/ScriptableAvatar.cpp +++ b/assignment-client/src/avatars/ScriptableAvatar.cpp @@ -21,6 +21,8 @@ #include #include #include +#include +#include ScriptableAvatar::ScriptableAvatar() { @@ -249,3 +251,52 @@ void ScriptableAvatar::setHasProceduralEyeFaceMovement(bool hasProceduralEyeFace void ScriptableAvatar::setHasAudioEnabledFaceMovement(bool hasAudioEnabledFaceMovement) { _headData->setHasAudioEnabledFaceMovement(hasAudioEnabledFaceMovement); } + +void ScriptableAvatar::updateAvatarEntity(const QUuid& id, const QScriptValue& data) { + if (data.isNull()) { + // interpret this as a DELETE + std::map::iterator itr = _entities.find(id); + if (itr != _entities.end()) { + _entities.erase(itr); + clearAvatarEntity(id); + } + } else { + EntityItemPointer entity; + EntityItemProperties properties; + bool honorReadOnly = true; + properties.copyFromScriptValue(data, honorReadOnly); + + std::map::iterator itr = _entities.find(id); + if (itr == _entities.end()) { + // this is an ADD + entity = EntityTypes::constructEntityItem(id, properties); + if (entity) { + OctreePacketData packetData(false, AvatarTraits::MAXIMUM_TRAIT_SIZE); + EncodeBitstreamParams params; + EntityTreeElementExtraEncodeDataPointer extra { nullptr }; + OctreeElement::AppendState appendState = entity->appendEntityData(&packetData, params, extra); + + if (appendState == OctreeElement::COMPLETED) { + _entities[id] = entity; + QByteArray tempArray((const char*)packetData.getUncompressedData(), packetData.getUncompressedSize()); + storeAvatarEntityDataPayload(id, tempArray); + } + } + } else { + // this is an UPDATE + entity = itr->second; + bool somethingChanged = entity->setProperties(properties); + if (somethingChanged) { + OctreePacketData packetData(false, AvatarTraits::MAXIMUM_TRAIT_SIZE); + EncodeBitstreamParams params; + EntityTreeElementExtraEncodeDataPointer extra { nullptr }; + OctreeElement::AppendState appendState = entity->appendEntityData(&packetData, params, extra); + + if (appendState == OctreeElement::COMPLETED) { + QByteArray tempArray((const char*)packetData.getUncompressedData(), packetData.getUncompressedSize()); + storeAvatarEntityDataPayload(id, tempArray); + } + } + } + } +} diff --git a/assignment-client/src/avatars/ScriptableAvatar.h b/assignment-client/src/avatars/ScriptableAvatar.h index 66b0b5ae3f..f56d7c66b0 100644 --- a/assignment-client/src/avatars/ScriptableAvatar.h +++ b/assignment-client/src/avatars/ScriptableAvatar.h @@ -16,6 +16,7 @@ #include #include #include +#include /**jsdoc * The Avatar API is used to manipulate scriptable avatars on the domain. This API is a subset of the @@ -184,6 +185,7 @@ public: bool getHasProceduralEyeFaceMovement() const override { return _headData->getHasProceduralEyeFaceMovement(); } void setHasAudioEnabledFaceMovement(bool hasAudioEnabledFaceMovement); bool getHasAudioEnabledFaceMovement() const override { return _headData->getHasAudioEnabledFaceMovement(); } + void updateAvatarEntity(const QUuid& id, const QScriptValue& data) override; public slots: void update(float deltatime); @@ -202,6 +204,7 @@ private: QHash _fstJointIndices; ///< 1-based, since zero is returned for missing keys QStringList _fstJointNames; ///< in order of depth-first traversal QUrl _skeletonFBXURL; + std::map _entities; /// Loads the joint indices, names from the FST file (if any) void updateJointMappings(); diff --git a/interface/src/avatar/MyAvatar.cpp b/interface/src/avatar/MyAvatar.cpp index f808928709..4bf446f6b9 100755 --- a/interface/src/avatar/MyAvatar.cpp +++ b/interface/src/avatar/MyAvatar.cpp @@ -2061,6 +2061,10 @@ void MyAvatar::clearJointsData() { _skeletonModel->getRig().clearJointStates(); } +void MyAvatar::updateAvatarEntity(const QUuid& id, const QScriptValue& data) { + // TODO: implement this +} + void MyAvatar::setSkeletonModelURL(const QUrl& skeletonModelURL) { _skeletonModelChangeCount++; int skeletonModelChangeCount = _skeletonModelChangeCount; diff --git a/interface/src/avatar/MyAvatar.h b/interface/src/avatar/MyAvatar.h index 4306db8242..98c93f9ccc 100644 --- a/interface/src/avatar/MyAvatar.h +++ b/interface/src/avatar/MyAvatar.h @@ -851,6 +851,8 @@ public: virtual void clearJointData(const QString& name) override; virtual void clearJointsData() override; + void updateAvatarEntity(const QUuid& id, const QScriptValue& data) override; + /**jsdoc * @function MyAvatar.pinJoint * @param {number} index diff --git a/libraries/avatars-renderer/src/avatars-renderer/Avatar.h b/libraries/avatars-renderer/src/avatars-renderer/Avatar.h index 9a57e46f1a..2b356675be 100644 --- a/libraries/avatars-renderer/src/avatars-renderer/Avatar.h +++ b/libraries/avatars-renderer/src/avatars-renderer/Avatar.h @@ -235,13 +235,13 @@ public: void updateDisplayNameAlpha(bool showDisplayName); virtual void setSessionDisplayName(const QString& sessionDisplayName) override { }; // no-op + virtual void updateAvatarEntity(const QUuid& entityID, const QScriptValue& entityData) override { }; // no-op + virtual int parseDataFromBuffer(const QByteArray& buffer) override; static void renderJointConnectingCone(gpu::Batch& batch, glm::vec3 position1, glm::vec3 position2, float radius1, float radius2, const glm::vec4& color); - virtual void applyCollision(const glm::vec3& contactPoint, const glm::vec3& penetration) { } - /**jsdoc * Set the offset applied to the current avatar. The offset adjusts the position that the avatar is rendered. For example, * with an offset of { x: 0, y: 0.1, z: 0 }, your avatar will appear to be raised off the ground slightly. diff --git a/libraries/avatars/src/AvatarData.cpp b/libraries/avatars/src/AvatarData.cpp index 9380699504..f08e66af82 100644 --- a/libraries/avatars/src/AvatarData.cpp +++ b/libraries/avatars/src/AvatarData.cpp @@ -2776,8 +2776,8 @@ void AvatarData::storeAvatarEntityDataPayload(const QUuid& entityID, const QByte } } -void AvatarData::updateAvatarEntity(const QUuid& entityID, const QString& entityPropertiesString) { - // TODO: implement this to expose AvatarEntity to JS +void AvatarData::updateAvatarEntity(const QUuid& id, const QScriptValue& data) { + // no op } void AvatarData::clearAvatarEntity(const QUuid& entityID, bool requiresRemovalFromTree) { diff --git a/libraries/avatars/src/AvatarData.h b/libraries/avatars/src/AvatarData.h index 65ea873502..3d6dd78e05 100644 --- a/libraries/avatars/src/AvatarData.h +++ b/libraries/avatars/src/AvatarData.h @@ -957,9 +957,9 @@ public: /**jsdoc * @function MyAvatar.updateAvatarEntity * @param {Uuid} entityID - * @param {string} entityData + * @param {object} entityData */ - Q_INVOKABLE virtual void updateAvatarEntity(const QUuid& entityID, const QString& entityPropertiesString); + Q_INVOKABLE virtual void updateAvatarEntity(const QUuid& id, const QScriptValue& data); /**jsdoc * @function MyAvatar.clearAvatarEntity @@ -1140,10 +1140,11 @@ public: Q_INVOKABLE AvatarEntityMap getAvatarEntityData() const; /**jsdoc + * Temporarily disabled. Use updateAvatarEntity(id, properties) in the meantime. * @function MyAvatar.setAvatarEntityData * @param {object} avatarEntityData */ - Q_INVOKABLE void setAvatarEntityData(const AvatarEntityMap& avatarEntityData); + void setAvatarEntityData(const AvatarEntityMap& avatarEntityData); virtual void setAvatarEntityDataChanged(bool value) { _avatarEntityDataChanged = value; } void insertDetachedEntityID(const QUuid entityID); diff --git a/libraries/entities/src/EntityTypes.cpp b/libraries/entities/src/EntityTypes.cpp index 6f05237b1e..584af55d5d 100644 --- a/libraries/entities/src/EntityTypes.cpp +++ b/libraries/entities/src/EntityTypes.cpp @@ -151,3 +151,7 @@ EntityItemPointer EntityTypes::constructEntityItem(const unsigned char* data, in } return nullptr; } + +EntityItemPointer EntityTypes::constructEntityItem(const QUuid& id, const EntityItemProperties& properties) { + return constructEntityItem(properties.getType(), id, properties); +} diff --git a/libraries/entities/src/EntityTypes.h b/libraries/entities/src/EntityTypes.h index 9f4ba36df1..2e8914c8a7 100644 --- a/libraries/entities/src/EntityTypes.h +++ b/libraries/entities/src/EntityTypes.h @@ -116,6 +116,7 @@ public: static void extractEntityTypeAndID(const unsigned char* data, int dataLength, EntityTypes::EntityType& typeOut, QUuid& idOut); static EntityItemPointer constructEntityItem(EntityType entityType, const EntityItemID& entityID, const EntityItemProperties& properties); static EntityItemPointer constructEntityItem(const unsigned char* data, int bytesToRead); + static EntityItemPointer constructEntityItem(const QUuid& id, const EntityItemProperties& properties); private: static QMap _typeToNameMap; From cbcd70aee6c00f2a39b4d3757470f626256dd1c8 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Wed, 12 Dec 2018 18:52:26 -0800 Subject: [PATCH 26/43] namechanges and comments to help clarify necessary fixes --- interface/src/avatar/MyAvatar.cpp | 69 ++++++++++-------- interface/src/avatar/MyAvatar.h | 2 +- .../src/avatars-renderer/Avatar.cpp | 6 +- libraries/avatars/src/AvatarData.cpp | 73 +++++++++++-------- libraries/avatars/src/AvatarData.h | 10 ++- 5 files changed, 90 insertions(+), 70 deletions(-) diff --git a/interface/src/avatar/MyAvatar.cpp b/interface/src/avatar/MyAvatar.cpp index 4bf446f6b9..7b7adf14ca 100755 --- a/interface/src/avatar/MyAvatar.cpp +++ b/interface/src/avatar/MyAvatar.cpp @@ -1321,22 +1321,22 @@ void MyAvatar::saveAvatarEntityDataToSettings() { for (const auto& id : _entitiesToRemoveFromSettings) { // remove entitiesToSave.erase(id); - std::map::iterator itr = _poorlyFormattedAvatarEntityData.find(id); - if (itr != _poorlyFormattedAvatarEntityData.end()) { - _poorlyFormattedAvatarEntityData.erase(itr); + std::map::iterator itr = _cachedAvatarEntityDataSettings.find(id); + if (itr != _cachedAvatarEntityDataSettings.end()) { + _cachedAvatarEntityDataSettings.erase(itr); } } for (const auto& id : entitiesToSave) { // remove old strings to be replaced by new saves - std::map::iterator itr = _poorlyFormattedAvatarEntityData.find(id); - if (itr != _poorlyFormattedAvatarEntityData.end()) { - _poorlyFormattedAvatarEntityData.erase(itr); + std::map::iterator itr = _cachedAvatarEntityDataSettings.find(id); + if (itr != _cachedAvatarEntityDataSettings.end()) { + _cachedAvatarEntityDataSettings.erase(itr); } } _entitiesToRemoveFromSettings.clear(); }); - uint32_t numEntities = (uint32_t)(entitiesToSave.size() + _poorlyFormattedAvatarEntityData.size()); + uint32_t numEntities = (uint32_t)(entitiesToSave.size() + _cachedAvatarEntityDataSettings.size()); uint32_t prevNumEntities = _avatarEntityCountSetting.get(0); resizeAvatarEntitySettingHandles(std::max(numEntities, prevNumEntities)); @@ -1353,11 +1353,11 @@ void MyAvatar::saveAvatarEntityDataToSettings() { } } }); - // convert properties to strings + // convert properties to our unfortunately-formatted-binary-blob format QScriptEngine scriptEngine; QScriptValue toStringMethod = scriptEngine.evaluate("(function() { return JSON.stringify(this) })"); for (const auto& entry : allProperties) { - // begin recipe our unfortunate legacy avatarEntityData format + // begin recipe for converting to our unfortunately-formatted-binar-blob QScriptValue scriptValue = EntityItemNonDefaultPropertiesToScriptValue(&scriptEngine, entry.second); QVariant variantProperties = scriptValue.toVariant(); QJsonDocument jsonProperties = QJsonDocument::fromVariant(variantProperties); @@ -1372,11 +1372,12 @@ void MyAvatar::saveAvatarEntityDataToSettings() { QByteArray binaryProperties = jsonProperties.toBinaryData(); // end recipe - _poorlyFormattedAvatarEntityData[entry.first] = binaryProperties; + // remember this unfortunately-formatted-binary-blob for later so we don't have go through this again + _cachedAvatarEntityDataSettings[entry.first] = binaryProperties; } - // save all strings + // save all unfortunately-formatted-binary-blobs to settings uint32_t i = 0; - for (const auto& entry : _poorlyFormattedAvatarEntityData) { + for (const auto& entry : _cachedAvatarEntityDataSettings) { _avatarEntityIDSettings[i].set(entry.first); _avatarEntityDataSettings[i].set(entry.second); ++i; @@ -1570,7 +1571,7 @@ void MyAvatar::loadData() { void MyAvatar::loadAvatarEntityDataFromSettings() { _avatarEntitiesLock.withReadLock([&] { - _avatarEntityData.clear(); + _packedAvatarEntityData.clear(); }); _reloadAvatarEntityDataFromSettings = false; @@ -1580,32 +1581,34 @@ void MyAvatar::loadAvatarEntityDataFromSettings() { } QScriptEngine scriptEngine; - std::vector entitiesToLoad; + std::vector< std::pair > entitiesToLoad; resizeAvatarEntitySettingHandles(numEntities); for (int i = 0; i < numEntities; i++) { - QUuid entityID = QUuid::createUuid(); // generate a new ID - QByteArray binaryData = _avatarEntityDataSettings[i].get(); - //updateAvatarEntity(entityID, binaryData); + QUuid id = QUuid::createUuid(); // generate a new ID + // the avatarEntityData is stored as an unfortunately-formatted-binary-blob + QByteArray binaryData = _avatarEntityDataSettings[i].get(); _avatarEntityDataChanged = true; if (_clientTraitsHandler) { // we have a client traits handler, so we need to mark this instanced trait as changed // so that changes will be sent next frame - _clientTraitsHandler->markInstancedTraitUpdated(AvatarTraits::AvatarEntity, entityID); + _clientTraitsHandler->markInstancedTraitUpdated(AvatarTraits::AvatarEntity, id); } + // begin recipe to extract EntityItemProperties from unfortunately-formatted-binary-blob QJsonDocument jsonProperties = QJsonDocument::fromBinaryData(binaryData); if (!jsonProperties.isObject()) { qCDebug(interfaceapp) << "bad avatarEntityData json" << QString(binaryData.toHex()); continue; } - - QVariant variantProperties = jsonProperties.toVariant(); - QVariantMap asMap = variantProperties.toMap(); - QScriptValue scriptProperties = variantMapToScriptValue(asMap, scriptEngine); + QVariant variant = jsonProperties.toVariant(); + QVariantMap variantMap = variant.toMap(); + QScriptValue scriptValue = variantMapToScriptValue(variantMap, scriptEngine); EntityItemProperties properties; - EntityItemPropertiesFromScriptValueHonorReadOnly(scriptProperties, properties); + EntityItemPropertiesFromScriptValueHonorReadOnly(scriptValue, properties); + // end recipe + properties.setEntityHostType(entity::HostType::AVATAR); properties.setOwningAvatarID(getID()); @@ -1624,36 +1627,38 @@ void MyAvatar::loadAvatarEntityDataFromSettings() { // and seems safe (per Seth). properties.markAllChanged(); - entitiesToLoad.push_back(properties); + entitiesToLoad.push_back({id, properties}); } - _poorlyFormattedAvatarEntityData.clear(); + _cachedAvatarEntityDataSettings.clear(); auto entityTreeRenderer = qApp->getEntities(); EntityTreePointer entityTree = entityTreeRenderer ? entityTreeRenderer->getTree() : nullptr; if (entityTree) { OctreePacketData packetData(false, AvatarTraits::MAXIMUM_TRAIT_SIZE); EncodeBitstreamParams params; EntityTreeElementExtraEncodeDataPointer extra { nullptr }; - for (uint32_t i = 0; i < entitiesToLoad.size(); ++i) { + uint32_t i = 0; + for (const auto& entry : entitiesToLoad) { // try to create the entity - QUuid entityID = QUuid::createUuid(); // generate a new ID entityTree->withWriteLock([&] { - EntityItemPointer entity = entityTree->addEntity(entityID, entitiesToLoad[i]); + QUuid id = entry.first; + EntityItemPointer entity = entityTree->addEntity(id, entry.second); if (entity) { // use the entity to build the data payload OctreeElement::AppendState appendState = entity->appendEntityData(&packetData, params, extra); if (appendState == OctreeElement::COMPLETED) { // only remember an AvatarEntity that successfully loads and can be packed QByteArray tempArray((const char*)packetData.getUncompressedData(), packetData.getUncompressedSize()); - storeAvatarEntityDataPayload(entityID, tempArray); + storeAvatarEntityDataPayload(id, tempArray); - // we store these binaryProperties for later: when saving back to settings - // unfortunately we must use this non-human-readable format because reasons - _poorlyFormattedAvatarEntityData[entityID] = _avatarEntityDataSettings[i].get(); + // only cache things that successfully loaded and packed + // we cache these binaryProperties for later: for when saving back to settings + _cachedAvatarEntityDataSettings[id] = _avatarEntityDataSettings[i].get(); } packetData.reset(); } }); + ++i; } } else { // TODO? handle this case: try to load AvatatEntityData from settings again later? diff --git a/interface/src/avatar/MyAvatar.h b/interface/src/avatar/MyAvatar.h index 98c93f9ccc..9bbb25a3be 100644 --- a/interface/src/avatar/MyAvatar.h +++ b/interface/src/avatar/MyAvatar.h @@ -1948,7 +1948,7 @@ private: Setting::Handle _allowTeleportingSetting { "allowTeleporting", true }; std::vector> _avatarEntityIDSettings; std::vector> _avatarEntityDataSettings; - std::map _poorlyFormattedAvatarEntityData; + std::map _cachedAvatarEntityDataSettings; std::set _entitiesToSaveToSettings; std::set _entitiesToRemoveFromSettings; }; diff --git a/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp b/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp index 59d9f13f04..b41e1381ef 100644 --- a/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp +++ b/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp @@ -436,12 +436,12 @@ void Avatar::updateAvatarEntities() { AvatarEntityIDs recentlyDetachedAvatarEntities = getAndClearRecentlyDetachedIDs(); if (!recentlyDetachedAvatarEntities.empty()) { // only lock this thread when absolutely necessary - AvatarEntityMap avatarEntityData; + AvatarEntityMap packedAvatarEntityData; _avatarEntitiesLock.withReadLock([&] { - avatarEntityData = _avatarEntityData; + packedAvatarEntityData = _packedAvatarEntityData; }); foreach (auto entityID, recentlyDetachedAvatarEntities) { - if (!avatarEntityData.contains(entityID)) { + if (!packedAvatarEntityData.contains(entityID)) { entityTree->deleteEntity(entityID, true, true); } } diff --git a/libraries/avatars/src/AvatarData.cpp b/libraries/avatars/src/AvatarData.cpp index f08e66af82..093b86d0e6 100644 --- a/libraries/avatars/src/AvatarData.cpp +++ b/libraries/avatars/src/AvatarData.cpp @@ -1908,10 +1908,9 @@ qint64 AvatarData::packAvatarEntityTraitInstance(AvatarTraits::TraitType traitTy // grab a read lock on the avatar entities and check for entity data for the given ID QByteArray entityBinaryData; - _avatarEntitiesLock.withReadLock([this, &entityBinaryData, &traitInstanceID] { - if (_avatarEntityData.contains(traitInstanceID)) { - entityBinaryData = _avatarEntityData[traitInstanceID]; + if (_packedAvatarEntityData.contains(traitInstanceID)) { + entityBinaryData = _packedAvatarEntityData[traitInstanceID]; } }); @@ -1998,7 +1997,7 @@ qint64 AvatarData::packTraitInstance(AvatarTraits::TraitType traitType, AvatarTr void AvatarData::prepareResetTraitInstances() { if (_clientTraitsHandler) { _avatarEntitiesLock.withReadLock([this]{ - foreach (auto entityID, _avatarEntityData.keys()) { + foreach (auto entityID, _packedAvatarEntityData.keys()) { _clientTraitsHandler->markInstancedTraitUpdated(AvatarTraits::AvatarEntity, entityID); } foreach (auto grabID, _avatarGrabData.keys()) { @@ -2367,7 +2366,7 @@ void AvatarData::setRecordingBasis(std::shared_ptr recordingBasis) { void AvatarData::createRecordingIDs() { _avatarEntitiesLock.withReadLock([&] { _avatarEntityForRecording.clear(); - for (int i = 0; i < _avatarEntityData.size(); i++) { + for (int i = 0; i < _packedAvatarEntityData.size(); i++) { _avatarEntityForRecording.insert(QUuid::createUuid()); } }); @@ -2422,6 +2421,25 @@ JointData jointDataFromJsonValue(int version, const QJsonValue& json) { return result; } +void AvatarData::avatarEntityDataToJson(QJsonObject& root) const { + // ADEBUG this is broken + _avatarEntitiesLock.withReadLock([&] { + AvatarEntityMap _avatarEntityData = _packedAvatarEntityData; // hack to compile + if (!_avatarEntityData.empty()) { + QJsonArray avatarEntityJson; + int entityCount = 0; + for (auto entityID : _avatarEntityData.keys()) { + QVariantMap entityData; + QUuid newId = _avatarEntityForRecording.size() == _avatarEntityData.size() ? _avatarEntityForRecording.values()[entityCount++] : entityID; + entityData.insert("id", newId); + entityData.insert("properties", _avatarEntityData.value(entityID).toBase64()); + avatarEntityJson.push_back(QVariant(entityData).toJsonObject()); + } + root[JSON_AVATAR_ENTITIES] = avatarEntityJson; + } + }); +} + QJsonObject AvatarData::toJson() const { QJsonObject root; @@ -2433,20 +2451,8 @@ QJsonObject AvatarData::toJson() const { if (!getDisplayName().isEmpty()) { root[JSON_AVATAR_DISPLAY_NAME] = getDisplayName(); } - _avatarEntitiesLock.withReadLock([&] { - if (!_avatarEntityData.empty()) { - QJsonArray avatarEntityJson; - int entityCount = 0; - for (auto entityID : _avatarEntityData.keys()) { - QVariantMap entityData; - QUuid newId = _avatarEntityForRecording.size() == _avatarEntityData.size() ? _avatarEntityForRecording.values()[entityCount++] : entityID; - entityData.insert("id", newId); - entityData.insert("properties", _avatarEntityData.value(entityID).toBase64()); - avatarEntityJson.push_back(QVariant(entityData).toJsonObject()); - } - root[JSON_AVATAR_ENTITIES] = avatarEntityJson; - } - }); + + avatarEntityDataToJson(root); auto recordingBasis = getRecordingBasis(); bool success; @@ -2568,10 +2574,10 @@ void AvatarData::fromJson(const QJsonObject& json, bool useFrameSkeleton) { for (auto attachmentJson : attachmentsJson) { if (attachmentJson.isObject()) { QVariantMap entityData = attachmentJson.toObject().toVariantMap(); - QUuid entityID = entityData.value("id").toUuid(); + QUuid id = entityData.value("id").toUuid(); // ADEBUG TODO: fix this broken path - QByteArray properties = QByteArray::fromBase64(entityData.value("properties").toByteArray()); - storeAvatarEntityDataPayload(entityID, properties); + QByteArray data = QByteArray::fromBase64(entityData.value("properties").toByteArray()); + updateAvatarEntityData(id, data); } } } @@ -2757,10 +2763,10 @@ const int MAX_NUM_AVATAR_ENTITIES = 42; void AvatarData::storeAvatarEntityDataPayload(const QUuid& entityID, const QByteArray& data) { _avatarEntitiesLock.withWriteLock([&] { - AvatarEntityMap::iterator itr = _avatarEntityData.find(entityID); - if (itr == _avatarEntityData.end()) { - if (_avatarEntityData.size() < MAX_NUM_AVATAR_ENTITIES) { - _avatarEntityData.insert(entityID, data); + PackedAvatarEntityMap::iterator itr = _packedAvatarEntityData.find(entityID); + if (itr == _packedAvatarEntityData.end()) { + if (_packedAvatarEntityData.size() < MAX_NUM_AVATAR_ENTITIES) { + _packedAvatarEntityData.insert(entityID, data); } } else { itr.value() = data; @@ -2776,8 +2782,12 @@ void AvatarData::storeAvatarEntityDataPayload(const QUuid& entityID, const QByte } } -void AvatarData::updateAvatarEntity(const QUuid& id, const QScriptValue& data) { - // no op +void AvatarData::updateAvatarEntity(const QUuid& id, const QScriptValue& scriptValue) { + // overridden where needed +} + +void AvatarData::updateAvatarEntityData(const QUuid& id, const QByteArray& data) { + // ADEBUG TODO: implement this } void AvatarData::clearAvatarEntity(const QUuid& entityID, bool requiresRemovalFromTree) { @@ -2785,7 +2795,7 @@ void AvatarData::clearAvatarEntity(const QUuid& entityID, bool requiresRemovalFr bool removedEntity = false; _avatarEntitiesLock.withWriteLock([this, &removedEntity, &entityID] { - removedEntity = _avatarEntityData.remove(entityID); + removedEntity = _packedAvatarEntityData.remove(entityID); }); insertDetachedEntityID(entityID); @@ -2798,9 +2808,10 @@ void AvatarData::clearAvatarEntity(const QUuid& entityID, bool requiresRemovalFr } AvatarEntityMap AvatarData::getAvatarEntityData() const { + // ADEBUG broken AvatarEntityMap result; _avatarEntitiesLock.withReadLock([&] { - result = _avatarEntityData; + result = _packedAvatarEntityData; }); return result; } @@ -2823,7 +2834,9 @@ void AvatarData::setAvatarEntityData(const AvatarEntityMap& avatarEntityData) { std::vector deletedEntityIDs; QList updatedEntityIDs; + // ADEBUG this is broken _avatarEntitiesLock.withWriteLock([&] { + AvatarEntityMap _avatarEntityData = _packedAvatarEntityData; // hack to compile if (_avatarEntityData != avatarEntityData) { // keep track of entities that were attached to this avatar but no longer are diff --git a/libraries/avatars/src/AvatarData.h b/libraries/avatars/src/AvatarData.h index 3d6dd78e05..1bae3cd32f 100644 --- a/libraries/avatars/src/AvatarData.h +++ b/libraries/avatars/src/AvatarData.h @@ -63,6 +63,7 @@ using AvatarWeakPointer = std::weak_ptr; using AvatarHash = QHash; using AvatarEntityMap = QMap; +using PackedAvatarEntityMap = QMap; // similar to AvatarEntityMap, but different internal format using AvatarEntityIDs = QSet; using AvatarGrabDataMap = QMap; @@ -959,7 +960,8 @@ public: * @param {Uuid} entityID * @param {object} entityData */ - Q_INVOKABLE virtual void updateAvatarEntity(const QUuid& id, const QScriptValue& data); + Q_INVOKABLE virtual void updateAvatarEntity(const QUuid& id, const QScriptValue& scriptValue); + virtual void updateAvatarEntityData(const QUuid& id, const QByteArray& data); /**jsdoc * @function MyAvatar.clearAvatarEntity @@ -1126,6 +1128,7 @@ public: TransformPointer getRecordingBasis() const; void setRecordingBasis(TransformPointer recordingBasis = TransformPointer()); void createRecordingIDs(); + void avatarEntityDataToJson(QJsonObject& root) const; QJsonObject toJson() const; void fromJson(const QJsonObject& json, bool useFrameSkeleton = true); @@ -1140,11 +1143,10 @@ public: Q_INVOKABLE AvatarEntityMap getAvatarEntityData() const; /**jsdoc - * Temporarily disabled. Use updateAvatarEntity(id, properties) in the meantime. * @function MyAvatar.setAvatarEntityData * @param {object} avatarEntityData */ - void setAvatarEntityData(const AvatarEntityMap& avatarEntityData); + Q_INVOKABLE void setAvatarEntityData(const AvatarEntityMap& avatarEntityData); virtual void setAvatarEntityDataChanged(bool value) { _avatarEntityDataChanged = value; } void insertDetachedEntityID(const QUuid entityID); @@ -1465,7 +1467,7 @@ protected: mutable ReadWriteLockable _avatarEntitiesLock; AvatarEntityIDs _avatarEntityDetached; // recently detached from this avatar AvatarEntityIDs _avatarEntityForRecording; // create new entities id for avatar recording - AvatarEntityMap _avatarEntityData; + PackedAvatarEntityMap _packedAvatarEntityData; bool _avatarEntityDataChanged { false }; mutable ReadWriteLockable _avatarGrabsLock; From 4dd6e23fefcbf6cfa337eb0a186ea1f92c634767 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Fri, 14 Dec 2018 15:05:26 -0800 Subject: [PATCH 27/43] fix typo in comment --- libraries/entities/src/EntityEditPacketSender.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/entities/src/EntityEditPacketSender.cpp b/libraries/entities/src/EntityEditPacketSender.cpp index eca431b565..8b051ef37e 100644 --- a/libraries/entities/src/EntityEditPacketSender.cpp +++ b/libraries/entities/src/EntityEditPacketSender.cpp @@ -44,7 +44,7 @@ void EntityEditPacketSender::queueEditAvatarEntityMessage(EntityTreePointer enti const EntityItemProperties& properties) { assert(_myAvatar); if (!entityTree) { - qCDebug(entities) << "EntityEditPacketSender::queueEditEntityMessage null entityTree."; + qCDebug(entities) << "EntityEditPacketSender::queueEditAvatarEntityMessage null entityTree."; return; } EntityItemPointer entity = entityTree->findEntityByEntityItemID(entityItemID); From 329ec84104fa7c8d3936ad8988f404ed2b8eaa92 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Fri, 14 Dec 2018 15:06:19 -0800 Subject: [PATCH 28/43] MyAvatar.setAvatarEntityData() works in theory, blobs saved to settings --- interface/src/Application.cpp | 3 + interface/src/avatar/MyAvatar.cpp | 569 +++++++++++++++++++-------- interface/src/avatar/MyAvatar.h | 41 +- libraries/avatars/src/AvatarData.cpp | 89 +---- libraries/avatars/src/AvatarData.h | 8 +- 5 files changed, 456 insertions(+), 254 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index e0128d0ceb..0a49afa95f 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -2459,6 +2459,9 @@ void Application::updateHeartbeat() const { } void Application::onAboutToQuit() { + // quickly save AvatarEntityData before the EntityTree is dismantled + getMyAvatar()->saveAvatarDataToSettings(); + emit beforeAboutToQuit(); if (getLoginDialogPoppedUp() && _firstRun.get()) { diff --git a/interface/src/avatar/MyAvatar.cpp b/interface/src/avatar/MyAvatar.cpp index 7b7adf14ca..051aab4cdf 100755 --- a/interface/src/avatar/MyAvatar.cpp +++ b/interface/src/avatar/MyAvatar.cpp @@ -282,6 +282,8 @@ MyAvatar::MyAvatar(QThread* thread) : MyAvatar::~MyAvatar() { _lookAtTargetAvatar.reset(); + delete _myScriptEngine; + _myScriptEngine = nullptr; } void MyAvatar::setDominantHand(const QString& hand) { @@ -1304,89 +1306,37 @@ void MyAvatar::saveData() { } void MyAvatar::saveAvatarEntityDataToSettings() { - if (_entitiesToSaveToSettings.size() + _entitiesToRemoveFromSettings.size() == 0) { - // nothing to do + if (!_needToSaveAvatarEntitySettings) { return; } - auto entityTreeRenderer = qApp->getEntities(); - EntityTreePointer entityTree = entityTreeRenderer ? entityTreeRenderer->getTree() : nullptr; - if (!entityTree) { + bool success = updateStaleAvatarEntityBlobs(); + if (!success) { return; } + _needToSaveAvatarEntitySettings = false; - // find set of things that changed - std::set entitiesToSave; - _avatarEntitiesLock.withWriteLock([&] { - entitiesToSave = std::move(_entitiesToSaveToSettings); - for (const auto& id : _entitiesToRemoveFromSettings) { - // remove - entitiesToSave.erase(id); - std::map::iterator itr = _cachedAvatarEntityDataSettings.find(id); - if (itr != _cachedAvatarEntityDataSettings.end()) { - _cachedAvatarEntityDataSettings.erase(itr); - } - } - for (const auto& id : entitiesToSave) { - // remove old strings to be replaced by new saves - std::map::iterator itr = _cachedAvatarEntityDataSettings.find(id); - if (itr != _cachedAvatarEntityDataSettings.end()) { - _cachedAvatarEntityDataSettings.erase(itr); - } - } - _entitiesToRemoveFromSettings.clear(); - }); - - uint32_t numEntities = (uint32_t)(entitiesToSave.size() + _cachedAvatarEntityDataSettings.size()); + uint32_t numEntities = (uint32_t)_cachedAvatarEntityBlobs.size(); uint32_t prevNumEntities = _avatarEntityCountSetting.get(0); resizeAvatarEntitySettingHandles(std::max(numEntities, prevNumEntities)); - // save new settings + // save new Settings if (numEntities > 0) { - // get all properties to save - std::map allProperties; - EntityItemPointer entity; - entityTree->withWriteLock([&] { - for (auto& id : entitiesToSave) { - EntityItemPointer entity = entityTree->findEntityByID(id); - if (entity) { - allProperties[id] = entity->getProperties(); - } + // save all unfortunately-formatted-binary-blobs to Settings + _avatarEntitiesLock.withWriteLock([&] { + uint32_t i = 0; + AvatarEntityMap::const_iterator itr = _cachedAvatarEntityBlobs.begin(); + while (itr != _cachedAvatarEntityBlobs.end()) { + _avatarEntityIDSettings[i].set(itr.key()); + _avatarEntityDataSettings[i].set(itr.value()); + ++itr; + ++i; } + numEntities = i; }); - // convert properties to our unfortunately-formatted-binary-blob format - QScriptEngine scriptEngine; - QScriptValue toStringMethod = scriptEngine.evaluate("(function() { return JSON.stringify(this) })"); - for (const auto& entry : allProperties) { - // begin recipe for converting to our unfortunately-formatted-binar-blob - QScriptValue scriptValue = EntityItemNonDefaultPropertiesToScriptValue(&scriptEngine, entry.second); - QVariant variantProperties = scriptValue.toVariant(); - QJsonDocument jsonProperties = QJsonDocument::fromVariant(variantProperties); - // the ID of the parent/avatar changes from session to session. use a special UUID to indicate the avatar - QJsonObject jsonObject = jsonProperties.object(); - if (jsonObject.contains("parentID")) { - if (QUuid(jsonObject["parentID"].toString()) == getID()) { - jsonObject["parentID"] = AVATAR_SELF_ID.toString(); - } - } - jsonProperties = QJsonDocument(jsonObject); - QByteArray binaryProperties = jsonProperties.toBinaryData(); - // end recipe - - // remember this unfortunately-formatted-binary-blob for later so we don't have go through this again - _cachedAvatarEntityDataSettings[entry.first] = binaryProperties; - } - // save all unfortunately-formatted-binary-blobs to settings - uint32_t i = 0; - for (const auto& entry : _cachedAvatarEntityDataSettings) { - _avatarEntityIDSettings[i].set(entry.first); - _avatarEntityDataSettings[i].set(entry.second); - ++i; - } - numEntities = i; } _avatarEntityCountSetting.set(numEntities); - // remove old settings if any + // remove old Settings if any if (numEntities < prevNumEntities) { uint32_t numEntitiesToRemove = prevNumEntities - numEntities; for (uint32_t i = 0; i < numEntitiesToRemove; ++i) { @@ -1499,35 +1449,304 @@ void MyAvatar::setEnableInverseKinematics(bool isEnabled) { void MyAvatar::storeAvatarEntityDataPayload(const QUuid& entityID, const QByteArray& payload) { AvatarData::storeAvatarEntityDataPayload(entityID, payload); _avatarEntitiesLock.withWriteLock([&] { - _entitiesToSaveToSettings.insert(entityID); + _cachedAvatarEntityBlobsToAddOrUpdate.push_back(entityID); }); } void MyAvatar::clearAvatarEntity(const QUuid& entityID, bool requiresRemovalFromTree) { - _avatarEntitiesLock.withWriteLock([&] { - _entitiesToRemoveFromSettings.insert(entityID); - }); AvatarData::clearAvatarEntity(entityID, requiresRemovalFromTree); + _avatarEntitiesLock.withWriteLock([&] { + _cachedAvatarEntityBlobsToDelete.push_back(entityID); + }); +} + +bool blobToProperties(QScriptEngine& scriptEngine, const QByteArray& blob, EntityItemProperties& properties) { + // begin recipe for converting unfortunately-formatted-binary-blob to EntityItemProperties + QJsonDocument jsonProperties = QJsonDocument::fromBinaryData(blob); + if (!jsonProperties.isObject()) { + qCDebug(interfaceapp) << "bad avatarEntityData json" << QString(blob.toHex()); + return false; + } + QVariant variant = jsonProperties.toVariant(); + QVariantMap variantMap = variant.toMap(); + QScriptValue scriptValue = variantMapToScriptValue(variantMap, scriptEngine); + EntityItemPropertiesFromScriptValueHonorReadOnly(scriptValue, properties); + // end recipe + return true; +} + +void propertiesToBlob(QScriptEngine& scriptEngine, const QUuid& myAvatarID, const EntityItemProperties& properties, QByteArray& blob) { + // begin recipe for extracting unfortunately-formatted-binary-blob from EntityItem + QScriptValue scriptValue = EntityItemNonDefaultPropertiesToScriptValue(&scriptEngine, properties); + QVariant variantProperties = scriptValue.toVariant(); + QJsonDocument jsonProperties = QJsonDocument::fromVariant(variantProperties); + // the ID of the parent/avatar changes from session to session. use a special UUID to indicate the avatar + QJsonObject jsonObject = jsonProperties.object(); + if (jsonObject.contains("parentID")) { + if (QUuid(jsonObject["parentID"].toString()) == myAvatarID) { + jsonObject["parentID"] = AVATAR_SELF_ID.toString(); + } + } + jsonProperties = QJsonDocument(jsonObject); + blob = jsonProperties.toBinaryData(); + // end recipe +} + +void MyAvatar::sanitizeAvatarEntityProperties(EntityItemProperties& properties) const { + properties.setEntityHostType(entity::HostType::AVATAR); + properties.setOwningAvatarID(getID()); + + // there's no entity-server to tell us we're the simulation owner, so always set the + // simulationOwner to the owningAvatarID and a high priority. + properties.setSimulationOwner(getID(), AVATAR_ENTITY_SIMULATION_PRIORITY); + + if (properties.getParentID() == AVATAR_SELF_ID) { + properties.setParentID(getID()); + } + + // When grabbing avatar entities, they are parented to the joint moving them, then when un-grabbed + // they go back to the default parent (null uuid). When un-gripped, others saw the entity disappear. + // The thinking here is the local position was noticed as changing, but not the parentID (since it is now + // back to the default), and the entity flew off somewhere. Marking all changed definitely fixes this, + // and seems safe (per Seth). + properties.markAllChanged(); } void MyAvatar::updateAvatarEntities() { + if (getID().isNull() || + getID() == AVATAR_SELF_ID || + DependencyManager::get()->getSessionUUID() == QUuid()) { + // wait until MyAvatar and this Node gets an ID before doing this. Otherwise, various things go wrong: + // things get their parent fixed up from AVATAR_SELF_ID to a null uuid which means "no parent". + return; + } if (_reloadAvatarEntityDataFromSettings) { - if (getID().isNull() || - getID() == AVATAR_SELF_ID || - DependencyManager::get()->getSessionUUID() == QUuid()) { - // wait until MyAvatar and this Node gets an ID before doing this. Otherwise, various things go wrong: - // things get their parent fixed up from AVATAR_SELF_ID to a null uuid which means "no parent". - return; - } - auto treeRenderer = DependencyManager::get(); - EntityTreePointer entityTree = treeRenderer ? treeRenderer->getTree() : nullptr; - if (!entityTree) { - return; - } loadAvatarEntityDataFromSettings(); } + + auto treeRenderer = DependencyManager::get(); + EntityTreePointer entityTree = treeRenderer ? treeRenderer->getTree() : nullptr; + if (!entityTree) { + return; + } + + // We collect changes to AvatarEntities and then handle them all in one spot per frame: updateAvatarEntities(). + // Basically this is a "transaction pattern" with an extra complication: these changes can come from two + // "directions" and the "authoritative source" of each direction is different, so maintain two distinct sets of + // transaction lists; + // + // The _entitiesToDelete/Add/Update lists are for changes whose "authoritative sources" are already + // correctly stored in _cachedAvatarEntityBlobs. These come from loadAvatarEntityDataFromSettings() and + // setAvatarEntityData(). These changes need to be extracted from _cachedAvatarEntityBlobs and applied to + // real EntityItems. + // + // The _cachedAvatarEntityBlobsToDelete/Add/Update lists are for changes whose "authoritative sources" are + // already reflected in real EntityItems. These changes need to be propagated to _cachedAvatarEntityBlobs + // and eventually to Settings. + // + // The DELETEs also need to be propagated to the traits, which will eventually propagate to + // AvatarData::_packedAvatarEntityData via deeper logic. + + // move the lists to minimize lock time + std::vector cachedBlobsToDelete; + std::vector cachedBlobsToUpdate; + std::vector entitiesToDelete; + std::vector entitiesToAdd; + std::vector entitiesToUpdate; + _avatarEntitiesLock.withWriteLock([&] { + cachedBlobsToDelete = std::move(_cachedAvatarEntityBlobsToDelete); + cachedBlobsToUpdate = std::move(_cachedAvatarEntityBlobsToAddOrUpdate); + entitiesToDelete = std::move(_entitiesToDelete); + entitiesToAdd = std::move(_entitiesToAdd); + entitiesToUpdate = std::move(_entitiesToUpdate); + }); + + auto removeAllInstancesHelper = [] (const QUuid& id, std::vector& v) { + uint32_t i = 0; + while (i < v.size()) { + if (id == v[i]) { + v[i] = v.back(); + v.pop_back(); + } else { + ++i; + } + } + }; + + // remove delete-add and delete-update overlap + for (const auto& id : entitiesToDelete) { + removeAllInstancesHelper(id, cachedBlobsToUpdate); + removeAllInstancesHelper(id, entitiesToAdd); + removeAllInstancesHelper(id, entitiesToUpdate); + } + for (const auto& id : cachedBlobsToDelete) { + removeAllInstancesHelper(id, entitiesToUpdate); + removeAllInstancesHelper(id, cachedBlobsToUpdate); + } + for (const auto& id : entitiesToAdd) { + removeAllInstancesHelper(id, entitiesToUpdate); + } + + // DELETE real entities + for (const auto& id : entitiesToDelete) { + entityTree->withWriteLock([&] { + entityTree->deleteEntity(id); + }); + } + + // ADD real entities + for (const auto& id : entitiesToAdd) { + bool blobFailed = false; + EntityItemProperties properties; + _avatarEntitiesLock.withReadLock([&] { + AvatarEntityMap::iterator itr = _cachedAvatarEntityBlobs.find(id); + if (itr == _cachedAvatarEntityBlobs.end()) { + blobFailed = true; // blob doesn't exist + return; + } + if (!blobToProperties(*_myScriptEngine, itr.value(), properties)) { + blobFailed = true; // blob is corrupt + } + }); + if (blobFailed) { + // remove from _cachedAvatarEntityBlobUpdatesToSkip just in case: + // avoids a resource leak when blob updates to be skipped are never actually skipped + // when the blob fails to result in a real EntityItem + _avatarEntitiesLock.withWriteLock([&] { + removeAllInstancesHelper(id, _cachedAvatarEntityBlobUpdatesToSkip); + }); + continue; + } + sanitizeAvatarEntityProperties(properties); + entityTree->withWriteLock([&] { + EntityItemPointer entity = entityTree->addEntity(id, properties); + }); + } + + // CHANGE real entities + for (const auto& id : entitiesToUpdate) { + EntityItemProperties properties; + bool skip = false; + _avatarEntitiesLock.withReadLock([&] { + AvatarEntityMap::iterator itr = _cachedAvatarEntityBlobs.find(id); + if (itr == _cachedAvatarEntityBlobs.end()) { + skip = true; + return; + } + if (!blobToProperties(*_myScriptEngine, itr.value(), properties)) { + skip = true; + } + }); + sanitizeAvatarEntityProperties(properties); + entityTree->withWriteLock([&] { + entityTree->updateEntity(id, properties); + }); + } + + // DELETE cached blobs + _avatarEntitiesLock.withWriteLock([&] { + for (const auto& id : cachedBlobsToDelete) { + AvatarEntityMap::iterator itr = _cachedAvatarEntityBlobs.find(id); + // remove blob and remember to remove from settings + if (itr != _cachedAvatarEntityBlobs.end()) { + _cachedAvatarEntityBlobs.erase(itr); + _needToSaveAvatarEntitySettings = true; + } + // also remove from list of stale blobs to avoid failed entity lookup later + std::set::iterator blobItr = _staleCachedAvatarEntityBlobs.find(id); + if (blobItr != _staleCachedAvatarEntityBlobs.end()) { + _staleCachedAvatarEntityBlobs.erase(blobItr); + } + // also remove from _cachedAvatarEntityBlobUpdatesToSkip just in case: + // avoids a resource leak when things are deleted before they could be skipped + removeAllInstancesHelper(id, _cachedAvatarEntityBlobUpdatesToSkip); + } + }); + + // ADD/UPDATE cached blobs + for (const auto& id : cachedBlobsToUpdate) { + // computing the blobs is expensive and we want to avoid it when possible + // so we add these ids to _staleCachedAvatarEntityBlobs for later + // and only build the blobs when absolutely necessary + bool skip = false; + uint32_t i = 0; + _avatarEntitiesLock.withWriteLock([&] { + while (i < _cachedAvatarEntityBlobUpdatesToSkip.size()) { + if (id == _cachedAvatarEntityBlobUpdatesToSkip[i]) { + _cachedAvatarEntityBlobUpdatesToSkip[i] = _cachedAvatarEntityBlobUpdatesToSkip.back(); + _cachedAvatarEntityBlobUpdatesToSkip.pop_back(); + skip = true; + break; // assume no duplicates + } else { + ++i; + } + } + }); + if (!skip) { + _staleCachedAvatarEntityBlobs.insert(id); + _needToSaveAvatarEntitySettings = true; + } + } + + // DELETE traits + // (no need to worry about the ADDs and UPDATEs: each will be handled when the interface + // tries to send a real update packet (via AvatarData::storeAvatarEntityDataPayload())) + if (_clientTraitsHandler) { + // we have a client traits handler + // flag removed entities as deleted so that changes are sent next frame + _avatarEntitiesLock.withWriteLock([&] { + for (const auto& id : entitiesToDelete) { + if (_packedAvatarEntityData.find(id) != _packedAvatarEntityData.end()) { + _clientTraitsHandler->markInstancedTraitDeleted(AvatarTraits::AvatarEntity, id); + } + } + for (const auto& id : cachedBlobsToDelete) { + if (_packedAvatarEntityData.find(id) != _packedAvatarEntityData.end()) { + _clientTraitsHandler->markInstancedTraitDeleted(AvatarTraits::AvatarEntity, id); + } + } + }); + } } +bool MyAvatar::updateStaleAvatarEntityBlobs() const { + // call this right before you actually need to use the blobs + // + // Note: this method is const (and modifies mutable data members) + // so we can call it at the Last Minute inside other const methods + // + auto treeRenderer = DependencyManager::get(); + EntityTreePointer entityTree = treeRenderer ? treeRenderer->getTree() : nullptr; + if (!entityTree) { + return false; + } + + std::set staleBlobs; + _avatarEntitiesLock.withWriteLock([&] { + staleBlobs = std::move(_staleCachedAvatarEntityBlobs); + }); + int32_t numFound = 0; + for (const auto& id : staleBlobs) { + bool found = false; + EntityItemProperties properties; + entityTree->withReadLock([&] { + EntityItemPointer entity = entityTree->findEntityByID(id); + if (entity) { + properties = entity->getProperties(); + found = true; + } + }); + if (found) { + ++numFound; + QByteArray blob; + propertiesToBlob(*_myScriptEngine, getID(), properties, blob); + _avatarEntitiesLock.withWriteLock([&] { + _cachedAvatarEntityBlobs[id] = blob; + }); + } + } + return true; +} void MyAvatar::rememberToReloadAvatarEntityDataFromSettings() { AvatarEntityMap emptyMap; @@ -1535,7 +1754,84 @@ void MyAvatar::rememberToReloadAvatarEntityDataFromSettings() { _reloadAvatarEntityDataFromSettings = true; } +AvatarEntityMap MyAvatar::getAvatarEntityData() const { + // NOTE: the return value is expected to be a map of unfortunately-formatted-binary-blobs + updateStaleAvatarEntityBlobs(); + AvatarEntityMap result; + _avatarEntitiesLock.withReadLock([&] { + result = _cachedAvatarEntityBlobs; + }); + return result; +} + +void MyAvatar::setAvatarEntityData(const AvatarEntityMap& avatarEntityData) { + // NOTE: the argument is expected to be a map of unfortunately-formatted-binary-blobs + if (avatarEntityData.size() > MAX_NUM_AVATAR_ENTITIES) { + // the data is suspect + qCDebug(interfaceapp) << "discard suspect AvatarEntityData with size =" << avatarEntityData.size(); + return; + } + + _avatarEntitiesLock.withWriteLock([&] { + // find new and updated IDs + AvatarEntityMap::const_iterator constItr = avatarEntityData.begin(); + while (constItr != avatarEntityData.end()) { + QUuid id = constItr.key(); + if (_cachedAvatarEntityBlobs.find(id) == _cachedAvatarEntityBlobs.end()) { + _entitiesToAdd.push_back(id); + } else { + _entitiesToUpdate.push_back(id); + } + } + // find and erase deleted IDs from _cachedAvatarEntityBlobs + std::vector deletedIDs; + AvatarEntityMap::iterator itr = _cachedAvatarEntityBlobs.begin(); + while (itr != _cachedAvatarEntityBlobs.end()) { + QUuid id = itr.key(); + if (std::find(_entitiesToUpdate.begin(), _entitiesToUpdate.end(), id) == _entitiesToUpdate.end()) { + deletedIDs.push_back(id); + itr = _cachedAvatarEntityBlobs.erase(itr); + } else { + ++itr; + } + } + // erase deleted IDs from _packedAvatarEntityData + for (const auto& id : deletedIDs) { + itr = _packedAvatarEntityData.find(id); + if (itr != _packedAvatarEntityData.end()) { + _packedAvatarEntityData.erase(itr); + } else { + ++itr; + } + _entitiesToDelete.push_back(id); + } + }); +} + +void MyAvatar::avatarEntityDataToJson(QJsonObject& root) const { + _avatarEntitiesLock.withReadLock([&] { + if (!_cachedAvatarEntityBlobs.empty()) { + QJsonArray avatarEntityJson; + int entityCount = 0; + AvatarEntityMap::const_iterator itr = _cachedAvatarEntityBlobs.begin(); + while (itr != _cachedAvatarEntityBlobs.end()) { + QVariantMap entityData; + QUuid id = _avatarEntityForRecording.size() == _cachedAvatarEntityBlobs.size() ? _avatarEntityForRecording.values()[entityCount++] : itr.key(); + entityData.insert("id", id); + entityData.insert("properties", itr.value().toBase64()); + avatarEntityJson.push_back(QVariant(entityData).toJsonObject()); + ++itr; + } + const QString JSON_AVATAR_ENTITIES = QStringLiteral("attachedEntities"); + root[JSON_AVATAR_ENTITIES] = avatarEntityJson; + } + }); +} + void MyAvatar::loadData() { + if (!_myScriptEngine) { + _myScriptEngine = new QScriptEngine(); + } getHead()->setBasePitch(_headPitchSetting.get()); _yawSpeed = _yawSpeedSetting.get(_yawSpeed); @@ -1570,99 +1866,33 @@ void MyAvatar::loadData() { } void MyAvatar::loadAvatarEntityDataFromSettings() { - _avatarEntitiesLock.withReadLock([&] { + _avatarEntitiesLock.withWriteLock([&] { _packedAvatarEntityData.clear(); + _entitiesToDelete.clear(); + _entitiesToAdd.clear(); + _entitiesToUpdate.clear(); }); - _reloadAvatarEntityDataFromSettings = false; + int numEntities = _avatarEntityCountSetting.get(0); if (numEntities == 0) { return; } - - QScriptEngine scriptEngine; - std::vector< std::pair > entitiesToLoad; resizeAvatarEntitySettingHandles(numEntities); - for (int i = 0; i < numEntities; i++) { - QUuid id = QUuid::createUuid(); // generate a new ID - // the avatarEntityData is stored as an unfortunately-formatted-binary-blob - QByteArray binaryData = _avatarEntityDataSettings[i].get(); - - _avatarEntityDataChanged = true; - if (_clientTraitsHandler) { - // we have a client traits handler, so we need to mark this instanced trait as changed - // so that changes will be sent next frame - _clientTraitsHandler->markInstancedTraitUpdated(AvatarTraits::AvatarEntity, id); + _avatarEntitiesLock.withWriteLock([&] { + _entitiesToAdd.reserve(numEntities); + // TODO: build map between old and new IDs so we can restitch parent-child relationships + for (int i = 0; i < numEntities; i++) { + QUuid id = QUuid::createUuid(); // generate a new ID + _cachedAvatarEntityBlobs[id] = _avatarEntityDataSettings[i].get(); + _entitiesToAdd.push_back(id); + // this blob is the "authoritative source" for this AvatarEntity and we want to avoid overwriting it + // (the outgoing update packet will flag it for save-back into the blob) + // which is why we remember its id: to skip its save-back later + _cachedAvatarEntityBlobUpdatesToSkip.push_back(id); } - - // begin recipe to extract EntityItemProperties from unfortunately-formatted-binary-blob - QJsonDocument jsonProperties = QJsonDocument::fromBinaryData(binaryData); - if (!jsonProperties.isObject()) { - qCDebug(interfaceapp) << "bad avatarEntityData json" << QString(binaryData.toHex()); - continue; - } - QVariant variant = jsonProperties.toVariant(); - QVariantMap variantMap = variant.toMap(); - QScriptValue scriptValue = variantMapToScriptValue(variantMap, scriptEngine); - EntityItemProperties properties; - EntityItemPropertiesFromScriptValueHonorReadOnly(scriptValue, properties); - // end recipe - - properties.setEntityHostType(entity::HostType::AVATAR); - properties.setOwningAvatarID(getID()); - - // there's no entity-server to tell us we're the simulation owner, so always set the - // simulationOwner to the owningAvatarID and a high priority. - properties.setSimulationOwner(getID(), AVATAR_ENTITY_SIMULATION_PRIORITY); - - if (properties.getParentID() == AVATAR_SELF_ID) { - properties.setParentID(getID()); - } - - // When grabbing avatar entities, they are parented to the joint moving them, then when un-grabbed - // they go back to the default parent (null uuid). When un-gripped, others saw the entity disappear. - // The thinking here is the local position was noticed as changing, but not the parentID (since it is now - // back to the default), and the entity flew off somewhere. Marking all changed definitely fixes this, - // and seems safe (per Seth). - properties.markAllChanged(); - - entitiesToLoad.push_back({id, properties}); - } - - _cachedAvatarEntityDataSettings.clear(); - auto entityTreeRenderer = qApp->getEntities(); - EntityTreePointer entityTree = entityTreeRenderer ? entityTreeRenderer->getTree() : nullptr; - if (entityTree) { - OctreePacketData packetData(false, AvatarTraits::MAXIMUM_TRAIT_SIZE); - EncodeBitstreamParams params; - EntityTreeElementExtraEncodeDataPointer extra { nullptr }; - uint32_t i = 0; - for (const auto& entry : entitiesToLoad) { - // try to create the entity - entityTree->withWriteLock([&] { - QUuid id = entry.first; - EntityItemPointer entity = entityTree->addEntity(id, entry.second); - if (entity) { - // use the entity to build the data payload - OctreeElement::AppendState appendState = entity->appendEntityData(&packetData, params, extra); - if (appendState == OctreeElement::COMPLETED) { - // only remember an AvatarEntity that successfully loads and can be packed - QByteArray tempArray((const char*)packetData.getUncompressedData(), packetData.getUncompressedSize()); - storeAvatarEntityDataPayload(id, tempArray); - - // only cache things that successfully loaded and packed - // we cache these binaryProperties for later: for when saving back to settings - _cachedAvatarEntityDataSettings[id] = _avatarEntityDataSettings[i].get(); - } - packetData.reset(); - } - }); - ++i; - } - } else { - // TODO? handle this case: try to load AvatatEntityData from settings again later? - } + }); } void MyAvatar::saveAttachmentData(const AttachmentData& attachment) const { @@ -2158,7 +2388,6 @@ void MyAvatar::removeWearableAvatarEntities() { QVariantList MyAvatar::getAvatarEntitiesVariant() { // NOTE: this method is NOT efficient QVariantList avatarEntitiesData; - QScriptEngine scriptEngine; auto treeRenderer = DependencyManager::get(); EntityTreePointer entityTree = treeRenderer ? treeRenderer->getTree() : nullptr; if (entityTree) { @@ -2174,7 +2403,7 @@ QVariantList MyAvatar::getAvatarEntitiesVariant() { desiredProperties += PROP_LOCAL_POSITION; desiredProperties += PROP_LOCAL_ROTATION; EntityItemProperties entityProperties = entity->getProperties(desiredProperties); - QScriptValue scriptProperties = EntityItemPropertiesToScriptValue(&scriptEngine, entityProperties); + QScriptValue scriptProperties = EntityItemPropertiesToScriptValue(_myScriptEngine, entityProperties); avatarEntityData["properties"] = scriptProperties.toVariant(); avatarEntitiesData.append(QVariant(avatarEntityData)); } diff --git a/interface/src/avatar/MyAvatar.h b/interface/src/avatar/MyAvatar.h index 9bbb25a3be..a20908e7cb 100644 --- a/interface/src/avatar/MyAvatar.h +++ b/interface/src/avatar/MyAvatar.h @@ -1210,6 +1210,10 @@ public: */ Q_INVOKABLE void releaseGrab(const QUuid& grabID); + AvatarEntityMap getAvatarEntityData() const override; + void setAvatarEntityData(const AvatarEntityMap& avatarEntityData) override; + void avatarEntityDataToJson(QJsonObject& root) const override; + public slots: /**jsdoc @@ -1410,6 +1414,7 @@ public slots: void storeAvatarEntityDataPayload(const QUuid& entityID, const QByteArray& payload) override; void clearAvatarEntity(const QUuid& entityID, bool requiresRemovalFromTree = true) override; + void sanitizeAvatarEntityProperties(EntityItemProperties& properties) const; /**jsdoc * Set whether or not your avatar mesh is visible. @@ -1621,6 +1626,7 @@ protected: virtual void recalculateChildCauterization() const override; private: + bool updateStaleAvatarEntityBlobs() const; bool requiresSafeLanding(const glm::vec3& positionIn, glm::vec3& positionOut); @@ -1948,9 +1954,38 @@ private: Setting::Handle _allowTeleportingSetting { "allowTeleporting", true }; std::vector> _avatarEntityIDSettings; std::vector> _avatarEntityDataSettings; - std::map _cachedAvatarEntityDataSettings; - std::set _entitiesToSaveToSettings; - std::set _entitiesToRemoveFromSettings; + + // AvatarEntities stuff: + // We cache the "map of unfortunately-formatted-binary-blobs" because they are expensive to compute + // Do not confuse these with AvatarData::_packedAvatarEntityData which are in wire-format. + mutable AvatarEntityMap _cachedAvatarEntityBlobs; + + // We collect changes to AvatarEntities and then handle them all in one spot per frame: updateAvatarEntities(). + // Basically this is a "transaction pattern" with an extra complication: these changes can come from two + // "directions" and the "authoritative source" of each direction is different, so maintain two distinct sets of + // transaction lists; + // + // The _entitiesToDelete/Add/Update lists are for changes whose "authoritative sources" are already + // correctly stored in _cachedAvatarEntityBlobs. These come from loadAvatarEntityDataFromSettings() and + // setAvatarEntityData(). These changes need to be extracted from _cachedAvatarEntityBlobs and applied to + // real EntityItems. + std::vector _entitiesToDelete; + std::vector _entitiesToAdd; + std::vector _entitiesToUpdate; + // + // The _cachedAvatarEntityBlobsToDelete/Add/Update lists are for changes whose "authoritative sources" are + // already reflected in real EntityItems. These changes need to be propagated to _cachedAvatarEntityBlobs + // and eventually to settings. + std::vector _cachedAvatarEntityBlobsToDelete; + std::vector _cachedAvatarEntityBlobsToAddOrUpdate; + std::vector _cachedAvatarEntityBlobUpdatesToSkip; + // + // Also these lists for tracking delayed changes to blobs and Settings + std::set _staleCachedAvatarEntityBlobs; + // + // keep a ScriptEngine around so we don't have to instantiate on the fly (these are very slow to create/delete) + QScriptEngine* _myScriptEngine { nullptr }; + bool _needToSaveAvatarEntitySettings { false }; }; QScriptValue audioListenModeToScriptValue(QScriptEngine* engine, const AudioListenerMode& audioListenerMode); diff --git a/libraries/avatars/src/AvatarData.cpp b/libraries/avatars/src/AvatarData.cpp index 093b86d0e6..3d22c50e1a 100644 --- a/libraries/avatars/src/AvatarData.cpp +++ b/libraries/avatars/src/AvatarData.cpp @@ -2422,22 +2422,7 @@ JointData jointDataFromJsonValue(int version, const QJsonValue& json) { } void AvatarData::avatarEntityDataToJson(QJsonObject& root) const { - // ADEBUG this is broken - _avatarEntitiesLock.withReadLock([&] { - AvatarEntityMap _avatarEntityData = _packedAvatarEntityData; // hack to compile - if (!_avatarEntityData.empty()) { - QJsonArray avatarEntityJson; - int entityCount = 0; - for (auto entityID : _avatarEntityData.keys()) { - QVariantMap entityData; - QUuid newId = _avatarEntityForRecording.size() == _avatarEntityData.size() ? _avatarEntityForRecording.values()[entityCount++] : entityID; - entityData.insert("id", newId); - entityData.insert("properties", _avatarEntityData.value(entityID).toBase64()); - avatarEntityJson.push_back(QVariant(entityData).toJsonObject()); - } - root[JSON_AVATAR_ENTITIES] = avatarEntityJson; - } - }); + // overridden where needed } QJsonObject AvatarData::toJson() const { @@ -2575,7 +2560,6 @@ void AvatarData::fromJson(const QJsonObject& json, bool useFrameSkeleton) { if (attachmentJson.isObject()) { QVariantMap entityData = attachmentJson.toObject().toVariantMap(); QUuid id = entityData.value("id").toUuid(); - // ADEBUG TODO: fix this broken path QByteArray data = QByteArray::fromBase64(entityData.value("properties").toByteArray()); updateAvatarEntityData(id, data); } @@ -2759,8 +2743,6 @@ void AvatarData::setAttachmentsVariant(const QVariantList& variant) { setAttachmentData(newAttachments); } -const int MAX_NUM_AVATAR_ENTITIES = 42; - void AvatarData::storeAvatarEntityDataPayload(const QUuid& entityID, const QByteArray& data) { _avatarEntitiesLock.withWriteLock([&] { PackedAvatarEntityMap::iterator itr = _packedAvatarEntityData.find(entityID); @@ -2787,7 +2769,8 @@ void AvatarData::updateAvatarEntity(const QUuid& id, const QScriptValue& scriptV } void AvatarData::updateAvatarEntityData(const QUuid& id, const QByteArray& data) { - // ADEBUG TODO: implement this + // overridden where needed + // NOTE: expects 'data' to be an unfortunately-formatted-binary-blob } void AvatarData::clearAvatarEntity(const QUuid& entityID, bool requiresRemovalFromTree) { @@ -2808,73 +2791,23 @@ void AvatarData::clearAvatarEntity(const QUuid& entityID, bool requiresRemovalFr } AvatarEntityMap AvatarData::getAvatarEntityData() const { - // ADEBUG broken - AvatarEntityMap result; - _avatarEntitiesLock.withReadLock([&] { - result = _packedAvatarEntityData; - }); - return result; + // overridden where needed + // NOTE: the return value is expected to be a map of unfortunately-formatted-binary-blobs + return AvatarEntityMap(); +} + +void AvatarData::setAvatarEntityData(const AvatarEntityMap& avatarEntityData) { + // overridden where needed + // NOTE: the argument is expected to be a map of unfortunately-formatted-binary-blobs } void AvatarData::insertDetachedEntityID(const QUuid entityID) { _avatarEntitiesLock.withWriteLock([&] { _avatarEntityDetached.insert(entityID); }); - _avatarEntityDataChanged = true; } -void AvatarData::setAvatarEntityData(const AvatarEntityMap& avatarEntityData) { - if (avatarEntityData.size() > MAX_NUM_AVATAR_ENTITIES) { - // the data is suspect - qCDebug(avatars) << "discard suspect AvatarEntityData with size =" << avatarEntityData.size(); - return; - } - - std::vector deletedEntityIDs; - QList updatedEntityIDs; - - // ADEBUG this is broken - _avatarEntitiesLock.withWriteLock([&] { - AvatarEntityMap _avatarEntityData = _packedAvatarEntityData; // hack to compile - if (_avatarEntityData != avatarEntityData) { - - // keep track of entities that were attached to this avatar but no longer are - AvatarEntityIDs previousAvatarEntityIDs = QSet::fromList(_avatarEntityData.keys()); - - _avatarEntityData = avatarEntityData; - setAvatarEntityDataChanged(true); - - deletedEntityIDs.reserve(previousAvatarEntityIDs.size()); - - foreach (auto entityID, previousAvatarEntityIDs) { - if (!_avatarEntityData.contains(entityID)) { - _avatarEntityDetached.insert(entityID); - deletedEntityIDs.push_back(entityID); - } - } - - updatedEntityIDs = _avatarEntityData.keys(); - } - }); - - if (_clientTraitsHandler) { - // we have a client traits handler - - // flag removed entities as deleted so that changes are sent next frame - for (auto& deletedEntityID : deletedEntityIDs) { - _clientTraitsHandler->markInstancedTraitDeleted(AvatarTraits::AvatarEntity, deletedEntityID); - } - - // flag any updated or created entities so that we send changes for them next frame - for (auto& entityID : updatedEntityIDs) { - _clientTraitsHandler->markInstancedTraitUpdated(AvatarTraits::AvatarEntity, entityID); - } - } - - -} - AvatarEntityIDs AvatarData::getAndClearRecentlyDetachedIDs() { AvatarEntityIDs result; _avatarEntitiesLock.withWriteLock([&] { diff --git a/libraries/avatars/src/AvatarData.h b/libraries/avatars/src/AvatarData.h index 1bae3cd32f..787f92f5db 100644 --- a/libraries/avatars/src/AvatarData.h +++ b/libraries/avatars/src/AvatarData.h @@ -72,6 +72,8 @@ using AvatarGrabMap = QMap; using AvatarDataSequenceNumber = uint16_t; +const int MAX_NUM_AVATAR_ENTITIES = 42; + // avatar motion behaviors const quint32 AVATAR_MOTION_ACTION_MOTOR_ENABLED = 1U << 0; const quint32 AVATAR_MOTION_SCRIPTED_MOTOR_ENABLED = 1U << 1; @@ -1128,7 +1130,7 @@ public: TransformPointer getRecordingBasis() const; void setRecordingBasis(TransformPointer recordingBasis = TransformPointer()); void createRecordingIDs(); - void avatarEntityDataToJson(QJsonObject& root) const; + virtual void avatarEntityDataToJson(QJsonObject& root) const; QJsonObject toJson() const; void fromJson(const QJsonObject& json, bool useFrameSkeleton = true); @@ -1140,13 +1142,13 @@ public: * @function MyAvatar.getAvatarEntityData * @returns {object} */ - Q_INVOKABLE AvatarEntityMap getAvatarEntityData() const; + Q_INVOKABLE virtual AvatarEntityMap getAvatarEntityData() const; /**jsdoc * @function MyAvatar.setAvatarEntityData * @param {object} avatarEntityData */ - Q_INVOKABLE void setAvatarEntityData(const AvatarEntityMap& avatarEntityData); + Q_INVOKABLE virtual void setAvatarEntityData(const AvatarEntityMap& avatarEntityData); virtual void setAvatarEntityDataChanged(bool value) { _avatarEntityDataChanged = value; } void insertDetachedEntityID(const QUuid entityID); From cbda905e3b3541f154ffe1c5298ac6606e41b8f1 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Fri, 14 Dec 2018 15:33:37 -0800 Subject: [PATCH 29/43] fix typo --- interface/src/Application.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 0a49afa95f..1a4d2db83e 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -2460,7 +2460,7 @@ void Application::updateHeartbeat() const { void Application::onAboutToQuit() { // quickly save AvatarEntityData before the EntityTree is dismantled - getMyAvatar()->saveAvatarDataToSettings(); + getMyAvatar()->saveAvatarEntityDataToSettings(); emit beforeAboutToQuit(); From a4be4fb6c0a492c1aadb71700bf1566660873124 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Fri, 14 Dec 2018 17:34:47 -0800 Subject: [PATCH 30/43] MyAvatar.setAvatarEntityData() works --- .../src/avatars/ScriptableAvatar.cpp | 4 +++- .../src/avatars/ScriptableAvatar.h | 2 +- interface/src/avatar/MyAvatar.cpp | 17 +++++++++++++---- interface/src/avatar/MyAvatar.h | 2 -- .../src/avatars-renderer/Avatar.h | 2 -- libraries/avatars/src/AvatarData.cpp | 8 ++------ libraries/avatars/src/AvatarData.h | 5 ++--- 7 files changed, 21 insertions(+), 19 deletions(-) diff --git a/assignment-client/src/avatars/ScriptableAvatar.cpp b/assignment-client/src/avatars/ScriptableAvatar.cpp index 25cf702b22..8e88d6c236 100644 --- a/assignment-client/src/avatars/ScriptableAvatar.cpp +++ b/assignment-client/src/avatars/ScriptableAvatar.cpp @@ -252,7 +252,8 @@ void ScriptableAvatar::setHasAudioEnabledFaceMovement(bool hasAudioEnabledFaceMo _headData->setHasAudioEnabledFaceMovement(hasAudioEnabledFaceMovement); } -void ScriptableAvatar::updateAvatarEntity(const QUuid& id, const QScriptValue& data) { +void ScriptableAvatar::updateAvatarEntity(const QUuid& id, const QByteArray& data) { + /* TODO: fix this if (data.isNull()) { // interpret this as a DELETE std::map::iterator itr = _entities.find(id); @@ -299,4 +300,5 @@ void ScriptableAvatar::updateAvatarEntity(const QUuid& id, const QScriptValue& d } } } + */ } diff --git a/assignment-client/src/avatars/ScriptableAvatar.h b/assignment-client/src/avatars/ScriptableAvatar.h index f56d7c66b0..b0e41d28eb 100644 --- a/assignment-client/src/avatars/ScriptableAvatar.h +++ b/assignment-client/src/avatars/ScriptableAvatar.h @@ -185,7 +185,7 @@ public: bool getHasProceduralEyeFaceMovement() const override { return _headData->getHasProceduralEyeFaceMovement(); } void setHasAudioEnabledFaceMovement(bool hasAudioEnabledFaceMovement); bool getHasAudioEnabledFaceMovement() const override { return _headData->getHasAudioEnabledFaceMovement(); } - void updateAvatarEntity(const QUuid& id, const QScriptValue& data) override; + void updateAvatarEntity(const QUuid& id, const QByteArray& data) override; public slots: void update(float deltatime); diff --git a/interface/src/avatar/MyAvatar.cpp b/interface/src/avatar/MyAvatar.cpp index 051aab4cdf..a59560860d 100755 --- a/interface/src/avatar/MyAvatar.cpp +++ b/interface/src/avatar/MyAvatar.cpp @@ -1772,16 +1772,23 @@ void MyAvatar::setAvatarEntityData(const AvatarEntityMap& avatarEntityData) { return; } + if (!avatarEntityData.empty() && !_cachedAvatarEntityBlobs.empty()) { + _needToSaveAvatarEntitySettings = true; + } _avatarEntitiesLock.withWriteLock([&] { // find new and updated IDs AvatarEntityMap::const_iterator constItr = avatarEntityData.begin(); + std::vector blobsToCache; + blobsToCache.reserve(avatarEntityData.size()); while (constItr != avatarEntityData.end()) { QUuid id = constItr.key(); if (_cachedAvatarEntityBlobs.find(id) == _cachedAvatarEntityBlobs.end()) { _entitiesToAdd.push_back(id); + blobsToCache.push_back(id); } else { _entitiesToUpdate.push_back(id); } + ++constItr; } // find and erase deleted IDs from _cachedAvatarEntityBlobs std::vector deletedIDs; @@ -1795,6 +1802,12 @@ void MyAvatar::setAvatarEntityData(const AvatarEntityMap& avatarEntityData) { ++itr; } } + // now that we've 'deleted' unknown ids, copy over the new ones + constItr = avatarEntityData.begin(); + while (constItr != avatarEntityData.end()) { + _cachedAvatarEntityBlobs.insert(constItr.key(), constItr.value()); + ++constItr; + } // erase deleted IDs from _packedAvatarEntityData for (const auto& id : deletedIDs) { itr = _packedAvatarEntityData.find(id); @@ -2296,10 +2309,6 @@ void MyAvatar::clearJointsData() { _skeletonModel->getRig().clearJointStates(); } -void MyAvatar::updateAvatarEntity(const QUuid& id, const QScriptValue& data) { - // TODO: implement this -} - void MyAvatar::setSkeletonModelURL(const QUrl& skeletonModelURL) { _skeletonModelChangeCount++; int skeletonModelChangeCount = _skeletonModelChangeCount; diff --git a/interface/src/avatar/MyAvatar.h b/interface/src/avatar/MyAvatar.h index a20908e7cb..3804df977e 100644 --- a/interface/src/avatar/MyAvatar.h +++ b/interface/src/avatar/MyAvatar.h @@ -851,8 +851,6 @@ public: virtual void clearJointData(const QString& name) override; virtual void clearJointsData() override; - void updateAvatarEntity(const QUuid& id, const QScriptValue& data) override; - /**jsdoc * @function MyAvatar.pinJoint * @param {number} index diff --git a/libraries/avatars-renderer/src/avatars-renderer/Avatar.h b/libraries/avatars-renderer/src/avatars-renderer/Avatar.h index 2b356675be..97218ac3c1 100644 --- a/libraries/avatars-renderer/src/avatars-renderer/Avatar.h +++ b/libraries/avatars-renderer/src/avatars-renderer/Avatar.h @@ -235,8 +235,6 @@ public: void updateDisplayNameAlpha(bool showDisplayName); virtual void setSessionDisplayName(const QString& sessionDisplayName) override { }; // no-op - virtual void updateAvatarEntity(const QUuid& entityID, const QScriptValue& entityData) override { }; // no-op - virtual int parseDataFromBuffer(const QByteArray& buffer) override; static void renderJointConnectingCone(gpu::Batch& batch, glm::vec3 position1, glm::vec3 position2, diff --git a/libraries/avatars/src/AvatarData.cpp b/libraries/avatars/src/AvatarData.cpp index 3d22c50e1a..e0f2b02d51 100644 --- a/libraries/avatars/src/AvatarData.cpp +++ b/libraries/avatars/src/AvatarData.cpp @@ -2561,7 +2561,7 @@ void AvatarData::fromJson(const QJsonObject& json, bool useFrameSkeleton) { QVariantMap entityData = attachmentJson.toObject().toVariantMap(); QUuid id = entityData.value("id").toUuid(); QByteArray data = QByteArray::fromBase64(entityData.value("properties").toByteArray()); - updateAvatarEntityData(id, data); + updateAvatarEntity(id, data); } } } @@ -2764,11 +2764,7 @@ void AvatarData::storeAvatarEntityDataPayload(const QUuid& entityID, const QByte } } -void AvatarData::updateAvatarEntity(const QUuid& id, const QScriptValue& scriptValue) { - // overridden where needed -} - -void AvatarData::updateAvatarEntityData(const QUuid& id, const QByteArray& data) { +void AvatarData::updateAvatarEntity(const QUuid& entityID, const QByteArray& entityData) { // overridden where needed // NOTE: expects 'data' to be an unfortunately-formatted-binary-blob } diff --git a/libraries/avatars/src/AvatarData.h b/libraries/avatars/src/AvatarData.h index 787f92f5db..0fea5a541e 100644 --- a/libraries/avatars/src/AvatarData.h +++ b/libraries/avatars/src/AvatarData.h @@ -960,10 +960,9 @@ public: /**jsdoc * @function MyAvatar.updateAvatarEntity * @param {Uuid} entityID - * @param {object} entityData + * @param {string} entityData */ - Q_INVOKABLE virtual void updateAvatarEntity(const QUuid& id, const QScriptValue& scriptValue); - virtual void updateAvatarEntityData(const QUuid& id, const QByteArray& data); + Q_INVOKABLE virtual void updateAvatarEntity(const QUuid& entityID, const QByteArray& entityData); /**jsdoc * @function MyAvatar.clearAvatarEntity From 855497e6044439c10a90b6bc62ffa1328af79456 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Sat, 15 Dec 2018 06:24:30 -0800 Subject: [PATCH 31/43] MyAvatar.updateAvatarEntity(id, data) works in theory --- interface/src/avatar/MyAvatar.cpp | 37 +++++++++++++++++++++++++--- interface/src/avatar/MyAvatar.h | 1 + libraries/avatars/src/AvatarData.cpp | 2 +- 3 files changed, 35 insertions(+), 5 deletions(-) diff --git a/interface/src/avatar/MyAvatar.cpp b/interface/src/avatar/MyAvatar.cpp index a59560860d..582bf1aea7 100755 --- a/interface/src/avatar/MyAvatar.cpp +++ b/interface/src/avatar/MyAvatar.cpp @@ -1513,6 +1513,7 @@ void MyAvatar::sanitizeAvatarEntityProperties(EntityItemProperties& properties) } void MyAvatar::updateAvatarEntities() { + // NOTE: this is a per-frame update if (getID().isNull() || getID() == AVATAR_SELF_ID || DependencyManager::get()->getSessionUUID() == QUuid()) { @@ -1765,16 +1766,25 @@ AvatarEntityMap MyAvatar::getAvatarEntityData() const { } void MyAvatar::setAvatarEntityData(const AvatarEntityMap& avatarEntityData) { - // NOTE: the argument is expected to be a map of unfortunately-formatted-binary-blobs + // Note: this is an invokable Script call + // The argument is expected to be a map of QByteArrays that represent EntityItemProperties objects from JavaScript, + // aka: unfortunately-formatted-binary-blobs because we store them in non-human-readable format in Settings. + // if (avatarEntityData.size() > MAX_NUM_AVATAR_ENTITIES) { // the data is suspect qCDebug(interfaceapp) << "discard suspect AvatarEntityData with size =" << avatarEntityData.size(); return; } - if (!avatarEntityData.empty() && !_cachedAvatarEntityBlobs.empty()) { - _needToSaveAvatarEntitySettings = true; - } + // this overwrites ALL AvatarEntityData so we clear pending operations + _avatarEntitiesLock.withWriteLock([&] { + _packedAvatarEntityData.clear(); + _entitiesToDelete.clear(); + _entitiesToAdd.clear(); + _entitiesToUpdate.clear(); + }); + _needToSaveAvatarEntitySettings = true; + _avatarEntitiesLock.withWriteLock([&] { // find new and updated IDs AvatarEntityMap::const_iterator constItr = avatarEntityData.begin(); @@ -1821,6 +1831,23 @@ void MyAvatar::setAvatarEntityData(const AvatarEntityMap& avatarEntityData) { }); } +void MyAvatar::updateAvatarEntity(const QUuid& entityID, const QByteArray& entityData) { + // NOTE: this is an invokable Script call + // TODO: we should handle the case where entityData is corrupt or invalid + // BEFORE we store into _cachedAvatarEntityBlobs + _needToSaveAvatarEntitySettings = true; + _avatarEntitiesLock.withWriteLock([&] { + AvatarEntityMap::iterator itr = _cachedAvatarEntityBlobs.find(entityID); + if (itr != _cachedAvatarEntityBlobs.end()) { + _entitiesToUpdate.push_back(entityID); + itr.value() = entityData; + } else { + _entitiesToAdd.push_back(entityID); + _cachedAvatarEntityBlobs.insert(entityID, entityData); + } + }); +} + void MyAvatar::avatarEntityDataToJson(QJsonObject& root) const { _avatarEntitiesLock.withReadLock([&] { if (!_cachedAvatarEntityBlobs.empty()) { @@ -1879,6 +1906,7 @@ void MyAvatar::loadData() { } void MyAvatar::loadAvatarEntityDataFromSettings() { + // this overwrites ALL AvatarEntityData so we clear pending operations _avatarEntitiesLock.withWriteLock([&] { _packedAvatarEntityData.clear(); _entitiesToDelete.clear(); @@ -1886,6 +1914,7 @@ void MyAvatar::loadAvatarEntityDataFromSettings() { _entitiesToUpdate.clear(); }); _reloadAvatarEntityDataFromSettings = false; + _needToSaveAvatarEntitySettings = false; int numEntities = _avatarEntityCountSetting.get(0); if (numEntities == 0) { diff --git a/interface/src/avatar/MyAvatar.h b/interface/src/avatar/MyAvatar.h index 3804df977e..44f0c29b0b 100644 --- a/interface/src/avatar/MyAvatar.h +++ b/interface/src/avatar/MyAvatar.h @@ -1210,6 +1210,7 @@ public: AvatarEntityMap getAvatarEntityData() const override; void setAvatarEntityData(const AvatarEntityMap& avatarEntityData) override; + void updateAvatarEntity(const QUuid& entityID, const QByteArray& entityData) override; void avatarEntityDataToJson(QJsonObject& root) const override; public slots: diff --git a/libraries/avatars/src/AvatarData.cpp b/libraries/avatars/src/AvatarData.cpp index e0f2b02d51..4f50333505 100644 --- a/libraries/avatars/src/AvatarData.cpp +++ b/libraries/avatars/src/AvatarData.cpp @@ -2766,7 +2766,7 @@ void AvatarData::storeAvatarEntityDataPayload(const QUuid& entityID, const QByte void AvatarData::updateAvatarEntity(const QUuid& entityID, const QByteArray& entityData) { // overridden where needed - // NOTE: expects 'data' to be an unfortunately-formatted-binary-blob + // expects 'entityData' to be a JavaScript EntityItemProperties Object in QByteArray form } void AvatarData::clearAvatarEntity(const QUuid& entityID, bool requiresRemovalFromTree) { From 63ed0a3a98545bb18b67d47ce18bff612dfa0896 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Mon, 17 Dec 2018 10:01:52 -0800 Subject: [PATCH 32/43] move blob conversion recipes to EntityItemProperties --- interface/src/avatar/MyAvatar.cpp | 45 +++---------------- libraries/avatars/src/AvatarData.cpp | 3 +- .../entities/src/EntityItemProperties.cpp | 32 +++++++++++++ libraries/entities/src/EntityItemProperties.h | 3 ++ 4 files changed, 42 insertions(+), 41 deletions(-) diff --git a/interface/src/avatar/MyAvatar.cpp b/interface/src/avatar/MyAvatar.cpp index 582bf1aea7..18494e2c42 100755 --- a/interface/src/avatar/MyAvatar.cpp +++ b/interface/src/avatar/MyAvatar.cpp @@ -1460,38 +1460,6 @@ void MyAvatar::clearAvatarEntity(const QUuid& entityID, bool requiresRemovalFrom }); } -bool blobToProperties(QScriptEngine& scriptEngine, const QByteArray& blob, EntityItemProperties& properties) { - // begin recipe for converting unfortunately-formatted-binary-blob to EntityItemProperties - QJsonDocument jsonProperties = QJsonDocument::fromBinaryData(blob); - if (!jsonProperties.isObject()) { - qCDebug(interfaceapp) << "bad avatarEntityData json" << QString(blob.toHex()); - return false; - } - QVariant variant = jsonProperties.toVariant(); - QVariantMap variantMap = variant.toMap(); - QScriptValue scriptValue = variantMapToScriptValue(variantMap, scriptEngine); - EntityItemPropertiesFromScriptValueHonorReadOnly(scriptValue, properties); - // end recipe - return true; -} - -void propertiesToBlob(QScriptEngine& scriptEngine, const QUuid& myAvatarID, const EntityItemProperties& properties, QByteArray& blob) { - // begin recipe for extracting unfortunately-formatted-binary-blob from EntityItem - QScriptValue scriptValue = EntityItemNonDefaultPropertiesToScriptValue(&scriptEngine, properties); - QVariant variantProperties = scriptValue.toVariant(); - QJsonDocument jsonProperties = QJsonDocument::fromVariant(variantProperties); - // the ID of the parent/avatar changes from session to session. use a special UUID to indicate the avatar - QJsonObject jsonObject = jsonProperties.object(); - if (jsonObject.contains("parentID")) { - if (QUuid(jsonObject["parentID"].toString()) == myAvatarID) { - jsonObject["parentID"] = AVATAR_SELF_ID.toString(); - } - } - jsonProperties = QJsonDocument(jsonObject); - blob = jsonProperties.toBinaryData(); - // end recipe -} - void MyAvatar::sanitizeAvatarEntityProperties(EntityItemProperties& properties) const { properties.setEntityHostType(entity::HostType::AVATAR); properties.setOwningAvatarID(getID()); @@ -1605,7 +1573,7 @@ void MyAvatar::updateAvatarEntities() { blobFailed = true; // blob doesn't exist return; } - if (!blobToProperties(*_myScriptEngine, itr.value(), properties)) { + if (!EntityItemProperties::blobToProperties(*_myScriptEngine, itr.value(), properties)) { blobFailed = true; // blob is corrupt } }); @@ -1634,7 +1602,7 @@ void MyAvatar::updateAvatarEntities() { skip = true; return; } - if (!blobToProperties(*_myScriptEngine, itr.value(), properties)) { + if (!EntityItemProperties::blobToProperties(*_myScriptEngine, itr.value(), properties)) { skip = true; } }); @@ -1740,7 +1708,7 @@ bool MyAvatar::updateStaleAvatarEntityBlobs() const { if (found) { ++numFound; QByteArray blob; - propertiesToBlob(*_myScriptEngine, getID(), properties, blob); + EntityItemProperties::propertiesToBlob(*_myScriptEngine, getID(), properties, blob); _avatarEntitiesLock.withWriteLock([&] { _cachedAvatarEntityBlobs[id] = blob; }); @@ -1767,7 +1735,7 @@ AvatarEntityMap MyAvatar::getAvatarEntityData() const { void MyAvatar::setAvatarEntityData(const AvatarEntityMap& avatarEntityData) { // Note: this is an invokable Script call - // The argument is expected to be a map of QByteArrays that represent EntityItemProperties objects from JavaScript, + // avatarEntityData is expected to be a map of QByteArrays that represent EntityItemProperties objects from JavaScript, // aka: unfortunately-formatted-binary-blobs because we store them in non-human-readable format in Settings. // if (avatarEntityData.size() > MAX_NUM_AVATAR_ENTITIES) { @@ -1788,13 +1756,10 @@ void MyAvatar::setAvatarEntityData(const AvatarEntityMap& avatarEntityData) { _avatarEntitiesLock.withWriteLock([&] { // find new and updated IDs AvatarEntityMap::const_iterator constItr = avatarEntityData.begin(); - std::vector blobsToCache; - blobsToCache.reserve(avatarEntityData.size()); while (constItr != avatarEntityData.end()) { QUuid id = constItr.key(); if (_cachedAvatarEntityBlobs.find(id) == _cachedAvatarEntityBlobs.end()) { _entitiesToAdd.push_back(id); - blobsToCache.push_back(id); } else { _entitiesToUpdate.push_back(id); } @@ -1812,7 +1777,7 @@ void MyAvatar::setAvatarEntityData(const AvatarEntityMap& avatarEntityData) { ++itr; } } - // now that we've 'deleted' unknown ids, copy over the new ones + // copy new data constItr = avatarEntityData.begin(); while (constItr != avatarEntityData.end()) { _cachedAvatarEntityBlobs.insert(constItr.key(), constItr.value()); diff --git a/libraries/avatars/src/AvatarData.cpp b/libraries/avatars/src/AvatarData.cpp index 4f50333505..5eda53329b 100644 --- a/libraries/avatars/src/AvatarData.cpp +++ b/libraries/avatars/src/AvatarData.cpp @@ -2794,7 +2794,8 @@ AvatarEntityMap AvatarData::getAvatarEntityData() const { void AvatarData::setAvatarEntityData(const AvatarEntityMap& avatarEntityData) { // overridden where needed - // NOTE: the argument is expected to be a map of unfortunately-formatted-binary-blobs + // avatarEntityData is expected to be a map of QByteArrays + // each QByteArray represents an EntityItemProperties object from JavaScript } void AvatarData::insertDetachedEntityID(const QUuid entityID) { diff --git a/libraries/entities/src/EntityItemProperties.cpp b/libraries/entities/src/EntityItemProperties.cpp index 2c8548033f..d466de0507 100644 --- a/libraries/entities/src/EntityItemProperties.cpp +++ b/libraries/entities/src/EntityItemProperties.cpp @@ -4612,6 +4612,38 @@ void EntityItemProperties::convertToCloneProperties(const EntityItemID& entityID setCloneAvatarEntity(ENTITY_ITEM_DEFAULT_CLONE_AVATAR_ENTITY); } +bool EntityItemProperties::blobToProperties(QScriptEngine& scriptEngine, const QByteArray& blob, EntityItemProperties& properties) { + // begin recipe for converting unfortunately-formatted-binary-blob to EntityItemProperties + QJsonDocument jsonProperties = QJsonDocument::fromBinaryData(blob); + if (!jsonProperties.isObject()) { + qCDebug(entities) << "bad avatarEntityData json" << QString(blob.toHex()); + return false; + } + QVariant variant = jsonProperties.toVariant(); + QVariantMap variantMap = variant.toMap(); + QScriptValue scriptValue = variantMapToScriptValue(variantMap, scriptEngine); + EntityItemPropertiesFromScriptValueHonorReadOnly(scriptValue, properties); + // end recipe + return true; +} + +void EntityItemProperties::propertiesToBlob(QScriptEngine& scriptEngine, const QUuid& myAvatarID, const EntityItemProperties& properties, QByteArray& blob) { + // begin recipe for extracting unfortunately-formatted-binary-blob from EntityItem + QScriptValue scriptValue = EntityItemNonDefaultPropertiesToScriptValue(&scriptEngine, properties); + QVariant variantProperties = scriptValue.toVariant(); + QJsonDocument jsonProperties = QJsonDocument::fromVariant(variantProperties); + // the ID of the parent/avatar changes from session to session. use a special UUID to indicate the avatar + QJsonObject jsonObject = jsonProperties.object(); + if (jsonObject.contains("parentID")) { + if (QUuid(jsonObject["parentID"].toString()) == myAvatarID) { + jsonObject["parentID"] = AVATAR_SELF_ID.toString(); + } + } + jsonProperties = QJsonDocument(jsonObject); + blob = jsonProperties.toBinaryData(); + // end recipe +} + QDebug& operator<<(QDebug& dbg, const EntityPropertyFlags& f) { QString result = "[ "; diff --git a/libraries/entities/src/EntityItemProperties.h b/libraries/entities/src/EntityItemProperties.h index 48f4aa333e..bb4d8c5878 100644 --- a/libraries/entities/src/EntityItemProperties.h +++ b/libraries/entities/src/EntityItemProperties.h @@ -98,6 +98,9 @@ class EntityItemProperties { friend class ZoneEntityItem; friend class MaterialEntityItem; public: + static bool blobToProperties(QScriptEngine& scriptEngine, const QByteArray& blob, EntityItemProperties& properties); + static void propertiesToBlob(QScriptEngine& scriptEngine, const QUuid& myAvatarID, const EntityItemProperties& properties, QByteArray& blob); + EntityItemProperties(EntityPropertyFlags desiredProperties = EntityPropertyFlags()); virtual ~EntityItemProperties() = default; From c998ddbb9e7e56826ae59b68d88d50229e092944 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Mon, 17 Dec 2018 17:11:27 -0800 Subject: [PATCH 33/43] ScriptableAvatar::setAvatarEntityData() works --- .../src/avatars/ScriptableAvatar.cpp | 165 ++++++++++++++---- .../src/avatars/ScriptableAvatar.h | 22 ++- interface/src/avatar/MyAvatar.cpp | 48 ++--- .../src/avatars-renderer/Avatar.cpp | 27 +-- .../entities/src/EntityItemProperties.cpp | 2 + 5 files changed, 202 insertions(+), 62 deletions(-) diff --git a/assignment-client/src/avatars/ScriptableAvatar.cpp b/assignment-client/src/avatars/ScriptableAvatar.cpp index 8e88d6c236..c61e41fbbe 100644 --- a/assignment-client/src/avatars/ScriptableAvatar.cpp +++ b/assignment-client/src/avatars/ScriptableAvatar.cpp @@ -252,26 +252,79 @@ void ScriptableAvatar::setHasAudioEnabledFaceMovement(bool hasAudioEnabledFaceMo _headData->setHasAudioEnabledFaceMovement(hasAudioEnabledFaceMovement); } -void ScriptableAvatar::updateAvatarEntity(const QUuid& id, const QByteArray& data) { - /* TODO: fix this - if (data.isNull()) { - // interpret this as a DELETE - std::map::iterator itr = _entities.find(id); - if (itr != _entities.end()) { - _entities.erase(itr); - clearAvatarEntity(id); +AvatarEntityMap ScriptableAvatar::getAvatarEntityData() const { + // DANGER: Now that we store the AvatarEntityData in packed format this call is potentially Very Expensive! + // Avoid calling this method if possible. + AvatarEntityMap data; + QUuid sessionID = getID(); + _avatarEntitiesLock.withReadLock([&] { + for (const auto& itr : _entities) { + QUuid id = itr.first; + EntityItemPointer entity = itr.second; + EntityItemProperties properties = entity->getProperties(); + QByteArray blob; + EntityItemProperties::propertiesToBlob(_scriptEngine, sessionID, properties, blob); + data[id] = blob; } - } else { - EntityItemPointer entity; - EntityItemProperties properties; - bool honorReadOnly = true; - properties.copyFromScriptValue(data, honorReadOnly); + }); + return data; +} - std::map::iterator itr = _entities.find(id); - if (itr == _entities.end()) { - // this is an ADD - entity = EntityTypes::constructEntityItem(id, properties); +void ScriptableAvatar::setAvatarEntityData(const AvatarEntityMap& avatarEntityData) { + // Note: this is an invokable Script call + // avatarEntityData is expected to be a map of QByteArrays that represent EntityItemProperties objects from JavaScript + // + if (avatarEntityData.size() > MAX_NUM_AVATAR_ENTITIES) { + // the data is suspect + qCDebug(avatars) << "discard suspect avatarEntityData with size =" << avatarEntityData.size(); + return; + } + + // convert binary data to EntityItemProperties + // NOTE: this operation is NOT efficient + std::map newProperties; + AvatarEntityMap::const_iterator dataItr = avatarEntityData.begin(); + while (dataItr != avatarEntityData.end()) { + EntityItemProperties properties; + const QByteArray& blob = dataItr.value(); + if (!blob.isNull() && EntityItemProperties::blobToProperties(_scriptEngine, blob, properties)) { + newProperties[dataItr.key()] = properties; + } + ++dataItr; + } + + // delete existing entities not found in avatarEntityData + std::vector idsToClear; + _avatarEntitiesLock.withWriteLock([&] { + std::map::iterator entityItr = _entities.begin(); + while (entityItr != _entities.end()) { + QUuid id = entityItr->first; + std::map::const_iterator propertiesItr = newProperties.find(id); + if (propertiesItr == newProperties.end()) { + idsToClear.push_back(id); + entityItr = _entities.erase(entityItr); + } else { + ++entityItr; + } + } + }); + + // add or update entities + _avatarEntitiesLock.withWriteLock([&] { + std::map::const_iterator propertiesItr = newProperties.begin(); + while (propertiesItr != newProperties.end()) { + QUuid id = propertiesItr->first; + const EntityItemProperties& properties = propertiesItr->second; + std::map::iterator entityItr = _entities.find(id); + EntityItemPointer entity; + if (entityItr != _entities.end()) { + entity = entityItr->second; + entity->setProperties(properties); + } else { + entity = EntityTypes::constructEntityItem(id, properties); + } if (entity) { + // build outgoing payload OctreePacketData packetData(false, AvatarTraits::MAXIMUM_TRAIT_SIZE); EncodeBitstreamParams params; EntityTreeElementExtraEncodeDataPointer extra { nullptr }; @@ -281,24 +334,74 @@ void ScriptableAvatar::updateAvatarEntity(const QUuid& id, const QByteArray& dat _entities[id] = entity; QByteArray tempArray((const char*)packetData.getUncompressedData(), packetData.getUncompressedSize()); storeAvatarEntityDataPayload(id, tempArray); + } else { + // payload doesn't fit + entityItr = _entities.find(id); + if (entityItr != _entities.end()) { + _entities.erase(entityItr); + idsToClear.push_back(id); + } + } } - } else { - // this is an UPDATE - entity = itr->second; - bool somethingChanged = entity->setProperties(properties); - if (somethingChanged) { - OctreePacketData packetData(false, AvatarTraits::MAXIMUM_TRAIT_SIZE); - EncodeBitstreamParams params; - EntityTreeElementExtraEncodeDataPointer extra { nullptr }; - OctreeElement::AppendState appendState = entity->appendEntityData(&packetData, params, extra); + ++propertiesItr; + } + }); - if (appendState == OctreeElement::COMPLETED) { - QByteArray tempArray((const char*)packetData.getUncompressedData(), packetData.getUncompressedSize()); - storeAvatarEntityDataPayload(id, tempArray); - } + // clear deleted traits + for (const auto& id : idsToClear) { + clearAvatarEntity(id); + } +} + +void ScriptableAvatar::updateAvatarEntity(const QUuid& entityID, const QByteArray& entityData) { + if (entityData.isNull()) { + // interpret this as a DELETE + std::map::iterator itr = _entities.find(entityID); + if (itr != _entities.end()) { + _entities.erase(itr); + clearAvatarEntity(entityID); + } + return; + } + + EntityItemPointer entity; + EntityItemProperties properties; + if (!EntityItemProperties::blobToProperties(_scriptEngine, entityData, properties)) { + // entityData is corrupt + return; + } + + std::map::iterator itr = _entities.find(entityID); + if (itr == _entities.end()) { + // this is an ADD + entity = EntityTypes::constructEntityItem(entityID, properties); + if (entity) { + OctreePacketData packetData(false, AvatarTraits::MAXIMUM_TRAIT_SIZE); + EncodeBitstreamParams params; + EntityTreeElementExtraEncodeDataPointer extra { nullptr }; + OctreeElement::AppendState appendState = entity->appendEntityData(&packetData, params, extra); + + if (appendState == OctreeElement::COMPLETED) { + _entities[entityID] = entity; + QByteArray tempArray((const char*)packetData.getUncompressedData(), packetData.getUncompressedSize()); + storeAvatarEntityDataPayload(entityID, tempArray); + } + } + } else { + // this is an UPDATE + entity = itr->second; + bool somethingChanged = entity->setProperties(properties); + if (somethingChanged) { + OctreePacketData packetData(false, AvatarTraits::MAXIMUM_TRAIT_SIZE); + EncodeBitstreamParams params; + EntityTreeElementExtraEncodeDataPointer extra { nullptr }; + OctreeElement::AppendState appendState = entity->appendEntityData(&packetData, params, extra); + + if (appendState == OctreeElement::COMPLETED) { + QByteArray tempArray((const char*)packetData.getUncompressedData(), packetData.getUncompressedSize()); + storeAvatarEntityDataPayload(entityID, tempArray); } } } - */ } diff --git a/assignment-client/src/avatars/ScriptableAvatar.h b/assignment-client/src/avatars/ScriptableAvatar.h index b0e41d28eb..df949f8bff 100644 --- a/assignment-client/src/avatars/ScriptableAvatar.h +++ b/assignment-client/src/avatars/ScriptableAvatar.h @@ -185,7 +185,26 @@ public: bool getHasProceduralEyeFaceMovement() const override { return _headData->getHasProceduralEyeFaceMovement(); } void setHasAudioEnabledFaceMovement(bool hasAudioEnabledFaceMovement); bool getHasAudioEnabledFaceMovement() const override { return _headData->getHasAudioEnabledFaceMovement(); } - void updateAvatarEntity(const QUuid& id, const QByteArray& data) override; + + /**jsdoc + * Potentially Very Expensive. Do not use. + * @function Avatar.getAvatarEntityData + * @returns {object} + */ + Q_INVOKABLE AvatarEntityMap getAvatarEntityData() const override; + + /**jsdoc + * @function MyAvatar.setAvatarEntityData + * @param {object} avatarEntityData + */ + Q_INVOKABLE void setAvatarEntityData(const AvatarEntityMap& avatarEntityData) override; + + /**jsdoc + * @function MyAvatar.updateAvatarEntity + * @param {Uuid} entityID + * @param {string} entityData + */ + Q_INVOKABLE void updateAvatarEntity(const QUuid& entityID, const QByteArray& entityData) override; public slots: void update(float deltatime); @@ -204,6 +223,7 @@ private: QHash _fstJointIndices; ///< 1-based, since zero is returned for missing keys QStringList _fstJointNames; ///< in order of depth-first traversal QUrl _skeletonFBXURL; + mutable QScriptEngine _scriptEngine; std::map _entities; /// Loads the joint indices, names from the FST file (if any) diff --git a/interface/src/avatar/MyAvatar.cpp b/interface/src/avatar/MyAvatar.cpp index 18494e2c42..67b933e18a 100755 --- a/interface/src/avatar/MyAvatar.cpp +++ b/interface/src/avatar/MyAvatar.cpp @@ -2353,8 +2353,11 @@ void MyAvatar::clearAvatarEntities() { auto treeRenderer = DependencyManager::get(); EntityTreePointer entityTree = treeRenderer ? treeRenderer->getTree() : nullptr; - AvatarEntityMap avatarEntities = getAvatarEntityData(); - for (auto entityID : avatarEntities.keys()) { + QList avatarEntityIDs; + _avatarEntitiesLock.withReadLock([&] { + avatarEntityIDs = _packedAvatarEntityData.keys(); + }); + for (const auto& entityID : avatarEntityIDs) { entityTree->withWriteLock([&entityID, &entityTree] { // remove this entity first from the entity tree entityTree->deleteEntity(entityID, true, true); @@ -2369,10 +2372,12 @@ void MyAvatar::clearAvatarEntities() { void MyAvatar::removeWearableAvatarEntities() { auto treeRenderer = DependencyManager::get(); EntityTreePointer entityTree = treeRenderer ? treeRenderer->getTree() : nullptr; - if (entityTree) { - AvatarEntityMap avatarEntities = getAvatarEntityData(); - for (auto entityID : avatarEntities.keys()) { + QList avatarEntityIDs; + _avatarEntitiesLock.withReadLock([&] { + avatarEntityIDs = _packedAvatarEntityData.keys(); + }); + for (const auto& entityID : avatarEntityIDs) { auto entity = entityTree->findEntityByID(entityID); if (entity && isWearableEntity(entity)) { entityTree->withWriteLock([&entityID, &entityTree] { @@ -2394,8 +2399,11 @@ QVariantList MyAvatar::getAvatarEntitiesVariant() { auto treeRenderer = DependencyManager::get(); EntityTreePointer entityTree = treeRenderer ? treeRenderer->getTree() : nullptr; if (entityTree) { - AvatarEntityMap avatarEntities = getAvatarEntityData(); - for (auto entityID : avatarEntities.keys()) { + QList avatarEntityIDs; + _avatarEntitiesLock.withReadLock([&] { + avatarEntityIDs = _packedAvatarEntityData.keys(); + }); + for (const auto& entityID : avatarEntityIDs) { auto entity = entityTree->findEntityByID(entityID); if (!entity) { continue; @@ -2795,17 +2803,17 @@ void MyAvatar::setAttachmentData(const QVector& attachmentData) } QVector MyAvatar::getAttachmentData() const { - QVector avatarData; - auto avatarEntities = getAvatarEntityData(); - AvatarEntityMap::const_iterator dataItr = avatarEntities.begin(); - while (dataItr != avatarEntities.end()) { - QUuid entityID = dataItr.key(); + QVector attachmentData; + QList avatarEntityIDs; + _avatarEntitiesLock.withReadLock([&] { + avatarEntityIDs = _packedAvatarEntityData.keys(); + }); + for (const auto& entityID : avatarEntityIDs) { auto properties = DependencyManager::get()->getEntityProperties(entityID); AttachmentData data = entityPropertiesToAttachmentData(properties); - avatarData.append(data); - dataItr++; + attachmentData.append(data); } - return avatarData; + return attachmentData; } QVariantList MyAvatar::getAttachmentsVariant() const { @@ -2834,16 +2842,16 @@ void MyAvatar::setAttachmentsVariant(const QVariantList& variant) { } bool MyAvatar::findAvatarEntity(const QString& modelURL, const QString& jointName, QUuid& entityID) { - auto avatarEntities = getAvatarEntityData(); - AvatarEntityMap::const_iterator dataItr = avatarEntities.begin(); - while (dataItr != avatarEntities.end()) { - entityID = dataItr.key(); + QList avatarEntityIDs; + _avatarEntitiesLock.withReadLock([&] { + avatarEntityIDs = _packedAvatarEntityData.keys(); + }); + for (const auto& entityID : avatarEntityIDs) { auto props = DependencyManager::get()->getEntityProperties(entityID); if (props.getModelURL() == modelURL && (jointName.isEmpty() || props.getParentJointIndex() == getJointIndex(jointName))) { return true; } - dataItr++; } return false; } diff --git a/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp b/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp index b41e1381ef..feb95f1dd0 100644 --- a/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp +++ b/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp @@ -340,10 +340,13 @@ void Avatar::updateAvatarEntities() { return; } + PackedAvatarEntityMap packedAvatarEntityData; + _avatarEntitiesLock.withReadLock([&] { + packedAvatarEntityData = _packedAvatarEntityData; + }); entityTree->withWriteLock([&] { - AvatarEntityMap avatarEntities = getAvatarEntityData(); - AvatarEntityMap::const_iterator dataItr = avatarEntities.begin(); - while (dataItr != avatarEntities.end()) { + AvatarEntityMap::const_iterator dataItr = packedAvatarEntityData.begin(); + while (dataItr != packedAvatarEntityData.end()) { // compute hash of data. TODO? cache this? QByteArray data = dataItr.value(); uint32_t newHash = qHash(data); @@ -381,10 +384,11 @@ void Avatar::updateAvatarEntities() { ++dataItr; EntityItemProperties properties; - { - int32_t bytesLeftToRead = data.size(); - unsigned char* dataAt = (unsigned char*)(data.data()); - properties.constructFromBuffer(dataAt, bytesLeftToRead); + int32_t bytesLeftToRead = data.size(); + unsigned char* dataAt = (unsigned char*)(data.data()); + if (!properties.constructFromBuffer(dataAt, bytesLeftToRead)) { + // properties are corrupt + continue; } properties.setEntityHostType(entity::HostType::AVATAR); @@ -454,7 +458,7 @@ void Avatar::updateAvatarEntities() { } } } - if (avatarEntities.size() != _avatarEntityForRecording.size()) { + if (packedAvatarEntityData.size() != _avatarEntityForRecording.size()) { createRecordingIDs(); } }); @@ -466,9 +470,12 @@ void Avatar::removeAvatarEntitiesFromTree() { auto treeRenderer = DependencyManager::get(); EntityTreePointer entityTree = treeRenderer ? treeRenderer->getTree() : nullptr; if (entityTree) { + QList avatarEntityIDs; + _avatarEntitiesLock.withReadLock([&] { + avatarEntityIDs = _packedAvatarEntityData.keys(); + }); entityTree->withWriteLock([&] { - AvatarEntityMap avatarEntities = getAvatarEntityData(); - for (auto entityID : avatarEntities.keys()) { + for (const auto& entityID : avatarEntityIDs) { entityTree->deleteEntity(entityID, true, true); } }); diff --git a/libraries/entities/src/EntityItemProperties.cpp b/libraries/entities/src/EntityItemProperties.cpp index d466de0507..3b1e9a3e2b 100644 --- a/libraries/entities/src/EntityItemProperties.cpp +++ b/libraries/entities/src/EntityItemProperties.cpp @@ -4613,6 +4613,7 @@ void EntityItemProperties::convertToCloneProperties(const EntityItemID& entityID } bool EntityItemProperties::blobToProperties(QScriptEngine& scriptEngine, const QByteArray& blob, EntityItemProperties& properties) { + // DANGER: this method is NOT efficient. // begin recipe for converting unfortunately-formatted-binary-blob to EntityItemProperties QJsonDocument jsonProperties = QJsonDocument::fromBinaryData(blob); if (!jsonProperties.isObject()) { @@ -4628,6 +4629,7 @@ bool EntityItemProperties::blobToProperties(QScriptEngine& scriptEngine, const Q } void EntityItemProperties::propertiesToBlob(QScriptEngine& scriptEngine, const QUuid& myAvatarID, const EntityItemProperties& properties, QByteArray& blob) { + // DANGER: this method is NOT efficient. // begin recipe for extracting unfortunately-formatted-binary-blob from EntityItem QScriptValue scriptValue = EntityItemNonDefaultPropertiesToScriptValue(&scriptEngine, properties); QVariant variantProperties = scriptValue.toVariant(); From 27318a3f1773706b3db466d0f0df91806e158798 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Tue, 18 Dec 2018 08:42:09 -0800 Subject: [PATCH 34/43] update AvatarEntitData blobs before writing to JSON during recordings --- interface/src/avatar/MyAvatar.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/interface/src/avatar/MyAvatar.cpp b/interface/src/avatar/MyAvatar.cpp index 67b933e18a..2fb265dfbd 100755 --- a/interface/src/avatar/MyAvatar.cpp +++ b/interface/src/avatar/MyAvatar.cpp @@ -1814,6 +1814,7 @@ void MyAvatar::updateAvatarEntity(const QUuid& entityID, const QByteArray& entit } void MyAvatar::avatarEntityDataToJson(QJsonObject& root) const { + updateStaleAvatarEntityBlobs(); _avatarEntitiesLock.withReadLock([&] { if (!_cachedAvatarEntityBlobs.empty()) { QJsonArray avatarEntityJson; From 9d6acf007a5e77d56d8e932a52f868d4a65a1bc6 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Fri, 21 Dec 2018 11:55:22 -0800 Subject: [PATCH 35/43] fix build after rebase --- libraries/entities/src/EntityTypes.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libraries/entities/src/EntityTypes.cpp b/libraries/entities/src/EntityTypes.cpp index 584af55d5d..ad078190dd 100644 --- a/libraries/entities/src/EntityTypes.cpp +++ b/libraries/entities/src/EntityTypes.cpp @@ -59,7 +59,7 @@ REGISTER_ENTITY_TYPE(Zone) REGISTER_ENTITY_TYPE(Material) bool EntityTypes::typeIsValid(EntityType type) { - return type > EntityType::Unknown && type <= EntityType::LAST; + return type > EntityType::Unknown && type <= EntityType::NUM_TYPES; } const QString& EntityTypes::getEntityTypeName(EntityType entityType) { @@ -144,7 +144,7 @@ EntityItemPointer EntityTypes::constructEntityItem(const unsigned char* data, in QUuid id; EntityTypes::EntityType type = EntityTypes::Unknown; extractEntityTypeAndID(data, bytesToRead, type, id); - if (type > EntityTypes::Unknown && type <= EntityTypes::LAST) { + if (type > EntityTypes::Unknown && type <= EntityTypes::NUM_TYPES) { EntityItemID tempEntityID(id); EntityItemProperties tempProperties; return constructEntityItem(type, tempEntityID, tempProperties); From e2d6e6f3dc03656919cc6f62086a12990f0ce719 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Fri, 21 Dec 2018 11:56:08 -0800 Subject: [PATCH 36/43] more correct AvatarEntityData settings when switching domains --- interface/src/Application.cpp | 7 +++---- interface/src/avatar/MyAvatar.cpp | 26 +++++++++++++++++--------- interface/src/avatar/MyAvatar.h | 5 ++--- 3 files changed, 22 insertions(+), 16 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 1a4d2db83e..5fc3bad8e5 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -6756,8 +6756,10 @@ void Application::updateWindowTitle() const { } void Application::clearDomainOctreeDetails() { + // before we delete all entities get MyAvatar's AvatarEntityData ready + getMyAvatar()->prepareAvatarEntityDataForReload(); - // if we're about to quit, we really don't need to do any of these things... + // if we're about to quit, we really don't need to do the rest of these things... if (_aboutToQuit) { return; } @@ -6785,9 +6787,6 @@ void Application::clearDomainOctreeDetails() { ShaderCache::instance().clearUnusedResources(); DependencyManager::get()->clearUnusedResources(); DependencyManager::get()->clearUnusedResources(); - - // we just deleted all of MyAvatar's AvatarEntities so we flag it to reload from settings - getMyAvatar()->rememberToReloadAvatarEntityDataFromSettings(); } void Application::domainURLChanged(QUrl domainURL) { diff --git a/interface/src/avatar/MyAvatar.cpp b/interface/src/avatar/MyAvatar.cpp index 2fb265dfbd..af748a14c0 100755 --- a/interface/src/avatar/MyAvatar.cpp +++ b/interface/src/avatar/MyAvatar.cpp @@ -1501,8 +1501,8 @@ void MyAvatar::updateAvatarEntities() { // We collect changes to AvatarEntities and then handle them all in one spot per frame: updateAvatarEntities(). // Basically this is a "transaction pattern" with an extra complication: these changes can come from two - // "directions" and the "authoritative source" of each direction is different, so maintain two distinct sets of - // transaction lists; + // "directions" and the "authoritative source" of each direction is different, so we maintain two distinct sets + // of transaction lists: // // The _entitiesToDelete/Add/Update lists are for changes whose "authoritative sources" are already // correctly stored in _cachedAvatarEntityBlobs. These come from loadAvatarEntityDataFromSettings() and @@ -1690,10 +1690,7 @@ bool MyAvatar::updateStaleAvatarEntityBlobs() const { return false; } - std::set staleBlobs; - _avatarEntitiesLock.withWriteLock([&] { - staleBlobs = std::move(_staleCachedAvatarEntityBlobs); - }); + std::set staleBlobs = std::move(_staleCachedAvatarEntityBlobs); int32_t numFound = 0; for (const auto& id : staleBlobs) { bool found = false; @@ -1717,9 +1714,20 @@ bool MyAvatar::updateStaleAvatarEntityBlobs() const { return true; } -void MyAvatar::rememberToReloadAvatarEntityDataFromSettings() { - AvatarEntityMap emptyMap; - setAvatarEntityData(emptyMap); +void MyAvatar::prepareAvatarEntityDataForReload() { + saveAvatarEntityDataToSettings(); + + _avatarEntitiesLock.withWriteLock([&] { + _packedAvatarEntityData.clear(); + _entitiesToDelete.clear(); + _entitiesToAdd.clear(); + _entitiesToUpdate.clear(); + _cachedAvatarEntityBlobs.clear(); + _cachedAvatarEntityBlobsToDelete.clear(); + _cachedAvatarEntityBlobsToAddOrUpdate.clear(); + _cachedAvatarEntityBlobUpdatesToSkip.clear(); + }); + _reloadAvatarEntityDataFromSettings = true; } diff --git a/interface/src/avatar/MyAvatar.h b/interface/src/avatar/MyAvatar.h index 44f0c29b0b..38cef264a7 100644 --- a/interface/src/avatar/MyAvatar.h +++ b/interface/src/avatar/MyAvatar.h @@ -1187,7 +1187,7 @@ public: glm::vec3 getNextPosition() { return _goToPending ? _goToPosition : getWorldPosition(); } void updateAvatarEntities() override; - void rememberToReloadAvatarEntityDataFromSettings(); + void prepareAvatarEntityDataForReload(); /**jsdoc * Create a new grab. @@ -1614,7 +1614,6 @@ signals: */ void disableHandTouchForIDChanged(const QUuid& entityID, bool disable); - private slots: void leaveDomain(); void updateCollisionCapsuleCache(); @@ -1980,7 +1979,7 @@ private: std::vector _cachedAvatarEntityBlobUpdatesToSkip; // // Also these lists for tracking delayed changes to blobs and Settings - std::set _staleCachedAvatarEntityBlobs; + mutable std::set _staleCachedAvatarEntityBlobs; // // keep a ScriptEngine around so we don't have to instantiate on the fly (these are very slow to create/delete) QScriptEngine* _myScriptEngine { nullptr }; From 8206daf1a62b0b1472173eb09cfc6e933e1a911e Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Wed, 2 Jan 2019 13:54:26 -0800 Subject: [PATCH 37/43] send update when loading AvatarEntity from Settings --- interface/src/avatar/MyAvatar.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/interface/src/avatar/MyAvatar.cpp b/interface/src/avatar/MyAvatar.cpp index af748a14c0..03c69076aa 100755 --- a/interface/src/avatar/MyAvatar.cpp +++ b/interface/src/avatar/MyAvatar.cpp @@ -1564,6 +1564,7 @@ void MyAvatar::updateAvatarEntities() { } // ADD real entities + EntityEditPacketSender* packetSender = qApp->getEntityEditPacketSender(); for (const auto& id : entitiesToAdd) { bool blobFailed = false; EntityItemProperties properties; @@ -1589,6 +1590,9 @@ void MyAvatar::updateAvatarEntities() { sanitizeAvatarEntityProperties(properties); entityTree->withWriteLock([&] { EntityItemPointer entity = entityTree->addEntity(id, properties); + if (entity) { + packetSender->queueEditEntityMessage(PacketType::EntityAdd, entityTree, id, properties); + } }); } From b9667a067919626546053cfb31f310189cb5b873 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Mon, 7 Jan 2019 11:41:57 -0800 Subject: [PATCH 38/43] fix merge conflict after rebase --- libraries/entities/src/EntityItem.cpp | 30 --------------------------- 1 file changed, 30 deletions(-) diff --git a/libraries/entities/src/EntityItem.cpp b/libraries/entities/src/EntityItem.cpp index b766485450..5b7a152318 100644 --- a/libraries/entities/src/EntityItem.cpp +++ b/libraries/entities/src/EntityItem.cpp @@ -329,36 +329,6 @@ OctreeElement::AppendState EntityItem::appendEntityData(OctreePacketData* packet APPEND_ENTITY_PROPERTY(PROP_CERTIFICATE_ID, getCertificateID()); APPEND_ENTITY_PROPERTY(PROP_STATIC_CERTIFICATE_VERSION, getStaticCertificateVersion()); - APPEND_ENTITY_PROPERTY(PROP_NAME, getName()); - APPEND_ENTITY_PROPERTY(PROP_COLLISION_SOUND_URL, getCollisionSoundURL()); - APPEND_ENTITY_PROPERTY(PROP_HREF, getHref()); - APPEND_ENTITY_PROPERTY(PROP_DESCRIPTION, getDescription()); - APPEND_ENTITY_PROPERTY(PROP_ACTION_DATA, getDynamicData()); - - // convert AVATAR_SELF_ID to actual sessionUUID. - QUuid actualParentID = getParentID(); - if (actualParentID == AVATAR_SELF_ID) { - auto nodeList = DependencyManager::get(); - actualParentID = nodeList->getSessionUUID(); - } - APPEND_ENTITY_PROPERTY(PROP_PARENT_ID, actualParentID); - - APPEND_ENTITY_PROPERTY(PROP_PARENT_JOINT_INDEX, getParentJointIndex()); - APPEND_ENTITY_PROPERTY(PROP_QUERY_AA_CUBE, getQueryAACube()); - APPEND_ENTITY_PROPERTY(PROP_LAST_EDITED_BY, getLastEditedBy()); - - APPEND_ENTITY_PROPERTY(PROP_CLONEABLE, getCloneable()); - APPEND_ENTITY_PROPERTY(PROP_CLONE_LIFETIME, getCloneLifetime()); - APPEND_ENTITY_PROPERTY(PROP_CLONE_LIMIT, getCloneLimit()); - APPEND_ENTITY_PROPERTY(PROP_CLONE_DYNAMIC, getCloneDynamic()); - APPEND_ENTITY_PROPERTY(PROP_CLONE_AVATAR_ENTITY, getCloneAvatarEntity()); - APPEND_ENTITY_PROPERTY(PROP_CLONE_ORIGIN_ID, getCloneOriginID()); - - withReadLock([&] { - _grabProperties.appendSubclassData(packetData, params, entityTreeElementExtraEncodeData, requestedProperties, - propertyFlags, propertiesDidntFit, propertyCount, appendState); - }); - appendSubclassData(packetData, params, entityTreeElementExtraEncodeData, requestedProperties, propertyFlags, From 9ea6968e351081aeb13884f50fe7d793da3824f4 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Wed, 9 Jan 2019 17:26:23 -0800 Subject: [PATCH 39/43] fix bug where attached AvatarEntities do not update in timely fashion --- .../src/avatars/AvatarMixerClientData.cpp | 2 +- interface/src/avatar/MyAvatar.cpp | 10 +- interface/src/avatar/MyAvatar.h | 4 +- interface/src/avatar/OtherAvatar.cpp | 301 +++++++++++++++++- interface/src/avatar/OtherAvatar.h | 9 + .../src/avatars-renderer/Avatar.cpp | 240 -------------- .../src/avatars-renderer/Avatar.h | 3 +- libraries/avatars/src/AvatarData.cpp | 12 +- libraries/avatars/src/AvatarData.h | 6 +- 9 files changed, 326 insertions(+), 261 deletions(-) diff --git a/assignment-client/src/avatars/AvatarMixerClientData.cpp b/assignment-client/src/avatars/AvatarMixerClientData.cpp index 7f1b40142e..bf90a4e727 100644 --- a/assignment-client/src/avatars/AvatarMixerClientData.cpp +++ b/assignment-client/src/avatars/AvatarMixerClientData.cpp @@ -157,7 +157,7 @@ void AvatarMixerClientData::processSetTraitsMessage(ReceivedMessage& message, if (traitSize == AvatarTraits::DELETED_TRAIT_SIZE) { _avatar->processDeletedTraitInstance(traitType, instanceID); // Mixer doesn't need deleted IDs. - _avatar->getAndClearRecentlyDetachedIDs(); + _avatar->getAndClearRecentlyRemovedIDs(); // to track a deleted instance but keep version information // the avatar mixer uses the negative value of the sent version diff --git a/interface/src/avatar/MyAvatar.cpp b/interface/src/avatar/MyAvatar.cpp index 03c69076aa..91ea8f0291 100755 --- a/interface/src/avatar/MyAvatar.cpp +++ b/interface/src/avatar/MyAvatar.cpp @@ -674,7 +674,7 @@ void MyAvatar::update(float deltaTime) { _clientTraitsHandler->sendChangedTraitsToMixer(); - simulate(deltaTime); + simulate(deltaTime, true); currentEnergy += energyChargeRate; currentEnergy -= getAccelerationEnergy(); @@ -746,7 +746,7 @@ void MyAvatar::updateChildCauterization(SpatiallyNestablePointer object, bool ca } } -void MyAvatar::simulate(float deltaTime) { +void MyAvatar::simulate(float deltaTime, bool inView) { PerformanceTimer perfTimer("simulate"); animateScaleChanges(deltaTime); @@ -890,7 +890,7 @@ void MyAvatar::simulate(float deltaTime) { _characterController.setCollisionlessAllowed(collisionlessAllowed); } - updateAvatarEntities(); + handleChangedAvatarEntityData(); updateFadingStatus(); } @@ -1480,7 +1480,7 @@ void MyAvatar::sanitizeAvatarEntityProperties(EntityItemProperties& properties) properties.markAllChanged(); } -void MyAvatar::updateAvatarEntities() { +void MyAvatar::handleChangedAvatarEntityData() { // NOTE: this is a per-frame update if (getID().isNull() || getID() == AVATAR_SELF_ID || @@ -1499,7 +1499,7 @@ void MyAvatar::updateAvatarEntities() { return; } - // We collect changes to AvatarEntities and then handle them all in one spot per frame: updateAvatarEntities(). + // We collect changes to AvatarEntities and then handle them all in one spot per frame: handleChangedAvatarEntityData(). // Basically this is a "transaction pattern" with an extra complication: these changes can come from two // "directions" and the "authoritative source" of each direction is different, so we maintain two distinct sets // of transaction lists: diff --git a/interface/src/avatar/MyAvatar.h b/interface/src/avatar/MyAvatar.h index 38cef264a7..67a449b274 100644 --- a/interface/src/avatar/MyAvatar.h +++ b/interface/src/avatar/MyAvatar.h @@ -1186,7 +1186,6 @@ public: virtual void setAttachmentsVariant(const QVariantList& variant) override; glm::vec3 getNextPosition() { return _goToPending ? _goToPosition : getWorldPosition(); } - void updateAvatarEntities() override; void prepareAvatarEntityDataForReload(); /**jsdoc @@ -1619,6 +1618,7 @@ private slots: void updateCollisionCapsuleCache(); protected: + void handleChangedAvatarEntityData(); virtual void beParentOfChild(SpatiallyNestablePointer newChild) const override; virtual void forgetChild(SpatiallyNestablePointer newChild) const override; virtual void recalculateChildCauterization() const override; @@ -1630,7 +1630,7 @@ private: virtual QByteArray toByteArrayStateful(AvatarDataDetail dataDetail, bool dropFaceTracking) override; - void simulate(float deltaTime); + void simulate(float deltaTime, bool inView) override; void updateFromTrackers(float deltaTime); void saveAvatarUrl(); virtual void render(RenderArgs* renderArgs) override; diff --git a/interface/src/avatar/OtherAvatar.cpp b/interface/src/avatar/OtherAvatar.cpp index a71d2478ad..569f92ac4d 100644 --- a/interface/src/avatar/OtherAvatar.cpp +++ b/interface/src/avatar/OtherAvatar.cpp @@ -7,10 +7,18 @@ // #include "OtherAvatar.h" -#include "Application.h" +#include +#include + +#include + +#include "Application.h" #include "AvatarMotionState.h" +const float DISPLAYNAME_FADE_TIME = 0.5f; +const float DISPLAYNAME_FADE_FACTOR = pow(0.01f, 1.0f / DISPLAYNAME_FADE_TIME); + static glm::u8vec3 getLoadingOrbColor(Avatar::LoadingStatus loadingStatus) { const glm::u8vec3 NO_MODEL_COLOR(0xe3, 0xe3, 0xe3); @@ -142,4 +150,293 @@ void OtherAvatar::updateCollisionGroup(bool myAvatarCollide) { _motionState->addDirtyFlags(Simulation::DIRTY_COLLISION_GROUP); } } -} \ No newline at end of file +} + +void OtherAvatar::simulate(float deltaTime, bool inView) { + PROFILE_RANGE(simulation, "simulate"); + + _globalPosition = _transit.isActive() ? _transit.getCurrentPosition() : _serverPosition; + if (!hasParent()) { + setLocalPosition(_globalPosition); + } + + _simulationRate.increment(); + if (inView) { + _simulationInViewRate.increment(); + } + + PerformanceTimer perfTimer("simulate"); + { + PROFILE_RANGE(simulation, "updateJoints"); + if (inView) { + Head* head = getHead(); + if (_hasNewJointData || _transit.isActive()) { + _skeletonModel->getRig().copyJointsFromJointData(_jointData); + glm::mat4 rootTransform = glm::scale(_skeletonModel->getScale()) * glm::translate(_skeletonModel->getOffset()); + _skeletonModel->getRig().computeExternalPoses(rootTransform); + _jointDataSimulationRate.increment(); + + _skeletonModel->simulate(deltaTime, true); + + locationChanged(); // joints changed, so if there are any children, update them. + _hasNewJointData = false; + + glm::vec3 headPosition = getWorldPosition(); + if (!_skeletonModel->getHeadPosition(headPosition)) { + headPosition = getWorldPosition(); + } + head->setPosition(headPosition); + } + head->setScale(getModelScale()); + head->simulate(deltaTime); + relayJointDataToChildren(); + } else { + // a non-full update is still required so that the position, rotation, scale and bounds of the skeletonModel are updated. + _skeletonModel->simulate(deltaTime, false); + } + _skeletonModelSimulationRate.increment(); + } + + // update animation for display name fade in/out + if ( _displayNameTargetAlpha != _displayNameAlpha) { + // the alpha function is + // Fade out => alpha(t) = factor ^ t => alpha(t+dt) = alpha(t) * factor^(dt) + // Fade in => alpha(t) = 1 - factor^t => alpha(t+dt) = 1-(1-alpha(t))*coef^(dt) + // factor^(dt) = coef + float coef = pow(DISPLAYNAME_FADE_FACTOR, deltaTime); + if (_displayNameTargetAlpha < _displayNameAlpha) { + // Fading out + _displayNameAlpha *= coef; + } else { + // Fading in + _displayNameAlpha = 1 - (1 - _displayNameAlpha) * coef; + } + _displayNameAlpha = abs(_displayNameAlpha - _displayNameTargetAlpha) < 0.01f ? _displayNameTargetAlpha : _displayNameAlpha; + } + + { + PROFILE_RANGE(simulation, "misc"); + measureMotionDerivatives(deltaTime); + simulateAttachments(deltaTime); + updatePalms(); + } + { + PROFILE_RANGE(simulation, "entities"); + handleChangedAvatarEntityData(); + updateAttachedAvatarEntities(); + } + + { + PROFILE_RANGE(simulation, "grabs"); + updateGrabs(); + } + + updateFadingStatus(); +} + +void OtherAvatar::handleChangedAvatarEntityData() { + PerformanceTimer perfTimer("attachments"); + + // AVATAR ENTITY UPDATE FLOW + // - if queueEditEntityMessage() sees "AvatarEntity" HostType it calls _myAvatar->storeAvatarEntityDataPayload() + // - storeAvatarEntityDataPayload() saves the payload and flags the trait instance for the entity as updated, + // - ClientTraitsHandler::sendChangedTraitsToMixea() sends the entity bytes to the mixer which relays them to other interfaces + // - AvatarHashMap::processBulkAvatarTraits() on other interfaces calls avatar->processTraitInstance() + // - AvatarData::processTraitInstance() calls storeAvatarEntityDataPayload(), which sets _avatarEntityDataChanged = true + // - (My)Avatar::simulate() calls handleChangedAvatarEntityData() every frame which checks _avatarEntityDataChanged + // and here we are... + + // AVATAR ENTITY DELETE FLOW + // - EntityScriptingInterface::deleteEntity() calls _myAvatar->clearAvatarEntity() for deleted avatar entities + // - clearAvatarEntity() removes the avatar entity and flags the trait instance for the entity as deleted + // - ClientTraitsHandler::sendChangedTraitsToMixer() sends a deletion to the mixer which relays to other interfaces + // - AvatarHashMap::processBulkAvatarTraits() on other interfaces calls avatar->processDeletedTraitInstace() + // - AvatarData::processDeletedTraitInstance() calls clearAvatarEntity() + // - AvatarData::clearAvatarEntity() sets _avatarEntityDataChanged = true and adds the ID to the detached list + // - (My)Avatar::simulate() calls handleChangedAvatarEntityData() every frame which checks _avatarEntityDataChanged + // and here we are... + + if (!_avatarEntityDataChanged) { + return; + } + + auto treeRenderer = DependencyManager::get(); + EntityTreePointer entityTree = treeRenderer ? treeRenderer->getTree() : nullptr; + if (!entityTree) { + return; + } + + PackedAvatarEntityMap packedAvatarEntityData; + _avatarEntitiesLock.withReadLock([&] { + packedAvatarEntityData = _packedAvatarEntityData; + }); + entityTree->withWriteLock([&] { + AvatarEntityMap::const_iterator dataItr = packedAvatarEntityData.begin(); + while (dataItr != packedAvatarEntityData.end()) { + // compute hash of data. TODO? cache this? + QByteArray data = dataItr.value(); + uint32_t newHash = qHash(data); + + // check to see if we recognize this hash and whether it was already successfully processed + QUuid entityID = dataItr.key(); + MapOfAvatarEntityDataHashes::iterator stateItr = _avatarEntityDataHashes.find(entityID); + if (stateItr != _avatarEntityDataHashes.end()) { + if (stateItr.value().success) { + if (newHash == stateItr.value().hash) { + // data hasn't changed --> nothing to do + ++dataItr; + continue; + } + } else { + // NOTE: if the data was unsuccessful in producing an entity in the past + // we will try again just in case something changed (unlikely). + // Unfortunately constantly trying to build the entity for this data costs + // CPU cycles that we'd rather not spend. + // TODO? put a maximum number of tries on this? + } + } else { + // sanity check data + QUuid id; + EntityTypes::EntityType type; + EntityTypes::extractEntityTypeAndID((unsigned char*)(data.data()), data.size(), type, id); + if (id != entityID || !EntityTypes::typeIsValid(type)) { + // skip for corrupt + ++dataItr; + continue; + } + // remember this hash for the future + stateItr = _avatarEntityDataHashes.insert(entityID, AvatarEntityDataHash(newHash)); + } + ++dataItr; + + EntityItemProperties properties; + int32_t bytesLeftToRead = data.size(); + unsigned char* dataAt = (unsigned char*)(data.data()); + if (!properties.constructFromBuffer(dataAt, bytesLeftToRead)) { + // properties are corrupt + continue; + } + + properties.setEntityHostType(entity::HostType::AVATAR); + properties.setOwningAvatarID(getID()); + + // there's no entity-server to tell us we're the simulation owner, so always set the + // simulationOwner to the owningAvatarID and a high priority. + properties.setSimulationOwner(getID(), AVATAR_ENTITY_SIMULATION_PRIORITY); + + if (properties.getParentID() == AVATAR_SELF_ID) { + properties.setParentID(getID()); + } + + // NOTE: if this avatar entity is not attached to us, strip its entity script completely... + auto attachedScript = properties.getScript(); + if (!isMyAvatar() && !attachedScript.isEmpty()) { + QString noScript; + properties.setScript(noScript); + } + + auto specifiedHref = properties.getHref(); + if (!isMyAvatar() && !specifiedHref.isEmpty()) { + qCDebug(avatars) << "removing entity href from avatar attached entity:" << entityID << "old href:" << specifiedHref; + QString noHref; + properties.setHref(noHref); + } + + // When grabbing avatar entities, they are parented to the joint moving them, then when un-grabbed + // they go back to the default parent (null uuid). When un-gripped, others saw the entity disappear. + // The thinking here is the local position was noticed as changing, but not the parentID (since it is now + // back to the default), and the entity flew off somewhere. Marking all changed definitely fixes this, + // and seems safe (per Seth). + properties.markAllChanged(); + + // try to build the entity + EntityItemPointer entity = entityTree->findEntityByEntityItemID(EntityItemID(entityID)); + bool success = true; + if (entity) { + QUuid oldParentID = entity->getParentID(); + if (entityTree->updateEntity(entityID, properties)) { + entity->updateLastEditedFromRemote(); + } else { + success = false; + } + if (oldParentID != entity->getParentID()) { + if (entity->getParentID() == getID()) { + onAddAttachedAvatarEntity(entityID); + } else if (oldParentID == getID()) { + onRemoveAttachedAvatarEntity(entityID); + } + } + } else { + entity = entityTree->addEntity(entityID, properties); + if (!entity) { + success = false; + } else if (entity->getParentID() == getID()) { + onAddAttachedAvatarEntity(entityID); + } + } + stateItr.value().success = success; + } + + AvatarEntityIDs recentlyRemovedAvatarEntities = getAndClearRecentlyRemovedIDs(); + if (!recentlyRemovedAvatarEntities.empty()) { + // only lock this thread when absolutely necessary + AvatarEntityMap packedAvatarEntityData; + _avatarEntitiesLock.withReadLock([&] { + packedAvatarEntityData = _packedAvatarEntityData; + }); + foreach (auto entityID, recentlyRemovedAvatarEntities) { + if (!packedAvatarEntityData.contains(entityID)) { + entityTree->deleteEntity(entityID, true, true); + } + } + + // TODO: move this outside of tree lock + // remove stale data hashes + foreach (auto entityID, recentlyRemovedAvatarEntities) { + MapOfAvatarEntityDataHashes::iterator stateItr = _avatarEntityDataHashes.find(entityID); + if (stateItr != _avatarEntityDataHashes.end()) { + _avatarEntityDataHashes.erase(stateItr); + } + onRemoveAttachedAvatarEntity(entityID); + } + } + if (packedAvatarEntityData.size() != _avatarEntityForRecording.size()) { + createRecordingIDs(); + } + }); + + setAvatarEntityDataChanged(false); +} + +void OtherAvatar::onAddAttachedAvatarEntity(const QUuid& id) { + for (uint32_t i = 0; i < _attachedAvatarEntities.size(); ++i) { + if (_attachedAvatarEntities[i] == id) { + return; + } + } + _attachedAvatarEntities.push_back(id); +} + +void OtherAvatar::onRemoveAttachedAvatarEntity(const QUuid& id) { + for (uint32_t i = 0; i < _attachedAvatarEntities.size(); ++i) { + if (_attachedAvatarEntities[i] == id) { + if (i != _attachedAvatarEntities.size() - 1) { + _attachedAvatarEntities[i] = _attachedAvatarEntities.back(); + } + _attachedAvatarEntities.pop_back(); + break; + } + } +} + +void OtherAvatar::updateAttachedAvatarEntities() { + if (!_attachedAvatarEntities.empty()) { + auto treeRenderer = DependencyManager::get(); + if (!treeRenderer) { + return; + } + for (const QUuid& id : _attachedAvatarEntities) { + treeRenderer->onEntityChanged(id); + } + } +} diff --git a/interface/src/avatar/OtherAvatar.h b/interface/src/avatar/OtherAvatar.h index 48402fe55c..a1dc5724a9 100644 --- a/interface/src/avatar/OtherAvatar.h +++ b/interface/src/avatar/OtherAvatar.h @@ -10,6 +10,7 @@ #define hifi_OtherAvatar_h #include +#include #include #include @@ -47,9 +48,17 @@ public: void updateCollisionGroup(bool myAvatarCollide); + void simulate(float deltaTime, bool inView) override; + friend AvatarManager; protected: + void handleChangedAvatarEntityData(); + void updateAttachedAvatarEntities(); + void onAddAttachedAvatarEntity(const QUuid& id); + void onRemoveAttachedAvatarEntity(const QUuid& id); + + std::vector _attachedAvatarEntities; std::shared_ptr _otherAvatarOrbMeshPlaceholder { nullptr }; OverlayID _otherAvatarOrbMeshPlaceholderID { UNKNOWN_OVERLAY_ID }; AvatarMotionState* _motionState { nullptr }; diff --git a/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp b/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp index feb95f1dd0..a6185d7e79 100644 --- a/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp +++ b/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp @@ -308,164 +308,6 @@ void Avatar::setAvatarEntityDataChanged(bool value) { _avatarEntityDataHashes.clear(); } -void Avatar::updateAvatarEntities() { - PerformanceTimer perfTimer("attachments"); - - // AVATAR ENTITY UPDATE FLOW - // - if queueEditEntityMessage sees "AvatarEntity" HostType it calls _myAvatar->storeAvatarEntityDataPayload - // - storeAvatarEntityDataPayload saves the payload and flags the trait instance for the entity as updated, - // - ClientTraitsHandler::sendChangedTraitsToMixer sends the entity bytes to the mixer which relays them to other interfaces - // - AvatarHashMap::processBulkAvatarTraits on other interfaces calls avatar->processTraitInstace - // - AvatarData::processTraitInstance calls storeAvatarEntityDataPayload, which sets _avatarEntityDataChanged = true - // - (My)Avatar::simulate calls updateAvatarEntities every frame which checks _avatarEntityDataChanged - // and here we are... - - // AVATAR ENTITY DELETE FLOW - // - EntityScriptingInterface::deleteEntity calls _myAvatar->clearAvatarEntity() for deleted avatar entities - // - clearAvatarEntity removes the avatar entity and flags the trait instance for the entity as deleted - // - ClientTraitsHandler::sendChangedTraitsToMixer sends a deletion to the mixer which relays to other interfaces - // - AvatarHashMap::processBulkAvatarTraits on other interfaces calls avatar->processDeletedTraitInstace - // - AvatarData::processDeletedTraitInstance calls clearAvatarEntity - // - AvatarData::clearAvatarEntity sets _avatarEntityDataChanged = true and adds the ID to the detached list - // - (My)Avatar::simulate calls updateAvatarEntities every frame which checks _avatarEntityDataChanged - // and here we are... - - if (!_avatarEntityDataChanged) { - return; - } - - auto treeRenderer = DependencyManager::get(); - EntityTreePointer entityTree = treeRenderer ? treeRenderer->getTree() : nullptr; - if (!entityTree) { - return; - } - - PackedAvatarEntityMap packedAvatarEntityData; - _avatarEntitiesLock.withReadLock([&] { - packedAvatarEntityData = _packedAvatarEntityData; - }); - entityTree->withWriteLock([&] { - AvatarEntityMap::const_iterator dataItr = packedAvatarEntityData.begin(); - while (dataItr != packedAvatarEntityData.end()) { - // compute hash of data. TODO? cache this? - QByteArray data = dataItr.value(); - uint32_t newHash = qHash(data); - - // check to see if we recognize this hash and whether it was already successfully processed - QUuid entityID = dataItr.key(); - MapOfAvatarEntityDataHashes::iterator stateItr = _avatarEntityDataHashes.find(entityID); - if (stateItr != _avatarEntityDataHashes.end()) { - if (stateItr.value().success) { - if (newHash == stateItr.value().hash) { - // data hasn't changed --> nothing to do - ++dataItr; - continue; - } - } else { - // NOTE: if the data was unsuccessful in producing an entity in the past - // we will try again just in case something changed (unlikely). - // Unfortunately constantly trying to build the entity for this data costs - // CPU cycles that we'd rather not spend. - // TODO? put a maximum number of tries on this? - } - } else { - // sanity check data - QUuid id; - EntityTypes::EntityType type; - EntityTypes::extractEntityTypeAndID((unsigned char*)(data.data()), data.size(), type, id); - if (id != entityID || !EntityTypes::typeIsValid(type)) { - // skip for corrupt - ++dataItr; - continue; - } - // remember this hash for the future - stateItr = _avatarEntityDataHashes.insert(entityID, AvatarEntityDataHash(newHash)); - } - ++dataItr; - - EntityItemProperties properties; - int32_t bytesLeftToRead = data.size(); - unsigned char* dataAt = (unsigned char*)(data.data()); - if (!properties.constructFromBuffer(dataAt, bytesLeftToRead)) { - // properties are corrupt - continue; - } - - properties.setEntityHostType(entity::HostType::AVATAR); - properties.setOwningAvatarID(getID()); - - if (properties.getParentID() == AVATAR_SELF_ID) { - properties.setParentID(getID()); - } - - // NOTE: if this avatar entity is not attached to us, strip its entity script completely... - auto attachedScript = properties.getScript(); - if (!isMyAvatar() && !attachedScript.isEmpty()) { - QString noScript; - properties.setScript(noScript); - } - - auto specifiedHref = properties.getHref(); - if (!isMyAvatar() && !specifiedHref.isEmpty()) { - qCDebug(avatars_renderer) << "removing entity href from avatar attached entity:" << entityID << "old href:" << specifiedHref; - QString noHref; - properties.setHref(noHref); - } - - // When grabbing avatar entities, they are parented to the joint moving them, then when un-grabbed - // they go back to the default parent (null uuid). When un-gripped, others saw the entity disappear. - // The thinking here is the local position was noticed as changing, but not the parentID (since it is now - // back to the default), and the entity flew off somewhere. Marking all changed definitely fixes this, - // and seems safe (per Seth). - properties.markAllChanged(); - - // try to build the entity - EntityItemPointer entity = entityTree->findEntityByEntityItemID(EntityItemID(entityID)); - bool success = true; - if (entity) { - if (entityTree->updateEntity(entityID, properties)) { - entity->updateLastEditedFromRemote(); - } else { - success = false; - } - } else { - entity = entityTree->addEntity(entityID, properties); - if (!entity) { - success = false; - } - } - stateItr.value().success = success; - } - - AvatarEntityIDs recentlyDetachedAvatarEntities = getAndClearRecentlyDetachedIDs(); - if (!recentlyDetachedAvatarEntities.empty()) { - // only lock this thread when absolutely necessary - AvatarEntityMap packedAvatarEntityData; - _avatarEntitiesLock.withReadLock([&] { - packedAvatarEntityData = _packedAvatarEntityData; - }); - foreach (auto entityID, recentlyDetachedAvatarEntities) { - if (!packedAvatarEntityData.contains(entityID)) { - entityTree->deleteEntity(entityID, true, true); - } - } - - // remove stale data hashes - foreach (auto entityID, recentlyDetachedAvatarEntities) { - MapOfAvatarEntityDataHashes::iterator stateItr = _avatarEntityDataHashes.find(entityID); - if (stateItr != _avatarEntityDataHashes.end()) { - _avatarEntityDataHashes.erase(stateItr); - } - } - } - if (packedAvatarEntityData.size() != _avatarEntityForRecording.size()) { - createRecordingIDs(); - } - }); - - setAvatarEntityDataChanged(false); -} - void Avatar::removeAvatarEntitiesFromTree() { auto treeRenderer = DependencyManager::get(); EntityTreePointer entityTree = treeRenderer ? treeRenderer->getTree() : nullptr; @@ -650,87 +492,6 @@ void Avatar::relayJointDataToChildren() { _reconstructSoftEntitiesJointMap = false; } -void Avatar::simulate(float deltaTime, bool inView) { - PROFILE_RANGE(simulation, "simulate"); - - _globalPosition = _transit.isActive() ? _transit.getCurrentPosition() : _serverPosition; - if (!hasParent()) { - setLocalPosition(_globalPosition); - } - - _simulationRate.increment(); - if (inView) { - _simulationInViewRate.increment(); - } - - PerformanceTimer perfTimer("simulate"); - { - PROFILE_RANGE(simulation, "updateJoints"); - if (inView) { - Head* head = getHead(); - if (_hasNewJointData || _transit.isActive()) { - _skeletonModel->getRig().copyJointsFromJointData(_jointData); - glm::mat4 rootTransform = glm::scale(_skeletonModel->getScale()) * glm::translate(_skeletonModel->getOffset()); - _skeletonModel->getRig().computeExternalPoses(rootTransform); - _jointDataSimulationRate.increment(); - - _skeletonModel->simulate(deltaTime, true); - - locationChanged(); // joints changed, so if there are any children, update them. - _hasNewJointData = false; - - glm::vec3 headPosition = getWorldPosition(); - if (!_skeletonModel->getHeadPosition(headPosition)) { - headPosition = getWorldPosition(); - } - head->setPosition(headPosition); - } - head->setScale(getModelScale()); - head->simulate(deltaTime); - relayJointDataToChildren(); - } else { - // a non-full update is still required so that the position, rotation, scale and bounds of the skeletonModel are updated. - _skeletonModel->simulate(deltaTime, false); - } - _skeletonModelSimulationRate.increment(); - } - - // update animation for display name fade in/out - if ( _displayNameTargetAlpha != _displayNameAlpha) { - // the alpha function is - // Fade out => alpha(t) = factor ^ t => alpha(t+dt) = alpha(t) * factor^(dt) - // Fade in => alpha(t) = 1 - factor^t => alpha(t+dt) = 1-(1-alpha(t))*coef^(dt) - // factor^(dt) = coef - float coef = pow(DISPLAYNAME_FADE_FACTOR, deltaTime); - if (_displayNameTargetAlpha < _displayNameAlpha) { - // Fading out - _displayNameAlpha *= coef; - } else { - // Fading in - _displayNameAlpha = 1 - (1 - _displayNameAlpha) * coef; - } - _displayNameAlpha = abs(_displayNameAlpha - _displayNameTargetAlpha) < 0.01f ? _displayNameTargetAlpha : _displayNameAlpha; - } - - { - PROFILE_RANGE(simulation, "misc"); - measureMotionDerivatives(deltaTime); - simulateAttachments(deltaTime); - updatePalms(); - } - { - PROFILE_RANGE(simulation, "entities"); - updateAvatarEntities(); - } - - { - PROFILE_RANGE(simulation, "grabs"); - updateGrabs(); - } - - updateFadingStatus(); -} - float Avatar::getSimulationRate(const QString& rateName) const { if (rateName == "") { return _simulationRate.rate(); @@ -1045,7 +806,6 @@ void Avatar::render(RenderArgs* renderArgs) { } } - void Avatar::setEnableMeshVisible(bool isEnabled) { if (_isMeshVisible != isEnabled) { _isMeshVisible = isEnabled; diff --git a/libraries/avatars-renderer/src/avatars-renderer/Avatar.h b/libraries/avatars-renderer/src/avatars-renderer/Avatar.h index 97218ac3c1..4ff3e9cc13 100644 --- a/libraries/avatars-renderer/src/avatars-renderer/Avatar.h +++ b/libraries/avatars-renderer/src/avatars-renderer/Avatar.h @@ -139,9 +139,8 @@ public: typedef render::Payload Payload; void init(); - virtual void updateAvatarEntities(); void removeAvatarEntitiesFromTree(); - void simulate(float deltaTime, bool inView); + virtual void simulate(float deltaTime, bool inView) = 0; virtual void simulateAttachments(float deltaTime); virtual void render(RenderArgs* renderArgs); diff --git a/libraries/avatars/src/AvatarData.cpp b/libraries/avatars/src/AvatarData.cpp index 5eda53329b..21e0a6aba2 100644 --- a/libraries/avatars/src/AvatarData.cpp +++ b/libraries/avatars/src/AvatarData.cpp @@ -2777,7 +2777,7 @@ void AvatarData::clearAvatarEntity(const QUuid& entityID, bool requiresRemovalFr removedEntity = _packedAvatarEntityData.remove(entityID); }); - insertDetachedEntityID(entityID); + insertRemovedEntityID(entityID); if (removedEntity && _clientTraitsHandler) { // we have a client traits handler, so we need to mark this removed instance trait as deleted @@ -2798,18 +2798,18 @@ void AvatarData::setAvatarEntityData(const AvatarEntityMap& avatarEntityData) { // each QByteArray represents an EntityItemProperties object from JavaScript } -void AvatarData::insertDetachedEntityID(const QUuid entityID) { +void AvatarData::insertRemovedEntityID(const QUuid entityID) { _avatarEntitiesLock.withWriteLock([&] { - _avatarEntityDetached.insert(entityID); + _avatarEntityRemoved.insert(entityID); }); _avatarEntityDataChanged = true; } -AvatarEntityIDs AvatarData::getAndClearRecentlyDetachedIDs() { +AvatarEntityIDs AvatarData::getAndClearRecentlyRemovedIDs() { AvatarEntityIDs result; _avatarEntitiesLock.withWriteLock([&] { - result = _avatarEntityDetached; - _avatarEntityDetached.clear(); + result = _avatarEntityRemoved; + _avatarEntityRemoved.clear(); }); return result; } diff --git a/libraries/avatars/src/AvatarData.h b/libraries/avatars/src/AvatarData.h index 0fea5a541e..ec5ea4a5d0 100644 --- a/libraries/avatars/src/AvatarData.h +++ b/libraries/avatars/src/AvatarData.h @@ -1150,8 +1150,7 @@ public: Q_INVOKABLE virtual void setAvatarEntityData(const AvatarEntityMap& avatarEntityData); virtual void setAvatarEntityDataChanged(bool value) { _avatarEntityDataChanged = value; } - void insertDetachedEntityID(const QUuid entityID); - AvatarEntityIDs getAndClearRecentlyDetachedIDs(); + AvatarEntityIDs getAndClearRecentlyRemovedIDs(); /**jsdoc * @function MyAvatar.getSensorToWorldMatrix @@ -1338,6 +1337,7 @@ public slots: void resetLastSent() { _lastToByteArray = 0; } protected: + void insertRemovedEntityID(const QUuid entityID); void lazyInitHeadData() const; float getDistanceBasedMinRotationDOT(glm::vec3 viewerPosition) const; @@ -1466,7 +1466,7 @@ protected: AABox _defaultBubbleBox; mutable ReadWriteLockable _avatarEntitiesLock; - AvatarEntityIDs _avatarEntityDetached; // recently detached from this avatar + AvatarEntityIDs _avatarEntityRemoved; // recently removed AvatarEntity ids AvatarEntityIDs _avatarEntityForRecording; // create new entities id for avatar recording PackedAvatarEntityMap _packedAvatarEntityData; bool _avatarEntityDataChanged { false }; From 0098dedcbb57599e38bd1c40e1a403dcc1fc116f Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Wed, 9 Jan 2019 18:25:12 -0800 Subject: [PATCH 40/43] don't update server for AvatarEntity queryAACube --- libraries/entities/src/EntityTree.cpp | 41 ++++++++++++++------------- 1 file changed, 22 insertions(+), 19 deletions(-) diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index 542e18fa6a..fb1a11d43f 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -2964,27 +2964,30 @@ void EntityTree::updateEntityQueryAACubeWorker(SpatiallyNestablePointer object, MovingEntitiesOperator& moveOperator, bool force, bool tellServer) { // if the queryBox has changed, tell the entity-server EntityItemPointer entity = std::dynamic_pointer_cast(object); - if (entity && (entity->updateQueryAACube() || force)) { - bool success; - AACube newCube = entity->getQueryAACube(success); - if (success) { - moveOperator.addEntityToMoveList(entity, newCube); - } - // send an edit packet to update the entity-server about the queryAABox. We do this for domain-hosted - // entities as well as for avatar-entities; the packet-sender will route the update accordingly - if (tellServer && packetSender && (entity->isDomainEntity() || entity->isAvatarEntity())) { - quint64 now = usecTimestampNow(); - EntityItemProperties properties = entity->getProperties(); - properties.setQueryAACubeDirty(); - properties.setLocationDirty(); - properties.setLastEdited(now); + if (entity) { + bool tellServerThis = tellServer && (entity->getEntityHostType() != entity::HostType::AVATAR); + if ((entity->updateQueryAACube() || force)) { + bool success; + AACube newCube = entity->getQueryAACube(success); + if (success) { + moveOperator.addEntityToMoveList(entity, newCube); + } + // send an edit packet to update the entity-server about the queryAABox. We do this for domain-hosted + // entities as well as for avatar-entities; the packet-sender will route the update accordingly + if (tellServerThis && packetSender && (entity->isDomainEntity() || entity->isAvatarEntity())) { + quint64 now = usecTimestampNow(); + EntityItemProperties properties = entity->getProperties(); + properties.setQueryAACubeDirty(); + properties.setLocationDirty(); + properties.setLastEdited(now); - packetSender->queueEditEntityMessage(PacketType::EntityEdit, getThisPointer(), entity->getID(), properties); - entity->setLastBroadcast(now); // for debug/physics status icons - } + packetSender->queueEditEntityMessage(PacketType::EntityEdit, getThisPointer(), entity->getID(), properties); + entity->setLastBroadcast(now); // for debug/physics status icons + } - entity->markDirtyFlags(Simulation::DIRTY_POSITION); - entityChanged(entity); + entity->markDirtyFlags(Simulation::DIRTY_POSITION); + entityChanged(entity); + } } object->forEachDescendant([&](SpatiallyNestablePointer descendant) { From f7b7db5ab40af85246e0f63595e5de9cc7a0f0df Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Thu, 10 Jan 2019 09:14:37 -0800 Subject: [PATCH 41/43] fix warning on OSX, use correct format for float literals --- interface/src/avatar/OtherAvatar.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/interface/src/avatar/OtherAvatar.cpp b/interface/src/avatar/OtherAvatar.cpp index 569f92ac4d..854f078c03 100644 --- a/interface/src/avatar/OtherAvatar.cpp +++ b/interface/src/avatar/OtherAvatar.cpp @@ -209,9 +209,9 @@ void OtherAvatar::simulate(float deltaTime, bool inView) { _displayNameAlpha *= coef; } else { // Fading in - _displayNameAlpha = 1 - (1 - _displayNameAlpha) * coef; + _displayNameAlpha = 1.0f - (1.0f - _displayNameAlpha) * coef; } - _displayNameAlpha = abs(_displayNameAlpha - _displayNameTargetAlpha) < 0.01f ? _displayNameTargetAlpha : _displayNameAlpha; + _displayNameAlpha = absf(_displayNameAlpha - _displayNameTargetAlpha) < 0.01f ? _displayNameTargetAlpha : _displayNameAlpha; } { From 16da42723e42349b8ea4f5bd3de21168254180c5 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Thu, 10 Jan 2019 10:15:39 -0800 Subject: [PATCH 42/43] use glm::abs() instead of absf() --- interface/src/avatar/OtherAvatar.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/interface/src/avatar/OtherAvatar.cpp b/interface/src/avatar/OtherAvatar.cpp index 854f078c03..0dfc349e18 100644 --- a/interface/src/avatar/OtherAvatar.cpp +++ b/interface/src/avatar/OtherAvatar.cpp @@ -211,7 +211,7 @@ void OtherAvatar::simulate(float deltaTime, bool inView) { // Fading in _displayNameAlpha = 1.0f - (1.0f - _displayNameAlpha) * coef; } - _displayNameAlpha = absf(_displayNameAlpha - _displayNameTargetAlpha) < 0.01f ? _displayNameTargetAlpha : _displayNameAlpha; + _displayNameAlpha = glm::abs(_displayNameAlpha - _displayNameTargetAlpha) < 0.01f ? _displayNameTargetAlpha : _displayNameAlpha; } { From bc196899100fbb32aed5891e66071aff01f53643 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Thu, 10 Jan 2019 17:32:52 -0800 Subject: [PATCH 43/43] remove unnecessary culling of properties --- libraries/entities/src/EntityItem.cpp | 5 ----- 1 file changed, 5 deletions(-) diff --git a/libraries/entities/src/EntityItem.cpp b/libraries/entities/src/EntityItem.cpp index 5b7a152318..498f0ff066 100644 --- a/libraries/entities/src/EntityItem.cpp +++ b/libraries/entities/src/EntityItem.cpp @@ -179,11 +179,6 @@ OctreeElement::AppendState EntityItem::appendEntityData(OctreePacketData* packet EntityPropertyFlags propertyFlags(PROP_LAST_ITEM); EntityPropertyFlags requestedProperties = getEntityProperties(params); - // the values of these two properties are known to the receiver by context - // therefore they don't need to be packed - requestedProperties -= PROP_ENTITY_HOST_TYPE; - requestedProperties -= PROP_OWNING_AVATAR_ID; - // If we are being called for a subsequent pass at appendEntityData() that failed to completely encode this item, // then our entityTreeElementExtraEncodeData should include data about which properties we need to append. if (entityTreeElementExtraEncodeData && entityTreeElementExtraEncodeData->entities.contains(getEntityItemID())) {