Merge pull request #10530 from Atlante45/fix/default-scripts

Ensure user never loses its running scripts

interface/src/Application.cpp

@@ -1622,7 +1622,6 @@ void Application::cleanupBeforeQuit() {
     // Clear any queued processing (I/O, FBX/OBJ/Texture parsing)
     QThreadPool::globalInstance()->clear();
 
-    DependencyManager::get<ScriptEngines>()->saveScripts();
     DependencyManager::get<ScriptEngines>()->shutdownScripting(); // stop all currently running global scripts
     DependencyManager::destroy<ScriptEngines>();
 

libraries/script-engine/src/ScriptEngines.cpp

@@ -26,8 +26,12 @@
 #define __LOC__ __FILE__ "(" __STR1__(__LINE__) ") : Warning Msg: "
 
 static const QString DESKTOP_LOCATION = QStandardPaths::writableLocation(QStandardPaths::DesktopLocation);
-
 static const bool HIFI_SCRIPT_DEBUGGABLES { true };
+static const QString SETTINGS_KEY { "RunningScripts" };
+static const QUrl DEFAULT_SCRIPTS_LOCATION { "file:///~//defaultScripts.js" };
+// Using a QVariantList so this is human-readable in the settings file
+static Setting::Handle<QVariantList> runningScriptsHandle(SETTINGS_KEY, { QVariant(DEFAULT_SCRIPTS_LOCATION) });
+
 
 ScriptsModel& getScriptsModel() {
     static ScriptsModel scriptsModel;
@@ -61,19 +65,6 @@ ScriptEngines::ScriptEngines(ScriptEngine::Context context)
     _scriptsModelFilter.setSourceModel(&_scriptsModel);
     _scriptsModelFilter.sort(0, Qt::AscendingOrder);
     _scriptsModelFilter.setDynamicSortFilter(true);
-
-    static const int SCRIPT_SAVE_COUNTDOWN_INTERVAL_MS = 5000;
-    QTimer* scriptSaveTimer = new QTimer(this);
-    scriptSaveTimer->setSingleShot(true);
-    QMetaObject::Connection timerConnection = connect(scriptSaveTimer, &QTimer::timeout, [] {
-        DependencyManager::get<ScriptEngines>()->saveScripts();
-    });
-    connect(qApp, &QCoreApplication::aboutToQuit, [=] {
-        disconnect(timerConnection);
-    });
-    connect(this, &ScriptEngines::scriptCountChanged, this, [scriptSaveTimer] {
-        scriptSaveTimer->start(SCRIPT_SAVE_COUNTDOWN_INTERVAL_MS);
-    }, Qt::QueuedConnection);
 }
 
 QUrl normalizeScriptURL(const QUrl& rawScriptURL) {
@@ -280,13 +271,8 @@ QVariantList ScriptEngines::getRunning() {
     return result;
 }
 
-
-static const QString SETTINGS_KEY = "RunningScripts";
-
 void ScriptEngines::loadDefaultScripts() {
-    QUrl defaultScriptsLoc = defaultScriptsLocation();
-    defaultScriptsLoc.setPath(defaultScriptsLoc.path() + "/defaultScripts.js");
-    loadScript(defaultScriptsLoc.toString());
+    loadScript(DEFAULT_SCRIPTS_LOCATION);
 }
 
 void ScriptEngines::loadOneScript(const QString& scriptFilename) {
@@ -294,17 +280,11 @@ void ScriptEngines::loadOneScript(const QString& scriptFilename) {
 }
 
 void ScriptEngines::loadScripts() {
-    // check first run...
-    Setting::Handle<bool> firstRun { Settings::firstRun, true };
-    if (firstRun.get()) {
-        qCDebug(scriptengine) << "This is a first run...";
-        // clear the scripts, and set out script to our default scripts
-        clearScripts();
-        loadDefaultScripts();
-        return;
-    }
-
-    // loads all saved scripts
+    // START BACKWARD COMPATIBILITY CODE
+    // The following code makes sure people don't lose all their scripts
+    // This should be removed after a reasonable ammount of time went by
+    // Load old setting format if present
+    bool foundDeprecatedSetting = false;
     Settings settings;
     int size = settings.beginReadArray(SETTINGS_KEY);
     for (int i = 0; i < size; ++i) {
@@ -312,35 +292,51 @@ void ScriptEngines::loadScripts() {
         QString string = settings.value("script").toString();
         if (!string.isEmpty()) {
             loadScript(string);
+            foundDeprecatedSetting = true;
         }
     }
     settings.endArray();
-}
+    if (foundDeprecatedSetting) {
+        // Remove old settings found and return
+        settings.beginWriteArray(SETTINGS_KEY);
+        settings.remove("");
+        settings.endArray();
+        settings.remove(SETTINGS_KEY + "/size");
+        return;
+    }
+    // END BACKWARD COMPATIBILITY CODE
 
-void ScriptEngines::clearScripts() {
-    // clears all scripts from the settingsSettings settings;
-    Settings settings;
-    settings.beginWriteArray(SETTINGS_KEY);
-    settings.remove("");
-    settings.endArray();
+    // loads all saved scripts
+    auto runningScripts = runningScriptsHandle.get();
+    for (auto script : runningScripts) {
+        auto string = script.toString();
+        if (!string.isEmpty()) {
+            loadScript(string);
+        }
+    }
 }
 
 void ScriptEngines::saveScripts() {
-    // Saves all currently running user-loaded scripts
-    Settings settings;
-    settings.beginWriteArray(SETTINGS_KEY);
-    settings.remove("");
+    // Do not save anything if we are in the process of shutting down
+    if (qApp->closingDown()) {
+        qWarning() << "Trying to save scripts during shutdown.";
+        return;
+    }
 
-    QStringList runningScripts = getRunningScripts();
-    int i = 0;
-    for (auto it = runningScripts.begin(); it != runningScripts.end(); ++it) {
-        if (getScriptEngine(*it)->isUserLoaded()) {
-            settings.setArrayIndex(i);
-            settings.setValue("script", normalizeScriptURL(*it).toString());
-            ++i;
+    // Saves all currently running user-loaded scripts
+    QVariantList list;
+
+    {
+        QReadLocker lock(&_scriptEnginesHashLock);
+        for (auto it = _scriptEnginesHash.begin(); it != _scriptEnginesHash.end(); ++it) {
+            if (it.value() && it.value()->isUserLoaded()) {
+                auto normalizedUrl = normalizeScriptURL(it.key());
+                list.append(normalizedUrl.toString());
+            }
         }
     }
-    settings.endArray();
+
+    runningScriptsHandle.set(list);
 }
 
 QStringList ScriptEngines::getRunningScripts() {
@@ -513,6 +509,9 @@ void ScriptEngines::onScriptEngineLoaded(const QString& rawScriptURL) {
         QUrl normalized = normalizeScriptURL(url);
         _scriptEnginesHash.insertMulti(normalized, scriptEngine);
     }
+
+    // Update settings with new script
+    saveScripts();
     emit scriptCountChanged();
 }
 
@@ -553,6 +552,8 @@ void ScriptEngines::onScriptFinished(const QString& rawScriptURL, ScriptEngine*
     }
 
     if (removed) {
+        // Update settings with removed script
+        saveScripts();
         emit scriptCountChanged();
     }
 }

libraries/script-engine/src/ScriptEngines.h

@@ -40,7 +40,6 @@ public:
 
     void loadScripts();
     void saveScripts();
-    void clearScripts();
 
     QString getScriptsLocation() const;
     void loadDefaultScripts();