[1.8] IAllowItem not checked for offhand

[coolAlias]

I'm trying to make a 2-handed weapon that can be held in either hand. Obviously this should preclude anything from being held in the other hand. Looking at the BG2 TwoHandedWeapon class, it seems that this is not currently supported, despite what one might think reading the Java-docs.

IOffhandWield claims that "compatibility will be determined externally against the mainhand item (in right hand), if any", yet implementing IAllowItem#allowOffhand to return true if either hand is null still allows the 2-handed weapon to be held in the off-hand even when there is another item held in the main hand.

The issue stems from BattlegearUtils#isMainHand in that it immediately returns true if the main hand is null without checking the offhand slot for IAllowItem, or perhaps rather the issue is that #isOffhand doesn't check the IAllowItem interface, but only IOffhandWield which is much less flexible due to the main-hand being unknown.

Forcing 2H weapons to be held in the main hand does make sense, but I'm sure there are plenty of people that would enjoy wielding them as lefties and it wouldn't be too difficult to modify the utility methods to allow it.

[GotoLink]

For now, I'd simply like to point out that the original design was "strict two-handed weapons go into main hotbar, dual slots are for dual-handed weapons". Indeed TwoHandedWeapon is specialized on the right hand side (IOffhandWield implementation returns false). But the javadoc should be on-point anyway. IAllowItem is for "right hand" items to implement. IOffhandWield is for any item to decide if they care about their slot side/player. Note by the point this interface is checked, the right hand item is already available through EntityPlayer#getCurrentEquippedItem(), thus you can still be specific here. Since items are added one after the other in the inventory. I'll check for corner cases though.

[coolAlias]

Yeah, I figured that was the original design after looking at your TwoHandedWeapon class implementation, but it wasn't obvious from the Java docs which had lead me to believe I could have it equipped in either hand while preventing anything else from being equipped.

A few points, though, if you are going to modify it:

1. From the inventory interface, #getHeldItem and #getCurrentEquippedItem still check the hotbar slots, not the corresponding mainhand for the offhand slot, so you are only able to place a 2H weapon in the offhand when not holding anything in the hotbar:

@Method(modid="battlegear2")
@Override
public boolean isOffhandWieldable(ItemStack stack, EntityPlayer player) {
    return player.getCurrentEquippedItem() == null; // doesn't actually check main hand slot
}

1. Once the 2H weapon is placed in the offhand slot, items can still be placed in the main hand slot so long as they do not implement IAllowItem to check the offhand slot; even if they did, there is no guarantee they would check the offhand slot for compatibility as the implementation is up to each implementing class, despite what the docs claim. When placing an item in the offhand slot, the main hand is checked first, but not the other way around.

Anyway, it's not a big deal - lefties have been putting up with right-handed characters for a long time, I don't think one more case of it is going to kill them (but I'm sure they would appreciate it if it was supported!) ;)

EDIT: Not sure why, but the 2nd point appears as a '1.' even though when editing it is '2.'...

[coolAlias]

Sorry to bother you with yet another small Java-doc related point: IAllowItem says that you should return true if the offhand item can be placed in the offhand slot, checked using the item in the main hand, but if you return false, you can't even place the item in the main hand.

I know you can return offhand == null and it will work as intended, but with that description and the name of the method, I would expect returning false to mean 'no, you can't wield an item in the offhand', not 'you can't wield this item in any hand'.

Don't you just love updating? :P

[GotoLink]

Your first point is definitely a bug. Will squash it right this instant. Second point...i'll check it out.

Javadoc could be more precise, i admit.

[GotoLink]

Now that i have more time to think, if IAllowItem was called for both hands, what would be the use of IOffhandWield ? Each interface is dedicated to a side, because it is much simpler for the usage...

And if you want to make a weapon for left hand only, how are you handling vanilla inventory, which is only right hand ? Note those interfaces are only making sense on "battlemode", where there are two hands, and i am not going to disrupt vanilla slots. There is already enough fuss about BG slots "not being compatible with everything" from users.

About your latest message, i don't understand what you meant. If a right hand item allow no combination of wielding item, nor the opposite hand to be void, then obviously it is not going to be wield. Because there is no possible satisfactory wielding state. As per javadoc, it is about combinations.

[coolAlias]

Yes, I was only talking about battlemode here, not vanilla inventory. I'm not making an item left-hand only, but an item that is 2-handed that can be placed in either the left or the right slot, i.e. 'left-hand forward' or 'right-hand forward' kind of thing. It's not really important, just something random I was trying, so we can forget about that bit.

IOffhandWield is fine as is - it determines if a weapon can be placed in the offhand slot; BUT, once it is in that slot, I can put ANYTHING in the main hand slot, even if the offhand slot item shouldn't be compatible with the main hand slot item.

This is because there is no guarantee the main hand item implements IAllowItem or that its implementation thereof checks the particular case for the offhand.

As an example: ItemX can be placed in either hand (IOffhandWield returns true), but for whatever reason cannot be wielded together with vanilla ItemSword (IAllowItem returns false if either hand is ItemSword).

Now, I place ItemX in the offhand, and then a diamond sword in my main hand. Since the diamond sword obviously doesn't implement IAllowItem, it is allowed to be placed, but ItemX should be prohibiting it - if ItemX#IAllowItem were called even when it is in the offhand, it could prevent the combination as intended.

So basically my proposition is that IAllowItem be called for any item in any slot that implements it any time an item is placed in either hand to ensure that the 2-item combination (or 1-item + null) is allowed by BOTH items, whereas IOffhandWield only cares if an item can be in the offhand slot or not.

Does that make sense?

[Alaberti]

Is that why I can have a bow in my offhand and a sword in my right?

[GotoLink]

@coolAlias I understand. Here is the new proposed javadoc and behavior, after suggested fix. Please ask if you have any trouble with it. Should be compatible with previous implementations (for right hand items, left hand's were not supported).

@Alaberti Neither vanilla bow nor sword use the interfaces. Their behavior is designed by Battlegear mod.

[coolAlias]

@GotoLink that looks great!