gak, build busters and more CR feedback

interface/src/Application.cpp

@@ -4807,7 +4807,7 @@ mat4 Application::getHMDSensorPose() const {
 
 void Application::setPalmData(Hand* hand, const controller::Pose& pose, float deltaTime, HandData::Hand whichHand, float triggerValue) {
 
-    // NOTE: the Hand::modifyPalms() will allow the lambda to modify the palm data while ensuring some other user isn't
+    // NOTE: the Hand::modifyPalm() will allow the lambda to modify the palm data while ensuring some other user isn't
     // reading or writing to the Palms. This is definitely not the best way of handling this, and I'd like to see more
     // of this palm manipulation in the Hand class itself. But unfortunately the Hand and Palm don't knbow about
     // controller::Pose. More work is needed to clean this up.
@@ -4866,7 +4866,7 @@ void Application::setPalmData(Hand* hand, const controller::Pose& pose, float de
             palm.setTipVelocity(glm::vec3(0.0f));
         }
         palm.setTipPosition(newTipPosition);
-        palm.setTrigger(triggerValue);
+        palm.setTrigger(triggerValue); // FIXME - we want to get rid of this idea of PalmData having a trigger
     });
 }
 

interface/src/avatar/MyAvatar.cpp

@@ -548,80 +548,46 @@ void MyAvatar::updateFromTrackers(float deltaTime) {
 }
 
 
-// FIXME - this is super duper dumb... but this is how master works. When you have
-// hydras plugged in, you'll get 4 "palms" but only the number of controllers lifted
-// of the base station are considered active. So when you ask for "left" you get the
-// first active controller. If you have both controllers held up or just the left, that
-// will be correct. But if you lift the right controller, then it will be reported
-// as "left"... you also see this in the avatars hands. 
-PalmData MyAvatar::getActivePalmData(int palmIndex) const {
-    auto palms = getHandData()->getCopyOfPalms();
-
-    int numberOfPalms = palms.size();
-    int numberOfActivePalms = 0;
-    for (int i = 0; i < numberOfPalms; i++) {
-        auto palm = palms[i];
-        if (palm.isActive()) {
-            // if we've reached the requested "active" palm, then we will return it
-            if (numberOfActivePalms == palmIndex) {
-                return palm;
-            }
-            numberOfActivePalms++;
-        }
-    }
-    ;
-    return PalmData();
-}
-
-
 glm::vec3 MyAvatar::getLeftHandPosition() const {
-    const int LEFT_HAND = 0;
+    auto palmData = getHandData()->getCopyOfPalmData(HandData::LeftHand);
-    auto palmData = getActivePalmData(LEFT_HAND);
     return palmData.isValid() ? palmData.getPosition() : glm::vec3(0.0f);
 }
 
 glm::vec3 MyAvatar::getRightHandPosition() const {
-    const int RIGHT_HAND = 1;
+    auto palmData = getHandData()->getCopyOfPalmData(HandData::RightHand);
-    auto palmData = getActivePalmData(RIGHT_HAND);
     return palmData.isValid() ? palmData.getPosition() : glm::vec3(0.0f);
 }
 
 glm::vec3 MyAvatar::getLeftHandTipPosition() const {
-    const int LEFT_HAND = 0;
+    auto palmData = getHandData()->getCopyOfPalmData(HandData::LeftHand);
-    auto palmData = getActivePalmData(LEFT_HAND);
     return palmData.isValid() ? palmData.getTipPosition() : glm::vec3(0.0f);
 }
 
 glm::vec3 MyAvatar::getRightHandTipPosition() const {
-    const int RIGHT_HAND = 1;
+    auto palmData = getHandData()->getCopyOfPalmData(HandData::RightHand);
-    auto palmData = getActivePalmData(RIGHT_HAND);
     return palmData.isValid() ? palmData.getTipPosition() : glm::vec3(0.0f);
 }
 
 controller::Pose MyAvatar::getLeftHandPose() const {
-    const int LEFT_HAND = 0;
+    auto palmData = getHandData()->getCopyOfPalmData(HandData::LeftHand);
-    auto palmData = getActivePalmData(LEFT_HAND);
     return palmData.isValid() ? controller::Pose(palmData.getPosition(), palmData.getRotation(),
         palmData.getVelocity(), palmData.getRawAngularVelocityAsQuat()) : controller::Pose();
 }
 
 controller::Pose MyAvatar::getRightHandPose() const {
-    const int RIGHT_HAND = 1;
+    auto palmData = getHandData()->getCopyOfPalmData(HandData::RightHand);
-    auto palmData = getActivePalmData(RIGHT_HAND);
     return palmData.isValid() ? controller::Pose(palmData.getPosition(), palmData.getRotation(),
         palmData.getVelocity(), palmData.getRawAngularVelocityAsQuat()) : controller::Pose();
 }
 
 controller::Pose MyAvatar::getLeftHandTipPose() const {
-    const int LEFT_HAND = 0;
+    auto palmData = getHandData()->getCopyOfPalmData(HandData::LeftHand);
-    auto palmData = getActivePalmData(LEFT_HAND);
     return palmData.isValid() ? controller::Pose(palmData.getTipPosition(), palmData.getRotation(),
         palmData.getTipVelocity(), palmData.getRawAngularVelocityAsQuat()) : controller::Pose();
 }
 
 controller::Pose MyAvatar::getRightHandTipPose() const {
-    const int RIGHT_HAND = 1;
+    auto palmData = getHandData()->getCopyOfPalmData(HandData::RightHand);
-    auto palmData = getActivePalmData(RIGHT_HAND);
     return palmData.isValid() ? controller::Pose(palmData.getTipPosition(), palmData.getRotation(),
         palmData.getTipVelocity(), palmData.getRawAngularVelocityAsQuat()) : controller::Pose();
 }

