Vanish command problem

The command itself works, but when someone log off and then log on again, they can actually see the vanished player. I think it's an error in the for loop but I don't know how to fix it. Can anyone help?

Code:

Code:

public class CustomCommands implements CommandExecutor {
    public String cmd1 = "vanish";
    ArrayList<Player> vanished = new ArrayList<>();

    @Override
    public boolean onCommand(CommandSender sender, Command command, String label, String[] args) {
        if (sender instanceof Player) {
            Player player = (Player) sender;
            if (command.getName().equalsIgnoreCase(cmd1)) {
                if (sender.hasPermission("hidefromothers")) {
                    if (vanished.contains(player)) {
                        for (Player online : Bukkit.getOnlinePlayers()) {
                            online.showPlayer(player);
                        }
                        vanished.remove(player);
                        player.sendMessage("§aYou are now visible to other players");
                        return true;
                    } else if (!vanished.contains(player)) {
                        for (Player online : Bukkit.getOnlinePlayers()) {
                            online.hidePlayer(player);
                        }
                        vanished.add(player);
                        player.sendMessage("§aYou are now invisible to other players");
                        return true;
                    }
                } else {
                    player.sendMessage("§cYou do not have permission to perform this command");
                }
            }
        }
    }
}

 

What timtower said. The for loop can't be the problem because it's an onCommand function that only gets called on commands.
You already have an ArrayList with vanished players so check if player is in this list when he joins, then do the for loop again (write an own function for the for loop so you don't have to copy paste it.

Reduce nesting of your code. F.e instead of

Code:

if (condition) {

} else {

}

do

Code:

if (inverted condition) {

    return;
}

And never ever store player objects in an arraylist (at least not for this reason). Use UUID instead.
Oh and no need to do

Code:

if (contains) {

} else if (not contains) {

}

Just say else. There is no other possible outcome.

 

That is called a guard clause.

I believe it should be fine if they use a weak reference, but they do not in this case.