From f25cc93936229efc139bd4e5b318cb22643353b5 Mon Sep 17 00:00:00 2001
From: Howard Stearns <howard@highfidelity.io>
Date: Fri, 16 Oct 2015 10:48:36 -0700
Subject: [PATCH 01/31] Initial prototype of exposing anim vars to javascript.

---
 interface/src/avatar/MyAvatar.h            | 12 +++
 libraries/animation/src/AnimVariant.h      |  6 ++
 libraries/animation/src/AnimVariantMap.cpp | 91 ++++++++++++++++++++++
 libraries/animation/src/Rig.cpp            | 14 ++++
 libraries/animation/src/Rig.h              |  6 ++
 5 files changed, 129 insertions(+)
 create mode 100644 libraries/animation/src/AnimVariantMap.cpp

diff --git a/interface/src/avatar/MyAvatar.h b/interface/src/avatar/MyAvatar.h
index 02c9f53082..372c0848bb 100644
--- a/interface/src/avatar/MyAvatar.h
+++ b/interface/src/avatar/MyAvatar.h
@@ -106,6 +106,18 @@ public:
     Q_INVOKABLE AnimationDetails getAnimationDetailsByRole(const QString& role);
     Q_INVOKABLE AnimationDetails getAnimationDetails(const QString& url);
     void clearJointAnimationPriorities();
+    // Adds handler(animStateDictionaryIn) => animStateDictionaryOut, which will be invoked just before each animGraph state update.
+    // The handler will be called with an animStateDictionaryIn that has all those properties specified by the (possibly empty)
+    // propertiesList argument. However for debugging, if the properties argument is null, all internal animGraph state is provided.
+    // The animStateDictionaryOut can be a different object than animStateDictionaryIn. Any properties set in animStateDictionaryOut
+    // will override those of the internal animation machinery.
+    // The animStateDictionaryIn may be shared among multiple handlers, and thus may contain additional properties specified when
+    // adding one of the other handlers. While any handler may change a value in animStateDictionaryIn (or supply different values in animStateDictionaryOut)
+    // a handler must not remove properties from animStateDictionaryIn, nor change property values that it does not intend to change.
+    // It is not specified in what order multiple handlers are called.
+    Q_INVOKABLE void addAnimationStateHandler(QScriptValue handler, QScriptValue propertiesList) { _rig->addAnimationStateHandler(handler, propertiesList); }
+    // Removes a handler previously added by addAnimationStateHandler.
+    Q_INVOKABLE void removeAnimationStateHandler(QScriptValue handler) { _rig->removeAnimationStateHandler(handler); }
 
     // get/set avatar data
     void saveData();
diff --git a/libraries/animation/src/AnimVariant.h b/libraries/animation/src/AnimVariant.h
index cb886cd369..b30a04e6bd 100644
--- a/libraries/animation/src/AnimVariant.h
+++ b/libraries/animation/src/AnimVariant.h
@@ -16,6 +16,7 @@
 #include <glm/gtx/quaternion.hpp>
 #include <map>
 #include <set>
