[Feature Request] Compatibility with Utility Belt

[SplendidAlakey]

I read project readme, checked client side and server side configurations, and no such feature exists - YES

Is your feature request related to a problem? Please describe.

Currently, trying to attack with a weapon equipped on the utility belt from the Utility Belt mod will result in feinting instead.

Describe the solution you'd like

Allow weapons in a utility belt to be used properly.

Describe alternatives you've considered

Not using anything that is considered a weapon with a utility belt.

Additional context

Created an issue on Utility Belt's side as well: https://github.com/JamCoreModding/UtilityBelt/issues/6

An example with a sword, holding LMB:

Logs, if necessary: https://gist.github.com/Footage2-Amply-Pounce/b110c3d394a5bb965fc7ed29101ec3d5

[Jamalam360]

👋 hi better combat developers...this is probably an issue on my side but I'll look into it and post a comment when I do.

[ZsoltMolnarrr]

Hello @Jamalam360
It would be appreciated if you could take a look.
Better Combat has to do mixins for retrieving offhand items in order to two-handed wielding and dual wielding features to work, so there may be a major incompatibility.

[Jamalam360]

Currently browsing on my phone, so not ideal, but I think it might be related to this inject

I might be able to get away with just specifying a higher priority on my mixin so it injects first, but not sure yet

[ZsoltMolnarrr]

Yes this is an important one.
However, shouldn't Better Combat have the higher priority in this case?
The idea is Better Combat does a filtering here, so it enables/disables the offhand slot completely. After the code is done doing this, your code would be free to manipulate what item to be returned exactly.

[Jamalam360]

After much debugging, I found the issue.

This line checks if player.getMainHandStack() != upswingStack, which always returns true when the item is in the tool belt.

This is because the tool belts inventory is stored in NBT, and player.getMainHandStack() reads the NBT. This means that the stack isn't guaranteed to be the same instance, even though it's actually the same stack (I verified this with a debugger).

@ZsoltMolnarrr do you think it would be appropriate for you to change the condition in that if to check for item stack equality, rather than instance equality?

[ZsoltMolnarrr]

Do you mean to use this function?

    private boolean isEqual(ItemStack stack) {
        if (this.count != stack.count) {
            return false;
        }
        if (!this.isOf(stack.getItem())) {
            return false;
        }
        if (this.nbt == null && stack.nbt != null) {
            return false;
        }
        return (this.nbt == null || this.nbt.equals(stack.nbt)) && this.areCapsCompatible(stack);
    }

[ZsoltMolnarrr]

    public static boolean areEqual(ItemStack left, ItemStack right) {
        if (left.isEmpty() && right.isEmpty()) {
            return true;
        }
        return !left.isEmpty() && !right.isEmpty() ? left.isEqual(right) : false;
    }

[Jamalam360]

isEqual should work. Mojang is weird and doesn't provide a proper implementation of equals in ItemStack...