juancarloscp52 / BedrockIfy

A Minecraft mod that implements some Minecraft Bedrock features into Java edition.
GNU General Public License v3.0
178 stars 38 forks source link

Incompatibility with Reacharound 1.1.1 #211

Closed SplendidAlakey closed 1 year ago

SplendidAlakey commented 1 year ago

MC 1.19.2 QFAPI 4.0.0-beta.25 Reacharound 1.1.1 Bedrockify 1.5.1

If both mods are installed together the game will crash on startup. I initially reported to Reacharound, but for now nothing will be done from their side: https://github.com/spAnser/reacharound/issues/9

If there's no better solution, then I figured Bedrockify could automatically disable its own reacharound mixin, if Reacharound 1.1.1 or higher is detected. So that people can play both mods out of the box.

I also made the report on 1.19.2, but both mods are also on 1.19.3, so I'm ok, if you implement the compat only for .3. Anyone, who plays on .2 can just manually disable the mixin.

Logs: https://gist.github.com/SplendidAlakey/a8b8e7f9a2426bccb9c57d2da6dc8e43

lonefelidae16 commented 1 year ago

Can confirm. Both mods use the Redirect annotation and target the same point. (And both method names are the same, too.)

If there's no better solution, then I figured Bedrockify could automatically disable its own reacharound mixin, if Reacharound 1.1.1 or higher is detected.

I am not in favor of this suggestion because it has to be considered in conflict with many other mods. This time it was just with Reacharound, but is that all?

Based on that, I checked it on Fabric; sorry I’m not on Quilt. There are two solutions.


Case 01: Do not require the redirection.

https://github.com/juancarloscp52/BedrockIfy/blob/d0b88b355194562a25e274c78250a3ea9bf08571/src/main/java/me/juancarloscp52/bedrockify/mixin/client/features/reacharoundPlacement/MinecraftClientMixin.java#L30

-    @Redirect(method = "doItemUse", at = @At(value = "INVOKE", target = "Lnet/minecraft/client/network/ClientPlayerEntity;getStackInHand(Lnet/minecraft/util/Hand;)Lnet/minecraft/item/ItemStack;"))
+    @Redirect(method = "doItemUse", at = @At(value = "INVOKE", target = "Lnet/minecraft/client/network/ClientPlayerEntity;getStackInHand(Lnet/minecraft/util/Hand;)Lnet/minecraft/item/ItemStack;"), require = 0)

Just add require = 0 and launch. BedrockIfy’s reachAround feature will be disabled, regardless of whether it is turned ON or OFF in the settings screen, if any mods appear that have a mixin with a Redirect annotation targets ClientPlayerEntity#getStackInHand.


Case 02: Change the injection method.

https://github.com/juancarloscp52/BedrockIfy/blob/d0b88b355194562a25e274c78250a3ea9bf08571/src/main/java/me/juancarloscp52/bedrockify/mixin/client/features/reacharoundPlacement/MinecraftClientMixin.java#L30-L38

-    @Redirect(method = "doItemUse", at = @At(value = "INVOKE", target = "Lnet/minecraft/client/network/ClientPlayerEntity;getStackInHand(Lnet/minecraft/util/Hand;)Lnet/minecraft/item/ItemStack;"))
-    private ItemStack onItemUse(ClientPlayerEntity player, Hand hand) {
+    @Inject(method = "doItemUse",
+            at = @At(
+                    value = "FIELD",
+                    target = "Lnet/minecraft/client/MinecraftClient;crosshairTarget:Lnet/minecraft/util/hit/HitResult;",
+                    opcode = Opcodes.GETFIELD,
+                    ordinal = 1
+            ),
+            locals = LocalCapture.CAPTURE_FAILSOFT
+    )
+    private void bedrockify$reachAroundPlacement(CallbackInfo ci, Hand[] hands, int handsLength, int inLoopIdx) {
+        final Hand hand = hands[inLoopIdx];
         ItemStack itemStack = this.player.getStackInHand(hand);

         if (BedrockifyClient.getInstance().settings.isReacharoundEnabled() && (isInSingleplayer() || BedrockifyClient.getInstance().settings.isReacharoundMultiplayerEnabled()))
             BedrockifyClient.getInstance().reachAroundPlacement.checkReachAroundAndExecute(hand, itemStack);

-        return itemStack;
     }

This code works for me, but the maintenance cost of the code is higher due to the local vars being captured. However, BedrockIfy and Reacharound will work at the same time. The Inject annotation is less likely to conflict, so compatibility with other mods will also be higher.


I can only offer these suggestions, but whatever the repo owner decides, the act of maintaining mod compatibility will be a difficult one.

SplendidAlakey commented 1 year ago

Can confirm it works on Quilt, too. Thanks!