MagmaGuy / EliteMobs

This is a spigot plugin that aims to extend Minecraft's survival endgame by making mobs more interesting.
http://www.magmaguy.com
GNU General Public License v3.0
157 stars 58 forks source link

Elite Mobs do not handle damage from non-EM weapons properly #102

Closed SuperKael closed 1 year ago

SuperKael commented 2 years ago

In EliteMobDamagedByPlayerEvent.EliteMobDamagedByPlayerEventFilter.onEliteMobAttack, a 'damage' value is obtained as event.getFinalDamage(). This value is then passed through Round.twoDecimalPlaces(damage), before eventually being passed into event.setDamage(EntityDamageEvent.DamageModifier.BASE, damage). This causes a problem - now, that final damage is being set as the raw damage. This causes Elite Mobs to experience double damage reduction against non-EM weapons, or in other words, their armor is twice as effective as it is supposed to be.

As an example, lets say I hit test_boss.yml for 15.1875 damage with a non-EM weapon. After factoring in the boss's armor, the final damage is ~11.9032. However, EM then takes this final damage value, rounds it, and sets it as the raw damage - this means, with 11.9 as the new raw damage value, the final damage accounts for armor again and is now ~8.5442 - quite a bit lower than it should have been.

It appears that Elite Mobs tries to prevent this by using this:

            //nullify vanilla reductions
            for (EntityDamageEvent.DamageModifier modifier : EntityDamageByEntityEvent.DamageModifier.values())
                if (event.isApplicable(modifier))
                    event.setDamage(modifier, 0);

However, this does not really work. The DamageModifier API is deprecated, and Spigot recalculates all of these modifiers whenever setDamage(double modifier) is called.

MagmaGuy commented 2 years ago

Thanks for reporting this issue. I can confirm that this used to be a problem when using previous versions of EliteMobs. I now use a different method for applying the damage, and testing it out on elites seems to yield the correct amount of final damage for vanilla items (in my case I tested a normal vanilla sword which I believe is meant to deal 7 damage, which it did).

If you find circumstances in which this does not hold true, please let me know.

Please note that the combat was overhauled for the latest public version, 7.3.13.

SuperKael commented 2 years ago

This issue was experienced with EliteMobs version 7.3.13, and the EliteMobs code that I analyzed to confirm the issue was also from that version. I suspect the reason it works fine in your testing is because no other plugins are calling setDamage. However, I am testing with a handful of plugins, and any number of them could be doing it. setDamage should be a 'safe' method to call - but EliteMobs is making this not so. If I am not mistaken, it seems to me like it would be simple for EliteMobs to fix the issue by just not resetting the damage modifiers, and using getDamage instead of getFinalDamage. As far as I can tell, the only thing that this damage value is actually used for is checking to see if an ender dragon is going to be killed by it, in which case, a workaround or a second variable could be used for this.

MagmaGuy commented 2 years ago

Ah yes reading back I see where this went wrong. Some wires got crossed during a rewrite, it should indeed be using getDamage.

That being said, what you said is half wrong in the specific context of EliteMobs.

To make a very long system short, in order to simplify how EliteMobs does health/damage all armor is visual only and all vanilla damage reductions are ignored outside of the ones EliteMobs mechanically enforces for Custom Boss theming. That's why the modifiers get messed with specifically when dealing damage to Elites.

If you are wondering why I don't just use vanilla armor values, you can read the formulas here if you scroll down. It's very hard to design balanced combat when you need to check several graphs just to understand how much damage players would be dealing.

If I was to not modify the damage modifiers for vanilla weapons people would find that certain bosses can be killed instantly with an elite diamond sword with no enchantments on it but is nearly unkillable with a vanilla diamond sword with enchantments on it. Additionally damage from other plugins would get reduced to account for armor that was only ever meant to be visual, drastically reducing their efficacy.

Using the raw damage isn't a perfect solution, but not using the raw damage as it turns out is much worse.

I have changed it to getDamage instead of getFinalDamage for now. This part of EliteMobs is in need of a rewrite to deal with the code rot, and I will try to address that as soon as I can.'

SuperKael commented 2 years ago

I would like to mention, there is another way to bypass vanilla armor, without having to worry about any tables. All of the damage modifiers on the EntityDamageByEntityEvent are flat values, so you could bypass them by doing something like this:

image

This will directly negate all of the damage modifiers, essentially canceling them out.

MagmaGuy commented 1 year ago

This has been completely rewritten.