BepInEx / HarmonyX

Harmony built on top of MonoMod.RuntimeDetours with additional features
MIT License
364 stars 44 forks source link

Improve CodeMatcher #13

Closed ghorsington closed 2 years ago

ghorsington commented 4 years ago

Currently HarmonyX contains CodeMatcher -- a particularly useful and versatile helper class to write Harmony transpilers as a stream:

public static IEnumerable<CodeInstruction> PatchSpeedloaderRoundTypeChecksTranspiler(IEnumerable<CodeInstruction> instrs)
{
    return new CodeMatcher(instrs)
        .MatchForward(false, // Place the cursor at the start of the match
        new CodeMatch(i => i.opcode == OpCodes.Ldfld && ((FieldInfo)i.operand).Name == "Type"), // match ldfld *.Type
        new CodeMatch(i => i.opcode == OpCodes.Bne_Un || i.opcode == OpCodes.Bne_Un_S)) // match bne.un
    .Repeat(m => // Find each of the above pattern and for each match
    {
        m.Advance(1) // Move one instruction (to bne.un)
        .SetOpcodeAndAdvance(OpCodes.Brfalse) // replace bne.un with brfalse
        .Advance(-1) // Move an instruction back
        .InsertAndAdvance(new CodeInstruction(OpCodes.Ceq, null)) // insert ceq
        .InsertAndAdvance(new CodeInstruction(OpCodes.Call, AccessTools.Method(typeof(RemoveRoundTypeCheckPlugin), "TypeCheck"))); // insert call
    })
    .InstructionEnumeration(); // Convert the stream into the final instruction enumeration
}

Originally the helper was part of official Harmony but it was subsequently removed citing it not being ready yet.
We decided to bring it back and did a few fixes to make some broken methods work.

However, there are some changes that can be done to CodeMatcher to make it more usable. As an example, right now it doesn't handle exceptions all that well.

This issue contains a list of some things on how CodeMatcher could be improved. Not all of them have to be done at once, this issue just contains a list of all CodeMatcher-related improvement ideas.

List of CodeMatcher improvements

Meivyn commented 3 months ago

Is there a reason this was closed without full implementation? I am specifically interested in this part:

  • [x] Add some chainable error handling logic. Can be .OnError(Action<string> err) that handles errors of a previous match. If an error happens, InstructionEnumeration and other methods return unmodified instructions. In addition, Obsolete RepotFailure

CodeMatcher is a really convenient tool to use, the major problem with it is that .ThrowWhenInvalid would actually break the patch. Returning unchanged instructions like suggested seems like a very good addition. This can't be done with CodeMatcher, it can only be achieved with classic transpilers.

ManlyMarco commented 3 months ago

@Meivyn You can just put it in a try catch and return original list if that's the error action you're looking for.

Meivyn commented 3 months ago

Yeah, but it would look cleaner if the matcher could provide that functionality. Catching exceptions is also less efficient than just looking for a match fail.

ManlyMarco commented 3 months ago

That is true. I don't remember exactly why the original idea wasn't implemented but at this point it's unlikely I think, since the throw methods are what is the most commonly intended, given that it's really hard to recover without knowing what the changed IL is going to be.