search

ChanceSD / PvPManager

A Spigot/Paper plugin to toggle PvP, stop combat log, protect new players and much more
https://www.spigotmc.org/resources/pvpmanager.845/
Other
131 stars 47 forks source link

Certain plugins and/or data packs can bypass PvP protection #357

Closed Etanarvazac closed 1 year ago

Etanarvazac commented 1 year ago

Description: There are plugins and/or data packs suck as Incendium that have items that can completely bypass the PvP protection. Taking Incendium as a prime example, some of the crossbows have effects that when shot by a player, including when both players have PvP disabled, can actually damage other players.

To Reproduce Steps to reproduce the behavior:

  1. Download Incendium and apply it to the server with PvPManager
  2. Get the Incendium items. Fastest way is by running the command /function incendium:admin/give/all
  3. Ensure you and the other player have PvP disabled
  4. Take a crossbow from the items you received in step 2, such as the Holy Wrath or Sentry's Wrath, and fire the crossbow at or near the other player.
  5. The effect of the item should damage the other player, spite PvP being disabled.

Expected behavior The other player should not take any damage during any part of the item's effect.

Plugin and Server version I am a member of XCraft Survival, not part of their staff, but trying to debug this issue with the server owner, MasonX3. The server runs 1.19-1.19.3, but is native to 1.19.2. PvPManager is version 3.10.9.

Screenshots Due to this bug, MasonX3 had to remove Incendium. As a result, I am unable to provide a screenshot or clip of the issue at this time.

Additional context While awaiting an official resolution for this bug, we, MasonX3 and myself are trying to find a alternative solution before MasonX3 is to re-add Incendium back to XCraft Survival. The Incendium data pack utilizes the .mcfunction files for the effects of many of the customized tools, weapons, and mobs. All of which run based on vanilla content (e.g.: minecraft:crossbow). Some of Incendium's function files, using the Holy Wrath as a prime example, summon additional projectiles when the initial shot is fired. One solution would be tracing a point of origin. Knowing each function file of a customized vanilla weapon is inter-connected could potentially help in such trace.

PhoenixCodingStuff commented 1 year ago

Can confirm this still works. The fireworks do the damage and not the arrow and hence its not recognized by PvP Manager. The type of firework can be viewed here and here

Etanarvazac commented 1 year ago

@ChanceSD Any fix for this? Disabling part of this pack, as an example, would break the functionality of the pack's items. So a work-around would be beneficial.

ChanceSD commented 1 year ago

For plugins, they should always check if other plugins are cancelling the damage event before applying their own damage/skills/spells/etc.

As far as datapacks go, I don't know much about them but I assume they have no way to access the bukkit API since they are limited to vanilla commands/features. So I don't think there's any way to solve this other than your server disabling those datapack effects. It seems there has been a similar situation with WorldGuard regions as you can see in that comment: https://github.com/Stardust-Labs-MC/Incendium/issues/11#issuecomment-1435966536. It should be the same for PvPManager, if anyone discovers a solution I'm happy to make it work.

catter1 commented 1 year ago

As far as datapacks go, I don't know much about them but I assume they have no way to access the bukkit API since they are limited to vanilla commands/features. So I don't think there's any way to solve this other than your server disabling those datapack effects.

Hi, Incendium dev here. Yes, datapacks have absolutely no way to access the Bukkit API. As far as I know, there isn't exactly an easy way to fix it.

However, if WorldGuard or PvPManager regions are somehow able to check for tags, we could attempt to come up with a creative solution where all fireworks/arrows/other special projectiles are given a certain tag, and the region would automatically kill them. Since it's the first thing that comes to mind, it might not be the best solution, but I'm happy to help figure out something better if possible.

ChanceSD commented 1 year ago

I don't think there's any simple way to do this unfortunately. I believe to check for tags it would be necessary to use NMS code and I would like to avoid adding version specific code to PvPManager, since it would break between Minecraft updates and be an extra burden to keep working. And I can't speak for WorldGuard of course, so I don't know their take on that.

I'm fully open to any pull requests or ways to do this using the Spigot or Paper API, as long as it wouldn't require huge code changes. Closing this for now, will reopen if necessary.