Hi there,

I'm having some reports that spawners mined via SilkSpawners can't be sold back to signs .
I've took a closer look into this. 😄

In the Trade class you check in L93 (https://github.com/drtshock/Essentials/blob/2.x/Essentials/src/com/earth2me/essentials/Trade.java#L93) for the same item via .containsAtLeast

Here is an image with the differences in the NBT tags

The first spawner is obtained via a Buy sign, the second spawner is obtained after mining the placed spawner (i.e. from SilkSpawners).

It seems that the .containsAtLeast is checking for the exact same NBT tags, too.
Modifying e.g. the Delay to 19 makes the spawner unsellable too. (containsAtLeast results to false)

In my opinion the spawner should be sellable regardless of the amount of NBT tags, except the BlockEntityTag -> EntityID tag must match.

Since you already created the SpawnerProvider (prior it was the SpawnerUtil class) it could be extended for a comparison of spawners, too? (Checking for the same NBT tag or legacy durability value).

I'm currently too busy for a PR, just wanted to add my 2 cents and maybe give a pointer in the right direction.

Cheers & thanks,
Tim

Update:

.containsAtLeast checks the ItemStack via .isSimiliar() in https://hub.spigotmc.org/stash/projects/SPIGOT/repos/craftbukkit/browse/src/main/java/org/bukkit/craftbukkit/inventory/CraftInventory.java#142

.isSimilar() compares the NBT tags, too.
https://hub.spigotmc.org/stash/projects/SPIGOT/repos/craftbukkit/browse/src/main/java/org/bukkit/craftbukkit/inventory/CraftItemStack.java#431

This results in the false return value

I want to avoid having NMS for future Spigot versions, so I'm heavily against introducing additional NMS in the BlockStateMeta SpawnerProvider. Might add special handling for spawners in those areas you pointed out.

Seems like this probably won't get fixed, as it would be pretty hacky. Also with the evidence provided, it seems like this is something that could/should be fixed in CraftBukkit.