Introduce GuiClickEvent and getItem(Slot slot)#2513
Conversation
|
1 year and half later 🔥 |
stefvanschie
left a comment
stefvanschie
left a comment
There was a problem hiding this comment.
Thank you for your pull request. I think the idea of your pull request is good. I have also been growing discontent with the usage of InventoryClickEvent myself and putting that behind a different type which contains more IF-oriented data seems like a good (first) step.
In my opinion, you could even expand this to click event on guis and gui components, although the absence of this would not be a blocker for this PR.
The main issue, besides the changes I addressed below, is that this is a breaking change, so this cannot be added to the next version, but needs to wait for a version with breaking changes.
|
Hi, I've decided to remove InventoryClickEvent completely and use just GuiClickEvent since this is already a breaking change, and using only GuiClickEvent is more consistent. |
Allow XML-defined handlers to receive GuiClickEvent - onClick handlers in XML now take GuiClickEvent instead of InventoryClickEvent - Removed wrapper that was unwrapping to InventoryClickEvent - XMLUtil.loadOnEventAttribute() now accepts any type, not just Bukkit Events Breaking change: update handler signatures from (InventoryClickEvent) to (GuiClickEvent)
stefvanschie
left a comment
stefvanschie
left a comment
There was a problem hiding this comment.
Moving everything to GuiClickEvent is fine. Please review the comment I made, then as far as I'm concerned this can go in the next breaking version.
Should be done. Fixing the CI Issue right now. |
|
I've been using IF for a while, so i know some things that i don't like abt it, and since this is already a breaking change, i will prolly push some other thing on these days. |
- PaginatedPane.getPanes(int): validate 'page' argument instead of 'this.page' - GuiItem.loadItem(): assign parsed damage value to 'damage', not 'amount'; drop unused 'hasDamage' variable - OutlinePane.display(): use getLength() for vertical orientation size, cache getItems() to avoid repeated list allocation per render iteration - MasonryPane.click(): guard against null cachedPositions when click is dispatched before first display - GuiItemContainer.setItem(): store item reference directly after applyUUID instead of deep-copying on every render
2 participants
Wrap InventoryClickEvent in GuiClickEvent for pane-level click handlers
Introduce GuiClickEvent, which wraps a Bukkit InventoryClickEvent together
with a pane-relative Slot, so click consumers always receive the slot
position expressed relative to the pane's top-left corner rather than the
raw inventory index.
GuiItem.action and Pane.onClick are now Consumer<? super GuiClickEvent>.
All pane click() implementations construct a GuiClickEvent(event, slot)
before forwarding to callOnClick/callAction. XML-loaded handlers that still
declare an InventoryClickEvent parameter continue to work via an automatic
unwrapping wrapper. Gui-level consumers (onTopClick, onBottomClick, etc.)
are unaffected as they have no pane-relative context.