HashMap Errors (I Think)

Discussion in 'Spigot Plugin Development' started by Leniency, Jun 1, 2016.

Thread Status:
Not open for further replies.
  1. So I have a code for a messaging system where you can message and reply to people.
    The error is that when the hashmaps are in the same class as the messaging commands, They don't work properly and I can't reply. However when I put them in a different class, they work. Can someone help me?
    Code that doesn't work:
    Code (Text):
    package com.blockops.core.commands;

    import java.util.HashMap;
    import java.util.Map;

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

    public class Messaging implements CommandExecutor {

        public Map<Player, Player> mes = new HashMap<Player, Player>();
        public Map<Player, Player> rep = new HashMap<Player, Player>();

        public boolean onCommand(CommandSender sender, Command command, String label, String[] args) {
            if (label.equalsIgnoreCase("msg")) {
                if (!(sender instanceof Player)) {
                    sender.sendMessage(ChatColor.DARK_RED + "You must be a player to use this command!");
                    return false;
                }
                Player p = (Player) sender;
                if (args.length < 2) {
                    p.sendMessage(ChatColor.RED + "Correct Usage: /msg <player> <message>");
                    return false;
                }
                Player t = Bukkit.getPlayer(args[0]);
                if (t == null) {
                    p.sendMessage(ChatColor.RED + "That player is not online!");
                    return false;
                }
                mes.put(p, t);
                rep.put(t, p);
                StringBuilder sb = new StringBuilder();
                for (int i = 1; i < args.length; i++) {
                    sb.append(" ").append(args[i]);
                }
                String m = sb.toString().trim();
                String f1 = ChatColor.DARK_GRAY + "[" + ChatColor.GOLD + "Me " + ChatColor.DARK_GRAY + "-> "
                        + ChatColor.GOLD + t.getName() + ChatColor.DARK_GRAY + "] " + ChatColor.GRAY + m;
                String f2 = ChatColor.DARK_GRAY + "[" + ChatColor.GOLD + p.getName() + ChatColor.DARK_GRAY + " -> "
                        + ChatColor.GOLD + "Me" + ChatColor.DARK_GRAY + "] " + ChatColor.GRAY + m;
                t.sendMessage(f2);
                t.playSound(t.getLocation(), Sound.ENTITY_EXPERIENCE_ORB_PICKUP, 1, 0);
                p.sendMessage(f1);
                p.playSound(t.getLocation(), Sound.ENTITY_EXPERIENCE_ORB_PICKUP, 1, 0);
                return true;
            }
            if (label.equalsIgnoreCase("r")) {
                if (!(sender instanceof Player)) {
                    sender.sendMessage(ChatColor.DARK_RED + "You must be a player to use this command!");
                    return false;
                }
                Player p = (Player) sender;
                if (args.length < 1) {
                    p.sendMessage(ChatColor.RED + "Correct Usage: /r <message>");
                    return false;
                }
                if (mes.containsKey(p)) {
                    Player t = mes.get(p);
                    if (!t.isOnline()) {
                        p.sendMessage(ChatColor.RED + "Player is no longer online!");
                    }
                    StringBuilder sb = new StringBuilder();
                    for (int i = 0; i < args.length; i++) {
                        sb.append(" ").append(args[i]);
                    }
                    String m = sb.toString().trim();
                    String f1 = ChatColor.DARK_GRAY + "[" + ChatColor.GOLD + "Me " + ChatColor.DARK_GRAY + "-> "
                            + ChatColor.GOLD + t.getName() + ChatColor.DARK_GRAY + "] " + ChatColor.GRAY + m;
                    String f2 = ChatColor.DARK_GRAY + "[" + ChatColor.GOLD + p.getName() + ChatColor.DARK_GRAY + " -> "
                            + ChatColor.GOLD + "Me" + ChatColor.DARK_GRAY + "] " + ChatColor.GRAY + m;
                    t.sendMessage(f2);
                    t.playSound(t.getLocation(), Sound.ENTITY_EXPERIENCE_ORB_PICKUP, 1, 0);
                    p.sendMessage(f1);
                    p.playSound(t.getLocation(), Sound.ENTITY_EXPERIENCE_ORB_PICKUP, 1, 0);
                    return true;
                } else if (rep.containsKey(p)) {
                    Player t = rep.get(p);
                    if (!t.isOnline()) {
                        p.sendMessage(ChatColor.RED + "Player is no longer online!");
                    }
                    StringBuilder sb = new StringBuilder();
                    for (int i = 0; i < args.length; i++) {
                        sb.append(" ").append(args[i]);
                    }
                    String m = sb.toString().trim();
                    String f1 = ChatColor.DARK_GRAY + "[" + ChatColor.GOLD + "Me " + ChatColor.DARK_GRAY + "-> "
                            + ChatColor.GOLD + t.getName() + ChatColor.DARK_GRAY + "] " + ChatColor.GRAY + m;
                    String f2 = ChatColor.DARK_GRAY + "[" + ChatColor.GOLD + p.getName() + ChatColor.DARK_GRAY + " -> "
                            + ChatColor.GOLD + "Me" + ChatColor.DARK_GRAY + "] " + ChatColor.GRAY + m;
                    t.sendMessage(f2);
                    t.playSound(t.getLocation(), Sound.ENTITY_EXPERIENCE_ORB_PICKUP, 1, 0);
                    p.sendMessage(f1);
                    p.playSound(t.getLocation(), Sound.ENTITY_EXPERIENCE_ORB_PICKUP, 1, 0);
                    return true;
                } else {
                    p.sendMessage(ChatColor.RED + "You have not messaged anyone recently!");
                    return false;
                }
            }
            return false;
        }

    }
     
    Code that works:
    Code (Text):
    package com.blockops.core.commands;

    import java.util.HashMap;
    import java.util.Map;

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

    public class Main.i.messaging implements CommandExecutor {

        public boolean onCommand(CommandSender sender, Command command, String label, String[] args) {
            if (label.equalsIgnoreCase("msg")) {
                if (!(sender instanceof Player)) {
                    sender.sendMain.i.message(ChatColor.DARK_RED + "You must be a player to use this command!");
                    return false;
                }
                Player p = (Player) sender;
                if (args.length < 2) {
                    p.sendMain.i.message(ChatColor.RED + "Correct Usage: /msg <player> <Main.i.message>");
                    return false;
                }
                Player t = Bukkit.getPlayer(args[0]);
                if (t == null) {
                    p.sendMain.i.message(ChatColor.RED + "That player is not online!");
                    return false;
                }
                Main.i.mes.put(p, t);
                Main.i.rep.put(t, p);
                StringBuilder sb = new StringBuilder();
                for (int i = 1; i < args.length; i++) {
                    sb.append(" ").append(args[i]);
                }
                String m = sb.toString().trim();
                String f1 = ChatColor.DARK_GRAY + "[" + ChatColor.GOLD + "Me " + ChatColor.DARK_GRAY + "-> "
                        + ChatColor.GOLD + t.getName() + ChatColor.DARK_GRAY + "] " + ChatColor.GRAY + m;
                String f2 = ChatColor.DARK_GRAY + "[" + ChatColor.GOLD + p.getName() + ChatColor.DARK_GRAY + " -> "
                        + ChatColor.GOLD + "Me" + ChatColor.DARK_GRAY + "] " + ChatColor.GRAY + m;
                t.sendMain.i.message(f2);
                t.playSound(t.getLocation(), Sound.ENTITY_EXPERIENCE_ORB_PICKUP, 1, 0);
                p.sendMain.i.message(f1);
                p.playSound(t.getLocation(), Sound.ENTITY_EXPERIENCE_ORB_PICKUP, 1, 0);
                return true;
            }
            if (label.equalsIgnoreCase("r")) {
                if (!(sender instanceof Player)) {
                    sender.sendMain.i.message(ChatColor.DARK_RED + "You must be a player to use this command!");
                    return false;
                }
                Player p = (Player) sender;
                if (args.length < 1) {
                    p.sendMain.i.message(ChatColor.RED + "Correct Usage: /r <Main.i.message>");
                    return false;
                }
                if (Main.i.mes.containsKey(p)) {
                    Player t = Main.i.mes.get(p);
                    if (!t.isOnline()) {
                        p.sendMain.i.message(ChatColor.RED + "Player is no longer online!");
                    }
                    StringBuilder sb = new StringBuilder();
                    for (int i = 0; i < args.length; i++) {
                        sb.append(" ").append(args[i]);
                    }
                    String m = sb.toString().trim();
                    String f1 = ChatColor.DARK_GRAY + "[" + ChatColor.GOLD + "Me " + ChatColor.DARK_GRAY + "-> "
                            + ChatColor.GOLD + t.getName() + ChatColor.DARK_GRAY + "] " + ChatColor.GRAY + m;
                    String f2 = ChatColor.DARK_GRAY + "[" + ChatColor.GOLD + p.getName() + ChatColor.DARK_GRAY + " -> "
                            + ChatColor.GOLD + "Me" + ChatColor.DARK_GRAY + "] " + ChatColor.GRAY + m;
                    t.sendMain.i.message(f2);
                    t.playSound(t.getLocation(), Sound.ENTITY_EXPERIENCE_ORB_PICKUP, 1, 0);
                    p.sendMain.i.message(f1);
                    p.playSound(t.getLocation(), Sound.ENTITY_EXPERIENCE_ORB_PICKUP, 1, 0);
                    return true;
                } else if (Main.i.rep.containsKey(p)) {
                    Player t = Main.i.rep.get(p);
                    if (!t.isOnline()) {
                        p.sendMain.i.message(ChatColor.RED + "Player is no longer online!");
                    }
                    StringBuilder sb = new StringBuilder();
                    for (int i = 0; i < args.length; i++) {
                        sb.append(" ").append(args[i]);
                    }
                    String m = sb.toString().trim();
                    String f1 = ChatColor.DARK_GRAY + "[" + ChatColor.GOLD + "Me " + ChatColor.DARK_GRAY + "-> "
                            + ChatColor.GOLD + t.getName() + ChatColor.DARK_GRAY + "] " + ChatColor.GRAY + m;
                    String f2 = ChatColor.DARK_GRAY + "[" + ChatColor.GOLD + p.getName() + ChatColor.DARK_GRAY + " -> "
                            + ChatColor.GOLD + "Me" + ChatColor.DARK_GRAY + "] " + ChatColor.GRAY + m;
                    t.sendMain.i.message(f2);
                    t.playSound(t.getLocation(), Sound.ENTITY_EXPERIENCE_ORB_PICKUP, 1, 0);
                    p.sendMain.i.message(f1);
                    p.playSound(t.getLocation(), Sound.ENTITY_EXPERIENCE_ORB_PICKUP, 1, 0);
                    return true;
                } else {
                    p.sendMain.i.message(ChatColor.RED + "You have not Main.i.messaged anyone recently!");
                    return false;
                }
            }
            return false;
        }

    }
     
    Thanks.
     
  2. Choco

    Moderator

    If it's working then what's the issue?
    Also, try making those HashMaps static. That may fix the issue if it's just an instancing issue. You're probably calling more than one instance of the class. They should be static anyways. private static final, even.
     
  3. Yes it is most likely an instance issue. You are probably creating multiple different objects of the same class to run different commands. (new Commands(this)); would create an object with it's own public and private variables)
     
  4. Choco

    Moderator

    Or, make the Maps static, which is suggested. People tend to leave static alone due to static-abuse, but it's not static-abuse if it's used properly. In this case, they should be static, as instances of these objects does not matter
     
    • Agree Agree x 2
  5. But don't forget to null them in onDisable, otherwise it may cause memry leaks (although most likely unnoticeable...)
     
    • Agree Agree x 1
  6. There is still no reason for using static here. The common OOP way is still dependency injection here. Also the maps can be dependencies or you can put them inside the main class or a seperate class and use/create one instance and inject ist.

    For example you can create a class called MessageManager.
     
  7. Indeed, the Factory model is the "common OOP way" instead of static. Static is more for a utility model, and if something fits the factory model it is preferred to be the factory. But, neither way is essentially wrong. Personal preference trumps all, in this case. ^_^
     
    • Winner Winner x 1
  8. Yes, I am creating 2 instances of the same class. One for /msg and one for /r
    So if I create both using the same instance it will fix the issue?
     
    • Winner Winner x 1
  9. It will
     
  10. If an object is no longer referenced java garbage collects it anyways, so no issue.
     
  11. Doesnt quite work the same if static is used (which is what he was referencing).
     
  12. Indeed, idk how spigot handles server reloads exactly but if the class stays in the JVM then the object will probably just be re-used after the reload. If it actually reloads all the classes then I'm not entirely sure, but I would assume it wouldn't be a memory leak.
     
  13. Loaded classes stay in the JVM forever. Its one of the reasons /reload is ill-advised. It can load up the class again, making a new "instance" of it (for lack of a better word), but the old one still hangs around in memory. This was a common problem with Java 7, which is the entire reason Spigot recommended 128MB max perm size. No longer as relevant with Java 8, since the perm size can recalculate it self (or something, not too sure how it all works these days. dark magic, I suppose :p). Still, /reload should be avoided for the same reasons, though.
     
    • Like Like x 1
    • Informative Informative x 1
  14. Did this and it worked.

    Thanks for your help guys :D
     
    • Like Like x 1
Thread Status:
Not open for further replies.