Merge pull request #3098 from ctrlaltdavid/19803

Code Review for Job #19803

interface/src/Application.cpp

@@ -3628,7 +3628,8 @@ ScriptEngine* Application::loadScript(const QString& scriptName, bool loadScript
 }
 
 void Application::scriptFinished(const QString& scriptName) {
-    QHash<QString, ScriptEngine*>::iterator it = _scriptEnginesHash.find(scriptName);
+    const QString& scriptURLString = QUrl(scriptName).toString();
+    QHash<QString, ScriptEngine*>::iterator it = _scriptEnginesHash.find(scriptURLString);
     if (it != _scriptEnginesHash.end()) {
         _scriptEnginesHash.erase(it);
         _runningScriptsWidget->scriptStopped(scriptName);

interface/src/ui/ScriptEditorWidget.cpp

@@ -28,8 +28,12 @@
 
 ScriptEditorWidget::ScriptEditorWidget() :
     _scriptEditorWidgetUI(new Ui::ScriptEditorWidget),
-    _scriptEngine(NULL)
+    _scriptEngine(NULL),
+    _isRestarting(false),
+    _isReloading(false)
 {
+    setAttribute(Qt::WA_DeleteOnClose);
+
     _scriptEditorWidgetUI->setupUi(this);
 
     connect(_scriptEditorWidgetUI->scriptEdit->document(), &QTextDocument::modificationChanged, this,
@@ -51,15 +55,19 @@ ScriptEditorWidget::~ScriptEditorWidget() {
 }
 
 void ScriptEditorWidget::onScriptModified() {
-    if(_scriptEditorWidgetUI->onTheFlyCheckBox->isChecked() && isRunning()) {
+    if(_scriptEditorWidgetUI->onTheFlyCheckBox->isChecked() && isModified() && isRunning() && !_isReloading) {
+        _isRestarting = true;
         setRunning(false);
-        setRunning(true);
+        // Script is restarted once current script instance finishes.
     }
 }
 
-void ScriptEditorWidget::onScriptEnding() {
+void ScriptEditorWidget::onScriptFinished(const QString& scriptPath) {
-    // signals will automatically be disonnected when the _scriptEngine is deleted later
     _scriptEngine = NULL;
+    if (_isRestarting) {
+        _isRestarting = false;
+        setRunning(true);
+    }
 }
 
 bool ScriptEditorWidget::isModified() {
@@ -71,27 +79,28 @@ bool ScriptEditorWidget::isRunning() {
 }
 
 bool ScriptEditorWidget::setRunning(bool run) {
-    if (run && !save()) {
+    if (run && isModified() && !save()) {
         return false;
     }
 
-    // Clean-up old connections.
     if (_scriptEngine != NULL) {
         disconnect(_scriptEngine, &ScriptEngine::runningStateChanged, this, &ScriptEditorWidget::runningStateChanged);
         disconnect(_scriptEngine, &ScriptEngine::errorMessage, this, &ScriptEditorWidget::onScriptError);
         disconnect(_scriptEngine, &ScriptEngine::printedMessage, this, &ScriptEditorWidget::onScriptPrint);
-        disconnect(_scriptEngine, &ScriptEngine::scriptEnding, this, &ScriptEditorWidget::onScriptEnding);
+        disconnect(_scriptEngine, &ScriptEngine::update, this, &ScriptEditorWidget::onScriptModified);
+        disconnect(_scriptEngine, &ScriptEngine::finished, this, &ScriptEditorWidget::onScriptFinished);
     }
 
     if (run) {
-        _scriptEngine = Application::getInstance()->loadScript(_currentScript, true);
+        const QString& scriptURLString = QUrl(_currentScript).toString();
+        _scriptEngine = Application::getInstance()->loadScript(scriptURLString, true);
         connect(_scriptEngine, &ScriptEngine::runningStateChanged, this, &ScriptEditorWidget::runningStateChanged);
-
-        // Make new connections.
         connect(_scriptEngine, &ScriptEngine::errorMessage, this, &ScriptEditorWidget::onScriptError);
         connect(_scriptEngine, &ScriptEngine::printedMessage, this, &ScriptEditorWidget::onScriptPrint);
-        connect(_scriptEngine, &ScriptEngine::scriptEnding, this, &ScriptEditorWidget::onScriptEnding);
+        connect(_scriptEngine, &ScriptEngine::update, this, &ScriptEditorWidget::onScriptModified);
+        connect(_scriptEngine, &ScriptEngine::finished, this, &ScriptEditorWidget::onScriptFinished);
     } else {
+        connect(_scriptEngine, &ScriptEngine::finished, this, &ScriptEditorWidget::onScriptFinished);
         Application::getInstance()->stopScript(_currentScript);
         _scriptEngine = NULL;
     }
@@ -108,13 +117,14 @@ bool ScriptEditorWidget::saveFile(const QString &scriptPath) {
 
      QTextStream out(&file);
      out << _scriptEditorWidgetUI->scriptEdit->toPlainText();
+     file.close();
 
      setScriptFile(scriptPath);
      return true;
 }
 
 void ScriptEditorWidget::loadFile(const QString& scriptPath) {
-     QUrl url(scriptPath);
+    QUrl url(scriptPath);
 
     // if the scheme length is one or lower, maybe they typed in a file, let's try
     const int WINDOWS_DRIVE_LETTER_SIZE = 1;
@@ -127,13 +137,15 @@ void ScriptEditorWidget::loadFile(const QString& scriptPath) {
         }
         QTextStream in(&file);
         _scriptEditorWidgetUI->scriptEdit->setPlainText(in.readAll());
+        file.close();
         setScriptFile(scriptPath);
 
         if (_scriptEngine != NULL) {
             disconnect(_scriptEngine, &ScriptEngine::runningStateChanged, this, &ScriptEditorWidget::runningStateChanged);
             disconnect(_scriptEngine, &ScriptEngine::errorMessage, this, &ScriptEditorWidget::onScriptError);
             disconnect(_scriptEngine, &ScriptEngine::printedMessage, this, &ScriptEditorWidget::onScriptPrint);
-            disconnect(_scriptEngine, &ScriptEngine::scriptEnding, this, &ScriptEditorWidget::onScriptEnding);
+            disconnect(_scriptEngine, &ScriptEngine::update, this, &ScriptEditorWidget::onScriptModified);
+            disconnect(_scriptEngine, &ScriptEngine::finished, this, &ScriptEditorWidget::onScriptFinished);
         }
     } else {
         QNetworkAccessManager* networkManager = new QNetworkAccessManager(this);
@@ -148,12 +160,14 @@ void ScriptEditorWidget::loadFile(const QString& scriptPath) {
         }
     }
 
-    _scriptEngine = Application::getInstance()->getScriptEngine(_currentScript);
+    const QString& scriptURLString = QUrl(_currentScript).toString();
+    _scriptEngine = Application::getInstance()->getScriptEngine(scriptURLString);
     if (_scriptEngine != NULL) {
         connect(_scriptEngine, &ScriptEngine::runningStateChanged, this, &ScriptEditorWidget::runningStateChanged);
         connect(_scriptEngine, &ScriptEngine::errorMessage, this, &ScriptEditorWidget::onScriptError);
         connect(_scriptEngine, &ScriptEngine::printedMessage, this, &ScriptEditorWidget::onScriptPrint);
-        connect(_scriptEngine, &ScriptEngine::scriptEnding, this, &ScriptEditorWidget::onScriptEnding);
+        connect(_scriptEngine, &ScriptEngine::update, this, &ScriptEditorWidget::onScriptModified);
+        connect(_scriptEngine, &ScriptEngine::finished, this, &ScriptEditorWidget::onScriptFinished);
     }
 }
 
@@ -175,6 +189,7 @@ bool ScriptEditorWidget::saveAs() {
 
 void ScriptEditorWidget::setScriptFile(const QString& scriptPath) {
     _currentScript = scriptPath;
+    _currentScriptModified = QFileInfo(_currentScript).lastModified();
     _scriptEditorWidgetUI->scriptEdit->document()->setModified(false);
     setWindowModified(false);
 
@@ -198,3 +213,29 @@ void ScriptEditorWidget::onScriptError(const QString& message) {
 void ScriptEditorWidget::onScriptPrint(const QString& message) {
     _scriptEditorWidgetUI->debugText->appendPlainText("> " + message);
 }
+
+void ScriptEditorWidget::onWindowActivated() {
+    if (!_isReloading) {
+        _isReloading = true;
+        
+        if (QFileInfo(_currentScript).lastModified() > _currentScriptModified) {
+            if (static_cast<ScriptEditorWindow*>(this->parent()->parent()->parent())->autoReloadScripts()
+                || QMessageBox::warning(this, _currentScript,
+                    tr("This file has been modified outside of the Interface editor.") + "\n\n"
+                        + (isModified()
+                        ? tr("Do you want to reload it and lose the changes you've made in the Interface editor?")
+                        : tr("Do you want to reload it?")),
+                    QMessageBox::Yes | QMessageBox::No) == QMessageBox::Yes) {
+                loadFile(_currentScript);
+                if (_scriptEditorWidgetUI->onTheFlyCheckBox->isChecked() && isRunning()) {
+                    _isRestarting = true;
+                    setRunning(false);
+                    // Script is restarted once current script instance finishes.
+                }
+
+            }
+        }
+
+        _isReloading = false;
+    }
+}

interface/src/ui/ScriptEditorWidget.h

@@ -13,6 +13,7 @@
 #define hifi_ScriptEditorWidget_h
 
 #include <QDockWidget>
+
 #include "ScriptEngine.h"
 
 namespace Ui {
@@ -42,16 +43,22 @@ signals:
     void scriptnameChanged();
     void scriptModified();
 
+public slots:
+    void onWindowActivated();
+
 private slots:
     void onScriptError(const QString& message);
     void onScriptPrint(const QString& message);
     void onScriptModified();
-    void onScriptEnding();
+    void onScriptFinished(const QString& scriptName);
 
 private:
     Ui::ScriptEditorWidget* _scriptEditorWidgetUI;
     ScriptEngine* _scriptEngine;
     QString _currentScript;
+    QDateTime _currentScriptModified;
+    bool _isRestarting;
+    bool _isReloading;
 };
 
 #endif // hifi_ScriptEditorWidget_h

interface/src/ui/ScriptEditorWindow.cpp

@@ -36,6 +36,8 @@ ScriptEditorWindow::ScriptEditorWindow() :
     _loadMenu(new QMenu),
     _saveMenu(new QMenu)
 {
+    setAttribute(Qt::WA_DeleteOnClose);
+
     _ScriptEditorWindowUI->setupUi(this);
     this->setWindowFlags(Qt::Tool);
     show();
@@ -140,6 +142,7 @@ ScriptEditorWidget* ScriptEditorWindow::addScriptEditorWidget(QString title) {
     connect(newScriptEditorWidget, &ScriptEditorWidget::scriptnameChanged, this, &ScriptEditorWindow::updateScriptNameOrStatus);
     connect(newScriptEditorWidget, &ScriptEditorWidget::scriptModified, this, &ScriptEditorWindow::updateScriptNameOrStatus);
     connect(newScriptEditorWidget, &ScriptEditorWidget::runningStateChanged, this, &ScriptEditorWindow::updateButtons);
+    connect(this, &ScriptEditorWindow::windowActivated, newScriptEditorWidget, &ScriptEditorWidget::onWindowActivated);
     _ScriptEditorWindowUI->tabWidget->addTab(newScriptEditorWidget, title);
     _ScriptEditorWindowUI->tabWidget->setCurrentWidget(newScriptEditorWidget);
     newScriptEditorWidget->setUpdatesEnabled(true);
@@ -216,3 +219,15 @@ void ScriptEditorWindow::terminateCurrentTab() {
         this->raise();
     }
 }
+
+bool ScriptEditorWindow::autoReloadScripts() {
+    return _ScriptEditorWindowUI->autoReloadCheckBox->isChecked();
+}
+
+bool ScriptEditorWindow::event(QEvent* event) {
+    if (event->type() == QEvent::WindowActivate) {
+        emit windowActivated();
+    }
+    return QWidget::event(event);
+}
+

interface/src/ui/ScriptEditorWindow.h

@@ -26,9 +26,14 @@ public:
     ~ScriptEditorWindow();
 
     void terminateCurrentTab();
+    bool autoReloadScripts();
+
+signals:
+    void windowActivated();
 
 protected:
     void closeEvent(QCloseEvent* event);
+    virtual bool event(QEvent* event);
 
 private:
     Ui::ScriptEditorWindow* _ScriptEditorWindowUI;

interface/ui/scriptEditorWindow.ui

@@ -33,7 +33,7 @@
     <number>0</number>
    </property>
    <item>
-    <layout class="QHBoxLayout" name="horizontalLayout" stretch="0,0,0,0,0">
+    <layout class="QHBoxLayout" name="horizontalLayout" stretch="0,0,0,0,0,0">
      <property name="spacing">
       <number>3</number>
      </property>
@@ -185,6 +185,16 @@
        </property>
       </spacer>
      </item>
+     <item>
+      <widget class="QCheckBox" name="autoReloadCheckBox">
+       <property name="styleSheet">
+        <string notr="true">font: 13px &quot;Helvetica&quot;,&quot;Arial&quot;,&quot;sans-serif&quot;;</string>
+       </property>
+       <property name="text">
+        <string>Automatically reload externally changed files</string>
+       </property>
+      </widget>
+     </item>
     </layout>
    </item>
    <item>

libraries/script-engine/src/ScriptEngine.cpp

@@ -349,6 +349,7 @@ void ScriptEngine::run() {
         init();
     }
     _isRunning = true;
+    _isFinished = false;
     emit runningStateChanged();
 
     QScriptValue result = _engine.evaluate(_scriptContents);