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

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

Open
SXRWahrheit opened this issue Aug 12, 2020 · 2 comments

Comments

@SXRWahrheit
Copy link

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:

  • The player's proximity to the entity they are damaging, as appropriate
  • Whether the damage event has already been modified
  • The cause of the damage

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
Copy link
Author

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
Copy link
Member

nossr50 commented Aug 12, 2020

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants