Drakulix / ScrollsModLoader

A Mod Loader Framework for the game "Scrolls" by Mojang
www.scrollsguide.com/summoner
GNU Lesser General Public License v2.1
15 stars 9 forks source link

Force developers to clearly separate between WantsToReplace and ReplaceMethod. #13

Open Drakulix opened 10 years ago

Drakulix commented 10 years ago

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.

MaPePeR commented 10 years ago

I came up with different ideas:

First of all, make the info-object read-only. From that point you can go several different ways:

The last one would be limiting, because then you can't involve the current mod-state into that decision.

And the easiest one:

Drakulix commented 10 years ago

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?

MaPePeR commented 10 years ago

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.

Drakulix commented 10 years ago

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?

MaPePeR commented 10 years ago

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)

Drakulix commented 10 years ago

If they can still access instance variables and there are performance advantages, I am going to implement it that way for all mod functions.

MaPePeR commented 10 years ago

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.