Interferes with McMMO fishing
Iiridayn opened this issue · 9 comments
As MnMaxon#6, just having the plugin loaded prevents mcMMO fishing loot, but using latest version of mcMMO (built 6 May 2020; found at https://popicraft.net/jenkins/job/mcMMO/) and of AutoPickup from this repo (5.3.0-Beta1). mcMMO-Dev/mcMMO#4129 might also help shed light on the issue, and contains another issue.
It would be enough if having it loaded but off wouldn't break mcMMO fishing, and giving up the autopickup for that scenario. Better would be to auto-disable if mcMMO fishing is detected. Best would be to autopickup mcMMO fishing items, though the thrown potion/TNT entities (if still present, tested w/mcMMO fishing level at 100 and max rod to not waste time) might complicate it?
This is a feature request for autopickup - and I have a patch that fixes it in a PR. Might not be the way you'd like it, but it works for my situation, and it seems of others. You mentioned a specific way you'd like to fix it - please leave this open until it is fixed in a manner to your satisfaction. Just because this is a feature request and not a bug, please do not close it. Again - there is a working solution in a PR!
This can NOT be fixed in auto pickup, it needs to be fixed in MCmmo as its caused by the way they are dealing with the drops hence this issue is closed.
the options for turning things on and off will be dealt with as a separate feature request.
Update: works when the inventory is full and not deleting on a full inventory, so at least option 1/2 should definitely be possible. Perhaps a config setting would be adequate.
I would prefer to have AutoPickup picking up mcMMO fishing drops - but this at least allows me to have both running at the same time.
so looking at the MCMMO ticket you linked, their explanation makes no sense,
Auto pickup is hooking into the onItemDrop call and placing the items in the player's inventory, possibly we can lower the priority to let there call go first, but I can't see anywhere they have hooked the call themselves so I cant see this being the fix.
Yeah - I believe they're also changing the item in the NOTIFY priority (final) handler, which the API docs say to not do - haven't dug deeper, but were they not I might have tried to reduce the priority of the event here and test that. As is, their event definitely fires after. I don't think Bukkit had autopickup/item replacement in mind when they designed the handler - the docs talk about event cancellation, but you don't cancel the event, you void the item instead.