CR feedback

assignment-client/src/Agent.cpp

@@ -499,7 +499,8 @@ void Agent::processAgentAvatar() {
     if (!_scriptEngine->isFinished() && _isAvatar) {
         auto scriptedAvatar = DependencyManager::get<ScriptableAvatar>();
 
-        QByteArray avatarByteArray = scriptedAvatar->toByteArray(true, randFloat() < AVATAR_SEND_FULL_UPDATE_RATIO);
+        QByteArray avatarByteArray = scriptedAvatar->toByteArray((randFloat() < AVATAR_SEND_FULL_UPDATE_RATIO) 
+                                                                    ? AvatarData::SendAllData : AvatarData::CullSmallData);
         scriptedAvatar->doneEncoding(true);
 
         static AvatarDataSequenceNumber sequenceNumber = 0;

assignment-client/src/avatars/AvatarMixer.cpp

@@ -386,17 +386,19 @@ void AvatarMixer::broadcastAvatarData() {
                     // determine if avatar is in view, to determine how much data to include...
                     glm::vec3 otherNodeBoxScale = (otherNodeData->getPosition() - otherNodeData->getGlobalBoundingBoxCorner()) * 2.0f;
                     AABox otherNodeBox(otherNodeData->getGlobalBoundingBoxCorner(), otherNodeBoxScale);
-                    bool sendMinimumForOutOfView = !nodeData->otherAvatarInView(otherNodeBox);
 
-                    if (sendMinimumForOutOfView) {
+                    AvatarData::AvatarDataDetail detail;
+                    if (!nodeData->otherAvatarInView(otherNodeBox)) {
+                        detail = AvatarData::MinimumData;
                         nodeData->incrementAvatarOutOfView();
                     } else {
+                        detail = distribution(generator) < AVATAR_SEND_FULL_UPDATE_RATIO
+                                        ? AvatarData::SendAllData : AvatarData::IncludeSmallData;
                         nodeData->incrementAvatarInView();
                     }
 
                     numAvatarDataBytes += avatarPacketList->write(otherNode->getUUID().toRfc4122());
-                    numAvatarDataBytes +=
+                    numAvatarDataBytes += avatarPacketList->write(otherAvatar.toByteArray(detail));
-                        avatarPacketList->write(otherAvatar.toByteArray(false, distribution(generator) < AVATAR_SEND_FULL_UPDATE_RATIO, sendMinimumForOutOfView));
 
                     avatarPacketList->endSegment();
             });

assignment-client/src/avatars/AvatarMixerClientData.cpp

@@ -62,10 +62,6 @@ void AvatarMixerClientData::readViewFrustumPacket(const QByteArray& message) {
     _currentViewFrustum.fromByteArray(message);
 }
 
-bool AvatarMixerClientData::otherAvatarInView(const glm::vec3& otherAvatar) {
-    return !_currentViewFrustumIsValid || _currentViewFrustum.pointIntersectsFrustum(otherAvatar);
-}
-
 bool AvatarMixerClientData::otherAvatarInView(const AABox& otherAvatarBox) {
     return !_currentViewFrustumIsValid || _currentViewFrustum.boxIntersectsKeyhole(otherAvatarBox);
 }

assignment-client/src/avatars/AvatarMixerClientData.h

@@ -95,7 +95,6 @@ public:
     void readViewFrustumPacket(const QByteArray& message);
 
     bool otherAvatarInView(const AABox& otherAvatarBox);
-    bool otherAvatarInView(const glm::vec3& otherAvatar);
 
     void resetInViewStats() { _recentOtherAvatarsInView = _recentOtherAvatarsOutOfView = 0; }
     void incrementAvatarInView() { _recentOtherAvatarsInView++; }

interface/src/avatar/MyAvatar.cpp

@@ -226,7 +226,7 @@ void MyAvatar::simulateAttachments(float deltaTime) {
     // don't update attachments here, do it in harvestResultsFromPhysicsSimulation()
 }
 
-QByteArray MyAvatar::toByteArray(bool cullSmallChanges, bool sendAll, bool sendMinimum) {
+QByteArray MyAvatar::toByteArray(AvatarDataDetail dataDetail) {
     CameraMode mode = qApp->getCamera()->getMode();
     _globalPosition = getPosition();
     _globalBoundingBoxCorner.x = _characterController.getCapsuleRadius();
@@ -237,12 +237,12 @@ QByteArray MyAvatar::toByteArray(bool cullSmallChanges, bool sendAll, bool sendM
         // fake the avatar position that is sent up to the AvatarMixer
         glm::vec3 oldPosition = getPosition();
         setPosition(getSkeletonPosition());
-        QByteArray array = AvatarData::toByteArray(cullSmallChanges, sendAll, sendMinimum);
+        QByteArray array = AvatarData::toByteArray(dataDetail);
         // copy the correct position back
         setPosition(oldPosition);
         return array;
     }
-    return AvatarData::toByteArray(cullSmallChanges, sendAll, sendMinimum);
+    return AvatarData::toByteArray(dataDetail);
 }
 
 void MyAvatar::centerBody() {

interface/src/avatar/MyAvatar.h

@@ -333,7 +333,7 @@ private:
 
     glm::vec3 getWorldBodyPosition() const;
     glm::quat getWorldBodyOrientation() const;
-    QByteArray toByteArray(bool cullSmallChanges, bool sendAll, bool sendMinimum) override;
+    QByteArray toByteArray(AvatarDataDetail dataDetail) override;
     void simulate(float deltaTime);
     void updateFromTrackers(float deltaTime);
     virtual void render(RenderArgs* renderArgs, const glm::vec3& cameraPositio) override;

libraries/avatars/src/AvatarData.cpp

@@ -181,7 +181,11 @@ void AvatarData::setHandPosition(const glm::vec3& handPosition) {
     _handPosition = glm::inverse(getOrientation()) * (handPosition - getPosition());
 }
 
-QByteArray AvatarData::toByteArray(bool cullSmallChanges, bool sendAll, bool sendMinimum) {
+
+QByteArray AvatarData::toByteArray(AvatarDataDetail dataDetail) {
+    bool cullSmallChanges = (dataDetail == CullSmallData);
+    bool sendAll = (dataDetail == SendAllData);
+    bool sendMinimum = (dataDetail == MinimumData);
     // TODO: DRY this up to a shared method
     // that can pack any type given the number of bytes
     // and return the number of bytes to push the pointer
@@ -1208,9 +1212,9 @@ void AvatarData::sendAvatarDataPacket() {
 
     // about 2% of the time, we send a full update (meaning, we transmit all the joint data), even if nothing has changed.
     // this is to guard against a joint moving once, the packet getting lost, and the joint never moving again.
-    bool sendFullUpdate = randFloat() < AVATAR_SEND_FULL_UPDATE_RATIO;
+    QByteArray avatarByteArray = toByteArray((randFloat() < AVATAR_SEND_FULL_UPDATE_RATIO) ? SendAllData : CullSmallData);
-    QByteArray avatarByteArray = toByteArray(true, sendFullUpdate);
+
-    doneEncoding(true);
+    doneEncoding(true); // FIXME - doneEncoding() takes a bool for culling small changes, that's janky!
 
     static AvatarDataSequenceNumber sequenceNumber = 0;
 

libraries/avatars/src/AvatarData.h

@@ -209,7 +209,14 @@ public:
     glm::vec3 getHandPosition() const;
     void setHandPosition(const glm::vec3& handPosition);
 
-    virtual QByteArray toByteArray(bool cullSmallChanges, bool sendAll, bool sendMinimum = false);
+    typedef enum { 
+        MinimumData, 
+        CullSmallData,
+        IncludeSmallData,
+        SendAllData
+    } AvatarDataDetail;
+
+    virtual QByteArray toByteArray(AvatarDataDetail dataDetail);
     virtual void doneEncoding(bool cullSmallChanges);
 
     /// \return true if an error should be logged