Interferes with McMMO fishing

[Iiridayn]

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?

[Iiridayn]

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.

[Iiridayn]

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.

[ThePhilderbeast]

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.

[Iiridayn]

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.

[ThePhilderbeast]

yea, to be honest, this needs to be fixed on the mcMMO side, so im less inclined to make a change here to accommodate their bad design.

changing the priority of our events is probably something that should be done regardless though.

[Iiridayn]

It might be worth talking directly with them with a concrete proposal, as the responding dev seemed to feel the opposite, as the "blame game" is never more than an excuse to be unproductive. (It may have some worth in front of a manager, but none for donated code). That said, disabling autopickup for fishing is a useful feature regardless, gives server admins more control.

…

On Fri, Jun 5, 2020, 16:41 Phillip Ledger ***@***.***> wrote: yea, to be honest, this needs to be fixed on the mcMMO side, so im less inclined to make a change here to accommodate their bad design. changing the priority of our events is probably something that should be done regardless though. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#54 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABVYH6KCS2UDH6E2C3CM23RVFX7ZANCNFSM4NR6XEOQ> .

[ThePhilderbeast]

closing this since it not a bug with Autopickup.

[Iiridayn]

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!

[ThePhilderbeast]

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.