Incompatibility with Angel Chest (dupe)

elyrodso commented 7 months ago

What happened?

When a player opens their bag, but the slot that was emptied when they opened the bag, and later the slot is filled with another item and the player is killed while still in that state with the AngelChest plugin installed, the bag will return to their inventory as soon as they respawn, but will also drop in the place where the player was killed, creating a dupe of the bag.

What was supposed to happen?

The bag won't return once the player respawn, so just leave the bag at the player's place of death.

How do I reproduce this issue?

1. Install AngelChest (no configs)
2.  Fill your inventory with items while you have a haven bag in a slot that can be used with right click, drop an item on the ground and pick it up as soon as the haven bag slot opens.
3. Die in this state with the Haven bag GUI open and your entire player inventory full of items.
4. Notice that there is a Haven bag in your inventory as soon as you respawn, and also at the place of death, creating a dupe.

Relevant log output

No response

Relevant file configuration

No response

Server Version

git-Purpur-2127 (MC: 1.20.4)

Plugin Version

1.7.5.565

Issue Confirmation

[X] I have downloaded and tried the latest version of HavenBags and the issue still persists.
[X] I have provided sufficient information for the issue to be replicated.

Valorless commented 7 months ago

Greetings! I'm not familiar with Angel Chest, so I can't say how long it'll take, or if I can fix it. But I'll go grab it right now, and attempt to replicate the issue. If I succeed, I'll start working on a fix :)

Valorless commented 7 months ago

I've been able to replicate the issue. Using StackTrace, I can see that AngelChest is calling for the GUI to be closed an additional time, which is what causes the dupe. The real bag is the one that's dropped when the player dies, and the duped one by AngelChest, is the one you spawn with. Using another plugin called Graves (https://www.spigotmc.org/resources/graves.74208/), I did not receive a dupe.

I am sorry to say, but as long as AngelChest calls an additional InventoryCloseEvent, I am unable to resolve this issue with the current knowledge I have.

mfnalex commented 7 months ago

AngelChest simply calls Player#closeInventory(). I don't see what's wrong with that. It does not call any "additional InventoryCloseEvent".

BTW, there's also a ton of other backpack plugins that do not cause any issues, so simply blaming AngelChest is a bit funny.

Would be nice if we could get this resolved together instead of you just closing the issue and blaming AngelChest.

Valorless commented 7 months ago

I never meant to simply blame AngelChest, I may have written wrongly, sorry.

I return bags on InventoryClose, and havent occurred another plugin yet that called it, so I thought AngelChest was calling an additional close event. (Why is AngelChest calling a close event on death, isn't that called by default when dying?)

I'm still relatively new to plugin development, and I do not know currently how I can prevent the dupe issue from happening. I still plan to try and fix this, and have it added on my todo list. (although not updated)

I don't know what other bags plugins do, or how they work.

My intentions were not to push this away and forget about it. It was my mistake about closing the issue, without thinking how that may have come across. I'll reopen the issue, and mark it as in progress :)

Valorless commented 7 months ago

Just had an idea. I can check on my close and return event, whether or not the bag is still open, as i keep track of open bags. Since the event is run twice, the bag is removed from the "open" list during the first trigger, and then when the second is run, I can cancel it.

I'm currently out, but once i return home, I'll work on this right away.

Valorless commented 7 months ago

Sooo... I've returned home. It seems I had a hard case of Brain Malfunction. I have fixed the dupe issue, by only letting a bag's close event trigger, if the bag is on the list of open bags. This fixes the issue of having it trigger twice. I feel very stupid.

Gonna upload the update soon.

mfnalex commented 7 months ago

AngelChest is closing the inventory because some plugins change the inventory contents while having GUIs open or similar stuff. To be able to fetch the original inventory, it calls Player#closeInventory().

Oh, glad to read that you fixed it! Thank you very much, can't wait for the new version :3

Have a nice day!

Valorless / HavenBags