+#include <QScriptValue>
 #include "AnimationLogging.h"
 
 class AnimVariant {
@@ -158,6 +159,11 @@ public:
 
     bool hasKey(const QString& key) const { return _map.find(key) != _map.end(); }
 
+    // Answer a Plain Old Javascript Object (for the given engine) all of our values set as properties.
+    QScriptValue animVariantMapToScriptValue(QScriptEngine* engine);
+    // Side-effect us with the value of object's own properties. (No inherited properties.)
+    void animVariantMapFromScriptValue(const QScriptValue& object);
+
 #ifdef NDEBUG
     void dump() const {
         qCDebug(animation) << "AnimVariantMap =";
diff --git a/libraries/animation/src/AnimVariantMap.cpp b/libraries/animation/src/AnimVariantMap.cpp
new file mode 100644
index 0000000000..f626e46c0f
--- /dev/null
+++ b/libraries/animation/src/AnimVariantMap.cpp
@@ -0,0 +1,91 @@
+//
+//  AnimVariantMap.cpp
+//  library/animation
+//
+//  Created by Howard Stearns on 10/15/15.
+//  Copyright (c) 2015 High Fidelity, Inc. All rights reserved.
+//
+//  Distributed under the Apache License, Version 2.0.
+//  See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html
+//
+
+#include <QScriptEngine>
+#include <QScriptValueIterator>
+#include <RegisteredMetaTypes.h>
+#include "AnimVariant.h"
+
+QScriptValue AnimVariantMap::animVariantMapToScriptValue(QScriptEngine* engine) {
+    QScriptValue target = engine->newObject();
+    for (auto& pair : _map) {
+        switch (pair.second.getType()) {
+            case AnimVariant::Type::Bool:
+                target.setProperty(pair.first, pair.second.getBool());
+                break;
+            case AnimVariant::Type::Int:
+                target.setProperty(pair.first, pair.second.getInt());
+                break;
+            case AnimVariant::Type::Float:
+                target.setProperty(pair.first, pair.second.getFloat());
+                break;
+            case AnimVariant::Type::String:
+                target.setProperty(pair.first, pair.second.getString());
+                break;
+            case AnimVariant::Type::Vec3:
+                target.setProperty(pair.first, vec3toScriptValue(engine, pair.second.getVec3()));
+                break;
+            case AnimVariant::Type::Quat:
+                target.setProperty(pair.first, quatToScriptValue(engine, pair.second.getQuat()));
+                break;
+            default:
+                // Note that we don't do mat4 in Javascript currently, and there's not yet a reason to start now.
+                assert("AnimVariant::Type" == "valid");
+        }
+    }
+    return target;
+}
+void AnimVariantMap::animVariantMapFromScriptValue(const QScriptValue& source) {
+    // POTENTIAL OPTIMIZATION: cache the types we've seen. I.e, keep a dictionary mapping property names to an enumeration of types.
+    // Whenever we identify a new outbound type in animVariantMapToScriptValue above, or a new inbound type in the code that follows here,
+    // we would enter it into the dictionary. Then switch on that type here, with the code that follow being executed only if
+    // the type is not known. One problem with that is that there is no checking that two different script use the same name differently.
+    QScriptValueIterator property(source);
+    // Note: QScriptValueIterator iterates only over source's own properties. It does not follow the prototype chain.
+    while (property.hasNext()) {
+        property.next();
+        QScriptValue value = property.value();
+        if (value.isBool()) {
+            set(property.name(), value.toBool());
+            continue;
+        } else if (value.isString()) {
+            set(property.name(), value.toString());
+            continue;
+        } else if (value.isNumber()) {
+            int asInteger = value.toInt32();
+            float asFloat = value.toNumber();
+            if (asInteger == asFloat) {
+                set(property.name(), asInteger);
+            } else {
+                set(property.name(), asFloat);
+            }
+            continue;
+        } else if (value.isObject()) {
+            QScriptValue x = value.property("x");
+            if (x.isNumber()) {
+                QScriptValue y = value.property("y");
+                if (y.isNumber()) {
+                    QScriptValue z = value.property("z");
+                    if (z.isNumber()) {
+                        QScriptValue w = value.property("w");
+                        if (w.isNumber()) {
+                            set(property.name(), glm::quat(x.toNumber(), y.toNumber(), z.toNumber(), w.toNumber()));
+                        } else {
+                            set(property.name(), glm::vec3(x.toNumber(), y.toNumber(), z.toNumber()));
+                        }
+                        continue;
+                    }
+                }
+            }
+        }
+        qCWarning(animation) << "Ignoring unrecognized data" << value.toString() << "for animation property" << property.name();
+     }
+}
diff --git a/libraries/animation/src/Rig.cpp b/libraries/animation/src/Rig.cpp
index 340c09060a..bf5edb6300 100644
--- a/libraries/animation/src/Rig.cpp
+++ b/libraries/animation/src/Rig.cpp
@@ -566,6 +566,20 @@ void Rig::updateAnimations(float deltaTime, glm::mat4 rootTransform) {
             return;
         }
 
+        if (_stateHandlers.isValid()) {
+            // TODO: iterate multiple handlers, but with one shared arg.
+            // TODO: fill the properties based on the union of requested properties. (Keep all properties objs and compute new union when add/remove handler.)
+            // TODO: check QScriptEngine::hasUncaughtException()
+            // TODO: call asynchronously (through a signal on script), so that each script is single threaded, and so we never block here.
+            //       This will require inboundMaps to be kept in the list of per-handler data.
+            QScriptEngine* engine = _stateHandlers.engine();
+            QScriptValue outboundMap = _animVars.animVariantMapToScriptValue(engine);
+            QScriptValueList args;
+            args << outboundMap;
+            QScriptValue inboundMap = _stateHandlers.call(QScriptValue(), args);
+            _animVars.animVariantMapFromScriptValue(inboundMap);
+            //qCDebug(animation) << _animVars.lookup("foo", QString("not set"));
+        }
         // evaluate the animation
         AnimNode::Triggers triggersOut;
         AnimPoseVec poses = _animNode->evaluate(_animVars, deltaTime, triggersOut);
diff --git a/libraries/animation/src/Rig.h b/libraries/animation/src/Rig.h
index 6d9f7b4688..0d74c3b956 100644
--- a/libraries/animation/src/Rig.h
+++ b/libraries/animation/src/Rig.h
@@ -37,6 +37,7 @@
 #define __hifi__Rig__
 
 #include <QObject>
+#include <QScriptValue>
 
 #include "JointState.h"  // We might want to change this (later) to something that doesn't depend on gpu, fbx and model. -HRS
 
@@ -198,6 +199,8 @@ public:
     AnimNode::ConstPointer getAnimNode() const { return _animNode; }
     AnimSkeleton::ConstPointer getAnimSkeleton() const { return _animSkeleton; }
     bool disableHands {false}; // should go away with rig animation (and Rig::inverseKinematics)
+    void addAnimationStateHandler(QScriptValue handler, QScriptValue propertiesList) { _stateHandlers = handler; }
+    void removeAnimationStateHandler(QScriptValue handler) { _stateHandlers = QScriptValue(); }
 
     bool getModelOffset(glm::vec3& modelOffsetOut) const;
 
@@ -238,6 +241,9 @@ public:
     RigRole _state = RigRole::Idle;
     float _leftHandOverlayAlpha = 0.0f;
     float _rightHandOverlayAlpha = 0.0f;
+
+private:
+    QScriptValue _stateHandlers {};
 };
 
 #endif /* defined(__hifi__Rig__) */

From 770282ad8849afdf0b5e7f87acaf89ce1f5ae54d Mon Sep 17 00:00:00 2001
From: Howard Stearns <howard@highfidelity.io>
Date: Fri, 16 Oct 2015 15:22:43 -0700
Subject: [PATCH 02/31] Javascript methods to getAvatars listing and getAvatar
 from id.

---
 interface/src/avatar/AvatarManager.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/interface/src/avatar/AvatarManager.h b/interface/src/avatar/AvatarManager.h
index 277e931419..50e585d72c 100644
--- a/interface/src/avatar/AvatarManager.h
+++ b/interface/src/avatar/AvatarManager.h
@@ -51,6 +51,10 @@ public:
     
     Q_INVOKABLE void setLocalLights(const QVector<AvatarManager::LocalLight>& localLights);
     Q_INVOKABLE QVector<AvatarManager::LocalLight> getLocalLights() const;
+    // Currently, your own avatar will be included as the null avatar id.
+    Q_INVOKABLE QVector<QUuid> getAvatars() const { return _avatarHash.keys().toVector(); }
+    // Minor Bug: A bogus avatarID answers your own avatar.
+    Q_INVOKABLE AvatarData* getAvatar(QUuid avatarID) const { return _avatarHash[avatarID].get(); }
 
     void getObjectsToDelete(VectorOfMotionStates& motionStates);
     void getObjectsToAdd(VectorOfMotionStates& motionStates);

From 38a967ac5493ba53940f8b768a0808d22f77b0f2 Mon Sep 17 00:00:00 2001
From: Howard Stearns <howard@highfidelity.io>
Date: Fri, 16 Oct 2015 16:28:11 -0700
Subject: [PATCH 03/31] Allow compiler after someone broke things.

---
 libraries/animation/src/AnimVariant.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/libraries/animation/src/AnimVariant.h b/libraries/animation/src/AnimVariant.h
index b30a04e6bd..0a91c82d80 100644
--- a/libraries/animation/src/AnimVariant.h
+++ b/libraries/animation/src/AnimVariant.h
@@ -18,6 +18,7 @@
 #include <set>
 #include <QScriptValue>
 #include "AnimationLogging.h"
+#include "StreamUtils.h"
 
 class AnimVariant {
 public:

From 2213a4bb021962ae38aa90f77d2dc0311aed2972 Mon Sep 17 00:00:00 2001
From: Howard Stearns <howard@highfidelity.io>
Date: Mon, 19 Oct 2015 20:09:48 -0700
Subject: [PATCH 04/31] Do not set (just rightHand) anim var if a script has
 done so.

---
 libraries/animation/src/Rig.cpp | 8 +++++---
 libraries/animation/src/Rig.h   | 1 +
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/libraries/animation/src/Rig.cpp b/libraries/animation/src/Rig.cpp
index 05be18b4cc..9737e8ded5 100644
--- a/libraries/animation/src/Rig.cpp
+++ b/libraries/animation/src/Rig.cpp
@@ -621,8 +621,8 @@ void Rig::updateAnimations(float deltaTime, glm::mat4 rootTransform) {
             QScriptValue outboundMap = _animVars.animVariantMapToScriptValue(engine);
             QScriptValueList args;
             args << outboundMap;
-            QScriptValue inboundMap = _stateHandlers.call(QScriptValue(), args);
-            _animVars.animVariantMapFromScriptValue(inboundMap);
+            _stateHandlersResults = _stateHandlers.call(QScriptValue(), args);
+            _animVars.animVariantMapFromScriptValue(_stateHandlersResults);
             //qCDebug(animation) << _animVars.lookup("foo", QString("not set"));
         }
         // evaluate the animation
@@ -1201,7 +1201,9 @@ void Rig::updateFromHandParameters(const HandParameters& params, float dt) {
             _animVars.set("leftHandType", (int)IKTarget::Type::HipsRelativeRotationAndPosition);
         }
         if (params.isRightEnabled) {
-            _animVars.set("rightHandPosition", rootBindPose.trans + rootBindPose.rot * yFlipHACK * params.rightPosition);
+            if (!_stateHandlersResults.property("rightHandPosition", QScriptValue::ResolveLocal).isValid()) {
+                _animVars.set("rightHandPosition", rootBindPose.trans + rootBindPose.rot * yFlipHACK * params.rightPosition);
+            }
             _animVars.set("rightHandRotation", rootBindPose.rot * yFlipHACK * params.rightOrientation);
             _animVars.set("rightHandType", (int)IKTarget::Type::RotationAndPosition);
         } else {
diff --git a/libraries/animation/src/Rig.h b/libraries/animation/src/Rig.h
index 69eedc2155..f13de90cba 100644
--- a/libraries/animation/src/Rig.h
+++ b/libraries/animation/src/Rig.h
@@ -246,6 +246,7 @@ public:
 
 private:
     QScriptValue _stateHandlers {};
+    QScriptValue _stateHandlersResults {};
 };
 
 #endif /* defined(__hifi__Rig__) */

From 3d2f00c609fc5ee48afcbaec5ffe56b4e0acc11d Mon Sep 17 00:00:00 2001
From: Howard Stearns <howard@highfidelity.io>
Date: Tue, 20 Oct 2015 17:01:45 -0700
Subject: [PATCH 05/31] Cleaner intgerface, including cleanup.

---
 libraries/animation/src/Rig.cpp | 56 +++++++++++++++++++++++----------
 libraries/animation/src/Rig.h   |  7 +++--
 2 files changed, 44 insertions(+), 19 deletions(-)

diff --git a/libraries/animation/src/Rig.cpp b/libraries/animation/src/Rig.cpp
index 2fd7364434..215e0095c0 100644
--- a/libraries/animation/src/Rig.cpp
+++ b/libraries/animation/src/Rig.cpp
@@ -13,6 +13,7 @@
 
 #include <glm/gtx/vector_angle.hpp>
 #include <queue>
+#include <QScriptValueIterator>
 
 #include <NumericalConstants.h>
 #include <DebugDraw.h>
@@ -604,6 +605,42 @@ void Rig::computeMotionAnimationState(float deltaTime, const glm::vec3& worldPos
     _lastPosition = worldPosition;
 }
 
+void Rig::addAnimationStateHandler(QScriptValue handler, QScriptValue propertiesList) {
+    _stateHandlers = handler;
+}
+void Rig::removeAnimationStateHandler(QScriptValue handler) {
+    _stateHandlersResultsToRemove = _stateHandlersResults;
+    _stateHandlers = _stateHandlersResults = QScriptValue();
+}
+void Rig::cleanupAnimationStateHandler() {
+    if (!_stateHandlersResultsToRemove.isValid()) {
+        return;
+    }
+    QScriptValueIterator property(_stateHandlersResultsToRemove);
+    while (property.hasNext()) {
+        property.next();
+        _animVars.unset(property.name());
+    }
+    _stateHandlersResultsToRemove = QScriptValue();
+}
+void Rig::updateAnimationStateHandlers() {
+    if (!_stateHandlers.isValid()) {
+        return;
+    }
+    // TODO: iterate multiple handlers, but with one shared arg.
+    // TODO: fill the properties based on the union of requested properties. (Keep all properties objs and compute new union when add/remove handler.)
+    // TODO: check QScriptEngine::hasUncaughtException()
+    // TODO: call asynchronously (through a signal on script), so that each script is single threaded, and so we never block here.
+    //       This will require inboundMaps to be kept in the list of per-handler data.
+    QScriptEngine* engine = _stateHandlers.engine();
+    QScriptValue outboundMap = _animVars.animVariantMapToScriptValue(engine);
+    QScriptValueList args;
+    args << outboundMap;
+    _stateHandlersResults = _stateHandlers.call(QScriptValue(), args);
+    _animVars.animVariantMapFromScriptValue(_stateHandlersResults);
+    //qCDebug(animation) << _animVars.lookup("foo", QString("not set"));
+}
+
 void Rig::updateAnimations(float deltaTime, glm::mat4 rootTransform) {
 
     if (_enableAnimGraph) {
@@ -611,20 +648,7 @@ void Rig::updateAnimations(float deltaTime, glm::mat4 rootTransform) {
             return;
         }
 
-        if (_stateHandlers.isValid()) {
-            // TODO: iterate multiple handlers, but with one shared arg.
-            // TODO: fill the properties based on the union of requested properties. (Keep all properties objs and compute new union when add/remove handler.)
-            // TODO: check QScriptEngine::hasUncaughtException()
-            // TODO: call asynchronously (through a signal on script), so that each script is single threaded, and so we never block here.
-            //       This will require inboundMaps to be kept in the list of per-handler data.
-            QScriptEngine* engine = _stateHandlers.engine();
-            QScriptValue outboundMap = _animVars.animVariantMapToScriptValue(engine);
-            QScriptValueList args;
-            args << outboundMap;
-            _stateHandlersResults = _stateHandlers.call(QScriptValue(), args);
-            _animVars.animVariantMapFromScriptValue(_stateHandlersResults);
-            //qCDebug(animation) << _animVars.lookup("foo", QString("not set"));
-        }
+        updateAnimationStateHandlers();
         // evaluate the animation
         AnimNode::Triggers triggersOut;
         AnimPoseVec poses = _animNode->evaluate(_animVars, deltaTime, triggersOut);
@@ -1201,9 +1225,7 @@ void Rig::updateFromHandParameters(const HandParameters& params, float dt) {
             _animVars.set("leftHandType", (int)IKTarget::Type::HipsRelativeRotationAndPosition);
         }
         if (params.isRightEnabled) {
-            if (!_stateHandlersResults.property("rightHandPosition", QScriptValue::ResolveLocal).isValid()) {
-                _animVars.set("rightHandPosition", rootBindPose.trans + rootBindPose.rot * yFlipHACK * params.rightPosition);
-            }
+            _animVars.set("rightHandPosition", rootBindPose.trans + rootBindPose.rot * yFlipHACK * params.rightPosition);
             _animVars.set("rightHandRotation", rootBindPose.rot * yFlipHACK * params.rightOrientation);
             _animVars.set("rightHandType", (int)IKTarget::Type::RotationAndPosition);
         } else {
diff --git a/libraries/animation/src/Rig.h b/libraries/animation/src/Rig.h
index 89fde9002f..3f68deef1a 100644
--- a/libraries/animation/src/Rig.h
+++ b/libraries/animation/src/Rig.h
@@ -200,12 +200,14 @@ public:
     AnimNode::ConstPointer getAnimNode() const { return _animNode; }
     AnimSkeleton::ConstPointer getAnimSkeleton() const { return _animSkeleton; }
     bool disableHands {false}; // should go away with rig animation (and Rig::inverseKinematics)
-    void addAnimationStateHandler(QScriptValue handler, QScriptValue propertiesList) { _stateHandlers = handler; }
-    void removeAnimationStateHandler(QScriptValue handler) { _stateHandlers = QScriptValue(); }
+    void addAnimationStateHandler(QScriptValue handler, QScriptValue propertiesList);
+    void removeAnimationStateHandler(QScriptValue handler);
+    void cleanupAnimationStateHandler();
 
     bool getModelOffset(glm::vec3& modelOffsetOut) const;
 
  protected:
+    void updateAnimationStateHandlers();
 
     void updateLeanJoint(int index, float leanSideways, float leanForward, float torsoTwist);
     void updateNeckJoint(int index, const HeadParameters& params);
@@ -248,6 +250,7 @@ public:
 private:
     QScriptValue _stateHandlers {};
     QScriptValue _stateHandlersResults {};
+    QScriptValue _stateHandlersResultsToRemove {};
 };
 
 #endif /* defined(__hifi__Rig__) */

From 574a51e83140e7784e7ee1e7ab37d6e23bd4df5f Mon Sep 17 00:00:00 2001
From: Howard Stearns <howard@highfidelity.io>
Date: Tue, 20 Oct 2015 17:02:16 -0700
Subject: [PATCH 06/31] Use cleaner interface.

---
 interface/src/avatar/SkeletonModel.cpp | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/interface/src/avatar/SkeletonModel.cpp b/interface/src/avatar/SkeletonModel.cpp
index 28c7941c52..5c98d4cd9b 100644
--- a/interface/src/avatar/SkeletonModel.cpp
+++ b/interface/src/avatar/SkeletonModel.cpp
@@ -111,11 +111,8 @@ static const PalmData* getPalmWithIndex(Hand* hand, int index) {
 const float PALM_PRIORITY = DEFAULT_PRIORITY;
 // Called within Model::simulate call, below.
 void SkeletonModel::updateRig(float deltaTime, glm::mat4 parentTransform) {
-    if (_owningAvatar->isMyAvatar()) {
-        _rig->computeMotionAnimationState(deltaTime, _owningAvatar->getPosition(), _owningAvatar->getVelocity(), _owningAvatar->getOrientation());
-    }
-    Model::updateRig(deltaTime, parentTransform);
     Head* head = _owningAvatar->getHead();
+    _rig->cleanupAnimationStateHandler();
     if (_owningAvatar->isMyAvatar()) {
         MyAvatar* myAvatar = static_cast<MyAvatar*>(_owningAvatar);
         const FBXGeometry& geometry = _geometry->getFBXGeometry();
@@ -191,6 +188,8 @@ void SkeletonModel::updateRig(float deltaTime, glm::mat4 parentTransform) {
 
         _rig->updateFromHandParameters(handParams, deltaTime);
 
+        _rig->computeMotionAnimationState(deltaTime, _owningAvatar->getPosition(), _owningAvatar->getVelocity(), _owningAvatar->getOrientation());
+
     } else {
         // This is a little more work than we really want.
         //
@@ -212,6 +211,7 @@ void SkeletonModel::updateRig(float deltaTime, glm::mat4 parentTransform) {
                               getTranslation(), getRotation(),
                               head->getFinalOrientationInWorldFrame(), head->getCorrectedLookAtPosition());
      }
+    Model::updateRig(deltaTime, parentTransform);
 }
 
 void SkeletonModel::updateAttitude() {

From 9fd61907f51aa79d136f5cbf1cb6a7032b3654d8 Mon Sep 17 00:00:00 2001
From: Howard Stearns <howard@highfidelity.io>
Date: Wed, 21 Oct 2015 20:50:07 -0700
Subject: [PATCH 07/31] Call back to Javascript asynchronously, so that we
 don't block and the script's engine doesn't have thread conflicts.

---
 interface/src/avatar/MyAvatar.h              |  2 +
 interface/src/avatar/SkeletonModel.cpp       |  1 -
 libraries/animation/src/AnimVariant.h        |  6 ++-
 libraries/animation/src/AnimVariantMap.cpp   |  7 ++-
 libraries/animation/src/Rig.cpp              | 50 ++++++++------------
 libraries/animation/src/Rig.h                |  8 ++--
 libraries/script-engine/src/ScriptEngine.cpp | 37 +++++++++++++--
 libraries/script-engine/src/ScriptEngine.h   |  5 ++
 8 files changed, 77 insertions(+), 39 deletions(-)

diff --git a/interface/src/avatar/MyAvatar.h b/interface/src/avatar/MyAvatar.h
index 49f6c33aca..1ce72cb33f 100644
--- a/interface/src/avatar/MyAvatar.h
+++ b/interface/src/avatar/MyAvatar.h
@@ -118,6 +118,8 @@ public:
     Q_INVOKABLE void addAnimationStateHandler(QScriptValue handler, QScriptValue propertiesList) { _rig->addAnimationStateHandler(handler, propertiesList); }
     // Removes a handler previously added by addAnimationStateHandler.
     Q_INVOKABLE void removeAnimationStateHandler(QScriptValue handler) { _rig->removeAnimationStateHandler(handler); }
+    // Processes a handler result. Not really for user code, but used by invokeAnimationCallback.
+    Q_INVOKABLE void animationStateHandlerResult(QScriptValue handler, QScriptValue result) { _rig->animationStateHandlerResult(handler, result); }
 
     // get/set avatar data
     void saveData();
diff --git a/interface/src/avatar/SkeletonModel.cpp b/interface/src/avatar/SkeletonModel.cpp
index 5c98d4cd9b..003677bf90 100644
--- a/interface/src/avatar/SkeletonModel.cpp
+++ b/interface/src/avatar/SkeletonModel.cpp
@@ -112,7 +112,6 @@ const float PALM_PRIORITY = DEFAULT_PRIORITY;
 // Called within Model::simulate call, below.
 void SkeletonModel::updateRig(float deltaTime, glm::mat4 parentTransform) {
     Head* head = _owningAvatar->getHead();
-    _rig->cleanupAnimationStateHandler();
     if (_owningAvatar->isMyAvatar()) {
         MyAvatar* myAvatar = static_cast<MyAvatar*>(_owningAvatar);
         const FBXGeometry& geometry = _geometry->getFBXGeometry();
diff --git a/libraries/animation/src/AnimVariant.h b/libraries/animation/src/AnimVariant.h
index 0a91c82d80..2d140bfa52 100644
--- a/libraries/animation/src/AnimVariant.h
+++ b/libraries/animation/src/AnimVariant.h
@@ -158,12 +158,14 @@ public:
     void setTrigger(const QString& key) { _triggers.insert(key); }
     void clearTriggers() { _triggers.clear(); }
 
+    void clearMap() { _map.clear(); }
     bool hasKey(const QString& key) const { return _map.find(key) != _map.end(); }
 
     // Answer a Plain Old Javascript Object (for the given engine) all of our values set as properties.
-    QScriptValue animVariantMapToScriptValue(QScriptEngine* engine);
+    QScriptValue animVariantMapToScriptValue(QScriptEngine* engine) const;
     // Side-effect us with the value of object's own properties. (No inherited properties.)
     void animVariantMapFromScriptValue(const QScriptValue& object);
+    void copyVariantsFrom(const AnimVariantMap& other);
 
 #ifdef NDEBUG
     void dump() const {
@@ -203,4 +205,6 @@ protected:
     std::set<QString> _triggers;
 };
 
+Q_DECLARE_METATYPE(AnimVariantMap)
+
 #endif // hifi_AnimVariant_h
diff --git a/libraries/animation/src/AnimVariantMap.cpp b/libraries/animation/src/AnimVariantMap.cpp
index f626e46c0f..0154c9698a 100644
--- a/libraries/animation/src/AnimVariantMap.cpp
+++ b/libraries/animation/src/AnimVariantMap.cpp
@@ -14,7 +14,7 @@
 #include <RegisteredMetaTypes.h>
 #include "AnimVariant.h"
 
-QScriptValue AnimVariantMap::animVariantMapToScriptValue(QScriptEngine* engine) {
+QScriptValue AnimVariantMap::animVariantMapToScriptValue(QScriptEngine* engine) const {
     QScriptValue target = engine->newObject();
     for (auto& pair : _map) {
         switch (pair.second.getType()) {
@@ -43,6 +43,11 @@ QScriptValue AnimVariantMap::animVariantMapToScriptValue(QScriptEngine* engine)
     }
     return target;
 }
+void AnimVariantMap::copyVariantsFrom(const AnimVariantMap& other) {
+    for (auto& pair : other._map) {
+        _map[pair.first] = pair.second;
+    }
+}
 void AnimVariantMap::animVariantMapFromScriptValue(const QScriptValue& source) {
     // POTENTIAL OPTIMIZATION: cache the types we've seen. I.e, keep a dictionary mapping property names to an enumeration of types.
     // Whenever we identify a new outbound type in animVariantMapToScriptValue above, or a new inbound type in the code that follows here,
diff --git a/libraries/animation/src/Rig.cpp b/libraries/animation/src/Rig.cpp
index 215e0095c0..14aef88e2f 100644
--- a/libraries/animation/src/Rig.cpp
+++ b/libraries/animation/src/Rig.cpp
@@ -605,40 +605,32 @@ void Rig::computeMotionAnimationState(float deltaTime, const glm::vec3& worldPos
     _lastPosition = worldPosition;
 }
 
-void Rig::addAnimationStateHandler(QScriptValue handler, QScriptValue propertiesList) {
+// Allow script to add/remove handlers and report results, from within their thread.
+// TODO: iterate multiple handlers, but with one shared arg.
+// TODO: fill the properties based on the union of requested properties. (Keep all properties objs and compute new union when add/remove handler.)
+void Rig::addAnimationStateHandler(QScriptValue handler, QScriptValue propertiesList) { // called in script thread
     _stateHandlers = handler;
 }
-void Rig::removeAnimationStateHandler(QScriptValue handler) {
-    _stateHandlersResultsToRemove = _stateHandlersResults;
-    _stateHandlers = _stateHandlersResults = QScriptValue();
+void Rig::removeAnimationStateHandler(QScriptValue handler) { // called in script thread
+    _stateHandlers = QScriptValue();
+    QMutexLocker locker(&_stateMutex); // guarding access to results
+    _stateHandlersResults.clearMap(); // TODO: When we have multiple handlers, we'll need to clear only his handler's results.
 }
-void Rig::cleanupAnimationStateHandler() {
-    if (!_stateHandlersResultsToRemove.isValid()) {
-        return;
-    }
-    QScriptValueIterator property(_stateHandlersResultsToRemove);
-    while (property.hasNext()) {
-        property.next();
-        _animVars.unset(property.name());
-    }
-    _stateHandlersResultsToRemove = QScriptValue();
+void Rig::animationStateHandlerResult(QScriptValue handler, QScriptValue result) { // called synchronously from script
+    // handler is currently ignored but might be used in storing individual results
+    QMutexLocker locker(&_stateMutex);
+    _stateHandlersResults.animVariantMapFromScriptValue(result); // Into our own copy.
 }
-void Rig::updateAnimationStateHandlers() {
-    if (!_stateHandlers.isValid()) {
-        return;
+void Rig::updateAnimationStateHandlers() { // called on avatar update thread (which may be main thread)
+    if (_stateHandlers.isValid()) {
+        // invokeMethod makes a copy of the args, and copies of AnimVariantMap do copy the underlying map, so this will correctly capture
+        // the state of _animVars and allow continued changes to _animVars in this thread without conflict.
+        QMetaObject::invokeMethod(_stateHandlers.engine(), "invokeAnimationCallback",  Qt::QueuedConnection,
+                                  Q_ARG(QScriptValue, _stateHandlers),
+                                  Q_ARG(AnimVariantMap, _animVars));
     }
-    // TODO: iterate multiple handlers, but with one shared arg.
-    // TODO: fill the properties based on the union of requested properties. (Keep all properties objs and compute new union when add/remove handler.)
-    // TODO: check QScriptEngine::hasUncaughtException()
-    // TODO: call asynchronously (through a signal on script), so that each script is single threaded, and so we never block here.
-    //       This will require inboundMaps to be kept in the list of per-handler data.
-    QScriptEngine* engine = _stateHandlers.engine();
-    QScriptValue outboundMap = _animVars.animVariantMapToScriptValue(engine);
-    QScriptValueList args;
-    args << outboundMap;
-    _stateHandlersResults = _stateHandlers.call(QScriptValue(), args);
-    _animVars.animVariantMapFromScriptValue(_stateHandlersResults);
-    //qCDebug(animation) << _animVars.lookup("foo", QString("not set"));
+    QMutexLocker locker(&_stateMutex); // as we examine/copy most recently computed state, if any. (Typically an earlier invocation.)
+    _animVars.copyVariantsFrom(_stateHandlersResults);
 }
 
 void Rig::updateAnimations(float deltaTime, glm::mat4 rootTransform) {
diff --git a/libraries/animation/src/Rig.h b/libraries/animation/src/Rig.h
index 3f68deef1a..c29bb6ad04 100644
--- a/libraries/animation/src/Rig.h
+++ b/libraries/animation/src/Rig.h
@@ -202,7 +202,7 @@ public:
     bool disableHands {false}; // should go away with rig animation (and Rig::inverseKinematics)
     void addAnimationStateHandler(QScriptValue handler, QScriptValue propertiesList);
     void removeAnimationStateHandler(QScriptValue handler);
-    void cleanupAnimationStateHandler();
+    void animationStateHandlerResult(QScriptValue handler, QScriptValue result);
 
     bool getModelOffset(glm::vec3& modelOffsetOut) const;
 
@@ -248,9 +248,9 @@ public:
     float _rightHandOverlayAlpha = 0.0f;
 
 private:
-    QScriptValue _stateHandlers {};
-    QScriptValue _stateHandlersResults {};
-    QScriptValue _stateHandlersResultsToRemove {};
+    QScriptValue _stateHandlers;
+    AnimVariantMap _stateHandlersResults;
+    QMutex _stateMutex;
 };
 
 #endif /* defined(__hifi__Rig__) */
diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp
index 76590f266b..0ec1a09d6d 100644
--- a/libraries/script-engine/src/ScriptEngine.cpp
+++ b/libraries/script-engine/src/ScriptEngine.cpp
@@ -258,6 +258,15 @@ void ScriptEngine::errorInLoadingScript(const QUrl& url) {
     }
 }
 
+// Even though we never pass AnimVariantMap directly to and from javascript, the queued invokeMethod of
+// invokeAnimationCallback requires that the type be registered.
+static QScriptValue animVarMapToScriptValue(QScriptEngine* engine, const AnimVariantMap& parameters) {
+    return parameters.animVariantMapToScriptValue(engine);
+}
+static void animVarMapFromScriptValue(const QScriptValue& value, AnimVariantMap& parameters) {
+    parameters.animVariantMapFromScriptValue(value);
+}
+
 void ScriptEngine::init() {
     if (_isInitialized) {
         return; // only initialize once
@@ -316,6 +325,7 @@ void ScriptEngine::init() {
     registerGlobalObject("Vec3", &_vec3Library);
     registerGlobalObject("Uuid", &_uuidLibrary);
     registerGlobalObject("AnimationCache", DependencyManager::get<AnimationCache>().data());
+    qScriptRegisterMetaType(this, animVarMapToScriptValue, animVarMapFromScriptValue);
 
     // constants
     globalObject().setProperty("TREE_SCALE", newVariant(QVariant(TREE_SCALE)));
@@ -718,6 +728,21 @@ void ScriptEngine::stop() {
     }
 }
 
+// Other threads can invoke this through invokeMethod, which causes the callback to be asynchronously executed in this script's thread.
+void ScriptEngine::invokeAnimationCallback(QScriptValue callback, AnimVariantMap parameters) {
+    checkThread();
+    QScriptValue javascriptParametgers = parameters.animVariantMapToScriptValue(this);
+    QScriptValueList callingArguments;
+    callingArguments << javascriptParametgers;
+    QScriptValue result = callback.call(QScriptValue(), callingArguments);
+    // We want to give the result back to the rig, but we don't have the rig or the avatar. But the global does.
+    // This is sort of like going through DependencyManager.get.
+    QScriptValue resultHandler = globalObject().property("MyAvatar").property("animationStateHandlerResult");
+    QScriptValueList resultArguments;
+    resultArguments << callback << result;
+    resultHandler.call(QScriptValue(), resultArguments); // Call it synchronously, on our own time and thread.
+}
+
 void ScriptEngine::timerFired() {
     QTimer* callingTimer = reinterpret_cast<QTimer*>(sender());
     QScriptValue timerFunction = _timerFunctionMap.value(callingTimer);
@@ -898,14 +923,20 @@ void ScriptEngine::load(const QString& loadFile) {
     }
 }
 
-// Look up the handler associated with eventName and entityID. If found, evalute the argGenerator thunk and call the handler with those args
-void ScriptEngine::generalHandler(const EntityItemID& entityID, const QString& eventName, std::function<QScriptValueList()> argGenerator) {
+bool ScriptEngine::checkThread() const {
     if (QThread::currentThread() != thread()) {
         qDebug() << "*** ERROR *** ScriptEngine::generalHandler() called on wrong thread [" << QThread::currentThread() << "], invoking on correct thread [" << thread() << "]";
         assert(false);
+        return true;
+    }
+    return false;
+}
+
+// Look up the handler associated with eventName and entityID. If found, evalute the argGenerator thunk and call the handler with those args
+void ScriptEngine::generalHandler(const EntityItemID& entityID, const QString& eventName, std::function<QScriptValueList()> argGenerator) {
+    if (checkThread()) {
         return;
     }
-
     if (!_registeredHandlers.contains(entityID)) {
         return;
     }
diff --git a/libraries/script-engine/src/ScriptEngine.h b/libraries/script-engine/src/ScriptEngine.h
index 1d3986143a..77f1e2d5f1 100644
--- a/libraries/script-engine/src/ScriptEngine.h
+++ b/libraries/script-engine/src/ScriptEngine.h
@@ -21,6 +21,7 @@
 #include <QtScript/QScriptEngine>
 
 #include <AnimationCache.h>
+#include <AnimVariant.h>
 #include <AvatarData.h>
 #include <AvatarHashMap.h>
 #include <LimitedNodeList.h>
@@ -142,6 +143,9 @@ public:
     // NOTE - this is used by the TypedArray implemetation. we need to review this for thread safety
     ArrayBufferClass* getArrayBufferClass() { return _arrayBufferClass; }
 
+public slots:
+    void invokeAnimationCallback(QScriptValue callback, AnimVariantMap parameters);
+
 signals:
     void scriptLoaded(const QString& scriptFilename);
     void errorLoadingScript(const QString& scriptFilename);
@@ -169,6 +173,7 @@ protected:
     bool _wantSignals = true;
     QHash<EntityItemID, EntityScriptDetails> _entityScripts;
 private:
+    bool checkThread() const;
     void init();
     QString getFilename() const;
     void waitTillDoneRunning();

From b5ccd49959bbb2d8706e6edbafb8d9c5a9e90595 Mon Sep 17 00:00:00 2001
From: Howard Stearns <howard@highfidelity.io>
Date: Thu, 22 Oct 2015 11:42:50 -0700
Subject: [PATCH 08/31] Make ubuntu happy.

---
 libraries/animation/src/Rig.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/libraries/animation/src/Rig.h b/libraries/animation/src/Rig.h
index c29bb6ad04..648a7f2c16 100644
--- a/libraries/animation/src/Rig.h
+++ b/libraries/animation/src/Rig.h
@@ -37,6 +37,7 @@
 #define __hifi__Rig__
 
 #include <QObject>
+#include <QMutex>
 #include <QScriptValue>
 
 #include "JointState.h"  // We might want to change this (later) to something that doesn't depend on gpu, fbx and model. -HRS

From 30429e8138776e3f56d2effc82972fb43dd26b3b Mon Sep 17 00:00:00 2001
From: Howard Stearns <howard@highfidelity.io>
Date: Thu, 22 Oct 2015 11:43:22 -0700
Subject: [PATCH 09/31] Don't use late-breaking results that got reported after
 the handler was removed.

---
 libraries/animation/src/Rig.cpp | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/libraries/animation/src/Rig.cpp b/libraries/animation/src/Rig.cpp
index 14aef88e2f..2a64114112 100644
--- a/libraries/animation/src/Rig.cpp
+++ b/libraries/animation/src/Rig.cpp
@@ -619,6 +619,9 @@ void Rig::removeAnimationStateHandler(QScriptValue handler) { // called in scrip
 void Rig::animationStateHandlerResult(QScriptValue handler, QScriptValue result) { // called synchronously from script
     // handler is currently ignored but might be used in storing individual results
     QMutexLocker locker(&_stateMutex);
+    if (!_stateHandlers.isValid()) {
+        return; // Don't use late-breaking results that got reported after the handler was removed.
+    }
     _stateHandlersResults.animVariantMapFromScriptValue(result); // Into our own copy.
 }
 void Rig::updateAnimationStateHandlers() { // called on avatar update thread (which may be main thread)

From 759e6525069991a04005910cc36b514ffa876122 Mon Sep 17 00:00:00 2001
From: Howard Stearns <howard@highfidelity.io>
Date: Thu, 22 Oct 2015 15:13:14 -0700
Subject: [PATCH 10/31] Thread test per comments.

---
 libraries/animation/src/AnimVariantMap.cpp | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/libraries/animation/src/AnimVariantMap.cpp b/libraries/animation/src/AnimVariantMap.cpp
index 0154c9698a..219a44b644 100644
--- a/libraries/animation/src/AnimVariantMap.cpp
+++ b/libraries/animation/src/AnimVariantMap.cpp
@@ -11,10 +11,15 @@
 
 #include <QScriptEngine>
 #include <QScriptValueIterator>
+#include <QThread.h>
 #include <RegisteredMetaTypes.h>
 #include "AnimVariant.h"
 
 QScriptValue AnimVariantMap::animVariantMapToScriptValue(QScriptEngine* engine) const {
+    if (QThread::currentThread() != engine->thread()) {
+        qCWarning(animation) << "Cannot create Javacript object from non-script thread" << QThread::currentThread();
+        return QScriptValue();
+    }
     QScriptValue target = engine->newObject();
     for (auto& pair : _map) {
         switch (pair.second.getType()) {

From 1d0464ede5d118be676e8ea9daf9821ef9d6c88f Mon Sep 17 00:00:00 2001
From: Howard Stearns <howard@highfidelity.io>
Date: Thu, 22 Oct 2015 15:15:10 -0700
Subject: [PATCH 11/31] Name change and thread checks per comments.

---
 interface/src/avatar/MyAvatar.h              |  2 +-
 libraries/animation/src/Rig.cpp              |  3 +-
 libraries/script-engine/src/ScriptEngine.cpp | 29 ++++++++++----------
 libraries/script-engine/src/ScriptEngine.h   |  3 +-
 4 files changed, 19 insertions(+), 18 deletions(-)

diff --git a/interface/src/avatar/MyAvatar.h b/interface/src/avatar/MyAvatar.h
index 1ce72cb33f..4f32989bf5 100644
--- a/interface/src/avatar/MyAvatar.h
+++ b/interface/src/avatar/MyAvatar.h
@@ -118,7 +118,7 @@ public:
     Q_INVOKABLE void addAnimationStateHandler(QScriptValue handler, QScriptValue propertiesList) { _rig->addAnimationStateHandler(handler, propertiesList); }
     // Removes a handler previously added by addAnimationStateHandler.
     Q_INVOKABLE void removeAnimationStateHandler(QScriptValue handler) { _rig->removeAnimationStateHandler(handler); }
-    // Processes a handler result. Not really for user code, but used by invokeAnimationCallback.
+    // Processes a handler result. Not really for user code, but used by callAnimationStateHandler.
     Q_INVOKABLE void animationStateHandlerResult(QScriptValue handler, QScriptValue result) { _rig->animationStateHandlerResult(handler, result); }
 
     // get/set avatar data
diff --git a/libraries/animation/src/Rig.cpp b/libraries/animation/src/Rig.cpp
index 2a64114112..0437e9cca8 100644
--- a/libraries/animation/src/Rig.cpp
+++ b/libraries/animation/src/Rig.cpp
@@ -624,11 +624,12 @@ void Rig::animationStateHandlerResult(QScriptValue handler, QScriptValue result)
     }
     _stateHandlersResults.animVariantMapFromScriptValue(result); // Into our own copy.
 }
+
 void Rig::updateAnimationStateHandlers() { // called on avatar update thread (which may be main thread)
     if (_stateHandlers.isValid()) {
         // invokeMethod makes a copy of the args, and copies of AnimVariantMap do copy the underlying map, so this will correctly capture
         // the state of _animVars and allow continued changes to _animVars in this thread without conflict.
-        QMetaObject::invokeMethod(_stateHandlers.engine(), "invokeAnimationCallback",  Qt::QueuedConnection,
+        QMetaObject::invokeMethod(_stateHandlers.engine(), "callAnimationStateHandler",  Qt::QueuedConnection,
                                   Q_ARG(QScriptValue, _stateHandlers),
                                   Q_ARG(AnimVariantMap, _animVars));
     }
diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp
index 0ec1a09d6d..5808340855 100644
--- a/libraries/script-engine/src/ScriptEngine.cpp
+++ b/libraries/script-engine/src/ScriptEngine.cpp
@@ -259,7 +259,7 @@ void ScriptEngine::errorInLoadingScript(const QUrl& url) {
 }
 
 // Even though we never pass AnimVariantMap directly to and from javascript, the queued invokeMethod of
-// invokeAnimationCallback requires that the type be registered.
+// callAnimationStateHandler requires that the type be registered.
 static QScriptValue animVarMapToScriptValue(QScriptEngine* engine, const AnimVariantMap& parameters) {
     return parameters.animVariantMapToScriptValue(engine);
 }
@@ -340,7 +340,7 @@ void ScriptEngine::init() {
 void ScriptEngine::registerValue(const QString& valueName, QScriptValue value) {
     if (QThread::currentThread() != thread()) {
 #ifdef THREAD_DEBUGGING
-        qDebug() << "*** WARNING *** ScriptEngine::registerValue() called on wrong thread [" << QThread::currentThread() << "], invoking on correct thread [" << thread() << "]  name:" << name;
+        qDebug() << "*** WARNING *** ScriptEngine::registerValue() called on wrong thread [" << QThread::currentThread() << "], invoking on correct thread [" << thread() << "]";
 #endif
         QMetaObject::invokeMethod(this, "registerValue",
             Q_ARG(const QString&, valueName),
@@ -729,8 +729,16 @@ void ScriptEngine::stop() {
 }
 
 // Other threads can invoke this through invokeMethod, which causes the callback to be asynchronously executed in this script's thread.
-void ScriptEngine::invokeAnimationCallback(QScriptValue callback, AnimVariantMap parameters) {
-    checkThread();
+void ScriptEngine::callAnimationStateHandler(QScriptValue callback, AnimVariantMap parameters) {
+    if (QThread::currentThread() != thread()) {
+#ifdef THREAD_DEBUGGING
+        qDebug() << "*** WARNING *** ScriptEngine::callAnimationStateHandler() called on wrong thread [" << QThread::currentThread() << "], invoking on correct thread [" << thread() << "]  name:" << name;
+#endif
+        QMetaObject::invokeMethod(this, "callAnimationStateHandler",
+                                  Q_ARG(QScriptValue, callback),
+                                  Q_ARG(AnimVariantMap, parameters));
+        return;
+    }
     QScriptValue javascriptParametgers = parameters.animVariantMapToScriptValue(this);
     QScriptValueList callingArguments;
     callingArguments << javascriptParametgers;
@@ -923,19 +931,12 @@ void ScriptEngine::load(const QString& loadFile) {
     }
 }
 
-bool ScriptEngine::checkThread() const {
+// Look up the handler associated with eventName and entityID. If found, evalute the argGenerator thunk and call the handler with those args
+void ScriptEngine::generalHandler(const EntityItemID& entityID, const QString& eventName, std::function<QScriptValueList()> argGenerator) {
     if (QThread::currentThread() != thread()) {
         qDebug() << "*** ERROR *** ScriptEngine::generalHandler() called on wrong thread [" << QThread::currentThread() << "], invoking on correct thread [" << thread() << "]";
         assert(false);
-        return true;
-    }
-    return false;
-}
-
-// Look up the handler associated with eventName and entityID. If found, evalute the argGenerator thunk and call the handler with those args
-void ScriptEngine::generalHandler(const EntityItemID& entityID, const QString& eventName, std::function<QScriptValueList()> argGenerator) {
-    if (checkThread()) {
-        return;
+        return ;
     }
     if (!_registeredHandlers.contains(entityID)) {
         return;
diff --git a/libraries/script-engine/src/ScriptEngine.h b/libraries/script-engine/src/ScriptEngine.h
index 77f1e2d5f1..7500e1d776 100644
--- a/libraries/script-engine/src/ScriptEngine.h
+++ b/libraries/script-engine/src/ScriptEngine.h
@@ -144,7 +144,7 @@ public:
     ArrayBufferClass* getArrayBufferClass() { return _arrayBufferClass; }
 
 public slots:
-    void invokeAnimationCallback(QScriptValue callback, AnimVariantMap parameters);
+    void callAnimationStateHandler(QScriptValue callback, AnimVariantMap parameters);
 
 signals:
     void scriptLoaded(const QString& scriptFilename);
@@ -173,7 +173,6 @@ protected:
     bool _wantSignals = true;
     QHash<EntityItemID, EntityScriptDetails> _entityScripts;
 private:
-    bool checkThread() const;
     void init();
     QString getFilename() const;
     void waitTillDoneRunning();

From ecc920199d6752671fabea491c24779c0b7b32f2 Mon Sep 17 00:00:00 2001
From: Howard Stearns <howard@highfidelity.io>
Date: Thu, 22 Oct 2015 15:24:24 -0700
Subject: [PATCH 12/31] Return id suitable for use with remover, per comments.

---
 interface/src/avatar/MyAvatar.h | 2 +-
 libraries/animation/src/Rig.cpp | 3 ++-
 libraries/animation/src/Rig.h   | 2 +-
 3 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/interface/src/avatar/MyAvatar.h b/interface/src/avatar/MyAvatar.h
index 4f32989bf5..435e6af5ef 100644
--- a/interface/src/avatar/MyAvatar.h
+++ b/interface/src/avatar/MyAvatar.h
@@ -115,7 +115,7 @@ public:
     // adding one of the other handlers. While any handler may change a value in animStateDictionaryIn (or supply different values in animStateDictionaryOut)
     // a handler must not remove properties from animStateDictionaryIn, nor change property values that it does not intend to change.
     // It is not specified in what order multiple handlers are called.
-    Q_INVOKABLE void addAnimationStateHandler(QScriptValue handler, QScriptValue propertiesList) { _rig->addAnimationStateHandler(handler, propertiesList); }
+    Q_INVOKABLE QScriptValue addAnimationStateHandler(QScriptValue handler, QScriptValue propertiesList) { return _rig->addAnimationStateHandler(handler, propertiesList); }
     // Removes a handler previously added by addAnimationStateHandler.
     Q_INVOKABLE void removeAnimationStateHandler(QScriptValue handler) { _rig->removeAnimationStateHandler(handler); }
     // Processes a handler result. Not really for user code, but used by callAnimationStateHandler.
diff --git a/libraries/animation/src/Rig.cpp b/libraries/animation/src/Rig.cpp
index 0437e9cca8..b4113fd4a5 100644
--- a/libraries/animation/src/Rig.cpp
+++ b/libraries/animation/src/Rig.cpp
@@ -608,8 +608,9 @@ void Rig::computeMotionAnimationState(float deltaTime, const glm::vec3& worldPos
 // Allow script to add/remove handlers and report results, from within their thread.
 // TODO: iterate multiple handlers, but with one shared arg.
 // TODO: fill the properties based on the union of requested properties. (Keep all properties objs and compute new union when add/remove handler.)
-void Rig::addAnimationStateHandler(QScriptValue handler, QScriptValue propertiesList) { // called in script thread
+QScriptValue Rig::addAnimationStateHandler(QScriptValue handler, QScriptValue propertiesList) { // called in script thread
     _stateHandlers = handler;
+    return handler; // suitable for giving to removeAnimationStateHandler
 }
 void Rig::removeAnimationStateHandler(QScriptValue handler) { // called in script thread
     _stateHandlers = QScriptValue();
diff --git a/libraries/animation/src/Rig.h b/libraries/animation/src/Rig.h
index 648a7f2c16..83f1a02b8a 100644
--- a/libraries/animation/src/Rig.h
+++ b/libraries/animation/src/Rig.h
@@ -201,7 +201,7 @@ public:
     AnimNode::ConstPointer getAnimNode() const { return _animNode; }
     AnimSkeleton::ConstPointer getAnimSkeleton() const { return _animSkeleton; }
     bool disableHands {false}; // should go away with rig animation (and Rig::inverseKinematics)
-    void addAnimationStateHandler(QScriptValue handler, QScriptValue propertiesList);
+    QScriptValue addAnimationStateHandler(QScriptValue handler, QScriptValue propertiesList);
     void removeAnimationStateHandler(QScriptValue handler);
     void animationStateHandlerResult(QScriptValue handler, QScriptValue result);
 

From f0a4e25e895017a4fb75f593564669761fb09e16 Mon Sep 17 00:00:00 2001
From: Howard Stearns <howard@highfidelity.io>
Date: Thu, 22 Oct 2015 15:25:14 -0700
Subject: [PATCH 13/31] Name change per comments.

---
 interface/src/avatar/AvatarManager.h | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/interface/src/avatar/AvatarManager.h b/interface/src/avatar/AvatarManager.h
index c71f2bc25b..ffc7cc8f92 100644
--- a/interface/src/avatar/AvatarManager.h
+++ b/interface/src/avatar/AvatarManager.h
@@ -53,9 +53,11 @@ public:
     Q_INVOKABLE void setLocalLights(const QVector<AvatarManager::LocalLight>& localLights);
     Q_INVOKABLE QVector<AvatarManager::LocalLight> getLocalLights() const;
     // Currently, your own avatar will be included as the null avatar id.
-    Q_INVOKABLE QVector<QUuid> getAvatars() const { return _avatarHash.keys().toVector(); }
+    Q_INVOKABLE QVector<QUuid> getAvatarIdentifiers() const { return _avatarHash.keys().toVector(); } // FIXME: see #6154
+    Q_INVOKABLE QVector<QUuid> getAvatars() const { return getAvatarIdentifiers(); } // FIXME: remove before merge. Compatability for testing scripts.
     // Minor Bug: A bogus avatarID answers your own avatar.
-    Q_INVOKABLE AvatarData* getAvatar(QUuid avatarID) const { return _avatarHash[avatarID].get(); }
+    Q_INVOKABLE AvatarData* getAvatar(QUuid avatarID) const { return _avatarHash[avatarID].get(); } // FIXME: see #6154
+
 
     void getObjectsToDelete(VectorOfMotionStates& motionStates);
     void getObjectsToAdd(VectorOfMotionStates& motionStates);

From 913842ac30575a208d7ed9c7ec0affc8b4e94eaa Mon Sep 17 00:00:00 2001
From: Howard Stearns <howard@highfidelity.io>
Date: Thu, 22 Oct 2015 15:31:17 -0700
Subject: [PATCH 14/31] Thread check, for consistency.

---
 libraries/animation/src/AnimVariantMap.cpp | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/libraries/animation/src/AnimVariantMap.cpp b/libraries/animation/src/AnimVariantMap.cpp
index 219a44b644..170662f092 100644
--- a/libraries/animation/src/AnimVariantMap.cpp
+++ b/libraries/animation/src/AnimVariantMap.cpp
@@ -54,6 +54,10 @@ void AnimVariantMap::copyVariantsFrom(const AnimVariantMap& other) {
     }
 }
 void AnimVariantMap::animVariantMapFromScriptValue(const QScriptValue& source) {
+    if (QThread::currentThread() != source.engine()->thread()) {
+        qCWarning(animation) << "Cannot examine Javacript object from non-script thread" << QThread::currentThread();
+        return;
+    }
     // POTENTIAL OPTIMIZATION: cache the types we've seen. I.e, keep a dictionary mapping property names to an enumeration of types.
     // Whenever we identify a new outbound type in animVariantMapToScriptValue above, or a new inbound type in the code that follows here,
     // we would enter it into the dictionary. Then switch on that type here, with the code that follow being executed only if

From 2b7ceffd643824b2abeb5dbd14819fd500a93bfc Mon Sep 17 00:00:00 2001
From: Howard Stearns <howard@highfidelity.io>
Date: Thu, 22 Oct 2015 16:23:09 -0700
Subject: [PATCH 15/31] Get rid of
 globalObject().property("MyAvatar").property("animationStateHandlerResult").

---
 libraries/animation/src/AnimVariant.h        |  2 ++
 libraries/animation/src/Rig.cpp              |  6 +++++-
 libraries/script-engine/src/ScriptEngine.cpp | 21 +++++++++++++-------
 libraries/script-engine/src/ScriptEngine.h   |  2 +-
 4 files changed, 22 insertions(+), 9 deletions(-)

diff --git a/libraries/animation/src/AnimVariant.h b/libraries/animation/src/AnimVariant.h
index 2d140bfa52..4190abfed8 100644
--- a/libraries/animation/src/AnimVariant.h
+++ b/libraries/animation/src/AnimVariant.h
@@ -205,6 +205,8 @@ protected:
     std::set<QString> _triggers;
 };
 
+typedef std::function<void(QScriptValue, QScriptValue)> AnimVariantResultHandler;
+Q_DECLARE_METATYPE(AnimVariantResultHandler);
 Q_DECLARE_METATYPE(AnimVariantMap)
 
 #endif // hifi_AnimVariant_h
diff --git a/libraries/animation/src/Rig.cpp b/libraries/animation/src/Rig.cpp
index b4113fd4a5..7ab39c2f34 100644
--- a/libraries/animation/src/Rig.cpp
+++ b/libraries/animation/src/Rig.cpp
@@ -628,11 +628,15 @@ void Rig::animationStateHandlerResult(QScriptValue handler, QScriptValue result)
 
 void Rig::updateAnimationStateHandlers() { // called on avatar update thread (which may be main thread)
     if (_stateHandlers.isValid()) {
+        auto handleResult = [this](QScriptValue handler, QScriptValue result) {
+            animationStateHandlerResult(handler, result);
+        };
         // invokeMethod makes a copy of the args, and copies of AnimVariantMap do copy the underlying map, so this will correctly capture
         // the state of _animVars and allow continued changes to _animVars in this thread without conflict.
         QMetaObject::invokeMethod(_stateHandlers.engine(), "callAnimationStateHandler",  Qt::QueuedConnection,
                                   Q_ARG(QScriptValue, _stateHandlers),
-                                  Q_ARG(AnimVariantMap, _animVars));
+                                  Q_ARG(AnimVariantMap, _animVars),
+                                  Q_ARG(AnimVariantResultHandler, handleResult));
     }
     QMutexLocker locker(&_stateMutex); // as we examine/copy most recently computed state, if any. (Typically an earlier invocation.)
     _animVars.copyVariantsFrom(_stateHandlersResults);
diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp
index 5808340855..acfa0c027b 100644
--- a/libraries/script-engine/src/ScriptEngine.cpp
+++ b/libraries/script-engine/src/ScriptEngine.cpp
@@ -260,12 +260,23 @@ void ScriptEngine::errorInLoadingScript(const QUrl& url) {
 
 // Even though we never pass AnimVariantMap directly to and from javascript, the queued invokeMethod of
 // callAnimationStateHandler requires that the type be registered.
+// These two are meaningful, if we ever do want to use them...
 static QScriptValue animVarMapToScriptValue(QScriptEngine* engine, const AnimVariantMap& parameters) {
     return parameters.animVariantMapToScriptValue(engine);
 }
 static void animVarMapFromScriptValue(const QScriptValue& value, AnimVariantMap& parameters) {
     parameters.animVariantMapFromScriptValue(value);
 }
+// ... while these two are not. But none of the four are ever used.
+static QScriptValue resultHandlerToScriptValue(QScriptEngine* engine, const AnimVariantResultHandler& resultHandler) {
+    qCCritical(scriptengine) << "Attempt to marshall result handler to javascript";
+    assert(false);
+    return QScriptValue();
+}
+static void resultHandlerFromScriptValue(const QScriptValue& value, AnimVariantResultHandler& resultHandler) {
+    qCCritical(scriptengine) << "Attempt to marshall result handler from javascript";
+    assert(false);
+}
 
 void ScriptEngine::init() {
     if (_isInitialized) {
@@ -326,6 +337,7 @@ void ScriptEngine::init() {
     registerGlobalObject("Uuid", &_uuidLibrary);
     registerGlobalObject("AnimationCache", DependencyManager::get<AnimationCache>().data());
     qScriptRegisterMetaType(this, animVarMapToScriptValue, animVarMapFromScriptValue);
+    qScriptRegisterMetaType(this, resultHandlerToScriptValue, resultHandlerFromScriptValue);
 
     // constants
     globalObject().setProperty("TREE_SCALE", newVariant(QVariant(TREE_SCALE)));
@@ -729,7 +741,7 @@ void ScriptEngine::stop() {
 }
 
 // Other threads can invoke this through invokeMethod, which causes the callback to be asynchronously executed in this script's thread.
-void ScriptEngine::callAnimationStateHandler(QScriptValue callback, AnimVariantMap parameters) {
+void ScriptEngine::callAnimationStateHandler(QScriptValue callback, AnimVariantMap parameters, AnimVariantResultHandler resultHandler) {
     if (QThread::currentThread() != thread()) {
 #ifdef THREAD_DEBUGGING
         qDebug() << "*** WARNING *** ScriptEngine::callAnimationStateHandler() called on wrong thread [" << QThread::currentThread() << "], invoking on correct thread [" << thread() << "]  name:" << name;
@@ -743,12 +755,7 @@ void ScriptEngine::callAnimationStateHandler(QScriptValue callback, AnimVariantM
     QScriptValueList callingArguments;
     callingArguments << javascriptParametgers;
     QScriptValue result = callback.call(QScriptValue(), callingArguments);
-    // We want to give the result back to the rig, but we don't have the rig or the avatar. But the global does.
-    // This is sort of like going through DependencyManager.get.
-    QScriptValue resultHandler = globalObject().property("MyAvatar").property("animationStateHandlerResult");
-    QScriptValueList resultArguments;
-    resultArguments << callback << result;
-    resultHandler.call(QScriptValue(), resultArguments); // Call it synchronously, on our own time and thread.
+    resultHandler(callback, result);
 }
 
 void ScriptEngine::timerFired() {
diff --git a/libraries/script-engine/src/ScriptEngine.h b/libraries/script-engine/src/ScriptEngine.h
index 7500e1d776..b6f736c846 100644
--- a/libraries/script-engine/src/ScriptEngine.h
+++ b/libraries/script-engine/src/ScriptEngine.h
@@ -144,7 +144,7 @@ public:
     ArrayBufferClass* getArrayBufferClass() { return _arrayBufferClass; }
 
 public slots:
-    void callAnimationStateHandler(QScriptValue callback, AnimVariantMap parameters);
+    void callAnimationStateHandler(QScriptValue callback, AnimVariantMap parameters, AnimVariantResultHandler resultHandler);
 
 signals:
     void scriptLoaded(const QString& scriptFilename);

From 92ddedd44b4d1487ea2e802351617136ab38cf0d Mon Sep 17 00:00:00 2001
From: howard-stearns <howard.stearns@gmail.com>
Date: Thu, 22 Oct 2015 16:44:15 -0700
Subject: [PATCH 16/31] Make msvc happy.

---
 libraries/animation/src/AnimVariant.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/libraries/animation/src/AnimVariant.h b/libraries/animation/src/AnimVariant.h
index 4190abfed8..7a80321f7b 100644
--- a/libraries/animation/src/AnimVariant.h
+++ b/libraries/animation/src/AnimVariant.h
@@ -12,6 +12,7 @@
 #define hifi_AnimVariant_h
 
 #include <cassert>
+#include <functional>
 #include <glm/glm.hpp>
 #include <glm/gtx/quaternion.hpp>
 #include <map>

From 84cfeaec13f79615e788815b14ef937dc76a10d5 Mon Sep 17 00:00:00 2001
From: Howard Stearns <howard@highfidelity.io>
Date: Thu, 22 Oct 2015 17:01:06 -0700
Subject: [PATCH 17/31] Linux QT wants .h-less headers.

---
 libraries/animation/src/AnimVariantMap.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libraries/animation/src/AnimVariantMap.cpp b/libraries/animation/src/AnimVariantMap.cpp
index 170662f092..59f10a96e0 100644
--- a/libraries/animation/src/AnimVariantMap.cpp
+++ b/libraries/animation/src/AnimVariantMap.cpp
@@ -11,7 +11,7 @@
 
 #include <QScriptEngine>
 #include <QScriptValueIterator>
-#include <QThread.h>
+#include <QThread>
 #include <RegisteredMetaTypes.h>
 #include "AnimVariant.h"
 

From e11b0add9a63d5497dbb30c184a18489cacb331d Mon Sep 17 00:00:00 2001
From: Howard Stearns <howard@highfidelity.io>
Date: Sat, 24 Oct 2015 15:29:49 -0700
Subject: [PATCH 18/31] Update safety trampoline with correct arguments.

---
 libraries/animation/src/Rig.cpp              | 9 +++++++++
 libraries/script-engine/src/ScriptEngine.cpp | 3 ++-
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/libraries/animation/src/Rig.cpp b/libraries/animation/src/Rig.cpp
index 7ab39c2f34..0576a8e420 100644
--- a/libraries/animation/src/Rig.cpp
+++ b/libraries/animation/src/Rig.cpp
@@ -637,6 +637,15 @@ void Rig::updateAnimationStateHandlers() { // called on avatar update thread (wh
                                   Q_ARG(QScriptValue, _stateHandlers),
                                   Q_ARG(AnimVariantMap, _animVars),
                                   Q_ARG(AnimVariantResultHandler, handleResult));
+        // It turns out that, for thread-safety reasons, ScriptEngine::callAnimationStateHandler will invoke itself if called from other
+        // than the script thread. Thus the above _could_ be replaced with an ordinary call, which will then trigger the same
+        // invokeMethod as is done explicitly above. However, the script-engine library depends on this animation library, not vice versa.
+        // We could create an AnimVariantCallingMixin class in shared, with an abstract virtual slot
+        // AnimVariantCallingMixin::callAnimationStateHandler (and move AnimVariantMap/AnimVaraintResultHandler to shared), but the
+        // call site here would look like this instead of the above:
+        //   dynamic_cast<AnimVariantCallingMixin*>(_stateHandlers.engine())->callAnimationStateHandler(_stateHandlers, _animVars, handleResult);
+        // This works (I tried it), but the result would be that we would still have same runtime type checks as the invokeMethod above
+        // (occuring within the ScriptEngine::callAnimationStateHandler invokeMethod trampoline), _plus_ another runtime check for the dynamic_cast.
     }
     QMutexLocker locker(&_stateMutex); // as we examine/copy most recently computed state, if any. (Typically an earlier invocation.)
     _animVars.copyVariantsFrom(_stateHandlersResults);
diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp
index acfa0c027b..86eb9570d8 100644
--- a/libraries/script-engine/src/ScriptEngine.cpp
+++ b/libraries/script-engine/src/ScriptEngine.cpp
@@ -748,7 +748,8 @@ void ScriptEngine::callAnimationStateHandler(QScriptValue callback, AnimVariantM
 #endif
         QMetaObject::invokeMethod(this, "callAnimationStateHandler",
                                   Q_ARG(QScriptValue, callback),
-                                  Q_ARG(AnimVariantMap, parameters));
+                                  Q_ARG(AnimVariantMap, parameters),
+                                  Q_ARG(AnimVariantResultHandler, resultHandler));
         return;
     }
     QScriptValue javascriptParametgers = parameters.animVariantMapToScriptValue(this);

From 900b07fdeef094f5e6ddf14939fc1a76934ae628 Mon Sep 17 00:00:00 2001
From: Howard Stearns <howard@highfidelity.io>
Date: Mon, 26 Oct 2015 10:00:07 -0700
Subject: [PATCH 19/31] dead code

---
 interface/src/avatar/MyAvatar.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/interface/src/avatar/MyAvatar.h b/interface/src/avatar/MyAvatar.h
index 435e6af5ef..1ffb27930e 100644
--- a/interface/src/avatar/MyAvatar.h
+++ b/interface/src/avatar/MyAvatar.h
@@ -118,8 +118,6 @@ public:
     Q_INVOKABLE QScriptValue addAnimationStateHandler(QScriptValue handler, QScriptValue propertiesList) { return _rig->addAnimationStateHandler(handler, propertiesList); }
     // Removes a handler previously added by addAnimationStateHandler.
     Q_INVOKABLE void removeAnimationStateHandler(QScriptValue handler) { _rig->removeAnimationStateHandler(handler); }
-    // Processes a handler result. Not really for user code, but used by callAnimationStateHandler.
-    Q_INVOKABLE void animationStateHandlerResult(QScriptValue handler, QScriptValue result) { _rig->animationStateHandlerResult(handler, result); }
 
     // get/set avatar data
     void saveData();

From 4b4907c9ef8bb4398d1db9cb7c8285c823c758e5 Mon Sep 17 00:00:00 2001
From: Howard Stearns <howard@highfidelity.io>
Date: Mon, 26 Oct 2015 10:04:55 -0700
Subject: [PATCH 20/31] Allow multiple scripts to register, and allow them to
 specify the specific anim vars they are interested in.

---
 libraries/animation/src/AnimVariant.h        |  4 +-
 libraries/animation/src/AnimVariantMap.cpp   | 31 +++++++---
 libraries/animation/src/Rig.cpp              | 60 +++++++++++++-------
 libraries/animation/src/Rig.h                | 12 +++-
 libraries/script-engine/src/ScriptEngine.cpp | 11 ++--
 libraries/script-engine/src/ScriptEngine.h   |  2 +-
 6 files changed, 79 insertions(+), 41 deletions(-)

diff --git a/libraries/animation/src/AnimVariant.h b/libraries/animation/src/AnimVariant.h
index 7a80321f7b..bd96dda77d 100644
--- a/libraries/animation/src/AnimVariant.h
+++ b/libraries/animation/src/AnimVariant.h
@@ -163,7 +163,7 @@ public:
     bool hasKey(const QString& key) const { return _map.find(key) != _map.end(); }
 
     // Answer a Plain Old Javascript Object (for the given engine) all of our values set as properties.
-    QScriptValue animVariantMapToScriptValue(QScriptEngine* engine) const;
+    QScriptValue animVariantMapToScriptValue(QScriptEngine* engine, const QStringList& names, bool useNames) const;
     // Side-effect us with the value of object's own properties. (No inherited properties.)
     void animVariantMapFromScriptValue(const QScriptValue& object);
     void copyVariantsFrom(const AnimVariantMap& other);
@@ -206,7 +206,7 @@ protected:
     std::set<QString> _triggers;
 };
 
-typedef std::function<void(QScriptValue, QScriptValue)> AnimVariantResultHandler;
+typedef std::function<void(QScriptValue)> AnimVariantResultHandler;
 Q_DECLARE_METATYPE(AnimVariantResultHandler);
 Q_DECLARE_METATYPE(AnimVariantMap)
 
diff --git a/libraries/animation/src/AnimVariantMap.cpp b/libraries/animation/src/AnimVariantMap.cpp
index 59f10a96e0..0c808bd404 100644
--- a/libraries/animation/src/AnimVariantMap.cpp
+++ b/libraries/animation/src/AnimVariantMap.cpp
@@ -15,36 +15,49 @@
 #include <RegisteredMetaTypes.h>
 #include "AnimVariant.h"
 
-QScriptValue AnimVariantMap::animVariantMapToScriptValue(QScriptEngine* engine) const {
+QScriptValue AnimVariantMap::animVariantMapToScriptValue(QScriptEngine* engine, const QStringList& names, bool useNames) const {
     if (QThread::currentThread() != engine->thread()) {
         qCWarning(animation) << "Cannot create Javacript object from non-script thread" << QThread::currentThread();
         return QScriptValue();
     }
     QScriptValue target = engine->newObject();
-    for (auto& pair : _map) {
-        switch (pair.second.getType()) {
+    auto setOne = [&] (QString name, AnimVariant value) {
+        switch (value.getType()) {
             case AnimVariant::Type::Bool:
-                target.setProperty(pair.first, pair.second.getBool());
+                target.setProperty(name, value.getBool());
                 break;
             case AnimVariant::Type::Int:
-                target.setProperty(pair.first, pair.second.getInt());
+                target.setProperty(name, value.getInt());
                 break;
             case AnimVariant::Type::Float:
-                target.setProperty(pair.first, pair.second.getFloat());
+                target.setProperty(name, value.getFloat());
                 break;
             case AnimVariant::Type::String:
-                target.setProperty(pair.first, pair.second.getString());
+                target.setProperty(name, value.getString());
                 break;
             case AnimVariant::Type::Vec3:
-                target.setProperty(pair.first, vec3toScriptValue(engine, pair.second.getVec3()));
+                target.setProperty(name, vec3toScriptValue(engine, value.getVec3()));
                 break;
             case AnimVariant::Type::Quat:
-                target.setProperty(pair.first, quatToScriptValue(engine, pair.second.getQuat()));
+                target.setProperty(name, quatToScriptValue(engine, value.getQuat()));
                 break;
             default:
                 // Note that we don't do mat4 in Javascript currently, and there's not yet a reason to start now.
                 assert("AnimVariant::Type" == "valid");
         }
+    };
+    if (useNames) { // copy only the requested names
+        for (const QString& name : names) {
+            auto search = _map.find(name);
+            if (search != _map.end()) { // scripts are allowed to request names that do not exist
+                setOne(name, search->second);
+            }
+        }
+
+    } else {  // copy all of them
+        for (auto& pair : _map) {
+            setOne(pair.first, pair.second);
+        }
     }
     return target;
 }
diff --git a/libraries/animation/src/Rig.cpp b/libraries/animation/src/Rig.cpp
index 0576a8e420..7810bb8693 100644
--- a/libraries/animation/src/Rig.cpp
+++ b/libraries/animation/src/Rig.cpp
@@ -606,36 +606,51 @@ void Rig::computeMotionAnimationState(float deltaTime, const glm::vec3& worldPos
 }
 
 // Allow script to add/remove handlers and report results, from within their thread.
-// TODO: iterate multiple handlers, but with one shared arg.
-// TODO: fill the properties based on the union of requested properties. (Keep all properties objs and compute new union when add/remove handler.)
 QScriptValue Rig::addAnimationStateHandler(QScriptValue handler, QScriptValue propertiesList) { // called in script thread
-    _stateHandlers = handler;
-    return handler; // suitable for giving to removeAnimationStateHandler
-}
-void Rig::removeAnimationStateHandler(QScriptValue handler) { // called in script thread
-    _stateHandlers = QScriptValue();
-    QMutexLocker locker(&_stateMutex); // guarding access to results
-    _stateHandlersResults.clearMap(); // TODO: When we have multiple handlers, we'll need to clear only his handler's results.
-}
-void Rig::animationStateHandlerResult(QScriptValue handler, QScriptValue result) { // called synchronously from script
-    // handler is currently ignored but might be used in storing individual results
     QMutexLocker locker(&_stateMutex);
-    if (!_stateHandlers.isValid()) {
+    int identifier = ++_nextStateHandlerId; // 0 is unused
+    StateHandler& data = _stateHandlers[identifier];
+    data.function = handler;
+    data.useNames = propertiesList.isArray();
+    if (data.useNames) {
+        data.propertyNames = propertiesList.toVariant().toStringList();
+    }
+    return QScriptValue(identifier); // suitable for giving to removeAnimationStateHandler
+}
+void Rig::removeAnimationStateHandler(QScriptValue identifier) { // called in script thread
+    QMutexLocker locker(&_stateMutex);
+    _stateHandlers.remove(identifier.isNumber() ? identifier.toInt32() : 0); // silently continues if handler not present
+}
+void Rig::animationStateHandlerResult(int identifier, QScriptValue result) { // called synchronously from script
+    QMutexLocker locker(&_stateMutex);
+    auto found = _stateHandlers.find(identifier);
+    if (found == _stateHandlers.end()) {
         return; // Don't use late-breaking results that got reported after the handler was removed.
     }
-    _stateHandlersResults.animVariantMapFromScriptValue(result); // Into our own copy.
+    found.value().results.animVariantMapFromScriptValue(result); // Into our own copy.
 }
 
 void Rig::updateAnimationStateHandlers() { // called on avatar update thread (which may be main thread)
-    if (_stateHandlers.isValid()) {
-        auto handleResult = [this](QScriptValue handler, QScriptValue result) {
-            animationStateHandlerResult(handler, result);
+    QMutexLocker locker(&_stateMutex);
+    // It might pay to produce just one AnimVariantMap copy here, with a union of all the requested propertyNames,
+    // rather than having each callAnimationStateHandler invocation make its own copy.
+    // However, that copying is done on the script's own time rather than ours, so even if it's less cpu, it would be more
+    // work on the avatar update thread (which is possibly the main thread).
+    for (auto data = _stateHandlers.begin(); data != _stateHandlers.end(); data++) {
+        // call out:
+        int identifier = data.key();
+        StateHandler& value = data.value();
+        QScriptValue& function = value.function;
+        auto handleResult = [this, identifier](QScriptValue result) {
+            animationStateHandlerResult(identifier, result);
         };
         // invokeMethod makes a copy of the args, and copies of AnimVariantMap do copy the underlying map, so this will correctly capture
         // the state of _animVars and allow continued changes to _animVars in this thread without conflict.
-        QMetaObject::invokeMethod(_stateHandlers.engine(), "callAnimationStateHandler",  Qt::QueuedConnection,
-                                  Q_ARG(QScriptValue, _stateHandlers),
+        QMetaObject::invokeMethod(function.engine(), "callAnimationStateHandler",  Qt::QueuedConnection,
+                                  Q_ARG(QScriptValue, function),
                                   Q_ARG(AnimVariantMap, _animVars),
+                                  Q_ARG(QStringList, value.propertyNames),
+                                  Q_ARG(bool, value.useNames),
                                   Q_ARG(AnimVariantResultHandler, handleResult));
         // It turns out that, for thread-safety reasons, ScriptEngine::callAnimationStateHandler will invoke itself if called from other
         // than the script thread. Thus the above _could_ be replaced with an ordinary call, which will then trigger the same
@@ -643,12 +658,13 @@ void Rig::updateAnimationStateHandlers() { // called on avatar update thread (wh
         // We could create an AnimVariantCallingMixin class in shared, with an abstract virtual slot
         // AnimVariantCallingMixin::callAnimationStateHandler (and move AnimVariantMap/AnimVaraintResultHandler to shared), but the
         // call site here would look like this instead of the above:
-        //   dynamic_cast<AnimVariantCallingMixin*>(_stateHandlers.engine())->callAnimationStateHandler(_stateHandlers, _animVars, handleResult);
+        //   dynamic_cast<AnimVariantCallingMixin*>(function.engine())->callAnimationStateHandler(function, ..., handleResult);
         // This works (I tried it), but the result would be that we would still have same runtime type checks as the invokeMethod above
         // (occuring within the ScriptEngine::callAnimationStateHandler invokeMethod trampoline), _plus_ another runtime check for the dynamic_cast.
+
+        // gather results in (likely from an earlier update):
+        _animVars.copyVariantsFrom(value.results); // If multiple handlers write the same anim var, the last registgered wins. (_map preserves order).
     }
-    QMutexLocker locker(&_stateMutex); // as we examine/copy most recently computed state, if any. (Typically an earlier invocation.)
-    _animVars.copyVariantsFrom(_stateHandlersResults);
 }
 
 void Rig::updateAnimations(float deltaTime, glm::mat4 rootTransform) {
diff --git a/libraries/animation/src/Rig.h b/libraries/animation/src/Rig.h
index 83f1a02b8a..5551b21e40 100644
--- a/libraries/animation/src/Rig.h
+++ b/libraries/animation/src/Rig.h
@@ -53,6 +53,12 @@ typedef std::shared_ptr<Rig> RigPointer;
 
 class Rig : public QObject, public std::enable_shared_from_this<Rig> {
 public:
+    struct StateHandler {
+        AnimVariantMap results;
+        QStringList propertyNames;
+        QScriptValue function;
+        bool useNames;
+    };
 
     struct HeadParameters {
         float leanSideways = 0.0f; // degrees
@@ -203,7 +209,7 @@ public:
     bool disableHands {false}; // should go away with rig animation (and Rig::inverseKinematics)
     QScriptValue addAnimationStateHandler(QScriptValue handler, QScriptValue propertiesList);
     void removeAnimationStateHandler(QScriptValue handler);
-    void animationStateHandlerResult(QScriptValue handler, QScriptValue result);
+    void animationStateHandlerResult(int identifier, QScriptValue result);
 
     bool getModelOffset(glm::vec3& modelOffsetOut) const;
 
@@ -249,8 +255,8 @@ public:
     float _rightHandOverlayAlpha = 0.0f;
 
 private:
-    QScriptValue _stateHandlers;
-    AnimVariantMap _stateHandlersResults;
+    QMap<int, StateHandler> _stateHandlers;
+    int _nextStateHandlerId {0};
     QMutex _stateMutex;
 };
 
diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp
index 86eb9570d8..91df9fa793 100644
--- a/libraries/script-engine/src/ScriptEngine.cpp
+++ b/libraries/script-engine/src/ScriptEngine.cpp
@@ -262,7 +262,8 @@ void ScriptEngine::errorInLoadingScript(const QUrl& url) {
 // callAnimationStateHandler requires that the type be registered.
 // These two are meaningful, if we ever do want to use them...
 static QScriptValue animVarMapToScriptValue(QScriptEngine* engine, const AnimVariantMap& parameters) {
-    return parameters.animVariantMapToScriptValue(engine);
+    QStringList unused;
+    return parameters.animVariantMapToScriptValue(engine, unused, false);
 }
 static void animVarMapFromScriptValue(const QScriptValue& value, AnimVariantMap& parameters) {
     parameters.animVariantMapFromScriptValue(value);
@@ -741,7 +742,7 @@ void ScriptEngine::stop() {
 }
 
 // Other threads can invoke this through invokeMethod, which causes the callback to be asynchronously executed in this script's thread.
-void ScriptEngine::callAnimationStateHandler(QScriptValue callback, AnimVariantMap parameters, AnimVariantResultHandler resultHandler) {
+void ScriptEngine::callAnimationStateHandler(QScriptValue callback, AnimVariantMap parameters, QStringList names, bool useNames, AnimVariantResultHandler resultHandler) {
     if (QThread::currentThread() != thread()) {
 #ifdef THREAD_DEBUGGING
         qDebug() << "*** WARNING *** ScriptEngine::callAnimationStateHandler() called on wrong thread [" << QThread::currentThread() << "], invoking on correct thread [" << thread() << "]  name:" << name;
@@ -749,14 +750,16 @@ void ScriptEngine::callAnimationStateHandler(QScriptValue callback, AnimVariantM
         QMetaObject::invokeMethod(this, "callAnimationStateHandler",
                                   Q_ARG(QScriptValue, callback),
                                   Q_ARG(AnimVariantMap, parameters),
+                                  Q_ARG(QStringList, names),
+                                  Q_ARG(bool, useNames),
                                   Q_ARG(AnimVariantResultHandler, resultHandler));
         return;
     }
-    QScriptValue javascriptParametgers = parameters.animVariantMapToScriptValue(this);
+    QScriptValue javascriptParametgers = parameters.animVariantMapToScriptValue(this, names, useNames);
     QScriptValueList callingArguments;
     callingArguments << javascriptParametgers;
     QScriptValue result = callback.call(QScriptValue(), callingArguments);
-    resultHandler(callback, result);
+    resultHandler(result);
 }
 
 void ScriptEngine::timerFired() {
diff --git a/libraries/script-engine/src/ScriptEngine.h b/libraries/script-engine/src/ScriptEngine.h
index b6f736c846..30468880a2 100644
--- a/libraries/script-engine/src/ScriptEngine.h
+++ b/libraries/script-engine/src/ScriptEngine.h
@@ -144,7 +144,7 @@ public:
     ArrayBufferClass* getArrayBufferClass() { return _arrayBufferClass; }
 
 public slots:
-    void callAnimationStateHandler(QScriptValue callback, AnimVariantMap parameters, AnimVariantResultHandler resultHandler);
+    void callAnimationStateHandler(QScriptValue callback, AnimVariantMap parameters, QStringList names, bool useNames, AnimVariantResultHandler resultHandler);
 
 signals:
     void scriptLoaded(const QString& scriptFilename);

From 4083c5c71b848d300ef73709003a119e06dbe4a1 Mon Sep 17 00:00:00 2001
From: Howard Stearns <howard@highfidelity.io>
Date: Thu, 29 Oct 2015 10:31:36 -0700
Subject: [PATCH 21/31] Handle wrapping of very long-lived sessions.

---
 libraries/animation/src/Rig.cpp | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/libraries/animation/src/Rig.cpp b/libraries/animation/src/Rig.cpp
index de2d3c54ad..b51907ea4a 100644
--- a/libraries/animation/src/Rig.cpp
+++ b/libraries/animation/src/Rig.cpp
@@ -580,18 +580,20 @@ void Rig::computeMotionAnimationState(float deltaTime, const glm::vec3& worldPos
 // Allow script to add/remove handlers and report results, from within their thread.
 QScriptValue Rig::addAnimationStateHandler(QScriptValue handler, QScriptValue propertiesList) { // called in script thread
     QMutexLocker locker(&_stateMutex);
-    int identifier = ++_nextStateHandlerId; // 0 is unused
-    StateHandler& data = _stateHandlers[identifier];
+    while (!_nextStateHandlerId || _stateHandlers.contains(_nextStateHandlerId)) { // 0 is unused, and don't reuse existing after wrap.
+      _nextStateHandlerId++;
+    }
+    StateHandler& data = _stateHandlers[_nextStateHandlerId];
     data.function = handler;
     data.useNames = propertiesList.isArray();
     if (data.useNames) {
         data.propertyNames = propertiesList.toVariant().toStringList();
     }
-    return QScriptValue(identifier); // suitable for giving to removeAnimationStateHandler
+    return QScriptValue(_nextStateHandlerId); // suitable for giving to removeAnimationStateHandler
 }
 void Rig::removeAnimationStateHandler(QScriptValue identifier) { // called in script thread
     QMutexLocker locker(&_stateMutex);
-    _stateHandlers.remove(identifier.isNumber() ? identifier.toInt32() : 0); // silently continues if handler not present
+    _stateHandlers.remove(identifier.isNumber() ? identifier.toInt32() : 0); // silently continues if handler not present. 0 is unused
 }
 void Rig::animationStateHandlerResult(int identifier, QScriptValue result) { // called synchronously from script
     QMutexLocker locker(&_stateMutex);

From 3c6d4f9c221ac19c8f3b12070ee396c52f72bec1 Mon Sep 17 00:00:00 2001
From: Howard Stearns <howard@highfidelity.io>
Date: Thu, 29 Oct 2015 11:02:24 -0700
Subject: [PATCH 22/31] Thread safety per #6154.

---
 interface/src/avatar/AvatarManager.cpp | 10 ++++++++++
 interface/src/avatar/AvatarManager.h   |  6 ++----
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/interface/src/avatar/AvatarManager.cpp b/interface/src/avatar/AvatarManager.cpp
index 9783590b05..fbfbbad2de 100644
--- a/interface/src/avatar/AvatarManager.cpp
+++ b/interface/src/avatar/AvatarManager.cpp
@@ -248,6 +248,16 @@ QVector<AvatarManager::LocalLight> AvatarManager::getLocalLights() const {
     return _localLights;
 }
 
+QVector<QUuid> AvatarManager::getAvatarIdentifiers() {
+    QReadLocker locker(&_hashLock);
+    return _avatarHash.keys().toVector();
+}
+AvatarData* AvatarManager::getAvatar(QUuid avatarID) {
+    QReadLocker locker(&_hashLock);
+    return _avatarHash[avatarID].get();  // Non-obvious: A bogus avatarID answers your own avatar.
+}
+
+
 void AvatarManager::getObjectsToDelete(VectorOfMotionStates& result) {
     result.clear();
     result.swap(_motionStatesToDelete);
diff --git a/interface/src/avatar/AvatarManager.h b/interface/src/avatar/AvatarManager.h
index ffc7cc8f92..fa0593368b 100644
--- a/interface/src/avatar/AvatarManager.h
+++ b/interface/src/avatar/AvatarManager.h
@@ -53,10 +53,8 @@ public:
     Q_INVOKABLE void setLocalLights(const QVector<AvatarManager::LocalLight>& localLights);
     Q_INVOKABLE QVector<AvatarManager::LocalLight> getLocalLights() const;
     // Currently, your own avatar will be included as the null avatar id.
-    Q_INVOKABLE QVector<QUuid> getAvatarIdentifiers() const { return _avatarHash.keys().toVector(); } // FIXME: see #6154
-    Q_INVOKABLE QVector<QUuid> getAvatars() const { return getAvatarIdentifiers(); } // FIXME: remove before merge. Compatability for testing scripts.
-    // Minor Bug: A bogus avatarID answers your own avatar.
-    Q_INVOKABLE AvatarData* getAvatar(QUuid avatarID) const { return _avatarHash[avatarID].get(); } // FIXME: see #6154
+    Q_INVOKABLE QVector<QUuid> getAvatarIdentifiers();
+    Q_INVOKABLE AvatarData* getAvatar(QUuid avatarID);
 
 
     void getObjectsToDelete(VectorOfMotionStates& motionStates);

From 303491817bd14dd17e2ab6eaa95674145c125bca Mon Sep 17 00:00:00 2001
From: Howard Stearns <howard@highfidelity.io>
Date: Thu, 29 Oct 2015 16:35:59 -0700
Subject: [PATCH 23/31] assert to get hard error in dev, warning and no-op in
 release.

---
 libraries/animation/src/AnimVariantMap.cpp | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/libraries/animation/src/AnimVariantMap.cpp b/libraries/animation/src/AnimVariantMap.cpp
index 0c808bd404..fc474c0cbd 100644
--- a/libraries/animation/src/AnimVariantMap.cpp
+++ b/libraries/animation/src/AnimVariantMap.cpp
@@ -18,6 +18,7 @@
 QScriptValue AnimVariantMap::animVariantMapToScriptValue(QScriptEngine* engine, const QStringList& names, bool useNames) const {
     if (QThread::currentThread() != engine->thread()) {
         qCWarning(animation) << "Cannot create Javacript object from non-script thread" << QThread::currentThread();
+        Q_ASSERT(false);
         return QScriptValue();
     }
     QScriptValue target = engine->newObject();
@@ -69,6 +70,7 @@ void AnimVariantMap::copyVariantsFrom(const AnimVariantMap& other) {
 void AnimVariantMap::animVariantMapFromScriptValue(const QScriptValue& source) {
     if (QThread::currentThread() != source.engine()->thread()) {
         qCWarning(animation) << "Cannot examine Javacript object from non-script thread" << QThread::currentThread();
+        Q_ASSERT(false);
         return;
     }
     // POTENTIAL OPTIMIZATION: cache the types we've seen. I.e, keep a dictionary mapping property names to an enumeration of types.

From 502cc7f580c90483b65d10638f2e622a40c8c702 Mon Sep 17 00:00:00 2001
From: Howard Stearns <howard@highfidelity.io>
Date: Thu, 29 Oct 2015 16:44:01 -0700
Subject: [PATCH 24/31] Don't copy while converting.

---
 libraries/animation/src/AnimVariantMap.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libraries/animation/src/AnimVariantMap.cpp b/libraries/animation/src/AnimVariantMap.cpp
index fc474c0cbd..2d9291db2d 100644
--- a/libraries/animation/src/AnimVariantMap.cpp
+++ b/libraries/animation/src/AnimVariantMap.cpp
@@ -22,7 +22,7 @@ QScriptValue AnimVariantMap::animVariantMapToScriptValue(QScriptEngine* engine,
         return QScriptValue();
     }
     QScriptValue target = engine->newObject();
-    auto setOne = [&] (QString name, AnimVariant value) {
+    auto setOne = [&] (const QString& name, const AnimVariant& value) {
         switch (value.getType()) {
             case AnimVariant::Type::Bool:
                 target.setProperty(name, value.getBool());

From ada32dd260528ab62efd9ce85eaa2062900040dc Mon Sep 17 00:00:00 2001
From: Howard Stearns <howard@highfidelity.io>
Date: Thu, 29 Oct 2015 19:17:52 -0700
Subject: [PATCH 25/31] typo

---
 libraries/script-engine/src/ScriptEngine.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp
index 46dbe7576c..79df1c3bb8 100644
--- a/libraries/script-engine/src/ScriptEngine.cpp
+++ b/libraries/script-engine/src/ScriptEngine.cpp
@@ -780,9 +780,9 @@ void ScriptEngine::callAnimationStateHandler(QScriptValue callback, AnimVariantM
                                   Q_ARG(AnimVariantResultHandler, resultHandler));
         return;
     }
-    QScriptValue javascriptParametgers = parameters.animVariantMapToScriptValue(this, names, useNames);
+    QScriptValue javascriptParameters = parameters.animVariantMapToScriptValue(this, names, useNames);
     QScriptValueList callingArguments;
-    callingArguments << javascriptParametgers;
+    callingArguments << javascriptParameters;
     QScriptValue result = callback.call(QScriptValue(), callingArguments);
     resultHandler(result);
 }

From 5d1ba90f1ebd7f652485bf3f01b9b23a23ef2c37 Mon Sep 17 00:00:00 2001
From: Howard Stearns <howard@highfidelity.io>
Date: Thu, 29 Oct 2015 19:18:34 -0700
Subject: [PATCH 26/31] More readable code.

---
 libraries/animation/src/AnimVariantMap.cpp | 37 +++++++++++-----------
 1 file changed, 19 insertions(+), 18 deletions(-)

diff --git a/libraries/animation/src/AnimVariantMap.cpp b/libraries/animation/src/AnimVariantMap.cpp
index 2d9291db2d..c3e452fa1e 100644
--- a/libraries/animation/src/AnimVariantMap.cpp
+++ b/libraries/animation/src/AnimVariantMap.cpp
@@ -67,6 +67,7 @@ void AnimVariantMap::copyVariantsFrom(const AnimVariantMap& other) {
         _map[pair.first] = pair.second;
     }
 }
+
 void AnimVariantMap::animVariantMapFromScriptValue(const QScriptValue& source) {
     if (QThread::currentThread() != source.engine()->thread()) {
         qCWarning(animation) << "Cannot examine Javacript object from non-script thread" << QThread::currentThread();
@@ -84,10 +85,8 @@ void AnimVariantMap::animVariantMapFromScriptValue(const QScriptValue& source) {
         QScriptValue value = property.value();
         if (value.isBool()) {
             set(property.name(), value.toBool());
-            continue;
         } else if (value.isString()) {
             set(property.name(), value.toString());
-            continue;
         } else if (value.isNumber()) {
             int asInteger = value.toInt32();
             float asFloat = value.toNumber();
@@ -96,25 +95,27 @@ void AnimVariantMap::animVariantMapFromScriptValue(const QScriptValue& source) {
             } else {
                 set(property.name(), asFloat);
             }
-            continue;
-        } else if (value.isObject()) {
-            QScriptValue x = value.property("x");
-            if (x.isNumber()) {
-                QScriptValue y = value.property("y");
-                if (y.isNumber()) {
-                    QScriptValue z = value.property("z");
-                    if (z.isNumber()) {
-                        QScriptValue w = value.property("w");
-                        if (w.isNumber()) {
-                            set(property.name(), glm::quat(x.toNumber(), y.toNumber(), z.toNumber(), w.toNumber()));
-                        } else {
-                            set(property.name(), glm::vec3(x.toNumber(), y.toNumber(), z.toNumber()));
+        } else { // Try to get x,y,z and possibly w
+            if (value.isObject()) {
+                QScriptValue x = value.property("x");
+                if (x.isNumber()) {
+                    QScriptValue y = value.property("y");
+                    if (y.isNumber()) {
+                        QScriptValue z = value.property("z");
+                        if (z.isNumber()) {
+                            QScriptValue w = value.property("w");
+                            if (w.isNumber()) {
+                                set(property.name(), glm::quat(x.toNumber(), y.toNumber(), z.toNumber(), w.toNumber()));
+                            } else {
+                                set(property.name(), glm::vec3(x.toNumber(), y.toNumber(), z.toNumber()));
+                            }
+                            continue; // we got either a vector or quaternion object, so don't fall through to warning
                         }
-                        continue;
                     }
                 }
             }
+            qCWarning(animation) << "Ignoring unrecognized data" << value.toString() << "for animation property" << property.name();
+            Q_ASSERT(false);
         }
-        qCWarning(animation) << "Ignoring unrecognized data" << value.toString() << "for animation property" << property.name();
-     }
+    }
 }

From dcc173c93a5da52364666e78f85081a884055a4b Mon Sep 17 00:00:00 2001
From: Howard Stearns <howard@highfidelity.io>
Date: Thu, 29 Oct 2015 19:21:24 -0700
Subject: [PATCH 27/31] comment.

---
 libraries/animation/src/Rig.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libraries/animation/src/Rig.cpp b/libraries/animation/src/Rig.cpp
index b51907ea4a..ed5e07696d 100644
--- a/libraries/animation/src/Rig.cpp
+++ b/libraries/animation/src/Rig.cpp
@@ -615,7 +615,7 @@ void Rig::updateAnimationStateHandlers() { // called on avatar update thread (wh
         int identifier = data.key();
         StateHandler& value = data.value();
         QScriptValue& function = value.function;
-        auto handleResult = [this, identifier](QScriptValue result) {
+        auto handleResult = [this, identifier](QScriptValue result) { // called in script thread to get the result back to us.
             animationStateHandlerResult(identifier, result);
         };
         // invokeMethod makes a copy of the args, and copies of AnimVariantMap do copy the underlying map, so this will correctly capture

From f7d558a2528e036b6869f4fa528002444f7878c1 Mon Sep 17 00:00:00 2001
From: Howard Stearns <howard@highfidelity.io>
Date: Thu, 29 Oct 2015 19:26:54 -0700
Subject: [PATCH 28/31] comment

---
 libraries/animation/src/Rig.cpp | 1 +
 1 file changed, 1 insertion(+)

diff --git a/libraries/animation/src/Rig.cpp b/libraries/animation/src/Rig.cpp
index ed5e07696d..4ddae07375 100644
--- a/libraries/animation/src/Rig.cpp
+++ b/libraries/animation/src/Rig.cpp
@@ -580,6 +580,7 @@ void Rig::computeMotionAnimationState(float deltaTime, const glm::vec3& worldPos
 // Allow script to add/remove handlers and report results, from within their thread.
 QScriptValue Rig::addAnimationStateHandler(QScriptValue handler, QScriptValue propertiesList) { // called in script thread
     QMutexLocker locker(&_stateMutex);
+    // Find a safe id, even if there are lots of many scripts add and remove handlers repeatedly.
     while (!_nextStateHandlerId || _stateHandlers.contains(_nextStateHandlerId)) { // 0 is unused, and don't reuse existing after wrap.
       _nextStateHandlerId++;
     }

From 1918f1835cfc44ee995ff2d0f64274d7cae66f5e Mon Sep 17 00:00:00 2001
From: Howard Stearns <howard@highfidelity.io>
Date: Thu, 29 Oct 2015 19:37:13 -0700
Subject: [PATCH 29/31] Tolerate AnimVars that are float when we want int, and
 vice versa.

---
 libraries/animation/src/AnimVariant.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/libraries/animation/src/AnimVariant.h b/libraries/animation/src/AnimVariant.h
index bd96dda77d..0d7c657058 100644
--- a/libraries/animation/src/AnimVariant.h
+++ b/libraries/animation/src/AnimVariant.h
@@ -61,8 +61,9 @@ public:
     void setString(const QString& value) { assert(_type == Type::String); _stringVal = value; }
 
     bool getBool() const { assert(_type == Type::Bool); return _val.boolVal; }
-    int getInt() const { assert(_type == Type::Int); return _val.intVal; }
-    float getFloat() const { assert(_type == Type::Float); return _val.floats[0]; }
+    int getInt() const { assert(_type == Type::Int || _type == Type::Float); return _type == Type::Float ? (int)_val.floats[0] : _val.intVal; }
+    float getFloat() const { assert(_type == Type::Float || _type == Type::Int); return _type == Type::Int ? (float)_val.intVal : _val.floats[0]; }
+
     const glm::vec3& getVec3() const { assert(_type == Type::Vec3); return *reinterpret_cast<const glm::vec3*>(&_val); }
     const glm::quat& getQuat() const { assert(_type == Type::Quat); return *reinterpret_cast<const glm::quat*>(&_val); }
     const glm::mat4& getMat4() const { assert(_type == Type::Mat4); return *reinterpret_cast<const glm::mat4*>(&_val); }

From d491ddc3d61a9fac53ac0efcb487eff6342967ad Mon Sep 17 00:00:00 2001
From: Howard Stearns <howard@highfidelity.io>
Date: Thu, 29 Oct 2015 19:43:57 -0700
Subject: [PATCH 30/31] comment.

---
 libraries/animation/src/AnimVariantMap.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libraries/animation/src/AnimVariantMap.cpp b/libraries/animation/src/AnimVariantMap.cpp
index c3e452fa1e..8d320195dd 100644
--- a/libraries/animation/src/AnimVariantMap.cpp
+++ b/libraries/animation/src/AnimVariantMap.cpp
@@ -13,7 +13,7 @@
 #include <QScriptValueIterator>
 #include <QThread>
 #include <RegisteredMetaTypes.h>
-#include "AnimVariant.h"
+#include "AnimVariant.h" // which has AnimVariant/AnimVariantMap
 
 QScriptValue AnimVariantMap::animVariantMapToScriptValue(QScriptEngine* engine, const QStringList& names, bool useNames) const {
     if (QThread::currentThread() != engine->thread()) {

From eb9e54de41eb7add77d5cc91cf85eb0164375106 Mon Sep 17 00:00:00 2001
From: Howard Stearns <howard@highfidelity.io>
Date: Thu, 29 Oct 2015 19:45:23 -0700
Subject: [PATCH 31/31] Make AnimVariantXXX.xxx consistent.

---
 libraries/animation/src/{AnimVariantMap.cpp => AnimVariant.cpp} | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 rename libraries/animation/src/{AnimVariantMap.cpp => AnimVariant.cpp} (100%)

diff --git a/libraries/animation/src/AnimVariantMap.cpp b/libraries/animation/src/AnimVariant.cpp
similarity index 100%
rename from libraries/animation/src/AnimVariantMap.cpp
rename to libraries/animation/src/AnimVariant.cpp