CrystalCraftMC / CrystalBanker

This Spigot plugin will convert Minecraft XP orbs into bottles and bottles into XP orbs for your players!
Apache License 2.0
0 stars 1 forks source link

Improve handling of high numbers of experience points #9

Open jwflory opened 8 years ago

jwflory commented 8 years ago

Problem

If a player has a large number of experience levels beyond the threshold of the number of bottles their inventory can hold, the plugin does not function as expected.

Solution

Add a check to see how many free spots are left in the player's inventory and only withdraw the correct number of bottles that can fill their inventory.

immaterial-ctrl commented 7 years ago

I'm on winter break, and going to make a couple other changes to Banker (noted in #10 ) so I'll just throw this in the mix.

immaterial-ctrl commented 7 years ago

The check being considered exists, but obviously must not be working correctly; I put the whole method below for easy reference; I think it is related to armor slots and possibility a off-by-one error (yay! /sarcasm).

    /**
     * Calculates the number of free spaces in the player's inventory that can be used to fill with experience bottles.
     *
     * @param player the player whose inventory is being checked
     * @return the number of tallied free spaces for bottles
     */
    private int numInvFreeSpace(Player player) {
        PlayerInventory pi = player.getInventory();
        ItemStack[] is = pi.getContents(); //returns array of ItemStacks[] from inv
        int tally = 0;
        for (int i = 0; i < pi.getSize(); i++) {
            if (is[i] == null || is[i].isSimilar(new ItemStack(Material.AIR))) {
                tally += 64;
            }
        }
        for (int i = 0; i < pi.getSize(); i++) { // pi = player inventory object
            if (is[i] != null && !is[i].isSimilar(new ItemStack(Material.AIR))) {
                if (is[i].isSimilar(new ItemStack(Material.EXP_BOTTLE))) {
                    tally += 64 - (is[i].getAmount());
                }
            }
        }
        return tally;
    }
immaterial-ctrl commented 7 years ago

Confirmed the error! (its not off by one) Using my working version for the changes I still had the error - the update that added the shield slot adds another slot to inventory. So all I have to do is find a way to make getContents() of inventory ignore the armor slots - there is a method called getStorageContents() that should do the trick, but it the API reference manual didn't seem certain it would always exclude the armor slots.