We need discussion about SmartInventory#close(Player) method

[wysohn]

Currently, the method manually fires InventoryCloseEvent, yet this can posses a problem.

Why this is a problem? I'm trying to create a trade plugin, so I pair two SmartInventory instances like gui1.other has gui2 and gui2.other has gui1.

So both instances are now depending on each other.

Now the problem happens when I want to close the GUI1 and GUI2 together. I'm trying to register listener to capture the InventoryCloseEvent, and what happens is that when GUI1 is closed by user, it will close the GUI2 using the close() method, and close() method will fire InventoryCloseEvent which will again invoke close() method of GUI1, and so on and so on.

In my opinion, just like the Bukkit API's Player#closeInventory() doesn't fire InventoryCloseEvent, the close() method shouldn't fire the InventoryCloseEvent as well.

What do you think? I have a commit ready for pull request if you agree on it. Or if you think this change will cause another problem instead.

[MinusKube]

I agree, but maybe we should replace it by a new event, like "SmartInventoryCloseEvent" so we're still able to know when SmartInventory#close(Player) is called.

If you do a PR for this, please use "master" as the target branch, I'm planning to do some branches/versioning rework.

[wysohn]

Well, the commit is in it the branch that was branched from 1.3.0 branch already, so it would be rather difficult to merge it on the master branch than fix it directly.

It's a short and simple change, so it wouldn't be a big deal anyway.

I will leave a link instead of PR for the future reference: https://github.com/wysohn/SmartInvs/commit/86af09db8421dae4299bcff9465c85b113babf2b

Or as you suggested, we can use SmartInventoryCloseEvent instead.