Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Severing and other loot modifiers don't apply when using an item in the offhand #5283

Open
katietheqt opened this issue Dec 21, 2024 · 4 comments
Labels
1.18 Issue affects 1.18 1.19 Issue affects 1.19 1.20 Bug Issue describes unintended or broken behavior Confirmed Issue has been verified as being caused by Tinkers, or an enhancement is planned to be added

Comments

@katietheqt
Copy link

katietheqt commented Dec 21, 2024

Minecraft Version

1.18.2

Forge Version

40.2.0

Mantle Version

1.9.54

Tinkers' Construct Version

3.7.2.167

Describe your issue

When using an item in the offhand, Severing and other loot modifiers don't apply unless you are also holding a Tinker's tool in the mainhand. This is because the required loot modifier requires the mainhand item or used tool to be a Tinker's tool, but neglects to check the offhand.

The fix should be like one line so I haven't submitted a PR as it would probably be more work than a core developer fixing it manually, but if it would be helpful I can submit one.

Crash Report

No response

Other mods

N/A (issue occurs with only Tinker's Construct installed)

Tried reproducing with just Tinkers?

Yes

Does this issue affect Tinkers' Construct in 1.19.2?

Yes

Performance Enchancers

None of the above

Searched for known issues?

Checked pinned issues, Searched open issues, Searched closed issues, Checked the FAQ, Checked the in game books

@katietheqt katietheqt added 1.18 Issue affects 1.18 Bug Issue describes unintended or broken behavior Unreviewed Issue is new and is awaiting the team to review it labels Dec 21, 2024
@KnightMiner
Copy link
Member

Simply checking both hands is not the right fix, as that just means holding a severing tool in the offhand makes the mainhand better. Better fix would be to reuse our hack from looting to make severing function; we wrote that hack long after severing was coded so I just never considered it there.

@KnightMiner KnightMiner added Confirmed Issue has been verified as being caused by Tinkers, or an enhancement is planned to be added 1.19 Issue affects 1.19 1.20 and removed Unreviewed Issue is new and is awaiting the team to review it labels Dec 22, 2024
@katietheqt
Copy link
Author

katietheqt commented Dec 22, 2024

Checking both hands is the right fix I believe? This just an optimization from what I can tell - if you ran it always it should still work, but there would be a performance hit.

(the implementation of the modifier hook class already uses the hack)

@KnightMiner
Copy link
Member

Checking both would be a major API break as modifier hooks are not expecting to be potentially run twice on loot happenings. And more importantly, modifier hooks shouldn't run when they are not the hand used to kill the monster. The fix is not as simple as checking both, it requires careful consideration to ensure modifiers know what they are signing up for.

@katietheqt
Copy link
Author

I think you are misunderstanding - I am only proposing changing the condition to execute the modifier_hook loot modifier to check if either hand is a tinkers item. This does nothing except fix the bug, as the loot modifier itself is doing the correct thing - it just doesn't get the chance to run if the mainhand item is not a tinkers tool.

You can demonstrate this by holding a severing tool in the offhand - it won't apply when used to kill mobs, unless you hold any tinkers weapon in your mainhand. The mainhand weapon doesn't get the severing buff either. The issue is simply that the modifier itself doesnt get the chance to execute in the mainhand-empty case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.18 Issue affects 1.18 1.19 Issue affects 1.19 1.20 Bug Issue describes unintended or broken behavior Confirmed Issue has been verified as being caused by Tinkers, or an enhancement is planned to be added
Projects
None yet
Development

No branches or pull requests

2 participants