Solved Is my code good?

Hello bukkit staff & users. My problem is my code, i don't know if it is good enough... i have tested it and it worked, but.. some of my friends told me that it won't 'whitelist' the players, are they right? tried to explain them that they must stay on to get whitelisted, and they told me that they stay...

I hope that i don't get any hate (just asking)

PHP:

/**
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but ITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program.  If not, see <http://www.gnu.org/licenses/>.
*/

public class WhitelistManager {

    private FileConfiguration customConfig = null;
    private File customConfigFile = null;

    // Access this later from commands :0
    public BukkitTask bukkitTask;

    boolean whitelistActive = false;

    ConfigHandler configHandler = new ConfigHandler();

    List<String> playersInConfig = this.getCustomConfig().getStringList("Whitelisted-Players");

    public void reloadCustomConfig() {
        if (customConfigFile != null) return;
        customConfigFile = new File(AntiBotUltra.getInstance().getDataFolder(), "whitelist.yml");
        // Look for defaults in the jar
        Reader defConfigStream = new InputStreamReader(AntiBotUltra.getInstance().getResource("whitelist.yml"));
        if (defConfigStream == null) return;
        YamlConfiguration defConfig = YamlConfiguration.loadConfiguration(defConfigStream);
        customConfig.setDefaults(defConfig);
    }

    public FileConfiguration getCustomConfig() {
        if (customConfig != null) return customConfig;
        reloadCustomConfig();
        return customConfig;
    }

    public void saveCusotmConfig() {
        // System.out.println("whitelist.yml is null!");
        if (customConfig != null && customConfigFile != null) try {
            getCustomConfig().save(customConfigFile);
        } catch (IOException ex) {
            AntiBotUltra.getInstance().getLogger().log(Level.SEVERE, "Could not save whitelist.yml to " + ex);
        }
        else return;
    }

    public void saveDefaultConfig() {
        if (customConfigFile != null) {
        } else customConfigFile = new File(AntiBotUltra.getInstance().getDataFolder(), "whitelist.yml");
        if (customConfigFile.exists()) return;
        AntiBotUltra.getInstance().saveResource("whitelist.yml", false);
    }

    public boolean isOnWhitelist(Player player) {
        if (!configHandler.getCustomConfig().getBoolean("Protection.enabled")) return false;
        List<String> stringList = this.getCustomConfig().getStringList("Whitelisted-Players");
        if (IntStream.range(0, stringList.size()).mapToObj(stringList::get).anyMatch(playersInConfig -> playersInConfig.equalsIgnoreCase(player.getName()))) {
            System.out.println("[AntiBot-Ultra] -> Player " + player.getName() + " was found on the whitelist!");
            return true;
        }
        return false;
    }

    public boolean addPlayerToWhitelist(Player player) {
        if (!configHandler.getCustomConfig().getBoolean("Protection.enabled")) return false;
        bukkitTask = Bukkit.getScheduler().runTaskLater(AntiBotUltra.getInstance(), () -> {
            if (player.isOnline() && !isOnWhitelist(player)) {
                playersInConfig.add(player.getName());
                this.getCustomConfig().set("Whitelisted-Players", playersInConfig);
                this.saveCusotmConfig();
                // TODO Add loop to msg players
                System.out.println("[AntiBot-Ultra] -> " + player.getName() + " was added to the whitelist");
            } else {
                // Even if it doesn't get added to the list, just to be sure :)
                if (playersInConfig.contains(player.getName())) playersInConfig.remove(player.getName());
                System.out.println("[AntiBot-Ultra] -> whitelist task (" + bukkitTask.getTaskId() + ") for player " + player.getName() + " has been cancelled! Reason: Player left the server!");
                bukkitTask.cancel();
                return;
            }

        }, configHandler.getCustomConfig().getLong("Whitelist.add-delay") * 60); return false;
    }

    // Not tested
    public boolean removePlayerFromWhitelist(Player player) {
        if (!configHandler.getCustomConfig().getBoolean("Protection.enabled")) return false;
        if (player.isOnline() && isOnWhitelist(player)) {
            playersInConfig.remove(player.getName());
            this.getCustomConfig().set("Whitelisted-Players", playersInConfig);
            this.saveCusotmConfig();
            System.out.println("[AntiBot-Ultra] -> Player " + player.getName() + " was removed from the whitelist!");
            return true;
        }
        return false;
    }

    // Yeah... simplify.. :D
    public boolean isWhitelistActive() {
        if (whitelistActive) return true;
        else return false;
    }

    // Yeah... simplify.. :D
    public void setWhitelistActive(boolean isWhitelistActive) {
        if (isWhitelistActive) whitelistActive = true;
        else whitelistActive = false;
    }

}

 

Hey! I think it would be more helpful if you also post how you are whitelisting/unwhitelisting players and your functionality of your whitelist.

 

Can be simplified to "whiteListActive = isWhitelistActive;"

Can be simplfied to return whitelistActive

You do not need the else case. Remove it.

Instead of returning, just invert the boolean

As Gaster posted, we need to see how you are using the whitelist. On its own, this class does nothing.

 

I whitelist like this:
When a player enters the server, "addPlayerToWhitelist" will fire and run a scheduler that will check if the player is still on after the time has passed, if he is online then sure, add to whitelist, if not... well they don't get added to the whitelist. When the server is attacked (bot attacks), a method called "protection" will fire. When that fires, joining will be disabled for normal users and enabled for whitelisted users, and after the time has passed from the "protection" scheduler, joining is enabled again. When i remove the player is when a command is fired.

@Zombie_Striker

 

@Kermit_23
Assuming your code works as intended, and you made the changes above, your code looks fine.

 

Also, some of the variables have default/public access modifiers, i sugest you turn them all private and use accessors (Setters) to access them from outside the class. (Encapsulation) Although this does not affect the code directly, it is a technique used to controll access and validate values passed through to variables and also make your code future proof.

 

@mine-care @Zombie_Striker
Just ran the code, i get NPE


Here is my code

PHP:

    public void reloadCustomConfig() {
        if (customConfigFile == null) {
            customConfigFile = new File(AntiBotUltra.getInstance().getDataFolder(), "whitelist.yml"); // here
        }
        customConfig = YamlConfiguration.loadConfiguration(customConfigFile);
        // Look for defaults in the jar
        Reader defConfigStream = new InputStreamReader(AntiBotUltra.getInstance().getResource("whitelist.yml"));
        if (defConfigStream != null) {
            YamlConfiguration defConfig = YamlConfiguration.loadConfiguration(defConfigStream);
            customConfig.setDefaults(defConfig);
        }
    }

    public FileConfiguration getCustomConfig() {
        if (customConfig == null) {
            reloadCustomConfig(); // here
        }
        return customConfig;
    }

Also, this happens with my customconfig class too (the config one (same code), made it so i can add comments)
config.yml/whitelist.yml exists in the plugin source files

 

getInstance may be returning a null value. That is the only thing I can think of that will throw an NPE.

 

@Zombie_Striker Might be as i haven't called "instance = this" on enable. I come with an edit and tell you if it throws null
EDIT: IT WORKS, you were right! just had to call it on enable! Now i have to deal with another errors (just have to put my messages in' ' so that : will work )