Can't get amount of shift-click craft item

Discussion in 'Plugin Development' started by KodekPL, Jun 4, 2012.

Thread Status:
Not open for further replies.
  1. Offline

    KodekPL

    I want to detect how many times player craft item using shift-click method. But there is no way to check that so I came up with idea to count the amount of craft items and divide it by amount of once crafted item.

    But here comes another problem, you can't check the amount of crafted items using shift-click! Listener detect this as only one craft... So I came up with another idea. I try to check slot where item goes after craft. Using first(ItemStack) method is not working too because it's counts from wrong "spot"! Or so I understand this that way. You can see this on picture. I know that I can do a while from other "spot" but really? Maybe you know easiest way to do it?

    I hope you understand everything I wrote, english is not my main language.
     
  2. Hmm, getting the slot where the items were placed thing is a good ideea for start but even if it worked it wouldn't work if the result items are more than 1 stack.

    I think a pre-post compare of the inventory should reveal the added items, then just sum them up and divide by result amount.

    EDIT: did a quick test:
    Code:
    if(event.isShiftClick())
    {
    	final ItemStack[] preInv = player.getInventory().getContents();
    	
    	Bukkit.getScheduler().scheduleSyncDelayedTask(RecipeManager.getPlugin(), new Runnable()
    	{
    		@Override
    		public void run()
    		{
    			final ItemStack[] postInv = player.getInventory().getContents();
    						
    			for(int i = 0; i < preInv.length; i++)
    			{
    				if(preInv[i] != postInv[i])
    				{
    					System.out.print("slot " + i + " differs: " + preInv[i] + " != " + postInv[i]);
    				}
    			}
    		}
    	});
    }
    And using a stack of wood dividided in half to make sticks, it succesfully detected my newly added 2 stacks of sticks:
    Still, AFAIK there isn't an easier way... there is however a feature request, you should vote: https://bukkit.atlassian.net/browse/BUKKIT-1582
     
  3. Offline

    KodekPL

    Later, I realized that result can be more than 1 stack. I am glad that you are trying to help, but your code is not perfect, checking difference between amount of preInv and postInv and I did some test with it.

    I got in slot 8, half a stack of sticks and I craft a sticks using one full stack of wood. From crafting, half of a stack goes to slot 8 but script not detect it. That's bad, it not see the difference of amount.

    Code:
    2012-06-05 10:30:16 [INFO] slot 6 differs: null != ItemStack{STICK x 32}
    2012-06-05 10:30:16 [INFO] slot 7 differs: null != ItemStack{STICK x 64}
    2012-06-05 10:30:16 [INFO] slot 8 differs: ItemStack{STICK x 64} != ItemStack{STICK x 64}
    I'm trying to do something with this code...

    I vote for this feature, thanks for the info.
     
  4. Of course it's not perfect because it's a *quick* test to verify that the method actually works, it requires improving of the checking with some trial and error.
    Also, please post the code once perfected, I'll use this as well if Bukkit doesn't add that getCraftedAmount() method eventually.
     
  5. Offline

    Comphenix

    I believe I got this method working, despite a number of pitfalls. One, ItemStack is mutable, so every item in the pre-crafting array will be updated along with the post-crafting array. Next, comparing ItemStacks is slightly tricky - you can't simply use equality, nor the equals() function (since it takes into account the item amount).

    And finally, you can't just compare the differences blindly - you have to filter out everything BUT the crafting result as some leftover material may have been moved to the inventory.

    Anyways, here's the relevant code (pastebin: XjmH5N6A):
    Code:
    private Plugin parentPlugin; // Reference to your plugin
     
    @EventHandler(priority = EventPriority.HIGHEST, ignoreCancelled = true)
    public void onInventoryClickEvent(InventoryClickEvent event) {
     
        if (event.getInventory() != null &&
            event.getSlotType() == SlotType.RESULT) {
       
            switch (event.getInventory().getType()) {
            case CRAFTING:
                handleCrafting(event);
                break;
            case WORKBENCH:
                handleCrafting(event);
                break;
            }
        }
    }
     
    private void handleCrafting(InventoryClickEvent event) {
     
        HumanEntity player = event.getWhoClicked();
        ItemStack toCraft = event.getCurrentItem();
        ItemStack toStore = event.getCursor();
     
        // Make sure we are actually crafting anything
        if (player != null && hasItems(toCraft)) {
     
            if (event.isShiftClick()) {
                // Hack ahoy
                schedulePostDetection(player, toCraft);
            } else {
                // The items are stored in the cursor. Make sure there's enough space.
                if (isStackSumLegal(toCraft, toStore)) {
                    int newItemsCount = toCraft.getAmount();
               
                    // Do what you need to do here!
                    // ...
                }
            }
        }
    }
     
    // HACK! The API doesn't allow us to easily determine the resulting number of
    // crafted items, so we're forced to compare the inventory before and after.
    private void schedulePostDetection(final HumanEntity player, final ItemStack compareItem) {
        final ItemStack[] preInv = player.getInventory().getContents();
        final int ticks = 1;
     
        // Clone the array. The content may (was for me) be mutable.
        for (int i = 0; i < preInv.length; i++) {
            preInv[i] = preInv[i] != null ? preInv[i].clone() : null;
        }
     
        Bukkit.getScheduler().scheduleSyncDelayedTask(parentPlugin, new Runnable() {
            @Override
            public void run() {
                final ItemStack[] postInv = player.getInventory().getContents();
                int newItemsCount = 0;
           
                for (int i = 0; i < preInv.length; i++) {
                    ItemStack pre = preInv[i];
                    ItemStack post = postInv[i];
     
                    // We're only interested in filled slots that are different
                    if (hasSameItem(compareItem, post) && (hasSameItem(compareItem, pre) || pre == null)) {
                        newItemsCount += post.getAmount() - (pre != null ? pre.getAmount() : 0);
                    }
                }
           
                if (newItemsCount > 0) {
                    // Do your thing here too.
                    // ...
                }
            }
        }, ticks);
    }
     
    private boolean hasSameItem(ItemStack a, ItemStack b) {
        if (a == null)
            return b == null;
        else if (b == null)
            return a == null;
     
        return a.getTypeId() == b.getTypeId() &&
              a.getDurability() == b.getDurability() &&
              Objects.equal(a.getData(), b.getData()) &&
              Objects.equal(a.getEnchantments(), b.getEnchantments());
    }
     
    private boolean isStackSumLegal(ItemStack a, ItemStack b) {
        // See if we can create a new item stack with the combined elements of a and b
        if (a == null || b == null)
            return true; // Treat null as an empty stack
        else
            return a.getAmount() + b.getAmount() <= a.getType().getMaxStackSize();
    }
     
    KodekPL likes this.
  6. Nice!
    I have some questions tough :p

    Why are you comparing durability and data ? They're the same thing basically... getData() returns MaterialData which contains the byte version of the durability which is a small.

    Why are you doing this:
    Code:
    // The items are stored in the cursor. Make sure there's enough space.
                if (isStackSumLegal(toCraft, toStore)) {
                    int newItemsCount = toCraft.getAmount();
               
                    // Do what you need to do here!
                    // ...
                }
    ?
    You don't get more than 1 result if you click the result box without shift, and every time you click you get the event... so there's no need for that... unless you got another reason.

    I'm curious what you wrote in hasItems() to check if crafting anything.

    Oh and I just noticed you used InventoryClickEvent... why not use CraftItemEvent ?

    And finally just a minor thing:
    Code:
    switch (event.getInventory().getType()) {
            case CRAFTING:
            case WORKBENCH:
                handleCrafting(event);
                break;
            }
    Since switch() is fall-through, to avoid duplicated code :}

    EDIT: I see that this cloning method is valid in the IDE:
    Code:
    final ItemStack[] preInv = player.getInventory().getContents().clone();
    I don't know if it is working as expected tough.

    EDIT: aparently it doesn't work as expected
     
  7. Offline

    Comphenix

    Ah, yeah ... you're right about that. A genuine case of confusion, I guess. :p

    Yes, unless you're holding a full item stack (64 elements) - then, no crafting will occur. Though, it's possible Bukkit won't invoke the event in that case. I didn't test it, but I coded the check just in case.

    Ah, good catch. :)

    It's just a simple helper function though, nothing fancy:

    Code:
    private boolean hasItems(ItemStack stack) {
            return stack != null && stack.getAmount() > 0;
        }
    Right, well, this comes down to the fact that the code I posted has been adapted and simplified. It's actually from a mod I'm writing that gives experience to players for crafting items, brewing potions and destroying blocks, all of which is irrelevant for this particular problem. So after working in the solution to my mod, I cut it out again and removed all the boring details about experience.

    This also explains why I'm using InventoryClickEvent - the event handler is supposed to catch brew and furnace events as well. ;)


    Ah, yeah. Didn't catch that. In my defense, the original unadapted code is longer. :p

    That's because Array.clone() just creates a shallow copy - that is, a new array, but with references to the same ItemStacks. You have to copy each and every ItemStack manually (or use serialization, or some library) to avoid pointing to the same memory location.

    Let me know if you find anything else, though. :)
     
  8. I see.

    Actually yes it does trigger the event, so nvm :}
     
Thread Status:
Not open for further replies.

Share This Page