Merge pull request #3885 from birarda/double-scripts

fix for ScriptEngine blocked load

interface/src/Application.cpp

@@ -4013,26 +4013,23 @@ ScriptEngine* Application::loadScript(const QString& scriptFilename, bool isUser
         return _scriptEnginesHash[scriptURLString];
     }
 
-    ScriptEngine* scriptEngine;
-    if (scriptFilename.isNull()) {
-        scriptEngine = new ScriptEngine(NO_SCRIPT, "", &_controllerScriptingInterface);
-    } else {
-        // start the script on a new thread...
-        scriptEngine = new ScriptEngine(scriptUrl, &_controllerScriptingInterface);
-
-        if (!scriptEngine->hasScript()) {
-            qDebug() << "Application::loadScript(), script failed to load...";
-            QMessageBox::warning(getWindow(), "Error Loading Script", scriptURLString + " failed to load.");
-            return NULL;
-        }
-
-        _scriptEnginesHash.insertMulti(scriptURLString, scriptEngine);
-        _runningScriptsWidget->setRunningScripts(getRunningScripts());
-        UserActivityLogger::getInstance().loadedScript(scriptURLString);
-    }
+    ScriptEngine* scriptEngine = new ScriptEngine(NO_SCRIPT, "", &_controllerScriptingInterface);
     scriptEngine->setUserLoaded(isUserLoaded);
     
