Can someone please look through my code?

Discussion in 'Spigot Plugin Development' started by GoduHD, May 18, 2017.

  1. I have the problem that nothing is Happening when i do a right click, but it should trigger a reaction! Can somebody please help me whats wrong with my Code? (Other Events in the same class are triggered correctly)
    Code (Text):
    @EventHandler
    public static void onClick(PlayerInteractEvent e){
            if(e.getAction() == Action.LEFT_CLICK_BLOCK){
                if(e.getClickedBlock().getLocation() == new Location(Bukkit.getWorld("HubLobbySpawn"), 93, 51, 0.5)){
                    e.setCancelled(true);
                    lobbysystem.addItem(1, e.getPlayer());
                } else if(e.getClickedBlock().getLocation() == new Location(Bukkit.getWorld("HubLobbySpawn"), 67, 51, 66)){
                    e.setCancelled(true);
                    lobbysystem.addItem(2, e.getPlayer());
                } else if(e.getClickedBlock().getLocation() == new Location(Bukkit.getWorld("HubLobbySpawn"), 1, 51, 92)){
                    e.setCancelled(true);
                    lobbysystem.addItem(3, e.getPlayer());
                } else if(e.getClickedBlock().getLocation() == new Location(Bukkit.getWorld("HubLobbySpawn"), -64, 51, 66)){
                    e.setCancelled(true);
                    lobbysystem.addItem(4, e.getPlayer());
                } else if(e.getClickedBlock().getLocation() == new Location(Bukkit.getWorld("HubLobbySpawn"), -2, 52, 11)){
                    e.setCancelled(true);
                }
            if(e.getAction() == Action.RIGHT_CLICK_AIR || e.getAction() == Action.RIGHT_CLICK_BLOCK){
                if(e.getPlayer().getItemInHand() == new ItemStack(Material.WATCH)){
                    List<String> list = new ArrayList<>();
                    Inventory spawnInv = Bukkit.createInventory(null, 3*9, "§7Wähle einen Spielmodus");
                    ItemStack placeholder = new ItemStack(Material.STAINED_GLASS_PANE, 1, (byte) 7);
                    ItemMeta placeholderM = placeholder.getItemMeta();
                    placeholderM.setDisplayName("");
                    placeholderM.setLore(list);
                    placeholder.setItemMeta(placeholderM);
                    spawnInv.setItem(0, placeholder);
                    spawnInv.setItem(1, placeholder);
                    spawnInv.setItem(2, placeholder);
                    ItemStack community = new ItemStack(Material.SKULL_ITEM);
                    SkullMeta communityM = (SkullMeta) community.getItemMeta();
                    communityM.setOwner("FitschLP");
                    communityM.setDisplayName("§6§lCommunity");
                    list.add("§7Zum Community-Bereich");
                    communityM.setLore(list);
                    list.remove("§7Zum Community-Bereich");
                    community.setItemMeta(communityM);
                    spawnInv.setItem(3, community);
                    spawnInv.setItem(4, placeholder);
                    ItemStack spawn = new ItemStack(Material.MAGMA_CREAM);
                    ItemMeta spawnM = spawn.getItemMeta();
                    spawnM.setDisplayName("§a§lSpawn");
                    list.add("§7Zum Spawn");
                    spawnM.setLore(list);
                    list.remove("§7Zum Spawn");
                    spawn.setItemMeta(spawnM);
                    spawnInv.setItem(5, spawn);
                    spawnInv.setItem(6, placeholder);
                    spawnInv.setItem(7, placeholder);
                    spawnInv.setItem(8, placeholder);
                    ItemStack ffa = new ItemStack(Material.DIAMOND_CHESTPLATE);
                    ItemMeta ffaM = ffa.getItemMeta();
                    ffaM.setDisplayName("§5§lFFA");
                    list.add("§7Zur Rundenauswahl");
                    ffaM.setLore(list);
                    ffa.setItemMeta(ffaM);
                    spawnInv.setItem(9, ffa);
                    ItemStack tof = new ItemStack(Material.WOOL);
                    ItemMeta tofM = tof.getItemMeta();
                    tofM.setDisplayName("§4§lTrouble On Fitschcraft");
                    tofM.setLore(list);
                    tof.setItemMeta(tofM);
                    spawnInv.setItem(10, tof);
                    ItemStack ptu = new ItemStack(Material.EYE_OF_ENDER);
                    ItemMeta ptuM = ptu.getItemMeta();
                    ptuM.setDisplayName("§2§lPersistent Universe");
                    ptuM.setLore(list);
                    ptu.setItemMeta(ptuM);
                    spawnInv.setItem(11, ptu);
                    spawnInv.setItem(12, placeholder);
                    spawnInv.setItem(13, placeholder);
                    spawnInv.setItem(14, placeholder);
                    spawnInv.setItem(15, placeholder);
                    spawnInv.setItem(16, placeholder);
                    spawnInv.setItem(17, placeholder);
                    spawnInv.setItem(18, placeholder);
                    spawnInv.setItem(19, placeholder);
                    spawnInv.setItem(20, placeholder);
                    spawnInv.setItem(21, placeholder);
                    spawnInv.setItem(22, placeholder);
                    spawnInv.setItem(23, placeholder);
                    spawnInv.setItem(24, placeholder);
                    spawnInv.setItem(25, placeholder);
                    spawnInv.setItem(26, placeholder);
                    e.getPlayer().openInventory(spawnInv);
                }
            }
        }
    }
     
    • Funny Funny x 2
  2. Strahan

    Benefactor

    Your right click check is within the left click logic block.
     
  3. This has nothing to do with any errors, but you shouldn't write any code that can be avoided by using loops.
    Code (Text):
    for(int i = 12; i <= 26; i++) spawnInv.setItem(i, placeholder);
     
  4. All of your equality checks will fail because they're comparing reference equality. Comparing a new object object via == to anything will always be false.

    Use #equals.
     
    • Agree Agree x 2
  5. Thanks for all of your replies but neither @1Rogue solution or @Strahan fixed the problem it just doesnt happen anything
     
  6. post your updated code, and show how you debugged it. Printing their actual location would be a good start.
     
  7. FrostedSnowman

    Resource Staff

    whats your code now?
     
  8. Why is your event handler even static?
     
  9. Instead of using else if statements, just use switch statement. It's slightly more efficient and I personally prefer it more. That's prolly not gonna fix your issue, just a suggestion. it also doesn't make sense why your eventhandler method is static. Comparing with "==" is for enums, ints, and what have you, but Not for objects. This was already stated above but I just thought I would add on. Objects have a equals() method that can actually be overridden in a object class to suite your needs. If you know about objects, you should already know that. In all honesty, it's probably a bracket issue, but idk for sure it's just a guess. I'm not home so I can't put it in my ide. Just debug it and go from there.
     
  10. Strahan

    Benefactor

    You likely have other problems but I can guarantee that the bracketing issue I pointed out will prevent your right click detection.

    What are you expecting to have happen? All you are doing is cancelling the event, there would be no action indicative of successful match anyway.

    I just tested, your method of comparing .getLocation() also does not work. Instead of comparing it to a new Location, compare .getLocation().getBlockX() / .getBlockY() / .getBlockZ() to check coords. I'd likely just make a little function to do it to prevent a lot of repetitive typing. I'd also make a List of the coords I want to match and just loop that rather than embed the locations in the checks. Makes it easier to change/add locations down the road. Just if you have it where players can input coords, you'd want to add data validation to the locationMatch function to ensure some moron doesn't put something like world/1/b or something.

    Code (Text):
    public Boolean locationMatch(Location source, String coords) {
        String[] data = coords.split("/");
        if (source.getWorld().getName().equalsIgnoreCase(data[0])) {
            if (source.getBlockX() == Integer.parseInt(data[1]) && source.getBlockY() == Integer.parseInt(data[2]) && source.getBlockZ() == Integer.parseInt(data[3])) return true;
        }
        return false;
    }

    @EventHandler
    public void onClick(PlayerInteractEvent e) {
        List<String> placesToMatch = new ArrayList();
        placesToMatch.add("HubLobbySpawn/93/51/0.5");
        placesToMatch.add("HubLobbySpawn/67/51/66");
        placesToMatch.add("HubLobbySpawn/1/51/92");
        placesToMatch.add("HubLobbySpawn/-64/51/66");
        placesToMatch.add("HubLobbySpawn/-2/52/11");

        if (e.getAction() == Action.LEFT_CLICK_BLOCK) {
            for (String locale : placesToMatch) {
                if (locationMatch(e.getClickedBlock().getLocation(), locale)) e.setCancelled(true);
            }
      }
    }
     
    #10 Strahan, May 19, 2017
    Last edited: May 19, 2017
  11. While that could work, that's also inefficient enough so that I wouldn't use it.
    Why turn the location into a string and then parse it again?
     
  12. Strahan

    Benefactor

    It wasn't my first choice, no, but when I tried comparing the block's .getLocation() to a new Location like OP is doing, it fails to match.

    EDIT: Nevermind, I see what was wrong. I had copied OP's line so I was using == instead of .equals lol. Now that it works, I'd use the config and do it this way:

    Code (Text):
    Player p = (Player)e.getPlayer();
    if (e.getAction() == Action.LEFT_CLICK_BLOCK) {
      for (String lcb : this.getConfig().getConfigurationSection("interact.leftclick").getKeys(false)) {
        if (e.getClickedBlock().getLocation().equals((Location)this.getConfig().get("interact.leftclick." + lcb))) {
          // do stuff
        }
      }
    }
     
    #12 Strahan, May 20, 2017
    Last edited: May 20, 2017
  13. Better, better; but I'd probably save the Locations from config to an array or something for quick lookup, seeing the interact even is called 'somewhat' frequently.
     
  14. Strahan

    Benefactor

    Ahh cool. Basically caching that part of the config to memory then for faster access? Thanks for the tip :)
     
  15. The config uses a Map internally anyhow, it's already in-memory.
     
  16. Yep, but by saving the Locations to an array or similar we'd avoid looping thru the map every time, plus cast it to the Location (I assume it's saved as a Location instance on the map?). Small optimization, I guess, but as a performance-nerd I'd sure do it.