From 71440dfbc551f9a33278bb3e4b25c0286094684c Mon Sep 17 00:00:00 2001
From: Andrew Meadows <andrew@highfidelity.io>
Date: Fri, 10 Jul 2015 11:30:11 -0700
Subject: [PATCH 1/6] add checks for NaN

---
 .../entities/src/EntityActionInterface.cpp    | 34 +++++++++++++------
 1 file changed, 23 insertions(+), 11 deletions(-)

diff --git a/libraries/entities/src/EntityActionInterface.cpp b/libraries/entities/src/EntityActionInterface.cpp
index 2b723d4e15..ba7f3afea4 100644
--- a/libraries/entities/src/EntityActionInterface.cpp
+++ b/libraries/entities/src/EntityActionInterface.cpp
@@ -127,21 +127,21 @@ glm::vec3 EntityActionInterface::extractVec3Argument(QString objectName, QVarian
             qDebug() << objectName << "requires argument:" << argumentName;
         }
         ok = false;
-        return glm::vec3();
+        return glm::vec3(0.0f);
     }
 
     QVariant resultV = arguments[argumentName];
     if (resultV.type() != (QVariant::Type) QMetaType::QVariantMap) {
         qDebug() << objectName << "argument" << argumentName << "must be a map";
         ok = false;
-        return glm::vec3();
+        return glm::vec3(0.0f);
     }
 
     QVariantMap resultVM = resultV.toMap();
     if (!resultVM.contains("x") || !resultVM.contains("y") || !resultVM.contains("z")) {
-        qDebug() << objectName << "argument" << argumentName << "must be a map with keys of x, y, z";
+        qDebug() << objectName << "argument" << argumentName << "must be a map with keys: x, y, z";
         ok = false;
-        return glm::vec3();
+        return glm::vec3(0.0f);
     }
 
     QVariant xV = resultVM["x"];