-    registerScriptEngineWithApplicationServices(scriptEngine);
+    if (scriptFilename.isNull()) {
+        // this had better be the script editor (we should de-couple so somebody who thinks they are loading a script
+        // doesn't just get an empty script engine)
+        
+        // we can complete setup now since there isn't a script we have to load
+        registerScriptEngineWithApplicationServices(scriptEngine);
+    } else {
+        // connect to the appropriate signals of this script engine
+        connect(scriptEngine, &ScriptEngine::scriptLoaded, this, &Application::handleScriptEngineLoaded);
+        connect(scriptEngine, &ScriptEngine::errorLoadingScript, this, &Application::handleScriptLoadError);
+        
+        // get the script engine object to load the script at the designated script URL
+        scriptEngine->loadURL(scriptUrl);
+    }
 
     // restore the main window's active state
     if (activateMainWindow && !loadScriptFromEditor) {
@@ -4043,6 +4040,22 @@ ScriptEngine* Application::loadScript(const QString& scriptFilename, bool isUser
     return scriptEngine;
 }
 
+void Application::handleScriptEngineLoaded(const QUrl& scriptURL) {
+    ScriptEngine* scriptEngine = qobject_cast<ScriptEngine*>(sender());
+    
+    _scriptEnginesHash.insertMulti(scriptURL.toString(), scriptEngine);
+    _runningScriptsWidget->setRunningScripts(getRunningScripts());
+    UserActivityLogger::getInstance().loadedScript(scriptURL.toString());
+    
+    // register our application services and set it off on its own thread
+    registerScriptEngineWithApplicationServices(scriptEngine);
+}
+
+void Application::handleScriptLoadError(const QUrl& scriptURL) {
+    qDebug() << "Application::loadScript(), script failed to load...";
+    QMessageBox::warning(getWindow(), "Error Loading Script", scriptURL.toString() + " failed to load.");
+}
+
 void Application::scriptFinished(const QString& scriptName) {
     const QString& scriptURLString = QUrl(scriptName).toString();
     QHash<QString, ScriptEngine*>::iterator it = _scriptEnginesHash.find(scriptURLString);

interface/src/Application.h

@@ -393,6 +393,9 @@ private slots:
     void timer();
     void idle();
     void aboutToQuit();
+    
+    void handleScriptEngineLoaded(const QUrl& scriptURL);
+    void handleScriptLoadError(const QUrl& scriptURL);
 
     void connectedToDomain(const QString& hostname);
 

libraries/script-engine/src/ScriptEngine.cpp

@@ -100,71 +100,6 @@ ScriptEngine::ScriptEngine(const QString& scriptContents, const QString& fileNam
 {
 }
 
-ScriptEngine::ScriptEngine(const QUrl& scriptURL,
-                           AbstractControllerScriptingInterface* controllerScriptingInterface)  :
-    _scriptContents(),
-    _isFinished(false),
-    _isRunning(false),
-    _isInitialized(false),
-    _isAvatar(false),
-    _avatarIdentityTimer(NULL),
-    _avatarBillboardTimer(NULL),
-    _timerFunctionMap(),
-    _isListeningToAudioStream(false),
-    _avatarSound(NULL),
-    _numAvatarSoundSentBytes(0),
-    _controllerScriptingInterface(controllerScriptingInterface),
-    _avatarData(NULL),
-    _scriptName(),
-    _fileNameString(),
-    _quatLibrary(),
-    _vec3Library(),
-    _uuidLibrary(),
-    _animationCache(this),
-    _isUserLoaded(false),
-    _arrayBufferClass(new ArrayBufferClass(this))
-{
-    QString scriptURLString = scriptURL.toString();
-    _fileNameString = scriptURLString;
-
-    QUrl url(scriptURL);
-    
-    // if the scheme length is one or lower, maybe they typed in a file, let's try
-    const int WINDOWS_DRIVE_LETTER_SIZE = 1;
-    if (url.scheme().size() <= WINDOWS_DRIVE_LETTER_SIZE) {
-        url = QUrl::fromLocalFile(scriptURLString);
-    }
-
-    // ok, let's see if it's valid... and if so, load it
-    if (url.isValid()) {
-        if (url.scheme() == "file") {
-            QString fileName = url.toLocalFile();
-            QFile scriptFile(fileName);
-            if (scriptFile.open(QFile::ReadOnly | QFile::Text)) {
-                qDebug() << "Loading file:" << fileName;
-                QTextStream in(&scriptFile);
-                _scriptContents = in.readAll();
-            } else {
-                qDebug() << "ERROR Loading file:" << fileName;
-                emit errorMessage("ERROR Loading file:" + fileName);
-            }
-        } else {
-            QNetworkAccessManager& networkAccessManager = NetworkAccessManager::getInstance();
-            QNetworkReply* reply = networkAccessManager.get(QNetworkRequest(url));
-            qDebug() << "Downloading script at" << url;
-            QEventLoop loop;
-            QObject::connect(reply, SIGNAL(finished()), &loop, SLOT(quit()));
-            loop.exec();
-            if (reply->error() == QNetworkReply::NoError && reply->attribute(QNetworkRequest::HttpStatusCodeAttribute) == 200) {
-                _scriptContents = reply->readAll();
-            } else {
-                qDebug() << "ERROR Loading file:" << url.toString();
-                emit errorMessage("ERROR Loading file:" + url.toString());
-            }
-        }
-    }
-}
-
 void ScriptEngine::setIsAvatar(bool isAvatar) {
     _isAvatar = isAvatar;
 
@@ -217,6 +152,56 @@ bool ScriptEngine::setScriptContents(const QString& scriptContents, const QStrin
     return true;
 }
 
+void ScriptEngine::loadURL(const QUrl& scriptURL) {
+    if (_isRunning) {
+        return;
+    }
+    
+    QString scriptURLString = scriptURL.toString();
+    _fileNameString = scriptURLString;
+    
+    QUrl url(scriptURL);
+    
+    // if the scheme length is one or lower, maybe they typed in a file, let's try
+    const int WINDOWS_DRIVE_LETTER_SIZE = 1;
+    if (url.scheme().size() <= WINDOWS_DRIVE_LETTER_SIZE) {
+        url = QUrl::fromLocalFile(scriptURLString);
+    }
+    
+    // ok, let's see if it's valid... and if so, load it
+    if (url.isValid()) {
+        if (url.scheme() == "file") {
+            QString fileName = url.toLocalFile();
+            QFile scriptFile(fileName);
+            if (scriptFile.open(QFile::ReadOnly | QFile::Text)) {
+                qDebug() << "Loading file:" << fileName;
+                QTextStream in(&scriptFile);
+                _scriptContents = in.readAll();
+                emit scriptLoaded(url);
+            } else {
+                qDebug() << "ERROR Loading file:" << fileName;
+                emit errorLoadingScript(url);
+            }
+        } else {
+            QNetworkAccessManager& networkAccessManager = NetworkAccessManager::getInstance();
+            QNetworkReply* reply = networkAccessManager.get(QNetworkRequest(url));
+            connect(reply, &QNetworkReply::finished, this, &ScriptEngine::handleScriptDownload);
+        }
+    }
+}
+
+void ScriptEngine::handleScriptDownload() {
+    QNetworkReply* reply = qobject_cast<QNetworkReply*>(sender());
+    
+    if (reply->error() == QNetworkReply::NoError && reply->attribute(QNetworkRequest::HttpStatusCodeAttribute) == 200) {
+        _scriptContents = reply->readAll();
+        emit scriptLoaded(reply->url());
+    } else {
+        qDebug() << "ERROR Loading file:" << reply->url().toString();
+        emit errorLoadingScript(reply->url());
+    }
+}
+
 Q_SCRIPT_DECLARE_QMETAOBJECT(LocalVoxels, QString)
 
 void ScriptEngine::init() {

libraries/script-engine/src/ScriptEngine.h

@@ -40,9 +40,6 @@ const unsigned int SCRIPT_DATA_CALLBACK_USECS = floor(((1.0 / 60.0f) * 1000 * 10
 class ScriptEngine : public QScriptEngine {
     Q_OBJECT
 public:
-    ScriptEngine(const QUrl& scriptURL,
-                 AbstractControllerScriptingInterface* controllerScriptingInterface = NULL);
-
     ScriptEngine(const QString& scriptContents = NO_SCRIPT,
                  const QString& fileNameString = QString(""),
                  AbstractControllerScriptingInterface* controllerScriptingInterface = NULL);
@@ -94,6 +91,7 @@ public:
     bool isUserLoaded() const { return _isUserLoaded; }
 
 public slots:
+    void loadURL(const QUrl& scriptURL);
     void stop();
 
     QScriptValue evaluate(const QString& program, const QString& fileName = QString(), int lineNumber = 1);
@@ -109,6 +107,8 @@ public slots:
     void nodeKilled(SharedNodePointer node);
 
 signals:
+    void scriptLoaded(const QUrl& scriptURL);
+    void errorLoadingScript(const QUrl& scriptURL);
     void update(float deltaTime);
     void scriptEnding();
     void finished(const QString& fileNameString);
@@ -155,6 +155,8 @@ private:
     ArrayBufferClass* _arrayBufferClass;
 
     QHash<QUuid, quint16> _outgoingScriptAudioSequenceNumbers;
+private slots:
+    void handleScriptDownload();
 };
 
 #endif // hifi_ScriptEngine_h