TelepathicGrunt / RepurposedStructures

Reusing and modifying vanilla structures for extra variety!
https://www.curseforge.com/minecraft/mc-mods/repurposed-structures
GNU Lesser General Public License v3.0
164 stars 26 forks source link

Mixin conflict with Simply Improved Terrain #308

Closed SmittyTheGod closed 1 year ago

SmittyTheGod commented 1 year ago

There is a conflict between the two

TelepathicGrunt commented 1 year ago

You’ll have to be more specific. Can you give me the latest.log file after getting conflict? What Mc version? Modloader?

SmittyTheGod commented 1 year ago

Yeah, sorry about that. I got help on the curseforge discord and was told to report this. 1.20.1 forge latest.log

TelepathicGrunt commented 1 year ago

The issue is simply improved terrain used an overwrite mixin which broke my safe mixin. Overwrites are dangerous and discouraged for this reason. The fix would have to be on simply improved terrain’s end

https://github.com/KdotJPG/Simply-Improved-Terrain/blob/7848e329fbed0741f257e629aafb0935ec00b7c0/common/src/main/java/jpg/k/simplyimprovedterrain/mixin/MixinBambooFeature.java#L53

my mixin for reference https://github.com/TelepathicGrunt/RepurposedStructures/blob/0798d713dac0484f5e6f2c038fac0d9696e87598/common/src/main/java/com/telepathicgrunt/repurposedstructures/mixins/features/FixBambooPlacementMixin.java

KdotJPG commented 1 year ago

Good find.

The reason I had used @Overwrite was because an @Inject performing the same task would bar the kind of interplay you're after entirely. That is, an @Inject that calls a modified copy of the code then cancels by setting the return value.

I would think it would work as-is, since the four calls to setBlock() are in the same order and perform the same duties. My goal was to preserve compatibility as much as possible by leaving as much of the code in the same place as possible, so I'm curious why it's not working as expected. it must be the local variable capture.

In any case, I think I can change it to a few targeted mixins with a bit of finagling.

TelepathicGrunt commented 1 year ago

I raised my mixin's priority and got around the overwrite. Overwrite mixins, if done, should be lower priority as an overwrite and inject at same priority will explode. Update RS and it'll work with Simply Improved Terrain right now