Merge branch 'master' of https://github.com/highfidelity/hifi into newEntityListView

interface/src/avatar/AvatarManager.cpp

@@ -443,6 +443,11 @@ void AvatarManager::handleRemovedAvatar(const AvatarSharedPointer& removedAvatar
     avatar->die();
     queuePhysicsChange(avatar);
 
+    // remove this avatar's entities from the tree now, if we wait (as we did previously) for this Avatar's destructor
+    // it might not fire until after we create a new instance for the same remote avatar, which creates a race
+    // on the creation of entities for that avatar instance and the deletion of entities for this instance
+    avatar->removeAvatarEntitiesFromTree();
+
     if (removalReason == KillAvatarReason::TheirAvatarEnteredYourBubble) {
         emit DependencyManager::get<UsersScriptingInterface>()->enteredIgnoreRadius();
     } else if (removalReason == KillAvatarReason::AvatarDisconnected) {

libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp

@@ -131,16 +131,6 @@ Avatar::Avatar(QThread* thread) :
 }
 
 Avatar::~Avatar() {
-    auto treeRenderer = DependencyManager::get<EntityTreeRenderer>();
-    EntityTreePointer entityTree = treeRenderer ? treeRenderer->getTree() : nullptr;
-    if (entityTree) {
-        entityTree->withWriteLock([&] {
-            AvatarEntityMap avatarEntities = getAvatarEntityData();
-            for (auto entityID : avatarEntities.keys()) {
-                entityTree->deleteEntity(entityID, true, true);
-            }
-        });
-    }
     auto geometryCache = DependencyManager::get<GeometryCache>();
     if (geometryCache) {
         geometryCache->releaseID(_nameRectGeometryID);
@@ -385,6 +375,19 @@ void Avatar::updateAvatarEntities() {
     setAvatarEntityDataChanged(false);
 }
 
+void Avatar::removeAvatarEntitiesFromTree() {
+    auto treeRenderer = DependencyManager::get<EntityTreeRenderer>();
+    EntityTreePointer entityTree = treeRenderer ? treeRenderer->getTree() : nullptr;
+    if (entityTree) {
+        entityTree->withWriteLock([&] {
+            AvatarEntityMap avatarEntities = getAvatarEntityData();
+            for (auto entityID : avatarEntities.keys()) {
+                entityTree->deleteEntity(entityID, true, true);
+            }
+        });
+    }
+}
+
 void Avatar::relayJointDataToChildren() {
     forEachChild([&](SpatiallyNestablePointer child) {
         if (child->getNestableType() == NestableType::Entity) {

libraries/avatars-renderer/src/avatars-renderer/Avatar.h

@@ -73,6 +73,7 @@ public:
 
     void init();
     void updateAvatarEntities();
+    void removeAvatarEntitiesFromTree();
     void simulate(float deltaTime, bool inView);
     virtual void simulateAttachments(float deltaTime);
 

libraries/avatars/src/AvatarData.cpp

@@ -1861,9 +1861,7 @@ qint64 AvatarData::packTrait(AvatarTraits::TraitType traitType, ExtendedIODevice
 }
 
 qint64 AvatarData::packTraitInstance(AvatarTraits::TraitType traitType, AvatarTraits::TraitInstanceID traitInstanceID,
-                                     ExtendedIODevice& destination, AvatarTraits::TraitVersion traitVersion,
+                                   ExtendedIODevice& destination, AvatarTraits::TraitVersion traitVersion) {
-                                     AvatarTraits::TraitInstanceID wireInstanceID) {
-
     qint64 bytesWritten = 0;
 
     bytesWritten += destination.writePrimitive(traitType);
@@ -1872,11 +1870,7 @@ qint64 AvatarData::packTraitInstance(AvatarTraits::TraitType traitType, AvatarTr
         bytesWritten += destination.writePrimitive(traitVersion);
     }
 
-    if (!wireInstanceID.isNull()) {
-        bytesWritten += destination.write(wireInstanceID.toRfc4122());
-    } else {
     bytesWritten += destination.write(traitInstanceID.toRfc4122());
-    }
 
     if (traitType == AvatarTraits::AvatarEntity) {
         // grab a read lock on the avatar entities and check for entity data for the given ID

libraries/avatars/src/AvatarData.h

@@ -962,8 +962,7 @@ public:
     qint64 packTrait(AvatarTraits::TraitType traitType, ExtendedIODevice& destination,
                      AvatarTraits::TraitVersion traitVersion = AvatarTraits::NULL_TRAIT_VERSION);
     qint64 packTraitInstance(AvatarTraits::TraitType traitType, AvatarTraits::TraitInstanceID instanceID,
-                             ExtendedIODevice& destination, AvatarTraits::TraitVersion traitVersion = AvatarTraits::NULL_TRAIT_VERSION,
+                             ExtendedIODevice& destination, AvatarTraits::TraitVersion traitVersion = AvatarTraits::NULL_TRAIT_VERSION);
-                             AvatarTraits::TraitInstanceID wireInstanceID = AvatarTraits::TraitInstanceID());
 
     void prepareResetTraitInstances();
 
@@ -1193,9 +1192,6 @@ public:
     void setReplicaIndex(int replicaIndex) { _replicaIndex = replicaIndex; }
     int getReplicaIndex() { return _replicaIndex; }
 
-    const AvatarTraits::TraitInstanceID getTraitInstanceXORID() const { return _traitInstanceXORID; }
-    void cycleTraitInstanceXORID() { _traitInstanceXORID = QUuid::createUuid(); }
-
 signals:
 
     /**jsdoc
@@ -1502,8 +1498,6 @@ private:
     // privatize the copy constructor and assignment operator so they cannot be called
     AvatarData(const AvatarData&);
     AvatarData& operator= (const AvatarData&);
-
-    AvatarTraits::TraitInstanceID _traitInstanceXORID { QUuid::createUuid() };
 };
 Q_DECLARE_METATYPE(AvatarData*)
 

libraries/avatars/src/AvatarHashMap.cpp

@@ -86,8 +86,7 @@ void AvatarReplicas::processDeletedTraitInstance(const QUuid& parentID, AvatarTr
     if (_replicasMap.find(parentID) != _replicasMap.end()) {
         auto &replicas = _replicasMap[parentID];
         for (auto avatar : replicas) {
-            avatar->processDeletedTraitInstance(traitType,
+            avatar->processDeletedTraitInstance(traitType, instanceID);
-                                                AvatarTraits::xoredInstanceID(instanceID, avatar->getTraitInstanceXORID()));
         }
     }
 }
@@ -96,9 +95,7 @@ void AvatarReplicas::processTraitInstance(const QUuid& parentID, AvatarTraits::T
     if (_replicasMap.find(parentID) != _replicasMap.end()) {
         auto &replicas = _replicasMap[parentID];
         for (auto avatar : replicas) {
-            avatar->processTraitInstance(traitType,
+            avatar->processTraitInstance(traitType, instanceID, traitBinaryData);
-                                         AvatarTraits::xoredInstanceID(instanceID, avatar->getTraitInstanceXORID()),
-                                         traitBinaryData);
         }
     }
 }
@@ -346,28 +343,16 @@ void AvatarHashMap::processBulkAvatarTraits(QSharedPointer<ReceivedMessage> mess
                 AvatarTraits::TraitInstanceID traitInstanceID =
                     QUuid::fromRfc4122(message->readWithoutCopy(NUM_BYTES_RFC4122_UUID));
 
-                // XOR the incoming trait instance ID with this avatar object's personal XOR ID
-
-                // this ensures that we have separate entity instances in the local tree
-                // if we briefly end up with two Avatar objects for this node
-
-                // (which can occur if the shared pointer for the
-                // previous instance of an avatar hasn't yet gone out of scope before the
-                // new instance is created)
-
-                auto xoredInstanceID = AvatarTraits::xoredInstanceID(traitInstanceID, avatar->getTraitInstanceXORID());
-
                 message->readPrimitive(&traitBinarySize);
 
                 auto& processedInstanceVersion = lastProcessedVersions.getInstanceValueRef(traitType, traitInstanceID);
                 if (packetTraitVersion > processedInstanceVersion) {
-                    // in order to handle re-connections to the avatar mixer when the other
                     if (traitBinarySize == AvatarTraits::DELETED_TRAIT_SIZE) {
-                        avatar->processDeletedTraitInstance(traitType, xoredInstanceID);
+                        avatar->processDeletedTraitInstance(traitType, traitInstanceID);
                         _replicas.processDeletedTraitInstance(avatarID, traitType, traitInstanceID);
                     } else {
                         auto traitData = message->read(traitBinarySize);
-                        avatar->processTraitInstance(traitType, xoredInstanceID, traitData);
+                        avatar->processTraitInstance(traitType, traitInstanceID, traitData);
                         _replicas.processTraitInstance(avatarID, traitType, traitInstanceID, traitData);
                     }
                     processedInstanceVersion = packetTraitVersion;

libraries/avatars/src/AvatarTraits.h

@@ -41,8 +41,7 @@ namespace AvatarTraits {
     const TraitWireSize DELETED_TRAIT_SIZE = -1;
 
     inline qint64 packInstancedTraitDelete(TraitType traitType, TraitInstanceID instanceID, ExtendedIODevice& destination,
-                                           TraitVersion traitVersion = NULL_TRAIT_VERSION,
+                                         TraitVersion traitVersion = NULL_TRAIT_VERSION) {
-                                           TraitInstanceID xoredInstanceID = TraitInstanceID()) {
         qint64 bytesWritten = 0;
 
         bytesWritten += destination.writePrimitive(traitType);
@@ -51,28 +50,12 @@ namespace AvatarTraits {
             bytesWritten += destination.writePrimitive(traitVersion);
         }
 
-        if (xoredInstanceID.isNull()) {
         bytesWritten += destination.write(instanceID.toRfc4122());
-        } else {
-            bytesWritten += destination.write(xoredInstanceID.toRfc4122());
-        }
 
         bytesWritten += destination.writePrimitive(DELETED_TRAIT_SIZE);
 
         return bytesWritten;
     }
-
-    inline TraitInstanceID xoredInstanceID(TraitInstanceID localInstanceID, TraitInstanceID xorKeyID) {
-        QByteArray xoredInstanceID { NUM_BYTES_RFC4122_UUID, 0 };
-        auto xorKeyIDBytes = xorKeyID.toRfc4122();
-        auto localInstanceIDBytes = localInstanceID.toRfc4122();
-
-        for (auto i = 0; i < localInstanceIDBytes.size(); ++i) {
-            xoredInstanceID[i] = localInstanceIDBytes[i] ^ xorKeyIDBytes[i];
-        }
-
-        return QUuid::fromRfc4122(xoredInstanceID);
-    }
 };
 
 #endif // hifi_AvatarTraits_h

libraries/avatars/src/ClientTraitsHandler.cpp

@@ -43,9 +43,6 @@ void ClientTraitsHandler::resetForNewMixer() {
 
     // pre-fill the instanced statuses that we will need to send next frame
     _owningAvatar->prepareResetTraitInstances();
-
-    // reset the trait XOR ID since we're resetting for a new avatar mixer
-    _owningAvatar->cycleTraitInstanceXORID();
 }
 
 void ClientTraitsHandler::sendChangedTraitsToMixer() {
@@ -96,19 +93,11 @@ void ClientTraitsHandler::sendChangedTraitsToMixer() {
                     || instanceIDValuePair.value == Updated) {
                     // this is a changed trait we need to send or we haven't send out trait information yet
                     // ask the owning avatar to pack it
-
+                    _owningAvatar->packTraitInstance(instancedIt->traitType, instanceIDValuePair.id, *traitsPacketList);
-                    // since this is going to the mixer, use the XORed instance ID (to anonymize trait instance IDs
-                    // that would typically persist across sessions)
-                    _owningAvatar->packTraitInstance(instancedIt->traitType, instanceIDValuePair.id, *traitsPacketList,
-                                                     AvatarTraits::NULL_TRAIT_VERSION,
-                                                     AvatarTraits::xoredInstanceID(instanceIDValuePair.id,
-                                                                                   _owningAvatar->getTraitInstanceXORID()));
                 } else if (!_shouldPerformInitialSend && instanceIDValuePair.value == Deleted) {
                     // pack delete for this trait instance
                     AvatarTraits::packInstancedTraitDelete(instancedIt->traitType, instanceIDValuePair.id,
-                                                           *traitsPacketList, AvatarTraits::NULL_TRAIT_VERSION,
+                                                           *traitsPacketList);
-                                                           AvatarTraits::xoredInstanceID(instanceIDValuePair.id,
-                                                                                         _owningAvatar->getTraitInstanceXORID()));
                 }
             }
 

libraries/avatars/src/ClientTraitsHandler.h

@@ -30,9 +30,9 @@ public:
 
     void markTraitUpdated(AvatarTraits::TraitType updatedTrait)
         { _traitStatuses[updatedTrait] = Updated; _hasChangedTraits = true; }
-    void markInstancedTraitUpdated(AvatarTraits::TraitType traitType, AvatarTraits::TraitInstanceID updatedInstanceID)
+    void markInstancedTraitUpdated(AvatarTraits::TraitType traitType, QUuid updatedInstanceID)
         { _traitStatuses.instanceInsert(traitType, updatedInstanceID, Updated); _hasChangedTraits = true; }
-    void markInstancedTraitDeleted(AvatarTraits::TraitType traitType, AvatarTraits::TraitInstanceID deleteInstanceID)
+    void markInstancedTraitDeleted(AvatarTraits::TraitType traitType, QUuid deleteInstanceID)
         { _traitStatuses.instanceInsert(traitType, deleteInstanceID, Deleted); _hasChangedTraits = true; }
 
     void resetForNewMixer();

scripts/system/controllers/controllerModules/highlightNearbyEntities.js

@@ -37,7 +37,7 @@
         this.highlightedEntities = [];
 
         this.parameters = dispatcherUtils.makeDispatcherModuleParameters(
-            480,
+            120,
             this.hand === dispatcherUtils.RIGHT_HAND ? ["rightHand"] : ["leftHand"],
             [],
             100);

scripts/system/controllers/controllerModules/inEditMode.js

@@ -29,7 +29,7 @@ Script.include("/~/system/libraries/utils.js");
         this.reticleMaxY;
 
         this.parameters = makeDispatcherModuleParameters(
-            160,
+            200,
             this.hand === RIGHT_HAND ? ["rightHand", "rightHandEquip", "rightHandTrigger"] : ["leftHand", "leftHandEquip", "leftHandTrigger"],
             [],
             100,

scripts/system/controllers/controllerModules/inVREditMode.js

@@ -21,7 +21,7 @@ Script.include("/~/system/libraries/controllerDispatcherUtils.js");
         this.disableModules = false;
         var NO_HAND_LASER = -1; // Invalid hand parameter so that default laser is not displayed.
         this.parameters = makeDispatcherModuleParameters(
-            200, // Not too high otherwise the tablet laser doesn't work.
+            240, // Not too high otherwise the tablet laser doesn't work.
             this.hand === RIGHT_HAND
                 ? ["rightHand", "rightHandEquip", "rightHandTrigger"]
                 : ["leftHand", "leftHandEquip", "leftHandTrigger"],

scripts/system/controllers/controllerModules/nearActionGrabEntity.js

@@ -26,7 +26,7 @@ Script.include("/~/system/libraries/cloneEntityUtils.js");
         this.hapticTargetID = null;
 
         this.parameters = makeDispatcherModuleParameters(
-            500,
+            140,
             this.hand === RIGHT_HAND ? ["rightHand"] : ["leftHand"],
             [],
             100);

scripts/system/controllers/controllerModules/nearGrabHyperLinkEntity.js

@@ -21,7 +21,7 @@
         this.hyperlink = "";
 
         this.parameters = makeDispatcherModuleParameters(
-            485,
+            125,
             this.hand === RIGHT_HAND ? ["rightHand"] : ["leftHand"],
             [],
             100);

scripts/system/controllers/controllerModules/nearParentGrabEntity.js

@@ -24,6 +24,9 @@ Script.include("/~/system/libraries/controllers.js");
     // XXX this.ignoreIK = (grabbableData.ignoreIK !== undefined) ? grabbableData.ignoreIK : true;
     // XXX this.kinematicGrab = (grabbableData.kinematic !== undefined) ? grabbableData.kinematic : NEAR_GRABBING_KINEMATIC;
 
+    // this offset needs to match the one in libraries/display-plugins/src/display-plugins/hmd/HmdDisplayPlugin.cpp:378
+    var GRAB_POINT_SPHERE_OFFSET = { x: 0.04, y: 0.13, z: 0.039 };  // x = upward, y = forward, z = lateral
+
     function getGrabOffset(handController) {
         var offset = GRAB_POINT_SPHERE_OFFSET;
         if (handController === Controller.Standard.LeftHand) {
@@ -54,7 +57,7 @@ Script.include("/~/system/libraries/controllers.js");
         this.cloneAllowed = true;
 
         this.parameters = makeDispatcherModuleParameters(
-            500,
+            140,
             this.hand === RIGHT_HAND ? ["rightHand"] : ["leftHand"],
             [],
             100);

scripts/system/controllers/controllerModules/nearTrigger.js

@@ -29,7 +29,7 @@ Script.include("/~/system/libraries/controllerDispatcherUtils.js");
         this.startSent = false;
 
         this.parameters = makeDispatcherModuleParameters(
-            480,
+            120,
             this.hand === RIGHT_HAND ? ["rightHandTrigger", "rightHand"] : ["leftHandTrigger", "leftHand"],
             [],
             100);

scripts/system/controllers/controllerModules/webSurfaceLaserInput.js

@@ -22,14 +22,14 @@ Script.include("/~/system/libraries/controllers.js");
         this.running = false;
 
         this.parameters = makeDispatcherModuleParameters(
-            120,
+            160,
             this.hand === RIGHT_HAND ? ["rightHand"] : ["leftHand"],
             [],
             100,
             makeLaserParams(hand, true));
 
         this.grabModuleWantsNearbyOverlay = function(controllerData) {
-            if (controllerData.triggerValues[this.hand] > TRIGGER_ON_VALUE) {
+            if (controllerData.triggerValues[this.hand] > TRIGGER_ON_VALUE || controllerData.secondaryValues[this.hand] > BUMPER_ON_VALUE) {
                 var nearGrabName = this.hand === RIGHT_HAND ? "RightNearParentingGrabOverlay" : "LeftNearParentingGrabOverlay";
                 var nearGrabModule = getEnabledModuleByName(nearGrabName);
                 if (nearGrabModule) {
@@ -42,6 +42,23 @@ Script.include("/~/system/libraries/controllers.js");
                         return true;
                     }
                 }
+                nearGrabName = this.hand === RIGHT_HAND ? "RightNearParentingGrabEntity" : "LeftNearParentingGrabEntity";
+                nearGrabModule = getEnabledModuleByName(nearGrabName);
+                if (nearGrabModule && nearGrabModule.isReady(controllerData)) {
+                    // check for if near parent module is active.
+                    var isNearGrabModuleActive = nearGrabModule.isReady(controllerData).active;
+                    if (isNearGrabModuleActive) {
+                        // if true, return true.
+                        return isNearGrabModuleActive;
+                    } else {
+                        // check near action grab entity as a second pass.
+                        nearGrabName = this.hand === RIGHT_HAND ? "RightNearActionGrabEntity" : "LeftNearActionGrabEntity";
+                        nearGrabModule = getEnabledModuleByName(nearGrabName);
+                        if (nearGrabModule && nearGrabModule.isReady(controllerData)) {
+                            return nearGrabModule.isReady(controllerData).active;
+                        }
+                    }
+                }
             }
             return false;
         };
@@ -50,12 +67,12 @@ Script.include("/~/system/libraries/controllers.js");
             return this.hand === RIGHT_HAND ? leftOverlayLaserInput : rightOverlayLaserInput;
         };
 
-        this.isPointingAtTriggerable = function(controllerData, triggerPressed) {
+        this.isPointingAtTriggerable = function(controllerData, triggerPressed, checkEntitiesOnly) {
             // allow pointing at tablet, unlocked web entities, or web overlays automatically without pressing trigger,
             // but for pointing at locked web entities or non-web overlays user must be pressing trigger
             var intersection = controllerData.rayPicks[this.hand];
-            if (intersection.type === Picks.INTERSECTED_OVERLAY) {
             var objectID = intersection.objectID;
+            if (intersection.type === Picks.INTERSECTED_OVERLAY && !checkEntitiesOnly) {
                 if ((HMD.tabletID && objectID === HMD.tabletID) ||
                     (HMD.tabletScreenID && objectID === HMD.tabletScreenID) ||
                     (HMD.homeButtonID && objectID === HMD.homeButtonID)) {
@@ -65,9 +82,9 @@ Script.include("/~/system/libraries/controllers.js");
                     return overlayType === "web3d" || triggerPressed;
                 }
             } else if (intersection.type === Picks.INTERSECTED_ENTITY) {
-                var entityProperty = Entities.getEntityProperties(intersection.objectID);
+                var entityProperties = Entities.getEntityProperties(objectID);
-                var entityType = entityProperty.type;
+                var entityType = entityProperties.type;
-                var isLocked = entityProperty.locked;
+                var isLocked = entityProperties.locked;
                 return entityType === "Web" && (!isLocked || triggerPressed);
             }
             return false;
@@ -103,7 +120,8 @@ Script.include("/~/system/libraries/controllers.js");
             var isTriggerPressed = controllerData.triggerValues[this.hand] > TRIGGER_OFF_VALUE &&
                                    controllerData.triggerValues[this.otherHand] <= TRIGGER_OFF_VALUE;
             var allowThisModule = !otherModuleRunning || isTriggerPressed;
-            if (allowThisModule && this.isPointingAtTriggerable(controllerData, isTriggerPressed)) {
+
+            if (allowThisModule && this.isPointingAtTriggerable(controllerData, isTriggerPressed, false)) {
                 this.updateAllwaysOn();
                 if (isTriggerPressed) {
                     this.dominantHandOverride = true; // Override dominant hand.
@@ -121,13 +139,27 @@ Script.include("/~/system/libraries/controllers.js");
             otherModuleRunning = otherModuleRunning && this.getDominantHand() !== this.hand; // Auto-swap to dominant hand.
             otherModuleRunning = otherModuleRunning || this.getOtherModule().dominantHandOverride; // Override dominant hand.
             var grabModuleNeedsToRun = this.grabModuleWantsNearbyOverlay(controllerData);
+            // only allow for non-near grab
             var allowThisModule = !otherModuleRunning && !grabModuleNeedsToRun;
             var isTriggerPressed = controllerData.triggerValues[this.hand] > TRIGGER_OFF_VALUE;
             var laserOn = isTriggerPressed || this.parameters.handLaser.allwaysOn;
-            if (allowThisModule && (laserOn && this.isPointingAtTriggerable(controllerData, isTriggerPressed))) {
+            if (allowThisModule) {
+                if (isTriggerPressed && !this.isPointingAtTriggerable(controllerData, isTriggerPressed, true)) {
+                    // if trigger is down + not pointing at a web entity, keep running web surface laser
                     this.running = true;
                     return makeRunningValues(true, [], []);
+                } else if (laserOn && this.isPointingAtTriggerable(controllerData, isTriggerPressed, false)) {
+                    // if trigger is down + pointing at a web entity/overlay, keep running web surface laser
+                    this.running = true;
+                    return makeRunningValues(true, [], []);
+                } else {
+                    this.deleteContextOverlay();
+                    this.running = false;
+                    this.dominantHandOverride = false;
+                    return makeRunningValues(false, [], []);
                 }
+            }
+            // if module needs to stop from near grabs or other modules are running, stop it.
             this.deleteContextOverlay();
             this.running = false;
             this.dominantHandOverride = false;