Repository: workcraft/workcraft
Issue: Inconsistent state of global settings

To reproduce, run Workcraft with the following script:
```
workcraft -nogui -exec:<(echo "setConfigVar(\\"CircuitSettings.gateLibrary\\", \\"libraries/test.lib\\");; exit();")
```
Examine the saved `~/.config/workcraft/config.xml` file. The `gateLibrary` variable in `CircuitSettings` group is still set to 'libraries/workcraft.lib' instead of the expected 'libraries/test.lib'.

The reason for this inconsistency is that plugins often cache the original config settings into local variables, and then only track the changes of settings via GUI. On exit plugins write these variables back into config dictionary, thus overwriting all the changes performed via `setConfigVar` interface.

Option 1 (did not work): Use config dictionary as the only storage for global settings, without caching them in plugins. This did not work because other config dictionaries are also used for resetting to default settings.

Option 2: Update plugin settings on every use of `setConfigVar` and re-read plugin settings on every `getConfigVar`.

Changed files:
WorkcraftCore/src/org/workcraft/Framework.java, WorkcraftCore/src/org/workcraft/gui/workspace/WorkspaceWindow.java, WorkcraftCore/src/org/workcraft/gui/MainWindow.java

diff --git a/WorkcraftCore/src/org/workcraft/Framework.java b/WorkcraftCore/src/org/workcraft/Framework.java
index c326e21b0..b986bfbf2 100644
--- a/WorkcraftCore/src/org/workcraft/Framework.java
+++ b/WorkcraftCore/src/org/workcraft/Framework.java
@@ -239,65 +239,57 @@ public final class Framework {
         return instance;
     }
 
