consolidate two bad string checks into one

libraries/animation/src/AnimationCache.h

@@ -22,7 +22,7 @@
 
 class Animation;
 
-typedef QSharedPointer<Animation> AnimationPointer;
+using AnimationPointer = QSharedPointer<Animation>;
 
 class AnimationCache : public ResourceCache, public Dependency  {
     Q_OBJECT

libraries/entities-renderer/src/RenderableModelEntityItem.cpp

@@ -1068,6 +1068,13 @@ void RenderableModelEntityItem::copyAnimationJointDataToModel() {
     }
 }
 
+bool RenderableModelEntityItem::readyToAnimate() const {
+    return resultWithReadLock<bool>([&] {
+        float firstFrame = _animationProperties.getFirstFrame();
+        return (firstFrame <= 0.0f) || (firstFrame <= _animationProperties.getLastFrame());
+    });
+}
+
 using namespace render;
 using namespace render::entities;
 
@@ -1155,7 +1162,7 @@ void ModelEntityRenderer::animate(const TypedEntityPointer& entity) {
 
     const QVector<glm::quat>& rotations = frames[_lastKnownCurrentFrame].rotations;
     const QVector<glm::vec3>& translations = frames[_lastKnownCurrentFrame].translations;
-                
+
     jointsData.resize(_jointMapping.size());
     for (int j = 0; j < _jointMapping.size(); j++) {
         int index = _jointMapping[j];
@@ -1169,13 +1176,12 @@ void ModelEntityRenderer::animate(const TypedEntityPointer& entity) {
                 }
             } else if (index < animationJointNames.size()) {
                 QString jointName = fbxJoints[index].name; // Pushing this here so its not done on every entity, with the exceptions of those allowing for translation
-                
                 if (originalFbxIndices.contains(jointName)) {
                     // Making sure the joint names exist in the original model the animation is trying to apply onto. If they do, then remap and get it's translation.
                     int remappedIndex = originalFbxIndices[jointName] - 1; // JointIndeces seem to always start from 1 and the found index is always 1 higher than actual.
                     translationMat = glm::translate(originalFbxJoints[remappedIndex].translation);
                 }
-            } 
+            }
             glm::mat4 rotationMat;
             if (index < rotations.size()) {
                 rotationMat = glm::mat4_cast(fbxJoints[index].preRotation * rotations[index] * fbxJoints[index].postRotation);
@@ -1477,14 +1483,16 @@ void ModelEntityRenderer::doRenderUpdateSynchronousTyped(const ScenePointer& sce
     if (_animating) {
         DETAILED_PROFILE_RANGE(simulation_physics, "Animate");
 
-        if (!jointsMapped()) {
-            mapJoints(entity, model->getJointNames());
-        //else the joint have been mapped before but we have a new animation to load
-        } else if (_animation && (_animation->getURL().toString() != entity->getAnimationURL())) {
+        if (_animation && (_animation->getURL().toString() != entity->getAnimationURL())) { // bad check
+            // the joints have been mapped before but we have a new animation to load
+            _animation.reset();
             _jointMappingCompleted = false;
-            mapJoints(entity, model->getJointNames());
         }
-        if (!(entity->getAnimationFirstFrame() < 0) && !(entity->getAnimationFirstFrame() > entity->getAnimationLastFrame())) {
+
+        if (!_jointMappingCompleted) {
+            mapJoints(entity, model);
+        }
+        if (entity->readyToAnimate()) {
             animate(entity);
         }
         emit requestRenderUpdate();
@@ -1518,19 +1526,20 @@ void ModelEntityRenderer::doRender(RenderArgs* args) {
 #endif
 }
 
-void ModelEntityRenderer::mapJoints(const TypedEntityPointer& entity, const QStringList& modelJointNames) {
+void ModelEntityRenderer::mapJoints(const TypedEntityPointer& entity, const ModelPointer& model) {
     // if we don't have animation, or we're already joint mapped then bail early
-    if (!entity->hasAnimation() || jointsMapped()) {
+    if (!entity->hasAnimation()) {
         return;
     }
 
-    if (!_animation || _animation->getURL().toString() != entity->getAnimationURL()) {
+    if (!_animation) {
         _animation = DependencyManager::get<AnimationCache>()->getAnimation(entity->getAnimationURL());
     }
 
     if (_animation && _animation->isLoaded()) {
         QStringList animationJointNames = _animation->getJointNames();
 
+        auto modelJointNames = model->getJointNames();
         if (modelJointNames.size() > 0 && animationJointNames.size() > 0) {
             _jointMapping.resize(modelJointNames.size());
             for (int i = 0; i < modelJointNames.size(); i++) {

libraries/entities-renderer/src/RenderableModelEntityItem.h

@@ -120,6 +120,7 @@ private:
     bool needsUpdateModelBounds() const;
     void autoResizeJointArrays();
     void copyAnimationJointDataToModel();
+    bool readyToAnimate() const;
 
     void getCollisionGeometryResource();
     GeometryResource::Pointer _compoundShapeResource;
@@ -169,8 +170,7 @@ protected:
 
 private:
     void animate(const TypedEntityPointer& entity);
-    void mapJoints(const TypedEntityPointer& entity, const QStringList& modelJointNames);
-    bool jointsMapped() const { return _jointMappingCompleted; }
+    void mapJoints(const TypedEntityPointer& entity, const ModelPointer& model);
 
     // Transparency is handled in ModelMeshPartPayload
     virtual bool isTransparent() const override { return false; }

libraries/entities/src/ModelEntityItem.cpp

@@ -634,30 +634,6 @@ bool ModelEntityItem::getAnimationHold() const {
     });
 }
 
-void ModelEntityItem::setAnimationFirstFrame(float firstFrame) { 
-    withWriteLock([&] {
-        _animationProperties.setFirstFrame(firstFrame);
-    });
-}
-
-float ModelEntityItem::getAnimationFirstFrame() const { 
-    return resultWithReadLock<float>([&] {
-        return _animationProperties.getFirstFrame();
-    });
-}
-
-void ModelEntityItem::setAnimationLastFrame(float lastFrame) { 
-    withWriteLock([&] {
-        _animationProperties.setLastFrame(lastFrame);
-    });
-}
-
-float ModelEntityItem::getAnimationLastFrame() const { 
-    return resultWithReadLock<float>([&] {
-        return _animationProperties.getLastFrame();
-    });
-}
-
 bool ModelEntityItem::getAnimationIsPlaying() const { 
     return resultWithReadLock<bool>([&] {
         return _animationProperties.getRunning();

libraries/entities/src/ModelEntityItem.h

@@ -83,6 +83,7 @@ public:
     // Animation related items...
     AnimationPropertyGroup getAnimationProperties() const;
 
+    // TODO: audit and remove unused Animation accessors
     bool hasAnimation() const;
     QString getAnimationURL() const;
     void setAnimationURL(const QString& url);
@@ -103,12 +104,6 @@ public:
     void setRelayParentJoints(bool relayJoints);
     bool getRelayParentJoints() const;
 
-    void setAnimationFirstFrame(float firstFrame);
-    float getAnimationFirstFrame() const;
-
-    void setAnimationLastFrame(float lastFrame);
-    float getAnimationLastFrame() const;
-
     bool getAnimationIsPlaying() const;
     float getAnimationCurrentFrame() const;
     float getAnimationFPS() const;