Control critical hits

[dawon]

Is your feature request related to a problem?

Currently, there is no way to prevent critical damage for specific damage instances. Simply if the conditions for crit are met, it will be crit, otherwise, it won't. You can disable it in config, but that's too global.

Describe the solution you'd like.

Currently, there are two events revolving player attack: PrePlayerAttackEntityEvent and EntityDamageByEntityEvent.

Former is too soon for decision of crits and the latter is too late.

I would suggest adding something like preventCriticalHit() method to PrePlayerAttackEntityEvent, which would prevent the whole crit logic.

I will gladly make a PR for that, but wanted to ask whether something like this has chance on being accepted, before I start working on it...

Describe alternatives you've considered.

I could use PrePlayerAttackEntityEvent to give player a Blindness effect, prevent blindness packets from being sent to the player, then removing the effect in DamageEvent... But that is way too hacky with many potential issues...

Other

No response

[lynxplay]

Would fit well on PreAttack yea. Arrows already have a setter so, that ought to work out.
I personally wouldn't mind it, I don't think it is particularly the most intuitive of APIs but given how vanilla sadly works here in terms of damage computation, we can't just "revert" the critical modifier down the line.

[dawon]

Another maybe more intuitive solution would be to add a new CriticalHitEvent, that would be executed at the time when the criticality is decided... But for me that would be too many events. So I will make the PR and let's see... Thank you for the reply!

[lynxplay]

Yea I like the flag on the Pre event better than a critical strike event I think.
Definitely something that can be a PR and not too much work that it'd be terrible if other maintainers think differently.

[dawon]

Another question would be, should I make it tri-state to allow to force the crit? Or that would be too much?

true -> will be crit
false -> will not be crit
null/none -> vanilla behaviour

If not, I will just add a way to prevent the crit...