Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Plugin assembly loading fix #222

Open
wants to merge 6 commits into
base: 13.1
Choose a base branch
from
Open

Conversation

Axwabo
Copy link

@Axwabo Axwabo commented Aug 28, 2023

Cross-plugin dependency fix

This PR fixes an issue where plugins would not load if they referenced another plugin that was not yet loaded.

The problem:

Assume the following scenario:

The following plugins are to be loaded:

  • PluginA
  • PluginB
  • PluginC

Both PluginA and PluginC depend on PluginB. PluginB has no dependencies.

  1. PluginA's assembly is loaded.
  2. The loader attempts to initialize PluginA, but fails because the dependency PluginB is not yet found.
  3. PluginB's assembly is loaded and initialized.
  4. PluginC is loaded and starts without any issues.

Changes:

  • Added the PluginAPI.Loader.PluginAssemblyInformation struct
  • Upon loading of a plugin assembly, it is not checked for entry points. If there are no missing references, the plugin information is added to a list.
  • A message is printed to the console: Initializing (count) plugins...
  • The list of plugins is iterated over and initialized.
  • Missing dependencies are now logged even when no exception was thrown.
  • Added a warning when a plugin has a dependency in a different version than the actually loaded dependency.

This way, a plugin can reference another that comes after it alphabetically and will still be initialized correctly.

@Lufou
Copy link

Lufou commented Dec 4, 2023

Please we need this PR, plugins loading is stuck at "Loading x plugins..."

@Lufou
Copy link

Lufou commented Dec 4, 2023

Encountered some issues with your PR, first I think it's better to load all dependencies before actual plugins, i.e:

Log.Info("<---<    Loading global dependencies     <---<");
LoadDependencies(Paths.GlobalPlugins.Dependencies);

Log.Info("<---<     Loading server dependencies    <---<");
LoadDependencies(Paths.LocalPlugins.Dependencies);

Log.Info("<---<    Loading global plugins     <---<");
// Load plugins from the Global directory inside "configs".
LoadPlugins(Paths.GlobalPlugins);

Log.Info("<---<     Loading server plugins    <---<");
// Load plugins from the [Server port] directory inside "configs".
LoadPlugins(Paths.LocalPlugins);

Log.Info("<---<        Load all plugins       <---<");

And I also think it's good to put try catch inside LoadPlugins function to be able to see if there's an error:

try
{
...........all the code of LoadPlugins..........
}
catch (Exception e)
{
   Log.Error(e.ToString());
}

@Lufou
Copy link

Lufou commented Dec 4, 2023

I'm not really not sure why but putting line 145

if (missingDependencies.Count != 0)
{
  Log.Error($"Failed loading plugin &2{Path.GetFileNameWithoutExtension(pluginInfo.Path)}&r, missing dependencies\n&2{string.Join("\n", missingDependencies.Select(x => "&r - &2" + x.Key + " v" + x.Value.ToString(3) + "&r"))}", "Loader");
  continue;
}

in the try will break the loader (System.Net.Http.dll from the Managed folder will be counted as a missing dependency even if the file is there). I would recommend keeping it only in the catch statement like NW did.

Lufou added a commit to Lufou/NwPluginAPI that referenced this pull request Dec 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants