From 5a4cf0613476f692816ec5bbcb4eb4c54f0d05f8 Mon Sep 17 00:00:00 2001 From: ksuprynowicz Date: Sun, 28 Aug 2022 18:23:45 +0200 Subject: [PATCH] Fixes for code review --- interface/src/Application.cpp | 3 - interface/src/avatar/MyAvatar.cpp | 8 +- .../src/raypick/PickScriptingInterface.cpp | 12 +++ .../PerformanceScriptingInterface.cpp | 2 - .../src/scripting/RenderScriptingInterface.h | 4 +- .../src/ModelScriptingInterface.cpp | 14 ++-- libraries/script-engine/src/EventTypes.cpp | 19 +++-- libraries/script-engine/src/MIDIEvent.cpp | 9 ++- .../script-engine/src/MenuItemProperties.cpp | 10 ++- .../script-engine/src/MenuItemProperties.h | 1 - .../script-engine/src/ScriptEngineCast.h | 18 ----- libraries/script-engine/src/ScriptManager.cpp | 38 ++++----- .../script-engine/src/ScriptValueUtils.cpp | 16 +++- .../script-engine/src/WebSocketClass.cpp | 9 +++ .../qtscript/ScriptEngineQtScript_cast.cpp | 18 +++-- .../src/qtscript/ScriptObjectQtProxy.cpp | 78 ++++++++++--------- 16 files changed, 142 insertions(+), 117 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 73fdae54b7..ca0a7fb6b9 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -7593,9 +7593,6 @@ void Application::registerScriptEngineWithApplicationServices(const ScriptManage scriptEngine->registerGlobalObject("HifiAbout", AboutUtil::getInstance()); // Deprecated. scriptEngine->registerGlobalObject("ResourceRequestObserver", DependencyManager::get().data()); - auto pickScriptingInterface = DependencyManager::get(); - pickScriptingInterface->registerProperties(scriptEngine.get()); - // connect this script engines printedMessage signal to the global ScriptEngines these various messages auto scriptEngines = DependencyManager::get().data(); connect(scriptManager.get(), &ScriptManager::printedMessage, scriptEngines, &ScriptEngines::onPrintedMessage); diff --git a/interface/src/avatar/MyAvatar.cpp b/interface/src/avatar/MyAvatar.cpp index a29246650f..9fada64917 100644 --- a/interface/src/avatar/MyAvatar.cpp +++ b/interface/src/avatar/MyAvatar.cpp @@ -46,6 +46,7 @@ #include #include #include +#include #include #include #include @@ -122,7 +123,12 @@ STATIC_SCRIPT_TYPES_INITIALIZER(+[](ScriptManager* manager){ STATIC_SCRIPT_INITIALIZER(+[](ScriptManager* manager){ auto scriptEngine = manager->engine(); - DependencyManager::get()->getMyAvatar()->registerProperties(scriptEngine); + auto avatarManager = DependencyManager::get(); + if (avatarManager) { + avatarManager->getMyAvatar()->registerProperties(scriptEngine); + } else { + qWarning(scriptengine) << "Cannot register MyAvatar with script engine, AvatarManager instance not available"; + } }); const std::array(MyAvatar::AllowAvatarStandingPreference::Count)> diff --git a/interface/src/raypick/PickScriptingInterface.cpp b/interface/src/raypick/PickScriptingInterface.cpp index 1365ef0aa2..c055e368e6 100644 --- a/interface/src/raypick/PickScriptingInterface.cpp +++ b/interface/src/raypick/PickScriptingInterface.cpp @@ -31,6 +31,7 @@ #include #include +#include #include 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(); + 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); diff --git a/interface/src/scripting/PerformanceScriptingInterface.cpp b/interface/src/scripting/PerformanceScriptingInterface.cpp index 82d2606401..e63bc83358 100644 --- a/interface/src/scripting/PerformanceScriptingInterface.cpp +++ b/interface/src/scripting/PerformanceScriptingInterface.cpp @@ -33,8 +33,6 @@ std::once_flag PerformanceScriptingInterface::registry_flag; PerformanceScriptingInterface::PerformanceScriptingInterface() { std::call_once(registry_flag, [] { qmlRegisterType("PerformanceEnums", 1, 0, "PerformanceEnums"); - //qRegisterMetaType("PerformanceScriptingInterface::PerformancePreset"); - //qRegisterMetaType("PerformanceScriptingInterface::RefreshRateProfile"); }); } diff --git a/interface/src/scripting/RenderScriptingInterface.h b/interface/src/scripting/RenderScriptingInterface.h index ff9ef77a90..2fc5aff18e 100644 --- a/interface/src/scripting/RenderScriptingInterface.h +++ b/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); diff --git a/libraries/entities-renderer/src/ModelScriptingInterface.cpp b/libraries/entities-renderer/src/ModelScriptingInterface.cpp index 5ec951378e..0d767d678e 100644 --- a/libraries/entities-renderer/src/ModelScriptingInterface.cpp +++ b/libraries/entities-renderer/src/ModelScriptingInterface.cpp @@ -19,6 +19,14 @@ #include #include +STATIC_SCRIPT_TYPES_INITIALIZER((+[](ScriptManager* manager){ + auto scriptEngine = manager->engine().get(); + + scriptRegisterSequenceMetaType>(scriptEngine); + scriptRegisterMetaType(scriptEngine); + scriptRegisterMetaType, 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(parent)->engine(); - Q_ASSERT(_modelScriptEngine != nullptr); - - scriptRegisterSequenceMetaType>(_modelScriptEngine.get()); - scriptRegisterMetaType(_modelScriptEngine.get()); - scriptRegisterMetaType, qVectorMeshFaceToScriptValue, qVectorMeshFaceFromScriptValue>(_modelScriptEngine.get()); } QString ModelScriptingInterface::meshToOBJ(MeshProxyList in) { diff --git a/libraries/script-engine/src/EventTypes.cpp b/libraries/script-engine/src/EventTypes.cpp index 4990067f2a..8e2f9364b8 100644 --- a/libraries/script-engine/src/EventTypes.cpp +++ b/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) { - scriptRegisterMetaType(engine, "KeyEvent"); - scriptRegisterMetaType(engine, "MouseEvent"); - scriptRegisterMetaType(engine, "PointerEvent"); - scriptRegisterMetaType(engine, "TouchEvent"); - scriptRegisterMetaType(engine, "WheelEvent"); - scriptRegisterMetaType(engine, "SpatialEvent"); -} +STATIC_SCRIPT_TYPES_INITIALIZER((+[](ScriptManager* manager){ + auto scriptEngine = manager->engine().get(); + + scriptRegisterMetaType(scriptEngine, "KeyEvent"); + scriptRegisterMetaType(scriptEngine, "MouseEvent"); + scriptRegisterMetaType(scriptEngine, "PointerEvent"); + scriptRegisterMetaType(scriptEngine, "TouchEvent"); + scriptRegisterMetaType(scriptEngine, "WheelEvent"); + scriptRegisterMetaType(scriptEngine, "SpatialEvent"); +})); diff --git a/libraries/script-engine/src/MIDIEvent.cpp b/libraries/script-engine/src/MIDIEvent.cpp index f7a6ec3382..1529881d42 100644 --- a/libraries/script-engine/src/MIDIEvent.cpp +++ b/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) { - scriptRegisterMetaType(engine, "MIDIEvent"); -} +STATIC_SCRIPT_TYPES_INITIALIZER((+[](ScriptManager* manager){ + auto scriptEngine = manager->engine().get(); + + scriptRegisterMetaType(scriptEngine, "MIDIEvent"); +})); const QString MIDI_DELTA_TIME_PROP_NAME = "deltaTime"; const QString MIDI_EVENT_TYPE_PROP_NAME = "type"; diff --git a/libraries/script-engine/src/MenuItemProperties.cpp b/libraries/script-engine/src/MenuItemProperties.cpp index 9b30c4a33e..18e09d317d 100644 --- a/libraries/script-engine/src/MenuItemProperties.cpp +++ b/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(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(engine, "MenuItemProperties"); -} - ScriptValue menuItemPropertiesToScriptValue(ScriptEngine* engine, const MenuItemProperties& properties) { ScriptValue obj = engine->newObject(); // not supported diff --git a/libraries/script-engine/src/MenuItemProperties.h b/libraries/script-engine/src/MenuItemProperties.h index 9bc7ed4e0e..67b451f330 100644 --- a/libraries/script-engine/src/MenuItemProperties.h +++ b/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); diff --git a/libraries/script-engine/src/ScriptEngineCast.h b/libraries/script-engine/src/ScriptEngineCast.h index a83ac7e8ec..5f5b0bb689 100644 --- a/libraries/script-engine/src/ScriptEngineCast.h +++ b/libraries/script-engine/src/ScriptEngineCast.h @@ -115,24 +115,6 @@ int scriptRegisterMetaTypeWithLambdas(ScriptEngine* eng, return id; } -/* -template -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(name); - } else { - id = qRegisterMetaType(); - } - eng->registerCustomType(id, reinterpret_cast(toScriptValue), - reinterpret_cast(fromScriptValue)); - return id; -}*/ - template ScriptValue scriptValueFromSequence(ScriptEngine* eng, const Container& cont) { ScriptValue a = eng->newArray(); diff --git a/libraries/script-engine/src/ScriptManager.cpp b/libraries/script-engine/src/ScriptManager.cpp index ce4572fd89..ad68b63002 100644 --- a/libraries/script-engine/src/ScriptManager.cpp +++ b/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(scriptEngine); + + scriptRegisterMetaType(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>(scriptEngine); - scriptRegisterSequenceMetaType>(scriptEngine); - - scriptRegisterSequenceMetaType>(scriptEngine); - scriptRegisterSequenceMetaType>(scriptEngine); - scriptRegisterSequenceMetaType>(scriptEngine); - - scriptRegisterMetaType(scriptEngine); - scriptRegisterMetaType(scriptEngine); - scriptRegisterMetaType(scriptEngine); - scriptRegisterMetaType(scriptEngine); - - scriptRegisterMetaType(scriptEngine); - - scriptRegisterMetaType(scriptEngine); - - scriptRegisterMetaType(scriptEngine); - scriptRegisterMetaType(scriptEngine); } void ScriptManager::init() { diff --git a/libraries/script-engine/src/ScriptValueUtils.cpp b/libraries/script-engine/src/ScriptValueUtils.cpp index 462d263e9e..2fa1d46de0 100644 --- a/libraries/script-engine/src/ScriptValueUtils.cpp +++ b/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(engine); - scriptRegisterMetaType, promiseToScriptValue, promiseFromScriptValue>(engine); + scriptRegisterMetaType(engine); + + scriptRegisterMetaType(engine); + scriptRegisterMetaType(engine); scriptRegisterSequenceMetaType >(engine); + scriptRegisterSequenceMetaType>(engine); + scriptRegisterSequenceMetaType>(engine); + + scriptRegisterSequenceMetaType>(engine); + scriptRegisterSequenceMetaType>(engine); + scriptRegisterSequenceMetaType>(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; diff --git a/libraries/script-engine/src/WebSocketClass.cpp b/libraries/script-engine/src/WebSocketClass.cpp index 4e250508ad..759ddc8fd9 100644 --- a/libraries/script-engine/src/WebSocketClass.cpp +++ b/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(scriptEngine); + scriptRegisterMetaType(scriptEngine); + scriptRegisterMetaType(scriptEngine); +})); WebSocketClass::WebSocketClass(ScriptEngine* engine, QString url) : _webSocket(new QWebSocket()), diff --git a/libraries/script-engine/src/qtscript/ScriptEngineQtScript_cast.cpp b/libraries/script-engine/src/qtscript/ScriptEngineQtScript_cast.cpp index 6a83e4eddc..8fc821b5d9 100644 --- a/libraries/script-engine/src/qtscript/ScriptEngineQtScript_cast.cpp +++ b/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()) { dest = QVariant::fromValue(ScriptValue(new ScriptValueQtWrapper(this, val))); diff --git a/libraries/script-engine/src/qtscript/ScriptObjectQtProxy.cpp b/libraries/script-engine/src/qtscript/ScriptObjectQtProxy.cpp index e0a92642de..a4416e9b8c 100644 --- a/libraries/script-engine/src/qtscript/ScriptObjectQtProxy.cpp +++ b/libraries/script-engine/src/qtscript/ScriptObjectQtProxy.cpp @@ -17,6 +17,8 @@ #include +#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(); - printf("Method with QMetaType::UnknownType"); + qDebug(scriptengine) << "Method with QMetaType::UnknownType " << metaObject->className() << " " << (*iter).name(); } } return static_cast(_engine)->newObject( @@ -466,25 +466,22 @@ QVariant ScriptMethodQtProxy::extension(Extension extension, const QVariant& arg QVector< QList > qScriptArgLists; QVector< QVector > qGenArgsVectors; QVector< QList > qVarArgLists; - QVector conversionPenaltyScore; - QVector isMetaRejected; qScriptArgLists.resize(num_metas); qGenArgsVectors.resize(num_metas); qVarArgLists.resize(num_metas); - conversionPenaltyScore.resize(num_metas); - isMetaRejected.resize(num_metas); + bool isValidMetaSelected = false; + 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); - ScriptContextGuard guard(&ourContext); - } - - bool isValidMetaSelected = false; - int bestMeta = 0; - for (int i = 0; i < num_metas; i++) { - if (!isValidMetaSelected && !isMetaRejected[i]) { + if (!isValidMetaSelected) { 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 &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([&]{ + 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()) { - return; // already exists + { + ConnectionList::iterator lookup = findConnection(callbackThis, callback); + 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()) { - return; // not here - } + { + ConnectionList::iterator lookup = findConnection(callbackThis, callback); + if (lookup == _connections.end()) { + return; // not here + } - // remove it from our internal list of connections - withWriteLock([&]{ - _connections.erase(lookup); - }); + // remove it from our internal list of connections + withWriteLock([&]{ + _connections.erase(lookup); + }); + } // remove a reference to ourselves from the destination callback QScriptValue destData = callback.data();