Fixed window click logic

[Johni0702]

This tries to fix all window click logic.
Related ticket: #1

New features:

• Adds shift clicking logic
• Handle taking items from crafting result and call GlowCraftingInventory.craft() accordingly

Fixes:

• Fixed hotbar swapping (number keys)
• Prevent items from being placed in the wrong slots (dirt as their pants, placing items in the output of crafting table, etc.)
• Fixed double-clicks not taking all items from hotbar (when the player is in their inventory, double-click wasn't collecting the hotbar slots 6-9)
• Fixed dragging items creating stacks with too many items (splitting a stack of items on top of another one crafted stacks with more than 64 items)

Not included in this PR:

• Removing items from the crafting matrix (this should be done in CraftingManager.removeItems). Note that this causes your whole inventory to be filled when you shift-click at the output slot of a crafting recipe. This bug will vanish once said method is implemented.
• Filtering of items for all different kinds of inventories (itemShiftClickAllowed and itemPlaceAllowed methods). This PR only implements those for the player inventory (armor slots).

Plugin used to test the InventoryClickEvent: WindowClickTest.py (MiniPython)

---

Edited by turt2live

Related Links:

• Ticket: Inventory and Crafting #1

[turt2live]

Initial review (as I can't really test it right now) of the diff indicates a few naming issues and possible organization/approach concerns. I'll look at it in more depth later tonight.

[olliestanley]

I feel like this doesn't warrant it's own package, or at least if it does it should have a different name :P

[choyiny]

< dx> let's start a bounty for the brave soul that fixes window click handling completely
@turt2live I'll throw in $10 via PayPal
@turt2live Can quote me on that.
@turt2live Whoever fixes all of window clicking: $10.

[turt2live]

@hkminegod once it's pulled :)

[Johni0702]

Note: This currently has two issues which I'll fix with others that are probably going to come up.

• Furnace should use left-to-right instead of right-to-left shift click logic
• Stacks can be placed in the input slot of the enchantment table but only single items should be placed

[Johni0702]

This should be ready to be tested now.
The problem with the enchanting slot still persists because I'm not sure on how to properly handle it as both slots in the enchantment table are SlotType.CRAFTING.
I'm not entirely convinced that that's correct. I feel more like the lapis slot should be either FUEL (as that's kind of what it is) or CONTAINER (as that's what the javadoc suggests for slots not fitting in any other category).

[turt2live]

@Johni0702 Creative inventories are not correctly handled.

[Johni0702]

This now calls the CraftItemEvent and the InventoryCreativeEvent as well as handling the later one when it's canceled.
Note: The CraftItemEvent is called everytime you click the output slot of the crafting inventory with a valid recipe in place, even if no crafting process takes place (full inventory, cursor). This mimics the behavior of CraftBukkit and is intended.
With the creative inventory another bug arises. If you double-click within the creative inventory to collect all similar items onto the cursor, then close the inventory and open a chest (or any other container) the items reappear in your inventory.
However this bug also happens in Vanilla and it is currently impossible to fix this as the client does not send the required packets and the server never gets informed about the double-click. I was not able to find the bug in Mojangs bug tracker (however I didn't spent more than 3 min searching for it).

[turt2live]

@Johni0702 There is regression from CraftBukkit due to this PR

• Opening your inventory while in creative mode and having some items fires events
• After clearing your inventory (/clear) in creative mode and then opening the inventory, events fire
• The raw slot when using creative mode to populate the hot bar (click, move, click) is incorrect
• The item is null when it should be air in many cases (current and new)
• 2 events fire for using the number keys when in creative mode
  • Both have wrong information as well
• Selecting an item from the creative menu (left click) and right-click dragging over the hotbar fires the wrong number and wrong events
• Selecting a stack of 64 from the creative menu (shift-left-click) and placing the whole stack on the taskbar results in the wrong slot information
• The above also applies to right-click placing the items across the hotbar
• Wrong action for clicking outside the inventory

Please note that many of these regressions may be due to 1.8 and not the code. Please verify that it is a protocol change before saying it's intended.

Also I stopped testing after the above list of issues to avoid potentially reporting duplicates.

[Johni0702]

• Opening your inventory while in creative mode and having some items fires events
• After clearing your inventory (/clear) in creative mode and then opening the inventory, events fire

Fixed. The Minecraft client sends quite a few redundant updates to the server even if the server already knows of these. Simply comparing the items in the messages with the current known stack and ignoring matches, solves this issue.

• The raw slot when using creative mode to populate the hot bar (click, move, click) is incorrect

Typo :[ Fixed. I'm so sorry. Please tell me I haven't wasted too much of your time.

• The item is null when it should be air in many cases (current and new)

I did not yet fix this one as it contradicts what is said in #265. If you wish to have it fixed, it should be rather easy to do so.
It would only require the GlowInventoryView class to override the getCursor method and to pass in AIR instead of null wherever the InventoryEvent is used. There does not seem to be a single occasion where the InventoryEvent does return null instead of AIR, please correct me if I'm wrong.

• 2 events fire for using the number keys when in creative mode
  • Both have wrong information as well
• Selecting an item from the creative menu (left click) and right-click dragging over the hotbar fires the wrong number and wrong events
• Selecting a stack of 64 from the creative menu (shift-left-click) and placing the whole stack on the taskbar results in the wrong slot information
• The above also applies to right-click placing the items across the hotbar

Fixed. See above

• Wrong action for clicking outside the inventory

Clicking outside the inventory doesn't call any events afaik. Fixed that.

Note that the behavior of theWindowClickHandler.drop(GlowPlayer, ItemStack) method will be changed by #193 so that it also calls the PlayerDropItemEvent (which is therefore not included in this PR).

[turt2live]

@Johni0702 Your pull request has not been mergeable for 5 days now. Please resolve the concerns as soon as possible, thanks!

[TheDiamondYT1]

Wtf lol