1.12.2 Problem with overriding.

Discussion in 'Spigot Plugin Development' started by FishyHyper, Jan 10, 2020.

  1. Hello,
    I'm trying to code a death chest plugin.
    My problem is, I turn the items of the dead player into itemstacks, so the items aren't in a chest at all
    its more of a GUI that opens when you click the chest
    Thing is, it works I click the chest I see the items I had when I died BUT
    if another player dies the GUI will have their items and mine will disappearing, how can I fix that? that every chest/gui will have the items of the player without switching.

    When explaining please explain as much as you can English is not my first language and I sometimes don't understand everything

    Thanks for whoever helps!

    Code (Java):
        private Map<Block, ItemStack[]> droppedChests = new HashMap<>();
        private Map<UUID, Player> getNameMap = new HashMap<>();
        private Map<Player, Inventory> InventoryMap = new HashMap<>();
        private Inventory inventory;

        public void onPlayerDeath(PlayerDeathEvent e) {
            Player player = e.getEntity();
            Location playerLocation = player.getLocation();

            // Set the block type as Chest
            Chest chest = (Chest) player.getLocation().getBlock().getState();

            ItemStack[] items = Stream.of(player.getInventory().getContents()) // Create a stream of ItemStack
                    .toArray(ItemStack[]::new); // Convert the result to ItemStack array
            // Set a new entry in our Map using chest location as key and "filte
            // red" player items as value
            droppedChests.put(player.getLocation().getBlock(), items);
            InventoryMap.put(player, player.getInventory());
            getNameMap.put(player.getUniqueId(), player);

            if (items.length <= 9) {
              inventory =  Bukkit.createInventory(null, 9, color("&c&lPoor Player&7 - Small Chest")).setContents(items);
            } else if (items.length <= 27) {
                inventory = Bukkit.createInventory(null, 27, color("&6&lGood Player&7 - Medium Chest"));
            } else {
                inventory = Bukkit.createInventory(null, 54, color("&a&lRich Player&7 - Large Chest"));

        public void onPlayerInteract(PlayerInteractEvent event) {
            Player player = event.getPlayer();

            if (event.getAction() == Action.RIGHT_CLICK_BLOCK) {
                Block block = event.getClickedBlock();

                if (block.getType() == Material.CHEST) {
                    ItemStack[] items = droppedChests.get(block); // Getting drops from dead player.

                    // DEATH CHEST CHECK
                    if (droppedChests.containsKey(block)) {
                        Chest chest = (Chest) block.getState();

                        event.setCancelled(true); // Cancel Event
                        event.getPlayer().openInventory(InventoryMap.get(player));  // Opening Inventory


            } else if (event.getAction() == Action.LEFT_CLICK_BLOCK){
                Block block = event.getClickedBlock();
                if (droppedChests.containsKey(block)) {
                    Player getWhoClicked = event.getPlayer();
                    Location playerLocation = getWhoClicked.getLocation();

                    ItemStack[] items = droppedChests.get(block); // Getting drops from dead player.


                    player.sendMessage(color("&8[&a!&8] &aYou have broken a &f&nDeath Chest&a!"));
                    ItemStack[] itemStacks = droppedChests.get(block);
                    player.sendMessage(color("&8[&a!&8] &aYou have broken a &f&nDeath Chest&a!"));
  2. Ok i'll give you some suggestion on your code.

    1) You dont name fields getSomething
    get is traditionally reserved for actual getter methods.

    2) non static fields are styled in camel case by convention. thisIsSomeInv

    3) Don't hold a reference to a Block, Chunk World or other hard linked Objects. It is better to use a Location or UUID for example.

    4) This Map is useless. Player already has an exposed field that contains the UUID

    5) This Map is also not useful as you always can get the Inventory from the player at any time.
    This does NOT copy the inventory of the player. It just points to the inventory. So the Map always contains the same inventory
    you would get with player.getInventory()

    6) You create new inventorys but you dont save them anywhere...

    7) This needs a null check. Try to explain to yourself what happens if the Map<Block, ItemStack[]> does in fact NOT contain the block.
    It will return null then.

    8) This checks if the clicked block is in the map.
    Then you get the block as a chest (but you never use it anywhere)
    Then you open the players OWN inventory.

    9) Not going to bother to explain whats all wrong here. A lot of unused codes. You will duplicate items like crazy, removing drops from other players (clearing your maps for no reason at all)

    You need to get back on the drawing board.

    1) Go here https://www.youtube.com/channel/UCNXt2MrZaqfIBknamqwzeXA/playlists
    2) Watch everything in 'Java Programming Tutorial'
    3) Watch everything in 'Bukkit Coding Tutorial 1.13' or 'Bukkit Coding Tutorial' if you live in 2014
    4) Watch at least 2-3 Vids from 'Advanced Bukkit Coding'
    • Like Like x 1
    • Friendly Friendly x 1
  3. Hey thank you for all the explaniations and telling me how to improve myself, actually I've learned java before using Spigot API (of course I'm still a newbie) so I'm still in the learning process of how to properly use spigot api to my will, and yeah I took what you said and changed the code, can't duplicate any more and I've removed some unused code.
    Thanks again!
  4. Oh i see why you got the Map<UUID, Player> now.
    You can get a Player Object via Bukkit.getPlayer(UUID);