Simpler and more uniform version that handles all the standard entity-script events.

libraries/entities-renderer/src/EntityTreeRenderer.cpp

@@ -874,22 +874,6 @@ void EntityTreeRenderer::connectSignalsToSlots(EntityScriptingInterface* entityS
     connect(this, &EntityTreeRenderer::enterEntity, entityScriptingInterface, &EntityScriptingInterface::enterEntity);
     connect(this, &EntityTreeRenderer::leaveEntity, entityScriptingInterface, &EntityScriptingInterface::leaveEntity);
     connect(this, &EntityTreeRenderer::collisionWithEntity, entityScriptingInterface, &EntityScriptingInterface::collisionWithEntity);
-
-    connect(entityScriptingInterface, &EntityScriptingInterface::addEntityEventHandler, this, &EntityTreeRenderer::addEntityEventHandler);
-    connect(entityScriptingInterface, &EntityScriptingInterface::removeEntityEventHandler, this, &EntityTreeRenderer::addEntityEventHandler);
-}
-
-void EntityTreeRenderer::addEntityEventHandler(EntityItemID entityID, QString entityEventName, QScriptValue handler) {
-    ScriptEngine* engine = static_cast<ScriptEngine*>(handler.engine());
-    if (engine) { // In case it's gone by the time we get the signal
-        engine->addEntityEventHandler(entityID, entityEventName, handler);
-    }
-}
-void EntityTreeRenderer::removeEntityEventHandler(EntityItemID entityID, QString entityEventName, QScriptValue handler) {
-    ScriptEngine* engine = static_cast<ScriptEngine*>(handler.engine());
-    if (engine) {
-        engine->removeEntityEventHandler(entityID, entityEventName, handler);
-    }
 }
 
 QScriptValueList EntityTreeRenderer::createMouseEventArgs(const EntityItemID& entityID, QMouseEvent* event, unsigned int deviceID) {

libraries/entities-renderer/src/EntityTreeRenderer.h

@@ -121,9 +121,6 @@ public slots:
     void setDisplayModelBounds(bool value) { _displayModelBounds = value; }
     void setDisplayModelElementProxy(bool value) { _displayModelElementProxy = value; }
     void setDontDoPrecisionPicking(bool value) { _dontDoPrecisionPicking = value; }
-    
-    void addEntityEventHandler(EntityItemID entityID, QString entityEventName, QScriptValue handler);
-    void removeEntityEventHandler(EntityItemID entityID, QString entityEventName, QScriptValue handler);
 
 protected:
     virtual Octree* createTree() { return new EntityTree(true); }

libraries/entities/src/EntityScriptingInterface.cpp

@@ -17,6 +17,7 @@
 #include "ModelEntityItem.h"
 #include "ZoneEntityItem.h"
 #include "EntitiesLogging.h"
+#include "../../script-engine/src/ScriptEngine.h" // FIXME
 
 
 EntityScriptingInterface::EntityScriptingInterface() :
@@ -202,10 +203,16 @@ void EntityScriptingInterface::dumpTree() const {
 }
 
 void EntityScriptingInterface::addEventHandler(EntityItemID entityID, QString entityEventName, QScriptValue handler) {
-    emit addEntityEventHandler(entityID, entityEventName, handler);
+    ScriptEngine* engine = static_cast<ScriptEngine*>(handler.engine());
+    if (engine) { // In case it's gone by the time we get the signal
+        engine->addEntityEventHandler(entityID, entityEventName, handler);
+    }
 }
 void EntityScriptingInterface::removeEventHandler(EntityItemID entityID, QString entityEventName, QScriptValue handler) {
-    emit removeEntityEventHandler(entityID, entityEventName, handler);
+    ScriptEngine* engine = static_cast<ScriptEngine*>(handler.engine());
+    if (engine) {
+        engine->removeEntityEventHandler(entityID, entityEventName, handler);
+    }
 }
 
 QVector<QUuid> EntityScriptingInterface::findEntities(const glm::vec3& center, float radius) const {

libraries/script-engine/src/ScriptEngine.cpp

@@ -389,30 +389,85 @@ void ScriptEngine::registerGetterSetter(const QString& name, QScriptEngine::Func
     }
 }
 
-void ScriptEngine::addEntityEventHandler(const EntityItemID& entityID, const QString& eventName, QScriptValue handler) {
+// Answer the previously registered handler for the given entityID/eventName, else an invalid QScriptValue.
-    auto entities = DependencyManager::get<EntityScriptingInterface>();
+QScriptValue ScriptEngine::getEntityEventHandler(const EntityItemID& entityID, const QString& eventName) {
-    if (!_registeredHandlers.contains(entityID)) {
+    if (!_registeredHandlers.contains(entityID)) return _illegal;
-        _registeredHandlers[entityID] = RegisteredEventHandlers();
+    const RegisteredEventHandlers& handlersOnEntity = _registeredHandlers[entityID];
-    }
+    if (!handlersOnEntity.contains(eventName)) return _illegal;
-    _registeredHandlers[entityID][eventName] = handler;
+    // FIXME: Need one more level of indirection. We need to allow multiple handlers per event, registered by different scripts.
-    if (eventName == "collisionWithEntity") {
+    return handlersOnEntity[eventName];
-        connect(entities.data(), &EntityScriptingInterface::collisionWithEntity, this, &ScriptEngine::collisionWithEntity);
-    }
-    // FIXME: deletingEntity, changingEntityID
 }
 void ScriptEngine::removeEntityEventHandler(const EntityItemID& entityID, const QString& eventName, QScriptValue handler) {
     // FIXME
 }
-void ScriptEngine::collisionWithEntity(const EntityItemID& idA, const EntityItemID& idB, const Collision& collision) {
+// FIXME: deletingEntity, changingEntityID
-    if (!_registeredHandlers.contains(idA)) return;
+void ScriptEngine::addEntityEventHandler(const EntityItemID& entityID, const QString& eventName, QScriptValue handler) {
-    const RegisteredEventHandlers& handlersOnEntity = _registeredHandlers[idA];
+    if (!_registeredHandlers.contains(entityID)) {
-    if (!handlersOnEntity.contains("collisionWithEntity")) return;
+        _registeredHandlers[entityID] = RegisteredEventHandlers();
-    // FIXME: Need one more level of indirection. We need to allow multiple handlers per event, registered by different scripts.
+    }
-    QScriptValue handlerForEvent = handlersOnEntity["collisionWithEntity"];
+    _registeredHandlers[entityID][eventName] = handler;
-    QScriptValueList args = QScriptValueList () << idA.toScriptValue(this) << idB.toScriptValue(this) << collisionToScriptValue(this, collision);
+    
-    handlerForEvent.call(QScriptValue(), args);
+    // The rest connects the Entities signal to the handler here.
+    auto entities = DependencyManager::get<EntityScriptingInterface>();
+    
+    // Given thunk that generates the arglist, evaluate that thunk only if needed, and call the handler.
+    auto generalHandler = [=](std::function<QScriptValueList()> argGenerator) {
+        QScriptValue handlerForEvent = getEntityEventHandler(entityID, eventName);
+        if (handlerForEvent.isValid()) {
+            QScriptValueList args = argGenerator();
+            handlerForEvent.call(QScriptValue(), args);
+        }
+    };
+    // Two common cases of event handler, differing only in argument signature.
+    auto singleEntityHandler = [=](const EntityItemID& entityItemID) {
+        generalHandler([=]() {
+            return QScriptValueList() << entityItemID.toScriptValue(this);
+        });
+    };
+    auto mouseHandler = [=](const EntityItemID& entityItemID, const MouseEvent& event) {
+        generalHandler([=]() {
+            return QScriptValueList() << entityItemID.toScriptValue(this) << event.toScriptValue(this);
+        });
+    };
+    // This string comparison maze only happens when adding a handler, which isn't often, rather than during events.
+    // Nonetheless, I wish it were more DRY. Variadic signals from strings? "I know reflection. I've worked with reflection. QT is no reflection."
+    if (eventName == "enterEntity") {
+        connect(entities.data(), &EntityScriptingInterface::enterEntity, this, singleEntityHandler);
+    } else if (eventName == "leaveEntity") {
+        connect(entities.data(), &EntityScriptingInterface::leaveEntity, this, singleEntityHandler);
+
+    } else if (eventName == "mousePressOnEntity") {
+        connect(entities.data(), &EntityScriptingInterface::mousePressOnEntity, this, mouseHandler);
+    } else if (eventName == "mouseMoveOnEntity") {
+        connect(entities.data(), &EntityScriptingInterface::mouseMoveOnEntity, this, mouseHandler);
+    } else if (eventName == "mouseReleaseOnEntity") {
+        connect(entities.data(), &EntityScriptingInterface::mouseReleaseOnEntity, this, mouseHandler);
+
+    } else if (eventName == "clickDownOnEntity") {
+        connect(entities.data(), &EntityScriptingInterface::clickDownOnEntity, this, mouseHandler);
+    } else if (eventName == "holdingClickOnEntity") {
+        connect(entities.data(), &EntityScriptingInterface::holdingClickOnEntity, this, mouseHandler);
+    } else if (eventName == "clickReleaseOnEntity") {
+        connect(entities.data(), &EntityScriptingInterface::clickReleaseOnEntity, this, mouseHandler);
+
+    } else if (eventName == "hoverEnterEntity") {
+        connect(entities.data(), &EntityScriptingInterface::hoverEnterEntity, this, mouseHandler);
+    } else if (eventName == "hoverOverEntity") {
+        connect(entities.data(), &EntityScriptingInterface::hoverOverEntity, this, mouseHandler);
+    } else if (eventName == "hoverLeaveEntity") {
+        connect(entities.data(), &EntityScriptingInterface::hoverLeaveEntity, this, mouseHandler);
+
+    } else if (eventName == "collisionWithEntity") {
+        connect(entities.data(), &EntityScriptingInterface::collisionWithEntity, this,
+                [=](const EntityItemID& idA, const EntityItemID& idB, const Collision& collision) {
+                    generalHandler([=]() {
+                        return QScriptValueList () << idA.toScriptValue(this) << idB.toScriptValue(this) << collisionToScriptValue(this, collision);
+                    });
+                });
+    }
 }
 
+
 void ScriptEngine::evaluate() {
     if (_stoppingAllScripts) {
         return; // bail early

libraries/script-engine/src/ScriptEngine.h

@@ -31,7 +31,7 @@
 #include "ScriptCache.h"
 #include "ScriptUUID.h"
 #include "Vec3.h"
-#include "EntityItemID.h"
+#include "../../entities/src/EntityItem.h" // FIXME
 
 const QString NO_SCRIPT("");
 
@@ -120,7 +120,6 @@ public slots:
     QUrl resolvePath(const QString& path) const;
 
     void nodeKilled(SharedNodePointer node);
-    void collisionWithEntity(const EntityItemID& idA, const EntityItemID& idB, const Collision& collision);
 
 signals:
     void scriptLoaded(const QString& scriptFilename);
@@ -172,6 +171,8 @@ private:
 
     QHash<QUuid, quint16> _outgoingScriptAudioSequenceNumbers;
     QHash<EntityItemID, RegisteredEventHandlers> _registeredHandlers;
+    QScriptValue _illegal;
+    QScriptValue getEntityEventHandler(const EntityItemID& entityID, const QString& eventName);
 
 private:
     static QSet<ScriptEngine*> _allKnownScriptEngines;