UnlimitedHugs / RimworldHugsLib

A lightweight shared library for Rimworld modding.
Other
245 stars 59 forks source link

Spurious dangerous detour warnings for detours that use the 'magic' _this_ parameter. #14

Closed FluffierThanThou closed 7 years ago

FluffierThanThou commented 7 years ago

I'm detouring an anonymous action in Birds and Bees, which has no parameters. Because I need access to the instance, I'm detouring to a static class and my target, let's call him Foo( object _this ) uses the 'magic' first parameter to get that instance.

HugsLib however, is complaining about parameter counts. I can't/don't want to subclass the Type I'm detouring because the type itself is an iterator block created by the compiler - it's already messy enough as it is.

Target of the detour; https://github.com/FluffierThanThou/BirdsAndBees/blob/master/Source/Fluffy_BirdsAndBees/Detours/_JobDriver_Lovin.cs

UnlimitedHugs commented 7 years ago

I can add support for that- your link is 404, though.

For now, feel free to ignore the warning, or go with Harmony.
I'm considering to drop all detour-related features for the A17 update and have everything use Harmony directly, so you could save yourself some updating work here.

FluffierThanThou commented 7 years ago

https://github.com/FluffierThanThou/BirdsAndBees/blob/master/Source/Fluffy_BirdsAndBees/Harmony/_JobDriver_Lovin.cs is the correct link, sorry about that.

And I'm ambivalent on dropping detours in favour of harmony. In cases where I want to replace the whole method detours are more convenient, and Harmony's attributes on a class level leave something to be desired - which is why I left this particular detour a detour and re-implemented the others as pre/postfixes. Then again I can see it might feel like a duplication of effort on your side.

UnlimitedHugs commented 7 years ago

Seems like a prefix that returns false would do the job. Still, whatever feels right to you. I will make the fix on the side of HugsLib in any case. I'm waiting on some feedback on another issue, so I'll get an update out when that one is cleared up.

FluffierThanThou commented 7 years ago

no worries, I'm in no rush. And yeah, a prefix returning false would work - but it's already a complex detour, I didn't want to make it any more complex.

UnlimitedHugs commented 7 years ago

Included the fix in 2.4.2. By the way, you don't have to use object as the parameter type- IEnumerator or any other type the iterator inherits from will also work.