[LIB] Harmony v1.2.0.1

[Brrainz]

I have run into what I consider an odd problem. I added this piece of code:

Code Select Expand

    [HarmonyPatch(typeof(Verse.Corpse))]
    [HarmonyPatch("SpecialDisplayStats")]
    class CorpseDisplay
    {
    }
This results in "System.ArgumentException: No target method specified for class....". The odd thing is that if I replace SpecialDisplayStats with something else, it works. The class is indeed completely empty, meaning it's not the contents of my class, which cause this issue. The question is then: what does and what do I do to solve this issue?

That method isn't a method but a property. So the underlying method is actually called get_SpecialDisplayStats(). You solve this by either renaming your string (not recommended) or by using this annotation instead:

Code Select Expand

    [HarmonyPatch(typeof(Verse.Corpse))]
    [HarmonyPatch("SpecialDisplayStats", PropertyMethod.Getter)]
    class CorpseDisplay
    {
    }

[Brrainz]

Another semi related question. When the vanilla function is based on IEnumerable and yield return, how do I append to the output in Postfix? I thought I could get __result to be a reference to a list, but I can't get that to work. I'm thinking it would be best to answer this question with an example on the wiki, which say reads __result, removes the first element and then appends it to the end. It might not be that useful, but it's simple and demonstrates both adding and removal/editing of the output from vanilla.

If you use ILSpy (or dnSpy) to look up the actual method you will be surprised to see that it is rather short:

Code Select Expand


