Fixes for code review

ksuprynowicz committed 4 years ago

Commit 586bd7f1 parent e9bfdd416 files · +142 −117Browse files

Showing the changes to libraries/script-engine/src/qtscript/ScriptObjectQtProxy.cpp (1 of 16 files). Show all

Unified · Split

libraries/script-engine/src/qtscript/ScriptObjectQtProxy.cpp+42−36

 #include <QtScript/QScriptContext>
+#include "../ScriptEngineLogging.h"
 #include "ScriptContextQtWrapper.h"
 #include "ScriptValueQtWrapper.h"
                 }
                 break;
             default:
-                ;
+                break;
         }
         QScriptString name = _engine->toStringHandle(QString::fromLatin1(szName));
         } 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()) {
             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(
     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) {
                 } 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.
                 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];
     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;
 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;
     }
     // 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
     }
     // 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();