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

Tinker's construct tools and weapons do not receive benefits from Botania's terrasteel wills #5171

Open
HighTemplar0 opened this issue Nov 15, 2023 · 7 comments
Labels
1.18 Issue affects 1.18 1.19 Issue affects 1.19 Bug Issue describes unintended or broken behavior Mod interaction Issue only happens when another mod is loaded Not enough information Insufficient information is provided in the issue to accurately respond Other mod Issue is caused by another mod, and it is best fixed in the other mod

Comments

@HighTemplar0
Copy link

Minecraft Version

1.18.2

Forge Version

Forge- 40.2.10

Mantle Version

Mantle-1.18.2-1.9.45

Tinkers' Construct Version

TConstruct-1.18.2-3.6.4.113

Describe your issue

When using any tinkers tool or weapon along with botania's terrasteel armor (And the respective wills that have an effect on a crit) the effects are not applied.

The effects are given when you deal any melee crit with any weapon, block, tool, item and even bare hands, some of them give slowness, some weakness and some armor pierce the enemy.

This bug persists even when i only install tinkers and botania, all you need is the terrasteel armor with any combination of the wills that proc on crit and any tinker's weapon or tool, then try the tool against an enemy with high armor, be it player or mob.

I am not using optifine.

This is a problem that persists in both 1.16.5 and 1.18.5 (i have not tested earlier versions)

Crash Report

No response

Other mods

Tinker's Construct
Botania
Mantle
Curios API (Forge)
Patchouli

Tried reproducing with just Tinkers?

No

Performance Enchancers

Rubidium/Embeddium

Searched for known issues?

Searched open issues

@HighTemplar0 HighTemplar0 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 Nov 15, 2023
@KnightMiner KnightMiner added Not enough information Insufficient information is provided in the issue to accurately respond Mod interaction Issue only happens when another mod is loaded Needs Crosspost Issue needs to also be reported to the other tracker or the issue needs to be linked here labels Dec 13, 2023
@KnightMiner
Copy link
Member

We fire the proper events on critical hit, so unless Botania is just bypassing the forge event entirely and using mixins, I don't see why this would happen.

Report this to Botania and link it here, they might have a better idea what is going on.

@KnightMiner KnightMiner removed the Unreviewed Issue is new and is awaiting the team to review it label Dec 13, 2023
@HighTemplar0
Copy link
Author

I posted a bug report to botania around the same time as i did here, this is the link to it
VazkiiMods/Botania#4478

Hopefully that helps, they confirmed it was a bug too. Let me know if you need anything else

@TheRealWormbo
Copy link

We fire the proper events on critical hit, so unless Botania is just bypassing the forge event entirely and using mixins, I don't see why this would happen.

Botania appears to use a combination of the Forge event and a Player mixin to apply Will effects. It uses the event itself to apply the damage modifier of the Will of Dharok (more damage if the player has low health), but for everything else it stores the target of the critical hit for the mixin to process. This processing is injected into Player::attack, right before the invocation of target.hurt(DamageSource, float).

How do Tinker tools/weapons apply their attack damage in this regard?

@KnightMiner
Copy link
Member

Tinker tools reimplement melee attack logic since the vanilla logic has basically no flexability. This is done in the item left click hook.

That said, we follow all proper forge events, and most of the enchantment hooks. Something like critical hits has a forge hook, there is absolutely no reason Botania should be using a mixin for that.

@TheRealWormbo
Copy link

This will probably need a bit of research, then. The terrasteel armor set can apply six different effects, only one of which can really make direct use of the critical hit event. Most of the others can probably use the damage event, but will need to get the critical hit information passed to them. One of them likely won't work through any of the available events though, as it needs to upgrade the damage type for a critical hit to ignore armor.

@KnightMiner KnightMiner added Other mod Issue is caused by another mod, and it is best fixed in the other mod and removed Needs Crosspost Issue needs to also be reported to the other tracker or the issue needs to be linked here labels Feb 21, 2024
@KnightMiner
Copy link
Member

Is this still an issue on the 1.19.2 version of Tinkers' Construct?

@HighTemplar0
Copy link
Author

Is this still an issue on the 1.19.2 version of Tinkers' Construct?

Yes i just tested it by replicating the tests i made originally

@HighTemplar0 HighTemplar0 closed this as not planned Won't fix, can't repro, duplicate, stale Sep 20, 2024
@HighTemplar0 HighTemplar0 reopened this Sep 20, 2024
@KnightMiner KnightMiner added the 1.19 Issue affects 1.19 label Sep 23, 2024
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 Bug Issue describes unintended or broken behavior Mod interaction Issue only happens when another mod is loaded Not enough information Insufficient information is provided in the issue to accurately respond Other mod Issue is caused by another mod, and it is best fixed in the other mod
Projects
None yet
Development

No branches or pull requests

3 participants