From 20022ebdc881d0884873b85e536373e31e255773 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Wed, 15 Mar 2017 17:44:35 -0700 Subject: [PATCH 1/5] use deleteLater for ESS script engine on nodeKilled --- assignment-client/src/scripts/EntityScriptServer.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/assignment-client/src/scripts/EntityScriptServer.cpp b/assignment-client/src/scripts/EntityScriptServer.cpp index f674ab006c..a47919bdd7 100644 --- a/assignment-client/src/scripts/EntityScriptServer.cpp +++ b/assignment-client/src/scripts/EntityScriptServer.cpp @@ -395,7 +395,8 @@ void EntityScriptServer::selectAudioFormat(const QString& selectedCodecName) { void EntityScriptServer::resetEntitiesScriptEngine() { auto engineName = QString("about:Entities %1").arg(++_entitiesScriptEngineCount); - auto newEngine = QSharedPointer(new ScriptEngine(ScriptEngine::ENTITY_SERVER_SCRIPT, NO_SCRIPT, engineName)); + auto newEngine = QSharedPointer(new ScriptEngine(ScriptEngine::ENTITY_SERVER_SCRIPT, NO_SCRIPT, engineName), + &ScriptEngine::deleteLater); auto webSocketServerConstructorValue = newEngine->newFunction(WebSocketServerClass::constructor); newEngine->globalObject().setProperty("WebSocketServer", webSocketServerConstructorValue); From a4d72c4e6766208ea323a2a8fe323758fe99234c Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Thu, 16 Mar 2017 10:29:22 -0700 Subject: [PATCH 2/5] make AudioScriptingInterface a Dependency --- .../src/scripts/EntityScriptServer.cpp | 2 ++ interface/src/Application.cpp | 21 ++++++++++--------- .../src/AudioScriptingInterface.cpp | 5 ----- .../src/AudioScriptingInterface.h | 9 +++++--- libraries/script-engine/src/ScriptEngine.cpp | 6 +++++- 5 files changed, 24 insertions(+), 19 deletions(-) diff --git a/assignment-client/src/scripts/EntityScriptServer.cpp b/assignment-client/src/scripts/EntityScriptServer.cpp index a47919bdd7..cea2d24534 100644 --- a/assignment-client/src/scripts/EntityScriptServer.cpp +++ b/assignment-client/src/scripts/EntityScriptServer.cpp @@ -58,6 +58,8 @@ EntityScriptServer::EntityScriptServer(ReceivedMessage& message) : ThreadedAssig DependencyManager::registerInheritance(); + DependencyManager::set(); + DependencyManager::set(); DependencyManager::set(); DependencyManager::set(); diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 02fe2a2dd3..9157bb7144 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -740,23 +740,24 @@ Application::Application(int& argc, char** argv, QElapsedTimer& startupTimer, bo } }); - auto& audioScriptingInterface = AudioScriptingInterface::getInstance(); + auto audioScriptingInterface = DependencyManager::set(); connect(audioThread, &QThread::started, audioIO.data(), &AudioClient::start); connect(audioIO.data(), &AudioClient::destroyed, audioThread, &QThread::quit); connect(audioThread, &QThread::finished, audioThread, &QThread::deleteLater); connect(audioIO.data(), &AudioClient::muteToggled, this, &Application::audioMuteToggled); - connect(audioIO.data(), &AudioClient::mutedByMixer, &audioScriptingInterface, &AudioScriptingInterface::mutedByMixer); - connect(audioIO.data(), &AudioClient::receivedFirstPacket, &audioScriptingInterface, &AudioScriptingInterface::receivedFirstPacket); - connect(audioIO.data(), &AudioClient::disconnected, &audioScriptingInterface, &AudioScriptingInterface::disconnected); + connect(audioIO.data(), &AudioClient::mutedByMixer, audioScriptingInterface.data(), &AudioScriptingInterface::mutedByMixer); + connect(audioIO.data(), &AudioClient::receivedFirstPacket, audioScriptingInterface.data(), &AudioScriptingInterface::receivedFirstPacket); + connect(audioIO.data(), &AudioClient::disconnected, audioScriptingInterface.data(), &AudioScriptingInterface::disconnected); connect(audioIO.data(), &AudioClient::muteEnvironmentRequested, [](glm::vec3 position, float radius) { auto audioClient = DependencyManager::get(); + auto audioScriptingInterface = DependencyManager::get(); auto myAvatarPosition = DependencyManager::get()->getMyAvatar()->getPosition(); float distance = glm::distance(myAvatarPosition, position); bool shouldMute = !audioClient->isMuted() && (distance < radius); if (shouldMute) { audioClient->toggleMute(); - AudioScriptingInterface::getInstance().environmentMuted(); + audioScriptingInterface->environmentMuted(); } }); @@ -1181,10 +1182,10 @@ Application::Application(int& argc, char** argv, QElapsedTimer& startupTimer, bo // set the local loopback interface for local sounds AudioInjector::setLocalAudioInterface(audioIO.data()); - AudioScriptingInterface::getInstance().setLocalAudioInterface(audioIO.data()); - connect(audioIO.data(), &AudioClient::noiseGateOpened, &AudioScriptingInterface::getInstance(), &AudioScriptingInterface::noiseGateOpened); - connect(audioIO.data(), &AudioClient::noiseGateClosed, &AudioScriptingInterface::getInstance(), &AudioScriptingInterface::noiseGateClosed); - connect(audioIO.data(), &AudioClient::inputReceived, &AudioScriptingInterface::getInstance(), &AudioScriptingInterface::inputReceived); + audioScriptingInterface->setLocalAudioInterface(audioIO.data()); + connect(audioIO.data(), &AudioClient::noiseGateOpened, audioScriptingInterface.data(), &AudioScriptingInterface::noiseGateOpened); + connect(audioIO.data(), &AudioClient::noiseGateClosed, audioScriptingInterface.data(), &AudioScriptingInterface::noiseGateClosed); + connect(audioIO.data(), &AudioClient::inputReceived, audioScriptingInterface.data(), &AudioScriptingInterface::inputReceived); this->installEventFilter(this); @@ -1949,7 +1950,7 @@ void Application::initializeUi() { // For some reason there is already an "Application" object in the QML context, // though I can't find it. Hence, "ApplicationInterface" rootContext->setContextProperty("ApplicationInterface", this); - rootContext->setContextProperty("Audio", &AudioScriptingInterface::getInstance()); + rootContext->setContextProperty("Audio", DependencyManager::get().data()); rootContext->setContextProperty("AudioStats", DependencyManager::get()->getStats().data()); rootContext->setContextProperty("AudioScope", DependencyManager::get().data()); diff --git a/libraries/script-engine/src/AudioScriptingInterface.cpp b/libraries/script-engine/src/AudioScriptingInterface.cpp index fcc1f201f9..8452494d95 100644 --- a/libraries/script-engine/src/AudioScriptingInterface.cpp +++ b/libraries/script-engine/src/AudioScriptingInterface.cpp @@ -19,11 +19,6 @@ void registerAudioMetaTypes(QScriptEngine* engine) { qScriptRegisterMetaType(engine, soundSharedPointerToScriptValue, soundSharedPointerFromScriptValue); } -AudioScriptingInterface& AudioScriptingInterface::getInstance() { - static AudioScriptingInterface staticInstance; - return staticInstance; -} - AudioScriptingInterface::AudioScriptingInterface() : _localAudioInterface(NULL) { diff --git a/libraries/script-engine/src/AudioScriptingInterface.h b/libraries/script-engine/src/AudioScriptingInterface.h index 6cce78d48f..e97bc329c6 100644 --- a/libraries/script-engine/src/AudioScriptingInterface.h +++ b/libraries/script-engine/src/AudioScriptingInterface.h @@ -14,18 +14,20 @@ #include #include +#include #include class ScriptAudioInjector; -class AudioScriptingInterface : public QObject { +class AudioScriptingInterface : public QObject, public Dependency { Q_OBJECT -public: - static AudioScriptingInterface& getInstance(); + SINGLETON_DEPENDENCY +public: void setLocalAudioInterface(AbstractAudioInterface* audioInterface) { _localAudioInterface = audioInterface; } protected: + // this method is protected to stop C++ callers from calling, but invokable from script Q_INVOKABLE ScriptAudioInjector* playSound(SharedSoundPointer sound, const AudioInjectorOptions& injectorOptions = AudioInjectorOptions()); @@ -42,6 +44,7 @@ signals: private: AudioScriptingInterface(); + AbstractAudioInterface* _localAudioInterface; }; diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp index 3e4e070717..1b3d5057f5 100644 --- a/libraries/script-engine/src/ScriptEngine.cpp +++ b/libraries/script-engine/src/ScriptEngine.cpp @@ -627,6 +627,9 @@ void ScriptEngine::init() { qScriptRegisterMetaType(this, qWSCloseCodeToScriptValue, qWSCloseCodeFromScriptValue); qScriptRegisterMetaType(this, wscReadyStateToScriptValue, wscReadyStateFromScriptValue); + // NOTE: You do not want to end up creating new instances of singletons here. They will be on the ScriptEngine thread + // and are likely to be unusable if we "reset" the ScriptEngine by creating a new one (on a whole new thread). + registerGlobalObject("Script", this); { @@ -638,7 +641,8 @@ void ScriptEngine::init() { resetModuleCache(); } - registerGlobalObject("Audio", &AudioScriptingInterface::getInstance()); + registerGlobalObject("Audio", DependencyManager::get().data()); + registerGlobalObject("Entities", entityScriptingInterface.data()); registerGlobalObject("Quat", &_quatLibrary); registerGlobalObject("Vec3", &_vec3Library); From c63a2c9cda223f8eeba2ed13c3dbf778ba57cd2b Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Thu, 16 Mar 2017 15:07:54 -0700 Subject: [PATCH 3/5] remove requirement of ScriptEngine in ScriptEngines slots --- libraries/script-engine/src/ScriptEngine.cpp | 8 ++++---- libraries/script-engine/src/ScriptEngine.h | 8 ++++---- libraries/script-engine/src/ScriptEngines.cpp | 20 +++++-------------- libraries/script-engine/src/ScriptEngines.h | 10 +++++----- 4 files changed, 18 insertions(+), 28 deletions(-) diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp index 1b3d5057f5..a5c94c1bb4 100644 --- a/libraries/script-engine/src/ScriptEngine.cpp +++ b/libraries/script-engine/src/ScriptEngine.cpp @@ -464,17 +464,17 @@ void ScriptEngine::loadURL(const QUrl& scriptURL, bool reload) { void ScriptEngine::scriptErrorMessage(const QString& message) { qCCritical(scriptengine) << qPrintable(message); - emit errorMessage(message); + emit errorMessage(message, getFilename()); } void ScriptEngine::scriptWarningMessage(const QString& message) { qCWarning(scriptengine) << message; - emit warningMessage(message); + emit warningMessage(message, getFilename()); } void ScriptEngine::scriptInfoMessage(const QString& message) { qCInfo(scriptengine) << message; - emit infoMessage(message); + emit infoMessage(message, getFilename()); } // Even though we never pass AnimVariantMap directly to and from javascript, the queued invokeMethod of @@ -1351,7 +1351,7 @@ QUrl ScriptEngine::resourcesPath() const { } void ScriptEngine::print(const QString& message) { - emit printedMessage(message); + emit printedMessage(message, getFilename()); } // Script.require.resolve -- like resolvePath, but performs more validation and throws exceptions on invalid module identifiers (for consistency with Node.js) diff --git a/libraries/script-engine/src/ScriptEngine.h b/libraries/script-engine/src/ScriptEngine.h index 95f377d92b..5ea8d052e9 100644 --- a/libraries/script-engine/src/ScriptEngine.h +++ b/libraries/script-engine/src/ScriptEngine.h @@ -236,10 +236,10 @@ signals: void scriptEnding(); void finished(const QString& fileNameString, ScriptEngine* engine); void cleanupMenuItem(const QString& menuItemString); - void printedMessage(const QString& message); - void errorMessage(const QString& message); - void warningMessage(const QString& message); - void infoMessage(const QString& message); + void printedMessage(const QString& message, const QString& scriptName); + void errorMessage(const QString& message, const QString& scriptName); + void warningMessage(const QString& message, const QString& scriptName); + void infoMessage(const QString& message, const QString& scriptName); void runningStateChanged(); void loadScript(const QString& scriptName, bool isUserLoaded); void reloadScript(const QString& scriptName, bool isUserLoaded); diff --git a/libraries/script-engine/src/ScriptEngines.cpp b/libraries/script-engine/src/ScriptEngines.cpp index 57887d2d96..88b0e0b7b5 100644 --- a/libraries/script-engine/src/ScriptEngines.cpp +++ b/libraries/script-engine/src/ScriptEngines.cpp @@ -34,34 +34,24 @@ ScriptsModel& getScriptsModel() { return scriptsModel; } -void ScriptEngines::onPrintedMessage(const QString& message) { - auto scriptEngine = qobject_cast(sender()); - auto scriptName = scriptEngine ? scriptEngine->getFilename() : ""; +void ScriptEngines::onPrintedMessage(const QString& message, const QString& scriptName) { emit printedMessage(message, scriptName); } -void ScriptEngines::onErrorMessage(const QString& message) { - auto scriptEngine = qobject_cast(sender()); - auto scriptName = scriptEngine ? scriptEngine->getFilename() : ""; +void ScriptEngines::onErrorMessage(const QString& message, const QString& scriptName) { emit errorMessage(message, scriptName); } -void ScriptEngines::onWarningMessage(const QString& message) { - auto scriptEngine = qobject_cast(sender()); - auto scriptName = scriptEngine ? scriptEngine->getFilename() : ""; +void ScriptEngines::onWarningMessage(const QString& message, const QString& scriptName) { emit warningMessage(message, scriptName); } -void ScriptEngines::onInfoMessage(const QString& message) { - auto scriptEngine = qobject_cast(sender()); - auto scriptName = scriptEngine ? scriptEngine->getFilename() : ""; +void ScriptEngines::onInfoMessage(const QString& message, const QString& scriptName) { emit infoMessage(message, scriptName); } void ScriptEngines::onErrorLoadingScript(const QString& url) { - auto scriptEngine = qobject_cast(sender()); - auto scriptName = scriptEngine ? scriptEngine->getFilename() : ""; - emit errorLoadingScript(url, scriptName); + emit errorLoadingScript(url); } ScriptEngines::ScriptEngines(ScriptEngine::Context context) diff --git a/libraries/script-engine/src/ScriptEngines.h b/libraries/script-engine/src/ScriptEngines.h index 2fadfc81f8..63b7e8f11c 100644 --- a/libraries/script-engine/src/ScriptEngines.h +++ b/libraries/script-engine/src/ScriptEngines.h @@ -79,13 +79,13 @@ signals: void errorMessage(const QString& message, const QString& engineName); void warningMessage(const QString& message, const QString& engineName); void infoMessage(const QString& message, const QString& engineName); - void errorLoadingScript(const QString& url, const QString& engineName); + void errorLoadingScript(const QString& url); public slots: - void onPrintedMessage(const QString& message); - void onErrorMessage(const QString& message); - void onWarningMessage(const QString& message); - void onInfoMessage(const QString& message); + void onPrintedMessage(const QString& message, const QString& scriptName); + void onErrorMessage(const QString& message, const QString& scriptName); + void onWarningMessage(const QString& message, const QString& scriptName); + void onInfoMessage(const QString& message, const QString& scriptName); void onErrorLoadingScript(const QString& url); protected slots: From 0a118bd268d0033fd35ce6c99891b7bb9092379d Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Thu, 16 Mar 2017 18:41:26 -0700 Subject: [PATCH 4/5] add a guard for trading entity servers by checking node count --- .../src/scripts/EntityScriptServer.cpp | 21 ++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/assignment-client/src/scripts/EntityScriptServer.cpp b/assignment-client/src/scripts/EntityScriptServer.cpp index cea2d24534..c46d658c52 100644 --- a/assignment-client/src/scripts/EntityScriptServer.cpp +++ b/assignment-client/src/scripts/EntityScriptServer.cpp @@ -326,7 +326,26 @@ void EntityScriptServer::nodeActivated(SharedNodePointer activatedNode) { void EntityScriptServer::nodeKilled(SharedNodePointer killedNode) { switch (killedNode->getType()) { case NodeType::EntityServer: { - clear(); + // Before we clear, make sure this was our only entity server. + // Otherwise we're assuming that we have "trading" entity servers + // (an old one going away and a new one coming onboard) + // and that we shouldn't clear here because we're still doing work. + bool hasAnotherEntityServer = false; + auto nodeList = DependencyManager::get(); + + nodeList->eachNodeBreakable([&hasAnotherEntityServer, &killedNode](const SharedNodePointer& node){ + if (node->getType() == NodeType::EntityServer && node->getUUID() != killedNode->getUUID()) { + // we're talking to > 1 entity servers, we know we won't clear + hasAnotherEntityServer = true; + return false; + } + + return true; + }); + + if (hasAnotherEntityServer) { + clear(); + } break; } From 96cf6b8c0418ca4e8548792d921405d70c81db30 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Fri, 17 Mar 2017 13:19:59 -0700 Subject: [PATCH 5/5] fix clear check for missing other entity servers --- assignment-client/src/scripts/EntityScriptServer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/assignment-client/src/scripts/EntityScriptServer.cpp b/assignment-client/src/scripts/EntityScriptServer.cpp index c46d658c52..954c25a342 100644 --- a/assignment-client/src/scripts/EntityScriptServer.cpp +++ b/assignment-client/src/scripts/EntityScriptServer.cpp @@ -343,7 +343,7 @@ void EntityScriptServer::nodeKilled(SharedNodePointer killedNode) { return true; }); - if (hasAnotherEntityServer) { + if (!hasAnotherEntityServer) { clear(); }