Solved java.lang.ArrayIndexOutOfBoundsException

I am getting this error wen I run my teleport command with one argument (I know there is a vanilla teleport command, my plugin does other stuff too). The teleport command isn't wrong because it works with 0 and 2 arguments.

Code:

Player target0 = Bukkit.getPlayer(args[0]);
        Player target1 = Bukkit.getPlayer(args[1]);
        if (args.length == 1) {
            if (target0 == null) {
                player.sendMessage(ChatColor.DARK_RED + "Player to teleport not found.");
                return true;
            } else {
                Location targetLocation = target0.getLocation();
                player.teleport(targetLocation);
                player.sendMessage(ChatColor.GOLD + "Teleported to " + ChatColor.RED + args[0]);
                return true;
            }
        }

 

What's the class' name?

EDIT:
It appears that your error is at line 27 in class Teleport:

Code:

at me.bananasaredumb.xcess.commands.Teleport.onCommand(Teleport.java:27) ~[?:?]

Show entire code please.

 

Code:

package me.bananasaredumb.xcess.commands;

import org.bukkit.Bukkit;
import org.bukkit.ChatColor;
import org.bukkit.Location;
import org.bukkit.command.Command;
import org.bukkit.command.CommandExecutor;
import org.bukkit.command.CommandSender;
import org.bukkit.entity.Player;

public class Teleport implements CommandExecutor {

    public boolean onCommand(CommandSender sender, Command cmd,
            String commandLabel, String[] args) {
       
        Player player = (Player) sender;
        if (args.length == 0) {
            if (!(sender instanceof Player)) {
                sender.sendMessage("This command is for players only");
                return false;
            } else {
            player.sendMessage(ChatColor.DARK_RED + "Usage: /teleport <player> [player]");
            return true;
            }
        }
        Player target0 = Bukkit.getPlayer(args[0]);
        Player target1 = Bukkit.getPlayer(args[1]);
        if (args.length == 1) {
            if (target0 == null) {
                player.sendMessage(ChatColor.DARK_RED + "Player to teleport not found.");
                return true;
            } else {
                Location targetLocation = target0.getLocation();
                player.teleport(targetLocation);
                player.sendMessage(ChatColor.GOLD + "Teleported to " + ChatColor.RED + args[0]);
                return true;
            }
        } else if (args.length == 2) {
            if (target0 == null) {
                player.sendMessage(ChatColor.DARK_RED + "Player to teleport not found.");
                return true;
            } else if (target1 == null) {
                player.sendMessage(ChatColor.DARK_RED + "Target player not found.");
                return true;
            } else {
                Location targetPlayer1Location = target1.getLocation();
                target0.teleport(targetPlayer1Location);
                target0.sendMessage(ChatColor.GOLD + "You were teleported to " + ChatColor.RED + args[1] + ChatColor.GOLD + ".");
                if (!(sender == Bukkit.getPlayer(args[0]))) {
                    player.sendMessage(ChatColor.DARK_RED + args[0] + ChatColor.GOLD + " was teleported to " + ChatColor.DARK_RED + args[1] + ChatColor.GOLD + ".");
                return true;   
                }
            }
        }else if (args.length > 2) {
            player.sendMessage(ChatColor.DARK_RED + "Usage: /teleport <player> [player]");
            return true;
        }
        return false;
           
}
}

 

Your error is you're trying to get an argument that doesn't exist, you can't do Player target1 = Bukkit.getPlayer(args[1]); unless there is an args[1] to begin with. Make your ifs check if there are enough players BEFORE getting the argument. If you read the error it said java.lang.ArrayIndexOutOfBoundsException: 1 which means you're trying to get argument 1 which is the error

 

Maybe the second argument is not provided...Oh yes, I see, you messed up where you did "if (args.length == 1) ...." instead of "if(args.length == 2)...."

EDIT: Ahh, got ninja'd by @Xerox262

 

Code:

Player target1 = Bukkit.getPlayer(args[1]);

There's your issue.

 

Dont cast without checking! first check, then cast.

 

Here's the full code with (I think) no errors:

Code:

package me.bananasaredumb.xcess.commands;

import org.bukkit.Bukkit;
import org.bukkit.ChatColor;
import org.bukkit.Location;
import org.bukkit.command.Command;
import org.bukkit.command.CommandExecutor;
import org.bukkit.command.CommandSender;
import org.bukkit.entity.Player;

public class Teleport implements CommandExecutor {

