mcMMO-Dev / mcMMO-Classic

mcMMO Classic.
https://www.spigotmc.org/resources/official-mcmmo-classic.2445/
Other
37 stars 44 forks source link

[Bug] McMMO skills affecting damage from plugins (MagicSpells, Denizen, MythicMobs, etc) #70

Open SXRWahrheit opened 4 years ago

SXRWahrheit commented 4 years ago

After much investigation, I determined today that mcMMO is the culprit behind the "overpowered" spells and other abilities on my server because it is modifying the damage event when it should not be.

Instead of running some checks to ensure the player is actually attacking a mob with their hand, sword, etc., mcMMO simply checks whether that item is in their hand and runs its own logic accordingly. This can be abused by players making use of holding items (or no items) to inflate the damage they cause with plugins from other abilities. Even if the damage type is custom, mcMMO applies the boost.

This should probably be fixed by checking, or at least adding settings to check:

Additionally, mcMMO should either manage itself or provide an API for managing its own damage alterations so that they can be disabled for specific damage events.

SXRWahrheit commented 4 years ago

Oh, and because the damage listeners run on HIGHEST Bukkit priority, even an (inefficient) workaround of trying to listen to the original damage and overwrite the modified damage is doomed.

https://github.com/mcMMO-Dev/mcMMO-Classic/blob/master/src/main/java/com/gmail/nossr50/listeners/EntityListener.java

nossr50 commented 4 years ago

Whether the damage event has already been modified

This would not be good enough reason to back out of modifying the damage.

The cause of the damage

I assume you mean checking DamageCause, it is true that I can add more things in relation to this. It might not solve your problem however. I could set mcMMO to only modify damage for a few DamageCause(s) but that would still include Custom.

The player's proximity to the entity they are damaging, as appropriate

I have concerns about such an implementation, for one I have to consider custom items and such from other plugins which may be extending the range of attacks. I can't think of a sane way to guarantee that I'm only boosting damage in appropriate circumstances. This could be a configurable option however (see my notes at the bottom).

Oh, and because the damage listeners run on HIGHEST Bukkit priority, even an (inefficient) workaround of trying to listen to the original damage and overwrite the modified damage is doomed.

Changing the listener priority should not be done carelessly, especially since mcMMO is historic (nearly 10 years old) and it may cause unwanted interactions to suddenly change the listener priority of such an important event.

As for options to solve this particular problem

  1. mcMMO could add some events that other plugins could listen to and cancel regarding combat damage modification
  2. mcMMO could have a configuration option to have stricter conditions in which to activate combat abilities

With that said, I can get to work on the solutions I've offered. If you think these are not adequate, or if you believe there is another solution let me know.