MinusKube / SmartInvs

Advanced Inventory API for your Minecraft Bukkit plugins.
https://minuskube.gitbook.io/smartinvs/
Apache License 2.0
263 stars 65 forks source link

Suggestion of Inventory tracking method #157

Open wysohn opened 4 years ago

wysohn commented 4 years ago

Currently, the Inventory instances are tracked per Player instance (by Map<Player, SmartInventory>)

I wonder if we can make it per Inventory instead. (Map<Inventory, SmartInventory>)

This makes things very easy for everything, especially when implementing transitioning process for GUI to GUI (close current GUI and open a new and vice versa).

Every Inventory instance is unique and is newly created as Bukkit.createInventory() is invoked, so we can make sure that appropriate events are handled only for the specific Inventory instance that is handled by the SmartInventory. (Just for additional info, Inventory instance is not unique in old versions like 1.5.2, yet who cares about those old version anyway?)

Lets say I want to implement that when a GUI is closed, it re-open the original GUI that opened this GUI, yet because the SmartInventory is per Player, calling close() method of SmartInventory will fire InventoryCloseEvent (because it invokes Player#closeInventory()), that event is again handled in InventoryManager, in the handler (of InventoryManager), it invokes the handlers registered in SmartInventory, which is not yet changed to the new GUI yet, and so on and so on... so it will again call the handler itself causing infinite loop.

InventoryCloseEvent ->
calls handlers of SmartInventory ->
the handler invokes SmartInventory#close() of another SmartInventory -> 
SmartInventory#close() invokes Player#closeInventory() ->
InventoryCloseEvent -> 
(at this point, the current SmartInventory is still the current GUI as Map<Player, SmartInventory> is not updated yet)
calls handlers of SmartInventory -> 
the handler invokes SmartInventory#close() of another GUI ->
SmartInventory#close() invokes Player#closeInventory() -> 
InventoryCloseEvent ->
 **infinite loop...**

However, if indeed the SmartInventory was tracked per Inventory instance, this wouldn't be the case.

Yes, for temporarilly, I managed to make it by closing the GUI 1 tick later using the scheduler, but such a case is not ideal for using an API! it should handle cases like these as smoothly as possible.

I have a very specific implementation done long long time ago (but it's poorly designed and poorly documented overall so not qualified for others to use), so please consider if you like the idea.

I can try it myself, yet it probably requires refactoring of lots of stuff, so author yourself working on it would be the best.

I can still give a shot if you are really running out of time to do it.