libraries/avatars/src/HandData.cpp

@@ -45,8 +45,7 @@ PalmData HandData::getCopyOfPalmData(Hand hand) const {
             return palm;
         }
     }
-    PalmData noData;
+    return PalmData(); // invalid hand
-    return noData; // invalid hand
 }
 
 void HandData::getLeftRightPalmIndices(int& leftPalmIndex, int& rightPalmIndex) const {

libraries/avatars/src/HandData.h

@@ -71,15 +71,7 @@ public:
 
     /// Allows a lamda function write access to the specific palm for this Hand, this might
     /// modify the _palms vector
-    template<typename PalmModifierFunction> void modifyPalm(Hand whichHand, PalmModifierFunction callback) {
+    template<typename PalmModifierFunction> void modifyPalm(Hand whichHand, PalmModifierFunction callback);
-        QReadLocker locker(&_palmsLock);
-        for (auto& palm : _palms) {
-            if (palm.whichHand() == whichHand && palm.isValid()) {
-                callback(palm);
-                return;
-            }
-        }
-    }
 
     friend class AvatarData;
 protected:
@@ -115,7 +107,7 @@ public:
     HandData::Hand whichHand() const { return _hand; }
     void setHand(HandData::Hand hand) { _hand = hand; }
 
-    void setRawRotation(const glm::quat rawRotation) { _rawRotation = rawRotation; };
+    void setRawRotation(const glm::quat& rawRotation) { _rawRotation = rawRotation; };
     glm::quat getRawRotation() const { return _rawRotation; }
     glm::quat getRotation() const { return _owningHandData->getBaseOrientation() * _rawRotation; }
     void setRawPosition(const glm::vec3& pos)  { _rawPosition = pos; }
@@ -168,9 +160,19 @@ private:
     float _trigger;
     
     bool _isActive; /// This has current valid data
-    HandData::Hand _hand;
     int _numFramesWithoutData; /// after too many frames without data, this tracked object assumed lost.
     HandData* _owningHandData;
+    HandData::Hand _hand;
 };
 
+template<typename PalmModifierFunction> void HandData::modifyPalm(Hand whichHand, PalmModifierFunction callback) {
+    QReadLocker locker(&_palmsLock);
+    for (auto& palm : _palms) {
+        if (palm.whichHand() == whichHand && palm.isValid()) {
+            callback(palm);
+            return;
+        }
+    }
+}
+
 #endif // hifi_HandData_h