Bug in giving prison pearl to imprisoner

[Rudd-O]

    PrisonPearl pp = pearls.newPearl(imprisoned, imprisoner); // create the prison pearl

this will fail if the inventory is full

you should check and tell the user.

[matthewbot]

I don't think I understand the problem. The line you listed has nothing to do with the players inventory, but the surrounding code does have what seems to me to be sufficient inventory validation. If the players inventory is full, the plugin opts to imprison the player by overwriting a stack of enderpearls with a single prison pearl instead of letting the player go free.

[Rudd-O]

But that is not what the code does, right?

    PrisonPearl pp = pearls.newPearl(imprisoned, imprisoner); // create the prison pearl
    inv.setItem(pearlnum, new ItemStack(Material.ENDER_PEARL, 1, pp.getID())); // give it to the imprisoner
    imprisoned.setBedSpawnLocation(respawnworld.getSpawnLocation()); // reset the player's normal spawn location

    Bukkit.getPluginManager().callEvent(new PrisonPearlEvent(pp, PrisonPearlEvent.Type.NEW)); // set off an event
    return true;

Seems to me like the "give" fails here, and then the event is inconsistent because the "give" didn't actually take place in the case of a full inventory. The give does not overwrite stuff just like that, does it?

[matthewbot]

inv.setItem() is what modifies the inventory, but it overwrites the contents of the slot with the given item, so it can't fail, whatever was in that slot just gets deleted. pearlnum, the slot number, gets chosen based on the logic above that snippit, which finds an empty slot if at all possible but otherwise elects to overwrite ender pearls.

[Rudd-O]

Ahhhh.

I think it should consume inventory but not nuke an entire stack, only overwriting if it falls back. Add it to the TODO file I guess.