Allow multiple scripts to register, and allow them to specify the specific anim vars they are interested in.

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)
 

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) {
+    auto setOne = [&] (QString name, AnimVariant value) {
-        switch (pair.second.getType()) {
+        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;
 }

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()) {
+    QMutexLocker locker(&_stateMutex);
-        auto handleResult = [this](QScriptValue handler, QScriptValue result) {
+    // It might pay to produce just one AnimVariantMap copy here, with a union of all the requested propertyNames,
-            animationStateHandlerResult(handler, result);
+    // 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,
+        QMetaObject::invokeMethod(function.engine(), "callAnimationStateHandler",  Qt::QueuedConnection,
-                                  Q_ARG(QScriptValue, _stateHandlers),
+                                  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) {

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;
+    QMap<int, StateHandler> _stateHandlers;
-    AnimVariantMap _stateHandlersResults;
+    int _nextStateHandlerId {0};
     QMutex _stateMutex;
 };
 

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() {

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);