CR feedback

interface/src/avatar/Avatar.cpp

@@ -1112,8 +1112,9 @@ void Avatar::setSkeletonModelURL(const QUrl& skeletonModelURL) {
 
 void Avatar::setModelURLFinished(bool success) {
     if (!success && _skeletonModelURL != AvatarData::defaultFullAvatarModelUrl()) {
-        const int MAX_SKELETON_DOWNLOAD_ATTEMPTS = 4; // NOTE: must be less than MAX_ATTEMPTS in ResourceCache.cpp
+        const int MAX_SKELETON_DOWNLOAD_ATTEMPTS = 4; // NOTE: we don't want to be as generous as ResourceCache is, we only want 4 attempts
-        if (_skeletonModel->getResourceDownloadAttempts() > MAX_SKELETON_DOWNLOAD_ATTEMPTS) {
+        if (_skeletonModel->getResourceDownloadAttemptsRemaining() <= 0 ||
+            _skeletonModel->getResourceDownloadAttempts() > MAX_SKELETON_DOWNLOAD_ATTEMPTS) {
             qCWarning(interfaceapp) << "Using default after failing to load Avatar model: " << _skeletonModelURL;
             // call _skeletonModel.setURL, but leave our copy of _skeletonModelURL alone.  This is so that
             // we don't redo this every time we receive an identity packet from the avatar with the bad url.

libraries/avatars/src/AvatarData.cpp

@@ -1454,7 +1454,7 @@ QStringList AvatarData::getJointNames() const {
     return _jointNames;
 }
 
-void AvatarData::parseAvatarIdentityPacket(QSharedPointer<ReceivedMessage> message, Identity& identityOut, quint64& messageNumberOut) {
+void AvatarData::parseAvatarIdentityPacket(const QSharedPointer<ReceivedMessage>& message, Identity& identityOut, quint64& messageNumberOut) {
     const QByteArray& data = message->getMessage();
     messageNumberOut = message->getMessageNumber();
     QDataStream packetStream(data);

libraries/avatars/src/AvatarData.h

@@ -532,7 +532,7 @@ public:
         AvatarEntityMap avatarEntityData;
     };
 
-    static void parseAvatarIdentityPacket(QSharedPointer<ReceivedMessage> message, Identity& identityOut, quint64& messageNumberOut);
+    static void parseAvatarIdentityPacket(const QSharedPointer<ReceivedMessage>& message, Identity& identityOut, quint64& messageNumberOut);
 
     // identityChanged returns true if identity has changed, false otherwise.
     // displayNameChanged returns true if displayName has changed, false otherwise.

libraries/model-networking/src/model-networking/ModelCache.h

@@ -113,6 +113,7 @@ public:
 
     QUrl getURL() const { return (bool)_resource ? _resource->getURL() : QUrl(); }
     int getResourceDownloadAttempts() { return _resource ? _resource->getDownloadAttempts() : 0; }
+    int getResourceDownloadAttemptsRemaining() { return _resource ? _resource->getDownloadAttemptsRemaining() : 0; }
 
 private:
     void startWatching();

libraries/networking/src/ReceivedMessage.cpp

@@ -27,7 +27,6 @@ ReceivedMessage::ReceivedMessage(const NLPacketList& packetList)
       _packetType(packetList.getType()),
       _packetVersion(packetList.getVersion()),
       _senderSockAddr(packetList.getSenderSockAddr()),
-      _isComplete(true),
       _messageNumber(packetList.getMessageNumber())
 {
 }

libraries/networking/src/ReceivedMessage.h

@@ -101,7 +101,7 @@ private:
     std::atomic<bool> _isComplete { true };  
     std::atomic<bool> _failed { false };
 
-    udt::Packet::MessageNumber _messageNumber { 0 };
+    const udt::Packet::MessageNumber _messageNumber; // only settable on construction
 
 };
 

libraries/networking/src/ResourceCache.cpp

@@ -621,8 +621,6 @@ void Resource::init() {
     }
 }
 
-const int MAX_ATTEMPTS = 8;
-
 void Resource::attemptRequest() {
     _startedLoading = true;
 
@@ -724,14 +722,17 @@ void Resource::handleReplyFinished() {
     } else {
         switch (result) {
             case ResourceRequest::Result::Timeout: {
-                qCDebug(networking) << "Timed out loading" << _url << "received" << _bytesReceived << "total" << _bytesTotal << "attempt:" << _attempts << "of" << MAX_ATTEMPTS;
+                qCDebug(networking) << "Timed out loading" << _url << "received" << _bytesReceived << "total" << _bytesTotal 
+                                    << "attempt:" << _attempts << "attemptsRemaining:" << _attemptsRemaining;
                 // Fall through to other cases
             }
             case ResourceRequest::Result::ServerUnavailable: {
-                qCDebug(networking) << "Server Unavailable loading" << _url << "attempt:" << _attempts << "of" << MAX_ATTEMPTS;
+                qCDebug(networking) << "Server Unavailable loading" << _url << "attempt:" << _attempts << "attemptsRemaining:" << _attemptsRemaining;
                 // retry with increasing delays
                 const int BASE_DELAY_MS = 1000;
                 if (_attempts++ < MAX_ATTEMPTS) {
+                    _attemptsRemaining--;
+
                     auto waitTime = BASE_DELAY_MS * (int)pow(2.0, _attempts);
 
                     qCDebug(networking).noquote() << "Server unavailable for" << _url << "- may retry in" << waitTime << "ms"
@@ -743,7 +744,8 @@ void Resource::handleReplyFinished() {
                 // fall through to final failure
             }
             default: {
-                qCDebug(networking) << "Error loading " << _url << "attempt:" << _attempts << "of" << MAX_ATTEMPTS;
+                _attemptsRemaining = 0;
+                qCDebug(networking) << "Error loading " << _url << "attempt:" << _attempts << "attemptsRemaining:" << _attemptsRemaining;
                 auto error = (result == ResourceRequest::Timeout) ? QNetworkReply::TimeoutError
                                                                   : QNetworkReply::UnknownNetworkError;
                 emit failed(error);

libraries/networking/src/ResourceCache.h

@@ -396,7 +396,7 @@ public:
     const QUrl& getURL() const { return _url; }
 
     int getDownloadAttempts() { return _attempts; }
-
+    int getDownloadAttemptsRemaining() { return _attemptsRemaining; }
 
 signals:
     /// Fired when the resource begins downloading.
@@ -477,6 +477,10 @@ private:
     qint64 _bytesTotal{ 0 };
     qint64 _bytes{ 0 };
     int _attempts{ 0 };
+
+    const int MAX_ATTEMPTS = 8;
+
+    int _attemptsRemaining { MAX_ATTEMPTS };
     bool _isInScript{ false };
 };
 

libraries/render-utils/src/Model.h

@@ -253,6 +253,7 @@ public:
     void renderDebugMeshBoxes(gpu::Batch& batch);
 
     int getResourceDownloadAttempts() { return _renderWatcher.getResourceDownloadAttempts(); }
+    int getResourceDownloadAttemptsRemaining() { return _renderWatcher.getResourceDownloadAttemptsRemaining(); }
 
 public slots:
     void loadURLFinished(bool success);