@@ -155,9 +155,15 @@ glm::vec3 EntityActionInterface::extractVec3Argument(QString objectName, QVarian
     float y = yV.toFloat(&yOk);
     float z = zV.toFloat(&zOk);
     if (!xOk || !yOk || !zOk) {
-        qDebug() << objectName << "argument" << argumentName << "must be a map with keys of x, y, z and values of type float.";
+        qDebug() << objectName << "argument" << argumentName << "must be a map with keys: x, y, and z of type float.";
         ok = false;
-        return glm::vec3();
+        return glm::vec3(0.0f);
+    }
+
+    if (x != x || y != y || z != z) {
+        // at least one of the values is NaN
+        ok = false;
+        return glm::vec3(0.0f);
     }
 
     return glm::vec3(x, y, z);
@@ -181,8 +187,8 @@ glm::quat EntityActionInterface::extractQuatArgument(QString objectName, QVarian
     }
 
     QVariantMap resultVM = resultV.toMap();
-    if (!resultVM.contains("x") || !resultVM.contains("y") || !resultVM.contains("z")) {
-        qDebug() << objectName << "argument" << argumentName << "must be a map with keys of x, y, z";
+    if (!resultVM.contains("x") || !resultVM.contains("y") || !resultVM.contains("z") || !resultVM.contains("w")) {
+        qDebug() << objectName << "argument" << argumentName << "must be a map with keys: x, y, z, and w";
         ok = false;
         return glm::quat();
     }
@@ -202,12 +208,18 @@ glm::quat EntityActionInterface::extractQuatArgument(QString objectName, QVarian
     float w = wV.toFloat(&wOk);
     if (!xOk || !yOk || !zOk || !wOk) {
         qDebug() << objectName << "argument" << argumentName
-                 << "must be a map with keys of x, y, z, w and values of type float.";
+                 << "must be a map with keys: x, y, z, and w of type float.";
         ok = false;
         return glm::quat();
     }
 
-    return glm::quat(w, x, y, z);
+    if (x != x || y != y || z != z || w != w) {
+        // at least one of the components is NaN!
+        ok = false;
+        return glm::quat();
+    }
+
+    return glm::normalize(glm::quat(w, x, y, z));
 }
 
 float EntityActionInterface::extractFloatArgument(QString objectName, QVariantMap arguments,
@@ -224,7 +236,7 @@ float EntityActionInterface::extractFloatArgument(QString objectName, QVariantMa
     bool vOk = true;
     float v = vV.toFloat(&vOk);
 
-    if (!vOk) {
+    if (!vOk || v != v) {
         ok = false;
         return 0.0f;
     }

From 90ae1fea8b2aef2bfc1a0d3d288917202e3aa657 Mon Sep 17 00:00:00 2001
From: Andrew Meadows <andrew@highfidelity.io>
Date: Fri, 10 Jul 2015 11:30:24 -0700
Subject: [PATCH 2/6] handle case where action params are missing

---
 libraries/physics/src/ObjectActionOffset.cpp | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/libraries/physics/src/ObjectActionOffset.cpp b/libraries/physics/src/ObjectActionOffset.cpp
index 2391ded13d..00b7fe6734 100644
--- a/libraries/physics/src/ObjectActionOffset.cpp
+++ b/libraries/physics/src/ObjectActionOffset.cpp
@@ -95,26 +95,21 @@ bool ObjectActionOffset::updateArguments(QVariantMap arguments) {
     glm::vec3 pointToOffsetFrom =
         EntityActionInterface::extractVec3Argument("offset action", arguments, "pointToOffsetFrom", ok, true);
     if (!ok) {
-        return false;
+        pointToOffsetFrom = _pointToOffsetFrom;
     }
 
     ok = true;
     float linearTimeScale =
         EntityActionInterface::extractFloatArgument("offset action", arguments, "linearTimeScale", ok, false);
-    if (ok) { 
-        if (linearTimeScale <= 0.0f) {
-            qDebug() << "offset action -- linearTimeScale must be greater than zero.";
-            return false;
-        }
-    } else {
-        linearTimeScale = 0.1f;
+    if (!ok) { 
+        linearTimeScale = _linearTimeScale;
     }
 
     ok = true;
     float linearDistance =
         EntityActionInterface::extractFloatArgument("offset action", arguments, "linearDistance", ok, false);
     if (!ok) {
-        linearDistance = 0.0f;
+        linearDistance = _linearDistance;
     }
 
     // only change stuff if something actually changed

From 56f00a526d1403a1844156794e78704222f88fda Mon Sep 17 00:00:00 2001
From: Andrew Meadows <andrew@highfidelity.io>
Date: Fri, 10 Jul 2015 11:31:37 -0700
Subject: [PATCH 3/6] cleanup of ObjectActionSpring

---
 libraries/physics/src/ObjectActionSpring.cpp | 189 ++++++++-----------
 libraries/physics/src/ObjectActionSpring.h   |   1 +
 2 files changed, 78 insertions(+), 112 deletions(-)

diff --git a/libraries/physics/src/ObjectActionSpring.cpp b/libraries/physics/src/ObjectActionSpring.cpp
index cb1dd20472..21e3c19bea 100644
--- a/libraries/physics/src/ObjectActionSpring.cpp
+++ b/libraries/physics/src/ObjectActionSpring.cpp
@@ -17,14 +17,16 @@ const float SPRING_MAX_SPEED = 10.0f;
 
 const uint16_t ObjectActionSpring::springVersion = 1;
 
+
 ObjectActionSpring::ObjectActionSpring(const QUuid& id, EntityItemPointer ownerEntity) :
     ObjectAction(ACTION_TYPE_SPRING, id, ownerEntity),
     _positionalTarget(glm::vec3(0.0f)),
-    _linearTimeScale(0.2f),
-    _positionalTargetSet(false),
+    _linearTimeScale(FLT_MAX),
+    _positionalTargetSet(true),
     _rotationalTarget(glm::quat()),
-    _angularTimeScale(0.2f),
-    _rotationalTargetSet(false) {
+    _angularTimeScale(FLT_MAX),
+    _rotationalTargetSet(true),
+    _needsActivation(true) {
     #if WANT_DEBUG
     qDebug() << "ObjectActionSpring::ObjectActionSpring";
     #endif
@@ -61,130 +63,97 @@ void ObjectActionSpring::updateActionWorker(btScalar deltaTimeStep) {
         return;
     }
 
-    // handle the linear part
-    if (_positionalTargetSet) {
-        // check for NaN
-        if (_positionalTarget.x != _positionalTarget.x ||
-            _positionalTarget.y != _positionalTarget.y ||
-            _positionalTarget.z != _positionalTarget.z) {
-            qDebug() << "ObjectActionSpring::updateActionWorker -- target position includes NaN";
-            unlock();
-            lockForWrite();
-            _active = false;
-            unlock();
-            return;
-        }
-        glm::vec3 offset = _positionalTarget - bulletToGLM(rigidBody->getCenterOfMassPosition());
-        float offsetLength = glm::length(offset);
-        float speed = offsetLength / _linearTimeScale;
-
-        // cap speed
-        if (speed > SPRING_MAX_SPEED) {
-            speed = SPRING_MAX_SPEED;
-        }
-
-        if (offsetLength > IGNORE_POSITION_DELTA) {
-            glm::vec3 newVelocity = glm::normalize(offset) * speed;
-            rigidBody->setLinearVelocity(glmToBullet(newVelocity));
-            rigidBody->activate();
-        } else {
-            rigidBody->setLinearVelocity(glmToBullet(glm::vec3(0.0f)));
-        }
+    const float MAX_TIMESCALE = 600.0f; // 10 min is a long time
+    if (_linearTimeScale < MAX_TIMESCALE) {
+        btVector3 offset = rigidBody->getCenterOfMassPosition() - glmToBullet(_positionalTarget);
+        float offsetLength = offset.length();
+        //float blend = glm::min(1.0f, deltaTimeStep / _linearTimeScale);
+        float blend = 1.0f;
+        float speed = (offsetLength > FLT_EPSILON) ? glm::min(offsetLength / _linearTimeScale, SPRING_MAX_SPEED) : 0.0f;
+        rigidBody->setLinearVelocity((1.0f - blend) * rigidBody->getLinearVelocity() - (blend * speed / (offsetLength * _linearTimeScale)) * offset);
     }
 
-    // handle rotation
-    if (_rotationalTargetSet) {
-        if (_rotationalTarget.x != _rotationalTarget.x ||
-            _rotationalTarget.y != _rotationalTarget.y ||
-            _rotationalTarget.z != _rotationalTarget.z ||
-            _rotationalTarget.w != _rotationalTarget.w) {
-            qDebug() << "AvatarActionHold::updateActionWorker -- target rotation includes NaN";
-            unlock();
-            lockForWrite();
-            _active = false;
-            unlock();
-            return;
-        }
+    if (_angularTimeScale < MAX_TIMESCALE) {
+        btVector3 targetVelocity(0.0f, 0.0f, 0.0f);
 
-        glm::quat bodyRotation = bulletToGLM(rigidBody->getOrientation());
-        // if qZero and qOne are too close to each other, we can get NaN for angle.
-        auto alignmentDot = glm::dot(bodyRotation, _rotationalTarget);
-        const float almostOne = 0.99999f;
-        if (glm::abs(alignmentDot) < almostOne) {
-            glm::quat target = _rotationalTarget;
-            if (alignmentDot < 0) {
+        btQuaternion bodyRotation = rigidBody->getOrientation();
+        auto alignmentDot = bodyRotation.dot(glmToBullet(_rotationalTarget));
+        const float ALMOST_ONE = 0.99999f;
+        if (glm::abs(alignmentDot) < ALMOST_ONE) {
+            btQuaternion target = glmToBullet(_rotationalTarget);
+            if (alignmentDot < 0.0f) {
                 target = -target;
             }
-            glm::quat qZeroInverse = glm::inverse(bodyRotation);
-            glm::quat deltaQ = target * qZeroInverse;
-            glm::vec3 axis = glm::axis(deltaQ);
-            float angle = glm::angle(deltaQ);
-            assert(!isNaN(angle));
-            glm::vec3 newAngularVelocity = (angle / _angularTimeScale) * glm::normalize(axis);
-            rigidBody->setAngularVelocity(glmToBullet(newAngularVelocity));
-            rigidBody->activate();
-        } else {
-            rigidBody->setAngularVelocity(glmToBullet(glm::vec3(0.0f)));
+            // if dQ is the incremental rotation that gets an object from Q0 to Q1 then:
+            //
+            //      Q1 = dQ * Q0 
+            //
+            // solving for dQ gives:
+            //
+            //      dQ = Q1 * Q0^
+            btQuaternion deltaQ = target * bodyRotation.inverse();
+            float angle = deltaQ.getAngle();
+            const float MIN_ANGLE = 1.0e-4;
+            if (angle > MIN_ANGLE) {
+                targetVelocity = (angle / _angularTimeScale) * deltaQ.getAxis();
+            }
         }
+        //float blend = glm::min(1.0f, deltaTimeStep / _angularTimeScale);
+        float blend = 1.0f;
+        rigidBody->setAngularVelocity((1.0f - blend) * rigidBody->getAngularVelocity() + blend * targetVelocity);
+    }
+    if (_needsActivation) {
+        rigidBody->activate();
+        _needsActivation = false;
     }
-
     unlock();
 }
 
+const float MIN_TIMESCALE = 0.1f;
 
 bool ObjectActionSpring::updateArguments(QVariantMap arguments) {
     // targets are required, spring-constants are optional
-    bool ptOk = true;
+    bool ok = true;
     glm::vec3 positionalTarget =
-        EntityActionInterface::extractVec3Argument("spring action", arguments, "targetPosition", ptOk, false);
-    bool pscOk = true;
+        EntityActionInterface::extractVec3Argument("spring action", arguments, "targetPosition", ok, false);
+    if (!ok) {
+        positionalTarget = _positionalTarget;
+    }
+    ok = true;
     float linearTimeScale =
-        EntityActionInterface::extractFloatArgument("spring action", arguments, "linearTimeScale", pscOk, false);
-    if (ptOk && pscOk && linearTimeScale <= 0.0f) {
-        qDebug() << "spring action -- linearTimeScale must be greater than zero.";
-        return false;
+        EntityActionInterface::extractFloatArgument("spring action", arguments, "linearTimeScale", ok, false);
+    if (!ok || linearTimeScale <= 0.0f) {
+        linearTimeScale = _linearTimeScale;
     }
 
-    bool rtOk = true;
+    ok = true;
     glm::quat rotationalTarget =
-        EntityActionInterface::extractQuatArgument("spring action", arguments, "targetRotation", rtOk, false);
-    bool rscOk = true;
+        EntityActionInterface::extractQuatArgument("spring action", arguments, "targetRotation", ok, false);
+    if (!ok) {
+        rotationalTarget = _rotationalTarget;
+    }
+
+    ok = true;
     float angularTimeScale =
-        EntityActionInterface::extractFloatArgument("spring action", arguments, "angularTimeScale", rscOk, false);
-
-    if (!ptOk && !rtOk) {
-        qDebug() << "spring action requires at least one of targetPosition or targetRotation argument";
-        return false;
+        EntityActionInterface::extractFloatArgument("spring action", arguments, "angularTimeScale", ok, false);
+    if (!ok) {
+        angularTimeScale = _angularTimeScale;
     }
 
-    lockForWrite();
-
-    _positionalTargetSet = _rotationalTargetSet = false;
-
-    if (ptOk) {
+    if (positionalTarget != _positionalTarget
+            || linearTimeScale != _linearTimeScale
+            || rotationalTarget != _rotationalTarget
+            || angularTimeScale != _angularTimeScale) {
+        // something changed
+        lockForWrite();
         _positionalTarget = positionalTarget;
-        _positionalTargetSet = true;
-
-        if (pscOk) {
-            _linearTimeScale = linearTimeScale;
-        } else {
-            _linearTimeScale = 0.1f;
-        }
-    }
-
-    if (rtOk) {
+        _linearTimeScale = glm::max(MIN_TIMESCALE, glm::abs(linearTimeScale));
         _rotationalTarget = rotationalTarget;
-        _rotationalTargetSet = true;
-
-        if (rscOk) {
-            _angularTimeScale = angularTimeScale;
-        } else {
-            _angularTimeScale = 0.1f;
-        }
+        _angularTimeScale = glm::max(MIN_TIMESCALE, glm::abs(angularTimeScale));
+        _active = true;
+        _needsActivation = true;
+        unlock();
     }
-
-    _active = true;
-    unlock();
     return true;
 }
 
@@ -192,15 +161,11 @@ QVariantMap ObjectActionSpring::getArguments() {
     QVariantMap arguments;
     lockForRead();
 
-    if (_positionalTargetSet) {
-        arguments["linearTimeScale"] = _linearTimeScale;
-        arguments["targetPosition"] = glmToQMap(_positionalTarget);
-    }
+    arguments["linearTimeScale"] = _linearTimeScale;
+    arguments["targetPosition"] = glmToQMap(_positionalTarget);
 
-    if (_rotationalTargetSet) {
-        arguments["targetRotation"] = glmToQMap(_rotationalTarget);
-        arguments["angularTimeScale"] = _angularTimeScale;
-    }
+    arguments["targetRotation"] = glmToQMap(_rotationalTarget);
+    arguments["angularTimeScale"] = _angularTimeScale;
 
     unlock();
     return arguments;
diff --git a/libraries/physics/src/ObjectActionSpring.h b/libraries/physics/src/ObjectActionSpring.h
index caa64c3d3a..a0a829bdab 100644
--- a/libraries/physics/src/ObjectActionSpring.h
+++ b/libraries/physics/src/ObjectActionSpring.h
@@ -37,6 +37,7 @@ protected:
     glm::quat _rotationalTarget;
     float _angularTimeScale;
     bool _rotationalTargetSet;
+    bool _needsActivation;
 };
 
 #endif // hifi_ObjectActionSpring_h

From 294c1ba367fe5de853bd9f3b91de62a60150ae17 Mon Sep 17 00:00:00 2001
From: Andrew Meadows <andrew@highfidelity.io>
Date: Fri, 10 Jul 2015 12:17:21 -0700
Subject: [PATCH 4/6] always serialize ACTION_TYPE_SPRING

---
 libraries/physics/src/ObjectActionSpring.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libraries/physics/src/ObjectActionSpring.cpp b/libraries/physics/src/ObjectActionSpring.cpp
index 21e3c19bea..c1ef94296a 100644
--- a/libraries/physics/src/ObjectActionSpring.cpp
+++ b/libraries/physics/src/ObjectActionSpring.cpp
@@ -175,7 +175,7 @@ QByteArray ObjectActionSpring::serialize() const {
     QByteArray serializedActionArguments;
     QDataStream dataStream(&serializedActionArguments, QIODevice::WriteOnly);
 
-    dataStream << getType();
+    dataStream << ACTION_TYPE_SPRING;
     dataStream << getID();
     dataStream << ObjectActionSpring::springVersion;
 
@@ -195,7 +195,7 @@ void ObjectActionSpring::deserialize(QByteArray serializedArguments) {
 
     EntityActionType type;
     dataStream >> type;
-    assert(type == getType());
+    assert(type == ACTION_TYPE_SPRING);
 
     QUuid id;
     dataStream >> id;

From 9cb7e86877f297944e845a919f4d5b3362dc28d0 Mon Sep 17 00:00:00 2001
From: Andrew Meadows <andrew@highfidelity.io>
Date: Fri, 10 Jul 2015 12:18:09 -0700
Subject: [PATCH 5/6] cleanup of AvatarActionHold deserialization

---
 interface/src/avatar/AvatarActionHold.cpp | 100 ++++++++--------------
 1 file changed, 38 insertions(+), 62 deletions(-)

diff --git a/interface/src/avatar/AvatarActionHold.cpp b/interface/src/avatar/AvatarActionHold.cpp
index ba37112fe1..1e75250ee2 100644
--- a/interface/src/avatar/AvatarActionHold.cpp
+++ b/interface/src/avatar/AvatarActionHold.cpp
@@ -70,30 +70,14 @@ void AvatarActionHold::updateActionWorker(float deltaTimeStep) {
         return;
     }
 
-    // check for NaNs
-    if (position.x != position.x ||
-        position.y != position.y ||
-        position.z != position.z) {
-        qDebug() << "AvatarActionHold::updateActionWorker -- target position includes NaN";
-        return;
-    }
-    if (rotation.x != rotation.x ||
-        rotation.y != rotation.y ||
-        rotation.z != rotation.z ||
-        rotation.w != rotation.w) {
-        qDebug() << "AvatarActionHold::updateActionWorker -- target rotation includes NaN";
-        return;
-    }
-
     if (_positionalTarget != position || _rotationalTarget != rotation) {
         auto ownerEntity = _ownerEntity.lock();
         if (ownerEntity) {
             ownerEntity->setActionDataDirty(true);
         }
+        _positionalTarget = position;
+        _rotationalTarget = rotation;
     }
-
-    _positionalTarget = position;
-    _rotationalTarget = rotation;
     unlock();
 
     ObjectActionSpring::updateActionWorker(deltaTimeStep);
@@ -101,59 +85,51 @@ void AvatarActionHold::updateActionWorker(float deltaTimeStep) {
 
 
 bool AvatarActionHold::updateArguments(QVariantMap arguments) {
-    bool rPOk = true;
+    bool ok = true;
     glm::vec3 relativePosition =
-        EntityActionInterface::extractVec3Argument("hold", arguments, "relativePosition", rPOk, false);
-    bool rROk = true;
+        EntityActionInterface::extractVec3Argument("hold", arguments, "relativePosition", ok, false);
+    if (!ok) {
+        relativePosition = _relativePosition;
+    }
+
+    ok = true;
     glm::quat relativeRotation =
-        EntityActionInterface::extractQuatArgument("hold", arguments, "relativeRotation", rROk, false);
-    bool tSOk = true;
+        EntityActionInterface::extractQuatArgument("hold", arguments, "relativeRotation", ok, false);
+    if (!ok) {
+        relativeRotation = _relativeRotation;
+    }
+    
+    ok = true;
     float timeScale =
-        EntityActionInterface::extractFloatArgument("hold", arguments, "timeScale", tSOk, false);
-    bool hOk = true;
+        EntityActionInterface::extractFloatArgument("hold", arguments, "timeScale", ok, false);
+    if (!ok) {
+        timeScale = _linearTimeScale;
+    }
+
+    ok = true;
     QString hand =
-        EntityActionInterface::extractStringArgument("hold", arguments, "hand", hOk, false);
+        EntityActionInterface::extractStringArgument("hold", arguments, "hand", ok, false);
+    if (!ok || !(hand == "left" || hand == "right")) {
+        hand = _hand;
+    }
 
-    lockForWrite();
-    if (rPOk) {
+    if (relativePosition != _relativePosition
+            || relativeRotation != _relativeRotation
+            || timeScale != _linearTimeScale
+            || hand != _hand) {
+        lockForWrite();
         _relativePosition = relativePosition;
-    } else {
-        _relativePosition = glm::vec3(0.0f, 0.0f, 1.0f);
-    }
-
-    if (rROk) {
         _relativeRotation = relativeRotation;
-    } else {
-        _relativeRotation = glm::quat(0.0f, 0.0f, 0.0f, 1.0f);
-    }
+        const float MIN_TIMESCALE = 0.1f;
+        _linearTimeScale = glm::min(MIN_TIMESCALE, timeScale);
+        _angularTimeScale = _linearTimeScale;
+        _hand = hand;
 
-    if (tSOk) {
-        _linearTimeScale = timeScale;
-        _angularTimeScale = timeScale;
-    } else {
-        _linearTimeScale = 0.2f;
-        _angularTimeScale = 0.2f;
+        _mine = true;
+        _active = true;
+        _needsActivation = true;
+        unlock();
     }
-
-    if (hOk) {
-        hand = hand.toLower();
-        if (hand == "left") {
-            _hand = "left";
-        } else if (hand == "right") {
-            _hand = "right";
-        } else {
-            qDebug() << "hold action -- invalid hand argument:" << hand;
-            _hand = "right";
-        }
-    } else {
-        _hand = "right";
-    }
-
-    _mine = true;
-    _positionalTargetSet = true;
-    _rotationalTargetSet = true;
-    _active = true;
-    unlock();
     return true;
 }
 

From 9ce9c541defef086bf9c24c0e3252a0ced649e71 Mon Sep 17 00:00:00 2001
From: Andrew Meadows <andrew@highfidelity.io>
Date: Fri, 10 Jul 2015 18:45:40 -0700
Subject: [PATCH 6/6] fix activation of object when action changes

---
 interface/src/avatar/AvatarActionHold.cpp     |  2 +-
 libraries/entities/src/EntityItem.cpp         |  6 +++++-
 .../entities/src/EntityScriptingInterface.cpp |  3 +++
 libraries/physics/src/ObjectAction.cpp        |  7 +++++++
 libraries/physics/src/ObjectAction.h          |  1 +
 libraries/physics/src/ObjectActionOffset.cpp  |  6 +-----
 libraries/physics/src/ObjectActionOffset.h    |  1 -
 libraries/physics/src/ObjectActionSpring.cpp  | 20 +++++++------------
 libraries/physics/src/ObjectActionSpring.h    |  1 -
 9 files changed, 25 insertions(+), 22 deletions(-)

diff --git a/interface/src/avatar/AvatarActionHold.cpp b/interface/src/avatar/AvatarActionHold.cpp
index 1e75250ee2..0c5145b596 100644
--- a/interface/src/avatar/AvatarActionHold.cpp
+++ b/interface/src/avatar/AvatarActionHold.cpp
@@ -127,7 +127,7 @@ bool AvatarActionHold::updateArguments(QVariantMap arguments) {
 
         _mine = true;
         _active = true;
-        _needsActivation = true;
+        activateBody();
         unlock();
     }
     return true;
diff --git a/libraries/entities/src/EntityItem.cpp b/libraries/entities/src/EntityItem.cpp
index 794d0752a1..024e54dbaa 100644
--- a/libraries/entities/src/EntityItem.cpp
+++ b/libraries/entities/src/EntityItem.cpp
@@ -1496,7 +1496,7 @@ bool EntityItem::addAction(EntitySimulation* simulation, EntityActionPointer act
 
     bool result = addActionInternal(simulation, action);
     if (!result) {
-        removeAction(simulation, action->getID());
+        removeActionInternal(action->getID());
     }
 
     unlock();
@@ -1520,6 +1520,7 @@ bool EntityItem::addActionInternal(EntitySimulation* simulation, EntityActionPoi
     QByteArray newDataCache = serializeActions(success);
     if (success) {
         _allActionsDataCache = newDataCache;
+        _dirtyFlags |= EntityItem::DIRTY_PHYSICS_ACTIVATION;
     }
     return success;
 }
@@ -1537,6 +1538,7 @@ bool EntityItem::updateAction(EntitySimulation* simulation, const QUuid& actionI
     bool success = action->updateArguments(arguments);
     if (success) {
         _allActionsDataCache = serializeActions(success);
+        _dirtyFlags |= EntityItem::DIRTY_PHYSICS_ACTIVATION;
     } else {
         qDebug() << "EntityItem::updateAction failed";
     }
@@ -1572,6 +1574,7 @@ bool EntityItem::removeActionInternal(const QUuid& actionID, EntitySimulation* s
 
         bool success = true;
         _allActionsDataCache = serializeActions(success);
+        _dirtyFlags |= EntityItem::DIRTY_PHYSICS_ACTIVATION;
         return success;
     }
     return false;
@@ -1590,6 +1593,7 @@ bool EntityItem::clearActions(EntitySimulation* simulation) {
     // empty _serializedActions means no actions for the EntityItem
     _actionsToRemove.clear();
     _allActionsDataCache.clear();
+    _dirtyFlags |= EntityItem::DIRTY_PHYSICS_ACTIVATION;
     unlock();
     return true;
 }
diff --git a/libraries/entities/src/EntityScriptingInterface.cpp b/libraries/entities/src/EntityScriptingInterface.cpp
index f1c6157694..18175da1f6 100644
--- a/libraries/entities/src/EntityScriptingInterface.cpp
+++ b/libraries/entities/src/EntityScriptingInterface.cpp
@@ -543,6 +543,9 @@ bool EntityScriptingInterface::actionWorker(const QUuid& entityID,
     }
 
     bool success = actor(simulation, entity);
+    if (success) {
+        _entityTree->entityChanged(entity);
+    }
     _entityTree->unlock();
 
     // transmit the change
diff --git a/libraries/physics/src/ObjectAction.cpp b/libraries/physics/src/ObjectAction.cpp
index 101b69f03a..5205e08c62 100644
--- a/libraries/physics/src/ObjectAction.cpp
+++ b/libraries/physics/src/ObjectAction.cpp
@@ -129,3 +129,10 @@ void ObjectAction::setAngularVelocity(glm::vec3 angularVelocity) {
     rigidBody->activate();
 }
 
+void ObjectAction::activateBody() {
+    auto rigidBody = getRigidBody();
+    if (rigidBody) {
+        rigidBody->activate();
+    }
+}
+
diff --git a/libraries/physics/src/ObjectAction.h b/libraries/physics/src/ObjectAction.h
index f619657e92..f27ed9ab07 100644
--- a/libraries/physics/src/ObjectAction.h
+++ b/libraries/physics/src/ObjectAction.h
@@ -55,6 +55,7 @@ protected:
     virtual void setLinearVelocity(glm::vec3 linearVelocity);
     virtual glm::vec3 getAngularVelocity();
     virtual void setAngularVelocity(glm::vec3 angularVelocity);
+    virtual void activateBody();
 
     void lockForRead() { _lock.lockForRead(); }
     bool tryLockForRead() { return _lock.tryLockForRead(); }
diff --git a/libraries/physics/src/ObjectActionOffset.cpp b/libraries/physics/src/ObjectActionOffset.cpp
index 00b7fe6734..a00bbbd418 100644
--- a/libraries/physics/src/ObjectActionOffset.cpp
+++ b/libraries/physics/src/ObjectActionOffset.cpp
@@ -59,10 +59,6 @@ void ObjectActionOffset::updateActionWorker(btScalar deltaTimeStep) {
 
     const float MAX_LINEAR_TIMESCALE = 600.0f;  // 10 minutes is a long time
     if (_positionalTargetSet && _linearTimeScale < MAX_LINEAR_TIMESCALE) {
-        if (_needsActivation) {
-            rigidBody->activate();
-            _needsActivation = false;
-        }
         glm::vec3 objectPosition = bulletToGLM(rigidBody->getCenterOfMassPosition());
         glm::vec3 springAxis = objectPosition - _pointToOffsetFrom; // from anchor to object
         float distance = glm::length(springAxis);
@@ -122,7 +118,7 @@ bool ObjectActionOffset::updateArguments(QVariantMap arguments) {
         _linearDistance = linearDistance;
         _positionalTargetSet = true;
         _active = true;
-        _needsActivation = true;
+        activateBody();
         unlock();
     }
     return true;
diff --git a/libraries/physics/src/ObjectActionOffset.h b/libraries/physics/src/ObjectActionOffset.h
index a0190f3832..1918da6996 100644
--- a/libraries/physics/src/ObjectActionOffset.h
+++ b/libraries/physics/src/ObjectActionOffset.h
@@ -36,7 +36,6 @@ public:
     float _linearDistance;
     float _linearTimeScale;
     bool _positionalTargetSet;
-    bool _needsActivation = true;
 };
 
 #endif // hifi_ObjectActionOffset_h
diff --git a/libraries/physics/src/ObjectActionSpring.cpp b/libraries/physics/src/ObjectActionSpring.cpp
index c1ef94296a..dde47f7c4e 100644
--- a/libraries/physics/src/ObjectActionSpring.cpp
+++ b/libraries/physics/src/ObjectActionSpring.cpp
@@ -25,8 +25,7 @@ ObjectActionSpring::ObjectActionSpring(const QUuid& id, EntityItemPointer ownerE
     _positionalTargetSet(true),
     _rotationalTarget(glm::quat()),
     _angularTimeScale(FLT_MAX),
-    _rotationalTargetSet(true),
-    _needsActivation(true) {
+    _rotationalTargetSet(true) {
     #if WANT_DEBUG
     qDebug() << "ObjectActionSpring::ObjectActionSpring";
     #endif
@@ -67,10 +66,10 @@ void ObjectActionSpring::updateActionWorker(btScalar deltaTimeStep) {
     if (_linearTimeScale < MAX_TIMESCALE) {
         btVector3 offset = rigidBody->getCenterOfMassPosition() - glmToBullet(_positionalTarget);
         float offsetLength = offset.length();
-        //float blend = glm::min(1.0f, deltaTimeStep / _linearTimeScale);
-        float blend = 1.0f;
         float speed = (offsetLength > FLT_EPSILON) ? glm::min(offsetLength / _linearTimeScale, SPRING_MAX_SPEED) : 0.0f;
-        rigidBody->setLinearVelocity((1.0f - blend) * rigidBody->getLinearVelocity() - (blend * speed / (offsetLength * _linearTimeScale)) * offset);
+
+        // this action is aggresively critically damped and defeats the current velocity
+        rigidBody->setLinearVelocity((- speed / offsetLength) * offset);
     }
 
     if (_angularTimeScale < MAX_TIMESCALE) {
@@ -98,13 +97,8 @@ void ObjectActionSpring::updateActionWorker(btScalar deltaTimeStep) {
                 targetVelocity = (angle / _angularTimeScale) * deltaQ.getAxis();
             }
         }
-        //float blend = glm::min(1.0f, deltaTimeStep / _angularTimeScale);
-        float blend = 1.0f;
-        rigidBody->setAngularVelocity((1.0f - blend) * rigidBody->getAngularVelocity() + blend * targetVelocity);
-    }
-    if (_needsActivation) {
-        rigidBody->activate();
-        _needsActivation = false;
+        // this action is aggresively critically damped and defeats the current velocity
+        rigidBody->setAngularVelocity(targetVelocity);
     }
     unlock();
 }
@@ -151,7 +145,7 @@ bool ObjectActionSpring::updateArguments(QVariantMap arguments) {
         _rotationalTarget = rotationalTarget;
         _angularTimeScale = glm::max(MIN_TIMESCALE, glm::abs(angularTimeScale));
         _active = true;
-        _needsActivation = true;
+        activateBody();
         unlock();
     }
     return true;
diff --git a/libraries/physics/src/ObjectActionSpring.h b/libraries/physics/src/ObjectActionSpring.h
index a0a829bdab..caa64c3d3a 100644
--- a/libraries/physics/src/ObjectActionSpring.h
+++ b/libraries/physics/src/ObjectActionSpring.h
@@ -37,7 +37,6 @@ protected:
     glm::quat _rotationalTarget;
     float _angularTimeScale;
     bool _rotationalTargetSet;
-    bool _needsActivation;
 };
 
 #endif // hifi_ObjectActionSpring_h