for loop errors (or something else?)

Discussion in 'Plugin Development' started by TheManiacGamers, Mar 30, 2017.

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

    TheManiacGamers

    Alright,

    So I'm coding an EXPFly plugin,
    where the player can type /ff to fly, and their EXP gets deducted each second whilst it's enabled.

    Enabling and Disabling fly and checking if the player has exp works,

    but deducting does not.

    I Log into the server after restarting,
    type /ff, and it works.
    EXP is deducted, I get warning levels, etc
    My friend logs in after the server restarts,
    types /ff, and it doesn't work. He can fly, but the EXP isn't deducted, and no warning levels.

    As soon as I leave the server, he can then type /ff and it will work for him (exp deduction, etc)

    I've done so much as re-code it, change bukkit versions, google around, and I couldn't find anything to fix it.
    MAIN CLASS IS EXPFLY.java
    TASK CLASS IS EXPHANDLER.java
    COMMANDS CLASS IS COMMANDS.java
    I use maven to compile the plugin
    I use sk89q's command framework.
    I am using Spigot: (1.11.2-R0.1-SNAPSHOT)

    EXPHandler.java (open)

    Code:
    package me.yeroC.EXPFly.handlers;
    
    
    import com.sk89q.minecraft.util.commands.ChatColor;
    import me.yeroC.EXPFly.EXPFly;
    import me.yeroC.EXPFly.managers.PermissionsManager;
    import me.yeroC.EXPFly.managers.StringsManager;
    import me.yeroC.EXPFly.utils.ActionBarUtil.ActionBar;
    import me.yeroC.EXPFly.utils.CoreysAPI;
    import org.bukkit.Bukkit;
    import org.bukkit.entity.Player;
    import org.bukkit.scheduler.BukkitRunnable;
    
    /**
    * Created by Corey on 26/03/2017.
    */
    public class EXPHandler {
        StringsManager strings = StringsManager.getInstance();
        PermissionsManager perms = PermissionsManager.getInstance();
        CoreysAPI api = CoreysAPI.getInstance();
        static EXPHandler instance = new EXPHandler();
    
        private EXPHandler() {
        }
    
        public static EXPHandler getInstance() {
            return instance;
        }
    
        public void expCheck() {
            Bukkit.getScheduler().scheduleSyncRepeatingTask(EXPFly.plugin, new BukkitRunnable() {
                @Override
                public void run() {
                    if (Bukkit.getOnlinePlayers().size() != 0) {
                        for (Player ap : Bukkit.getOnlinePlayers()) {
                            if (ap.hasPermission(perms.EXPFly_Use)) {
                                if (EXPFly.expFlyToggle.get(ap.getPlayer()).equalsIgnoreCase("on")) {
                                    ap.setLevel(ap.getLevel() - 1);
                                    if (ap.getLevel() == 10) {
                                        ActionBar.send(ap.getPlayer(), ChatColor.RED + "WARNING: LOW EXP!!");
                                        return;
                                    }
                                    if (ap.getLevel() == 5) {
                                        ActionBar.send(ap.getPlayer(), ChatColor.RED + "WARNING: LOW EXP!!");
                                        return;
                                    }
                                    if (ap.getLevel() == 4) {
                                        ActionBar.send(ap.getPlayer(), ChatColor.RED + "WARNING: LOW EXP!!");
                                        return;
                                    }
                                    if (ap.getLevel() == 3) {
                                        ActionBar.send(ap.getPlayer(), ChatColor.RED + "WARNING: LOW EXP!!");
                                        return;
                                    }
                                    if (ap.getLevel() == 2) {
                                        ActionBar.send(ap.getPlayer(), ChatColor.RED + "WARNING: LOW EXP!!");
                                        return;
                                    }
                                    if (ap.getLevel() == 1) {
                                        ActionBar.send(ap.getPlayer(), ChatColor.RED + "WARNING: LOW EXP!!");
                                        return;
                                    }
                                    if (ap.getLevel() == 0) {
                                        ap.sendMessage(strings.defaultMsgs + ChatColor.RED + "EXP Flying has been disabled.");
                                        EXPFly.expFlyToggle.remove(ap.getPlayer());
                                        EXPFly.expFlyToggle.put(ap.getPlayer(), "off");
                                        ap.setAllowFlight(false);
                                        return;
                                    }
    
                                    return;
                                }
                                if (EXPFly.expFlyToggle.get(ap.getPlayer()).equalsIgnoreCase(null)) {
                                    EXPFly.expFlyToggle.remove(ap.getPlayer());
                                    EXPFly.expFlyToggle.put(ap.getPlayer(), "off");
                                }
                                return;
                            }
                            return;
                        }
                    }
                    // no return needed
                }
            }, 20L, 20L);
        }
    
    }
    


    EXPFly.java (open)

    Code:
    package me.yeroC.EXPFly;
    
    import com.sk89q.bukkit.util.CommandsManagerRegistration;
    import com.sk89q.minecraft.util.commands.*;
    import me.yeroC.EXPFly.handlers.EXPHandler;
    import me.yeroC.EXPFly.managers.StringsManager;
    import me.yeroC.EXPFly.registers.ListenersRegister;
    import org.bukkit.Bukkit;
    import org.bukkit.command.Command;
    import org.bukkit.command.CommandSender;
    import org.bukkit.command.ConsoleCommandSender;
    import org.bukkit.configuration.file.FileConfiguration;
    import org.bukkit.configuration.file.YamlConfiguration;
    import org.bukkit.entity.Player;
    import org.bukkit.event.Listener;
    import org.bukkit.plugin.java.JavaPlugin;
    
    import java.io.ByteArrayOutputStream;
    import java.io.DataOutputStream;
    import java.io.File;
    import java.io.IOException;
    import java.util.ArrayList;
    import java.util.HashMap;
    
    /**
    * Created by Corey on 19/08/2016.
    */
    public class EXPFly extends JavaPlugin implements Listener {
        private static EXPFly instance;
        public static EXPFly plugin;
        private ArrayList<Class> cmdClasses;
        public StringsManager strings = StringsManager.getInstance();
        public EXPHandler exphandler = EXPHandler.getInstance();
        private ListenersRegister listeners = ListenersRegister.getInstance();
        private CommandsManager<CommandSender> commands;
        public static HashMap<Player, String> expFlyToggle = new HashMap<>();
    
        public EXPFly() {
        }
    
        public static EXPFly getInstance() {
            return instance;
        }
    
        public static void log(String message) {
            System.out.println("[EXPFly] " + message);
        }
    
    
        public void onEnable() {
            plugin = this;
            this.plugin = plugin;
            registerCommandClass(Commands.class);
            registerCommands();
            getServer().getMessenger().registerOutgoingPluginChannel(this, "BungeeCord");
    
            listeners.registerListeners(plugin);
    
            // STARTING THE EXP CHECK TASK
            exphandler.expCheck();
    
            log("Plugin was loaded.");
    
            if (Bukkit.getOnlinePlayers().size() != 0) {
                for (Player p : Bukkit.getOnlinePlayers()) {
                    expFlyToggle.remove(p.getPlayer());
                    expFlyToggle.put(p.getPlayer(), "off");
                }
            }
        }
    
        public void onDisable() {
            if (Bukkit.getOnlinePlayers().size() != 0) {
                for (Player p : Bukkit.getOnlinePlayers()) {
                    expFlyToggle.remove(p.getPlayer());
                }
            }
        }
    
        @Override
        public boolean onCommand(CommandSender sender, Command cmd, String label, String[] args) {
            try {
                commands.execute(cmd.getName(), args, sender, sender);
            } catch (CommandPermissionsException e) {
    
                sender.sendMessage(ChatColor.RED + "You don't have permission.");
            } catch (MissingNestedCommandException e) {
                sender.sendMessage(ChatColor.RED + e.getUsage());
            } catch (CommandUsageException e) {
                sender.sendMessage(ChatColor.RED + e.getMessage());
                sender.sendMessage(ChatColor.RED + e.getUsage());
            } catch (WrappedCommandException e) {
                if (e.getCause() instanceof NumberFormatException) {
    
                    sender.sendMessage(ChatColor.RED + "You need to enter a number!");
                } else {
                    sender.sendMessage(ChatColor.RED + "Error occurred, contact developer. [TheManiacGamers]");
                    sender.sendMessage(ChatColor.RED + "Message: " + e.getMessage());
                    e.printStackTrace();
                }
            } catch (CommandException e) {
                sender.sendMessage(ChatColor.RED + e.getMessage());
            }
    
            return true;
        }
    
        protected void registerCommandClass(Class cmdClass) {
            if (cmdClasses == null)
                cmdClasses = new ArrayList<Class>();
    
            cmdClasses.add(cmdClass);
        }
    
        protected void registerCommands() {
            if (cmdClasses == null || cmdClasses.size() < 1) {
    
                log("Could not register commands! Perhaps you registered no classes?");
                return;
            }
    
            // Register the commands that we want to use
            commands = new CommandsManager<CommandSender>() {
                @Override
                public boolean hasPermission(CommandSender player, String perm) {
                    return getInstance().hasPerm(player, perm);
                }
    
    
            };
            commands.setInjector(new SimpleInjector(this));
            final CommandsManagerRegistration cmdRegister = new CommandsManagerRegistration(this, commands);
    
            for (Class cmdClass : cmdClasses)
                cmdRegister.register(cmdClass);
        }
    
        public boolean hasPerm(CommandSender sender, String perm) {
            return sender instanceof ConsoleCommandSender || sender.hasPermission(perm);
        }
    }
    
    //    public void onReload() {
    //        if (Bukkit.getOnlinePlayers().size() != 0) {
    //            for(Player player : Bukkit.getOnlinePlayers()) {
    //                PlayerData.loadedPlayer.put(player.getUniqueId(), new PlayerData(player.getUniqueId()));
    //            }
    //        }
    //    }
    
    
    //        if (Bukkit.getOnlinePlayers().size() != 0) {
    //            for (Player p : Bukkit.getOnlinePlayers()) {
    //
    //
    //            }
    //        }
    


    Commands.java (open)

    Code:
    package me.yeroC.EXPFly;
    
    
    import com.sk89q.minecraft.util.commands.ChatColor;
    import com.sk89q.minecraft.util.commands.Command;
    import com.sk89q.minecraft.util.commands.CommandContext;
    import me.yeroC.EXPFly.managers.PermissionsManager;
    import me.yeroC.EXPFly.managers.StringsManager;
    import me.yeroC.EXPFly.utils.CoreysAPI;
    import org.bukkit.command.CommandSender;
    import org.bukkit.entity.Player;
    
    
    /**
    * Created by Corey on 3/12/2016.
    */
    public class Commands {
        CoreysAPI api = CoreysAPI.getInstance();
        StringsManager strings = StringsManager.getInstance();
        PermissionsManager perms = PermissionsManager.getInstance();
        EXPFly plugin;
    
        public Commands(EXPFly plugin) {
            this.plugin = plugin;
        }
    
    
        @Command(aliases = "ff", desc = "Toggle EXP Flying on and off.")
        public void onFFCommand(CommandContext args, CommandSender sender) {
            if (sender instanceof Player) {
                Player p = ((Player) sender).getPlayer();
                if (p.hasPermission(perms.EXPFly_Use)) {
                    if (p.getLevel() >= 1) {
                        if (EXPFly.expFlyToggle.get(p.getPlayer()).equalsIgnoreCase("on")) {
                            EXPFly.expFlyToggle.remove(p.getPlayer());
                            EXPFly.expFlyToggle.put(p.getPlayer(), "off");
                            p.sendMessage(strings.defaultMsgs + ChatColor.GOLD + "EXP Flying now" + ChatColor.RED + "" + ChatColor.BOLD + " disabled" + ChatColor.GOLD + ".");
                            p.setAllowFlight(false);
                            return;
                        }
                        if (EXPFly.expFlyToggle.get(p.getPlayer()).equalsIgnoreCase("off")) {
                            EXPFly.expFlyToggle.remove(p.getPlayer());
                            EXPFly.expFlyToggle.put(p.getPlayer(), "on");
                            p.setAllowFlight(true);
                            p.sendMessage(strings.defaultMsgs + ChatColor.GOLD + "EXP Flying now" + ChatColor.GREEN + "" + ChatColor.BOLD + " enabled" + ChatColor.GOLD + ".");
                        } else {
                            EXPFly.expFlyToggle.remove(p.getPlayer());
                            EXPFly.expFlyToggle.put(p.getPlayer(), "off");
                            p.sendMessage(strings.defaultMsgs + ChatColor.GOLD + "EXP Flying now" + ChatColor.RED + "" + ChatColor.BOLD + " disabled" + ChatColor.GOLD + ".");
                            p.setAllowFlight(false);
                        }
                    } else {
                        p.sendMessage(strings.defaultMsgs + ChatColor.RED + "You have no EXP.");
                    }
                } else {
                    p.sendMessage(strings.noPermissionCommand);
                }
            } else {
                sender.sendMessage(strings.needToBePlayerCMD);
            }
        }
    }


    Your help would be greatly appreciated, thanks.

    This really baffles me, I've never seen it before.

    Thanks, Corey.
     
  2. Offline

    mine-care

    Oh dear...
    We need to resolve a few logic issues that appear in the code and then we will work on the problem.

    Please follow Java naming conventions ;) It makes the code much easier to follow.

    Please encapsulate the variables (private) and access them through getters and setters.

    The two things menthioned above apply here too ;)

    Right, first of all, if the hashmap does not contain that player, it will return null. So null.equalsIgnoreCase("on") will throw a null pointer exception. Check if the value is contained in the map before getting it or simply null-check.
    Then moving on to those repeated if's lets have a look:
    if 'number = 1' do 'thing'
    if 'number = 2' do 'thing'
    if 'number = 3' do 'thing'
    if 'number = 4' do 'thing'
    if 'number = 5' do 'thing'
    if 'number = 6' do 'thing'
    if 'number = 7' do 'thing'
    if 'number = 8' do 'thing'
    if 'number = 9' do 'thing'
    if 'number = 10' do 'thing'
    if 'number = 0' do 'other thing'
    Well wouldnt it be the same as:
    if 'number == 0' do 'other thing'
    else if 'number <=10' do 'thing'
    ?

    This is an attempted null check but it will fail because if get(ap.getPlayer()) returns null, then you are invoking the method on nothing which is impossible. Null checks are performed with == .
    in other words:
    if(pointer == null) { //it is null }

    again as i said above you need to encapsulate ;)
    Also storing Player instances in long-term storage during runtime is deprecated unless you remove the Players when they disconnect. Nevertheless, static is not needed here. You can not have more than one instances of the main class so static has no effect but causes RAM leaks since it is not handled propperly.

    So, plugin = this which is correct. However this.plugin = plugin isnt...
    this line is pointless because it is assigning 'plugin' to itself! its like saying:
    int x = 100;
    x = x;


    At a first stage mabe you should fix those issues and then we can have a look on the updated/wrapped code ;)
     
  3. Offline

    timtower Administrator Administrator Moderator

Thread Status:
Not open for further replies.

Share This Page