Minecraft version - 1.20.1
Mod loader - Forge 47.1.28
Mod version - 1.7.2+1.20.1
I am using the latest version available - Mod version - Yes | Forge version - No

Describe the bug
When using mods that add 2 handed weapons, if swapping that weapon to your off hand, it deletes any item in your offhand and the weapon.

To Reproduce
Steps to reproduce the behavior:
Use a mod such a Simply Swords that offers 2 handed weapons.
Press hotkey to swap 2H weapon from main hand to off hand.
If anything held in offhand, its now deleted. At this point, the weapon will not render in the offhand in first person but it will appear in offhand slot in player inventory screen only. Press swap hotkey again and the weapon is now deleted as well.

Expected behavior
Pressing the hand swap hotkey while holding 2H weapon will do nothing.

I am unable to reproduce this issue.
I suspect this might be a compatibility issue with some other mod. Please provide information on which mod causes this, so I can move on with fixing.

Are you sure the item gets deleted?
It should just move into the offhand slot, and the offhand slot should disappear. You can still access the item from the inventory screen.

Here is a video for reference. Let me know if I can assist you in troubleshooting this in any way.

https://youtu.be/cajVDU-Mu-Y

Forge update 47.1.27+ is the cause of the issue.

Thank you for the details.
This will only get fixed when a "Recommended" version of Forge will have this issue.
Do not use latest, it is known to have such random behaviours.

Just checking in and providing update, I went through and tested every available Forge version. Forge 47.1.27 is 100% the cause, works fine with just BetterCombat and SimplySwords on all versions before 47.1.27 and is broken on all versions after.

• 47.1.27 Add LivingSwapHandItemsEvent

Something about that update deletes the offhand item when swapping 2handed weapon

Alright, I made a modpack with only Better Combat (and Cloth Config API and PlayerAnimator as those are dependency mods). No issues there as there are no 2 handed weapons by default.

I then add the Simple Weapons for Better Combat mod. Equipping any 2 handed item and then pressing F to swap to your offhand will delete your offhand item, and pressing it again deletes your weapon too. Bug is 100% reproduceable, happens every single time.

Minecraft version 1.20.1, forge version 47.1.44.

Alright, I made a modpack with only Better Combat (and Cloth Config API and PlayerAnimator as those are dependency mods). No issues there as there are no 2 handed weapons by default.

I then add the Simple Weapons for Better Combat mod. Equipping any 2 handed item and then pressing F to swap to your offhand will delete your offhand item, and pressing it again deletes your weapon too. Bug is 100% reproduceable, happens every single time.

Minecraft version 1.20.1, forge version 47.1.44.

Apparently he can't fix it. Forge update 47.1.27 added LivingSwapHandItemsEvent which apparently somehow causes this.
You'll need to revert down to Forge 47.1.26 or below to avoid the unintended behavior. But that causes problems with a slew of other popular mods, sooo idk, unbind your swap key? :(

This issue will likely be not fixed at all.
Forge as a platform is dead at this point, I am not planning to write code that specifically fixes problems of a dead platform.
As soon as NeoForge is release ready, Forge support will be dropped completely.

As stated on #troubleshooting channel of our Discord, Forge support is very complicated and time consuming, due to how Forge introduces a slew of miscelanous errors at any random time.
The primarily recommended loader is Fabric.

Good to know, thanks for clarifying

I'm having exactly the same issue, but on Fabric. Is that a different cause than with the forge version?

Hello @finnglink
I need answers for the following questions:

1. Does this happen when only Better Combat is installed?
2. What is the absolute fewest mods to install to reproduce the issue?

Okay, so I've figured out that I can't really fix this on my end. This is because you're setting the return item to ItemStack.EMPTY.

I'll be releasing these optimisations once you've done the required on your end, however, you're going to have to find an alternative way to getting the stack to return empty.

I'd suggest storing something (probably the entity) in an access, and then to return ItemStack#isEmpty as true whilst the same conditions have been met. I've checked and things shouldn't be using stack == ItemStack.EMPTY over ItemStack#isEmpty unless they're doing something very specific like in my case.

On the flip side, I could directly reference a player's inventory, but it'd be a lot more lines of code to fix it on my end rather than just moving your mixin to imo a better place.

Hey @ZsoltMolnarrr, thanks for the fast reply!
I've done some extensive testing and apparently, the "TooManyOrigins" Mod is the culprit. Once I've removed that, the problem is gone. That's probably the least expected mod. I'll go open an issue at their end.

Thank you that is some useful information, I will also reach out to them when I can.

@ZsoltMolnarrr Hopefully the above explains why I'm unable to fix this, if you're stuck on how to do the above change, I'd be happy to PR my suggestion over.

