From 5099e68b563ecb7a5488212b1f53c103636c17c7 Mon Sep 17 00:00:00 2001 From: ksuprynowicz Date: Mon, 23 Jan 2023 00:08:49 +0100 Subject: [PATCH] V8 fixes, including calls and signals --- interface/src/ui/overlays/Overlays.cpp | 2 +- .../script-engine/src/ScriptValueUtils.cpp | 24 ++++++++++++++++- .../src/v8/ScriptContextV8Wrapper.cpp | 8 +++--- .../script-engine/src/v8/ScriptEngineV8.cpp | 3 +++ .../script-engine/src/v8/ScriptEngineV8.h | 2 +- .../src/v8/ScriptObjectV8Proxy.cpp | 15 +++++++---- .../src/v8/ScriptValueV8Wrapper.cpp | 26 ++++++++++++++++--- .../controllerModules/inEditMode.js | 4 +++ scripts/system/create/edit.js | 4 +-- .../system/create/entityList/entityList.js | 1 + .../system/libraries/networkingConstants.js | 3 ++- 11 files changed, 73 insertions(+), 19 deletions(-) diff --git a/interface/src/ui/overlays/Overlays.cpp b/interface/src/ui/overlays/Overlays.cpp index b7c13da84e..dec0e4e84e 100644 --- a/interface/src/ui/overlays/Overlays.cpp +++ b/interface/src/ui/overlays/Overlays.cpp @@ -642,7 +642,7 @@ EntityItemProperties Overlays::convertOverlayToEntityProperties(QVariantMap& ove ScriptEnginePointer scriptEngine = newScriptEngine(); ScriptValue props = variantMapToScriptValue(overlayProps, *scriptEngine); - qDebug() << "Overlay props: " << scriptEngine->scriptValueDebugDetails(props); + //qDebug() << "Overlay props: " << scriptEngine->scriptValueDebugDetails(props); EntityItemProperties toReturn; EntityItemPropertiesFromScriptValueHonorReadOnly(props, toReturn); return toReturn; diff --git a/libraries/script-engine/src/ScriptValueUtils.cpp b/libraries/script-engine/src/ScriptValueUtils.cpp index 4733dd7b72..f1c1461271 100644 --- a/libraries/script-engine/src/ScriptValueUtils.cpp +++ b/libraries/script-engine/src/ScriptValueUtils.cpp @@ -188,6 +188,7 @@ ScriptValue vec3ColorToScriptValue(ScriptEngine* engine, const glm::vec3& vec3) return value; } +// V8TODO: add similar checks to rest of the conversions bool vec3FromScriptValue(const ScriptValue& object, glm::vec3& vec3) { if (object.isNumber()) { vec3 = glm::vec3(object.toVariant().toFloat()); @@ -197,6 +198,8 @@ bool vec3FromScriptValue(const ScriptValue& object, glm::vec3& vec3) { vec3.x = qColor.red(); vec3.y = qColor.green(); vec3.z = qColor.blue(); + } else { + return false; } } else if (object.isArray()) { QVariantList list = object.toVariant().toList(); @@ -204,8 +207,10 @@ bool vec3FromScriptValue(const ScriptValue& object, glm::vec3& vec3) { vec3.x = list[0].toFloat(); vec3.y = list[1].toFloat(); vec3.z = list[2].toFloat(); + } else { + return false; } - } else { + } else if (object.isObject()) { ScriptValue x = object.property("x"); if (!x.isValid()) { x = object.property("r"); @@ -230,9 +235,15 @@ bool vec3FromScriptValue(const ScriptValue& object, glm::vec3& vec3) { z = object.property("blue"); } + if (!x.isValid() || !y.isValid() || !z.isValid()) { + return false; + } + vec3.x = x.toVariant().toFloat(); vec3.y = y.toVariant().toFloat(); vec3.z = z.toVariant().toFloat(); + } else { + return false; } return true; } @@ -446,7 +457,18 @@ ScriptValue quatToScriptValue(ScriptEngine* engine, const glm::quat& quat) { return obj; } +// V8TODO: add similar check to other conversions bool quatFromScriptValue(const ScriptValue& object, glm::quat& quat) { + if (!object.isObject()) { + return false; + } + QVariant x = object.property("x").toVariant(); + QVariant y = object.property("y").toVariant(); + QVariant z = object.property("z").toVariant(); + QVariant w = object.property("w").toVariant(); + if (!x.isValid() || !y.isValid() || !z.isValid() || !w.isValid()) { + return false; + } quat.x = object.property("x").toVariant().toFloat(); quat.y = object.property("y").toVariant().toFloat(); quat.z = object.property("z").toVariant().toFloat(); diff --git a/libraries/script-engine/src/v8/ScriptContextV8Wrapper.cpp b/libraries/script-engine/src/v8/ScriptContextV8Wrapper.cpp index 44d8338b0e..d02abbb308 100644 --- a/libraries/script-engine/src/v8/ScriptContextV8Wrapper.cpp +++ b/libraries/script-engine/src/v8/ScriptContextV8Wrapper.cpp @@ -99,10 +99,10 @@ QStringList ScriptContextV8Wrapper::backtrace() const { for (int i = 0; i < stackTrace->GetFrameCount(); i++) { v8::Local stackFrame = stackTrace->GetFrame(isolate, i); backTrace.append(QString(*v8::String::Utf8Value(isolate, stackFrame->GetScriptNameOrSourceURL())) + - QString(" ") + - QString(*v8::String::Utf8Value(isolate, stackFrame->GetFunctionName())) + - QString(":") + - QString(stackFrame->GetLineNumber()) + QString(" ") + + QString(*v8::String::Utf8Value(isolate, stackFrame->GetFunctionName())) + + QString(":") + + QStringLiteral("%1").arg(stackFrame->GetLineNumber()) ); } return backTrace; diff --git a/libraries/script-engine/src/v8/ScriptEngineV8.cpp b/libraries/script-engine/src/v8/ScriptEngineV8.cpp index 9fc46c15c4..2921141311 100644 --- a/libraries/script-engine/src/v8/ScriptEngineV8.cpp +++ b/libraries/script-engine/src/v8/ScriptEngineV8.cpp @@ -1185,6 +1185,9 @@ Q_INVOKABLE ScriptValue ScriptEngineV8::evaluate(const ScriptProgramPointer& pro raiseException(errorValue); maybeEmitUncaughtException("evaluate"); hasFailed = true; + } else { + // V8TODO this is just to check if run will always return false for uncaught exception + Q_ASSERT(!tryCatchRun.HasCaught()); } } if(!hasFailed) { diff --git a/libraries/script-engine/src/v8/ScriptEngineV8.h b/libraries/script-engine/src/v8/ScriptEngineV8.h index b622d8ef0f..660af501bf 100644 --- a/libraries/script-engine/src/v8/ScriptEngineV8.h +++ b/libraries/script-engine/src/v8/ScriptEngineV8.h @@ -182,6 +182,7 @@ public: // not for public use, but I don't like how Qt strings this along with p v8::Isolate* getIsolate() {return _v8Isolate;} v8::Local getContext(); const v8::Local getConstContext() const; + QString formatErrorMessageFromTryCatch(v8::TryCatch &tryCatch); // Useful for debugging //QStringList getCurrentStackTrace(); @@ -207,7 +208,6 @@ protected: static QMutex _v8InitMutex; static std::once_flag _v8InitOnceFlag; static v8::Platform* getV8Platform(); - QString formatErrorMessageFromTryCatch(v8::TryCatch &tryCatch); ScriptContextV8Pointer pushContext(v8::Local &context); void popContext(); diff --git a/libraries/script-engine/src/v8/ScriptObjectV8Proxy.cpp b/libraries/script-engine/src/v8/ScriptObjectV8Proxy.cpp index 95ceb67cb1..21c218868e 100644 --- a/libraries/script-engine/src/v8/ScriptObjectV8Proxy.cpp +++ b/libraries/script-engine/src/v8/ScriptObjectV8Proxy.cpp @@ -816,7 +816,7 @@ void ScriptMethodV8Proxy::call(const v8::FunctionCallbackInfo& argume QByteArray argTypeName = _engine->valueType(V8ScriptValue(isolate, argVal)).toLatin1(); QString errorMessage = QString("Native call of %1 failed: Cannot convert parameter %2 from %3 to %4") .arg(fullName()).arg(arg+1).arg(argTypeName, methodTypeName); - qDebug(scriptengine) << errorMessage; + qDebug(scriptengine) << errorMessage << "\n Backtrace:" << _engine->currentContext()->backtrace(); isolate->ThrowError(v8::String::NewFromUtf8(isolate, errorMessage.toStdString().c_str()).ToLocalChecked()); //context->throwError(V8ScriptContext::TypeError, QString("Native call of %1 failed: Cannot convert parameter %2 from %3 to %4") // .arg(fullName()).arg(arg+1).arg(argTypeName, methodTypeName)); @@ -1042,13 +1042,18 @@ int ScriptSignalV8Proxy::qt_metacall(QMetaObject::Call call, int id, void** argu Connection& conn = *iter; v8::Local callback = v8::Local::Cast(conn.callback.get()); v8::Local v8This; - if (conn.thisValue.get()->IsNull()) { - v8This = _engine->getContext()->Global(); - } else { + if (conn.thisValue.get()->IsObject()) { v8This = conn.thisValue.get(); + } else { + v8This = _engine->getContext()->Global(); } - //V8TODO: should there be a trycatch here? + + v8::TryCatch tryCatch(isolate); callback->Call(_engine->getContext(), v8This, numArgs, args); + if (tryCatch.HasCaught()) { + qCDebug(scriptengine) << "Signal proxy " << fullName() << " connection call failed: \"" + << _engine->formatErrorMessageFromTryCatch(tryCatch) << "This: " << conn.thisValue.get()->IsObject(); + } } return -1; diff --git a/libraries/script-engine/src/v8/ScriptValueV8Wrapper.cpp b/libraries/script-engine/src/v8/ScriptValueV8Wrapper.cpp index e0c40d1dca..c8122ac26f 100644 --- a/libraries/script-engine/src/v8/ScriptValueV8Wrapper.cpp +++ b/libraries/script-engine/src/v8/ScriptValueV8Wrapper.cpp @@ -71,7 +71,8 @@ ScriptValue ScriptValueV8Wrapper::call(const ScriptValue& thisObject, const Scri v8::Locker locker(isolate); v8::Isolate::Scope isolateScope(isolate); v8::HandleScope handleScope(isolate); - v8::Context::Scope contextScope(_engine->getContext()); + auto context = _engine->getContext(); + v8::Context::Scope contextScope(context); V8ScriptValue v8This = fullUnwrap(thisObject); //V8ScriptValueList qArgs; Q_ASSERT(args.length() <= Q_METAMETHOD_INVOKE_MAX_ARGS); @@ -81,10 +82,23 @@ ScriptValue ScriptValueV8Wrapper::call(const ScriptValue& thisObject, const Scri for (ScriptValueList::const_iterator iter = args.begin(); iter != args.end(); ++iter) { v8Args[argIndex++] = fullUnwrap(*iter).get(); } - //V8TODO should there be a v8 try-catch here? - // IsFunction check should be here probably + // V8TODO: IsFunction check should be here probably v8::Local v8Function = v8::Local::Cast(_value.get()); - auto maybeResult = v8Function->Call(_engine->getContext(), v8This.get(), args.length(), v8Args); + v8::TryCatch tryCatch(isolate); + v8::Local recv; + if (v8This.get()->IsObject()) { + recv = v8This.get(); + //qDebug() << "V8 This: " << _engine->scriptValueDebugDetailsV8(v8This); + }else{ + recv = _engine->getContext()->Global(); + //qDebug() << "global"; + } + //qDebug() << "V8 Call: " << *v8::String::Utf8Value(isolate, v8This.get()->TypeOf(isolate)); + auto maybeResult = v8Function->Call(_engine->getContext(), recv, args.length(), v8Args); + // V8TODO: Exceptions don't seem to actually be caught here? + if (tryCatch.HasCaught()) { + qCDebug(scriptengine) << "Function call failed: \"" << _engine->formatErrorMessageFromTryCatch(tryCatch); + } v8::Local result; if (maybeResult.ToLocal(&result)) { return ScriptValue(new ScriptValueV8Wrapper(_engine, V8ScriptValue(_engine->getIsolate(), result))); @@ -262,7 +276,11 @@ ScriptValue ScriptValueV8Wrapper::property(const QString& name, const ScriptValu qDebug() << "Failed to get property, parent of value: " << name << ", parent type: " << QString(*v8::String::Utf8Value(isolate, _value.constGet()->TypeOf(isolate))) << " parent value: " << parentValueQString; } } + if (name == QString("x")) { + printf("x"); + } qDebug() << "Failed to get property, parent of value: " << name << " is not a V8 object, reported type: " << QString(*v8::String::Utf8Value(isolate, _value.constGet()->TypeOf(isolate))); + qDebug() << "Backtrace: " << _engine->currentContext()->backtrace(); return _engine->undefinedValue(); /*v8::Local nullValue = v8::Null(_engine->getIsolate()); V8ScriptValue nullScriptValue(_engine->getIsolate(), std::move(nullValue)); diff --git a/scripts/system/controllers/controllerModules/inEditMode.js b/scripts/system/controllers/controllerModules/inEditMode.js index 8453a7d8d3..568d0b6c14 100644 --- a/scripts/system/controllers/controllerModules/inEditMode.js +++ b/scripts/system/controllers/controllerModules/inEditMode.js @@ -106,6 +106,10 @@ Script.include("/~/system/libraries/utils.js"); this.sendPointingAtData = function(controllerData) { var rayPick = controllerData.rayPicks[this.hand]; var hudRayPick = controllerData.hudRayPicks[this.hand]; + // V8TODO: this needs to be checked if it works correctly + if (!hudRayPick.intersects) { + return; + } var point2d = this.calculateNewReticlePosition(hudRayPick.intersection); var desktopWindow = Window.isPointOnDesktopWindow(point2d); var tablet = this.pointingAtTablet(rayPick.objectID); diff --git a/scripts/system/create/edit.js b/scripts/system/create/edit.js index 317b8c9d3f..eed471dc57 100644 --- a/scripts/system/create/edit.js +++ b/scripts/system/create/edit.js @@ -1682,8 +1682,8 @@ Script.scriptEnding.connect(function () { createButton = null; }); -var lastOrientation = null; -var lastPosition = null; +var lastOrientation = Camera.orientation; +var lastPosition = Camera.position; // Do some stuff regularly, like check for placement of various overlays Script.update.connect(function (deltaTime) { diff --git a/scripts/system/create/entityList/entityList.js b/scripts/system/create/entityList/entityList.js index 4892d9ee03..179239c916 100644 --- a/scripts/system/create/entityList/entityList.js +++ b/scripts/system/create/entityList/entityList.js @@ -297,6 +297,7 @@ var EntityListTool = function(shouldUseEditTabletApp) { } var onWebEventReceived = function(data) { + print("entityList.js onWebEventReceived: " + data); try { data = JSON.parse(data); } catch(e) { diff --git a/scripts/system/libraries/networkingConstants.js b/scripts/system/libraries/networkingConstants.js index 2a9c60f525..6613d0c81a 100644 --- a/scripts/system/libraries/networkingConstants.js +++ b/scripts/system/libraries/networkingConstants.js @@ -7,7 +7,8 @@ // See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html // Interface Metadata Source -var INTERFACE_METADATA_SOURCE = "https://cdn.vircadia.com/dist/launcher/vircadiaMeta.json"; +//var INTERFACE_METADATA_SOURCE = "https://cdn.vircadia.com/dist/launcher/vircadiaMeta.json"; +var INTERFACE_METADATA_SOURCE = ""; // Base CDN URLs var CONTENT_CDN_URL = Script.getExternalPath(Script.ExternalPaths.HF_Content, "/");