first cut at cleaning up ScriptEngine class

assignment-client/src/Agent.cpp

@@ -169,9 +169,8 @@ void Agent::run() {
     _scriptEngine.setAvatarData(&scriptedAvatar, "Avatar");
     
     auto avatarHashMap = DependencyManager::set<AvatarHashMap>();
-   
+    _scriptEngine.registerGlobalObject("AvatarList", avatarHashMap.data());
-    _scriptEngine.setAvatarHashMap(avatarHashMap.data(), "AvatarList");
+
-    
     auto& packetReceiver = DependencyManager::get<NodeList>()->getPacketReceiver();
     packetReceiver.registerListener(PacketType::BulkAvatarData, avatarHashMap.data(), "processAvatarDataPacket");
     packetReceiver.registerListener(PacketType::KillAvatar, avatarHashMap.data(), "processKillAvatar");
@@ -185,6 +184,8 @@ void Agent::run() {
         _scriptEngine.setParentURL(_payload);
     }
 
+    // FIXME -we shouldn't be calling this directly, it's normally called by run(), not sure why 
+    // viewers would need this called.
     _scriptEngine.init(); // must be done before we set up the viewers
 
     _scriptEngine.registerGlobalObject("SoundCache", DependencyManager::get<SoundCache>().data());

assignment-client/src/Agent.h

@@ -37,7 +37,7 @@ class Agent : public ThreadedAssignment {
 public:
     Agent(NLPacket& packet);
     
-    void setIsAvatar(bool isAvatar) { QMetaObject::invokeMethod(&_scriptEngine, "setIsAvatar", Q_ARG(bool, isAvatar)); }
+    void setIsAvatar(bool isAvatar) { _scriptEngine.setIsAvatar(isAvatar); }
     bool isAvatar() const { return _scriptEngine.isAvatar(); }
     
     bool isPlayingAvatarSound() const  { return _scriptEngine.isPlayingAvatarSound(); }

interface/src/Application.cpp

@@ -4011,8 +4011,8 @@ void Application::registerScriptEngineWithApplicationServices(ScriptEngine* scri
     AvatarManager::registerMetaTypes(scriptEngine);
 
     // hook our avatar and avatar hash map object into this script engine
-    scriptEngine->setAvatarData(_myAvatar, "MyAvatar"); // leave it as a MyAvatar class to expose thrust features
+    scriptEngine->registerGlobalObject("MyAvatar", _myAvatar);
-    scriptEngine->setAvatarHashMap(DependencyManager::get<AvatarManager>().data(), "AvatarList");
+    scriptEngine->registerGlobalObject("AvatarList", DependencyManager::get<AvatarManager>().data());
 
     scriptEngine->registerGlobalObject("Camera", &_myCamera);
 
@@ -4036,9 +4036,9 @@ void Application::registerScriptEngineWithApplicationServices(ScriptEngine* scri
 
     scriptEngine->registerGlobalObject("Desktop", DependencyManager::get<DesktopScriptingInterface>().data());
 
-    QScriptValue windowValue = scriptEngine->registerGlobalObject("Window", DependencyManager::get<WindowScriptingInterface>().data());
+    scriptEngine->registerGlobalObject("Window", DependencyManager::get<WindowScriptingInterface>().data());
     scriptEngine->registerGetterSetter("location", LocationScriptingInterface::locationGetter,
-                                       LocationScriptingInterface::locationSetter, windowValue);
+                        LocationScriptingInterface::locationSetter, "Window");
     // register `location` on the global object.
     scriptEngine->registerGetterSetter("location", LocationScriptingInterface::locationGetter,
                                        LocationScriptingInterface::locationSetter);
@@ -4068,9 +4068,9 @@ void Application::registerScriptEngineWithApplicationServices(ScriptEngine* scri
 
     scriptEngine->registerGlobalObject("Paths", DependencyManager::get<PathUtils>().data());
 
-    QScriptValue hmdInterface = scriptEngine->registerGlobalObject("HMD", &HMDScriptingInterface::getInstance());
+    scriptEngine->registerGlobalObject("HMD", &HMDScriptingInterface::getInstance());
-    scriptEngine->registerFunction(hmdInterface, "getHUDLookAtPosition2D", HMDScriptingInterface::getHUDLookAtPosition2D, 0);
+    scriptEngine->registerFunction("HMD", "getHUDLookAtPosition2D", HMDScriptingInterface::getHUDLookAtPosition2D, 0);
-    scriptEngine->registerFunction(hmdInterface, "getHUDLookAtPosition3D", HMDScriptingInterface::getHUDLookAtPosition3D, 0);
+    scriptEngine->registerFunction("HMD", "getHUDLookAtPosition3D", HMDScriptingInterface::getHUDLookAtPosition3D, 0);
 
     scriptEngine->registerGlobalObject("Scene", DependencyManager::get<SceneScriptingInterface>().data());
 
@@ -4079,31 +4079,6 @@ void Application::registerScriptEngineWithApplicationServices(ScriptEngine* scri
 #ifdef HAVE_RTMIDI
     scriptEngine->registerGlobalObject("MIDI", &MIDIManager::getInstance());
 #endif
-
-    // TODO: Consider moving some of this functionality into the ScriptEngine class instead. It seems wrong that this
-    // work is being done in the Application class when really these dependencies are more related to the ScriptEngine's
-    // implementation
-    QThread* workerThread = new QThread(this);
-    QString scriptEngineName = QString("Script Thread:") + scriptEngine->getFilename();
-    workerThread->setObjectName(scriptEngineName);
-
-    // when the worker thread is started, call our engine's run..
-    connect(workerThread, &QThread::started, scriptEngine, &ScriptEngine::run);
-    
-    // when the thread is terminated, add both scriptEngine and thread to the deleteLater queue
-    connect(scriptEngine, &ScriptEngine::doneRunning, scriptEngine, &ScriptEngine::deleteLater);
-    connect(workerThread, &QThread::finished, workerThread, &QThread::deleteLater);
-    
-    // tell the thread to stop when the script engine is done
-    connect(scriptEngine, &ScriptEngine::destroyed, workerThread, &QThread::quit);
-
-    auto nodeList = DependencyManager::get<NodeList>();
-    connect(nodeList.data(), &NodeList::nodeKilled, scriptEngine, &ScriptEngine::nodeKilled);
-
-    scriptEngine->moveToThread(workerThread);
-
-    // Starts an event loop, and emits workerThread->started()
-    workerThread->start();
 }
 
 void Application::initializeAcceptedFiles() {
@@ -4249,17 +4224,34 @@ ScriptEngine* Application::loadScript(const QString& scriptFilename, bool isUser
     scriptEngine->setUserLoaded(isUserLoaded);
 
     if (scriptFilename.isNull()) {
+        // This appears to be the script engine used by the script widget's evaluation window before the file has been saved...
+
+        qDebug() << "############################# HELLO WE ARE HERE!!!! ##################################";
         // this had better be the script editor (we should de-couple so somebody who thinks they are loading a script
         // doesn't just get an empty script engine)
 
         // we can complete setup now since there isn't a script we have to load
         registerScriptEngineWithApplicationServices(scriptEngine);
+        scriptEngine->runInThread();
+
+        //FIXME - intentionally attempting to test thread safe call to registerGlobalObject()
+        qDebug() << "about to attempt to call registerGlobalObject on wrong thread!!! <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<";
+        scriptEngine->registerGlobalObject("LODManager2", DependencyManager::get<LODManager>().data());
+
+        qDebug() << "about to attempt to call registerFunction on wrong thread!!! <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<";
+        scriptEngine->registerFunction("WebWindow2", WebWindowClass::constructor, 1);
+
+        qDebug() << "about to attempt to call registerGetterSetter on wrong thread!!! <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<";
+        scriptEngine->registerGetterSetter("foo_location", LocationScriptingInterface::locationGetter, LocationScriptingInterface::locationSetter);
+
+
     } else {
         // connect to the appropriate signals of this script engine
         connect(scriptEngine, &ScriptEngine::scriptLoaded, this, &Application::handleScriptEngineLoaded);
         connect(scriptEngine, &ScriptEngine::errorLoadingScript, this, &Application::handleScriptLoadError);
 
         // get the script engine object to load the script at the designated script URL
+        qDebug() << "calling scriptEngine->loadURL(" << scriptUrl << ", " << reload << ");";
         scriptEngine->loadURL(scriptUrl, reload);
     }
 
@@ -4276,6 +4268,7 @@ void Application::reloadScript(const QString& scriptName, bool isUserLoaded) {
 }
 
 void Application::handleScriptEngineLoaded(const QString& scriptFilename) {
+    qDebug() << "handleScriptEngineLoaded().... scriptFilename:" << scriptFilename;
     ScriptEngine* scriptEngine = qobject_cast<ScriptEngine*>(sender());
 
     _scriptEnginesHash.insertMulti(scriptFilename, scriptEngine);
@@ -4284,6 +4277,17 @@ void Application::handleScriptEngineLoaded(const QString& scriptFilename) {
 
     // register our application services and set it off on its own thread
     registerScriptEngineWithApplicationServices(scriptEngine);
+    scriptEngine->runInThread();
+
+    //FIXME - intentionally attempting to test thread safe call to registerGlobalObject()
+    qDebug() << "about to attempt to call registerGlobalObject on wrong thread!!! <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<";
+    scriptEngine->registerGlobalObject("LODManager2", DependencyManager::get<LODManager>().data());
+
+    qDebug() << "about to attempt to call registerFunction on wrong thread!!! <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<";
+    scriptEngine->registerFunction("WebWindow2", WebWindowClass::constructor, 1);
+
+    qDebug() << "about to attempt to call registerGetterSetter on wrong thread!!! <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<";
+    scriptEngine->registerGetterSetter("foo_location", LocationScriptingInterface::locationGetter, LocationScriptingInterface::locationSetter);
 }
 
 void Application::handleScriptLoadError(const QString& scriptFilename) {

libraries/entities-renderer/src/EntityTreeRenderer.cpp

@@ -116,6 +116,9 @@ void EntityTreeRenderer::init() {
                                         _scriptingServices->getControllerScriptingInterface(), false);
         _scriptingServices->registerScriptEngineWithApplicationServices(_entitiesScriptEngine);
 
+        // FIXME - this is dubious need to rework
+        _entitiesScriptEngine->runInThread();
+
         _sandboxScriptEngine = new ScriptEngine(NO_SCRIPT, "Entities Sandbox", NULL, false);
     }
 

libraries/script-engine/src/ScriptEngine.cpp

@@ -44,6 +44,8 @@
 
 #include "MIDIEvent.h"
 
+Q_DECLARE_METATYPE(QScriptEngine::FunctionSignature)
+static int functionSignatureMetaID = qRegisterMetaType<QScriptEngine::FunctionSignature>();
 
 static QScriptValue debugPrint(QScriptContext* context, QScriptEngine* engine){
     QString message = "";
@@ -97,7 +99,6 @@ ScriptEngine::ScriptEngine(const QString& scriptContents, const QString& fileNam
     _controllerScriptingInterface(controllerScriptingInterface),
     _wantSignals(wantSignals),
     _avatarData(NULL),
-    _scriptName(),
     _fileNameString(fileNameString),
     _quatLibrary(),
     _vec3Library(),
@@ -122,6 +123,27 @@ ScriptEngine::~ScriptEngine() {
     }
 }
 
+void ScriptEngine::runInThread() {
+    QThread* workerThread = new QThread(this);
+    QString scriptEngineName = QString("Script Thread:") + getFilename();
+    workerThread->setObjectName(scriptEngineName);
+
+    // when the worker thread is started, call our engine's run..
+    connect(workerThread, &QThread::started, this, &ScriptEngine::run);
+
+    // when the thread is terminated, add both scriptEngine and thread to the deleteLater queue
+    connect(this, &ScriptEngine::doneRunning, this, &ScriptEngine::deleteLater);
+    connect(workerThread, &QThread::finished, workerThread, &QThread::deleteLater);
+
+    // tell the thread to stop when the script engine is done
+    connect(this, &ScriptEngine::destroyed, workerThread, &QThread::quit);
+
+    moveToThread(workerThread);
+
+    // Starts an event loop, and emits workerThread->started()
+    workerThread->start();
+}
+
 QSet<ScriptEngine*> ScriptEngine::_allKnownScriptEngines;
 QMutex ScriptEngine::_allScriptsMutex;
 bool ScriptEngine::_stoppingAllScripts = false;
@@ -180,8 +202,6 @@ void ScriptEngine::stopAllScripts(QObject* application) {
 
 
 void ScriptEngine::waitTillDoneRunning() {
-    QString scriptName = getFilename();
-
     // If the script never started running or finished running before we got here, we don't need to wait for it
     if (_isRunning) {
 
@@ -244,14 +264,6 @@ void ScriptEngine::setAvatarData(AvatarData* avatarData, const QString& objectNa
     registerGlobalObject(objectName, _avatarData);
 }
 
-void ScriptEngine::setAvatarHashMap(AvatarHashMap* avatarHashMap, const QString& objectName) {
-    // remove the old Avatar property, if it exists
-    globalObject().setProperty(objectName, QScriptValue());
-
-    // give the script engine the new avatar hash map
-    registerGlobalObject(objectName, avatarHashMap);
-}
-
 bool ScriptEngine::setScriptContents(const QString& scriptContents, const QString& fileNameString) {
     if (_isRunning) {
         return false;
@@ -261,7 +273,10 @@ bool ScriptEngine::setScriptContents(const QString& scriptContents, const QStrin
     return true;
 }
 
+// FIXME - remove the file/url scheme code here and let the script cache handle things
 void ScriptEngine::loadURL(const QUrl& scriptURL, bool reload) {
+    qDebug() << "ScriptEngine::loadURL(" << scriptURL << ", " << reload << ");";
+
     if (_isRunning) {
         return;
     }
@@ -298,7 +313,10 @@ void ScriptEngine::loadURL(const QUrl& scriptURL, bool reload) {
         } else {
             bool isPending;
             auto scriptCache = DependencyManager::get<ScriptCache>();
+            qDebug() << "calling scriptCache->getScript(" << url << ", " << reload << ");";
             scriptCache->getScript(url, this, isPending, reload);
+            qDebug() << "    scriptCache->getScript(" << url << ", " << reload << ") ... isPending:" << isPending;
+
         }
     }
 }
@@ -383,57 +401,97 @@ void ScriptEngine::init() {
     globalObject().setProperty("TREE_SCALE", newVariant(QVariant(TREE_SCALE)));
 }
 
-QScriptValue ScriptEngine::registerGlobalObject(const QString& name, QObject* object) {
+void ScriptEngine::registerGlobalObject(const QString& name, QObject* object) {
+    if (QThread::currentThread() != thread()) {
+        qDebug() << "*** WARNING *** ScriptEngine::registerGlobalObject() called on wrong thread [" << QThread::currentThread() << "], invoking on correct thread [" << thread() << "]  name:" << name;
+        QMetaObject::invokeMethod(this, "registerGlobalObject",
+            Q_ARG(const QString&, name),
+            Q_ARG(QObject*, object));
+        return;
+    }
+    //qDebug() << "ScriptEngine::registerGlobalObject() called on thread [" << QThread::currentThread() << "] name:" << name;
+
     if (object) {
         QScriptValue value = newQObject(object);
         globalObject().setProperty(name, value);
-        return value;
     }
-    return QScriptValue::NullValue;
 }
 
-void ScriptEngine::registerFunction(const QString& name, QScriptEngine::FunctionSignature fun, int numArguments) {
+void ScriptEngine::registerFunction(const QString& name, QScriptEngine::FunctionSignature functionSignature, int numArguments) {
-    registerFunction(globalObject(), name, fun, numArguments);
+    if (QThread::currentThread() != thread()) {
+        qDebug() << "*** WARNING *** ScriptEngine::registerFunction() called on wrong thread [" << QThread::currentThread() << "], invoking on correct thread [" << thread() << "] name:" << name;
+        QMetaObject::invokeMethod(this, "registerFunction",
+            Q_ARG(const QString&, name),
+            Q_ARG(QScriptEngine::FunctionSignature, functionSignature),
+            Q_ARG(int, numArguments));
+        return;
+    }
+    //qDebug() << "ScriptEngine::registerFunction() called on thread [" << QThread::currentThread() << "] name:" << name;
+
+    QScriptValue scriptFun = newFunction(functionSignature, numArguments);
+    globalObject().setProperty(name, scriptFun);
 }
 
-void ScriptEngine::registerFunction(QScriptValue parent, const QString& name, QScriptEngine::FunctionSignature fun, int numArguments) {
+void ScriptEngine::registerFunction(const QString& parent, const QString& name, QScriptEngine::FunctionSignature functionSignature, int numArguments) {
-    QScriptValue scriptFun = newFunction(fun, numArguments);
+    if (QThread::currentThread() != thread()) {
-    parent.setProperty(name, scriptFun);
+        qDebug() << "*** WARNING *** ScriptEngine::registerFunction() called on wrong thread [" << QThread::currentThread() << "], invoking on correct thread [" << thread() << "] parent:" << parent << "name:" << name;
+        QMetaObject::invokeMethod(this, "registerFunction",
+            Q_ARG(const QString&, name),
+            Q_ARG(QScriptEngine::FunctionSignature, functionSignature),
+            Q_ARG(int, numArguments));
+        return;
+    }
+    //qDebug() << "ScriptEngine::registerFunction() called on thread [" << QThread::currentThread() << "] parent:" << parent << "name:" << name;
+
+    QScriptValue object = globalObject().property(parent);
+    if (object.isValid()) {
+        QScriptValue scriptFun = newFunction(functionSignature, numArguments);
+        object.setProperty(name, scriptFun);
+    }
 }
 
 void ScriptEngine::registerGetterSetter(const QString& name, QScriptEngine::FunctionSignature getter,
-                                        QScriptEngine::FunctionSignature setter, QScriptValue object) {
+                            QScriptEngine::FunctionSignature setter, const QString& parent) {
+    if (QThread::currentThread() != thread()) {
+        qDebug() << "*** WARNING *** ScriptEngine::registerGetterSetter() called on wrong thread [" << QThread::currentThread() << "], invoking on correct thread [" << thread() << "] "
+                    " name:" << name << "parent:" << parent;
+        QMetaObject::invokeMethod(this, "registerGetterSetter",
+            Q_ARG(const QString&, name),
+            Q_ARG(QScriptEngine::FunctionSignature, getter),
+            Q_ARG(QScriptEngine::FunctionSignature, setter),
+            Q_ARG(const QString&, parent));
+        return;
+    }
+    //qDebug() << "ScriptEngine::registerGetterSetter() called on thread [" << QThread::currentThread() << "] name:" << name << "parent:" << parent;
+
     QScriptValue setterFunction = newFunction(setter, 1);
     QScriptValue getterFunction = newFunction(getter);
 
-    if (!object.isNull()) {
+    if (!parent.isNull()) {
-        object.setProperty(name, setterFunction, QScriptValue::PropertySetter);
+        QScriptValue object = globalObject().property(parent);
-        object.setProperty(name, getterFunction, QScriptValue::PropertyGetter);
+        if (object.isValid()) {
+            object.setProperty(name, setterFunction, QScriptValue::PropertySetter);
+            object.setProperty(name, getterFunction, QScriptValue::PropertyGetter);
+        }
     } else {
         globalObject().setProperty(name, setterFunction, QScriptValue::PropertySetter);
         globalObject().setProperty(name, getterFunction, QScriptValue::PropertyGetter);
     }
 }
 
-// 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 (!_registeredHandlers.contains(entityID)) {
-        return;
-    }
-    const RegisteredEventHandlers& handlersOnEntity = _registeredHandlers[entityID];
-    if (!handlersOnEntity.contains(eventName)) {
-        return;
-    }
-    QScriptValueList handlersForEvent = handlersOnEntity[eventName];
-    if (!handlersForEvent.isEmpty()) {
-        QScriptValueList args = argGenerator();
-        for (int i = 0; i < handlersForEvent.count(); ++i) {
-            handlersForEvent[i].call(QScriptValue(), args);
-        }
-    }
-}
 // Unregister the handlers for this eventName and entityID.
 void ScriptEngine::removeEventHandler(const EntityItemID& entityID, const QString& eventName, QScriptValue handler) {
+    if (QThread::currentThread() != thread()) {
+        qDebug() << "*** WARNING *** ScriptEngine::removeEventHandler() called on wrong thread [" << QThread::currentThread() << "], invoking on correct thread [" << thread() << "] "
+                    "entityID:" << entityID << " eventName:" << eventName;
+        QMetaObject::invokeMethod(this, "removeEventHandler",
+            Q_ARG(const EntityItemID&, entityID),
+            Q_ARG(const QString&, eventName),
+            Q_ARG(QScriptValue, handler));
+        return;
+    }
+    //qDebug() << "ScriptEngine::removeEventHandler() called on thread [" << QThread::currentThread() << "] entityID:" << entityID << " eventName : " << eventName;
+
     if (!_registeredHandlers.contains(entityID)) {
         return;
     }
@@ -449,6 +507,17 @@ void ScriptEngine::removeEventHandler(const EntityItemID& entityID, const QStrin
 }
 // Register the handler.
 void ScriptEngine::addEventHandler(const EntityItemID& entityID, const QString& eventName, QScriptValue handler) {
+    if (QThread::currentThread() != thread()) {
+        qDebug() << "*** WARNING *** ScriptEngine::addEventHandler() called on wrong thread [" << QThread::currentThread() << "], invoking on correct thread [" << thread() << "] "
+            "entityID:" << entityID << " eventName:" << eventName;
+        QMetaObject::invokeMethod(this, "addEventHandler",
+            Q_ARG(const EntityItemID&, entityID),
+            Q_ARG(const QString&, eventName),
+            Q_ARG(QScriptValue, handler));
+        return;
+    }
+    //qDebug() << "ScriptEngine::addEventHandler() called on thread [" << QThread::currentThread() << "] entityID:" << entityID << " eventName : " << eventName;
+
     if (_registeredHandlers.count() == 0) { // First time any per-entity handler has been added in this script...
         // Connect up ALL the handlers to the global entities object's signals.
         // (We could go signal by signal, or even handler by handler, but I don't think the efficiency is worth the complexity.)
@@ -503,34 +572,23 @@ void ScriptEngine::addEventHandler(const EntityItemID& entityID, const QString&
 }
 
 
-void ScriptEngine::evaluate() {
-    if (_stoppingAllScripts) {
-        return; // bail early
-    }
-
-    if (!_isInitialized) {
-        init();
-    }
-
-    QScriptValue result = evaluate(_scriptContents);
-
-    // TODO: why do we check this twice? It seems like the call to clearExceptions() in the lower level evaluate call
-    // will cause this code to never actually run...
-    if (hasUncaughtException()) {
-        int line = uncaughtExceptionLineNumber();
-        qCDebug(scriptengine) << "Uncaught exception at (" << _fileNameString << ") line" << line << ":" << result.toString();
-        if (_wantSignals) {
-            emit errorMessage("Uncaught exception at (" + _fileNameString + ") line" + QString::number(line) + ":" + result.toString());
-        }
-        clearExceptions();
-    }
-}
-
 QScriptValue ScriptEngine::evaluate(const QString& program, const QString& fileName, int lineNumber) {
     if (_stoppingAllScripts) {
         return QScriptValue(); // bail early
     }
 
+    if (QThread::currentThread() != thread()) {
+        QScriptValue result;
+        qDebug() << "*** WARNING *** ScriptEngine::evaluate() called on wrong thread [" << QThread::currentThread() << "], invoking on correct thread [" << thread() << "] "
+            "program:" << program << " fileName:" << fileName << "lineNumber:" << lineNumber;
+        QMetaObject::invokeMethod(this, "evaluate", Qt::BlockingQueuedConnection,
+            Q_RETURN_ARG(QScriptValue, result),
+            Q_ARG(const QString&, program),
+            Q_ARG(const QString&, fileName),
+            Q_ARG(int, lineNumber));
+        return result;
+    }
+
     _evaluatesPending++;
     QScriptValue result = QScriptEngine::evaluate(program, fileName, lineNumber);
     if (hasUncaughtException()) {
@@ -967,6 +1025,26 @@ void ScriptEngine::load(const QString& loadFile) {
     }
 }
 
-void ScriptEngine::nodeKilled(SharedNodePointer node) {
+// Look up the handler associated with eventName and entityID. If found, evalute the argGenerator thunk and call the handler with those args
-    _outgoingScriptAudioSequenceNumbers.remove(node->getUUID());
+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;
+    }
+
+    if (!_registeredHandlers.contains(entityID)) {
+        return;
+    }
+    const RegisteredEventHandlers& handlersOnEntity = _registeredHandlers[entityID];
+    if (!handlersOnEntity.contains(eventName)) {
+        return;
+    }
+    QScriptValueList handlersForEvent = handlersOnEntity[eventName];
+    if (!handlersForEvent.isEmpty()) {
+        QScriptValueList args = argGenerator();
+        for (int i = 0; i < handlersForEvent.count(); ++i) {
+            handlersForEvent[i].call(QScriptValue(), args);
+        }
+    }
 }

libraries/script-engine/src/ScriptEngine.h

@@ -50,79 +50,75 @@ public:
 
     ~ScriptEngine();
 
-    ArrayBufferClass* getArrayBufferClass() { return _arrayBufferClass; }
+    /// Launch the script running in a dedicated thread. This will have the side effect of evalulating
+    /// the current script contents and calling run(). Callers will likely want to register the script with external
+    /// services before calling this.
+    void runInThread();
+
+    ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
+    // NOTE - these are NOT intended to be public interfaces available to scripts, the are only Q_INVOKABLE so we can
+    //        properly ensure they are only called on the correct thread
+
+    /// registers a global object by name
+    Q_INVOKABLE void registerGlobalObject(const QString& name, QObject* object);
+
+    /// registers a global getter/setter
+    Q_INVOKABLE void registerGetterSetter(const QString& name, QScriptEngine::FunctionSignature getter,
+                            QScriptEngine::FunctionSignature setter, const QString& parent = QString(""));
     
-    /// sets the script contents, will return false if failed, will fail if script is already running
+    /// register a global function
-    bool setScriptContents(const QString& scriptContents, const QString& fileNameString = QString(""));
+    Q_INVOKABLE void registerFunction(const QString& name, QScriptEngine::FunctionSignature fun, int numArguments = -1);
 
-    const QString& getScriptName() const { return _scriptName; }
+    /// register a function as a method on a previously registered global object
-
+    Q_INVOKABLE void registerFunction(const QString& parent, const QString& name, QScriptEngine::FunctionSignature fun,
-    QScriptValue registerGlobalObject(const QString& name, QObject* object); /// registers a global object by name
-    void registerGetterSetter(const QString& name, QScriptEngine::FunctionSignature getter,
-                              QScriptEngine::FunctionSignature setter, QScriptValue object = QScriptValue::NullValue);
-    void registerFunction(const QString& name, QScriptEngine::FunctionSignature fun, int numArguments = -1);
-    void registerFunction(QScriptValue parent, const QString& name, QScriptEngine::FunctionSignature fun,
                           int numArguments = -1);
 
-    Q_INVOKABLE void setIsAvatar(bool isAvatar);
+    /// evaluate some code in the context of the ScriptEngine and return the result
-    bool isAvatar() const { return _isAvatar; }
+    Q_INVOKABLE QScriptValue evaluate(const QString& program, const QString& fileName = QString(), int lineNumber = 1); // this is also used by the script tool widget
 
-    void setAvatarData(AvatarData* avatarData, const QString& objectName);
+    /// if the script engine is not already running, this will download the URL and start the process of seting it up
-    void setAvatarHashMap(AvatarHashMap* avatarHashMap, const QString& objectName);
+    /// to run... NOTE - this is used by Application currently to load the url. We don't really want it to be exposed 
-
+    /// to scripts. we may not need this to be invokable
-    bool isListeningToAudioStream() const { return _isListeningToAudioStream; }
+    void loadURL(const QUrl& scriptURL, bool reload);
-    void setIsListeningToAudioStream(bool isListeningToAudioStream) { _isListeningToAudioStream = isListeningToAudioStream; }
-
-    void setAvatarSound(Sound* avatarSound) { _avatarSound = avatarSound; }
-    bool isPlayingAvatarSound() const { return _avatarSound != NULL; }
-
-    void init();
-    void run(); /// runs continuously until Agent.stop() is called
-    void evaluate(); /// initializes the engine, and evaluates the script, but then returns control to caller
-
-    void timerFired();
-
-    bool hasScript() const { return !_scriptContents.isEmpty(); }
-
-    bool isFinished() const { return _isFinished; }
-    bool isRunning() const { return _isRunning; }
-    bool evaluatePending() const { return _evaluatesPending > 0; }
-
-    void setUserLoaded(bool isUserLoaded) { _isUserLoaded = isUserLoaded;  }
-    bool isUserLoaded() const { return _isUserLoaded; }
-    
-    void setIsAgent(bool isAgent) { _isAgent = isAgent; }
-
-    void setParentURL(const QString& parentURL) { _parentURL = parentURL;  }
-    
-    QString getFilename() const;
-    
-    static void stopAllScripts(QObject* application);
-    
-    void waitTillDoneRunning();
-
-    virtual void scriptContentsAvailable(const QUrl& url, const QString& scriptContents);
-    virtual void errorInLoadingScript(const QUrl& url);
 
+    ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
+    // NOTE - these are intended to be public interfaces available to scripts 
     Q_INVOKABLE void addEventHandler(const EntityItemID& entityID, const QString& eventName, QScriptValue handler);
     Q_INVOKABLE void removeEventHandler(const EntityItemID& entityID, const QString& eventName, QScriptValue handler);
 
-public slots:
+    Q_INVOKABLE void load(const QString& loadfile);
-    void loadURL(const QUrl& scriptURL, bool reload);
+    Q_INVOKABLE void include(const QStringList& includeFiles, QScriptValue callback = QScriptValue());
-    void stop();
+    Q_INVOKABLE void include(const QString& includeFile, QScriptValue callback = QScriptValue());
 
-    QScriptValue evaluate(const QString& program, const QString& fileName = QString(), int lineNumber = 1);
+    Q_INVOKABLE QObject* setInterval(const QScriptValue& function, int intervalMS);
-    QObject* setInterval(const QScriptValue& function, int intervalMS);
+    Q_INVOKABLE QObject* setTimeout(const QScriptValue& function, int timeoutMS);
-    QObject* setTimeout(const QScriptValue& function, int timeoutMS);
+    Q_INVOKABLE void clearInterval(QObject* timer) { stopTimer(reinterpret_cast<QTimer*>(timer)); }
-    void clearInterval(QObject* timer) { stopTimer(reinterpret_cast<QTimer*>(timer)); }
+    Q_INVOKABLE void clearTimeout(QObject* timer) { stopTimer(reinterpret_cast<QTimer*>(timer)); }
-    void clearTimeout(QObject* timer) { stopTimer(reinterpret_cast<QTimer*>(timer)); }
+    Q_INVOKABLE void print(const QString& message);
-    void include(const QStringList& includeFiles, QScriptValue callback = QScriptValue());
+    Q_INVOKABLE QUrl resolvePath(const QString& path) const;
-    void include(const QString& includeFile, QScriptValue callback = QScriptValue());
-    void load(const QString& loadfile);
-    void print(const QString& message);
-    QUrl resolvePath(const QString& path) const;
 
-    void nodeKilled(SharedNodePointer node);
+    ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
+    // NOTE - this is intended to be a public interface for Agent scripts, and local scripts, but not for EntityScripts
+    Q_INVOKABLE void stop();
+
+    bool isFinished() const { return _isFinished; } // used by Application and ScriptWidget
+    bool isRunning() const { return _isRunning; } // used by ScriptWidget
+
+    static void stopAllScripts(QObject* application); // used by Application on shutdown
+
+    ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
+    // NOTE - These are the callback implementations for ScriptUser the get called by ScriptCache when the contents
+    // of a script are available.
+    virtual void scriptContentsAvailable(const QUrl& url, const QString& scriptContents);
+    virtual void errorInLoadingScript(const QUrl& url);
+
+    // These are currently used by Application to track if a script is user loaded or not. Consider finding a solution
+    // inside of Application so that the ScriptEngine class is not polluted by this notion
+    void setUserLoaded(bool isUserLoaded) { _isUserLoaded = isUserLoaded; }
+    bool isUserLoaded() const { return _isUserLoaded; }
+
+    // NOTE - this is used by the TypedArray implemetation. we need to review this for thread safety
+    ArrayBufferClass* getArrayBufferClass() { return _arrayBufferClass; }
 
 signals:
     void scriptLoaded(const QString& scriptFilename);
@@ -158,16 +154,17 @@ protected:
     bool _wantSignals = true;
     
 private:
+    QString getFilename() const;
+    void waitTillDoneRunning();
+    bool evaluatePending() const { return _evaluatesPending > 0; }
+    void timerFired();
     void stopAllTimers();
-    void sendAvatarIdentityPacket();
-    void sendAvatarBillboardPacket();
 
     QObject* setupTimerWithInterval(const QScriptValue& function, int intervalMS, bool isSingleShot);
     void stopTimer(QTimer* timer);
 
     AbstractControllerScriptingInterface* _controllerScriptingInterface;
     AvatarData* _avatarData;
-    QString _scriptName;
     QString _fileNameString;
     Quat _quatLibrary;
     Vec3 _vec3Library;
@@ -186,6 +183,39 @@ private:
     static QMutex _allScriptsMutex;
     static bool _stoppingAllScripts;
     static bool _doneRunningThisScript;
+
+private:
+    //FIXME- EntityTreeRender shouldn't be using these methods directly -- these methods need to be depricated from the public interfaces
+    friend class EntityTreeRenderer;
+
+    //FIXME - used in EntityTreeRenderer when evaluating entity scripts, which needs to be moved into ScriptEngine
+    //        also used in Agent, but that would be fixed if Agent was using proper apis for loading content
+    void setParentURL(const QString& parentURL) { _parentURL = parentURL; }
+
+private:
+    //FIXME- Agent shouldn't be using these methods directly -- these methods need to be depricated from the public interfaces
+    friend class Agent;
+
+    /// FIXME - DEPRICATED - remove callers and fix to use standard API
+    /// sets the script contents, will return false if failed, will fail if script is already running
+    bool setScriptContents(const QString& scriptContents, const QString& fileNameString = QString(""));
+
+    /// FIXME - these should only be used internally
+    void init();
+    void run();
+
+    // FIXME - all of these needto be removed and the code that depends on it in Agent.cpp should be moved into Agent.cpp
+    void setIsAgent(bool isAgent) { _isAgent = isAgent; }
+    void setIsAvatar(bool isAvatar);
+    bool isAvatar() const { return _isAvatar; }
+    void setAvatarData(AvatarData* avatarData, const QString& objectName);
+    bool isListeningToAudioStream() const { return _isListeningToAudioStream; }
+    void setIsListeningToAudioStream(bool isListeningToAudioStream) { _isListeningToAudioStream = isListeningToAudioStream; }
+    void setAvatarSound(Sound* avatarSound) { _avatarSound = avatarSound; }
+    bool isPlayingAvatarSound() const { return _avatarSound != NULL; }
+
+    void sendAvatarIdentityPacket();
+    void sendAvatarBillboardPacket();
 };
 
 #endif // hifi_ScriptEngine_h