I'm bumping this issue and stating that it's happening when you have Apugli on your client.

Basically what's happening, is that 2H weapons are swapping an item to always be empty in the method below, which trips some of my code that involves checking for ItemStack#isEmpty up.

The point of the code is probably to make specific items not render and make items unusable, however, it will trip up any checks as an added side effect.

I'd look into alternatives to ensure mod compatibility in this area, as people should expect empty itemstack checks to only return when the itemstack is actually empty.

BetterCombat/common/src/main/java/net/bettercombat/mixin/PlayerEntityMixin.java

Line 89 in 05784bc

public void getEquippedStack_Pre(EquipmentSlot slot, CallbackInfoReturnable<ItemStack> cir) {

Please read this for an explanation as to what's going on with Apugli/TooManyOrigins.

Hello @MerchantPug !

Thank you for the response.
The linked code from Better Combat is crucial for dual wielding feature, and luckily has not caused major issues so far. I am not aware of any alternative technical solution I could use, in order to keep the current functionality (and not cause more issues).

I actually have an idea for your side tho.
Based on the code, the conflict seems to occour when calling getOffHandSlot (or something similar)
Can you try using inventory indexing instead to retrieve the offhand slot?
For example something like:
player.getInventory().offHand.get(0); // This code is in YARN mapping

Hello @MerchantPug !

Thank you for the response. The linked code from Better Combat is crucial for dual wielding feature, and luckily has not caused major issues so far. I am not aware of any alternative technical solution I could use, in order to keep the current functionality (and not cause more issues).

I actually have an idea for your side tho. Based on the code, the conflict seems to occour when calling getOffHandSlot (or something similar) Can you try using inventory indexing instead to retrieve the offhand slot? For example something like: player.getInventory().offHand.get(0); // This code is in YARN mapping

So I've kinda realised that it is on my end, and I thought of a fix a few days ago.

Simply put, I should be checking for ItemStack.EMPTY rather than ItemStack#isEmpty for my code to work.

Simply put, I should be checking for ItemStack.EMPTY rather than ItemStack#isEmpty for my code to work.

I am curious, why would that work?
What is the difference?:D

So, point is. Part of my code is setting empty itemstacks to be an empty itemstack that isn't ItemStack.EMPTY, ItemStack.EMPTY cannot be used because it is a single static instance, meaning that we can't link an entity via an accessor on it, because it's on all entities.

By specifically checking for equals EMPTY instead of ItemStack#isEmpty, two things will happen.

1. This bug will be fixed on my end.
2. This code shouldn't run outside of an actual ItemStack.EMPTY

Hopefully this explains it.

Thank you for the help!

This also got me to realise another optimisation with this code. I now can remove discarded entities from the cache.

You guys are great, thanks for sorting that out!

@MerchantPug Have you tried using this player.getInventory().offHand.get(0); to get the offhand item? (Instead player.getOffHandStack())

I don't just handle offhand items with my code, neither do I specifically only handle the player.
I have to handle all equipment slots and not just the player, so I use LivingEntity#getItemBySlot (mojmap)/LivingEntity#getEquippedStack in yarn.

On the flip side, I could directly reference a player's inventory, but it'd be a lot more lines of code to fix it on my end rather than just moving your mixin to imo a better place.

I also talked about your suggestion as to fixing it on my end above, here.

@MerchantPug
Have you tried using this player.getInventory().offHand.get(0); to get the offhand item?
(Instead player.getOffHandStack())

As mentioned, the code in question is a crucial part of Better Combat.

Changing this code is problematic, for the following reasons:

• It is difficult to find an alternative solution, that actually works, and also does not increase the complexity (For example: stateful solutions are not allowed for this code)
• Any seemingly good solution is found, lots of testing is required because of how many modpacks using BC, and how much features this code change can break potentially

Fine... I'll implement this band-aid fix on my end...

I'm bumping this issue and stating that it's happening when you have Apugli on your client.

Basically what's happening, is that 2H weapons are swapping an item to always be empty in the method below, which trips some of my code that involves checking for ItemStack#isEmpty up.

The point of the code is probably to make specific items not render and make items unusable, however, it will trip up any checks as an added side effect.

I'd look into alternatives to ensure mod compatibility in this area, as people should expect empty itemstack checks to only return when the itemstack is actually empty.

Related issue MerchantPug/apugli#51

Thank you for the cooperation!

This affects me as well. It can also delete the weapon itself.

This issue is still not actionable for me, due to not being able to reproduce.

I need answers for the following questions:

1. Does this happen when only Better Combat is installed?
2. What is the absolute fewest mods to install to reproduce the issue?

Fixed in Apugli, will update TooManyOrigins with this new Apugli now.

Thank you for the fix!