Open Inventory from another Inventory

Discussion in 'Plugin Development' started by ComputerTurtle, Apr 10, 2016.

Thread Status:
Not open for further replies.
  1. Offline

    ComputerTurtle

    I am trying to make a custom shop plugin, everything works except, it's not letting me open a second inventory when clicking on certain items in the inventory

    Code:
        @EventHandler
        public void onClick(InventoryClickEvent e) {
            Player p = (Player) e.getWhoClicked();
            ItemStack clicked = e.getCurrentItem();
            Inventory inv = p.getInventory();
            Inventory inv2 = Bukkit.createInventory(null, 18,
                    ChatColor.translateAlternateColorCodes('&', pl.getConfig().getString("Shop.Gui.BuyMenuName")));
    
            if (inv.getName().equalsIgnoreCase(
                    ChatColor.translateAlternateColorCodes('&', pl.getConfig().getString("Shop.Gui.HomeMenuName")))) {
                if (clicked.getType() == Material.CHEST && clicked.getItemMeta().equals(
                        ChatColor.translateAlternateColorCodes('&', pl.getConfig().getString("Shop.Gui.BuyMenuName"))));
                p.closeInventory();
                e.setCancelled(true);
                p.openInventory(inv2);
            } else {
                return;
            }
        }
    
     
  2. Offline

    Zombie_Striker

    This can throw ann NPE if the player does not click on a slot/ the slot is empty. Make sure the "clicked variable is not null.
    Are you sure the player's inventory is named after the inventory in the config?

    This will never be true. How can an Itemmeta be equal to a String.
    This is redundant. If this is the end of the method, it will stop reading anyway. Remove this statement.
     
  3. Offline

    MinionDoesMC

    Code:
    @EventHandler
        public void onClick(InventoryClickEvent e) {
            Player p = (Player) e.getWhoClicked();
            ItemStack clicked = e.getCurrentItem();
            Inventory inv = p.getInventory();
            Inventory inv2 = Bukkit.createInventory(null, 18,
                    ChatColor.translateAlternateColorCodes('&', pl.getConfig().getString("Shop.Gui.BuyMenuName")));
    
            if (inv.getName().equalsIgnoreCase(
                    ChatColor.translateAlternateColorCodes('&', pl.getConfig().getString("Shop.Gui.HomeMenuName")))) {
                if (clicked.getType() == Material.CHEST && clicked.getItemMeta().equals(
                        ChatColor.translateAlternateColorCodes('&', pl.getConfig().getString("Shop.Gui.BuyMenuName"))));
                p.closeInventory();
                e.setCancelled(true);
                p.openInventory(inv2);
            } else {
                return;
            }
        }
    }
    You are closing the if statement
    Code:
    if (clicked.getType() == Material.CHEST && clicked.getItemMeta().equals(
                        ChatColor.translateAlternateColorCodes('&', pl.getConfig().getString("Shop.Gui.BuyMenuName"))));
    It should be (If I am not mistaken)
    Code:
    @EventHandler
        public void onClick(InventoryClickEvent e) {
            Player p = (Player) e.getWhoClicked();
            ItemStack clicked = e.getCurrentItem();
            Inventory inv = p.getInventory();
            Inventory inv2 = Bukkit.createInventory(null, 18,
                    ChatColor.translateAlternateColorCodes('&', pl.getConfig().getString("Shop.Gui.BuyMenuName")));
    
            if (inv.getName().equalsIgnoreCase(
                    ChatColor.translateAlternateColorCodes('&', pl.getConfig().getString("Shop.Gui.HomeMenuName")))) {
                if (clicked.getType() == Material.CHEST && clicked.getItemMeta().equals(
                        ChatColor.translateAlternateColorCodes('&', pl.getConfig().getString("Shop.Gui.BuyMenuName")))) {
                    p.closeInventory();
                    e.setCancelled(true);
                    p.openInventory(inv2);
                }
            } else {
                return;
            }
        }
    }
    Removed the semicolon from
    Code:
    if (clicked.getType() == Material.CHEST && clicked.getItemMeta().equals(
                        ChatColor.translateAlternateColorCodes('&', pl.getConfig().getString("Shop.Gui.BuyMenuName"))));
    And replaced it with a brace and an ending brace
    Code:
    if (clicked.getType() == Material.CHEST && clicked.getItemMeta().equals(
                        ChatColor.translateAlternateColorCodes('&', pl.getConfig().getString("Shop.Gui.BuyMenuName")))) {
                    p.closeInventory();
                    e.setCancelled(true);
                    p.openInventory(inv2);
                }
            } else {
                return;
            }
        }
    }
    Also, I would do what Zombie_Striker said and remove the return statement.
     
    Last edited: Apr 10, 2016
  4. Offline

    Zombie_Striker

    @MinionDoesMC
    Thanks Captain Spoonfeeder! Giving code without even telling the other person what you did or why it's wrong!

    BTW: You still have the redundant return, have not added the null check to the clicked type, and did not fix the problem with a Player's inventories name being the same as a stored inventory name.
     
  5. Offline

    MinionDoesMC

  6. Offline

    Zombie_Striker

    @MinionDoesMC
    Still, you are writing code that he should be able to do, posting it here for him to copy and paste (one of the worst ways to solve a problem), and not telling him what you did or why it works!
     
  7. Offline

    MinionDoesMC

    So sorry for not explain what I did (Will work on this). But this place is for asking for help. I help people my way. You help people your way. Stop making a big deal out of it.
     
  8. Offline

    Zombie_Striker

    Wrong. Your way is to help him now. My way is to make sure he doesn't need other people to tell him whats wrong. For what you're doing, he does not even have to be a coder to make his plugin, all he needs to know is how to use ctrl+c and ctrl+v. He have had multiple thread discussing this very topic and why you should never spoonfeed anyone. (Here's the link if you want to read it ) In short, he will not learn anything by you doing his job.
     
  9. Offline

    ComputerTurtle

    I wasn't planning on copying or pasting, it's annoying, and like you said, doesn't help me.
     
  10. Offline

    Zombie_Striker

    @ComputerTurtle
    Luckly, it seems you like to know what each line of code does. But for some people, to be able to just copy and paste code is exactly what they are looking for. We have to make sure that they have to learn what they're doing and now allow them to do barely any thinking to solve their problem.
     
    MasterDoctor01 likes this.
  11. Offline

    MinionDoesMC

    I'm with you with the spoonfeeding thing. But why you are complaining about two stupid braces? Very childish indeed.
     
  12. Offline

    Zombie_Striker

    @MinionDoesMC
    Again, this is because adding "two stupid brackets" and posting that for him is not needed. All you needed to say was "add two stupid brackets" and it would have been fine. Although this is not as bad as posting whole classes/methods, posting code just to solve a problem is something you should not do.
     
  13. Offline

    ComputerTurtle

    Mhm, true.

    Here's what I've done so far, not sure if it'll work, but worth a shot.

    (This should be right?)
    Code:
        @EventHandler
        public void onClick(InventoryClickEvent e) {
            Player p = (Player) e.getWhoClicked();
            ItemStack clicked = e.getCurrentItem();
            Inventory inv = p.getInventory();
            Inventory inv2 = Bukkit.createInventory(null, 18,
                    ChatColor.translateAlternateColorCodes('&', pl.getConfig().getString("Shop.Gui.BuyMenuName")));
            if (inv.getName().equalsIgnoreCase(ChatColor.translateAlternateColorCodes('&', pl.getConfig().getString("Shop.Gui.HomeMenuName")))) {
                if (clicked.getType() == null) { //This should be right
                    e.setCancelled(true); //Idk if I put this in here or not, worth a shot
                }
              } 
                if (clicked.getType() == Material.CHEST && clicked.getItemMeta().equals(
                        ChatColor.translateAlternateColorCodes('&', (String) pl.getConfig().get("Shop.Gui.BuyMenuName")))) //Closest I could get here? {
                    p.closeInventory();
                    e.setCancelled(true);
                    p.openInventory(inv2);
                }
            }
        }
     
  14. Offline

    MinionDoesMC

    @Zombie_Striker
    I'm still learning the community. I have been moving to a new location irl so I've forgotten some stuff on here. I'll do my best to not spoonfeed.

    @ComputerTurtle
    Test it yourself. We can't test it for you.
     
  15. Offline

    ComputerTurtle

    I've done that.
    And it doesn't seem to work, I post the code, and I ask if it is right.
     
  16. Offline

    Zombie_Striker

    You should add a return statement after you cancel the event. This will prevent the rest of the code (including the part where you reference click) from being read.

    This is still a problem though. You are comparing am Itemmeta to a String. You are most likely thinking of something along the lines of ".getDisplayName"
     
  17. Offline

    ComputerTurtle

    Yeah, I was just checking the itemmeta
    code (open)

    Code:
       
        @EventHandler
        public void onClick(InventoryClickEvent e) {
            Player p = (Player) e.getWhoClicked();
            ItemStack clicked = e.getCurrentItem();
            Inventory inv = p.getInventory();
            Inventory inv2 = Bukkit.createInventory(null, 18,
                    ChatColor.translateAlternateColorCodes('&', pl.getConfig().getString("Shop.Gui.BuyMenuName")));
            if (inv.getName().equalsIgnoreCase(
                    ChatColor.translateAlternateColorCodes('&', pl.getConfig().getString("Shop.Gui.HomeMenuName")))) {
                if (clicked.getType() == null) {
                    e.setCancelled(true);
                    return;
                }
                    if (clicked.getType() == Material.CHEST && clicked.getItemMeta().getDisplayName() == ChatColor.translateAlternateColorCodes('&', pl.getConfig().getString("Shop.Gui.BuyMenuName"))); //Closest I could get here? {
                        p.closeInventory();
                        e.setCancelled(true);
                        p.openInventory(inv2);
            }
        }
    

    However, I tested, and it still does not open a new inventory
     
  18. Offline

    Zombie_Striker

    Code:
    .getDisplayName() == ChatColor.translateAlternateColorCodes('&', pl.getConfig().getString("Shop.Gui.BuyMenuName"))); //Closest I could get here? {
    First, do not use '=='. == is used for comparing memory space. Use equals when comparing the values of objects.

    Second, the semi colon at the end makes this if statement redundant. What you need to do is use the bracket for encapsulation.
     
  19. Offline

    ComputerTurtle

    Haha did not even notice I has a semicolon there,
     
  20. Offline

    mcdorli

    You shouldn't open an inventory in an inventory event
     
  21. Offline

    xDeeKay

    @ComputerTurtle I've ran into this issue in the past. Keep in mind I haven't messed around with inventory stuff for a while, so there may be another more practical way to fix this. What I did was simply run the second openInventory 1 tick later using a BukkitRunnable like so:

    Code:
    ...
    
    p.closeInventory();
    e.setCancelled(true);
                           
    new BukkitRunnable() {
        @Override
        public void run() {
            p.openInventory(inv2);
        }
    }.runTaskLater(plugin, 1);
    
    ...
    Again, this was a fix back in the day, so if anyone else comes up with a better fix then use theirs.
     
Thread Status:
Not open for further replies.

Share This Page