Unknown error with Vanish plugin

I'm making a plugin that should make the player invisible to everyone except others with the permission(SneakyAdmins.Investigate).
It is not Vanishing the player and not adding the player to the active list. I see no reason why it shouldn't be adding the player to the list or why it shouldn't be making players without the appropriate permissions invisible unless my understanding of the getOnlinePlayers() is wrong.

I am not getting any error messages only it repeatedly saying that the player is hidden and creating a leave message.

Here is the code:

Code:

package com.thegamerplayz.SneakyAdmins.commands;

import java.util.ArrayList;
import java.util.List;

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

public class Investigate implements CommandExecutor {

    public boolean onCommand(CommandSender sender, Command command, String label, String[] args) {

        if(label.equalsIgnoreCase("investigate")){

            if (!(sender instanceof Player)) {

                sender.sendMessage("Silly Robot, This command is for players!");

                return false;
            }

            Player player = (Player) sender;
        
            List<Player> active = new ArrayList<Player>();
    
            if(player.hasPermission("SneakyAdmins.Investigate")){
    
                if(!(active.contains(player))){
            
                    for (Player muggle : Bukkit.getOnlinePlayers()) {
                        if((muggle.hasPermission("SneakyAdmins.Investigate"))){
                            continue;
                        }
                        muggle.hidePlayer(player); //Hides the wizards from muggles
                    }
                
                        player.setCanPickupItems(false);
                        player.sendMessage(ChatColor.RED + player.getName() + " is now Hidden!");
                    
                        Bukkit.broadcastMessage(ChatColor.YELLOW + player.getName() + " left the game.");
                    
                        active.add(player);
                }
                else if(active.contains(player)){

                    for (Player muggle : Bukkit.getOnlinePlayers()) {
                        if((muggle.hasPermission("SneakyAdmins.Investigate"))){
                            continue;
                        }
                        muggle.showPlayer(player); //Shows the wizards to muggles
                    }
                
                        player.setCanPickupItems(true);
                        player.sendMessage(ChatColor.RED + player.getName() + " is now Visible!");
                    
                        Bukkit.broadcastMessage(ChatColor.YELLOW + player.getName() + " joined the game.");
                    
                        active.remove(player);
                }
            }
            else{
                sender.sendMessage(ChatColor.DARK_RED + "You do not have the proper permissions");
            }
        }
            return false;
    }
}

 

Every time someone runs the command, you are creating a new List<Player>, since the code to create the list is inside the OnCommand section. This means the list is overriden every time someone typed the command

Declare it is a global variable underneath the "public class Investigate extends CommandExecutor"

Also, use command.getName().equalsIgnoreCase() instead of label.equalsIgnoreCase

 

Ontop of @ItsMas_ 's recomendations for code improvement, i would like to add:

Follow naming conventions.

Why not

Code:

if(contains player){ ... }else{...}

remember boolean !the primitive! have 2 possible values: true or false, so if it is not true it will be false. no need for a second check that potentially runs the code of 'contains' over.

 

I have plans to add event listeners in another package (com.thegamerplayz.listeners), but I do see your point so I'm changing it to it to SneakyAdmins.commands and SneakyAdmins.listeners .

Also do you see any reason why it would not be vanishing the player?

When i made the list outside of the boolean it made the list work correctly, but it is still not vanishing the player

Here is an update as i added some invisiblity for a fall back for when a player joins for now

Code:

package SneakyAdmins.commands;

import java.util.ArrayList;
import java.util.List;

import org.bukkit.Bukkit;
import org.bukkit.ChatColor;
import org.bukkit.command.Command;
import org.bukkit.command.CommandExecutor;
import org.bukkit.command.CommandSender;
import org.bukkit.entity.Player;
import org.bukkit.potion.PotionEffect;
import org.bukkit.potion.PotionEffectType;

public class Investigate implements CommandExecutor {

    List<Player> active = new ArrayList<Player>();
   
    public boolean onCommand(CommandSender sender, Command cmd, String label, String[] args) {

        if(cmd.getName().equalsIgnoreCase("investigate")){

            if (!(sender instanceof Player)) {

                sender.sendMessage("Silly Robot, This command is for players!");

                return false;
            }

            Player player = (Player) sender;
       
            if(player.hasPermission("SneakyAdmins.Investigate")){
       
                if(active.contains(player)){
               
                    for (Player muggle : Bukkit.getOnlinePlayers()) {
                        if((muggle.hasPermission("SneakyAdmins.Investigate"))){
                            continue;
                        }
                        muggle.showPlayer(player); //Hides the wizards from muggles
                    }
                   
                        player.setCanPickupItems(false);
                        player.sendMessage(ChatColor.RED + " You are now Visible!");
                        player.removePotionEffect(PotionEffectType.LEVITATION);
                       
                        Bukkit.broadcastMessage(ChatColor.YELLOW + player.getName() + " joined the game.");
                       
                        active.remove(player);
                }
                else{

                    for (Player muggle : Bukkit.getOnlinePlayers()) {
                        if((muggle.hasPermission("SneakyAdmins.Investigate"))){
                            continue;
                        }
                        muggle.hidePlayer(player); //Shows the wizards to muggles
                    }
                   

                    player.addPotionEffect(new PotionEffect(PotionEffectType.INVISIBILITY, 999999, 2, false, true));
                    player.setCanPickupItems(true);
                       
                    player.sendMessage(ChatColor.RED + "You are now Vanished!");
                       
                    Bukkit.broadcastMessage(ChatColor.YELLOW + player.getName() + " left the game.");
                       
                    active.add(player);
                }
            }
            else{
                sender.sendMessage(ChatColor.DARK_RED + "You do not have the proper permissions");
            }
        }
            return false;
    }
}

EDIT by Moderator: merged posts, please use the edit button instead of double posting.

 

1. Pretty sure you don't own thegamerplayz.com
2. You're still not following name conventions with the packages SneakyAdmins.commands
3. Format your code, it's pretty hard to read...
4. That's not how you use the continue keyword.
5. Why are you returning false?