search

Slimefun / Slimefun4

Slimefun 4 - A unique Spigot/Paper plugin that looks and feels like a modpack. We've been giving you backpacks, jetpacks, reactors and much more since 2013.
GNU General Public License v3.0
959 stars 548 forks source link

Pìckaxe_Of_Containment + SilkSpawners duplicating spawners #2814

Open MizukageMaTes opened 3 years ago

MizukageMaTes commented 3 years ago

:round_pushpin: Description (REQUIRED)

I have SilkSpawners on my server. Players who place SilkTouch in Pickaxe_OF_Containment are able to double spawners, receiving a normal and a Broken Spawner. With that they can put the normal spawner on the ground and then collect again, thus having infinite broken spawners.

:bookmark_tabs: Steps to reproduce the Issue (REQUIRED)

  1. Have SilkSpawners in your plugins.
  2. Have silkspawners permission to collect spawners.
  3. Enchant Pickaxe Of Containment with Silk Touch and then collect any spawner.

:bulb: Expected behavior (REQUIRED)

The player should only be able to obtain a broken spawner when collecting with the pickaxe of containment.

:compass: Environment (REQUIRED)

bobhenl commented 3 years ago

Same with CMI charges https://github.com/Zrips/CMI/issues/4775

MizukageMaTes commented 3 years ago

I remember seeing this before. I realize you would need the 'Silk Spawners' plugin. I remember it does drop the extra broken spawner that is empty. When the broken spawner is placed, does it default to pig? I forget. I remember thinking that this is only good for infinite pig spawners right? I am not on a server that has both pickaxe of containment AND silk spawners plugin, but it is easy to test for someone who is.

In my case it puts the spawner of the same mob type, thus being able to get broken spawners of any mob. To prevent this for the time being I disabled the Pickaxe of containment .. but making it impossible to craft broken spawners.

variananora commented 3 years ago

Pickaxe of Contaiment with Silk Touch enchantment are triggering Slimefun and SilkSpawner (or other spawner plugins) to drop the spawner, Slimefun dropping Broken Spawner, and SilkSpawner dropped their version of spawner. I don't know is this something easy to fix or what, but the workaround that you can use is either disable Picaxe of Contaiment or disable Silk Touch enchantment dropping on SilkSpawner.

MizukageMaTes commented 3 years ago

Pickaxe of Contaiment with Silk Touch enchantment are triggering Slimefun and SilkSpawner (or other spawner plugins) to drop the spawner, Slimefun dropping Broken Spawner, and SilkSpawner dropped their version of spawner. I don't know is this something easy to fix or what, but the workaround that you can use is either disable Picaxe of Contaiment or disable Silk Touch enchantment dropping on SilkSpawner.

Yes, I have already disabled the Pickaxe of containment to avoid the problem. But I did it as a temporary workaround until the problem is resolved.

It will probably be difficult. I have a solution (I don't know if it is viable, but I'll leave my idea here): Make it impossible to enchant the pickaxe of containment. It is no longer possible to place items from Slimefun on anvils, it could do the same with the altar of enchantments and also make it impossible to put the pickaxe of containment to enchant in the autoenchanter.

I think that with this it would not be possible to put silktouch in the pickaxe of containment and it would solve the problem.

Tarluin commented 3 years ago

Disable the use of silktouch all together. Just change minSilkTouchLevel: 1 to 2 now you still have all the other stuff about silk spawners but no one can ever get silk touch II so the picks no longer work.

mrcoffee1026 commented 3 years ago

I solved this with another plugin that allows silk spawners... in that one it had a setting in the config where you could set the required tool needed to break a spawner. The pickaxe containment is an IRON_PICKAXE. So the very simple work-around for this was to require a DIAMOND_PICKAXE or NETHERITE_PICKAXE for using silk to mine the spawner. There's also the added benefit that if you catch a person who has upgraded a containment pickaxe to diamond or netherite using addon abilities... you know you found a user who is duping spawners and can take whatever action you feel is necessary.

WalshyDev commented 3 years ago

Taken a look into this, we drop the item naturally rather than adding to drops (I know we disable drops but still causes issues due to plugins not knowing about the drop). That can definitely cause issues however SilkSpawners (at least) is a separate issue.

So, SilkSpawners ALSO drop naturally rather than add to the drops, this means we both drop the items and dupe. The only way to prevent it is by using their API (repo accessible here). So, a bit annoying that they do it this way however it is easy to fix.

EpicSpawners, I can't actually see where they drop the spawners so err not sure what to do for them.

Cancelling the event isn't a fix for SilkSpawners but would fix EpicSpawners. SilkSpawners is at HIGHEST priority and ignoreCancelled (ref). They do not check for cancelled in the event. EpicSpawners however, do (ref).

So, not sure the best approach for this honestly. From what I can tell the best approach is:

if (isUsingSilkSpawners) {
  // Just don't do anything, we will handle it with their API event
  return;
} else if (isUsingEpicSpawners) {
  setCancelled(true);
  setType(AIR);
  drop
} else {
  // preferably add to drops but we can keep current behaviour too
}
TheBusyBiscuit commented 3 years ago

@WalshyDev SilkSpawners-API doesn't really allow us to fix this without changes to it. They don't exactly expose much, we looked into this in the past.

WalshyDev commented 3 years ago

@WalshyDev SilkSpawners-API doesn't really allow us to fix this without changes to it. They don't exactly expose much, we looked into this in the past.

what do you mean? We eiher need to cancel the event or change the ItemStack being dropped. Not sure what we even want the behaviour to be for that but yeah.

TheBusyBiscuit commented 3 years ago

what do you mean? We eiher need to cancel the event or change the ItemStack being dropped. Not sure what we even want the behaviour to be for that but yeah.

Look at the actual API, if the event is cancelled, the entire BlockBreakEvent is cancelled. If we change the ItemStack it won't even be dropped unless millions of other factors apply. We have no control over whether the item will be dropped or not.

WalshyDev commented 3 years ago

what do you mean? We eiher need to cancel the event or change the ItemStack being dropped. Not sure what we even want the behaviour to be for that but yeah.

Look at the actual API, if the event is cancelled, the entire BlockBreakEvent is cancelled. If we change the ItemStack it won't even be dropped unless millions of other factors apply. We have no control over whether the item will be dropped or not.

yeah... SS for some reason is annoyingly awkward...

Plugin compatibiliy can be such a pain sometimes