    public boolean onCommand(CommandSender sender, Command cmd,
            String commandLabel, String[] args) {
        if (sender instanceof Player) {
            Player player = (Player) sender;
        } else {
            //send the console a message
            return true;
        }

        if (args.length == 0 || args.length > 2) {
            return false;
        }

        if (args.length == 1) {
            Player target0 = Bukkit.getPlayer(args[0]);
            if (target0 == null) {
                player.sendMessage(ChatColor.DARK_RED + "Player to teleport not found.");
                return true;
            } else {
                Location targetLocation = target0.getLocation();
                player.teleport(targetLocation);
                player.sendMessage(ChatColor.GOLD + "Teleported to " + ChatColor.RED + args[0]);
                return true;
            }
        } else if (args.length == 2) {
            Player target0 = Bukkit.getPlayer(args[0]);
            Player target1 = Bukkit.getPlayer(args[1]);
            if (target0 == null) {
                player.sendMessage(ChatColor.DARK_RED + "Player to teleport not found.");
                return true;
            } else if (target1 == null) {
                player.sendMessage(ChatColor.DARK_RED + "Target player not found.");
                return true;
            } else {
                Location targetPlayer1Location = target1.getLocation();
                target0.teleport(targetPlayer1Location);
                if (!(sender == Bukkit.getPlayer(args[0]))) {
                    player.sendMessage(ChatColor.DARK_RED + args[0] + ChatColor.GOLD + " was teleported to " + ChatColor.DARK_RED + args[1] + ChatColor.GOLD + ".");
                return true;
                } else {
                    target0.sendMessage(ChatColor.GOLD + "You were teleported to " + ChatColor.RED + args[1] + ChatColor.GOLD + ".");
                }
            }
        }
        return false;
}
}

There was so much problem with this code.

1.: Never. EVER. send the player a "/usage" message, do "return false" instead and set the argument "usage: /usage ..." for the command in the plugin.yml file.

2.: WHY was the code in pieces? You did "if (args == 0)..." and then like 40 rows later you do "if (args > 2)...", why not just use "if (args == 0 || args > 2)..."?

3. You did "Player player = (Player) sender" and then you do "if (sender instanceof Player)".

Learn java before programming a bukkit plugin.

And like always: Do not take this offensive. If you think this is offensive, then you never seen me being offensive.

 

I don't see how its wrong.

 

What? the first two just isn't efficient, and because bukkit is doing everything in 1 thread, you need to be efficient. The third is like if I raise in poker and just then check my cards.

 

Wait... You're ALLOWED to check your cards before you raise in poker? O_O xD I love the analogy and it's totally true. @robotballs You should NEVER cast before knowing if you're correctly casting.

 

I only see two where do you see the third?

Code:

Player target0 = Bukkit.getPlayer(args[0]);
        Player target1 = Bukkit.getPlayer(args[1]);

 

What did you mean about "I don't see what's wrong"?

 

About the

Code:

Player target1 = Bukkit.getPlayer(args[1]);

I did the same exact thing for this just different arguments.

Code:

Player target0 = Bukkit.getPlayer(args[0]);

 

That wasn't the problem, the main problem was with the sender casting and you initialized the target, before checking, that there is an argument for that, in code:

You did:

Code:

Player target0 = Bukkit.getPlayer(args[0]);
Player target1 = Bukkit.getPlayer(args[1]);
//args[1] doesn't exists, so it's a Out of bounds exception 

I did:

Code:

if (args.length == 2) {
    Player target0 = Bukkit.getPlayer(args[0]);
    Player target1 = Bukkit.getPlayer(args[1]);
    //args[1] doesn't exists, but this code doesn't run because the if statement
}

 

I see now. So I need to move it down or up some, correct?

 

Gotta get those nanosecond micro-optimisations. Heaven forbid we spend 1/1,000,000th of the amount of time we have doing something (that was an exaggeration, it's actually far less than one one-millionth).

It really bothers me when people talk about "efficiency" when they have no clue what they're talking about.

That being said, logical short-circuiting is a very good thing, but not because it's more efficient (that's just a nice benefit on the side).



@robotballs

You need to check the length of the array before you use an element in it. Simple as that.

 

@mythbusterma I don't say him/her, that this is millions time more efficient, I say it because its more efficient. Imagine this: You and a another programmer are onbthe same job interview. You say, you can make a 3d model displayed with nice shades in 300 lines, the other says he can display a 3d model with less shading, but only in 30 lines. Guess who would get the job.

I want to say him, to search for the best and shortest variation of doing it. Maybe it uses a lot more time, than it actually saves, but if you have a big code, doing the same thing, it can save you real amounts of time, and it's cleaner too.

 

@mcdorli

Again, logical short-circuiting is a good thing. I'm not talking about readability, I'm talking about pre-emptive micro-optimisation. Which is what you were doing. I can guarantee pre-emptive optimisation will cost you any job worth its salt.

Readability is the only thing that matters in this case, as you're nowhere near any amount of processing that would ever matter on any server.

 

I think it's more readable, if the If statements are in one place if they check the samme thing.

 

@mcdorli

That is also more readable, but efficiency is not the reason you should do it. Readability is.