Fixes for code review

interface/src/Application.cpp

@@ -7593,9 +7593,6 @@ void Application::registerScriptEngineWithApplicationServices(const ScriptManage
     scriptEngine->registerGlobalObject("HifiAbout", AboutUtil::getInstance());  // Deprecated.
     scriptEngine->registerGlobalObject("ResourceRequestObserver", DependencyManager::get<ResourceRequestObserver>().data());
 
-    auto pickScriptingInterface = DependencyManager::get<PickScriptingInterface>();
-    pickScriptingInterface->registerProperties(scriptEngine.get());
-
     // connect this script engines printedMessage signal to the global ScriptEngines these various messages
     auto scriptEngines = DependencyManager::get<ScriptEngines>().data();
     connect(scriptManager.get(), &ScriptManager::printedMessage, scriptEngines, &ScriptEngines::onPrintedMessage);

interface/src/avatar/MyAvatar.cpp

@@ -46,6 +46,7 @@
 #include <PerfStat.h>
 #include <ScriptEngine.h>
 #include <ScriptEngineCast.h>
+#include <ScriptEngineLogging.h>
 #include <SharedUtil.h>
 #include <SoundCache.h>
 #include <ModelEntityItem.h>
@@ -122,7 +123,12 @@ STATIC_SCRIPT_TYPES_INITIALIZER(+[](ScriptManager* manager){
 STATIC_SCRIPT_INITIALIZER(+[](ScriptManager* manager){
     auto scriptEngine = manager->engine();
 
-    DependencyManager::get<AvatarManager>()->getMyAvatar()->registerProperties(scriptEngine);
+    auto avatarManager = DependencyManager::get<AvatarManager>();
+    if (avatarManager) {
+        avatarManager->getMyAvatar()->registerProperties(scriptEngine);
+    } else {
+        qWarning(scriptengine) << "Cannot register MyAvatar with script engine, AvatarManager instance not available";
+    }
 });
 
 const std::array<QString, static_cast<uint>(MyAvatar::AllowAvatarStandingPreference::Count)>

interface/src/raypick/PickScriptingInterface.cpp

@@ -31,6 +31,7 @@
 
 #include <ScriptEngine.h>
 #include <ScriptEngineCast.h>
+#include <ScriptEngineLogging.h>
 #include <ScriptValueUtils.h>
 
 STATIC_SCRIPT_TYPES_INITIALIZER(+[](ScriptManager* manager){
@@ -39,6 +40,17 @@ STATIC_SCRIPT_TYPES_INITIALIZER(+[](ScriptManager* manager){
     PickScriptingInterface::registerMetaTypes(scriptEngine);
 });
 
+STATIC_SCRIPT_INITIALIZER(+[](ScriptManager* manager){
+    auto scriptEngine = manager->engine().get();
+
+    auto pickScriptingInterface = DependencyManager::get<PickScriptingInterface>();
+    if (pickScriptingInterface) {
+        pickScriptingInterface->registerProperties(scriptEngine);
+    } else {
+        qWarning(scriptengine) << "Cannot register PickScriptingInterface properties with script engine, PickScriptingInterface instance not available";
+    }
+});
+
 static const float WEB_TOUCH_Y_OFFSET = 0.105f;  // how far forward (or back with a negative number) to slide stylus in hand
 static const glm::vec3 TIP_OFFSET = glm::vec3(0.0f, StylusPick::WEB_STYLUS_LENGTH - WEB_TOUCH_Y_OFFSET, 0.0f);
 

interface/src/scripting/PerformanceScriptingInterface.cpp

@@ -33,8 +33,6 @@ std::once_flag PerformanceScriptingInterface::registry_flag;
 PerformanceScriptingInterface::PerformanceScriptingInterface() {
     std::call_once(registry_flag, [] {
         qmlRegisterType<PerformanceScriptingInterface>("PerformanceEnums", 1, 0, "PerformanceEnums");
-        //qRegisterMetaType<PerformanceScriptingInterface::PerformancePreset>("PerformanceScriptingInterface::PerformancePreset");
-        //qRegisterMetaType<PerformanceScriptingInterface::RefreshRateProfile>("PerformanceScriptingInterface::RefreshRateProfile");
     });
 }
 

interface/src/scripting/RenderScriptingInterface.h

@@ -147,14 +147,14 @@ public slots:
     /*@jsdoc
      * Gets the active anti-aliasing mode.
      * @function Render.getAntialiasingMode
-     * @returns {AntialiasingMode} the active anti-aliasing mode.
+     * @returns {AntialiasingMode} The active anti-aliasing mode.
      */
     AntialiasingConfig::Mode getAntialiasingMode() const;
 
     /*@jsdoc
      * Sets the active anti-aliasing mode.
      * @function Render.setAntialiasingMode
-     * @param {AntialiasingMode} the active anti-aliasing mode.
+     * @param {AntialiasingMode} The active anti-aliasing mode.
      */
     void setAntialiasingMode(AntialiasingConfig::Mode mode);
 

libraries/entities-renderer/src/ModelScriptingInterface.cpp

@@ -19,6 +19,14 @@
 #include <ScriptValueUtils.h>
 #include <OBJWriter.h>
 
+STATIC_SCRIPT_TYPES_INITIALIZER((+[](ScriptManager* manager){
+    auto scriptEngine = manager->engine().get();
+
+    scriptRegisterSequenceMetaType<QList<MeshProxy*>>(scriptEngine);
+    scriptRegisterMetaType<MeshFace, meshFaceToScriptValue, meshFaceFromScriptValue>(scriptEngine);
+    scriptRegisterMetaType<QVector< MeshFace >, qVectorMeshFaceToScriptValue, qVectorMeshFaceFromScriptValue>(scriptEngine);
+}));
+
 STATIC_SCRIPT_INITIALIZER(+[](ScriptManager* manager) {
     auto scriptEngine = manager->engine();
 
@@ -26,12 +34,6 @@ STATIC_SCRIPT_INITIALIZER(+[](ScriptManager* manager) {
 });
 
 ModelScriptingInterface::ModelScriptingInterface(QObject* parent) : QObject(parent) {
-    _modelScriptEngine = qobject_cast<ScriptManager*>(parent)->engine();
-    Q_ASSERT(_modelScriptEngine != nullptr);
-
-    scriptRegisterSequenceMetaType<QList<MeshProxy*>>(_modelScriptEngine.get());
-    scriptRegisterMetaType<MeshFace, meshFaceToScriptValue, meshFaceFromScriptValue>(_modelScriptEngine.get());
-    scriptRegisterMetaType<QVector< MeshFace >, qVectorMeshFaceToScriptValue, qVectorMeshFaceFromScriptValue>(_modelScriptEngine.get());
 }
 
 QString ModelScriptingInterface::meshToOBJ(MeshProxyList in) {

libraries/script-engine/src/EventTypes.cpp

@@ -17,15 +17,18 @@
 #include "PointerEvent.h"
 #include "ScriptEngine.h"
 #include "ScriptEngineCast.h"
+#include "ScriptManager.h"
 #include "SpatialEvent.h"
 #include "TouchEvent.h"
 #include "WheelEvent.h"
 
-void registerEventTypes(ScriptEngine* engine) {
+STATIC_SCRIPT_TYPES_INITIALIZER((+[](ScriptManager* manager){
-    scriptRegisterMetaType<KeyEvent, KeyEvent::toScriptValue, KeyEvent::fromScriptValue>(engine, "KeyEvent");
+    auto scriptEngine = manager->engine().get();
-    scriptRegisterMetaType<MouseEvent, MouseEvent::toScriptValue, MouseEvent::fromScriptValue>(engine, "MouseEvent");
+    
-    scriptRegisterMetaType<PointerEvent, PointerEvent::toScriptValue, PointerEvent::fromScriptValue>(engine, "PointerEvent");
+    scriptRegisterMetaType<KeyEvent, KeyEvent::toScriptValue, KeyEvent::fromScriptValue>(scriptEngine, "KeyEvent");
-    scriptRegisterMetaType<TouchEvent, TouchEvent::toScriptValue, TouchEvent::fromScriptValue>(engine, "TouchEvent");
+    scriptRegisterMetaType<MouseEvent, MouseEvent::toScriptValue, MouseEvent::fromScriptValue>(scriptEngine, "MouseEvent");
-    scriptRegisterMetaType<WheelEvent, WheelEvent::toScriptValue, WheelEvent::fromScriptValue>(engine, "WheelEvent");
+    scriptRegisterMetaType<PointerEvent, PointerEvent::toScriptValue, PointerEvent::fromScriptValue>(scriptEngine, "PointerEvent");
-    scriptRegisterMetaType<SpatialEvent, SpatialEvent::toScriptValue, SpatialEvent::fromScriptValue>(engine, "SpatialEvent");
+    scriptRegisterMetaType<TouchEvent, TouchEvent::toScriptValue, TouchEvent::fromScriptValue>(scriptEngine, "TouchEvent");
-}
+    scriptRegisterMetaType<WheelEvent, WheelEvent::toScriptValue, WheelEvent::fromScriptValue>(scriptEngine, "WheelEvent");
+    scriptRegisterMetaType<SpatialEvent, SpatialEvent::toScriptValue, SpatialEvent::fromScriptValue>(scriptEngine, "SpatialEvent");
+}));

libraries/script-engine/src/MIDIEvent.cpp

@@ -17,10 +17,13 @@
 #include "ScriptEngine.h"
 #include "ScriptEngineCast.h"
 #include "ScriptValue.h"
+#include "ScriptManager.h"
 
-void registerMIDIMetaTypes(ScriptEngine* engine) {
+STATIC_SCRIPT_TYPES_INITIALIZER((+[](ScriptManager* manager){
-    scriptRegisterMetaType<MIDIEvent, midiEventToScriptValue, midiEventFromScriptValue>(engine, "MIDIEvent");
+    auto scriptEngine = manager->engine().get();
-}
+
+    scriptRegisterMetaType<MIDIEvent, midiEventToScriptValue, midiEventFromScriptValue>(scriptEngine, "MIDIEvent");
+}));
 
 const QString MIDI_DELTA_TIME_PROP_NAME = "deltaTime";
 const QString MIDI_EVENT_TYPE_PROP_NAME = "type";

libraries/script-engine/src/MenuItemProperties.cpp

@@ -18,6 +18,12 @@
 #include "ScriptEngine.h"
 #include "ScriptEngineCast.h"
 #include "ScriptValue.h"
+#include "ScriptManager.h"
+
+STATIC_SCRIPT_TYPES_INITIALIZER((+[](ScriptManager* manager){
+    auto scriptEngine = manager->engine().get();
+    scriptRegisterMetaType<MenuItemProperties, menuItemPropertiesToScriptValue, menuItemPropertiesFromScriptValue>(scriptEngine, "MenuItemProperties");
+}));
 
 MenuItemProperties::MenuItemProperties(const QString& menuName, const QString& menuItemName,
                         const QString& shortcutKey, bool checkable, bool checked, bool separator) :
@@ -44,10 +50,6 @@ MenuItemProperties::MenuItemProperties(const QString& menuName, const QString& m
 {
 }
 
-void registerMenuItemProperties(ScriptEngine* engine) {
-    scriptRegisterMetaType<MenuItemProperties, menuItemPropertiesToScriptValue, menuItemPropertiesFromScriptValue>(engine, "MenuItemProperties");
-}
-
 ScriptValue menuItemPropertiesToScriptValue(ScriptEngine* engine, const MenuItemProperties& properties) {
     ScriptValue obj = engine->newObject();
     // not supported

libraries/script-engine/src/MenuItemProperties.h

@@ -56,7 +56,6 @@ private:
 Q_DECLARE_METATYPE(MenuItemProperties)
 ScriptValue menuItemPropertiesToScriptValue(ScriptEngine* engine, const MenuItemProperties& props);
 bool menuItemPropertiesFromScriptValue(const ScriptValue& object, MenuItemProperties& props);
-void registerMenuItemProperties(ScriptEngine* engine);
 
 
 

libraries/script-engine/src/ScriptEngineCast.h

@@ -115,24 +115,6 @@ int scriptRegisterMetaTypeWithLambdas(ScriptEngine* eng,
     return id;
 }
 
-/*
-template <typename T>
-int scriptRegisterMetaTypeWithLambdas(ScriptEngine* eng,
-                           ScriptValue (*toScriptValue)(ScriptEngine*, const T& t),
-                           bool (*fromScriptValue)(const ScriptValue&, T& t), const char* name = "",
-                           T* = 0)
-{
-    int id;
-    if (strlen(name) > 0) { // make sure it's registered
-        id = qRegisterMetaType<T>(name);
-    } else {
-        id = qRegisterMetaType<T>();
-    }
-    eng->registerCustomType(id, reinterpret_cast<ScriptEngine::MarshalFunction>(toScriptValue),
-                            reinterpret_cast<ScriptEngine::DemarshalFunction>(fromScriptValue));
-    return id;
-}*/
-
 template <class Container>
 ScriptValue scriptValueFromSequence(ScriptEngine* eng, const Container& cont) {
     ScriptValue a = eng->newArray();

libraries/script-engine/src/ScriptManager.cpp

@@ -91,6 +91,21 @@ Q_DECLARE_METATYPE(ScriptValue);
 static ScriptManager::StaticInitializerNode* rootInitializer = nullptr;
 static ScriptManager::StaticTypesInitializerNode* rootTypesInitializer = nullptr;
 
+
+using ScriptableResourceRawPtr = ScriptableResource*;
+ScriptValue externalResourceBucketToScriptValue(ScriptEngine* engine, ExternalResource::Bucket const& in);
+bool externalResourceBucketFromScriptValue(const ScriptValue& object, ExternalResource::Bucket& out);
+static ScriptValue scriptableResourceToScriptValue(ScriptEngine* engine,
+                                                    const ScriptableResourceRawPtr& resource);
+static bool scriptableResourceFromScriptValue(const ScriptValue& value, ScriptableResourceRawPtr& resource);
+STATIC_SCRIPT_TYPES_INITIALIZER((+[](ScriptManager* manager){
+    auto scriptEngine = manager->engine().get();
+
+    scriptRegisterMetaType<ExternalResource::Bucket, externalResourceBucketToScriptValue, externalResourceBucketFromScriptValue>(scriptEngine);
+
+    scriptRegisterMetaType<ScriptableResourceRawPtr, scriptableResourceToScriptValue, scriptableResourceFromScriptValue>(scriptEngine);
+}));
+
 void ScriptManager::registerNewStaticInitializer(StaticInitializerNode* dest) {
     // this function is assumed to be called on LoadLibrary, where we are explicitly operating in single-threaded mode
     // Therefore there is no mutex or threadsafety here and the structure is assumed not to change after loading
@@ -634,30 +649,7 @@ void ScriptManager::initMetaTypes() {
         return;
     }
     _areMetaTypesInitialized = true;
-    auto scriptEngine = _engine.get();
     runStaticTypesInitializers(this);
-    registerMIDIMetaTypes(scriptEngine);
-    registerEventTypes(scriptEngine);
-    registerMenuItemProperties(scriptEngine);
-
-    scriptRegisterSequenceMetaType<QVector<QUuid>>(scriptEngine);
-    scriptRegisterSequenceMetaType<QVector<EntityItemID>>(scriptEngine);
-
-    scriptRegisterSequenceMetaType<QVector<glm::vec2>>(scriptEngine);
-    scriptRegisterSequenceMetaType<QVector<glm::quat>>(scriptEngine);
-    scriptRegisterSequenceMetaType<QVector<QString>>(scriptEngine);
-    
-    scriptRegisterMetaType<AnimationDetails, animationDetailsToScriptValue, animationDetailsFromScriptValue>(scriptEngine);
-    scriptRegisterMetaType<WebSocketClass*, webSocketToScriptValue, webSocketFromScriptValue>(scriptEngine);
-    scriptRegisterMetaType<QWebSocketProtocol::CloseCode, qWSCloseCodeToScriptValue, qWSCloseCodeFromScriptValue>(scriptEngine);
-    scriptRegisterMetaType<WebSocketClass::ReadyState, wscReadyStateToScriptValue, wscReadyStateFromScriptValue>(scriptEngine);
-
-    scriptRegisterMetaType<ExternalResource::Bucket, externalResourceBucketToScriptValue, externalResourceBucketFromScriptValue>(scriptEngine);
-
-    scriptRegisterMetaType<ScriptableResourceRawPtr, scriptableResourceToScriptValue, scriptableResourceFromScriptValue>(scriptEngine);
-
-    scriptRegisterMetaType<MeshProxy*, meshToScriptValue, meshFromScriptValue>(scriptEngine);
-    scriptRegisterMetaType<MeshProxyList, meshesToScriptValue, meshesFromScriptValue>(scriptEngine);
 }
 
 void ScriptManager::init() {

libraries/script-engine/src/ScriptValueUtils.cpp

@@ -29,6 +29,7 @@
 #include "ScriptEngine.h"
 #include "ScriptEngineCast.h"
 #include "ScriptValueIterator.h"
+#include "ScriptEngineLogging.h"
 
 bool isListOfStrings(const ScriptValue& arg) {
     if (!arg.isArray()) {
@@ -78,9 +79,18 @@ void registerMetaTypes(ScriptEngine* engine) {
 
     scriptRegisterMetaType<StencilMaskMode, stencilMaskModeToScriptValue, stencilMaskModeFromScriptValue>(engine);
 
-    scriptRegisterMetaType<std::shared_ptr< MiniPromise >, promiseToScriptValue, promiseFromScriptValue>(engine);
+    scriptRegisterMetaType<AnimationDetails, animationDetailsToScriptValue, animationDetailsFromScriptValue>(engine);
+
+    scriptRegisterMetaType<MeshProxy*, meshToScriptValue, meshFromScriptValue>(engine);
+    scriptRegisterMetaType<MeshProxyList, meshesToScriptValue, meshesFromScriptValue>(engine);
 
     scriptRegisterSequenceMetaType<QVector<unsigned int> >(engine);
+    scriptRegisterSequenceMetaType<QVector<QUuid>>(engine);
+    scriptRegisterSequenceMetaType<QVector<EntityItemID>>(engine);
+
+    scriptRegisterSequenceMetaType<QVector<glm::vec2>>(engine);
+    scriptRegisterSequenceMetaType<QVector<glm::quat>>(engine);
+    scriptRegisterSequenceMetaType<QVector<QString>>(engine);
 }
 
 ScriptValue vec2ToScriptValue(ScriptEngine* engine, const glm::vec2& vec2) {
@@ -858,7 +868,7 @@ ScriptValue meshesToScriptValue(ScriptEngine* engine, const MeshProxyList& in) {
 bool meshesFromScriptValue(const ScriptValue& value, MeshProxyList& out) {
     ScriptValueIteratorPointer itr(value.newIterator());
 
-    qDebug() << "in meshesFromScriptValue, value.length =" << value.property("length").toInt32();
+    qDebug(scriptengine) << "in meshesFromScriptValue, value.length =" << value.property("length").toInt32();
 
     while (itr->hasNext()) {
         itr->next();
@@ -866,7 +876,7 @@ bool meshesFromScriptValue(const ScriptValue& value, MeshProxyList& out) {
         if (meshProxy) {
             out.append(meshProxy);
         } else {
-            qDebug() << "null meshProxy";
+            qDebug(scriptengine) << "null meshProxy";
         }
     }
     return true;

libraries/script-engine/src/WebSocketClass.cpp

@@ -19,6 +19,15 @@
 #include "ScriptEngineCast.h"
 #include "ScriptEngineLogging.h"
 #include "ScriptValue.h"
+#include "ScriptManager.h"
+
+STATIC_SCRIPT_TYPES_INITIALIZER((+[](ScriptManager* manager){
+    auto scriptEngine = manager->engine().get();
+
+    scriptRegisterMetaType<WebSocketClass*, webSocketToScriptValue, webSocketFromScriptValue>(scriptEngine);
+    scriptRegisterMetaType<QWebSocketProtocol::CloseCode, qWSCloseCodeToScriptValue, qWSCloseCodeFromScriptValue>(scriptEngine);
+    scriptRegisterMetaType<WebSocketClass::ReadyState, wscReadyStateToScriptValue, wscReadyStateFromScriptValue>(scriptEngine);
+}));
 
 WebSocketClass::WebSocketClass(ScriptEngine* engine, QString url) :
     _webSocket(new QWebSocket()),

libraries/script-engine/src/qtscript/ScriptEngineQtScript_cast.cpp

@@ -199,6 +199,7 @@ int ScriptEngineQtScript::computeCastPenalty(QScriptValue& val, int destTypeId)
     if (val.isNumber()) {
         switch (destTypeId){
             case QMetaType::Bool:
+                // Conversion to bool is acceptable, but numbers are preferred
                 return 5;
                 break;
             case QMetaType::UInt:
@@ -211,20 +212,24 @@ int ScriptEngineQtScript::computeCastPenalty(QScriptValue& val, int destTypeId)
             case QMetaType::ULongLong:
             case QMetaType::LongLong:
             case QMetaType::UShort:
+                // Perfect case. JS doesn't have separate integer and floating point type
                 return 0;
                 break;
             case QMetaType::QString:
             case QMetaType::QByteArray:
             case QMetaType::QDateTime:
             case QMetaType::QDate:
+                // Conversion to string should be avoided, it's probably not what we want
                 return 100;
                 break;
             default:
+                // Other, not predicted cases
                 return 5;
         }
     } else if (val.isString() || val.isDate() || val.isRegExp()) {
         switch (destTypeId){
             case QMetaType::Bool:
+                // Conversion from to bool should be avoided if possible, it's probably not what we want
                 return 100;
             case QMetaType::UInt:
             case QMetaType::ULong:
@@ -236,12 +241,15 @@ int ScriptEngineQtScript::computeCastPenalty(QScriptValue& val, int destTypeId)
             case QMetaType::ULongLong:
             case QMetaType::LongLong:
             case QMetaType::UShort:
+                // Conversion from to number should be avoided if possible, it's probably not what we want
                 return 100;
             case QMetaType::QString:
+                // Perfect case
                 return 0;
             case QMetaType::QByteArray:
             case QMetaType::QDateTime:
             case QMetaType::QDate:
+                // String to string should be slightly preferred
                 return 5;
             default:
                 return 5;
@@ -249,6 +257,7 @@ int ScriptEngineQtScript::computeCastPenalty(QScriptValue& val, int destTypeId)
     } else if (val.isBool() || val.isBoolean()) {
         switch (destTypeId){
             case QMetaType::Bool:
+                // Perfect case
                 return 0;
                 break;
             case QMetaType::UInt:
@@ -261,12 +270,14 @@ int ScriptEngineQtScript::computeCastPenalty(QScriptValue& val, int destTypeId)
             case QMetaType::ULongLong:
             case QMetaType::LongLong:
             case QMetaType::UShort:
+                // Using function with bool parameter should be preferred over converted bool to nimber
                 return 5;
                 break;
             case QMetaType::QString:
             case QMetaType::QByteArray:
             case QMetaType::QDateTime:
             case QMetaType::QDate:
+                // Bool probably shouldn't be converted to string if there are better alternatives
                 return 50;
                 break;
             default:
@@ -292,13 +303,6 @@ bool ScriptEngineQtScript::castValueToVariant(const QScriptValue& val, QVariant&
             }
         }
     }
-/*    if (QMetaType(destTypeId).name() == "MenuItemProperties") {
-        qDebug() << "castValueToVariant MenuItemProperties " << destTypeId << "map size: " << _customTypes.size();
-        for (auto iter = _customTypes.keyBegin(); iter != _customTypes.keyEnd(); iter++){
-            qDebug() << (*iter);
-        }
-        printf("castValueToVariant MenuItemProperties");
-    }*/
 
     if (destTypeId == qMetaTypeId<ScriptValue>()) {
         dest = QVariant::fromValue(ScriptValue(new ScriptValueQtWrapper(this, val)));

libraries/script-engine/src/qtscript/ScriptObjectQtProxy.cpp

@@ -17,6 +17,8 @@
 
 #include <QtScript/QScriptContext>
 
+#include "../ScriptEngineLogging.h"
+
 #include "ScriptContextQtWrapper.h"
 #include "ScriptValueQtWrapper.h"
 
@@ -190,7 +192,7 @@ void ScriptObjectQtProxy::investigate() {
                 }
                 break;
             default:
-                ;
+                break;
         }
 
         QScriptString name = _engine->toStringHandle(QString::fromLatin1(szName));
@@ -214,11 +216,11 @@ void ScriptObjectQtProxy::investigate() {
         } else {
             int parameterCount = method.parameterCount();
             if(method.returnType() == QMetaType::UnknownType) {
-                qCritical() << "Method " << metaObject->className() << "::" << name << " has QMetaType::UnknownType return value";
+                qCritical(scriptengine) << "Method " << metaObject->className() << "::" << name << " has QMetaType::UnknownType return value";
             }
             for (int i = 0; i < method.parameterCount(); i++) {
                 if (method.parameterType(i) == QMetaType::UnknownType) {
-                    qCritical() << "Parameter " << i << "in method " << metaObject->className() << "::" << name << " is of type QMetaType::UnknownType";
+                    qCritical(scriptengine) << "Parameter " << i << "in method " << metaObject->className() << "::" << name << " is of type QMetaType::UnknownType";
                 }
             }
             if (nameLookup == methodNames.end()) {
@@ -331,10 +333,8 @@ QScriptValue ScriptObjectQtProxy::property(const QScriptValue& object, const QSc
             if (lookup == _methods.cend()) return QScriptValue();
             const MethodDef& methodDef = lookup.value();
             for (auto iter = methodDef.methods.begin(); iter != methodDef.methods.end(); iter++ ) {
-                //qDebug() << (*iter).returnType();
                 if((*iter).returnType() == QMetaType::UnknownType) {
-                    qDebug() << "Method with QMetaType::UnknownType " << metaObject->className() << " " << (*iter).name();
+                    qDebug(scriptengine) << "Method with QMetaType::UnknownType " << metaObject->className() << " " << (*iter).name();
-                    printf("Method with QMetaType::UnknownType");
                 }
             }
             return static_cast<QScriptEngine*>(_engine)->newObject(
@@ -466,25 +466,22 @@ QVariant ScriptMethodQtProxy::extension(Extension extension, const QVariant& arg
     QVector< QList<ScriptValue> > qScriptArgLists;
     QVector< QVector <QGenericArgument> > qGenArgsVectors;
     QVector< QList<QVariant> > qVarArgLists;
-    QVector<int> conversionPenaltyScore;
-    QVector<bool> isMetaRejected;
     qScriptArgLists.resize(num_metas);
     qGenArgsVectors.resize(num_metas);
     qVarArgLists.resize(num_metas);
-    conversionPenaltyScore.resize(num_metas);
+    bool isValidMetaSelected = false;
-    isMetaRejected.resize(num_metas);
+    int bestMeta = 0;
+    int bestConversionPenaltyScore = 0;
 
     for (int i = 0; i < num_metas; i++) {
         const QMetaMethod& meta = _metas[i];
-        isMetaRejected[i] = false;
         int methodNumArgs = meta.parameterCount();
         if (methodNumArgs != numArgs) {
-            isMetaRejected[i] = true;
             continue;
         }
 
         qGenArgsVectors[i].resize(10);
-        conversionPenaltyScore[i] = 0;
+        int conversionPenaltyScore = 0;
         int conversionFailures = 0;
 
         for (int arg = 0; arg < numArgs; ++arg) {
@@ -504,7 +501,7 @@ QVariant ScriptMethodQtProxy::extension(Extension extension, const QVariant& arg
                 } else {
                     qVarArgLists[i].append(varArgVal);
                     const QVariant& converted = qVarArgLists[i].back();
-                    conversionPenaltyScore[i] = _engine->computeCastPenalty(argVal, methodArgTypeId);
+                    conversionPenaltyScore = _engine->computeCastPenalty(argVal, methodArgTypeId);
 
                     // a lot of type conversion assistance thanks to https://stackoverflow.com/questions/28457819/qt-invoke-method-with-qvariant
                     // A const_cast is needed because calling data() would detach the QVariant.
@@ -518,27 +515,23 @@ QVariant ScriptMethodQtProxy::extension(Extension extension, const QVariant& arg
                 parameterConversionFailureCount = conversionFailures;
                 parameterConversionFailureId = meta.methodIndex();
             }
-            isMetaRejected[i] = true;
             continue;
         }
 
-        ScriptContextQtWrapper ourContext(_engine, context);
+        if (!isValidMetaSelected) {
-        ScriptContextGuard guard(&ourContext);
-    }
-
-    bool isValidMetaSelected = false;
-    int bestMeta = 0;
-    for (int i = 0; i < num_metas; i++) {
-        if (!isValidMetaSelected && !isMetaRejected[i]) {
             isValidMetaSelected = true;
             bestMeta = i;
+            bestConversionPenaltyScore = conversionPenaltyScore;
         }
-        if (isValidMetaSelected && !isMetaRejected[i] && conversionPenaltyScore[bestMeta] > conversionPenaltyScore[i]) {
+        if (isValidMetaSelected && bestConversionPenaltyScore > conversionPenaltyScore) {
             bestMeta = i;
+            bestConversionPenaltyScore = conversionPenaltyScore;
         }
     }
 
     if (isValidMetaSelected) {
+        ScriptContextQtWrapper ourContext(_engine, context);
+        ScriptContextGuard guard(&ourContext);
         const QMetaMethod& meta = _metas[bestMeta];
         int returnTypeId = meta.returnType();
         QVector <QGenericArgument> &qGenArgs = qGenArgsVectors[bestMeta];
@@ -649,7 +642,7 @@ int ScriptSignalQtProxy::qt_metacall(QMetaObject::Call call, int id, void** argu
 
     for (ConnectionList::iterator iter = connections.begin(); iter != connections.end(); ++iter) {
         Connection& conn = *iter;
-            conn.callback.call(conn.thisValue, args);
+        conn.callback.call(conn.thisValue, args);
     }
 
     return -1;
@@ -662,6 +655,15 @@ int ScriptSignalQtProxy::discoverMetaCallIdx() {
 
 ScriptSignalQtProxy::ConnectionList::iterator ScriptSignalQtProxy::findConnection(QScriptValue thisObject, QScriptValue callback) {
     ConnectionList::iterator iter;
+/*    resultWithReadLock<ScriptSignalQtProxy::ConnectionList::iterator>([&]{
+        for (iter = _connections.begin(); iter != _connections.end(); ++iter) {
+            Connection& conn = *iter;
+            if (conn.callback.strictlyEquals(callback) && conn.thisValue.strictlyEquals(thisObject)) {
+                break;
+            }
+        }
+        return iter;
+    });*/
     withReadLock([&]{
         for (iter = _connections.begin(); iter != _connections.end(); ++iter) {
             Connection& conn = *iter;
@@ -698,9 +700,11 @@ void ScriptSignalQtProxy::connect(QScriptValue arg0, QScriptValue arg1) {
     }
 
     // are we already connected?
-    ConnectionList::iterator lookup = findConnection(callbackThis, callback);
+    {
-    if (lookup != _connections.end()) {
+        ConnectionList::iterator lookup = findConnection(callbackThis, callback);
-        return; // already exists
+        if (lookup != _connections.end()) {
+            return; // already exists
+        }
     }
 
     // add a reference to ourselves to the destination callback
@@ -757,15 +761,17 @@ void ScriptSignalQtProxy::disconnect(QScriptValue arg0, QScriptValue arg1) {
     }
 
     // locate this connection in our list of connections
-    ConnectionList::iterator lookup = findConnection(callbackThis, callback);
+    {
-    if (lookup == _connections.end()) {
+        ConnectionList::iterator lookup = findConnection(callbackThis, callback);
-        return;  // not here
+        if (lookup == _connections.end()) {
-    }
+            return;  // not here
+        }
 
-    // remove it from our internal list of connections
+        // remove it from our internal list of connections
-    withWriteLock([&]{
+        withWriteLock([&]{
-        _connections.erase(lookup);
+            _connections.erase(lookup);
-    });
+        });
+    }
 
     // remove a reference to ourselves from the destination callback
     QScriptValue destData = callback.data();