.method public hidebysig specialname virtual
instance class [mscorlib]System.Collections.Generic.IEnumerable`1<class RimWorld.StatDrawEntry> get_SpecialDisplayStats () cil managed
{
// Method begins at RVA 0x16b2a8
// Code size 23 (0x17)
.maxstack 3
.locals init (
[0] class Verse.Corpse/'<>c__Iterator22A'
)

IL_0000: newobj instance void Verse.Corpse/'<>c__Iterator22A'::.ctor()
IL_0005: stloc.0
IL_0006: ldloc.0
IL_0007: ldarg.0
IL_0008: stfld class Verse.Corpse Verse.Corpse/'<>c__Iterator22A'::'<>f__this'
IL_000d: ldloc.0
IL_000e: dup
IL_000f: ldc.i4.s -2
IL_0011: stfld int32 Verse.Corpse/'<>c__Iterator22A'::$PC
IL_0016: ret
} // end of method Corpse::get_SpecialDisplayStats

This is because that method internally only returns an iterator object (the class name of that in this case is called '<>c__Iterator22A') that does the job. It's that one you need to patch. The class '<>c__Iterator22A' is an inner class of the class Verse.Corpse and has, as every iterator, two main methods: MoveNext() and Reset(). It's MoveNext() you need to patch.

Alternatively, you could patch the getter and just return your own instance of your own iterator implementation with like a prefix returning false. In that iterator implementation you use reflections to iterate through the original corpses iterator and yield return all the original values. Then you just yield more after the loop and you got yourself new elements added.

Sorry that this is all very complicated but that's how C# works. I did not add this to the wiki since the wikis purpose is to explain the things Harmony adds to the mix. It requires that you learn the inner workings of C# too.

[Nightinggale]

This is because that method internally only returns an iterator object (the class name of that in this case is called '<>c__Iterator22A') that does the job. It's that one you need to patch. The class '<>c__Iterator22A' is an inner class of the class Verse.Corpse and has, as every iterator, two main methods: MoveNext() and Reset(). It's MoveNext() you need to patch.

So I should add a Postfix to MoveNext(), but wouldn't that trigger each time the caller requests the next item, effectively make my method trigger and inject every second line? Alternatively it should check if it is pointing to nil and then make it point to a new list, which adds the item I want to add, but then the caller will call MoveNext() again and then it will end up pointing to nil again, which could make my code trigger again 

If IEnumerable return type is just an iterator to a list, I wonder if it would be doable to make a reference to __result, then use __result.MoveNext() to read all the contents to a new list, modify that list as needed and then make an iterator to the new list and store it in __result. While it looks good on paper, I'm concerned about leaking memory and what will happen to the new list once it goes out of scope before it's read by the caller. Do anybody know this or should I just try my luck and see what happens?

Performance is not a concern due to the lists in question being rather short and the low call frequency.

Alternatively, you could patch the getter and just return your own instance of your own iterator implementation with like a prefix returning false. In that iterator implementation you use reflections to iterate through the original corpses iterator and yield return all the original values. Then you just yield more after the loop and you got yourself new elements added.

Having a Prefix, which stops execution sounds like it's on the path to the problems Harmony should fix, which is compatibility problems with other mods. I might as well just replace the function without using Harmony if it blocks other mods from using Postfix anyway.

Sorry that this is all very complicated but that's how C# works. I did not add this to the wiki since the wikis purpose is to explain the things Harmony adds to the mix. It requires that you learn the inner workings of C# too.

I both agree and disagree with what you are saying here. Harmony is brilliant and it solves a major compatibility issue. However it only works if the mods actually use it. If the difficulty is so high that the majority of modders can't figure out how to use it even if they want to, the result will be a lot of mods not using Harmony, which defeats the purpose. My experience from years of modding is that most modders are somewhat self taught and they fight to do the best they can. You can't just tell them to use better coding theory, because they would be like "what does that mean?", then ignore you and either change nothing and make a mess or give up on modding. We don't want either.

I admit I'm not a C# guru. In fact I decided to write a mod to get to know C#. However I am an engineer (master of science no less) and I do have a strong C++ background as well as knowing a bunch of other languages and the approach to write some production code to learn a new language has served we well before and I would say it has here as well. However the problems I have run into aren't C# as such. It's an issue that it enforces low level programming as well as apparently reading assembler. While I'm not lost in doing that (particularly when I know that's what I have to do), I would say that I wouldn't except modders in general to be able to do that.

In order to get Harmony to be used by all mods (xml only excluded), it will require some very simple examples to show how to use a subset in order to get people started. I might write that myself, that is if I figure out how to actually make it work myself.

[HarmonyPatch(String, PropertyMethod)] Defines the property to be patched by name

I admit it eluded me that the second argument takes PropertyMethod.Getter and PropertyMethod.Setter. I seems obvious in retrospect, but PropertyMethod is actually not mentioned other than the line I quoted.

[Brrainz]

It is easy to say that Harmony should make all patching available to even the uninformed/uneducated modders out there. But when it comes to actually solving this problem you seem to not be able to offer a solution. That's a bit like saying that everybody should be able to drive a racing car or do figure skating or any other sophisticated task, without the requirements of training, dedication or risk. That won't happen.

If you come up with any sketch of a good solution to this problem that does not involve endless hours of work, or results in endless hours of text/video/code or other educational material that basically shortcuts the skills and knowledge required to solve these kind of patching you are talking about, please let me know and I am more than happy to implement it.

In my opinion Harmony does live on the sweet spot between easy enough to adapt for 60% of patching and powerful enough to allow more advanced coders to do their job too. It is not an educational tool.

Let me know what you think.

[Dingo]

I think results speak for themselves here. Not only do all self-respecting RimWorld modders use Harmony for things we would have detoured in the past, but the library itself has been adopted by modders for other games and even some companies.

[Nightinggale]

I have done some thinking and the main problem is not with Harmony itself. If __result is say an int, it's just an int and it can easily be modded. Also it's fairly easy to hook up, particularly after you realize how to use PropertyMethod, meaning most calls are actually fairly doable.

The problem is the clash between Harmony and IEnumerable<> because it provides an iterator rather than the data itself. It makes perfect sense to do so if you think about function calls and return values in registers and patching a compiled DLL will indeed have to consider low level stuff like that.

However this knowledge does not help me with my current problem. Iterators have read only access to the data, meaning if I want to modify the data itself, I have to write code like the following:

Code Select Expand

    [HarmonyPatch(typeof(Verse.Corpse))]
    [HarmonyPatch("SpecialDisplayStats", PropertyMethod.Getter)]
    class CorpseDisplay
    {
        static void Postfix(Verse.Pawn __instance, ref IEnumerator<StatDrawEntry> __result)
        {
            IEnumerable<StatDrawEntry> NewList = new List<StatDrawEntry>();
           
            while (__result.MoveNext())
            {
                // NewList.Add(__result.Current); <-- this line crashes the game
                StatDrawEntry temp = __result.Current; // this doesn't
                NewList.Add(temp);
            }
            // todo append modded lines to NewLine

            __result = NewList.GetEnumerator(); // causes a crash
        }
    }
However this crashes the game. The first crash is fixable using a buffer, but the second one isn't. I tried making NewList static to avoid the risk of an issue of freeing the memory once it goes out of scope, but that changed nothing.

Do anybody have an idea on how to get this working?

I think results speak for themselves here. Not only do all self-respecting RimWorld modders use Harmony for things we would have detoured in the past, but the library itself has been adopted by modders for other games and even some companies.

Point well taken and once I realized the issue is specific to IEnumerable, I kind of regretted what I wrote yesterday, or at least how I wrote it. It still doesn't change the fact that IEnumerable are widely used and that people should be able to mod the contents of such lists without reinventing the wheel.

[Brrainz]

Without the time right now to go into your issue in-depth I can at least say that I might add a helper function to the already existing transpiler tools in Harmony. Something that will allow anyone to patch enumerators on a higher level. I already did that for replacing all occurrences of a call to a method A with B inside a patched method. I might just find a generic way to do what you want.

Personally, I habe successfully patched enumerators before, I just hadn't have the time to make it generic.

[Dingo]

            while (__result.MoveNext())
            {
                // NewList.Add(__result.Current); <-- this line crashes the game
                StatDrawEntry temp = __result.Current; // this doesn't
                NewList.Add(temp);
            }
            // todo append modded lines to NewLine[/code]

Do anybody have an idea on how to get this working?

If what you want to do is read __result and then return a different IEnumerable, I would try adding a small method to the class which does this. Consider this and see if it works for you:

Code Select Expand

[HarmonyPatch(typeof(Verse.Corpse))]
[HarmonyPatch("SpecialDisplayStats", PropertyMethod.Getter)]
class CorpseDisplay
{
static void Postfix(Verse.Pawn __instance, ref IEnumerator<StatDrawEntry> __result)
{
IEnumerable<StatDrawEntry> NewList = this.modifiedResult(__result);

Log.Message("This is a debug message that shows I'm making a new IEnumerable!");

__result = NewList;
}

private IEnumerable<StatDrawEntry> modifiedResult(IEnumerable<StatDrawEntry> source)
{
foreach (StatDrawEntry entry in source)
{
if (myCustomBoolHere || !someOtherBoolIWantToBeFalse)
{
yield return entry; //Add source entry to my modified IEnumerable if I don't need to change it etc.
}

else
{
StatDrawEntry replacementEntry = entry;
replacementEntry.overrideReportText = "Hi, I'm a new entry"

yield return replacementEntry;
}
}
}
}

[CannibarRechter]

Hmmm. I didn't even know that postfix methods could change the return value. I thought only prefix methods could do that. This confuses me a lot.

In any case, your code appears to simply make a copy of the enumerator, and you say that this "crashes the game". Can you explain what the crash error is? Is there a specific exception?

[Nightinggale]

I finally got it to work!!!! 

Code Select Expand

    [HarmonyPatch(typeof(Verse.Corpse))]
    [HarmonyPatch("SpecialDisplayStats", PropertyMethod.Getter)]
    class CorpseDisplay
    {
        static void Postfix(Verse.Corpse __instance, ref IEnumerable<StatDrawEntry> __result)
        {
            // Create a modifyable list
            List<StatDrawEntry> NewList = new List<StatDrawEntry>();

            // copy vanilla entries into the new list
            foreach (StatDrawEntry entry in __result)
            {
                // it's possible to discard entries here if needed
                NewList.Add(entry);
            }

            // custom code to modify list contents
            // add vanilla leather line just to verify that output is modified
            StatDef leatherAmount = StatDefOf.LeatherAmount;
            NewList.Add(new StatDrawEntry(leatherAmount.category, leatherAmount, __instance.InnerPawn.GetStatValue(leatherAmount, true), StatRequest.For(__instance.InnerPawn), ToStringNumberSense.Undefined));

            // convert list to IEnumerable to match the caller's expectations
            IEnumerable<StatDrawEntry> output = NewList;
           
            // make caller use the list
            __result = output;
        }
    }
Yes, this will indeed just duplicate the leather line, but it's fine for testing if __result is modified. When I started having problems with IEnumerable, I decided on focusing on the GUI first precisely because it's so easy to see results ingame.

It works and quite importantly it seems to allow multiple mods to make Postfixes for this function without conflicting.

There is still a bit weirdness, which is __result only contains 3 lines while 8 are displayed ingame. It likely has to do with SpecialDisplayStats being overloaded and perhaps multiple versions are called. Luckily my goal is to add a line, in which case this doesn't matter.

If what you want to do is read __result and then return a different IEnumerable, I would try adding a small method to the class which does this. Consider this and see if it works for you:

Thanks for trying, but I couldn't get that approach to work. It did however provoke my train of thought and it might have played a role in finally getting it right. Also thanks to pardeike. You also helped me a lot in getting this to work.

[Brrainz]

See? That wasn't too hard. And we haven't violated the multiple mods compatibility either. Great work.

[Nightinggale]

Now that I got Harmony working and has a bit of experience with it, I will say that despite what I said earlier, it's absolutely great. The code I pasted earlier has worked as a template and I have made multiple mods since (only one published), making hooking up to new methods really fast and easy. Adding multiple postfixes to the same method works well and without conflicts and updating from A17 to A18 didn't even require me to recompile.

In short, great work on Harmony. I absolutely love it now. The only issue is the learning curve, but now I plan to write something on the RimWorld wiki based on my experience, which will hopefully help other people get started.

[Brrainz]

Most excellent. I am really glad you like what I created. Keep up the good work and if you find time, maybe write a tutorial. So far, I have made the Harmony wiki and a gist tutorial about Transpilers: https://gist.github.com/pardeike/c02e29f9e030e6a016422ca8a89eefc9

Cheers

[RawCode]

I think results speak for themselves here. Not only do all self-respecting RimWorld modders use Harmony for things we would have detoured in the past, but the library itself has been adopted by modders for other games and even some companies.

calling developers of alternative technologies "non self-respecting" is not ever close to reasonable.

Harmony is wrapper over unsafe and it implements exactly same single line payload that used in early code injection.

as for iteration\yeld return - Zenthar great job backfired, as it hide actual implementation from developer below tick layer of syntax sugar.
It's possible to automate recovery and redirect injections, but this harmful in longrun as developers must have at least basic understanding of C# internals and must be aware about syntax sugar and how real CIL looks like.

[CannibarRechter]

I actually don't like the decorator methods used by Harmony. It invited the developer to eschew proper exception handling. If the decorator method wesn't used, and you always had to call the manual methods, the pardeike probably would have had exceptions thrown all the way through to the caller. That's the way they should be. Instead, you often end up with silent failures. It's a bit of a debugging disaster.