Algorithm Critique

Discussion in 'Spigot Plugin Development' started by Hunky524, May 11, 2016.

  1. I don't have a question, but as this section of the forum is dedicated to plugin development, I'd thought I'd just get your ideas from a quick algorithm I made. So the problem I needed to solve was to be able to give a player an item. Sounds simple right? The idea was, when a player is given/bought an item, the method will only give them the item if they have space for it; the method will also try to fill pre-existing slots already containing the same item before filling empty slots if needed. I know Spigot has a method to give players items, however, that method returns a HashMap of any items it was not able to give the play, which is not exactly what I want. Anyway, if you want to take a look and give feedback on areas that I might have redundant code, or if you know a more efficient method to do something, I am open to all ideas :)

    Code (Text):
    private boolean givePlayerItem(Player p, ItemStack item) {
           
            //Slot Number --- Item Amount
            //Stores Slots and Amounts for Pre-Existing Items
            HashMap<Integer, Integer> itemStore = new HashMap<Integer, Integer>();
            //Stores Last Known Slot Number of an Empty Slot
            int emptyStore = -1;
            //Stores ItemStack Amount
            int stack = item.getAmount();
           
            //Check for existing ItemStacks and Empty Spaces
            for (int i = 0; i < p.getInventory().getSize(); i++) {
                if (p.getInventory().getItem(i).isSimilar(item)) {
                    if (p.getInventory().getItem(i).getAmount() == item.getMaxStackSize()) {
                       
                    } else if (stack - p.getInventory().getItem(i).getAmount() < 1) {
                        itemStore.put(i, stack);
                        break;
                    } else {
                        itemStore.put(i, stack - p.getInventory().getItem(i).getAmount());
                        stack = stack - (item.getMaxStackSize() - p.getInventory().getItem(i).getAmount());
                        if (stack < 1)
                            break;
                    }
                } else if (p.getInventory().getItem(i) == null || p.getInventory().getItem(i).getType() == Material.AIR) {
                    emptyStore = i;
                } else {
                   
                }
            }//Initial for Loop
           
            //No Slots Available
            if (itemStore.isEmpty() && emptyStore == -1) {
                return false;
            }
           
            //Not Enough Space
            else if (!itemStore.isEmpty() && emptyStore == -1 && stack > 0) {
                return false;
            }
           
            //Filled Empty Slot
            else if (itemStore.isEmpty() && !(emptyStore == -1)) {
                p.getInventory().setItem(emptyStore, item);
                return true;
            }
           
            //Has Enough Slots of the Same Item
            else if (!itemStore.isEmpty() && stack < 1) {
                for (int i : itemStore.keySet()) {
                    ItemStack givenItem = p.getInventory().getItem(i);
                    givenItem.setAmount(p.getInventory().getItem(i).getAmount() + itemStore.get(i));
                    p.getInventory().setItem(i, givenItem);
                }
               
                return true;
            }
           
            //Has Some Slots, Fill Rest in Empty Slots
            else {
                int leftOver = item.getAmount();
               
                for (int i : itemStore.keySet()) {
                    ItemStack givenItemLoop = p.getInventory().getItem(i);
                    givenItemLoop.setAmount(p.getInventory().getItem(i).getAmount() + itemStore.get(i));
                    p.getInventory().setItem(i, givenItemLoop);
                    leftOver = leftOver - itemStore.get(i);
                }
               
                ItemStack emptyItemGive = item;
                emptyItemGive.setAmount(leftOver);
                p.getInventory().setItem(emptyStore, emptyItemGive);
               
                return true;
            }
        }
     
  2. You can check if addItem HashMap is empty then the inventory has enough space for the ItemStack, and if it's not empty then it doesn't, however, this would add the item with the addItem method, you can remove the item later, but this is not really an accurate solution.
    What I would do is make an integer object and then a for loop to the player.getInventory().getStorageContents() and check if the item is null, if it is, then add the maximum stack size to the integer object, if the item isn't null, check if the item is similar to the bought item, if so, you'll add the maximum item stack size minus the item amount to the integer object, then end the loop, and check if the bought item amount is smaller than or equals to the integer object, if so, add the item. I didn't test your method but it looks great, but you know, you could've done it much simpler.
    Here's a code I just wrote
    Code (Text):
            ItemStack BOUGHT_ITEM = new ItemStack(Material.DIAMOND, 10);
            int EMPTY = 0;

            for (ItemStack ITEMSTACK : PLAYER.getInventory().getStorageContents()) {
                if (ITEMSTACK == null) {
                    EMPTY += BOUGHT_ITEM.getType().getMaxStackSize();
                } else if (ITEMSTACK.isSimilar(BOUGHT_ITEM)) {
                    EMPTY += ITEMSTACK.getType().getMaxStackSize() - ITEMSTACK.getAmount();
                }
            }
            if (BOUGHT_ITEM.getAmount() <= EMPTY) {
                PLAYER.getInventory().addItem(BOUGHT_ITEM);
                PLAYER.sendMessage(ChatColor.GOLD
                        + "You have enough space to get " + BOUGHT_ITEM.getAmount() + " " + BOUGHT_ITEM.getType().toString() + ", EMPTY is "
                        + EMPTY + ".");
            } else {
                PLAYER.sendMessage(ChatColor.RED
                        + "You do not have enough space to get " + BOUGHT_ITEM.getAmount() + " " + BOUGHT_ITEM.getType().toString() + ", EMPTY is "
                        + EMPTY + ".");
            }
     
    #2 KazamiThe9029, May 11, 2016
    Last edited: May 11, 2016
    • Like Like x 1
  3. Oh damn, yea. I see your method is definitely much shorter and cleaner than mine. Your method utilizes the pre-built method in spigot to add items with a simple check before hand. In my method I am keeping track of all the potential spaces that I can store the item and then run a bunch of if-checks to see if I actually can store the item. The only real thing I would (maybe) add to your method if I were to "borrow" it, would be to modify it a little bit so that when/if it found enough spaces of the same item, it broke out of the for loop even though it probably makes an extremely insignificant difference.
     
    • Creative Creative x 1
  4. Sounds like a good idea, I'll make sure to do it if I needed it in the future :)