Merging upstream #97
Merging upstream #97
Conversation
… in HarmonySharedState. - Bump the internal version field in HarmonySharedState
Fix Unpatch default parameter
Use complex LambdaExpression walking to allow GetMethodInfo (() => myMethod) rather than GetMethodInfo(() => myMethod(null,null,null....))
…decoding_pr Ok, after consideration, especially with the fact that you need the latest language version, I decided that the risk is minimal. I will just take it as it is since less experienced users are most likely to affected due to them not running the latest language features.
They are supported when just inspecting the IL code
mentions that filter blocks are unsupported in the documentation
Improve Performance of `FindReplacement`
|
Yeah, I actually cannot reproduce the same behavior with my patch as well. It seems to happen only to this patch: https://github.com/Aeroluna/Heck/blob/master/Heck/HarmonyPatches/PlayViewInterrupter.cs#L76, at least from my testing. Using Harmony 2.12.0 broke this patch, and it was not throwing before that. Reverting to Harmony 2.10.2 fixes it. |
|
Okay, reverse patches are broken in general in 1.11.0+, affirmative. GC collects them. I'll see what I can come up with |
|
Far from perfect, but let's call it a hotfix for now |
|
That doesn't fix the inline issue with my test patch, however it seems to have fixed the broken patch I mentioned earlier. Guess I'm only missing a fix for the IL error that happens when reverse patching some methods before using this PR in BSIPA. I highly doubt those are the only bugs we are gonna encounter, but I guess it is usable enough. Thanks for your fixes. |
|
Restructured. Last push in this PR, unless fixes related to it are needed. I'm planning some structural changes of project based on this PR, but as separate one |
|
Pardon, that fix was important. Also, friendly reminder that without |
Calling PatchManager.AddReplacementOriginal and not calling SimpleNativeDetour.ChangeTarget on PlatformTriple.CreateNativeDetour result
|
As far as I can tell it should work the same way, but with two packages. Not sure about integration with jenkins though |
This reverts commit b0d85cc. Incorrect implementation
|
whats missing for this to be merged? CI? HarmonyX.Ref? |
There was a problem hiding this comment.
Sorry it took me so long to review, most of the changes are just syntax fluff which makes it hard to spot and review any material changes.
As far as I can see this is good to merge and shouldn't break binary backwards compatibility. There may be some minor source code breaks but that's fine.
It might require a major version bump still, but that's for later.
My main worry is workflows and dealing with the ref/full situation. Ideally builds and packages would stay in the same format as they are now, in my opinion at least.
If no one else raises any issues with this PR in the meantime I'll merge this later next week.
|
Theoretically I can merge netstandard references into main package, will check. |
|
Okay, there's a reason for separate package (harmony's discord). |
As far as I can see there are no incompatible changes. But now additional target - net5.0