MinecraftForge
Allow enchantments to modify damage based on the entity hit
Enchantments can only modify the dealt damage based on the entity's CreatureAttribute, not on the entity object itself. Therefore it's not possible to e.g. change the damage based some capability of a player. More information in the linked PR below.
I once made a PR https://github.com/MinecraftForge/MinecraftForge/pull/4052, which was eventually declined due to some thread safety concerns by Lex.
However, the issue is still present. I don't assume Lex got around talking to grum about this, but I noticed that in PlayerEntity#attackTargetEntityWithCurrentItem the entity object is passed to a CriticalHitEvent just a few lines after the enchantment damage calculation, which means it is also available to modders.
I do not exactly know what's the thread safety issue here (as I don't see any difference from other parts of MC), but if there is a potential issue, then it is already exposed by said event. So why not add the enchantment thing as well.
If you agree, I would update the PR for 1.15 and make sure to add a comment that the entity should not be modified in that method.
Is there any reason why this couldn't be done through LivingDamageEvent?
Probably better to use LivingAttackEvent as it is called earlier. However, I do not think that works.
The dealt damage is calculated in Player#attack as follows:
Total = (0.8*Att * Scale^2 + 0.2 * Att) * Critical + Ench * Scale
Where
Att AttackDamage attribute
Scale multiplier from the attack cooldown
Critical multiplier based on critical hit
Ench Additional damage caused by enchantment
During LivingAttackEvent only Total is available. The attack cooldown has been reset already, so there is no way to accurately calculate the damage caused by a potential enchantment during that event. Only workaround I can see is to cache the attack cooldown during AttackEntityEvent event. But that will get messy, because two different entities (attacker and victim) are involved and the event (or the firing method) is called from many different places.
Based on the assumption you want to modify the Ench part of that function, couldn't this be scaled just by adding to that single value e.g. (0.8*Att * Scale^2 + 0.2 * Att) * Critical + (Ench + Mod) * Scale? That Mod value could then be supplied by an event which would provide even greater flexibility in scaling with existing enchantments. This is especially since each enchantment bonus is calculated separately and summed together at the end making the function equivalent with very few changes to the underlying system. Or, at the very least, you could defer the enchantment calculation to some forge hook? I took a look at the original PR and it seemed very invasive into vanilla's default enchantment handling instead of just handling it yourself.
That sounds good. The original PR is very invasive indeed. A simple event would work just fine for me. If you think such PR would have a realistic chance of being accepted, I will work on that.
I would say so since it has the same invasiveness as CriticalHitEvent imo.
Alright, I will do this soonish. BTW thank you for having a look at this issue