From 5a257eb137b3fc6043bcf4cae688cfdb19d60b1e Mon Sep 17 00:00:00 2001
From: Andrew Meadows <andrew@highfidelity.io>
Date: Tue, 14 May 2019 09:48:06 -0700
Subject: [PATCH] avoid crash for RenderableModelEntityItem with invalid model

---
 .../src/RenderableModelEntityItem.cpp               | 11 ++++++-----
 libraries/entities/src/ModelEntityItem.cpp          |  9 ++-------
 libraries/physics/src/PhysicalEntitySimulation.cpp  | 13 ++++++++++---
 3 files changed, 18 insertions(+), 15 deletions(-)

diff --git a/libraries/entities-renderer/src/RenderableModelEntityItem.cpp b/libraries/entities-renderer/src/RenderableModelEntityItem.cpp
index 7602028e36..2a677f23d8 100644
--- a/libraries/entities-renderer/src/RenderableModelEntityItem.cpp
+++ b/libraries/entities-renderer/src/RenderableModelEntityItem.cpp
@@ -361,6 +361,12 @@ void RenderableModelEntityItem::computeShapeInfo(ShapeInfo& shapeInfo) {
     const uint32_t QUAD_STRIDE = 4;
 
     ShapeType type = getShapeType();
+
+    auto model = getModel();
+    if (!model) {
+        type = SHAPE_TYPE_NONE;
+    }
+
     if (type == SHAPE_TYPE_COMPOUND) {
         updateModelBounds();
 
@@ -442,10 +448,6 @@ void RenderableModelEntityItem::computeShapeInfo(ShapeInfo& shapeInfo) {
         // to the visual model and apply them to the collision model (without regard for the
         // collision model's extents).
 
-        auto model = getModel();
-        // assert we never fall in here when model not fully loaded
-        assert(model && model->isLoaded());
-
         glm::vec3 dimensions = getScaledDimensions();
         glm::vec3 scaleToFit = dimensions / model->getHFMModel().getUnscaledMeshExtents().size();
         // multiply each point by scale before handing the point-set off to the physics engine.
@@ -461,7 +463,6 @@ void RenderableModelEntityItem::computeShapeInfo(ShapeInfo& shapeInfo) {
         adjustShapeInfoByRegistration(shapeInfo);
     } else if (type >= SHAPE_TYPE_SIMPLE_HULL && type <= SHAPE_TYPE_STATIC_MESH) {
         updateModelBounds();
-        auto model = getModel();
         // assert we never fall in here when model not fully loaded
         assert(model && model->isLoaded());
         model->updateGeometry();
diff --git a/libraries/entities/src/ModelEntityItem.cpp b/libraries/entities/src/ModelEntityItem.cpp
index cb9637acd5..f276bea05d 100644
--- a/libraries/entities/src/ModelEntityItem.cpp
+++ b/libraries/entities/src/ModelEntityItem.cpp
@@ -294,9 +294,7 @@ void ModelEntityItem::setModelURL(const QString& url) {
     withWriteLock([&] {
         if (_modelURL != url) {
             _modelURL = url;
-            if (_shapeType == SHAPE_TYPE_STATIC_MESH) {
-                _flags |= Simulation::DIRTY_SHAPE | Simulation::DIRTY_MASS;
-            }
+            _flags |= Simulation::DIRTY_SHAPE | Simulation::DIRTY_MASS;
         }
     });
 }
@@ -329,11 +327,8 @@ const Transform ModelEntityItem::getTransform(bool& success, int depth) const {
 void ModelEntityItem::setCompoundShapeURL(const QString& url) {
     withWriteLock([&] {
         if (_compoundShapeURL.get() != url) {
-            ShapeType oldType = computeTrueShapeType();
             _compoundShapeURL.set(url);
-            if (oldType != computeTrueShapeType()) {
-                _flags |= Simulation::DIRTY_SHAPE | Simulation::DIRTY_MASS;
-            }
+            _flags |= Simulation::DIRTY_SHAPE | Simulation::DIRTY_MASS;
         }
     });
 }
diff --git a/libraries/physics/src/PhysicalEntitySimulation.cpp b/libraries/physics/src/PhysicalEntitySimulation.cpp
index daa2b5d954..3c730fc6cf 100644
--- a/libraries/physics/src/PhysicalEntitySimulation.cpp
+++ b/libraries/physics/src/PhysicalEntitySimulation.cpp
@@ -249,12 +249,19 @@ void PhysicalEntitySimulation::buildMotionStatesForEntitiesThatNeedThem() {
                 btCollisionShape* shape = const_cast<btCollisionShape*>(ObjectMotionState::getShapeManager()->getShapeByKey(requestItr->shapeHash));
                 if (shape) {
                     // shape is ready at last!
-                    // But the entity's desired shape might have changed since last requested
-                    // --> rebuild the ShapeInfo to verify hash
+                    // but the entity's physics desired physics status may have changed since last requested
+                    if (!entity->shouldBePhysical()) {
+                        requestItr = _shapeRequests.erase(requestItr);
+                        continue;
+                    }
+                    // rebuild the ShapeInfo to verify hash because entity's desired shape may have changed
                     // TODO? is there a better way to do this?
                     ShapeInfo shapeInfo;
                     entity->computeShapeInfo(shapeInfo);
-                    if (shapeInfo.getHash() != requestItr->shapeHash) {
+
+                    if (shapeInfo.getType() == SHAPE_TYPE_NONE) {
+                        requestItr = _shapeRequests.erase(requestItr);
+                    } else if (shapeInfo.getHash() != requestItr->shapeHash) {
                         // bummer, the hashes are different and we no longer want the shape we've received
                         ObjectMotionState::getShapeManager()->releaseShape(shape);
                         // try again