diff --git a/Dalamud/Interface/Internal/DalamudInterface.cs b/Dalamud/Interface/Internal/DalamudInterface.cs index 5d21e954a..42907016f 100644 --- a/Dalamud/Interface/Internal/DalamudInterface.cs +++ b/Dalamud/Interface/Internal/DalamudInterface.cs @@ -46,6 +46,8 @@ using ImPlotNET; using PInvoke; using Serilog.Events; +using Task = System.Threading.Tasks.Task; + namespace Dalamud.Interface.Internal; /// @@ -1013,7 +1015,7 @@ internal class DalamudInterface : IInternalDisposableService if (ImGui.MenuItem("Scan dev plugins")) { - pluginManager.ScanDevPlugins(); + Task.Run(pluginManager.ScanDevPluginsAsync); } ImGui.Separator(); diff --git a/Dalamud/Interface/Internal/Windows/PluginInstaller/PluginInstallerWindow.cs b/Dalamud/Interface/Internal/Windows/PluginInstaller/PluginInstallerWindow.cs index 317fa86d0..6868827b4 100644 --- a/Dalamud/Interface/Internal/Windows/PluginInstaller/PluginInstallerWindow.cs +++ b/Dalamud/Interface/Internal/Windows/PluginInstaller/PluginInstallerWindow.cs @@ -283,7 +283,7 @@ internal class PluginInstallerWindow : Window, IDisposable var pluginManager = Service.Get(); _ = pluginManager.ReloadPluginMastersAsync(); - Service.Get().ScanDevPlugins(); + _ = pluginManager.ScanDevPluginsAsync(); if (!this.isSearchTextPrefilled) { @@ -782,7 +782,7 @@ internal class PluginInstallerWindow : Window, IDisposable ImGui.SameLine(); if (ImGui.Button(Locs.FooterButton_ScanDevPlugins)) { - pluginManager.ScanDevPlugins(); + _ = pluginManager.ScanDevPluginsAsync(); } } diff --git a/Dalamud/Interface/Internal/Windows/Settings/Widgets/DevPluginsSettingsEntry.cs b/Dalamud/Interface/Internal/Windows/Settings/Widgets/DevPluginsSettingsEntry.cs index 4c5dc8b83..d7f8b1ca1 100644 --- a/Dalamud/Interface/Internal/Windows/Settings/Widgets/DevPluginsSettingsEntry.cs +++ b/Dalamud/Interface/Internal/Windows/Settings/Widgets/DevPluginsSettingsEntry.cs @@ -52,7 +52,7 @@ public class DevPluginsSettingsEntry : SettingsEntry if (this.devPluginLocationsChanged) { - Service.Get().ScanDevPlugins(); + _ = Service.Get().ScanDevPluginsAsync(); this.devPluginLocationsChanged = false; } } diff --git a/Dalamud/Plugin/Internal/PluginManager.cs b/Dalamud/Plugin/Internal/PluginManager.cs index a20f87241..28fc1fcb1 100644 --- a/Dalamud/Plugin/Internal/PluginManager.cs +++ b/Dalamud/Plugin/Internal/PluginManager.cs @@ -776,7 +776,8 @@ internal class PluginManager : IInternalDisposableService /// only shown as disabled in the installed plugins window. This is a modified version of LoadAllPlugins that works /// a little differently. /// - public void ScanDevPlugins() + /// A representing the asynchronous operation. This function generally will not block as new plugins aren't loaded. + public async Task ScanDevPluginsAsync() { // devPlugins are more freeform. Look for any dll and hope to get lucky. var devDllFiles = new List(); @@ -823,8 +824,7 @@ internal class PluginManager : IInternalDisposableService try { // Add them to the list and let the user decide, nothing is auto-loaded. - this.LoadPluginAsync(dllFile, manifest, PluginLoadReason.Installer, isDev: true, doNotLoad: true) - .Wait(); + await this.LoadPluginAsync(dllFile, manifest, PluginLoadReason.Installer, isDev: true, doNotLoad: true); listChanged = true; } catch (InvalidPluginException) @@ -1188,32 +1188,20 @@ internal class PluginManager : IInternalDisposableService { // Testing exclusive if (manifest.IsTestingExclusive && !this.configuration.DoPluginTest) - { - Log.Verbose($"Testing exclusivity: {manifest.InternalName} - {manifest.AssemblyVersion} - {manifest.TestingAssemblyVersion}"); return false; - } // Applicable version if (manifest.ApplicableVersion < this.dalamud.StartInfo.GameVersion) - { - Log.Verbose($"Game version: {manifest.InternalName} - {manifest.AssemblyVersion} - {manifest.TestingAssemblyVersion}"); return false; - } // API level - we keep the API before this in the installer to show as "outdated" var effectiveApiLevel = this.UseTesting(manifest) && manifest.TestingDalamudApiLevel != null ? manifest.TestingDalamudApiLevel.Value : manifest.DalamudApiLevel; if (effectiveApiLevel < DalamudApiLevel - 1 && !this.LoadAllApiLevels) - { - Log.Verbose($"API Level: {manifest.InternalName} - {manifest.AssemblyVersion} - {manifest.TestingAssemblyVersion}"); return false; - } // Banned if (this.IsManifestBanned(manifest)) - { - Log.Verbose($"Banned: {manifest.InternalName} - {manifest.AssemblyVersion} - {manifest.TestingAssemblyVersion}"); return false; - } return true; } @@ -1572,6 +1560,8 @@ internal class PluginManager : IInternalDisposableService /// The loaded plugin. private async Task LoadPluginAsync(FileInfo dllFile, LocalPluginManifest manifest, PluginLoadReason reason, bool isDev = false, bool isBoot = false, bool doNotLoad = false) { + // TODO: Split this function - it should only take care of adding the plugin to the list, not loading itself, that should be done through the plugin instance + var loadPlugin = !doNotLoad; LocalPlugin? plugin; @@ -1582,21 +1572,34 @@ internal class PluginManager : IInternalDisposableService throw new Exception("No internal name"); } - if (isDev) + // Track the plugin as soon as it is instantiated to prevent it from being loaded twice, + // if the installer or DevPlugin scanner is attempting to add plugins while we are still loading boot plugins + lock (this.pluginListLock) { - Log.Information("Loading dev plugin {Name}", manifest.InternalName); - plugin = new LocalDevPlugin(dllFile, manifest); + // Check if this plugin is already loaded + if (this.installedPluginsList.Any(lp => lp.DllFile.FullName == dllFile.FullName)) + throw new InvalidOperationException("Plugin at the provided path is already loaded"); - // This is a dev plugin - turn ImGui asserts on by default if we haven't chosen yet - // TODO(goat): Re-enable this when we have better tracing for what was rendering when - // this.configuration.ImGuiAssertsEnabledAtStartup ??= true; - } - else - { - Log.Information("Loading plugin {Name}", manifest.InternalName); - plugin = new LocalPlugin(dllFile, manifest); + if (isDev) + { + Log.Information("Loading dev plugin {Name}", manifest.InternalName); + plugin = new LocalDevPlugin(dllFile, manifest); + + // This is a dev plugin - turn ImGui asserts on by default if we haven't chosen yet + // TODO(goat): Re-enable this when we have better tracing for what was rendering when + // this.configuration.ImGuiAssertsEnabledAtStartup ??= true; + } + else + { + Log.Information("Loading plugin {Name}", manifest.InternalName); + plugin = new LocalPlugin(dllFile, manifest); + } + + this.installedPluginsList.Add(plugin); } + Log.Verbose("Starting to load plugin {Name} at {FileLocation}", manifest.InternalName, dllFile.FullName); + // Perform a migration from InternalName to GUIDs. The plugin should definitely have a GUID here. // This will also happen if you are installing a plugin with the installer, and that's intended! // It means that, if you have a profile which has unsatisfied plugins, installing a matching plugin will @@ -1697,43 +1700,34 @@ internal class PluginManager : IInternalDisposableService catch (BannedPluginException) { // Out of date plugins get added so they can be updated. - Log.Information($"Plugin was banned, adding anyways: {dllFile.Name}"); + Log.Information("{InternalName}: Plugin was banned, adding anyways", plugin.Manifest.InternalName); } catch (Exception ex) { if (plugin.IsDev) { // Dev plugins always get added to the list so they can be fiddled with in the UI - Log.Information(ex, $"Dev plugin failed to load, adding anyways: {dllFile.Name}"); - - // NOTE(goat): This can't work - plugins don't "unload" if they fail to load. - // plugin.Disable(); // Disable here, otherwise you can't enable+load later + Log.Information(ex, "{InternalName}: Dev plugin failed to load", plugin.Manifest.InternalName); } else if (plugin.IsOutdated) { // Out of date plugins get added, so they can be updated. - Log.Information(ex, $"Plugin was outdated, adding anyways: {dllFile.Name}"); + Log.Information(ex, "{InternalName}: Plugin was outdated", plugin.Manifest.InternalName); } else if (plugin.IsOrphaned) { // Orphaned plugins get added, so that users aren't confused. - Log.Information(ex, $"Plugin was orphaned, adding anyways: {dllFile.Name}"); + Log.Information(ex, "{InternalName}: Plugin was orphaned", plugin.Manifest.InternalName); } else if (isBoot) { // During boot load, plugins always get added to the list so they can be fiddled with in the UI - Log.Information(ex, $"Regular plugin failed to load, adding anyways: {dllFile.Name}"); - - // NOTE(goat): This can't work - plugins don't "unload" if they fail to load. - // plugin.Disable(); // Disable here, otherwise you can't enable+load later + Log.Information(ex, "{InternalName}: Regular plugin failed to load", plugin.Manifest.InternalName); } else if (!plugin.CheckPolicy()) { // During boot load, plugins always get added to the list so they can be fiddled with in the UI - Log.Information(ex, $"Plugin not loaded due to policy, adding anyways: {dllFile.Name}"); - - // NOTE(goat): This can't work - plugins don't "unload" if they fail to load. - // plugin.Disable(); // Disable here, otherwise you can't enable+load later + Log.Information(ex, "{InternalName}: Plugin not loaded due to policy", plugin.Manifest.InternalName); } else { @@ -1742,14 +1736,6 @@ internal class PluginManager : IInternalDisposableService } } - if (plugin == null) - throw new Exception("Plugin was null when adding to list"); - - lock (this.pluginListLock) - { - this.installedPluginsList.Add(plugin); - } - // Mark as finished loading if (manifest.LoadSync) this.StartupLoadTracking?.Finish(manifest.InternalName); diff --git a/Dalamud/Plugin/Internal/Types/LocalDevPlugin.cs b/Dalamud/Plugin/Internal/Types/LocalDevPlugin.cs index 581bfd724..b8f2b2708 100644 --- a/Dalamud/Plugin/Internal/Types/LocalDevPlugin.cs +++ b/Dalamud/Plugin/Internal/Types/LocalDevPlugin.cs @@ -15,7 +15,7 @@ namespace Dalamud.Plugin.Internal.Types; /// This class represents a dev plugin and all facets of its lifecycle. /// The DLL on disk, dependencies, loaded assembly, etc. /// -internal class LocalDevPlugin : LocalPlugin +internal sealed class LocalDevPlugin : LocalPlugin { private static readonly ModuleLog Log = new("PLUGIN"); @@ -41,7 +41,7 @@ internal class LocalDevPlugin : LocalPlugin configuration.DevPluginSettings[dllFile.FullName] = this.devSettings = new DevPluginSettings(); configuration.QueueSave(); } - + // Legacy dev plugins might not have this! if (this.devSettings.WorkingPluginId == Guid.Empty) { @@ -85,7 +85,7 @@ internal class LocalDevPlugin : LocalPlugin } } } - + /// /// Gets an ID uniquely identifying this specific instance of a devPlugin. /// @@ -152,7 +152,7 @@ internal class LocalDevPlugin : LocalPlugin if (manifestPath.Exists) this.manifest = LocalPluginManifest.Load(manifestPath) ?? throw new Exception("Could not reload manifest."); } - + /// protected override void OnPreReload() { diff --git a/Dalamud/Plugin/Internal/Types/LocalPlugin.cs b/Dalamud/Plugin/Internal/Types/LocalPlugin.cs index 1b9025538..144e09b47 100644 --- a/Dalamud/Plugin/Internal/Types/LocalPlugin.cs +++ b/Dalamud/Plugin/Internal/Types/LocalPlugin.cs @@ -62,12 +62,13 @@ internal class LocalPlugin : IAsyncDisposable } this.DllFile = dllFile; - this.State = PluginState.Unloaded; // Although it is conditionally used here, we need to set the initial value regardless. this.manifestFile = LocalPluginManifest.GetManifestFile(this.DllFile); this.manifest = manifest; + this.State = PluginState.Unloaded; + var needsSaveDueToLegacyFiles = false; // This converts from the ".disabled" file feature to the manifest instead. @@ -352,19 +353,13 @@ internal class LocalPlugin : IAsyncDisposable } this.loader.Reload(); + this.RefreshAssemblyInformation(); } - // Load the assembly - this.pluginAssembly ??= this.loader.LoadDefaultAssembly(); - - this.AssemblyName = this.pluginAssembly.GetName(); - - // Find the plugin interface implementation. It is guaranteed to exist after checking in the ctor. - this.pluginType ??= this.pluginAssembly.GetTypes() - .First(type => type.IsAssignableTo(typeof(IDalamudPlugin))); + Log.Verbose("{Name} ({Guid}): Have type", this.InternalName, this.EffectiveWorkingPluginId); // Check for any loaded plugins with the same assembly name - var assemblyName = this.pluginAssembly.GetName().Name; + var assemblyName = this.pluginAssembly!.GetName().Name; foreach (var otherPlugin in pluginManager.InstalledPlugins) { // During hot-reloading, this plugin will be in the plugin list, and the instance will have been disposed @@ -376,7 +371,7 @@ internal class LocalPlugin : IAsyncDisposable if (otherPluginAssemblyName == assemblyName && otherPluginAssemblyName != null) { this.State = PluginState.Unloaded; - Log.Debug($"Duplicate assembly: {this.Name}"); + Log.Debug("Duplicate assembly: {Name}", this.InternalName); throw new DuplicatePluginException(assemblyName); } @@ -392,7 +387,7 @@ internal class LocalPlugin : IAsyncDisposable this.instance = await CreatePluginInstance( this.manifest, this.serviceScope, - this.pluginType, + this.pluginType!, this.dalamudInterface); this.State = PluginState.Loaded; Log.Information("Finished loading {PluginName}", this.InternalName); @@ -620,42 +615,60 @@ internal class LocalPlugin : IAsyncDisposable throw; } + this.RefreshAssemblyInformation(); + } + + private void RefreshAssemblyInformation() + { + if (this.loader == null) + throw new InvalidOperationException("No loader available"); + try { this.pluginAssembly = this.loader.LoadDefaultAssembly(); + this.AssemblyName = this.pluginAssembly.GetName(); } catch (Exception ex) { - this.pluginAssembly = null; - this.pluginType = null; - this.loader.Dispose(); - + this.ResetLoader(); Log.Error(ex, $"Not a plugin: {this.DllFile.FullName}"); throw new InvalidPluginException(this.DllFile); } + if (this.pluginAssembly == null) + { + this.ResetLoader(); + Log.Error("Plugin assembly is null: {DllFileFullName}", this.DllFile.FullName); + throw new InvalidPluginException(this.DllFile); + } + try { this.pluginType = this.pluginAssembly.GetTypes().FirstOrDefault(type => type.IsAssignableTo(typeof(IDalamudPlugin))); } catch (ReflectionTypeLoadException ex) { - Log.Error(ex, $"Could not load one or more types when searching for IDalamudPlugin: {this.DllFile.FullName}"); - // Something blew up when parsing types, but we still want to look for IDalamudPlugin. Let Load() handle the error. - this.pluginType = ex.Types.FirstOrDefault(type => type != null && type.IsAssignableTo(typeof(IDalamudPlugin))); + this.ResetLoader(); + Log.Error(ex, "Could not load one or more types when searching for IDalamudPlugin: {DllFileFullName}", this.DllFile.FullName); + throw; } - if (this.pluginType == default) + if (this.pluginType == null) { - this.pluginAssembly = null; - this.pluginType = null; - this.loader.Dispose(); - - Log.Error($"Nothing inherits from IDalamudPlugin: {this.DllFile.FullName}"); + this.ResetLoader(); + Log.Error("Nothing inherits from IDalamudPlugin: {DllFileFullName}", this.DllFile.FullName); throw new InvalidPluginException(this.DllFile); } } + private void ResetLoader() + { + this.pluginAssembly = null; + this.pluginType = null; + this.loader?.Dispose(); + this.loader = null; + } + /// Clears and disposes all resources associated with the plugin instance. /// Whether to clear and dispose . /// Exceptions, if any occurred.