search

BepInEx / HarmonyX

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

Bugfix attempt for regression in PR #77 #89

Closed StarFluxGames closed 6 months ago

StarFluxGames commented 8 months ago

Please Note I'm not sure if this is a sure-fire solution, although it seemed to work fine for myself.

Regarding the regression in #77 I speculated that this was caused by removing instructions, potentially offsetting labels and blocks, rather than removing the instruction, I've adjusted it to a Nop.

In my recreated test environment this seems to have solved the regression mentioned, so any input would be appreciated.

Please note I really have no clue what I'm doing... This worked for me so I'm hoping it doesn't break anything else

StarFluxGames commented 8 months ago

It does appear to be working, the labels are the same as in #77 and the regression doesn't happen any more.

Not sure if intended, but the parameter is not nulled out. It does make it easier to debug but might potentially mess with transpliers? image

One thing I noticed is that in some cases where there were duplicate jumps some IL jumped to the second one, not the first. (IL_0068: br IL_00c0 on the right in the screenshot) image In the cases I noticed, the jump was always to the very next instruction so a nop was essentially the same, but I'm worried there may be cases where this is not true. I guess it would be safer to copy the first jump's target label over to the second jump? Or maybe the order should be swapped, where the first jump is noped and the second jump uses the correct label?

Happy to try swapping them around.

StarFluxGames commented 8 months ago

It does appear to be working, the labels are the same as in #77 and the regression doesn't happen any more.

Not sure if intended, but the parameter is not nulled out. It does make it easier to debug but might potentially mess with transpliers? image

One thing I noticed is that in some cases where there were duplicate jumps some IL jumped to the second one, not the first. (IL_0068: br IL_00c0 on the right in the screenshot) image In the cases I noticed, the jump was always to the very next instruction so a nop was essentially the same, but I'm worried there may be cases where this is not true. I guess it would be safer to copy the first jump's target label over to the second jump? Or maybe the order should be swapped, where the first jump is noped and the second jump uses the correct label?

image

I'm struggling to replicate your issue here. Using the same game, environment, and version of Harmony, mine isn't jumping to the second nop'd in-fact there is no nop here.