-    public void resetConfig() {
-        config = new Config();
+    private void loadConfigPlugins() {
         for (PluginInfo<? extends Settings> info : pluginManager.getPlugins(Settings.class)) {
             info.getSingleton().load(config);
         }
     }
 
+    private void saveConfigPlugins() {
+        for (PluginInfo<? extends Settings> info : pluginManager.getPlugins(Settings.class)) {
+            info.getSingleton().save(config);
+        }
+    }
+
+    public void resetConfig() {
+        config = new Config();
+        loadConfigPlugins();
+    }
+
     public void loadConfig() {
         File file = new File(CONFIG_FILE_PATH);
         LogUtils.logMessageLine("Loading global preferences from " + file.getAbsolutePath());
         config.load(file);
-        for (PluginInfo<? extends Settings> info : pluginManager.getPlugins(Settings.class)) {
-            info.getSingleton().load(config);
-        }
+        loadConfigPlugins();
     }
 
     public void saveConfig() {
-        for (PluginInfo<? extends Settings> info : pluginManager.getPlugins(Settings.class)) {
-            info.getSingleton().save(config);
-        }
+        saveConfigPlugins();
         File file = new File(CONFIG_FILE_PATH);
         LogUtils.logMessageLine("Saving global preferences to " + file.getAbsolutePath());
         config.save(file);
     }
 
-    public void setConfigVar(String key, String value) {
+    public void setConfigCoreVar(String key, String value) {
+        // Set a core variable, that does not require updating plugin settings.
         config.set(key, value);
     }
 
-    public void setConfigVar(String key, int value) {
-        config.set(key, Integer.toString(value));
-    }
-
-    public void setConfigVar(String key, boolean value) {
-        config.set(key, Boolean.toString(value));
+    public void setConfigVar(String key, String value) {
+        setConfigCoreVar(key, value);
+        // For consistency, update plugin settings.
+        loadConfigPlugins();
     }
 
-    public String getConfigVar(String key) {
+    public String getConfigCoreVar(String key) {
+        // Get a core variable, that does not require flushing plugin settings.
         return config.get(key);
     }
 
-    public int getConfigVarAsInt(String key, int defaultValue) {
-        String s = config.get(key);
-
-        try {
-            return Integer.parseInt(s);
-        } catch (NumberFormatException e) {
-            return defaultValue;
-        }
-    }
-
-    public boolean getConfigVarAsBool(String key, boolean defaultValue) {
-        String s = config.get(key);
-
-        if (s == null) {
-            return defaultValue;
-        } else {
-            return Boolean.parseBoolean(s);
-        }
+    public String getConfigVar(String key) {
+        // For consistency, flush plugin settings.
+        saveConfigPlugins();
+        return getConfigCoreVar(key);
     }
 
     public String[] getModelNames() {
@@ -341,11 +333,9 @@ public final class Framework {
             public Object run(Context arg0) {
                 Object scriptable = Context.javaToJS(object, scope);
                 ScriptableObject.putProperty(scope, name, scriptable);
-
                 if (readOnly) {
                     scope.setAttributes(name, ScriptableObject.READONLY);
                 }
-
                 return scriptable;
             }
         });
@@ -357,7 +347,6 @@ public final class Framework {
                 return ScriptableObject.deleteProperty(scope, name);
             }
         });
-
     }
 
     public Object execJavaScript(File file) throws FileNotFoundException {
@@ -380,7 +369,8 @@ public final class Framework {
 
         @Override
         public String getMessage() {
-            return String.format("Java %s was unhandled in javascript. \\nJavascript stack trace: %s", getCause().getClass().getSimpleName(), getScriptTrace());
+            return String.format("Java %s was unhandled in javascript. \\nJavascript stack trace: %s",
+                    getCause().getClass().getSimpleName(), getScriptTrace());
         }
 
         public String getScriptTrace() {
diff --git a/WorkcraftCore/src/org/workcraft/gui/MainWindow.java b/WorkcraftCore/src/org/workcraft/gui/MainWindow.java
index 1ce606746..bc0960b84 100644
--- a/WorkcraftCore/src/org/workcraft/gui/MainWindow.java
+++ b/WorkcraftCore/src/org/workcraft/gui/MainWindow.java
@@ -654,9 +654,9 @@ public class MainWindow extends JFrame {
 
     public void loadWindowGeometryFromConfig() {
         final Framework framework = Framework.getInstance();
-        String maximisedStr = framework.getConfigVar(CONFIG_GUI_MAIN_MAXIMISED);
-        String widthStr = framework.getConfigVar(CONFIG_GUI_MAIN_WIDTH);
-        String heightStr = framework.getConfigVar(CONFIG_GUI_MAIN_HEIGHT);
+        String maximisedStr = framework.getConfigCoreVar(CONFIG_GUI_MAIN_MAXIMISED);
+        String widthStr = framework.getConfigCoreVar(CONFIG_GUI_MAIN_WIDTH);
+        String heightStr = framework.getConfigCoreVar(CONFIG_GUI_MAIN_HEIGHT);
 
         boolean maximised = (maximisedStr == null) ? true : Boolean.parseBoolean(maximisedStr);
         this.setExtendedState(maximised ? JFrame.MAXIMIZED_BOTH : JFrame.NORMAL);
@@ -685,17 +685,17 @@ public class MainWindow extends JFrame {
     public void saveWindowGeometryToConfig() {
         final Framework framework = Framework.getInstance();
         boolean maximised = (getExtendedState() & JFrame.MAXIMIZED_BOTH) != 0;
-        framework.setConfigVar(CONFIG_GUI_MAIN_MAXIMISED, Boolean.toString(maximised));
-        framework.setConfigVar(CONFIG_GUI_MAIN_WIDTH, Integer.toString(getWidth()));
-        framework.setConfigVar(CONFIG_GUI_MAIN_HEIGHT, Integer.toString(getHeight()));
+        framework.setConfigCoreVar(CONFIG_GUI_MAIN_MAXIMISED, Boolean.toString(maximised));
+        framework.setConfigCoreVar(CONFIG_GUI_MAIN_WIDTH, Integer.toString(getWidth()));
+        framework.setConfigCoreVar(CONFIG_GUI_MAIN_HEIGHT, Integer.toString(getHeight()));
     }
 
     public void loadRecentFilesFromConfig() {
         final Framework framework = Framework.getInstance();
-        lastSavePath = framework.getConfigVar(CONFIG_GUI_MAIN_LAST_SAVE_PATH);
-        lastOpenPath = framework.getConfigVar(CONFIG_GUI_MAIN_LAST_OPEN_PATH);
+        lastSavePath = framework.getConfigCoreVar(CONFIG_GUI_MAIN_LAST_SAVE_PATH);
+        lastOpenPath = framework.getConfigCoreVar(CONFIG_GUI_MAIN_LAST_OPEN_PATH);
         for (int i = 0; i < CommonEditorSettings.getRecentCount(); i++) {
-            String entry = framework.getConfigVar(CONFIG_GUI_MAIN_RECENT_FILE + i);
+            String entry = framework.getConfigCoreVar(CONFIG_GUI_MAIN_RECENT_FILE + i);
             pushRecentFile(entry, false);
         }
         updateRecentFilesMenu();
@@ -704,15 +704,15 @@ public class MainWindow extends JFrame {
     public void saveRecentFilesToConfig() {
         final Framework framework = Framework.getInstance();
         if (lastSavePath != null) {
-            framework.setConfigVar(CONFIG_GUI_MAIN_LAST_SAVE_PATH, lastSavePath);
+            framework.setConfigCoreVar(CONFIG_GUI_MAIN_LAST_SAVE_PATH, lastSavePath);
         }
         if (lastOpenPath != null) {
-            framework.setConfigVar(CONFIG_GUI_MAIN_LAST_OPEN_PATH, lastOpenPath);
+            framework.setConfigCoreVar(CONFIG_GUI_MAIN_LAST_OPEN_PATH, lastOpenPath);
         }
         int recentCount = CommonEditorSettings.getRecentCount();
         String[] tmp = recentFiles.toArray(new String[recentCount]);
         for (int i = 0; i < recentCount; i++) {
-            framework.setConfigVar(CONFIG_GUI_MAIN_RECENT_FILE + i, tmp[i]);
+            framework.setConfigCoreVar(CONFIG_GUI_MAIN_RECENT_FILE + i, tmp[i]);
         }
     }
 
diff --git a/WorkcraftCore/src/org/workcraft/gui/workspace/WorkspaceWindow.java b/WorkcraftCore/src/org/workcraft/gui/workspace/WorkspaceWindow.java
index 6257140a4..b43673234 100644
--- a/WorkcraftCore/src/org/workcraft/gui/workspace/WorkspaceWindow.java
+++ b/WorkcraftCore/src/org/workcraft/gui/workspace/WorkspaceWindow.java
@@ -141,17 +141,17 @@ public class WorkspaceWindow extends JPanel {
         setLayout(new BorderLayout(0, 0));
         this.add(scrollPane, BorderLayout.CENTER);
 
-        lastSavePath = framework.getConfigVar("gui.workspace.lastSavePath");
-        lastOpenPath = framework.getConfigVar("gui.workspace.lastOpenPath");
+        lastSavePath = framework.getConfigCoreVar("gui.workspace.lastSavePath");
+        lastOpenPath = framework.getConfigCoreVar("gui.workspace.lastOpenPath");
     }
 
     public void shutdown() {
         final Framework framework = Framework.getInstance();
         if (lastSavePath != null) {
-            framework.setConfigVar("gui.workspace.lastSavePath", lastSavePath);
+            framework.setConfigCoreVar("gui.workspace.lastSavePath", lastSavePath);
         }
         if (lastOpenPath != null) {
-            framework.setConfigVar("gui.workspace.lastOpenPath", lastOpenPath);
+            framework.setConfigCoreVar("gui.workspace.lastOpenPath", lastOpenPath);
         }
     }