I spoke to the elite mobs dev's and they're under the impression that it's something that'd need to be fixed in the magic plugin and that they cant implement a fix on their end
according to their dev:

eltemobs cancels the item pickup event at a low priority, so whatever system that plugin is using to pick items up just ignore cancelled and don't set it to low or lowest priority

https://github.com/MagmaGuy/EliteMobs/blob/master/src/main/java/com/magmaguy/elitemobs/items/ItemLootShower.java

it appears that with the spell inventory open, the cancellation of pickup for loot shower items is ignored. Would it be possible to have a compatibility fix for EliteMobs + Magic to prevent this behaviour, it'll kill gold based economies like mine if left unchecked unfortunately, and I really dont want to uninstall either plugin as our player base thoroughly enjoys both

I'd be happy to fix this on my end if I can, but I may need more information on the problem.

Is it basically that EliteMobs is generating loot items that are not meant to be picked up (they just disappear on pickup?), but if you have your spell inventory open they instead get put in the player's survival inventory?

If that's all correct, then I may need to test it for myself. I have never used EliteMobs, is there a quick/simple set of steps from installing the plugin from scratch to generating a loot shower?

I am not entirely sure what would be going wrong. Looking at my code, I have two separate event handlers for item pickup. The first is at lowest priority, but is only there to do something similar to EliteMobs, detect and remove temporary items.

The other eventhandler is set to highest priority (should go last, other than monitor handlers), this is the one responsible for bypassing the spell inventory and putting item pickups in your survival inventory.

Both handlers are set to ignore cancelled events, so if I'm understanding everything it seems like they should work together.

You're the man Nathan.

Essentially, when you kill an elite mob the plugin drops some item stacks and has them float toward you as exp would, these item stacks represent the elite coins you have earned killing the mob.

They should just disappear on pickup however yes they get added to the player survival inventory if the spell inventory is open.

Steps to reproduce:

• Open spell inventory with Q.
• Keep spell inventory open (a lot of our magic sword users do this often)
• Kill an elite mob. Loot shower will appear, showing the reward of elite coins etc (Can summon them with the em summon command or just essentials spawn mob and there's a chance it'll be elite, spawn 10 zombies and you'll probably get 9 standard zombies and 1 elite mob)
• Close spell inventory with Q, coins will be in inventory.

Without spell inventory open the pickups are cancelled as expected.

Happy to collaborate on a fix, I'm not an elite mobs dev, but I've started working on a few plugins lately. If you need any more info I'll gladly provide it

Ok thanks! I think you gave me enough to go on, if I can figure out how to summon one I should be good.

The only thing I'm seeing that may be an issue, I am using an older PlayerPickupItemEvent- I think there is a newer EntityPickupItemEvent .. I wonder if EliteMobs is using that one and they (for some reason) don't get along.

I can screen cap an overview of the issue/expected behaviour and drop it to you via discord if that'd be useful

I'm about to test, but just looking at the Spigot code tells me that EliteMobs is probably using the newer event, which is unfortunately called after the legacy one:

                itemstack.setCount(canHold);
                // Call legacy event
                PlayerPickupItemEvent playerEvent = new PlayerPickupItemEvent((org.bukkit.entity.Player) entityhuman.getBukkitEntity(), (org.bukkit.entity.Item) this.getBukkitEntity(), remaining);
                playerEvent.setCancelled(!playerEvent.getPlayer().getCanPickupItems());
                this.level.getCraftServer().getPluginManager().callEvent(playerEvent);
                if (playerEvent.isCancelled()) {
                    itemstack.setCount(i); // SPIGOT-5294 - restore count
                    return;
                }

                // Call newer event afterwards
                EntityPickupItemEvent entityEvent = new EntityPickupItemEvent((org.bukkit.entity.Player) entityhuman.getBukkitEntity(), (org.bukkit.entity.Item) this.getBukkitEntity(), remaining);
                entityEvent.setCancelled(!entityEvent.getEntity().getCanPickupItems());
                this.level.getCraftServer().getPluginManager().callEvent(entityEvent);
                if (entityEvent.isCancelled()) {
                    itemstack.setCount(i); // SPIGOT-5294 - restore count
                    return;
                }

This may be challenging to fix without breaking Magic's backwards-compatibility (the reason I'm still using the legacy event) :(

Can confirm that is the issue, reproduced and also tested a quick fix by switching to the new event.

Actually fixing this without breaking pre-1.12 compatibility is going to be a bit of a pain but I think I can make it work.

You are a hero Nathan! compatibility is always a paIn, thanks for looking into this so quickly! Anything I can do to help?

Nah, thanks though!
Will be fixed in the next dev build:

https://jenkins.elmakers.com/job/MagicPlugin/4010/

Can confirm that the fix works! you're the man Nathan!