From 27c873dbe9e88ddc5cca23cad39faa3e190cbedd Mon Sep 17 00:00:00 2001
From: meli <57847713+ff-meli@users.noreply.github.com>
Date: Sun, 22 Mar 2020 09:41:38 -0700
Subject: [PATCH] fix plugin disable during updates failing due to concurrent
 file modification; fix plugin updates failing to load config due to assembly
 version issues

---
 Dalamud/Configuration/PluginConfigurations.cs | 20 +++++++++++++++++++
 Dalamud/Plugin/DalamudPluginInterface.cs      | 18 +++++++++++++++++
 Dalamud/Plugin/PluginInstallerWindow.cs       | 14 ++++++++-----
 3 files changed, 47 insertions(+), 5 deletions(-)

diff --git a/Dalamud/Configuration/PluginConfigurations.cs b/Dalamud/Configuration/PluginConfigurations.cs
index 74404abe7..d108c5777 100644
--- a/Dalamud/Configuration/PluginConfigurations.cs
+++ b/Dalamud/Configuration/PluginConfigurations.cs
@@ -16,6 +16,10 @@ namespace Dalamud.Configuration
             this.configDirectory.Create();
         }
 
+        // NOTE:  Save/Load are still using Type information for now, despite LoadForType<> superseding Load
+        // and not requiring or using it.  It might be worth removing the Type info from Save, to strip it from
+        // all future saved configs, and then Load() can probably be removed entirey.
+
         public void Save(IPluginConfiguration config, string pluginName) {
             File.WriteAllText(GetPath(pluginName).FullName, JsonConvert.SerializeObject(config, Formatting.Indented, new JsonSerializerSettings
             {
@@ -38,6 +42,22 @@ namespace Dalamud.Configuration
                                                                        });
         }
 
+        // Parameterized deserialization
+        // Currently this is called via reflection from DalamudPluginInterface.GetPluginConfig()
+        // Eventually there may be an additional pluginInterface method that can call this directly
+        // without reflection - for now this is in support of the existing plugin api
+        public T LoadForType<T>(string pluginName) where T : IPluginConfiguration
+        {
+            var path = GetPath(pluginName);
+
+            if (!path.Exists)
+                return default(T);
+
+            // intentionally no type handling - it will break when updating a plugin at runtime
+            // and turns out to be unnecessary when we fully qualify the object type
+            return JsonConvert.DeserializeObject<T>(File.ReadAllText(path.FullName));
+        }
+
         private FileInfo GetPath(string pluginName) => new FileInfo(Path.Combine(this.configDirectory.FullName, $"{pluginName}.json"));
     }
 }
diff --git a/Dalamud/Plugin/DalamudPluginInterface.cs b/Dalamud/Plugin/DalamudPluginInterface.cs
index 8c0797e30..19f36c8cc 100644
--- a/Dalamud/Plugin/DalamudPluginInterface.cs
+++ b/Dalamud/Plugin/DalamudPluginInterface.cs
@@ -94,6 +94,24 @@ namespace Dalamud.Plugin
         /// </summary>
         /// <returns>A previously saved config or null if none was saved before.</returns>
         public IPluginConfiguration GetPluginConfig() {
+            // This is done to support json deserialization of plugin configurations
+            // even after running an in-game update of plugins, where the assembly version
+            // changes.
+            // Eventually it might make sense to have a separate method on this class
+            // T GetPluginConfig<T>() where T : IPluginConfiguration
+            // that can invoke LoadForType() directly instead of via reflection
+            // This is here for now to support the current plugin API
+            foreach (var type in Assembly.GetCallingAssembly().GetTypes())
+            {
+                if (type.GetInterface(typeof(IPluginConfiguration).FullName) != null)
+                {
+                    var mi = this.configs.GetType().GetMethod("LoadForType");
+                    var fn = mi.MakeGenericMethod(type);
+                    return (IPluginConfiguration)fn.Invoke(this.configs, new object[] { this.pluginName });
+                }
+            }
+
+            // this shouldn't be a thing, I think, but just in case
             return this.configs.Load(this.pluginName);
         }
 
diff --git a/Dalamud/Plugin/PluginInstallerWindow.cs b/Dalamud/Plugin/PluginInstallerWindow.cs
index a857524f3..fcc26fba6 100644
--- a/Dalamud/Plugin/PluginInstallerWindow.cs
+++ b/Dalamud/Plugin/PluginInstallerWindow.cs
@@ -99,15 +99,19 @@ namespace Dalamud.Plugin
                     {
                         Log.Information("Eligible for update: {0}", remoteInfo.InternalName);
 
-                        foreach (var sortedVersion in sortedVersions) {
-                            File.Create(Path.Combine(sortedVersion.FullName, ".disabled"));
-                        }
+                        // DisablePlugin() below immediately creates a .disabled file anyway, but will fail
+                        // with an exception if we try to do it twice in row like this
+                        // TODO: not sure if doing this for all versions is really necessary, since the
+                        // others really needed to be disabled before anyway
+                        //foreach (var sortedVersion in sortedVersions) {
+                        //    File.Create(Path.Combine(sortedVersion.FullName, ".disabled"));
+                        //}
 
                         // Try to disable plugin if it is loaded
                         try {
                             this.manager.DisablePlugin(info);
-                        } catch {
-                            // ignored
+                        } catch (Exception ex) {
+                            Log.Error(ex, "Plugin disable failed");
                         }
 
                         InstallPlugin(remoteInfo);