search

Jikoo / OpenInv

Open anyone's inventory as a chest, real-time!
GNU General Public License v3.0
118 stars 38 forks source link

SilentContainer conflicting with InventoryGui (Library) created Inventories Menus #124

Closed LOOHP closed 1 year ago

LOOHP commented 1 year ago

I'm currently facing this weird issue where if I create an inventory menu with this handy library called InventoryGui, and if I have OpenInv's SilentContainer feature enabled, in certain circumstances, the menu would stop working.

Way to reproduce:

  1. Create a main menu with InventoryGui, include a button (item with click action) that opens another InventoryGui menu
  2. Show the main menu to a player
  3. Click the button to open the other menu
  4. Hit E to close the inventory to go back to the last menu (the main menu)
  5. The main menu is now broken and you can take the button (the item) out of the main menu

This doesn't happen with SilentContainer disabled. In my own testing, the items that you moved out in step 6 don't actually exist on the server, once the inventory updates they are gone. However, I have other users report that those items are real and they are added to the player's inventory, not sure whether this is true. But in either case, there's something wrong.

I did some further debugging and I found that after step 4 when we are back at the main menu, InventoryClickEvent & InventoryDragEvent stop firing as if the player isn't viewing the inventory anymore.

Jikoo commented 1 year ago

My suspicion is that the user internally becomes stuck as a spectator. The client isn't informed because OI forces it purely serverside - no events are fired, no packets are sent. It should be unset immediately as soon as OI finishes handling an open/close, but there may be an edge case where a double close causes OI to set the player to spectator, set the player to spectator again, restore original gamemode, then restore "original" spectator gamemode.

Clientside items become real if the client is in creative and the server doesn't actively rewrite the client's items after the interaction, so that may be the difference between what you and your user experienced.

Jikoo commented 1 year ago

Yeah, looks like InventoryGUI can cause duplicate InventoryCloseEvents to fire https://github.com/Phoenix616/InventoryGui/blob/65bf894012d4dee69c3e6e2c61114e22021999e6/src/main/java/de/themoep/inventorygui/InventoryGui.java#L1288 OI handles repeats when opening but lacks that check when closing.

LOOHP commented 1 year ago

Thanks for looking into it so quickly, from what I understand, OI would just need to apply the same check as it does with opening and not repeat setting the player internally to spectator mode again if the inventory had already been closed to resolve this issue?

Jikoo commented 1 year ago

Yeah. There's an opportunity here for improvement for consistency, but the simple fix is just to ignore spectators because they don't need to be handled in the first place.