From e680b5dc724412684eb4e08fd540ebd51c6eaf7a Mon Sep 17 00:00:00 2001 From: ksuprynowicz Date: Sun, 20 Nov 2022 17:45:25 +0100 Subject: [PATCH] Fixed method calls --- libraries/script-engine/src/Vec3.cpp | 4 + libraries/script-engine/src/Vec3.h | 4 + .../script-engine/src/v8/ScriptEngineV8.h | 4 +- .../src/v8/ScriptObjectV8Proxy.cpp | 123 +++++++++++++----- .../src/v8/ScriptObjectV8Proxy.h | 6 +- 5 files changed, 108 insertions(+), 33 deletions(-) diff --git a/libraries/script-engine/src/Vec3.cpp b/libraries/script-engine/src/Vec3.cpp index 84df545bd2..b50aabf7df 100644 --- a/libraries/script-engine/src/Vec3.cpp +++ b/libraries/script-engine/src/Vec3.cpp @@ -23,6 +23,10 @@ #include "ScriptEngineLogging.h" #include "ScriptManager.h" +Vec3::~Vec3() { + qDebug(scriptengine) << "ScriptMethodV8Proxy destroyed"; + printf("ScriptMethodV8Proxy destroyed"); +} float Vec3::orientedAngle(const glm::vec3& v1, const glm::vec3& v2, const glm::vec3& v3) { float radians = glm::orientedAngle(glm::normalize(v1), glm::normalize(v2), glm::normalize(v3)); diff --git a/libraries/script-engine/src/Vec3.h b/libraries/script-engine/src/Vec3.h index cbb7055a95..7d44e69c21 100644 --- a/libraries/script-engine/src/Vec3.h +++ b/libraries/script-engine/src/Vec3.h @@ -418,6 +418,10 @@ private: const glm::vec3& RIGHT() { return Vectors::RIGHT; } const glm::vec3& UP() { return Vectors::UP; } const glm::vec3& FRONT() { return Vectors::FRONT; } + + //V8TODO delete after V8 works - used only for debugging +public: + virtual ~Vec3(); }; #endif // hifi_Vec3_h diff --git a/libraries/script-engine/src/v8/ScriptEngineV8.h b/libraries/script-engine/src/v8/ScriptEngineV8.h index fcdceae7cd..f0161e1d8a 100644 --- a/libraries/script-engine/src/v8/ScriptEngineV8.h +++ b/libraries/script-engine/src/v8/ScriptEngineV8.h @@ -184,7 +184,9 @@ public: // not for public use, but I don't like how Qt strings this along with p using ObjectWrapperMap = QMap>; mutable QMutex _qobjectWrapperMapProtect; ObjectWrapperMap _qobjectWrapperMap; - + // Second map, from which wrappers are removed by script engine upon deletion + // V8TODO add a V8 callback that removes pointer from the map so that it gets deleted + QMap> _qobjectWrapperMapV8; protected: // like `newFunction`, but allows mapping inline C++ lambdas with captures as callable V8ScriptValues diff --git a/libraries/script-engine/src/v8/ScriptObjectV8Proxy.cpp b/libraries/script-engine/src/v8/ScriptObjectV8Proxy.cpp index 1f8f66fd3b..76db368169 100644 --- a/libraries/script-engine/src/v8/ScriptObjectV8Proxy.cpp +++ b/libraries/script-engine/src/v8/ScriptObjectV8Proxy.cpp @@ -33,7 +33,7 @@ Q_DECLARE_METATYPE(QSharedPointer) static const void *internalPointsToQObjectProxy = (void *)0x13370000; static const void *internalPointsToQVariantProxy = (void *)0x13371000; static const void *internalPointsToSignalProxy = (void *)0x13372000; -static const void *internalPointsToMethodProxy = (void *)0x13372000; +static const void *internalPointsToMethodProxy = (void *)0x13373000; // Used strictly to replace the "this" object value for property access. May expand to a full context element // if we find it necessary to, but hopefully not needed @@ -114,9 +114,10 @@ V8ScriptValue ScriptObjectV8Proxy::newQObject(ScriptEngineV8* engine, QObject* o return V8ScriptValue(engine->getIsolate(), proxy.get()->toV8Value()); } } - + // V8TODO add a V8 callback that removes pointer from the map so that it gets deleted // register the wrapper with the engine and make sure it cleans itself up engine->_qobjectWrapperMap.insert(object, proxy); + engine->_qobjectWrapperMapV8.insert(object, proxy); QPointer enginePtr = engine; object->connect(object, &QObject::destroyed, engine, [enginePtr, object]() { if (!enginePtr) return; @@ -137,13 +138,16 @@ ScriptObjectV8Proxy* ScriptObjectV8Proxy::unwrapProxy(const V8ScriptValue& val) v8::HandleScope handleScope(const_cast(val.constGetIsolate())); auto v8Value = val.constGet(); if (!v8Value->IsObject()) { + qDebug(scriptengine) << "Cannot unwrap proxy - value is not an object"; return nullptr; } v8::Local v8Object = v8::Local::Cast(v8Value); if (v8Object->InternalFieldCount() != 2) { + qDebug(scriptengine) << "Cannot unwrap proxy - wrong number of internal fields"; return nullptr; } - if (v8Object->GetAlignedPointerFromInternalField(0) == internalPointsToQObjectProxy) { + if (v8Object->GetAlignedPointerFromInternalField(0) != internalPointsToQObjectProxy) { + qDebug(scriptengine) << "Cannot unwrap proxy - internal fields don't point to object proxy"; return nullptr; } return reinterpret_cast(v8Object->GetAlignedPointerFromInternalField(1)); @@ -155,6 +159,7 @@ QObject* ScriptObjectV8Proxy::unwrap(const V8ScriptValue& val) { } ScriptObjectV8Proxy::~ScriptObjectV8Proxy() { + qDebug(scriptengine) << "Deleting object proxy: " << name(); if (_ownsObject) { QObject* qobject = _object; if(qobject) qobject->deleteLater(); @@ -170,11 +175,18 @@ void ScriptObjectV8Proxy::investigate() { v8::Context::Scope contextScope(_engine->getContext()); auto objectTemplate = _v8ObjectTemplate.Get(_engine->getIsolate()); - objectTemplate->SetInternalFieldCount(3); + objectTemplate->SetInternalFieldCount(2); objectTemplate->SetHandler(v8::NamedPropertyHandlerConfiguration(v8Get, v8Set)); const QMetaObject* metaObject = qobject->metaObject(); + qDebug(scriptengine) << "Investigate: " << metaObject->className(); + if (QString("Vec3") == metaObject->className()) { + printf("Vec3"); + } + if (QString("ConsoleScriptingInterface") == metaObject->className()) { + printf("ConsoleScriptingInterface"); + } // discover properties int startIdx = _wrapOptions & ScriptEngine::ExcludeSuperClassProperties ? metaObject->propertyOffset() : 0; int num = metaObject->propertyCount(); @@ -182,6 +194,7 @@ void ScriptObjectV8Proxy::investigate() { QMetaProperty prop = metaObject->property(idx); if (!prop.isScriptable()) continue; + qDebug(scriptengine) << "Investigate: " << metaObject->className() << " Property: " << prop.name(); // always exclude child objects (at least until we decide otherwise) int metaTypeId = prop.userType(); if (metaTypeId != QMetaType::UnknownType) { @@ -204,6 +217,7 @@ void ScriptObjectV8Proxy::investigate() { QHash methodNames; for (int idx = startIdx; idx < num; ++idx) { QMetaMethod method = metaObject->method(idx); + qDebug(scriptengine) << "Investigate: " << metaObject->className() << " Method: " << method.name(); // perhaps keep this comment? Calls (like AudioScriptingInterface::playSound) seem to expect non-public methods to be script-accessible /* if (method.access() != QMetaMethod::Public) continue;*/ @@ -281,7 +295,7 @@ void ScriptObjectV8Proxy::investigate() { v8::Local v8Object = objectTemplate->NewInstance(_engine->getContext()).ToLocalChecked(); v8Object->SetAlignedPointerInInternalField(0, const_cast(internalPointsToQObjectProxy)); v8Object->SetAlignedPointerInInternalField(1, reinterpret_cast(this)); - + _v8Object.Reset(_engine->getIsolate(), v8Object); } @@ -295,24 +309,31 @@ QString ScriptObjectV8Proxy::name() const { } ScriptObjectV8Proxy::QueryFlags ScriptObjectV8Proxy::queryProperty(const V8ScriptValue& object, const V8ScriptString& name, QueryFlags flags, uint* id) { - // check for properties - for (PropertyDefMap::const_iterator trans = _props.cbegin(); trans != _props.cend(); ++trans) { - const PropertyDef& propDef = trans.value(); - if (propDef.name.constGet() != name.constGet()) continue; - *id = trans.key() | PROPERTY_TYPE; - return flags & (HandlesReadAccess | HandlesWriteAccess); - } + v8::HandleScope handleScope(_engine->getIsolate()); + // V8TODO: this might be inefficient when there's large number of properties + v8::Local context = _engine->getContext(); + v8::String::Utf8Value nameStr(_engine->getIsolate(), name.constGet()); // check for methods for (MethodDefMap::const_iterator trans = _methods.cbegin(); trans != _methods.cend(); ++trans) { - if (trans.value().name.constGet() != name.constGet()) continue; + v8::String::Utf8Value methodNameStr(_engine->getIsolate(), trans.value().name.constGet()); + qDebug(scriptengine) << "queryProperty : " << *nameStr << " method: " << *methodNameStr; + if (!(trans.value().name == name)) continue; *id = trans.key() | METHOD_TYPE; return flags & (HandlesReadAccess | HandlesWriteAccess); } + // check for properties + for (PropertyDefMap::const_iterator trans = _props.cbegin(); trans != _props.cend(); ++trans) { + const PropertyDef& propDef = trans.value(); + if (!(propDef.name == name)) continue; + *id = trans.key() | PROPERTY_TYPE; + return flags & (HandlesReadAccess | HandlesWriteAccess); + } + // check for signals for (SignalDefMap::const_iterator trans = _signals.cbegin(); trans != _signals.cend(); ++trans) { - if (trans.value().name.constGet() != name.constGet()) continue; + if (!(trans.value().name == name)) continue; *id = trans.key() | SIGNAL_TYPE; return flags & (HandlesReadAccess | HandlesWriteAccess); } @@ -348,17 +369,51 @@ ScriptValue::PropertyFlags ScriptObjectV8Proxy::propertyFlags(const V8ScriptValu } void ScriptObjectV8Proxy::v8Get(v8::Local name, const v8::PropertyCallbackInfo& info) { - // V8TODO - //info.GetReturnValue().Set(); + v8::HandleScope handleScope(info.GetIsolate()); + v8::String::Utf8Value utf8Value(info.GetIsolate(), name); + qDebug(scriptengine) << "Get: " << *utf8Value; + V8ScriptValue object(info.GetIsolate(), info.This()); + ScriptObjectV8Proxy *proxy = ScriptObjectV8Proxy::unwrapProxy(object); + if (!proxy) { + qDebug(scriptengine) << "Proxy object not found when getting: " << *utf8Value; + return; + } + V8ScriptString nameString(info.GetIsolate(), v8::Local::Cast(name)); + uint id; + QueryFlags flags = proxy->queryProperty(object, nameString, HandlesReadAccess, &id); + if (flags) { + V8ScriptValue value = proxy->property(object, nameString, id); + info.GetReturnValue().Set(value.get()); + } else { + qDebug(scriptengine) << "Value not found: " << *utf8Value; + } } void ScriptObjectV8Proxy::v8Set(v8::Local name, v8::Local value, const v8::PropertyCallbackInfo& info) { - // V8TODO - info.GetReturnValue().Set(value); + v8::HandleScope handleScope(info.GetIsolate()); + v8::String::Utf8Value utf8Value(info.GetIsolate(), name); + qDebug(scriptengine) << "Set: " << *utf8Value; + V8ScriptValue object(info.GetIsolate(), info.This()); + ScriptObjectV8Proxy *proxy = ScriptObjectV8Proxy::unwrapProxy(object); + if (!proxy) { + qDebug(scriptengine) << "Proxy object not found when setting: " << *utf8Value; + return; + } + V8ScriptString nameString(info.GetIsolate(), v8::Local::Cast(name)); + //V8ScriptString nameString(info.GetIsolate(), name->ToString(proxy->_engine->getContext()).ToLocalChecked()); + uint id; + QueryFlags flags = proxy->queryProperty(object, nameString, HandlesWriteAccess, &id); + if (flags) { + proxy->setProperty(object, nameString, id, V8ScriptValue(info.GetIsolate(), value)); + info.GetReturnValue().Set(value); + } else { + qDebug(scriptengine) << "Value not found: " << *utf8Value; + } } V8ScriptValue ScriptObjectV8Proxy::property(const V8ScriptValue& object, const V8ScriptString& name, uint id) { + v8::HandleScope handleScope(_engine->getIsolate()); QObject* qobject = _object; if (!qobject) { _engine->getIsolate()->ThrowError("Referencing deleted native object"); @@ -390,7 +445,7 @@ V8ScriptValue ScriptObjectV8Proxy::property(const V8ScriptValue& object, const V if((*iter).returnType() == QMetaType::UnknownType) { qDebug(scriptengine) << "Method with QMetaType::UnknownType " << metaObject->className() << " " << (*iter).name(); } - } + } //V8TODO: is new method created during every call? It needs to be cached instead return ScriptMethodV8Proxy::newMethod(_engine, qobject, object, methodDef.methods, methodDef.numMaxParms); } case SIGNAL_TYPE: { @@ -417,6 +472,7 @@ V8ScriptValue ScriptObjectV8Proxy::property(const V8ScriptValue& object, const V } void ScriptObjectV8Proxy::setProperty(V8ScriptValue& object, const V8ScriptString& name, uint id, const V8ScriptValue& value) { + v8::HandleScope handleScope(_engine->getIsolate()); if (!(id & PROPERTY_TYPE)) return; QObject* qobject = _object; if (!qobject) { @@ -483,7 +539,7 @@ ScriptVariantV8Proxy* ScriptVariantV8Proxy::unwrapProxy(const V8ScriptValue& val if (v8Object->InternalFieldCount() != 2) { return nullptr; } - if (v8Object->GetAlignedPointerFromInternalField(0) == internalPointsToQVariantProxy) { + if (v8Object->GetAlignedPointerFromInternalField(0) != internalPointsToQVariantProxy) { return nullptr; } return reinterpret_cast(v8Object->GetAlignedPointerFromInternalField(1)); @@ -495,8 +551,13 @@ QVariant ScriptVariantV8Proxy::unwrap(const V8ScriptValue& val) { } ScriptMethodV8Proxy::ScriptMethodV8Proxy(ScriptEngineV8* engine, QObject* object, V8ScriptValue lifetime, - const QList& metas, int numMaxParms) : - _numMaxParms(numMaxParms), _engine(engine), _object(object), _objectLifetime(lifetime), _metas(metas) { + const QList& metas, int numMaxParams) : + _numMaxParams(numMaxParams), _engine(engine), _object(object), _objectLifetime(lifetime), _metas(metas) { +} + +ScriptMethodV8Proxy::~ScriptMethodV8Proxy() { + qDebug(scriptengine) << "ScriptMethodV8Proxy destroyed"; + printf("ScriptMethodV8Proxy destroyed"); } V8ScriptValue ScriptMethodV8Proxy::newMethod(ScriptEngineV8* engine, QObject* object, V8ScriptValue lifetime, @@ -534,23 +595,25 @@ QString ScriptMethodV8Proxy::fullName() const { }*/ void ScriptMethodV8Proxy::callback(const v8::FunctionCallbackInfo& arguments) { - if (!arguments.This()->IsObject()) { + v8::HandleScope handleScope(arguments.GetIsolate()); + if (!arguments.Data()->IsObject()) { arguments.GetIsolate()->ThrowError("Method value is not an object"); return; } - if (!arguments.This()->IsCallable()) { + v8::Local data = v8::Local::Cast(arguments.Data()); + /*if (!arguments.Data()->IsCallable()) { arguments.GetIsolate()->ThrowError("Method value is not callable"); return; - } - if (arguments.This()->InternalFieldCount() != 2) { + }*/ + if (data->InternalFieldCount() != 2) { arguments.GetIsolate()->ThrowError("Incorrect number of internal fields during method call"); return; } - if (arguments.This()->GetAlignedPointerFromInternalField(0) == internalPointsToMethodProxy) { + if (data->GetAlignedPointerFromInternalField(0) != internalPointsToMethodProxy) { arguments.GetIsolate()->ThrowError("Internal field 0 of ScriptMethodV8Proxy V8 object has wrong value"); return; } - ScriptMethodV8Proxy *proxy = reinterpret_cast(arguments.This()->GetAlignedPointerFromInternalField(1)); + ScriptMethodV8Proxy *proxy = reinterpret_cast(data->GetAlignedPointerFromInternalField(1)); proxy->call(arguments); } @@ -565,7 +628,7 @@ void ScriptMethodV8Proxy::call(const v8::FunctionCallbackInfo& argume v8::HandleScope handleScope(_engine->getIsolate()); int scriptNumArgs = arguments.Length(); - int numArgs = std::min(scriptNumArgs, _numMaxParms); + int numArgs = std::min(scriptNumArgs, _numMaxParams); const int scriptValueTypeId = qMetaTypeId(); @@ -739,7 +802,7 @@ void ScriptMethodV8Proxy::call(const v8::FunctionCallbackInfo& argume } int scriptNumArgs = context->argumentCount(); - int numArgs = std::min(scriptNumArgs, _numMaxParms); + int numArgs = std::min(scriptNumArgs, _numMaxParams); const int scriptValueTypeId = qMetaTypeId(); diff --git a/libraries/script-engine/src/v8/ScriptObjectV8Proxy.h b/libraries/script-engine/src/v8/ScriptObjectV8Proxy.h index 22bd9618bd..165152648d 100644 --- a/libraries/script-engine/src/v8/ScriptObjectV8Proxy.h +++ b/libraries/script-engine/src/v8/ScriptObjectV8Proxy.h @@ -114,6 +114,7 @@ private: // storage v8::UniquePersistent _v8ObjectTemplate; // V8TODO Maybe it doesn't really need to point to itsef? v8::UniquePersistent _v8Object; + int pointerCorruptionTest = 12345678; Q_DISABLE_COPY(ScriptObjectV8Proxy) }; @@ -167,7 +168,8 @@ private: // storage class ScriptMethodV8Proxy final { public: // construction ScriptMethodV8Proxy(ScriptEngineV8* engine, QObject* object, V8ScriptValue lifetime, - const QList& metas, int numMaxParms); + const QList& metas, int numMaxParams); + virtual ~ScriptMethodV8Proxy(); public: // QScriptClass implementation virtual QString name() const { return fullName(); } @@ -182,7 +184,7 @@ private: QString fullName() const; private: // storage - const int _numMaxParms; + const int _numMaxParams; ScriptEngineV8* _engine; QPointer _object; V8ScriptValue _objectLifetime;