Better Combat [Fabric & Forge]

Better Combat [Fabric & Forge]

21M Downloads

[Feature Request] Compatibility with Utility Belt

SplendidAlakey opened this issue ยท 8 comments

commented

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:

2022-08-26_15 17 28

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

commented

๐Ÿ‘‹ 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.

commented

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.

commented

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.

commented

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

commented

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?

commented

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);
    }

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

commented

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