[WL21389] Some code and todo cleanup in prep for PR1.

Changes to be committed:
	modified:   libraries/entities-renderer/src/RenderableShapeEntityItem.cpp
	modified:   libraries/entities/src/ShapeEntityItem.cpp
	modified:   libraries/physics/src/ShapeFactory.cpp
	modified:   libraries/shared/src/ShapeInfo.cpp

libraries/entities-renderer/src/RenderableShapeEntityItem.cpp

@@ -114,26 +114,13 @@ void RenderableShapeEntityItem::computeShapeInfo(ShapeInfo& info) {
     break;
     case entity::Shape::Cylinder: {
         _collisionShapeType = SHAPE_TYPE_CYLINDER_Y;
-        // TODO_CUSACK: determine if rotation is axis-aligned
+        // TODO WL21389: determine if rotation is axis-aligned
         //const Transform::Quat & rot = _transform.getRotation();
 
-#if 0
+        // TODO WL21389: some way to tell apart SHAPE_TYPE_CYLINDER_Y, _X, _Z based on rotation and
-        // TODO: some way to tell apart SHAPE_TYPE_CYLINDER_Y
+        //       hull ( or dimensions, need circular cross section)
-        // TODO_CUSACK: Should allow for minor variance along axes?
+        // Should allow for minor variance along axes?
-        if ((entityDimensions.y >= entityDimensions.x) && (entityDimensions.y >= entityDimensions.z)) {
+
-        }
-        else if (entityDimensions.x >= entityDimensions.z) {
-            _collisionShapeType = SHAPE_TYPE_CYLINDER_X;
-        }
-        else if (entityDimensions.z >= entityDimensions.x) {
-            _collisionShapeType = SHAPE_TYPE_CYLINDER_Z;
-        }
-        else //...there was no major axis, treat as a hull
-        {
-            _collisionShapeType = SHAPE_TYPE_SIMPLE_HULL;
-            //TODO_CUSACK: pointCollection
-        }
-#endif
     }
     break;
     case entity::Shape::Triangle:
@@ -145,15 +132,15 @@ void RenderableShapeEntityItem::computeShapeInfo(ShapeInfo& info) {
     case entity::Shape::Dodecahedron:
     case entity::Shape::Icosahedron:
     case entity::Shape::Cone: {
-        _collisionShapeType = SHAPE_TYPE_SIMPLE_HULL;
+        //TODO WL21389: SHAPE_TYPE_SIMPLE_HULL and pointCollection (later)
-        //TODO_CUSACK: pointCollection
+        //_collisionShapeType = SHAPE_TYPE_SIMPLE_HULL;
     }
     break;
     case entity::Shape::Torus:
     {
         // Not in GeometryCache::buildShapes, unsupported.
         _collisionShapeType = SHAPE_TYPE_NONE;
-        //TODO_CUSACK: SHAPE_TYPE_SIMPLE_HULL and pointCollection (later)
+        //TODO WL21389: SHAPE_TYPE_SIMPLE_HULL and pointCollection (later if desired support)
     }
     break;
     default:{

libraries/entities/src/ShapeEntityItem.cpp

@@ -88,14 +88,14 @@ EntityItemProperties ShapeEntityItem::getProperties(EntityPropertyFlags desiredP
 
 void ShapeEntityItem::setShape(const entity::Shape& shape) {
     _shape = shape;
-    switch (_shape) { // TODO_CUSACK fill out?
+    switch (_shape) { // TODO WL21389: fill out with other shapes?
         case entity::Shape::Cube:
             _type = EntityTypes::Box;
             _collisionShapeType = ShapeType::SHAPE_TYPE_BOX;
             break;
         case entity::Shape::Sphere:
             _type = EntityTypes::Sphere;
-            _collisionShapeType = ShapeType::SHAPE_TYPE_ELLIPSOID; // TODO_CUSACK defer?  Check to see if sphere is more appropriate?
+            _collisionShapeType = ShapeType::SHAPE_TYPE_ELLIPSOID;
             break;
         default:
             _type = EntityTypes::Shape;
@@ -162,8 +162,7 @@ void ShapeEntityItem::appendSubclassData(OctreePacketData* packetData, EncodeBit
     APPEND_ENTITY_PROPERTY(PROP_ALPHA, getAlpha());
 }
 
-// This value specifes how the shape should be treated by physics calculations.  
+// This value specifes how the shape should be treated by physics calculations.
-// For now, all polys will act as spheres
 ShapeType ShapeEntityItem::getShapeType() const {
     return _collisionShapeType;
 }

libraries/physics/src/ShapeFactory.cpp

@@ -307,11 +307,13 @@ const btCollisionShape* ShapeFactory::createShapeFromInfo(const ShapeInfo& info)
             const btVector3 btHalfExtents(halfExtents.x, halfExtents.y, halfExtents.z);
             shape = new btCylinderShapeX(btHalfExtents);
         }
+        break;
         case SHAPE_TYPE_CYLINDER_Z: {
             const glm::vec3 halfExtents = info.getHalfExtents();
             const btVector3 btHalfExtents(halfExtents.x, halfExtents.y, halfExtents.z);
             shape = new btCylinderShapeZ(btHalfExtents);
         }
+        break;
         case SHAPE_TYPE_CYLINDER_Y: {
             const glm::vec3 halfExtents = info.getHalfExtents();
             const btVector3 btHalfExtents(halfExtents.x, halfExtents.y, halfExtents.z);

libraries/shared/src/ShapeInfo.cpp

@@ -29,7 +29,7 @@ void ShapeInfo::clear() {
 }
 
 void ShapeInfo::setParams(ShapeType type, const glm::vec3& halfExtents, QString url) {
-    //TODO_CUSACK: Does this need additional cases and handling added?
+    //TODO WL21389: Does this need additional cases and handling added?
     _url = "";
     _type = type;
     setHalfExtents(halfExtents);
@@ -56,8 +56,9 @@ void ShapeInfo::setParams(ShapeType type, const glm::vec3& halfExtents, QString
 }
 
 void ShapeInfo::setBox(const glm::vec3& halfExtents) {
-    //TODO_CUSACK:  Should this pointlist clearance added in case
+    //TODO WL21389:  Should this pointlist clearance added in case
     //              this is a re-purposed instance?
+    // See https://github.com/highfidelity/hifi/pull/11024#discussion_r128885491
     _url = "";
     _type = SHAPE_TYPE_BOX;
     setHalfExtents(halfExtents);
@@ -65,8 +66,7 @@ void ShapeInfo::setBox(const glm::vec3& halfExtents) {
 }
 
 void ShapeInfo::setSphere(float radius) {
-    //TODO_CUSACK:  Should this pointlist clearance added in case
+    //TODO WL21389: See comment in setBox regarding clearance
-    //              this is a re-purposed instance?
     _url = "";
     _type = SHAPE_TYPE_SPHERE;
     radius = glm::max(radius, MIN_HALF_EXTENT);
@@ -75,17 +75,14 @@ void ShapeInfo::setSphere(float radius) {
 }
 
 void ShapeInfo::setPointCollection(const ShapeInfo::PointCollection& pointCollection) {
-    //TODO_CUSACK:  Should this have protection against inadvertant clearance and type
+    //TODO WL21389: May need to skip resetting type here.
-    // resetting?   If for some reason this was called and point list was and is emtpy
-    //              would we still wish to clear out everything?
     _pointCollection = pointCollection;
     _type = (_pointCollection.size() > 0) ? SHAPE_TYPE_COMPOUND : SHAPE_TYPE_NONE;
     _doubleHashKey.clear();
 }
 
 void ShapeInfo::setCapsuleY(float radius, float halfHeight) {
-    //TODO_CUSACK:  Should this pointlist clearance added in case
+    //TODO WL21389: See comment in setBox regarding clearance
-    //              this is a re-purposed instance?
     _url = "";
     _type = SHAPE_TYPE_CAPSULE_Y;
     radius = glm::max(radius, MIN_HALF_EXTENT);
@@ -127,7 +124,7 @@ int ShapeInfo::getLargestSubshapePointCount() const {
 }
 
 float ShapeInfo::computeVolume() const {
-    //TODO_CUSACK: Add support for other ShapeTypes.
+    //TODO WL21389: Add support for other ShapeTypes.
     const float DEFAULT_VOLUME = 1.0f;
     float volume = DEFAULT_VOLUME;
     switch(_type) {
@@ -161,7 +158,7 @@ float ShapeInfo::computeVolume() const {
 }
 
 bool ShapeInfo::contains(const glm::vec3& point) const {
-    //TODO_CUSACK:  Add support for other ShapeTypes like Ellipsoid/Compound.
+    //TODO WL21389:  Add support for other ShapeTypes like Ellipsoid/Compound.
     switch(_type) {
         case SHAPE_TYPE_SPHERE:
             return glm::length(point) <= _halfExtents.x;
@@ -206,6 +203,7 @@ bool ShapeInfo::contains(const glm::vec3& point) const {
 }
 
 const DoubleHashKey& ShapeInfo::getHash() const {
+    //TODO WL21389: Need to include the pointlist for SIMPLE_HULL in hash
     // NOTE: we cache the key so we only ever need to compute it once for any valid ShapeInfo instance.
     if (_doubleHashKey.isNull() && _type != SHAPE_TYPE_NONE) {
         bool useOffset = glm::length2(_offset) > MIN_SHAPE_OFFSET * MIN_SHAPE_OFFSET;