1.8.8 Problem with getting chest inventory

Discussion in 'Spigot Plugin Development' started by brs73, Feb 18, 2020.

  1. Hi, i have a problem with getting the chest inventory from a deathplayer, the chest got placed but i can't get the items.


    package hirogram.survival;

    import java.util.*;
    import org.bukkit.entity.*;
    import org.bukkit.block.*;
    import org.bukkit.event.player.*;
    import org.bukkit.inventory.ItemStack;
    import org.bukkit.event.block.*;
    import org.bukkit.event.*;


    public class DeathChestInteractListener implements Listener
    {
    Main plugin;
    Map<Player, Chest> chestMap;

    public DeathChestInteractListener(final Main plugin) {
    this.chestMap = (Map<Player, Chest>)DeathChest.chestMap;
    this.plugin = plugin;
    }
    @EventHandler
    public void onChestOpen(PlayerInteractEvent event) {
    Player p = (Player) event.getPlayer();
    Block block = event.getClickedBlock();
    if(event.getAction().equals(Action.LEFT_CLICK_BLOCK) || event.getAction().equals(Action.RIGHT_CLICK_BLOCK) && block != null);
    {
    if(block.getState() instanceof Chest || block.getState() instanceof DoubleChest)
    {
    event.setCancelled(true);
    Chest chest = ((Chest) block.getState());
    ItemStack[] items = chest.getInventory().getContents();
    p.getInventory().addItem(items);
    p.updateInventory();

    }
    }
    }
    }
     
  2. Is this throwing an error in the console? If so please post the error as well. And it definitely cannot hurt to put your code in code tags
     
  3. What are you trying to do, can you explain the problem? also show the code where you put the items in that chest
     
  4. Im making a DeathChest plugin, when you die a chest got placed in your location with your items, if you click the chest with right click the chest give the items from the deathplayer
     
  5. You might want to try something like:

    Code (Java):
    for(ItemStack items : chest.getInventory().getContents()){
       // Get the player's inventory and add the current item to it.
    }
     
  6. drives_a_ford

    Moderator

    Please go read Beginner Programming Mistakes and Why You're Making Them before you continue. Lack of encapsulation as well as static abuse are what jump out right away. * imports are also generally considered bad practice.

    You'd want to make sure the item is not null before adding to another container. The individual objects of the array returned by Inventory#getContents may be null yet Inventory#addItem expects non-null objects to be passed.
     
    • Like Like x 1
  7. I have a few things for this:

    1. I am not sure where you are seeing static abuse, if you could point it out that would be great, as far as I can see there is not static abuse, in fact there is no static keyword anywhere in the code he posted.
    2. This is just his event listener, where could he possibly add encapsulation to make it better? The only thing I can see that you might be referencing is that in
    Code (Java):
    package hirogram.survival;

    import java.util.*;
    import org.bukkit.entity.*;
    import org.bukkit.block.*;
    import org.bukkit.event.player.*;
    import org.bukkit.inventory.ItemStack;
    import org.bukkit.event.block.*;
    import org.bukkit.event.*;


    public class DeathChestInteractListener implements Listener
    {
    Main plugin;
    Map<Player, Chest> chestMap;

    public DeathChestInteractListener(final Main plugin) {
    this.chestMap = (Map<Player, Chest>)DeathChest.chestMap;
    this.plugin = plugin;
    }
    @EventHandler
    public void onChestOpen(PlayerInteractEvent event) {
    Player p = (Player) event.getPlayer();
    Block block = event.getClickedBlock();
    if(event.getAction().equals(Action.LEFT_CLICK_BLOCK) || event.getAction().equals(Action.RIGHT_CLICK_BLOCK) && block != null);
    {
    if(block.getState() instanceof Chest || block.getState() instanceof DoubleChest)
    {
    event.setCancelled(true);
    Chest chest = ((Chest) block.getState());
    ItemStack[] items = chest.getInventory().getContents();
    p.getInventory().addItem(items);
    p.updateInventory();

    }
    }
    }
    }
    the line:
    Code (Java):
    Main plugin;
    could be changed to:
    Code (Java):
    private Main plugin;
    3. I completely agree that
    Code (Java):
    *
    imports are normally a bad idea, and you should only import what you need.



    looking at the code again however, I think one of the issues might be with:
    Code (Java):
    public DeathChestInteractListener(final Main plugin) {
    this.chestMap = (Map<Player, Chest>)DeathChest.chestMap;
    this.plugin = plugin;
    }
    if OP could post the contents of DeathChest.chestMap, I think it might help, especially looks to be unused in the event listener, if that is where the items are being stored into the chest, it would be nice to see how you are doing it and see if that might be the issue.
     
  8. drives_a_ford

    Moderator

    The this.chestMap = (Map<Player, Chest>) DeathChest.chestMap line is a classic example. Not only that, but he should be storing UUIDs instead of Player objects as keys because the latter are not consistent across logins.

    Not could, but should. There is no reason to expose them like that, there is no reason for them to be package private. In fact, I'd personally make them final as well.
     
  9. Thanks, sorry for the problems