search

UnlimitedHugs / RimworldAllowTool

A set of tools to simplify common tasks in Rimworld.
https://ludeon.com/forums/index.php?topic=17218.0
Other
60 stars 34 forks source link

Don't check if a transpiler patch was applied before applying it. #21

Closed alextd closed 6 years ago

alextd commented 6 years ago

This breaks if two mods transpile the same method (e.g. https://github.com/alextd/RimWorld-WhatIsMyPurpose/blob/c54ce65d81b95881a08278c52764868f4bee56d8/Source/TargetGizmo.cs#L233).

Both transpilers get called again on a fresh copy of the method, tossing out the original transpile (perhaps for priority's sake, the order might change and the original may be invalid).

So the second call to the transpiler here would see the global bool as "patched" and not apply the patch.

Of course if the load order put Allowtools second, it would not be the "second" time, and the patch would apply.

(Perhaps Harmony should wait to transpiling until all mods define their patches, but also transpilers don't need to check if they're already run)

pardeike commented 6 years ago

The sentence “transpilers get called again on a fresh copy of the method” is false. The second transpiler sees the transpiled instructions and thus can decide to do the right thing.

alextd commented 6 years ago

well yes they don't each get a fresh copy, a new copy is given to one, then the second~

pardeike commented 6 years ago

Yes. Transpiler functions must not have side effects or state. Because one patch (even Prefix or Postfix) can be called when another mod decides to put a new patch before or after the existing one.

Therefore, there is no easy way to submit state or information into the patch from elsewhere. Because Harmony calls your patches from across Assembly boundaries.

UnlimitedHugs commented 6 years ago

This will require further investigation. I assumed that patches are applied once, in the specified order, as @pardeike mentions. If so, we can probably achieve compatibility by using Harmony's priority annotations. Meaning, your patch could be applied after mine, regardless of mod load order. Since your code is more recent, the onus of ensuring compatibility falls on you, @alextd. Still, I'm ready to discuss this further, if need be.

UnlimitedHugs commented 6 years ago

Never mind, I see what the problem is. I'll see if I can release a fix sometime soon.

UnlimitedHugs commented 6 years ago

Released as part of 3.4.0.