Sjouwer / pick-block-pro

An advanced client side Block, Entity, NBT, Tool and ID picker for minecraft
https://modrinth.com/mod/pick-block-pro
GNU Lesser General Public License v3.0
9 stars 5 forks source link

@Inject with unconditional cancel at "HEAD" should be replaced with @Overwrite #19

Closed supersaiyansubtlety closed 1 year ago

supersaiyansubtlety commented 1 year ago

https://github.com/Sjouwer/pick-block-pro/blob/1.20/src/main/java/io/github/sjouwer/pickblockpro/mixin/MixinMinecraftClient.java#L13

This has the same effect as an @Overwrite except that when other mods mixin 1) if their inject aren't required there's no log of it (which there should be) 2) if their injects are required they don't fail (which they should)

The current inject just makes issues harder to track down.

Sjouwer commented 1 year ago

That's not possible because of Fabric API injecting into it too: https://github.com/FabricMC/fabric/blob/1.20.1/fabric-events-interaction-v0/src/client/java/net/fabricmc/fabric/mixin/event/interaction/client/MinecraftClientMixin.java#L67

It's a rather difficult situation, same with the issue with ICT. Since PBP changes so much I have no clue how I could work around this issue properly.

supersaiyansubtlety commented 1 year ago

Could PBP use the FAPI event?

If it did, I think that would actually fix #18

Sjouwer commented 1 year ago

that might actually be possible, I'll look into that this weekend

Sjouwer commented 1 year ago

In the events branch I've added the 2 Fabric API events at pretty much the same locations. ClientPickBlockGatherCallback: Right after getting getting the raytrace hit ClientPickBlockApplyCallback: Right before the inventory manager does its job

I believe that should at least resolve this issue in the best way possible Though I've got a feeling this won't be enough for #18, though I'll explain that over there