Harmony Transpiler Help (Solved)

Started by Kiame, October 27, 2017, 12:56:00 AM

Previous topic - Next topic

Kiame

Solution in post: https://ludeon.com/forums/index.php?topic=36406.msg374910#msg374910

This is my first time getting into the transpiler side of harmony and I think i'm close, just missing something.

Trying to add an if check around some code in PawnRenderer.RenderPawnInternal (part of the code included next w/ the change i want to add (marked with ->>))


if (!portrait || !Prefs.HatsOnlyOnMap)
{
->> if (Settings.HideAllHats)
{
...


Settings.HideAllHats is a property defined as "bool HideAllHats { get; }" I can convert this into a simple field - "bool HideAllHats" if that's easier. (... i really should refactor the settings file as it's a mess from when i first started out modding  :P)

Looking at the IL code for the same call:


IL_0256: ldarg.s portrait
IL_0258: brfalse IL_0267

IL_025d: call bool Verse.Prefs::get_HatsOnlyOnMap()
IL_0262: brtrue IL_03b7


The brtrue is what i want to duplicate with a call so I wrote the transpiler as follows:


public static IEnumerable<CodeInstruction> Transpiler(IEnumerable<CodeInstruction> instructions)
{
List<CodeInstruction> instructionList = instructions.ToList();
for (int i = 0; i < instructionList.Count; ++i)
{
CodeInstruction instruction = instructionList[i];
if (instruction.opcode == OpCodes.Call &&
instruction.operand.ToString().Equals("Boolean get_HatsOnlyOnMap()"))
{
// call Boolean get_HatsOnlyOnMap
yield return instructionList[i];
++i;
// Copy the brtrue instruction
CodeInstruction gotoEndOfForLoopIfTrueInstruction = new CodeInstruction(instructionList[i]);
// brtrue - jump to the end of for loop
yield return instructionList[i];

// Inject SettingsController.HideAllHats check
yield return new CodeInstruction(OpCodes.Call, AccessTools.Method(typeof(SettingsController), "get_HideAllHats"));
yield return gotoEndOfForLoopIfTrueInstruction;
}
else
{
yield return instruction;
}
}
}


Anything pop out to anyone? I did try AccessTools.Property originally but that threw an Error at startup.

I did try replacing the CodeInstruction for setting get_HideAllHats with OpCodes.Ldc_I4_1 so the injected return true will work but it appears not to. I'm beginning to think I cannot copy the brtrue. I have tried doing it manually but it fails at game launch: new CodeInstrcution(OpCodes.brtrue, 0x03b7)
IL_03b7 is the instruction i want to jump to -- i'm concerned about doing this anyway as the instruction points will be off anyway with my added code correct? or does harmony fix that?

In any case, when I run the game it behaves as if there's nothing different. I do know this is being injected as I can add Log messages in pre or postfix and see them.

Thanks!

CannibarRechter

Sorry. I've worked with Harmony quit a bit now, but haven't specifically done a Transpiler. Can you tell me why you're not just doing a pre or post patch method instead?
CR All Mods and Tools Download Link
CR Total Texture Overhaul : Gives RimWorld a Natural Feel
CR Moddable: make RimWorld more moddable.
CR CompFX: display dynamic effects over RimWorld objects

Kiame

#2
I am currently doing a prefix that returns false which works great.

Problem is it does not work with the AlienRace framework which uses a transpiler on the original method. If i'm able to get this to work I should be able to be compatible w/ AlienRaces. At least that's my hope.

I'll add this is for the mod "Show Hair or Hide All Hats" https://ludeon.com/forums/index.php?topic=33575.msg342365#msg342365

I have a request for this to be compatible w/ AlienRaces so a new alien race can use goggles w/o their hair being hidden

P.S. I could also consider adding the alien races code into mine but I would really prefer not to as any change on their side i'd need to change on mine as well (at least w/ the render function)

CannibarRechter

> P.S. I could also consider adding the alien races code into mine but I would really prefer not to as any change on their side i'd need to change on mine as well (at least w/ the render function)

No, that would suck for a variety of reasons, so I can see why you're doing what you're doing now. Unfortunately, my strategy was to wait for you to give me a reason that didn't justify your transpiler and steer you back, but I have no officially failed at my cunning plan. Sorry I couldn't be more help.
CR All Mods and Tools Download Link
CR Total Texture Overhaul : Gives RimWorld a Natural Feel
CR Moddable: make RimWorld more moddable.
CR CompFX: display dynamic effects over RimWorld objects

Kiame

Np!

I've also tried in the prefix to remove any helms - if they met the criteria - then in the postfix re-equip the helm; storing the helm in __state. I was getting (forget exact exception) an error. After adding a plethora of Log.warnings I still couldn't figure out the problem. I'm wondering if at that point the textures are pre-loaded and the indexing was getting off.

I may try this again and see if i was just being stupid   :P

Mehni

The author of the Alien Framework asked me to post this, since he doesn't have a forum account anymore.

public static IEnumerable<CodeInstruction> Transpiler(IEnumerable<CodeInstruction> instructions)
{
    MethodInfo hatsOnlyOnMap = AccessTools.Property(typeof(Prefs), nameof(Prefs.HatsOnlyOnMap)).GetGetMethod();
    List<CodeInstruction> instructionList = instructions.ToList();
    for (int i = 0; i < instructionList.Count; ++i)
    {
        CodeInstruction instruction = instructionList[i];

        yield return instruction;
        if (instruction.opcode == OpCodes.Call &&
            instruction.operand == hatsOnlyOnMap)
        {
            yield return instructionList[i];
            yield return instructionList[i+1];
            // Inject SettingsController.HideAllHats check
            yield return new CodeInstruction(OpCodes.Call, AccessTools.Property(typeof(SettingsController), nameof(SettingsController.HideAllHats)).GetGetMethod()) { labels = instructionList[i+2].labels};
            yield return new CodeInstruction(instructionList[i+1]);
            instructionList[i+2].labels.Clear();
        }
    }
}

Kiame

Oh! Looks like im missing labels which are still a mystery to me (i need to remidy this it appears!). I'll give it a shot. A huge thank you to the author for the code and mehni for posting this!

Kiame

#7
Hate to report back and say it did not work:

System.ArgumentException: Label not marked

The four lines including and after start of the original code:

214: opCode: [call] : operand [Boolean get_HatsOnlyOnMap()] Labels: []
215: opCode: [brtrue] : operand [System.Reflection.Emit.Label] Labels: []
216: opCode: [nop] : operand [] Labels: [System.Reflection.Emit.Label]
217: opCode: [ldarg.0] : operand [] Labels: []


I can see that we're trying to copy the nop's label to the injected call statement. I tried moving the label to different injected lines which result in TargetInvocationException. I also tried to place the nop w/o clearing the labels (and removing the label from the injection as i was getting duplicate label exception) and that did not work ether.

Any ideas?

P.S. Am i correct to assume that the indexer 'i' needs to be incremented in the case we're returning future CodeInstructions?

Mehni

idunno. You can ask erdelf directly if you join the RimWorld discord.

https://discord.gg/rimworld

Nightinggale

Quote from: Kiame on October 27, 2017, 12:56:00 AM
if (!portrait || !Prefs.HatsOnlyOnMap)
{
->> if (Settings.HideAllHats)
{
...
I haven't tried using Transpiler and it looks kind of complex, meaning I would try to work around it first. Have you tried anything like this:
Prefix()
{
  __state = Prefs.HatsOnlyOnMap;
  Prefs.HatsOnlyOnMap = Settings.HideAllHats;
}

Postfix()
{
  Prefs.HatsOnlyOnMap = __state;
}

Sure it's a quick pseudo code, but the idea is there. Place the real pref in __state, overwrite the pref with thatever you need to make the code draw as you want and then in postfix you restore the pref from __state.

Granted it's not as powerful as Transpillar, but if it works, it works and since Transpillar is giving problems, it's worth a try.
ModCheck - boost your patch loading times and include patchmods in your main mod.

Kiame

Prefs.HatsOnlyOnMap only works on the pawn profiles, not the pawns walking around on the map which I what I am doing with this.

The previous way i was doing this was overriding the Prefix and returning false. I then copied the vanilla code and added my stuff. The problem with this approach is it prevents other mods - AlienRaces in this case - from working with the mod.

Nightinggale

#11
I suspect your problem might be due to instruction count. If you add or remove instructions, you will mess up instructions like IL_0262: brtrue IL_03b7
At least I think you will because then IL_03b7 will be another instruction due to an offset error.

So basically you want to replace if (!portrait || !Prefs.HatsOnlyOnMap)withif (Settings.HideAllHats && (!portrait || !Prefs.HatsOnlyOnMap))
What if you replace all the instructions for that if statement with:
call bool YourClass::yourNewBoolCheck()
brtrue IL_03b7

You would need to add portrait as an argument and then fill out all the instructions you don't want anymore with nop. The result would be to redirect the if statement calculation and you will preserve the number of instructions, hence avoiding an offset issue.

If you are able to write static bool YourClass::yourNewBoolCheck(bool portrait) in C#, then writing the branch condition itself shouldn't be a problem.
ModCheck - boost your patch loading times and include patchmods in your main mod.

Kiame

Interesting suggestion Nightinggale

I vaguely remember at some point when i was first reading up on Harmony that it does update offsets but I couldn't find any mention of that when i was looking a week ago as that had been a concern of mine as well.

I think i'll try your suggestion out and see if i can get that to work!

Kiame

#13
HAHA success. Thank you Nightinggale for a fresh pair of eyes.

Basically I look ahead 2 instructions as I need to remove the first if check (whether this is a portrait or not). Then I replace the call to Prefs.HatsOnlyOnMap {get} with a local call from your suggestion
Whether i replace the instructions with NOPs doesnt seem to matter.


            bool found = false;
            for (int i = 0; i < instructionList.Count; ++i)
            {
                CodeInstruction instruction = instructionList[i];
                if (instructionList.Count > i + 2 &&
                    instructionList[i + 2].opcode == OpCodes.Call &&
                    instructionList[i + 2].operand == hatsOnlyOnMap)
                {
                    found = true;
                    // ldarg.s portrait
                    yield return instruction;
                    // Skip brfalse (5 nop)
                    /*for (int nop = 0; nop < 5; ++nop)
                    {
                        yield return new CodeInstruction(OpCodes.Nop);
                    }*/

                    // Call RenderHat
                    instruction = instructionList[i + 2];
                    instruction.operand = typeof(Patch_PawnRenderer_RenderPawnInternal).GetMethod(
                        nameof(Patch_PawnRenderer_RenderPawnInternal.HideHats), BindingFlags.Static | BindingFlags.NonPublic);
                    yield return instruction;
                    i += 2;
                }
                else
                {
                    yield return instruction;
                }
            }
            if (!found)
            {
                Log.Error("Show Hair or Hide All Hats could not inject itself properly. This is due to other mods modifying the same code this mod needs to modify.");
            }

...
        private static bool HideHats(bool portrait )
        {
            return SettingsController.HideAllHats || (portrait && Prefs.HatsOnlyOnMap);
        }


Now i need to fix something in the Postfix as hair is drawing under the head when pawns are facing up  :o

Nightinggale

Nice. I actually learned some stuff about Transpillar and IL from this, which could be useful at a later date. Also I'm happy it's working for you.

Quote from: Kiame on November 01, 2017, 10:38:01 PMWhether i replace the instructions with NOPs doesnt seem to matter.
That's interesting. It would seem that it can figure out something about labels or Harmony will automatically create some offset code. Either way this is good news.

Quote from: Kiame on November 01, 2017, 10:38:01 PMThe fun hack here was that I still needed to retrieve portrait variable which is an argument. The 'hack' i put in (because i don't know how to pass an argument variable in IL)
The code you replaced more or less provides the answer.
QuoteIL_0256: ldarg.s portrait
IL_0258: brfalse IL_0267

IL_025d: call bool Verse.Prefs::get_HatsOnlyOnMap()
IL_0262: brtrue IL_03b7
At the start of this, the stack is empty, then portrait is added. brfalse is removed, meaning it will not pull portrait off the stack and once the call line is reached, portrait is still there. According to what I read earlier (and haven't tested), this would cause portrait to be used as argument to the call. In case of multiple arguments, the first added to the stack is the first argument.

Quote from: Kiame on November 01, 2017, 10:38:01 PMNow i need to fix something in the Postfix as hair is drawing under the head when pawns are facing up  :o
Sound like a bunch of bald guys with overgrown beards  ;D
ModCheck - boost your patch loading times and include patchmods in your main mod.