fix more grab bugs

libraries/entities/src/EntityItem.cpp

@@ -3360,7 +3360,19 @@ void EntityItem::addGrab(GrabPointer grab) {
     enableNoBootstrap();
     SpatiallyNestable::addGrab(grab);
 
-    if (_physicsInfo && getParentID().isNull()) {
+    if (!getParentID().isNull()) {
+        return;
+    }
+
+    int jointIndex = grab->getParentJointIndex();
+    bool isFarGrab = jointIndex == FARGRAB_RIGHTHAND_INDEX
+        || jointIndex == FARGRAB_LEFTHAND_INDEX
+        || jointIndex == FARGRAB_MOUSE_INDEX;
+
+    // GRAB HACK: FarGrab doesn't work on non-dynamic things yet, but we really really want NearGrab
+    // (aka Hold) to work for such ojects, hence we filter the useAction case like so:
+    bool useAction = getDynamic() || (_physicsInfo && !isFarGrab);
+    if (useAction) {
         EntityTreePointer entityTree = getTree();
         assert(entityTree);
         EntitySimulationPointer simulation = entityTree ? entityTree->getSimulation() : nullptr;
@@ -3371,13 +3383,11 @@ void EntityItem::addGrab(GrabPointer grab) {
 
         EntityDynamicType dynamicType;
         QVariantMap arguments;
-        int grabParentJointIndex =grab->getParentJointIndex();
-        if (grabParentJointIndex == FARGRAB_RIGHTHAND_INDEX || grabParentJointIndex == FARGRAB_LEFTHAND_INDEX ||
-            grabParentJointIndex == FARGRAB_MOUSE_INDEX) {
+        if (isFarGrab) {
             // add a far-grab action
             dynamicType = DYNAMIC_TYPE_FAR_GRAB;
             arguments["otherID"] = grab->getOwnerID();
-            arguments["otherJointIndex"] = grabParentJointIndex;
+            arguments["otherJointIndex"] = jointIndex;
             arguments["targetPosition"] = vec3ToQMap(grab->getPositionalOffset());
             arguments["targetRotation"] = quatToQMap(grab->getRotationalOffset());
             arguments["linearTimeScale"] = 0.05;
@@ -3404,7 +3414,16 @@ void EntityItem::addGrab(GrabPointer grab) {
 }
 
 void EntityItem::removeGrab(GrabPointer grab) {
+    int oldNumGrabs = _grabs.size();
     SpatiallyNestable::removeGrab(grab);
+    if (!getDynamic() && _grabs.size() != oldNumGrabs) {
+        // GRAB HACK: the expected behavior is for non-dynamic grabbed things to NOT be throwable
+        // so we slam the velocities to zero here whenever the number of grabs change.
+        // NOTE: if there is still another grab in effect this shouldn't interfere with the object's motion
+        // because that grab will slam the position+velocities next frame.
+        setLocalVelocity(glm::vec3(0.0f));
+        setAngularVelocity(glm::vec3(0.0f));
+    }
     markDirtyFlags(Simulation::DIRTY_MOTION_TYPE);
 
     QUuid actionID = grab->getActionID();

libraries/physics/src/EntityMotionState.cpp

@@ -236,11 +236,19 @@ void EntityMotionState::getWorldTransform(btTransform& worldTrans) const {
     assert(entityTreeIsLocked());
     if (_motionType == MOTION_TYPE_KINEMATIC) {
         BT_PROFILE("kinematicIntegration");
+        uint32_t thisStep = ObjectMotionState::getWorldSimulationStep();
+        if (hasInternalKinematicChanges()) {
+            // ACTION_CAN_CONTROL_KINEMATIC_OBJECT_HACK: This kinematic body was moved by an Action
+            // and doesn't require transform update because the body is authoritative and its transform
+            // has already been copied out --> do no kinematic integration.
+            clearInternalKinematicChanges();
+            _lastKinematicStep = thisStep;
+            return;
+        }
         // This is physical kinematic motion which steps strictly by the subframe count
         // of the physics simulation and uses full gravity for acceleration.
         _entity->setAcceleration(_entity->getGravity());
 
-        uint32_t thisStep = ObjectMotionState::getWorldSimulationStep();
         float dt = (thisStep - _lastKinematicStep) * PHYSICS_ENGINE_FIXED_SUBSTEP;
         _lastKinematicStep = thisStep;
         _entity->stepKinematicMotion(dt);

libraries/physics/src/ObjectMotionState.h

@@ -160,8 +160,9 @@ public:
 
     bool hasInternalKinematicChanges() const { return _hasInternalKinematicChanges; }
 
-    void dirtyInternalKinematicChanges() { _hasInternalKinematicChanges = true; }
-    void clearInternalKinematicChanges() { _hasInternalKinematicChanges = false; }
+    // these methods are declared const so they can be called inside other const methods
+    void dirtyInternalKinematicChanges() const { _hasInternalKinematicChanges = true; }
+    void clearInternalKinematicChanges() const { _hasInternalKinematicChanges = false; }
 
     virtual bool isLocallyOwned() const { return false; }
     virtual bool isLocallyOwnedOrShouldBe() const { return false; } // aka shouldEmitCollisionEvents()
@@ -185,8 +186,11 @@ protected:
     btRigidBody* _body { nullptr };
     float _density { 1.0f };
 
+    // ACTION_CAN_CONTROL_KINEMATIC_OBJECT_HACK: These date members allow an Action
+    // to operate on a kinematic object without screwing up our default kinematic integration
+    // which is done in the MotionState::getWorldTransform().
     mutable uint32_t _lastKinematicStep;
-    bool _hasInternalKinematicChanges { false };
+    mutable bool _hasInternalKinematicChanges { false };
 };
 
 using SetOfMotionStates = QSet<ObjectMotionState*>;

libraries/physics/src/ThreadSafeDynamicsWorld.cpp

@@ -100,9 +100,11 @@ void ThreadSafeDynamicsWorld::synchronizeMotionState(btRigidBody* body) {
     if (body->isKinematicObject()) {
         ObjectMotionState* objectMotionState = static_cast<ObjectMotionState*>(body->getMotionState());
         if (objectMotionState->hasInternalKinematicChanges()) {
-            // this is a special case where the kinematic motion has been updated by an Action
-            // so we supply the body's current transform to the MotionState
-            objectMotionState->clearInternalKinematicChanges();
+            // ACTION_CAN_CONTROL_KINEMATIC_OBJECT_HACK:
+            // This is a special case where the kinematic motion has been updated by an Action
+            // so we supply the body's current transform to the MotionState,
+            // but we DON'T clear the internalKinematicChanges bit here because
+            // objectMotionState.getWorldTransform() will use and clear it later
             body->getMotionState()->setWorldTransform(body->getWorldTransform());
         }
         return;