v7: Redesign the plugin loading api to a phase model #999
Conversation
| Logger.Log(LogLevel.Message, "Started phase " + phase); | ||
| CurrentPhase = phase; | ||
| OnPhaseStarted?.Invoke(phase); | ||
| Logger.Log(LogLevel.Message, "Ended phase " + phase); |
There was a problem hiding this comment.
The logging overall seems pretty excessive or at least at wrong levels. Something to look into later once things are getting finalized.
This seems like a good place to add a StopWatch and log how long a phase took.
| private IList<IPluginLoadContext> GetLoadContexts() | ||
| { | ||
| var loadContexts = new List<IPluginLoadContext>(); | ||
| foreach (var dll in Directory.GetFiles(Path.GetFullPath(Paths.PluginPath), "*.dll", SearchOption.AllDirectories)) |
There was a problem hiding this comment.
How about caching all of these contexts (as cached contexts) instead of reiterating over the files on every phase? Or do we expect dll files to be dropped at runtime? Wouldn't this add up across different providers having to reload on every phase?
There was a problem hiding this comment.
Added a discussion item in the OP because it depends if we expect the discovery to not change at entrypoint time
| [AttributeUsage(AttributeTargets.Class)] | ||
| public class BepInPluginProvider : BepInPlugin | ||
| public class BepInPhaseAttribute : Attribute |
There was a problem hiding this comment.
Is the phase attribute supposed to be unique per plugin? If yes, it would make sense to combine it with BepInMetadataAttribute to ensure people don't forget it.
There was a problem hiding this comment.
Along with the general comment, added a discussion item. I feel like it shouldn't be part of metadata, but I like the idea to default it to GameInitialised
|
Conceptually it seems fine but I would have to try actually using it to tell for sure. Main worry is the phases being confusing, they need to be easy to figure out even without reading docs. Maybe move the phase to the metadata attribute and give it a default value of |
|
For the sake of discussions, I provided some samples of trivial plugins: a patcher, a game plugin and a provider. Here's the regular game plugin: using BepInEx;
namespace TestPlugin;
[BepInMetadata("testPlugin", "test plugin", "1.0.0")]
[BepInPhase(BepInPhases.GameInitialised)]
public class TestPlugin : Plugin
{
public override void Load()
{
Logger.LogInfo("Inside TestPlugin!");
}
}To note, this type of plugin would be declared the exact same way no matter the runtime here. Unity plugins no longer are GameObjects, but nothing stops a plugin from using the extension method for this: Next, here's a patcher plugin: using BepInEx;
using BepInEx.Preloader.Core.Patching;
using Mono.Cecil;
namespace TestPatcher;
[BepInMetadata("TestPatchr", "TestPatcher", "1.0.0")]
[BepInPhase(BepInPhases.BeforeGameAssembliesLoaded)]
public class TestPatcher : Plugin
{
public override void Load()
{
AssemblyPatcher.Instance
.AddDefinition(new("Assembly-CSharp.dll", "MainManager", this,
(_, type, _) => PatchAssembly(type)));
}
private bool PatchAssembly(TypeDefinition type)
{
// Patching logic
return true;
}
}The important part is the phase: it's the second of the predefined ones which is the last before game assemblies loading. This means this plugin won't be able to reference any game assemblies, but that would allow to prepatch them. The way to declare the patching is changed. We now have an API exposed by AssemblyPatcher and every patcher would call it to declare their patches. The rest is essentially the same: use Cecil to doe the patching. As for when the patching will occur, it is managed by the runtime's preloader still on the same phase, but after the phase fully started. This means every patcher plugins declared their patcher by this point and the AssemblyPatcher simply calls all of them to patch everything. It's essentially the last thing the preloader would do before the chainloader starts. Finally, here's a plugin providing another (I made a simple one here where I hardcoded a path, but you can imagine this working with zip or from a network): using System.Collections.Generic;
using System.IO;
using BepInEx;
namespace TestProvider;
[BepInMetadata("testProvider", "test plugin provider", "1.0.0")]
[BepInPhase(BepInPhases.AfterGameAssembliesLoaded)]
public class TestProvider : Plugin
{
public override void Load()
{
Logger.LogInfo("Starting custom plugin provider");
PluginManager.Instance.Providers.Add(Info, GetLoadContexts);
Logger.LogInfo("Added a provider");
}
private IList<IPluginLoadContext> GetLoadContexts()
{
return [new TestPluginLoadContext()];
}
}
internal class TestPluginLoadContext : IPluginLoadContext
{
public string AssemblyIdentifier => "TestPlugin";
public string AssemblyHash => File.GetLastWriteTimeUtc(Path).ToString("0");
private const string Path = @"C:\Users\aldelaro5\Documents\bepinex v7 api samples\test\TestPlugin.dll";
public byte[] GetAssemblyData()
{
return File.ReadAllBytes(Path);
}
public byte[] GetAssemblySymbolsData()
{
return File.ReadAllBytes(System.IO.Path.ChangeExtension(Path, "pdb"));
}
public byte[] GetFile(string relativePath)
{
return File.ReadAllBytes(System.IO.Path.Combine(Utility.ParentDirectory(Path), relativePath));
}
}There's a bit more here so:
Hopefully this helps. There's just a lot of open questions around the redesign so I had requests to give some samples. |
Description
This pr supercedes #988 by redesigning the api around the idea of having a single plugin loading system, but divided up into multiple phases.
The way it works is there's no more different plugin types, only a single plugin type exists that can do everything in theory. In practice however, not everything can be doable (for example, a patcher can't reference game assemblies) so to accommodate this, every plugins now requires a phase attribute that will determine when the Load method will be invoked and the plugin loaded.
A phase is defined as a simple string. BepInEx would define standard ones, but in theory, a plugin could create phases of their own. It only defines a moment in time that can be listened to via a global event. The caretaker of that event is a new class called PhaseManager. It allows anyone to listen to any phase or to start one.
By itself, nothing happens, but where the power of this new API model comes in is when the plugin loading system gets involved. There is now a single loading system called PluginManager and all it does is listen to every phase. On every phases, it will do 2 important things:
In order for BepInEx to bootstrap everything on the first phase, a default provider is registered first which will always provide load contexts from the plugins folder before every phases.
This effectively isolates the entire plugin loading system. All BepInEx needs to do is to start the phases at the appropriate moments depending on the runtime and PluginManager will take care of the rest.
As a side effect of this, it also means there's no specialised APIs offered to plugins: they just have a Load, Info, Config and Logger. All apis that BepInEx offers will be done in some other ways such as calbacks or public data structures. A notable improvement to this is AssemblyPatcher: the api is no longer attributes based. Plugins now creates their PatchDefinition themselves on load which contains a calback that performs the patching. This greatly simplifies the api because it eliminates complex attributes and methodInfo parsing.
Currently, this pr defines 4 phases:
Motivation and Context
This has several implications:
The overall effect is the APi becomes MUCH simpler and it becomes more malleable.
How Has This Been Tested?
I tested on new/old mono (including unity 4.x) and il2cpp and everything seemed to work.
Types of changes
Checklist:
Possible discussions
I am opening this as a draft because while I think this pr includes most of the foundation we need, there's some open questions that would need to be further discussed: