Dynamic patching

[rayzerbrain]

The main purpose of this pr is to implement a new Exiled.Events feature, patching per event. Basically, event patches will only be patched if there are methods subscribed to the event they target. I think this is a necessary feature to lessen the performance and stability issues of requiring the many unused patches exiled has.

How this works is, all existing events in the Handlers namespace will be replaced with a wrapper Event (or Event<T>), which handles subscribing methods in relatively the same way, to lessen the breaking changes.

The difference is, on method subscription, the Patcher class will look for any patches that target that event, and patch ONLY them.

Two things I still want/need input on before I have to brute force all the handlers and patches, which is why this is initially a draft:

1. To get patch classes to target a specific event, I started with the idea of giving them all an attribute with the event in it, but realized all event patches could inherit from an IEventPatch and refer to an event in the class, but this would require making all patches non static, and I'm not sure if that could potentially be a problem.
2. I was wondering if the OnXEvent(XEventArgs) methods are still required in the handler classes, I assume they were there to help transpilers raise the event, but I think it'd be easier for transpilers to call straight from the Event classes (especially if they inherit from IEventPatch and have it right inside the patch class). Changing this would also require changing every event patch so I want to make sure before I go ahead.

~~How I envision it currently:~~ How it works currently:

public static class Player //handler class
{
    public static Event<ShootingEventArgs> Shooting { get; set; } = new(); // Property required for += op
    //other handlers here
}

[EventPatch(typeof(Player), nameof(Player.Shooting))] //Patcher class will look for this
[HarmonyPatch( /*same thing here*/ )]
public static class Shooting //patch
{
    private IEnumerator<float> Transpiler() {} //Patch functions mostly the same
}

This is configurable too, so you can choose to patch the old way as well, for debugging patches and such.

Any feedback or if this is completely useless for some reason, please lmk

[Undid-Iridium]

Dumb question, are there patches that need to be patched by default no matter what the user specifies? Just curious (like a ghost patch, or something). Thanks

[rayzerbrain]

Yes, that's a good point. I haven't yet accounted for all the patches that aren't in the Patches.Events namespace, which should all be patched statically. (If that's what you meant)

[rayzerbrain]

I'm leaning away from attributes and more towards one or two interfaces to specify event patches, with the example patch/handler relationship given above. Still gonna wait for approval before finishing in case any core changes come up

[rayzerbrain]

I lied, I like attributes more now. Probably best we don't have to mess with literally every existing transpiler, plus it's technically more efficient anyway.

[rayzerbrain]

Most impl. is done but there are some events that have Exiled.Events functionality embedded in the patches. I'm gonna try and find them and seperate them into handlers instead.

[Jesus-QC]

I see it like this:

Cons:

• If something in exiled is broken we wouldnt notice until a user uses the broken patch
• More work to run exioed events that need patches yes or yes when exiled is loaded

Pros:

• Some patches not loaded?? I dont think thats even a pro

[rayzerbrain]

Look at it this way:

Would you rather have all 450 exiled servers having the same broken patch when only 50 of them even use the event? I think it's better we avoid that if we can, why would we require people to have broken patches if they don't use them?

Take for example the recent tantrum crashing issue. I can assume a very small percentage of servers use the corresponding events. If that's our fault somehow, we can lighten the blow for all the servers that don't even need it patched while we work on a fix (which might take a while). Even if that isn't our fault, we could prove it because we could figure out that the issue still happens on servers that don't have the corresponding logic patched.

Plus, this way it'll be slightly easier to detect the events that are broken because we can use deduction based on what they have patched/unpatched to know isn't an issue on our end. (see the show up command)

[Jesus-QC]

Well, that is cool. Waiting for it then. 👀

[rayzerbrain]

Mostly done and tested, but still want feedback one thing before I mark ready:

Currently I have the + ops for both event wrapper classes so all plugins don't have to be refactored to adjust. I don't know if I should mark the ops as obsolete since it's faster to just call Subscribe(), or not.

Also even though event handlers can be subscribed with += the way they are now, all plugins will have to recompile so that the types they target match, so I recommend saving this for exiled 6, cause they'll probably have to recompile anyway.

[rayzerbrain]

Just gonna open it as is, we can always obsolete stuff later if we need to

[rayzerbrain]

Probably should be Do Not Merge for exiled 6

[PintTheDragon]

This looks very exciting. SCPStats registers to events with a coroutine delay in order to ensure that it is registered last. Would this change break such functionality? From what I can tell registration is on-demand and events themselves manage the patches rather than the EXILED loader, so I'd image it wouldn't, but I'd like to make sure just in case.

[rayzerbrain]

It might; events should all be subscribed the same way and in the same order, however the time it takes to enable all plugins is bound to be much larger, because enabling a plugin requires patching all events the plugin subscribes to, which takes a good chunk of time.

A seperate conflict I did think of however is that some plugin-specific transpiler patches that target methods that Exiled also patches with transpilers may be thrown off, because it isn't guaranteed that those methods will be patched or not. If this is a big enough issue I could add some sort of method to allow a plugin to make sure those patches are always run, but there aren't a whole lot of plugins (public at least) that use their own transpilers. ScpStats should be fine in that too, cause it seems like it uses prefixes and postfixes

[Jesus-QC]

I suggest a [StaticEventPatch] attribute so it patches the event yes or yes

[rayzerbrain]

Wouldn't ensuring a patch is run be done by a plugin tho?

[Jesus-QC]

Wouldn't ensuring a patch is run be done by a plugin tho?

Yep, I mean for fix patches, but nvm cuz that ones shouldnt use the eventpatchattribute