From 22a764ed823a17ea2494ecfa47b72ab8beffd025 Mon Sep 17 00:00:00 2001 From: goat Date: Mon, 26 Jun 2023 17:39:11 +0200 Subject: [PATCH] fix: don't save manifests every time a plugin loads, note reason for save if save fails --- Dalamud/Plugin/Internal/PluginManager.cs | 2 +- Dalamud/Plugin/Internal/Types/LocalPlugin.cs | 28 +++++++++++++------ .../Internal/Types/LocalPluginManifest.cs | 17 ++++++++++- 3 files changed, 36 insertions(+), 11 deletions(-) diff --git a/Dalamud/Plugin/Internal/PluginManager.cs b/Dalamud/Plugin/Internal/PluginManager.cs index ccb54cc99..82def29d0 100644 --- a/Dalamud/Plugin/Internal/PluginManager.cs +++ b/Dalamud/Plugin/Internal/PluginManager.cs @@ -865,7 +865,7 @@ internal partial class PluginManager : IDisposable, IServiceType // Document the url the plugin was installed from manifest.InstalledFromUrl = repoManifest.SourceRepo.IsThirdParty ? repoManifest.SourceRepo.PluginMasterUrl : LocalPluginManifest.FlagMainRepo; - manifest.Save(manifestFile); + manifest.Save(manifestFile, "installation"); Log.Information($"Installed plugin {manifest.Name} (testing={useTesting})"); diff --git a/Dalamud/Plugin/Internal/Types/LocalPlugin.cs b/Dalamud/Plugin/Internal/Types/LocalPlugin.cs index 14ae4a0c0..42fb40f91 100644 --- a/Dalamud/Plugin/Internal/Types/LocalPlugin.cs +++ b/Dalamud/Plugin/Internal/Types/LocalPlugin.cs @@ -125,13 +125,15 @@ internal class LocalPlugin : IDisposable // Save the manifest to disk so there won't be any problems later. // We'll update the name property after it can be retrieved from the instance. - this.Manifest.Save(this.manifestFile); + this.Manifest.Save(this.manifestFile, "manifest was null"); } else { this.Manifest = manifest; } + var needsSaveDueToLegacyFiles = false; + // This converts from the ".disabled" file feature to the manifest instead. this.disabledFile = LocalPluginManifest.GetDisabledFile(this.DllFile); if (this.disabledFile.Exists) @@ -140,6 +142,8 @@ internal class LocalPlugin : IDisposable this.Manifest.Disabled = true; #pragma warning restore CS0618 this.disabledFile.Delete(); + + needsSaveDueToLegacyFiles = true; } // This converts from the ".testing" file feature to the manifest instead. @@ -148,13 +152,16 @@ internal class LocalPlugin : IDisposable { this.Manifest.Testing = true; this.testingFile.Delete(); + + needsSaveDueToLegacyFiles = true; } var pluginManager = Service.Get(); this.IsBanned = pluginManager.IsManifestBanned(this.Manifest) && !this.IsDev; this.BanReason = pluginManager.GetBanReason(this.Manifest); - this.SaveManifest(); + if (needsSaveDueToLegacyFiles) + this.SaveManifest("legacy"); } /// @@ -322,8 +329,11 @@ internal class LocalPlugin : IDisposable } // If we reload a plugin we don't want to delete it. Makes sense, right? - this.Manifest.ScheduledForDeletion = false; - this.SaveManifest(); + if (this.Manifest.ScheduledForDeletion) + { + this.Manifest.ScheduledForDeletion = false; + this.SaveManifest("Scheduled for deletion, but loading"); + } switch (this.State) { @@ -470,10 +480,10 @@ internal class LocalPlugin : IDisposable } // In-case the manifest name was a placeholder. Can occur when no manifest was included. - if (this.Manifest.Name.IsNullOrEmpty()) + if (this.Manifest.Name.IsNullOrEmpty() && !this.IsDev) { this.Manifest.Name = this.instance.Name; - this.Manifest.Save(this.manifestFile); + this.Manifest.Save(this.manifestFile, "manifest name null or empty"); } this.State = PluginState.Loaded; @@ -618,7 +628,7 @@ internal class LocalPlugin : IDisposable public void ScheduleDeletion(bool status = true) { this.Manifest.ScheduledForDeletion = status; - this.SaveManifest(); + this.SaveManifest("scheduling for deletion"); } /// @@ -633,7 +643,7 @@ internal class LocalPlugin : IDisposable this.Manifest = LocalPluginManifest.Load(manifest) ?? throw new Exception("Could not reload manifest."); // this.Manifest.Disabled = isDisabled; - this.SaveManifest(); + this.SaveManifest("dev reload"); } } @@ -665,5 +675,5 @@ internal class LocalPlugin : IDisposable config.SharedAssemblies.Add(typeof(Lumina.Excel.ExcelSheetImpl).Assembly.GetName()); } - private void SaveManifest() => this.Manifest.Save(this.manifestFile); + private void SaveManifest(string reason) => this.Manifest.Save(this.manifestFile, reason); } diff --git a/Dalamud/Plugin/Internal/Types/LocalPluginManifest.cs b/Dalamud/Plugin/Internal/Types/LocalPluginManifest.cs index e142f9cb0..bcb8cd9e7 100644 --- a/Dalamud/Plugin/Internal/Types/LocalPluginManifest.cs +++ b/Dalamud/Plugin/Internal/Types/LocalPluginManifest.cs @@ -3,6 +3,7 @@ using System.IO; using Dalamud.Utility; using Newtonsoft.Json; +using Serilog; namespace Dalamud.Plugin.Internal.Types; @@ -69,7 +70,21 @@ internal record LocalPluginManifest : PluginManifest /// Save a plugin manifest to file. /// /// Path to save at. - public void Save(FileInfo manifestFile) => Util.WriteAllTextSafe(manifestFile.FullName, JsonConvert.SerializeObject(this, Formatting.Indented)); + /// The reason the manifest was saved. + public void Save(FileInfo manifestFile, string reason) + { + Log.Verbose("Saving manifest for '{PluginName}' because '{Reason}'", this.InternalName, reason); + + try + { + Util.WriteAllTextSafe(manifestFile.FullName, JsonConvert.SerializeObject(this, Formatting.Indented)); + } + catch + { + Log.Error("Could not write out manifest for '{PluginName}' because '{Reason}'", this.InternalName, reason); + throw; + } + } /// /// Loads a plugin manifest from file.