Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wrong decode Bge_Un_S, decoding as Bge_Un ... #69

Open
deltaone opened this issue Mar 26, 2023 · 4 comments
Open

Wrong decode Bge_Un_S, decoding as Bge_Un ... #69

deltaone opened this issue Mar 26, 2023 · 4 comments

Comments

@deltaone
Copy link

deltaone commented Mar 26, 2023

2023-03-26_222846
2023-03-26_222902

Use this patch function, original Harmony - ok, HarmonyX - failed ...
After debugger - HarmonyX decode Bge_Un_S as Bge_Un ...

		static IEnumerable<CodeInstruction> Transpiler(IEnumerable<CodeInstruction> instructions)
		{			
			return new CodeMatcher(instructions)
				.MatchEndForward(
					new CodeMatch(i => i.opcode == OpCodes.Ldsfld  && ((FieldInfo)i.operand).Name == "worldSurface"),
//					new CodeMatch(OpCodes.Bge_Un_S),
					new CodeMatch(OpCodes.Bge_Un),
					new CodeMatch(OpCodes.Ret))
				.ThrowIfInvalid("MinecartDiggerHelper.TryDigging(): Error! Can't find pattern!")
				.SetAndAdvance(OpCodes.Nop, null)
				.InstructionEnumeration();
		}

@Neutron3529
Copy link

this might be feature
since Bge_Un_S and Bge_Un do the same thing.

@ManlyMarco
Copy link
Member

It's on purpose. I don't remember what the reasoning was but it was solid, Horse should know. https://github.com/BepInEx/HarmonyX/blob/46b77d08eac32f8d17f0cbe761bd74cd2be21ff7/Harmony/Internal/Patching/ILManipulator.cs#LL25C64-L25C64

@RoboPhred
Copy link

I'm running into this same issue. I was hoping to convince a project to switch to HarmonyX but since it seems HX rewrites instructions its breaking transpilers of other stuff built with it...

Would you be willing to accept making NormalizeInstructions an optional step that can be disabled?

@ManlyMarco
Copy link
Member

Sadly it looks like Horse is on a hiatus so someone else that knows what's up will have to look into this. I'm worried about changing this in case it breaks existing mods, since I assume that fixing some edge case is why the commit happened in the first place. It'd be nice if there was an easy way to compare results between fix and no fix in bulk and do it for a bunch of games...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants