More efficient way of doing this?

Discussion in 'Plugin Development' started by Johnzeh, Apr 11, 2013.

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

    Johnzeh

    I want to check if the right clicked block is wool, then check if the wools color is the correct color, then check if the location in the config is correct, and if so do something.
    Can I get some ideas on a better way of doing this?

    Code:
        @EventHandler
        public void onObtain(PlayerInteractEvent e) {
            Player p = e.getPlayer();
            Block clicked = e.getClickedBlock();
            if(e.getAction() == Action.RIGHT_CLICK_BLOCK) {
                if(clicked.getType() == Material.WOOL) {
                    Wool wool = (Wool)clicked.getState().getData();
                    if(wool.getColor() == DyeColor.ORANGE) {
                        if(clicked.getLocation().getBlockX() == main.getConfig().getInt("Maps.Mapname.Wools.Orange.X")) {
                            if(clicked.getLocation().getBlockY() == main.getConfig().getInt("Maps.Mapname.Wools.Orange.Y")) {
                                if(clicked.getLocation().getBlockZ() == main.getConfig().getInt("Maps.Mapname.Wools.Orange.Z")) {
                                    //Do something
                                }else{
                                    e.setCancelled(true);
                                }
                            }else{
                                e.setCancelled(true);
                            }
                        }else{
                            e.setCancelled(true);
                        }
                    }
                    else if(wool.getColor() == DyeColor.YELLOW) {
                        if(clicked.getLocation().getBlockX() == main.getConfig().getInt("Maps.Mapname.Wools.Yellow.X")) {
                            if(clicked.getLocation().getBlockY() == main.getConfig().getInt("Maps.Mapname.Wools.Yellow.Y")) {
                                if(clicked.getLocation().getBlockZ() == main.getConfig().getInt("Maps.Mapname.Wools.Yellow.Z")) {
                                    //Do something
                                }else{
                                    e.setCancelled(true);
                                }
                            }else{
                                e.setCancelled(true);
                            }
                        }else{
                            e.setCancelled(true);
                        }
                    }
                    else if(wool.getColor() == DyeColor.CYAN) {
                        if(clicked.getLocation().getBlockX() == main.getConfig().getInt("Maps.Mapname.Wools.Cyan.X")) {
                            if(clicked.getLocation().getBlockY() == main.getConfig().getInt("Maps.Mapname.Wools.Cyan.Y")) {
                                if(clicked.getLocation().getBlockZ() == main.getConfig().getInt("Maps.Mapname.Wools.Cyan.Z")) {
                                    //Do something
                                }else{
                                    e.setCancelled(true);
                                }
                            }else{
                                e.setCancelled(true);
                            }
                        }else{
                            e.setCancelled(true);
                        }
                    }
                }
            }
        }
     
  2. Offline

    TGF

    Firstly - You don't have to setCancelled event every time ;) Don't make any else with it either. If if return false then you don't have to do anything.
     
  3. Offline

    Johnzeh

    Any ideas on checking locations?
     
  4. Offline

    ConflictRealms

    I actually had to do this with my code once. Couldn't find a more efficient way.
     
  5. Offline

    macguy8

    ConflictRealms
    Well, when you're checking locations, == doesn't work.
    Either compare the cords with &&s, (if (something && someOtherThing)), or use .equals(Location l);
     
  6. Offline

    ZeusAllMighty11

    You should really just store locations like this:
    Code:
    public String locToString(Location loc){
        String serialized = loc.getWorld().getName() + "," + loc.getX() + "," + loc.getY() + "," + loc.getZ() + "," + loc.getPitch() + "," + loc.getYaw());
        return serialized;
    }
     
  7. Consider using wool.getColor().toString().toLowerCase() to get the string lowercase name of the wool color so you don't have to go through all of them manually...
     
  8. Offline

    devilquak

    Johnzeh

    Much, much easier way to store & compare locations:

    Code:
    public String locationToString(Location l){
            return (l.getX() + ", " + l.getY() + ", " + l.getZ() + ", " + l.getPitch() + ", " + l.getYaw() + ", " + l.getWorld().getName());
        }
     
    public Location stringToLocation(String s){
        String[] rawLoc = s.split(", ");
        return new Location(Bukkit.getServer().getWorld(rawLoc[5]), Double.parseDouble(rawLoc[0]), Double.parseDouble(rawLoc[1]), Double.parseDouble(rawLoc[2]), Float.parseFloat(rawLoc[4]), Float.parseFloat(rawLoc[3]));
        }
     
Thread Status:
Not open for further replies.

Share This Page