Let's resolve incompatibility with IPT2 together

[bloodypenguin]

Dear Mr. Negoescu,

You made a very impolite move deliberately making your mod incompatible with my Improved Public Transport 2 without warning me. We could have resolved that together but you don't even respond to my messages in Steam. I'm still waiting for a response from you and hopefully we'll be able to resolve the incompatibility together.

Kindly regards, BloodyPenguin

[VictorPhilipp]

Hey Mr. BloodyPenguin ;) Sorry, I have not intended to deliberately cause any issues. The only thing I have added in the latest update is the extended mod compatibility check in order to be able to check whether certain issues are caused by incompatibilities. And it seems to work flawlessly ;-) I see that TMPE overrides the ReleaseVehicle method since July 2016: https://github.com/VictorPhilipp/Cities-Skylines-Traffic-Manager-President-Edition/blob/3624aa889da66ad29d4a887ffe3ba6dcc1e9a138/TLM/TLM/Custom/Manager/CustomVehicleManager.cs

I have still to perform further checks whether TMPE really needs to override the method in question but I don't think that there is anything to do on your side. Hopefully, I can remove the detour on my side.

How about creating a mod that allows us to add hooks into base game methods such that multiple mods can modifiy the same methods? :-)

Regarding my responsiveness: I generally avoid using the steam chat client because messages are not preserved and incoming chat notifications do not really seem to work for me - e.g. I did not get any notification that you sent me a message over steam chat.

[DaEgi01]

How about creating a mod that allows us to add hooks into base game methods such that multiple mods can modifiy the same methods? :-)

We already have that. Check out https://github.com/pardeike/Harmony Its not a mod though but an alternative to the detours lib.

[VictorPhilipp]

So essentially Harmony introduces aspect-oriented programming for C#? Wow that's really great! As a starting point I will try to apply Harmony on this issue.

[DaEgi01]

Well, not really AOP as I understand it, but it can be used as such. It messes directly with IL. For it to work with another mod, the other mod must also use harmony though. So both TM:PE and IPT2 must incorporate the lib. Each change (patch) made with harmony can be identified by harmony from anywhere else and the behavior can be adjusted appropriately.

[VictorPhilipp]

As you already said it, @DaEgi01 - using Harmony works as long as every mod uses it instead of manual detouring. I just tested a Harmony-driven detour (they call it "patch") of the ReleaseVehicle method together with the previous build of IPT2 (which still has the ReleaseVehicle detour): My code does not get executed.

Anyway, for a next test I am going to replace all manual detours with Harmony patches to see

1. whether any problems arise, and
2. if it is still possible to check for mod incompatibilities (when other mods deploy manual detours).

For the live version I will prevent the mod compatibility error message from popping up (players can disable it in the options dialog anyway, but currently it is displayed when running TMPE with default settings).

[VictorPhilipp]

👍 Good news: Verifying detours also works for methods that are patched with Harmony. 👎 Probably bad news: Game methods that call their base class's implementation may cause problems (and there are many): https://github.com/pardeike/Harmony/issues/83

[bloodypenguin]

Hi! I think that moving to Harmony is not necessary. I have already resolved the incompatibility on IPT2's side. As long as TM:PE calls ReleaseVehicle so that the method I've detoured gets invoked, I'm OK with the current state of things. Issue can be closed. On 5/28/2018 1:37 AM, Victor-Philipp Negoescu wrote:

👍 Good news: Verifying detours also works for methods that are patched with Harmony. 👎 Probably bad news: Game methods that call their base class's implementation may cause problems (and there are many): pardeike/Harmony#83 https://github.com/pardeike/Harmony/issues/83

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/VictorPhilipp/Cities-Skylines-Traffic-Manager-President-Edition/issues/138#issuecomment-392388143, or mute the thread https://github.com/notifications/unsubscribe-auth/ALLPUe8guA6obw_QhbfcbhvrNciBQRvEks5t2zidgaJpZM4UPERf.

[VictorPhilipp]

I ported all detours that do not modify base game functionality to Harmony (prefix/postfix) and included the RedirectionFramework repo for all remaining detours.

[pardeike]

I am not sure that calling base implementations from a patch is technically possible. Patches (i.e. prefixes) are static methods that are called from within the (modified) original and get the current instance passed to them as an argument. The question now is how to call a method on Foo that is extended by Bar if all you have is an instance of Bar inside a static method.

[pardeike]

The original issue on the Harmony github suggests to use a delegate to the base method stored in a field during the initial patching. That gives you almost optimal speed (only a transpiler can give you the optimal speed if you need it).

[pardeike]

Actually, after discussing it in the Harmony discord, we found that the MethodInvoker (or simply creating a delegate yourself from the MethodInfo of the method you want to call via reflections) works well and has almost no performance hit. Doing so during patch time and storing the delegate in a field makes for pretty compact and readable code too. The latest master on Harmony has a fix for MethodInvoker to access restricted methods. The v1.1.1 release is planed in the coming days.