From 55b5a2cd030a4a8a5cdd9835adfff61dcdc926d9 Mon Sep 17 00:00:00 2001 From: Heather Anderson Date: Tue, 14 Dec 2021 19:57:50 -0800 Subject: [PATCH] Continued tightening and testing of the scripting engine "glue" logic - QObject wrappers will now be reused when asked to wrap the same QObject multiple times - improved error reporting when we can't form a native call from script - dropped support for having the existing QtScript logic handle Qt object transformations - assuming that our code will handle all calls into native objects - changing timer functions to use CTimer* publicly rather than casted QObject* --- libraries/audio/src/AudioEffectOptions.cpp | 2 +- libraries/script-engine/src/ScriptManager.cpp | 9 +- libraries/script-engine/src/ScriptManager.h | 13 +- .../src/qtscript/ArrayBufferClass.cpp | 3 - .../src/qtscript/ScriptContextQtAgent.cpp | 73 ----- .../src/qtscript/ScriptContextQtAgent.h | 48 --- .../src/qtscript/ScriptEngineQtScript.cpp | 21 +- .../src/qtscript/ScriptEngineQtScript.h | 19 +- .../qtscript/ScriptEngineQtScript_cast.cpp | 275 ++++++------------ .../src/qtscript/ScriptObjectQtProxy.cpp | 162 +++++++++-- .../src/qtscript/ScriptObjectQtProxy.h | 19 +- 11 files changed, 262 insertions(+), 382 deletions(-) delete mode 100644 libraries/script-engine/src/qtscript/ScriptContextQtAgent.cpp delete mode 100644 libraries/script-engine/src/qtscript/ScriptContextQtAgent.h diff --git a/libraries/audio/src/AudioEffectOptions.cpp b/libraries/audio/src/AudioEffectOptions.cpp index 2c9c946202..bed083d7db 100644 --- a/libraries/audio/src/AudioEffectOptions.cpp +++ b/libraries/audio/src/AudioEffectOptions.cpp @@ -150,5 +150,5 @@ AudioEffectOptions& AudioEffectOptions::operator=(const AudioEffectOptions &othe } ScriptValue AudioEffectOptions::constructor(ScriptContext* context, ScriptEngine* engine) { - return engine->newQObject(new AudioEffectOptions(context->argument(0))); + return engine->newQObject(new AudioEffectOptions(context->argument(0)), ScriptEngine::ScriptOwnership); } diff --git a/libraries/script-engine/src/ScriptManager.cpp b/libraries/script-engine/src/ScriptManager.cpp index 258d8737a9..d35cdfa100 100644 --- a/libraries/script-engine/src/ScriptManager.cpp +++ b/libraries/script-engine/src/ScriptManager.cpp @@ -79,9 +79,6 @@ static const ScriptValue::PropertyFlags READONLY_HIDDEN_PROP_FLAGS{ READONLY_PRO static const bool HIFI_AUTOREFRESH_FILE_SCRIPTS { true }; -Q_DECLARE_METATYPE(ScriptEngine::FunctionSignature) -int functionSignatureMetaID = qRegisterMetaType(); - int scriptManagerPointerMetaID = qRegisterMetaType(); Q_DECLARE_METATYPE(ExternalResource::Bucket); @@ -1054,7 +1051,7 @@ void ScriptManager::timerFired() { } } -QObject* ScriptManager::setupTimerWithInterval(const ScriptValue& function, int intervalMS, bool isSingleShot) { +QTimer* ScriptManager::setupTimerWithInterval(const ScriptValue& function, int intervalMS, bool isSingleShot) { // create the timer, add it to the map, and start it QTimer* newTimer = new QTimer(this); newTimer->setSingleShot(isSingleShot); @@ -1078,7 +1075,7 @@ QObject* ScriptManager::setupTimerWithInterval(const ScriptValue& function, int return newTimer; } -QObject* ScriptManager::setInterval(const ScriptValue& function, int intervalMS) { +QTimer* ScriptManager::setInterval(const ScriptValue& function, int intervalMS) { if (isStopped()) { scriptWarningMessage("Script.setInterval() while shutting down is ignored... parent script:" + getFilename()); return NULL; // bail early @@ -1087,7 +1084,7 @@ QObject* ScriptManager::setInterval(const ScriptValue& function, int intervalMS) return setupTimerWithInterval(function, intervalMS, false); } -QObject* ScriptManager::setTimeout(const ScriptValue& function, int timeoutMS) { +QTimer* ScriptManager::setTimeout(const ScriptValue& function, int timeoutMS) { if (isStopped()) { scriptWarningMessage("Script.setTimeout() while shutting down is ignored... parent script:" + getFilename()); return NULL; // bail early diff --git a/libraries/script-engine/src/ScriptManager.h b/libraries/script-engine/src/ScriptManager.h index 58db2bef69..76540bab81 100644 --- a/libraries/script-engine/src/ScriptManager.h +++ b/libraries/script-engine/src/ScriptManager.h @@ -27,6 +27,7 @@ #include #include #include +#include #include #include @@ -60,6 +61,8 @@ using ScriptValueList = QList; Q_DECLARE_METATYPE(ScriptManagerPointer) +const int QTREGISTER_QTimerStar = qRegisterMetaType(); + class CallbackData { public: ScriptValue function; @@ -429,7 +432,7 @@ public: * print("Interval timer fired"); * }, 1000); */ - Q_INVOKABLE QObject* setInterval(const ScriptValue& function, int intervalMS); + Q_INVOKABLE QTimer* setInterval(const ScriptValue& function, int intervalMS); /**jsdoc * Calls a function once, after a delay. @@ -442,7 +445,7 @@ public: * print("Timeout timer fired"); * }, 1000); */ - Q_INVOKABLE QObject* setTimeout(const ScriptValue& function, int timeoutMS); + Q_INVOKABLE QTimer* setTimeout(const ScriptValue& function, int timeoutMS); /**jsdoc * Stops an interval timer set by {@link Script.setInterval|setInterval}. @@ -460,7 +463,7 @@ public: * Script.clearInterval(timer); * }, 10000); */ - Q_INVOKABLE void clearInterval(QObject* timer) { stopTimer(reinterpret_cast(timer)); } + Q_INVOKABLE void clearInterval(QTimer* timer) { stopTimer(timer); } /**jsdoc * Stops a timeout timer set by {@link Script.setTimeout|setTimeout}. @@ -475,7 +478,7 @@ public: * // Uncomment the following line to stop the timer from firing. * //Script.clearTimeout(timer); */ - Q_INVOKABLE void clearTimeout(QObject* timer) { stopTimer(reinterpret_cast(timer)); } + Q_INVOKABLE void clearTimeout(QTimer* timer) { stopTimer(timer); } /**jsdoc * Prints a message to the program log and emits {@link Script.printedMessage}. @@ -901,7 +904,7 @@ protected: void setEntityScriptDetails(const EntityItemID& entityID, const EntityScriptDetails& details); void setParentURL(const QString& parentURL) { _parentURL = parentURL; } - QObject* setupTimerWithInterval(const ScriptValue& function, int intervalMS, bool isSingleShot); + QTimer* setupTimerWithInterval(const ScriptValue& function, int intervalMS, bool isSingleShot); void stopTimer(QTimer* timer); QHash _registeredHandlers; diff --git a/libraries/script-engine/src/qtscript/ArrayBufferClass.cpp b/libraries/script-engine/src/qtscript/ArrayBufferClass.cpp index 85564bc114..e5ae13e7d4 100644 --- a/libraries/script-engine/src/qtscript/ArrayBufferClass.cpp +++ b/libraries/script-engine/src/qtscript/ArrayBufferClass.cpp @@ -23,10 +23,7 @@ static const QString CLASS_NAME = "ArrayBuffer"; // FIXME: Q_DECLARE_METATYPE is global and really belongs in a shared header file, not per .cpp like this // (see DataViewClass.cpp, etc. which would also have to be updated to resolve) -Q_DECLARE_METATYPE(QScriptClass*) Q_DECLARE_METATYPE(QByteArray*) -int qScriptClassPointerMetaTypeId = qRegisterMetaType(); -int qByteArrayPointerMetaTypeId = qRegisterMetaType(); ArrayBufferClass::ArrayBufferClass(ScriptEngineQtScript* scriptEngine) : QObject(scriptEngine), diff --git a/libraries/script-engine/src/qtscript/ScriptContextQtAgent.cpp b/libraries/script-engine/src/qtscript/ScriptContextQtAgent.cpp deleted file mode 100644 index e082272a63..0000000000 --- a/libraries/script-engine/src/qtscript/ScriptContextQtAgent.cpp +++ /dev/null @@ -1,73 +0,0 @@ -// -// ScriptContextQtAgent.cpp -// libraries/script-engine/src/qtscript -// -// Created by Heather Anderson on 5/22/21. -// Copyright 2021 Vircadia contributors. -// -// Distributed under the Apache License, Version 2.0. -// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html -// - -#include "ScriptContextQtAgent.h" - -#include - -#include "../Scriptable.h" -#include "ScriptContextQtWrapper.h" -#include "ScriptEngineQtScript.h" - -void ScriptContextQtAgent::contextPop() { - if (_prevAgent) { - _prevAgent->contextPop(); - } - if (_engine->currentContext() == _currContext.get()) { - _currContext.reset(); - if (!_contextActive && !_contextStack.empty()) { - _currContext = _contextStack.back(); - _contextStack.pop_back(); - _contextActive = true; - } - } -} - -void ScriptContextQtAgent::contextPush() { - if (_prevAgent) { - _prevAgent->contextPush(); - } - if (_contextActive && _currContext) { - _contextStack.push_back(_currContext); - _contextActive = false; - } - _currContext.reset(); -} - -void ScriptContextQtAgent::functionEntry(qint64 scriptId) { - if (_prevAgent) { - _prevAgent->functionEntry(scriptId); - } - if (scriptId != -1) { - return; - } - if (!_currContext) { - _currContext = std::make_shared(_engine, static_cast(_engine)->currentContext()); - } - Scriptable::setContext(_currContext.get()); - _contextActive = true; -} - -void ScriptContextQtAgent::functionExit(qint64 scriptId, const QScriptValue& returnValue) { - if (_prevAgent) { - _prevAgent->functionExit(scriptId, returnValue); - } - if (scriptId != -1) { - return; - } - _contextActive = false; - if (!_contextActive && !_contextStack.empty()) { - _currContext = _contextStack.back(); - _contextStack.pop_back(); - Scriptable::setContext(_currContext.get()); - _contextActive = true; - } -} diff --git a/libraries/script-engine/src/qtscript/ScriptContextQtAgent.h b/libraries/script-engine/src/qtscript/ScriptContextQtAgent.h deleted file mode 100644 index 0869e5a50f..0000000000 --- a/libraries/script-engine/src/qtscript/ScriptContextQtAgent.h +++ /dev/null @@ -1,48 +0,0 @@ -// -// ScriptContextQtAgent.h -// libraries/script-engine/src/qtscript -// -// Created by Heather Anderson on 5/22/21. -// Copyright 2021 Vircadia contributors. -// -// Distributed under the Apache License, Version 2.0. -// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html -// - -#ifndef hifi_ScriptContextQtAgent_h -#define hifi_ScriptContextQtAgent_h - -#include - -#include -#include - -#include "ScriptEngineQtScript.h" - -class QScriptContext; -class QScriptValue; -class ScriptContextQtWrapper; -class ScriptEngineQtScript; -using ScriptContextQtPointer = std::shared_ptr; - -class ScriptContextQtAgent final : public QScriptEngineAgent { -public: // construction - inline ScriptContextQtAgent(ScriptEngineQtScript* engine, QScriptEngineAgent* prevAgent) : - QScriptEngineAgent(engine), _engine(engine), _prevAgent(prevAgent) {} - virtual ~ScriptContextQtAgent() {} - -public: // QScriptEngineAgent implementation - virtual void contextPop() override; - virtual void contextPush() override; - virtual void functionEntry(qint64 scriptId) override; - virtual void functionExit(qint64 scriptId, const QScriptValue& returnValue) override; - -private: // storage - bool _contextActive{ false }; - QList _contextStack; - ScriptContextQtPointer _currContext; - ScriptEngineQtScript* _engine; - QScriptEngineAgent* _prevAgent; -}; - -#endif // hifi_ScriptContextQtAgent_h \ No newline at end of file diff --git a/libraries/script-engine/src/qtscript/ScriptEngineQtScript.cpp b/libraries/script-engine/src/qtscript/ScriptEngineQtScript.cpp index e5ce15898b..7b4a39f942 100644 --- a/libraries/script-engine/src/qtscript/ScriptEngineQtScript.cpp +++ b/libraries/script-engine/src/qtscript/ScriptEngineQtScript.cpp @@ -52,15 +52,9 @@ #include "ScriptObjectQtProxy.h" #include "ScriptProgramQtWrapper.h" #include "ScriptValueQtWrapper.h" -#include "ScriptContextQtAgent.h" static const int MAX_DEBUG_VALUE_LENGTH { 80 }; -Q_DECLARE_METATYPE(QScriptEngine::FunctionSignature) -int qfunctionSignatureMetaID = qRegisterMetaType(); - -int scriptEnginePointerMetaID = qRegisterMetaType(); - bool ScriptEngineQtScript::IS_THREADSAFE_INVOCATION(const QThread* thread, const QString& method) { const QThread* currentThread = QThread::currentThread(); if (currentThread == thread) { @@ -336,13 +330,6 @@ ScriptEngineQtScript::ScriptEngineQtScript(ScriptManager* scriptManager) : _undefinedValue = ScriptValue(new ScriptValueQtWrapper(this, std::move(undefined))); QScriptEngine::setProcessEventsInterval(MSECS_PER_SECOND); - - _contextAgent = new ScriptContextQtAgent(this, agent()); - setAgent(_contextAgent); -} - -ScriptEngineQtScript::~ScriptEngineQtScript() { - delete _contextAgent; } void ScriptEngineQtScript::registerEnum(const QString& enumName, QMetaEnum newEnum) { @@ -912,20 +899,20 @@ ScriptValue ScriptEngineQtScript::create(int type, const void* ptr) { return ScriptValue(new ScriptValueQtWrapper(this, std::move(scriptValue))); } -QVariant ScriptEngineQtScript::convert(const ScriptValue& value, int type) { +QVariant ScriptEngineQtScript::convert(const ScriptValue& value, int typeId) { ScriptValueQtWrapper* unwrapped = ScriptValueQtWrapper::unwrap(value); if (unwrapped == nullptr) { return QVariant(); } QVariant var; - if (!castValueToVariant(unwrapped->toQtValue(), var, type)) { + if (!castValueToVariant(unwrapped->toQtValue(), var, typeId)) { return QVariant(); } int destType = var.userType(); - if (destType != type) { - var.convert(type); // if conversion fails then var is set to QVariant() + if (destType != typeId) { + var.convert(typeId); // if conversion fails then var is set to QVariant() } return var; diff --git a/libraries/script-engine/src/qtscript/ScriptEngineQtScript.h b/libraries/script-engine/src/qtscript/ScriptEngineQtScript.h index 5dd685292f..dcc87207c8 100644 --- a/libraries/script-engine/src/qtscript/ScriptEngineQtScript.h +++ b/libraries/script-engine/src/qtscript/ScriptEngineQtScript.h @@ -24,6 +24,7 @@ #include #include #include +#include #include #include @@ -34,13 +35,12 @@ #include "ArrayBufferClass.h" class ScriptContextQtWrapper; -class ScriptContextQtAgent; class ScriptEngineQtScript; class ScriptManager; -using ScriptEngineQtScriptPointer = std::shared_ptr; +class ScriptObjectQtProxy; using ScriptContextQtPointer = std::shared_ptr; -Q_DECLARE_METATYPE(ScriptEngineQtScriptPointer); +Q_DECLARE_METATYPE(ScriptEngine::FunctionSignature) /// [QtScript] Implements ScriptEngine for QtScript and translates calls for QScriptEngine class ScriptEngineQtScript final : public QScriptEngine, @@ -50,7 +50,6 @@ class ScriptEngineQtScript final : public QScriptEngine, public: // construction ScriptEngineQtScript(ScriptManager* scriptManager = nullptr); - ~ScriptEngineQtScript(); //////////////////////////////////////////////////////////////////////////////////////////////////////////////////// // NOTE - these are NOT intended to be public interfaces available to scripts, the are only Q_INVOKABLE so we can @@ -150,11 +149,16 @@ public: // public non-interface methods for other QtScript-specific classes to u public: // not for public use, but I don't like how Qt strings this along with private friend functions virtual ScriptValue create(int type, const void* ptr) override; - virtual QVariant convert(const ScriptValue& value, int type) override; + virtual QVariant convert(const ScriptValue& value, int typeId) override; virtual void registerCustomType(int type, ScriptEngine::MarshalFunction marshalFunc, ScriptEngine::DemarshalFunction demarshalFunc) override; - bool castValueToVariant(const QScriptValue& val, QVariant& dest, int destType); + bool castValueToVariant(const QScriptValue& val, QVariant& dest, int destTypeId); QScriptValue castVariantToValue(const QVariant& val); + static QString valueType(const QScriptValue& val); + + using ObjectWrapperMap = QMap>; + mutable QMutex _qobjectWrapperMapProtect; + ObjectWrapperMap _qobjectWrapperMap; protected: // like `newFunction`, but allows mapping inline C++ lambdas with captures as callable QScriptValues @@ -180,14 +184,11 @@ protected: mutable QMutex _customTypeProtect; CustomMarshalMap _customTypes; CustomPrototypeMap _customPrototypes; - int _nextCustomType = 0; ScriptValue _nullValue; ScriptValue _undefinedValue; mutable ScriptContextQtPointer _currContext; ArrayBufferClass* _arrayBufferClass; - - ScriptContextQtAgent* _contextAgent{ nullptr }; }; // Lambda helps create callable QScriptValues out of std::functions: diff --git a/libraries/script-engine/src/qtscript/ScriptEngineQtScript_cast.cpp b/libraries/script-engine/src/qtscript/ScriptEngineQtScript_cast.cpp index 69125ec607..7b799d168f 100644 --- a/libraries/script-engine/src/qtscript/ScriptEngineQtScript_cast.cpp +++ b/libraries/script-engine/src/qtscript/ScriptEngineQtScript_cast.cpp @@ -16,64 +16,16 @@ #include #include +#include "../ScriptEngineCast.h" #include "../ScriptValueIterator.h" #include "ScriptObjectQtProxy.h" #include "ScriptValueQtWrapper.h" -template -class CustomTypeInstance { -public: - static ScriptEngine::MarshalFunction marshalFunc; - static ScriptEngine::DemarshalFunction demarshalFunc; - - static QScriptValue internalMarshalFunc(QScriptEngine* engine, const void* src) { - ScriptEngineQtScript* unwrappedEngine = static_cast(engine); - ScriptValue dest = marshalFunc(unwrappedEngine, src); - return ScriptValueQtWrapper::fullUnwrap(unwrappedEngine, dest); - } - - static void internalDemarshalFunc(const QScriptValue& src, void* dest) { - ScriptEngineQtScript* unwrappedEngine = static_cast(src.engine()); - ScriptValue wrappedSrc(new ScriptValueQtWrapper(unwrappedEngine, src)); - demarshalFunc(wrappedSrc, dest); - } -}; -template -ScriptEngine::MarshalFunction CustomTypeInstance::marshalFunc; -template -ScriptEngine::DemarshalFunction CustomTypeInstance::demarshalFunc; - -// I would *LOVE* it if there was a different way to do this, jeez! -// Qt requires two functions that have no parameters that give any context, -// one of the must return a QScriptValue (so we can't void* them into generics and stick them in the templates). -// This *has* to be done via templates but the whole point of this is to avoid leaking types into the rest of -// the system that would require anyone other than us to have a dependency on QtScript -#define CUSTOM_TYPE_ENTRY(idx) \ - case idx: \ - CustomTypeInstance::marshalFunc = marshalFunc; \ - CustomTypeInstance::demarshalFunc = demarshalFunc; \ - internalMarshalFunc = CustomTypeInstance::internalMarshalFunc; \ - internalDemarshalFunc = CustomTypeInstance::internalDemarshalFunc; \ - break; -#define CUSTOM_TYPE_ENTRY_10(idx) \ - CUSTOM_TYPE_ENTRY((idx * 10)); \ - CUSTOM_TYPE_ENTRY((idx * 10) + 1); \ - CUSTOM_TYPE_ENTRY((idx * 10) + 2); \ - CUSTOM_TYPE_ENTRY((idx * 10) + 3); \ - CUSTOM_TYPE_ENTRY((idx * 10) + 4); \ - CUSTOM_TYPE_ENTRY((idx * 10) + 5); \ - CUSTOM_TYPE_ENTRY((idx * 10) + 6); \ - CUSTOM_TYPE_ENTRY((idx * 10) + 7); \ - CUSTOM_TYPE_ENTRY((idx * 10) + 8); \ - CUSTOM_TYPE_ENTRY((idx * 10) + 9); - void ScriptEngineQtScript::setDefaultPrototype(int metaTypeId, const ScriptValue& prototype) { ScriptValueQtWrapper* unwrappedPrototype = ScriptValueQtWrapper::unwrap(prototype); if (unwrappedPrototype) { const QScriptValue& scriptPrototype = unwrappedPrototype->toQtValue(); - QScriptEngine::setDefaultPrototype(metaTypeId, scriptPrototype); - QMutexLocker guard(&_customTypeProtect); _customPrototypes.insert(metaTypeId, scriptPrototype); } @@ -83,59 +35,12 @@ void ScriptEngineQtScript::registerCustomType(int type, ScriptEngine::MarshalFunction marshalFunc, ScriptEngine::DemarshalFunction demarshalFunc) { - QScriptEngine::MarshalFunction internalMarshalFunc; - QScriptEngine::DemarshalFunction internalDemarshalFunc; + QMutexLocker guard(&_customTypeProtect); - { - QMutexLocker guard(&_customTypeProtect); - - // storing it in a map for our own benefit - CustomMarshal& customType = _customTypes.insert(type, CustomMarshal()).value(); - customType.demarshalFunc = demarshalFunc; - customType.marshalFunc = marshalFunc; - - // creating a conversion for QtScript's benefit - if (_nextCustomType >= 300) { // have we ran out of translators? - Q_ASSERT(false); - return; - } - - switch (_nextCustomType++) { - CUSTOM_TYPE_ENTRY_10(0); - CUSTOM_TYPE_ENTRY_10(1); - CUSTOM_TYPE_ENTRY_10(2); - CUSTOM_TYPE_ENTRY_10(3); - CUSTOM_TYPE_ENTRY_10(4); - CUSTOM_TYPE_ENTRY_10(5); - CUSTOM_TYPE_ENTRY_10(6); - CUSTOM_TYPE_ENTRY_10(7); - CUSTOM_TYPE_ENTRY_10(8); - CUSTOM_TYPE_ENTRY_10(9); - CUSTOM_TYPE_ENTRY_10(10); - CUSTOM_TYPE_ENTRY_10(11); - CUSTOM_TYPE_ENTRY_10(12); - CUSTOM_TYPE_ENTRY_10(13); - CUSTOM_TYPE_ENTRY_10(14); - CUSTOM_TYPE_ENTRY_10(15); - CUSTOM_TYPE_ENTRY_10(16); - CUSTOM_TYPE_ENTRY_10(17); - CUSTOM_TYPE_ENTRY_10(18); - CUSTOM_TYPE_ENTRY_10(19); - CUSTOM_TYPE_ENTRY_10(20); - CUSTOM_TYPE_ENTRY_10(21); - CUSTOM_TYPE_ENTRY_10(22); - CUSTOM_TYPE_ENTRY_10(23); - CUSTOM_TYPE_ENTRY_10(24); - CUSTOM_TYPE_ENTRY_10(25); - CUSTOM_TYPE_ENTRY_10(26); - CUSTOM_TYPE_ENTRY_10(27); - CUSTOM_TYPE_ENTRY_10(28); - CUSTOM_TYPE_ENTRY_10(29); - CUSTOM_TYPE_ENTRY_10(30); - } - } - - qScriptRegisterMetaType_helper(this, type, internalMarshalFunc, internalDemarshalFunc, QScriptValue()); + // storing it in a map for our own benefit + CustomMarshal& customType = _customTypes.insert(type, CustomMarshal()).value(); + customType.demarshalFunc = demarshalFunc; + customType.marshalFunc = marshalFunc; } Q_DECLARE_METATYPE(ScriptValue); @@ -149,8 +54,7 @@ static void ScriptValueFromQScriptValue(const QScriptValue& src, ScriptValue& de dest = ScriptValue(new ScriptValueQtWrapper(engine, src)); } -static ScriptValue StringListToScriptValue(ScriptEngine* engine, const void* pSrc) { - const QStringList& src = *reinterpret_cast(pSrc); +static ScriptValue StringListToScriptValue(ScriptEngine* engine, const QStringList& src) { int len = src.length(); ScriptValue dest = engine->newArray(len); for (int idx = 0; idx < len; ++idx) { @@ -159,9 +63,8 @@ static ScriptValue StringListToScriptValue(ScriptEngine* engine, const void* pSr return dest; } -static bool StringListFromScriptValue(const ScriptValue& src, void* pDest) { +static bool StringListFromScriptValue(const ScriptValue& src, QStringList& dest) { if(!src.isArray()) return false; - QStringList& dest = *reinterpret_cast(pDest); int len = src.property("length").toInteger(); dest.clear(); for (int idx = 0; idx < len; ++idx) { @@ -170,8 +73,7 @@ static bool StringListFromScriptValue(const ScriptValue& src, void* pDest) { return true; } -static ScriptValue VariantListToScriptValue(ScriptEngine* engine, const void* pSrc) { - const QVariantList& src = *reinterpret_cast(pSrc); +static ScriptValue VariantListToScriptValue(ScriptEngine* engine, const QVariantList& src) { int len = src.length(); ScriptValue dest = engine->newArray(len); for (int idx = 0; idx < len; ++idx) { @@ -180,9 +82,8 @@ static ScriptValue VariantListToScriptValue(ScriptEngine* engine, const void* pS return dest; } -static bool VariantListFromScriptValue(const ScriptValue& src, void* pDest) { +static bool VariantListFromScriptValue(const ScriptValue& src, QVariantList& dest) { if(!src.isArray()) return false; - QVariantList& dest = *reinterpret_cast(pDest); int len = src.property("length").toInteger(); dest.clear(); for (int idx = 0; idx < len; ++idx) { @@ -191,8 +92,7 @@ static bool VariantListFromScriptValue(const ScriptValue& src, void* pDest) { return true; } -static ScriptValue VariantMapToScriptValue(ScriptEngine* engine, const void* pSrc) { - const QVariantMap& src = *reinterpret_cast(pSrc); +static ScriptValue VariantMapToScriptValue(ScriptEngine* engine, const QVariantMap& src) { ScriptValue dest = engine->newObject(); for (QVariantMap::const_iterator iter = src.cbegin(); iter != src.cend(); ++iter) { dest.setProperty(iter.key(), engine->newVariant(iter.value())); @@ -200,8 +100,7 @@ static ScriptValue VariantMapToScriptValue(ScriptEngine* engine, const void* pSr return dest; } -static bool VariantMapFromScriptValue(const ScriptValue& src, void* pDest) { - QVariantMap& dest = *reinterpret_cast(pDest); +static bool VariantMapFromScriptValue(const ScriptValue& src, QVariantMap& dest) { dest.clear(); ScriptValueIteratorPointer iter = src.newIterator(); while (iter->hasNext()) { @@ -211,8 +110,7 @@ static bool VariantMapFromScriptValue(const ScriptValue& src, void* pDest) { return true; } -static ScriptValue VariantHashToScriptValue(ScriptEngine* engine, const void* pSrc) { - const QVariantHash& src = *reinterpret_cast(pSrc); +static ScriptValue VariantHashToScriptValue(ScriptEngine* engine, const QVariantHash& src) { ScriptValue dest = engine->newObject(); for (QVariantHash::const_iterator iter = src.cbegin(); iter != src.cend(); ++iter) { dest.setProperty(iter.key(), engine->newVariant(iter.value())); @@ -220,8 +118,7 @@ static ScriptValue VariantHashToScriptValue(ScriptEngine* engine, const void* pS return dest; } -static bool VariantHashFromScriptValue(const ScriptValue& src, void* pDest) { - QVariantHash& dest = *reinterpret_cast(pDest); +static bool VariantHashFromScriptValue(const ScriptValue& src, QVariantHash& dest) { dest.clear(); ScriptValueIteratorPointer iter = src.newIterator(); while (iter->hasNext()) { @@ -231,19 +128,16 @@ static bool VariantHashFromScriptValue(const ScriptValue& src, void* pDest) { return true; } -static ScriptValue JsonValueToScriptValue(ScriptEngine* engine, const void* pSrc) { - const QJsonValue& src = *reinterpret_cast(pSrc); +static ScriptValue JsonValueToScriptValue(ScriptEngine* engine, const QJsonValue& src) { return engine->newVariant(src.toVariant()); } -static bool JsonValueFromScriptValue(const ScriptValue& src, void* pDest) { - QJsonValue& dest = *reinterpret_cast(pDest); +static bool JsonValueFromScriptValue(const ScriptValue& src, QJsonValue& dest) { dest = QJsonValue::fromVariant(src.toVariant()); return true; } -static ScriptValue JsonObjectToScriptValue(ScriptEngine* engine, const void* pSrc) { - const QJsonObject& src = *reinterpret_cast(pSrc); +static ScriptValue JsonObjectToScriptValue(ScriptEngine* engine, const QJsonObject& src) { QVariantMap map = src.toVariantMap(); ScriptValue dest = engine->newObject(); for (QVariantMap::const_iterator iter = map.cbegin(); iter != map.cend(); ++iter) { @@ -252,8 +146,7 @@ static ScriptValue JsonObjectToScriptValue(ScriptEngine* engine, const void* pSr return dest; } -static bool JsonObjectFromScriptValue(const ScriptValue& src, void* pDest) { - QJsonObject& dest = *reinterpret_cast(pDest); +static bool JsonObjectFromScriptValue(const ScriptValue& src, QJsonObject& dest) { QVariantMap map; ScriptValueIteratorPointer iter = src.newIterator(); while (iter->hasNext()) { @@ -264,8 +157,7 @@ static bool JsonObjectFromScriptValue(const ScriptValue& src, void* pDest) { return true; } -static ScriptValue JsonArrayToScriptValue(ScriptEngine* engine, const void* pSrc) { - const QJsonArray& src = *reinterpret_cast(pSrc); +static ScriptValue JsonArrayToScriptValue(ScriptEngine* engine, const QJsonArray& src) { QVariantList list = src.toVariantList(); int len = list.length(); ScriptValue dest = engine->newArray(len); @@ -275,9 +167,8 @@ static ScriptValue JsonArrayToScriptValue(ScriptEngine* engine, const void* pSrc return dest; } -static bool JsonArrayFromScriptValue(const ScriptValue& src, void* pDest) { +static bool JsonArrayFromScriptValue(const ScriptValue& src, QJsonArray& dest) { if(!src.isArray()) return false; - QJsonArray& dest = *reinterpret_cast(pDest); QVariantList list; int len = src.property("length").toInteger(); for (int idx = 0; idx < len; ++idx) { @@ -292,63 +183,33 @@ static bool JsonArrayFromScriptValue(const ScriptValue& src, void* pDest) { void ScriptEngineQtScript::registerSystemTypes() { qScriptRegisterMetaType(this, ScriptValueToQScriptValue, ScriptValueFromQScriptValue); - QMutexLocker guard(&_customTypeProtect); - - { - CustomMarshal& customType = _customTypes.insert(QMetaType::QStringList, CustomMarshal()).value(); - customType.demarshalFunc = StringListFromScriptValue; - customType.marshalFunc = StringListToScriptValue; - } - { - CustomMarshal& customType = _customTypes.insert(QMetaType::QVariantList, CustomMarshal()).value(); - customType.demarshalFunc = VariantListFromScriptValue; - customType.marshalFunc = VariantListToScriptValue; - } - { - CustomMarshal& customType = _customTypes.insert(QMetaType::QVariantMap, CustomMarshal()).value(); - customType.demarshalFunc = VariantMapFromScriptValue; - customType.marshalFunc = VariantMapToScriptValue; - } - { - CustomMarshal& customType = _customTypes.insert(QMetaType::QVariantHash, CustomMarshal()).value(); - customType.demarshalFunc = VariantHashFromScriptValue; - customType.marshalFunc = VariantHashToScriptValue; - } - { - CustomMarshal& customType = _customTypes.insert(QMetaType::QJsonValue, CustomMarshal()).value(); - customType.demarshalFunc = JsonValueFromScriptValue; - customType.marshalFunc = JsonValueToScriptValue; - } - { - CustomMarshal& customType = _customTypes.insert(QMetaType::QJsonObject, CustomMarshal()).value(); - customType.demarshalFunc = JsonObjectFromScriptValue; - customType.marshalFunc = JsonObjectToScriptValue; - } - { - CustomMarshal& customType = _customTypes.insert(QMetaType::QJsonArray, CustomMarshal()).value(); - customType.demarshalFunc = JsonArrayFromScriptValue; - customType.marshalFunc = JsonArrayToScriptValue; - } + scriptRegisterMetaType(this, StringListToScriptValue, StringListFromScriptValue); + scriptRegisterMetaType(this, VariantListToScriptValue, VariantListFromScriptValue); + scriptRegisterMetaType(this, VariantMapToScriptValue, VariantMapFromScriptValue); + scriptRegisterMetaType(this, VariantHashToScriptValue, VariantHashFromScriptValue); + scriptRegisterMetaType(this, JsonValueToScriptValue, JsonValueFromScriptValue); + scriptRegisterMetaType(this, JsonObjectToScriptValue, JsonObjectFromScriptValue); + scriptRegisterMetaType(this, JsonArrayToScriptValue, JsonArrayFromScriptValue); } -bool ScriptEngineQtScript::castValueToVariant(const QScriptValue& val, QVariant& dest, int destType) { +bool ScriptEngineQtScript::castValueToVariant(const QScriptValue& val, QVariant& dest, int destTypeId) { // if we're not particularly interested in a specific type, try to detect if we're dealing with a registered type - if (destType == QMetaType::UnknownType) { + if (destTypeId == QMetaType::UnknownType) { QObject* obj = ScriptObjectQtProxy::unwrap(val); if (obj) { for (const QMetaObject* metaObject = obj->metaObject(); metaObject; metaObject = metaObject->superClass()) { QByteArray typeName = QByteArray(metaObject->className()) + "*"; int typeId = QMetaType::type(typeName.constData()); if (typeId != QMetaType::UnknownType) { - destType = typeId; + destTypeId = typeId; break; } } } } - if (destType == qMetaTypeId()) { + if (destTypeId == qMetaTypeId()) { dest = QVariant::fromValue(ScriptValue(new ScriptValueQtWrapper(this, val))); return true; } @@ -357,20 +218,19 @@ bool ScriptEngineQtScript::castValueToVariant(const QScriptValue& val, QVariant& ScriptEngine::DemarshalFunction demarshalFunc = nullptr; { QMutexLocker guard(&_customTypeProtect); - CustomMarshalMap::const_iterator lookup = _customTypes.find(destType); + CustomMarshalMap::const_iterator lookup = _customTypes.find(destTypeId); if (lookup != _customTypes.cend()) { demarshalFunc = lookup.value().demarshalFunc; } } if (demarshalFunc) { - void* destStorage = QMetaType::create(destType); + dest = QVariant(destTypeId, static_cast(NULL)); ScriptValue wrappedVal(new ScriptValueQtWrapper(this, val)); - bool success = demarshalFunc(wrappedVal, destStorage); - dest = success ? QVariant(destType, destStorage) : QVariant(); - QMetaType::destroy(destType, destStorage); + bool success = demarshalFunc(wrappedVal, const_cast(dest.constData())); + if(!success) dest = QVariant(); return success; } else { - switch (destType) { + switch (destTypeId) { case QMetaType::UnknownType: if (val.isUndefined()) { dest = QVariant(); @@ -443,8 +303,19 @@ bool ScriptEngineQtScript::castValueToVariant(const QScriptValue& val, QVariant& break; default: // check to see if this is a pointer to a QObject-derived object - if (QMetaType::typeFlags(destType) & QMetaType::PointerToQObject) { - dest = QVariant::fromValue(ScriptObjectQtProxy::unwrap(val)); + if (QMetaType::typeFlags(destTypeId) & (QMetaType::PointerToQObject | QMetaType::TrackingPointerToQObject)) { + /* Do we really want to permit regular passing of nullptr to native functions? + if (!val.isValid() || val.isUndefined() || val.isNull()) { + dest = QVariant::fromValue(nullptr); + break; + }*/ + QObject* obj = ScriptObjectQtProxy::unwrap(val); + if (!obj) return false; + const QMetaObject* destMeta = QMetaType::metaObjectForType(destTypeId); + Q_ASSERT(destMeta); + obj = destMeta->cast(obj); + if (!obj) return false; + dest = QVariant::fromValue(obj); break; } // check to see if we have a registered prototype @@ -461,7 +332,40 @@ bool ScriptEngineQtScript::castValueToVariant(const QScriptValue& val, QVariant& } } - return destType == QMetaType::UnknownType || dest.userType() == destType || dest.convert(destType); + return destTypeId == QMetaType::UnknownType || dest.userType() == destTypeId || dest.convert(destTypeId); +} + +QString ScriptEngineQtScript::valueType(const QScriptValue& val) { + if (val.isUndefined()) { + return "undefined"; + } + if (val.isNull()) { + return "null"; + } + if (val.isBool()) { + return "boolean"; + } + if (val.isString()) { + return "string"; + } + if (val.isNumber()) { + return "number"; + } + { + QObject* obj = ScriptObjectQtProxy::unwrap(val); + if (obj) { + QString objectName = obj->objectName(); + if (!objectName.isEmpty()) return objectName; + return obj->metaObject()->className(); + } + } + { + QVariant var = ScriptVariantQtProxy::unwrap(val); + if (var.isValid()) { + return var.typeName(); + } + } + return val.toVariant().typeName(); } QScriptValue ScriptEngineQtScript::castVariantToValue(const QVariant& val) { @@ -513,16 +417,21 @@ QScriptValue ScriptEngineQtScript::castVariantToValue(const QVariant& val) { return QScriptValue(this, val.toString()); case QMetaType::QVariant: return castVariantToValue(val.value()); - case QMetaType::QObjectStar: - return ScriptObjectQtProxy::newQObject(this, val.value(), ScriptEngine::QtOwnership); + case QMetaType::QObjectStar: { + QObject* obj = val.value(); + if (obj == nullptr) return QScriptValue(this, QScriptValue::NullValue); + return ScriptObjectQtProxy::newQObject(this, obj); + } case QMetaType::QDateTime: return static_cast(this)->newDate(val.value()); case QMetaType::QDate: return static_cast(this)->newDate(val.value().startOfDay()); default: // check to see if this is a pointer to a QObject-derived object - if (QMetaType::typeFlags(valTypeId) & QMetaType::PointerToQObject) { - return ScriptObjectQtProxy::newQObject(this, val.value(), ScriptEngine::QtOwnership); + if (QMetaType::typeFlags(valTypeId) & (QMetaType::PointerToQObject | QMetaType::TrackingPointerToQObject)) { + QObject* obj = val.value(); + if (obj == nullptr) return QScriptValue(this, QScriptValue::NullValue); + return ScriptObjectQtProxy::newQObject(this, obj); } // have we set a prototype'd variant? { diff --git a/libraries/script-engine/src/qtscript/ScriptObjectQtProxy.cpp b/libraries/script-engine/src/qtscript/ScriptObjectQtProxy.cpp index f981534b07..17b93e77d9 100644 --- a/libraries/script-engine/src/qtscript/ScriptObjectQtProxy.cpp +++ b/libraries/script-engine/src/qtscript/ScriptObjectQtProxy.cpp @@ -53,6 +53,18 @@ private: // storage QScriptValue ScriptObjectQtProxy::newQObject(ScriptEngineQtScript* engine, QObject* object, ScriptEngine::ValueOwnership ownership, const ScriptEngine::QObjectWrapOptions& options) { + QScriptEngine* qengine = static_cast(engine); + + // do we already have a valid wrapper for this QObject? + { + QMutexLocker guard(&engine->_qobjectWrapperMapProtect); + ScriptEngineQtScript::ObjectWrapperMap::const_iterator lookup = engine->_qobjectWrapperMap.find(object); + if (lookup != engine->_qobjectWrapperMap.end()) { + QSharedPointer proxy = lookup.value().lock(); + if (proxy) return static_cast(engine)->newObject(proxy.get(), qengine->newVariant(QVariant::fromValue(proxy)));; + } + } + bool ownsObject; switch (ownership) { case ScriptEngine::QtOwnership: @@ -65,8 +77,33 @@ QScriptValue ScriptObjectQtProxy::newQObject(ScriptEngineQtScript* engine, QObje ownsObject = !object->parent(); break; } - QScriptEngine* qengine = static_cast(engine); + + // create the wrapper auto proxy = QSharedPointer::create(engine, object, ownsObject, options); + + { + QMutexLocker guard(&engine->_qobjectWrapperMapProtect); + + // check again to see if someone else created the wrapper while we were busy + ScriptEngineQtScript::ObjectWrapperMap::const_iterator lookup = engine->_qobjectWrapperMap.find(object); + if (lookup != engine->_qobjectWrapperMap.end()) { + QSharedPointer proxy = lookup.value().lock(); + if (proxy) return static_cast(engine)->newObject(proxy.get(), qengine->newVariant(QVariant::fromValue(proxy)));; + } + + // register the wrapper with the engine and make sure it cleans itself up + engine->_qobjectWrapperMap.insert(object, proxy); + QPointer enginePtr = engine; + object->connect(object, &QObject::destroyed, engine, [enginePtr, object]() { + if (!enginePtr) return; + QMutexLocker guard(&enginePtr->_qobjectWrapperMapProtect); + ScriptEngineQtScript::ObjectWrapperMap::iterator lookup = enginePtr->_qobjectWrapperMap.find(object); + if (lookup != enginePtr->_qobjectWrapperMap.end()) { + enginePtr->_qobjectWrapperMap.erase(lookup); + } + }); + } + return static_cast(engine)->newObject(proxy.get(), qengine->newVariant(QVariant::fromValue(proxy))); } @@ -96,7 +133,6 @@ void ScriptObjectQtProxy::investigate() { if (!qobject) return; const QMetaObject* metaObject = qobject->metaObject(); - _name = QString::fromLatin1(metaObject->className()); // discover properties int startIdx = _wrapOptions & ScriptEngine::ExcludeSuperClassProperties ? metaObject->propertyOffset() : 0; @@ -185,6 +221,15 @@ void ScriptObjectQtProxy::investigate() { } } +QString ScriptObjectQtProxy::name() const { + Q_ASSERT(_object); + if (!_object) return ""; + return _object ? _object->objectName() : ""; + QString objectName = _object->objectName(); + if (!objectName.isEmpty()) return objectName; + return _object->metaObject()->className(); +} + QScriptClass::QueryFlags ScriptObjectQtProxy::queryProperty(const QScriptValue& object, const QScriptString& name, QueryFlags flags, uint* id) { // check for properties for (PropertyDefMap::const_iterator trans = _props.cbegin(); trans != _props.cend(); ++trans) { @@ -268,7 +313,7 @@ QScriptValue ScriptObjectQtProxy::property(const QScriptValue& object, const QSc MethodDefMap::const_iterator lookup = _methods.find(methodId); if (lookup == _methods.cend()) return QScriptValue(); return static_cast(_engine)->newObject( - new ScriptMethodQtProxy(_engine, qobject, object, name, lookup.value().methods)); + new ScriptMethodQtProxy(_engine, qobject, object, lookup.value().methods)); } case SIGNAL_TYPE: { int signalId = id & ~TYPE_MASK; @@ -278,7 +323,7 @@ QScriptValue ScriptObjectQtProxy::property(const QScriptValue& object, const QSc InstanceMap::const_iterator instLookup = _signalInstances.find(signalId); if (instLookup == _signalInstances.cend() || instLookup.value().isNull()) { instLookup = _signalInstances.insert(signalId, - new ScriptSignalQtProxy(_engine, qobject, object, name, defLookup.value().signal)); + new ScriptSignalQtProxy(_engine, qobject, object, defLookup.value().signal)); Q_ASSERT(instLookup != _signalInstances.cend()); } ScriptSignalQtProxy* proxy = instLookup.value(); @@ -315,10 +360,11 @@ void ScriptObjectQtProxy::setProperty(QScriptValue& object, const QScriptString& ScriptContextGuard guard(&ourContext); int propTypeId = prop.userType(); + Q_ASSERT(propTypeId != QMetaType::UnknownType); QVariant varValue; if(!_engine->castValueToVariant(value, varValue, propTypeId)) { QByteArray propTypeName = QMetaType(propTypeId).name(); - QByteArray valTypeName = value.toVariant().typeName(); + QByteArray valTypeName = _engine->valueType(value).toLatin1(); QScriptContext* currentContext = static_cast(_engine)->currentContext(); currentContext->throwError(QScriptContext::TypeError, QString("Cannot convert %1 to %2").arg(valTypeName, propTypeName)); return; @@ -352,6 +398,18 @@ QVariant ScriptVariantQtProxy::unwrap(const QScriptValue& val) { return proxy ? proxy->toQtValue() : QVariant(); } +QString ScriptMethodQtProxy::fullName() const { + Q_ASSERT(_object); + if (!_object) return ""; + Q_ASSERT(!_metas.isEmpty()); + const QMetaMethod& firstMethod = _metas.front(); + QString objectName = _object->objectName(); + if (!objectName.isEmpty()) { + return QString("%1.%2").arg(objectName, firstMethod.name()); + } + return QString("%1::%2").arg(_object->metaObject()->className(), firstMethod.name()); +} + bool ScriptMethodQtProxy::supportsExtension(Extension extension) const { switch (extension) { case Callable: @@ -377,6 +435,9 @@ QVariant ScriptMethodQtProxy::extension(Extension extension, const QVariant& arg const int scriptValueTypeId = qMetaTypeId(); + int parameterConversionFailureId = 0; + int parameterConversionFailureCount = 0; + for (auto iter = _metas.cbegin(); iter != _metas.end(); ++iter) { const QMetaMethod& meta = *iter; int methodNumArgs = meta.parameterCount(); @@ -387,10 +448,10 @@ QVariant ScriptMethodQtProxy::extension(Extension extension, const QVariant& arg QList qScriptArgList; QList qVarArgList; QGenericArgument qGenArgs[10]; - int arg; - bool conversionFailed = false; - for (arg = 0; arg < numArgs && !conversionFailed; ++arg) { + int conversionFailures = 0; + for (int arg = 0; arg < numArgs; ++arg) { int methodArgTypeId = meta.parameterType(arg); + Q_ASSERT(methodArgTypeId != QMetaType::UnknownType); QScriptValue argVal = context->argument(arg); if (methodArgTypeId == scriptValueTypeId) { qScriptArgList.append(ScriptValue(new ScriptValueQtWrapper(_engine, argVal))); @@ -401,24 +462,23 @@ QVariant ScriptMethodQtProxy::extension(Extension extension, const QVariant& arg } else { QVariant varArgVal; if (!_engine->castValueToVariant(argVal, varArgVal, methodArgTypeId)) { - conversionFailed = true; - break; - /*QByteArray methodTypeName = QMetaType(methodArgTypeId).name(); - QByteArray argTypeName = argVal.toVariant().typeName(); - context->throwError(QScriptContext::TypeError, - QString("Cannot convert %1 to %2").arg(argTypeName, methodTypeName)); - return QVariant();*/ - } - qVarArgList.append(varArgVal); - const QVariant& converted = qVarArgList.back(); + conversionFailures++; + } else { + qVarArgList.append(varArgVal); + const QVariant& converted = qVarArgList.back(); - // 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. - qGenArgs[arg] = - QGenericArgument(QMetaType::typeName(converted.userType()), const_cast(converted.constData())); + // 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. + qGenArgs[arg] = + QGenericArgument(QMetaType::typeName(converted.userType()), const_cast(converted.constData())); + } } } - if (conversionFailed) { + if (conversionFailures) { + if (conversionFailures < parameterConversionFailureCount || !parameterConversionFailureCount) { + parameterConversionFailureCount = conversionFailures; + parameterConversionFailureId = meta.methodIndex(); + } continue; } @@ -426,11 +486,18 @@ QVariant ScriptMethodQtProxy::extension(Extension extension, const QVariant& arg ScriptContextGuard guard(&ourContext); int returnTypeId = meta.returnType(); - if (returnTypeId == QMetaType::Void) { + + // The Qt MOC engine will automatically call qRegisterMetaType on invokable parameters and properties, but there's + // nothing in there for return values so these need to be explicitly runtime-registered! + Q_ASSERT(returnTypeId != QMetaType::UnknownType); + if (returnTypeId == QMetaType::UnknownType) { + context->throwError(QString("Cannot call native function %1, its return value has not been registered with Qt").arg(fullName())); + return QVariant(); + } else if (returnTypeId == QMetaType::Void) { bool success = meta.invoke(qobject, Qt::DirectConnection, qGenArgs[0], qGenArgs[1], qGenArgs[2], qGenArgs[3], qGenArgs[4], qGenArgs[5], qGenArgs[6], qGenArgs[7], qGenArgs[8], qGenArgs[9]); if (!success) { - context->throwError("Native call failed"); + context->throwError(QString("Unexpected: Native call of %1 failed").arg(fullName())); } return QVariant(); } else if (returnTypeId == scriptValueTypeId) { @@ -439,7 +506,7 @@ QVariant ScriptMethodQtProxy::extension(Extension extension, const QVariant& arg qGenArgs[1], qGenArgs[2], qGenArgs[3], qGenArgs[4], qGenArgs[5], qGenArgs[6], qGenArgs[7], qGenArgs[8], qGenArgs[9]); if (!success) { - context->throwError("Native call failed"); + context->throwError(QString("Unexpected: Native call of %1 failed").arg(fullName())); return QVariant(); } QScriptValue qResult = ScriptValueQtWrapper::fullUnwrap(_engine, result); @@ -454,15 +521,53 @@ QVariant ScriptMethodQtProxy::extension(Extension extension, const QVariant& arg meta.invoke(qobject, Qt::DirectConnection, sRetVal, qGenArgs[0], qGenArgs[1], qGenArgs[2], qGenArgs[3], qGenArgs[4], qGenArgs[5], qGenArgs[6], qGenArgs[7], qGenArgs[8], qGenArgs[9]); if (!success) { - context->throwError("Native call failed"); + context->throwError(QString("Unexpected: Native call of %1 failed").arg(fullName())); return QVariant(); } QScriptValue qResult = _engine->castVariantToValue(qRetVal); return QVariant::fromValue(qResult); } } - context->throwError("Native call failed: could not locate an overload with the requested arguments"); + + // we failed to convert the call to C++, try to create a somewhat sane error message + if (parameterConversionFailureCount == 0) { + context->throwError(QString("Native call of %1 failed: unexpected parameter count").arg(fullName())); + return QVariant(); + } + + const QMetaMethod& meta = _object->metaObject()->method(parameterConversionFailureId); + int methodNumArgs = meta.parameterCount(); + Q_ASSERT(methodNumArgs == numArgs); + + for (int arg = 0; arg < numArgs; ++arg) { + int methodArgTypeId = meta.parameterType(arg); + Q_ASSERT(methodArgTypeId != QMetaType::UnknownType); + QScriptValue argVal = context->argument(arg); + if (methodArgTypeId != scriptValueTypeId && methodArgTypeId != QMetaType::QVariant) { + QVariant varArgVal; + if (!_engine->castValueToVariant(argVal, varArgVal, methodArgTypeId)) { + QByteArray methodTypeName = QMetaType(methodArgTypeId).name(); + QByteArray argTypeName = _engine->valueType(argVal).toLatin1(); + context->throwError(QScriptContext::TypeError, QString("Native call of %1 failed: Cannot convert parameter %2 from %3 to %4") + .arg(fullName()).arg(arg+1).arg(argTypeName, methodTypeName)); + return QVariant(); + } + } + } + + Q_ASSERT(false); // really shouldn't have gotten here -- it didn't work before and it's working now? return QVariant(); + context->throwError(QString("Native call of %1 failed: could not locate an overload with the requested arguments").arg(fullName())); +} + +QString ScriptSignalQtProxy::fullName() const { + Q_ASSERT(_object); + if (!_object) return ""; + QString objectName = _object->objectName(); + if (!objectName.isEmpty()) { + return QString("%1.%2").arg(objectName, _meta.name()); + } + return QString("%1::%2").arg(_object->metaObject()->className(), _meta.name()); } // Adapted from https://doc.qt.io/archives/qq/qq16-dynamicqobject.html, for connecting to a signal without a compile-time definition for it @@ -476,6 +581,7 @@ int ScriptSignalQtProxy::qt_metacall(QMetaObject::Call call, int id, void** argu int numArgs = _meta.parameterCount(); for (int arg = 0; arg < numArgs; ++arg) { int methodArgTypeId = _meta.parameterType(arg); + Q_ASSERT(methodArgTypeId != QMetaType::UnknownType); QVariant argValue(methodArgTypeId, arguments[arg+1]); args.append(_engine->castVariantToValue(argValue)); } diff --git a/libraries/script-engine/src/qtscript/ScriptObjectQtProxy.h b/libraries/script-engine/src/qtscript/ScriptObjectQtProxy.h index efd9070bb3..a3844a8e53 100644 --- a/libraries/script-engine/src/qtscript/ScriptObjectQtProxy.h +++ b/libraries/script-engine/src/qtscript/ScriptObjectQtProxy.h @@ -73,7 +73,7 @@ public: // construction inline QObject* toQtValue() const { return _object; } public: // QScriptClass implementation - virtual QString name() const override { return _name; } + virtual QString name() const override; virtual QScriptValue property(const QScriptValue& object, const QScriptString& name, uint id) override; virtual QScriptValue::PropertyFlags propertyFlags(const QScriptValue& object, const QScriptString& name, uint id) override; @@ -92,7 +92,6 @@ private: // storage InstanceMap _signalInstances; const bool _ownsObject; QPointer _object; - QString _name; Q_DISABLE_COPY(ScriptObjectQtProxy) }; @@ -138,19 +137,21 @@ private: // storage class ScriptMethodQtProxy final : public QScriptClass { public: // construction - inline ScriptMethodQtProxy(ScriptEngineQtScript* engine, QObject* object, QScriptValue lifetime, QString name, const QList& metas) : - QScriptClass(engine), _engine(engine), _object(object), _objectLifetime(lifetime), _name(name), _metas(metas) {} + inline ScriptMethodQtProxy(ScriptEngineQtScript* engine, QObject* object, QScriptValue lifetime, const QList& metas) : + QScriptClass(engine), _engine(engine), _object(object), _objectLifetime(lifetime), _metas(metas) {} public: // QScriptClass implementation - virtual QString name() const override { return _name; } + virtual QString name() const override { return fullName(); } virtual bool supportsExtension(Extension extension) const override; virtual QVariant extension(Extension extension, const QVariant& argument = QVariant()) override; +private: + QString fullName() const; + private: // storage ScriptEngineQtScript* _engine; QPointer _object; QScriptValue _objectLifetime; - const QString _name; const QList _metas; Q_DISABLE_COPY(ScriptMethodQtProxy) @@ -175,13 +176,14 @@ private: // storage using ConnectionList = QList; public: // construction - inline ScriptSignalQtProxy(ScriptEngineQtScript* engine, QObject* object, QScriptValue lifetime, QString name, const QMetaMethod& meta) : - _engine(engine), _object(object), _objectLifetime(lifetime), _name(name), _meta(meta), _metaCallId(discoverMetaCallIdx()) {} + inline ScriptSignalQtProxy(ScriptEngineQtScript* engine, QObject* object, QScriptValue lifetime, const QMetaMethod& meta) : + _engine(engine), _object(object), _objectLifetime(lifetime), _meta(meta), _metaCallId(discoverMetaCallIdx()) {} private: // implementation virtual int qt_metacall(QMetaObject::Call call, int id, void** arguments); int discoverMetaCallIdx(); ConnectionList::iterator findConnection(QScriptValue thisObject, QScriptValue callback); + QString fullName() const; public: // API virtual void connect(QScriptValue arg0, QScriptValue arg1 = QScriptValue()) override; @@ -191,7 +193,6 @@ private: // storage ScriptEngineQtScript* _engine; QPointer _object; QScriptValue _objectLifetime; - QString _name; const QMetaMethod _meta; const int _metaCallId; ConnectionList _connections;