CitiesSkylinesMods / TMPE

Cities: Skylines Traffic Manager: President Edition
https://steamcommunity.com/sharedfiles/filedetails/?id=1637663252
MIT License
571 stars 85 forks source link

Fix incorrect target method for harmony patches #1611

Closed krzychu124 closed 2 years ago

krzychu124 commented 2 years ago

This bug was not spotted before probably because we don't use __instance parameter (it'd be null because of invalid cast)

kianzarrin commented 2 years ago

Why haven't this caused any functional problems before? is it only a performance issue?

kianzarrin commented 2 years ago

should we get rid of the unused parameters too? __instance and data ?

krzychu124 commented 2 years ago

Ok... I know why it didn't cause any issues... Both target methods are static so instance parameter does not make sense here (probably Harmony doesn't even bother checking for instance parameter in patch declaration and everything is done by some param name switch..case then passed with param index)

kianzarrin commented 2 years ago

Both target methods are static

Interesting. but that wasn't what I was trying to ask. I mean if we patch wrong methods then not only the patch would be ineffective but also it can cause problems. how that has not happened here.

kianzarrin commented 2 years ago

OH wait I got my answer. we needed to patch both methods and we swapped them. If both patches are doing the exact same thing it might be a good idea to merge them together.