Drakulix
Force developers to clearly separate between WantsToReplace and ReplaceMethod.
Currently many mods just execute all their code in WantsToReplace and skip the ReplaceMethod call, which breaks the expected execution order.
Other mods relying on BeforeInvoke might get manipulated Arguments or similar, that should only be expected for AfterInvoke calls.
Please brainstorm about ways to programmatically force developers to do only the most necessary checks in WantsToReplace.
I came up with different ideas:
First of all, make the info-object read-only. From that point you can go several different ways:
- Add More Documentation, so Mod-Developers are aware, that they should not misuse that method
- Make the WantsToReplace-Method a delegate, that is returned by a method
- Make the WantsToReplace-Method a delegate, that is returned by a static method
The last one would be limiting, because then you can't involve the current mod-state into that decision.
And the easiest one:
- Just don't care. Mod-Developers can always misuse something. For Repos this should be a reason to not include the mod, but it is not a design fault, that needs to be forced away. (Also i haven't seen any mod do this)
Read-only and documentation is a good idea. I have not used delegates in C# yet, how would that work exactly and whats the advantage of it?
its comparable to lambda expressions/function pointers.
i imagined it like this:
public delegate bool WantsToReplaceDelegate(IInvocationInfo info);
public WantsToReplaceDelegate getWantsToReplace() {
return (IInvocationInfo info) => (info.targetMethod.Equals("abc") && (/*...*/));
}
The thing is: no-one stops you from not using complicated methods with side effects inside these delegates, because you could also implement it like this:
bool myWantsToReplace(IInvocationInfo info) {
Console.writeLine("...");
mess.withStuffHorribly();
return true;
}
public WantsToReplaceDelegate getWantsToReplace() {
return myWantsToReplace;
}
The advantage of a delegate is, that it suggests a small-expression.
Note: i wrote all this code straight from my head without looking anything up in terms of syntax and correctness.
That looks good. I will not do it statically, as there are some rather easy ways to work around that and it basically just adds a lot of overhead to mods, that still want to mess around in that function. But a normal delegate will give those devs some hints, as you mentioned.
Just one question about them. You can treat them as objects, right? So you could just build up a dictionary with delegates belonging to the hooked functions and call them directly? Would that have any performance advantages if we cache the mod functions that way?
Yes, it should have some performance advantages, but i would not expect them to be big. But when you save them in a Dictionary (Yes, they are objects in some way), mods will not 'switch' between them and that should be good.
The great thing i just noticed is: They still can access instance-variables (when not creating the delegate in a static function)
If they can still access instance variables and there are performance advantages, I am going to implement it that way for all mod functions.
i think i was a bit unclear: when comparing: storing the delegate in a dictionary or getting it from the mod class everytime i expect the dict to be faster. when comparing: calling a method of the mod or calling a delegate from a dict i think it might be faster to call the mod-method.