Solved HashMap Problem- Need Another Pair of Eyes

Discussion in 'Spigot Plugin Development' started by Gannam3, Aug 3, 2018.

  1. Hi,

    I might be incredibly dense here, but why am I not able to access this Hashmap in my pathfinding testing class? I've been beating my head against a wall. I don't usually have any problems accessing Hashmaps. What am I overlooking?

    Code (Java):

    //The class in question
    package pickquest.ai.pathfinding.tool;

    import java.util.EnumSet;
    import java.util.HashMap;
    import java.util.UUID;

    import org.bukkit.GameMode;
    import org.bukkit.Location;
    import org.bukkit.Material;
    import org.bukkit.entity.Player;
    import org.bukkit.event.EventHandler;
    import org.bukkit.event.Listener;
    import org.bukkit.event.block.Action;
    import org.bukkit.event.player.PlayerInteractEvent;
    import org.bukkit.inventory.EquipmentSlot;

    import pickquest.data.SafeBlocks;



    public class OriginPoint implements Listener {
     
        public Location atLocation;
     
        private HashMap<UUID, Location> definedOrigin = new HashMap<>();
        public UUID pid;
        public Location getDefinedOrigin(UUID pid) {
            return definedOrigin.get(pid);
        }
        public void setDefinedOrigin(HashMap<UUID, Location> definedOrigin) {
            this.definedOrigin = definedOrigin;
        }

        @EventHandler
        public void createOriginPoint(PlayerInteractEvent e) {
            Player player = e.getPlayer();
            UUID pid = player.getUniqueId();
            SafeBlocks sb = new SafeBlocks();
            EnumSet<Material> unsafeUnder = sb.getUnsafeUnder();
            EnumSet<Material> safeAt = sb.getSafeAt();
            EnumSet<Material> safeAbove = sb.getSafeAbove();
            String unsafeUnderString = unsafeUnder.toString();
            String safeAtString = safeAt.toString();
            String safeAboveString = safeAbove.toString();
            if(e.getAction() == Action.LEFT_CLICK_BLOCK) {
                if(e.getHand().equals(EquipmentSlot.HAND) && player.getInventory().getItemInMainHand() != null && player.getInventory().getItemInMainHand().getType().equals(Material.LIGHT_GRAY_DYE)) {
                if(player.getGameMode() == GameMode.CREATIVE) {
                    Location blockLocation = e.getClickedBlock().getLocation();
                    Location atLocation = new Location(player.getWorld(),blockLocation.getX(),blockLocation.getY() + 1,blockLocation.getZ());
                    Location upLocation = new Location(player.getWorld(),blockLocation.getX(),blockLocation.getY() + 2,blockLocation.getZ());
                    String blockMaterial = e.getClickedBlock().getLocation().getBlock().getType().toString();
                    String atLocationBlock = atLocation.getBlock().getType().toString();
                    String upLocationBlock = upLocation.getBlock().getType().toString();
                    if(unsafeUnderString.contains(blockMaterial)) {
                        player.sendMessage("That block is unsafe for humanoid mobs!");
                    }
                    else if(safeAtString.contains(atLocationBlock)) {
                        if(safeAboveString.contains(upLocationBlock)) {
                            definedOrigin.put(pid, atLocation);
                            player.sendMessage("Path origin set!");
                        }
                    }
                    e.setCancelled(true);
                }
        }
    }
    }
    }
    Code (Java):

    //The class I'm testing with - a bunch of stuff for pathfinding
    package pickquest.testing;

    import java.util.ArrayList;
    import java.util.HashMap;
    import java.util.LinkedList;
    import java.util.UUID;

    import org.bukkit.Location;
    import org.bukkit.command.Command;
    import org.bukkit.command.CommandExecutor;
    import org.bukkit.command.CommandSender;
    import org.bukkit.entity.Player;
    import org.bukkit.event.EventHandler;
    import pickquest.ai.pathfinding.Heuristic;
    import pickquest.ai.pathfinding.tool.OriginPoint;
    import pickquest.common.HumanoidSafety;
    import pickquest.common.operations.ScanRadiusBlk;
    import pickquest.data.BlockMaterialData;

    public class TestSafeLoc implements CommandExecutor {
        public HashMap<UUID,Location> getOriginPoint = new HashMap<>();
        @EventHandler  
        @Override
     
     
        public boolean onCommand(CommandSender sender, Command cmd, String label, String[] arg3) {
            if(sender instanceof Player) {
            int i1 = 0;
                Player p = (Player) sender;
                //Start with player location (will be modified)
                OriginPoint originpoint = new OriginPoint();
                UUID pid = p.getUniqueId();
                //HashMap<UUID, Location> getOriginPoint = originpoint.getDefinedOrigin();
                if(originpoint.getDefinedOrigin(pid) == null) {
                    return false;
                }
                else {
                Location origin = getOriginPoint.get(pid);
                Location current = origin;
                //The current testing point where path should move
                Location destination = new Location(p.getWorld(), (int)190, (int)69, (int)212);
                //Initialise radius class
                ScanRadiusBlk sra = new ScanRadiusBlk();
                //Initialise array in which to put locations in radius
                ArrayList<Location> srd = sra.getCoordinateCandidates();
                //Initialise class to test whether location is safe for human entrance
                HumanoidSafety hs = new HumanoidSafety();
                //Arraylist in which to put locations in radius that are safe
                ArrayList<Location> sl = hs.getSafeList();
                //Initialise class from which heuristics are first calculated (possible testing)
                Heuristic bh = new Heuristic();
                //As long as the current location isn't the end location
                while(!(current.equals(destination))) {
                //Do I need two maps to store heuristics? Trying to remember  
                //---Map<Double, Location> heuristic = bh.getHeuristicValues();
                    //Initialise operation to grab blocks in radius around current location, 1 radius
                    sra.scanRadius(current,1);
                    //For each block put in the radius arraylist
                    for(int i2 = 0; i2<srd.size(); i2++) {
                     
                        hs.testSafe(srd.get(i2));
                        if(srd.get(i2).equals(current)) {
                            srd.remove(i2);
                            sl.remove(origin);
                        }
                }
                    for(int i = 0;i<sl.size();i++) {
                        bh.findHeuristic(sl.get(i), destination);
                    }
                    sl.clear();
                    srd.clear();
                    LinkedList<Location> bestValues = bh.returnBestValues();
                    int currentHeuristicIndex = bestValues.size() - 1;
                    current = bestValues.get(currentHeuristicIndex);
                    i1++;  
                BlockMaterialData bm = new BlockMaterialData();      
                        for(Location loc: sl) {
                                bm.setBlockMaterialData(loc);
                        }
                    }
                }
                }
            return true;
        }
    }
     
  2. What works:
    Left click sets block location properly
    Broken block cancelled
    In class, UUID is properly stored with location
    The blocks are properly checked and the location is confirmed safe


    What doesn't:
    In testing class, getting the hashmap returns a null pointer exception any time it is called, even when I get the hashmap directly (right now, I'm getting the location from the hashmap in the other class - that doesn't work either)
     
  3. Inside your testing class, where you do your code to check whether or not an origin point for a particular UUID exists, you instantiate a new OriginPoint object and then you immediately call the #getDeclaredOrigin() method on that new object expecting to get some sort of result. You're overlooking the fact that, in your OriginPoint class, you're declaring and initializing a new empty HashMap called "definedOrigin".

    In your testing class, I think you intended to use the HashMap you've created in there, but instead suffered from a brain fart :p


    **EDIT**: Perhaps you intended the "definedOrigin" HashMap inside of OriginPoint to be a static member so the map persists throughout new instances?
     
    • Like Like x 1
  4. Oh, duh.

    Yeah, I did. I want to access that value statically. But I've always been told creating static variables and objects is just a bad idea. At the same time though, my goal is to set points and access them similarly to the way you might access points through WorldEdit's tool - that is, keeping them in memory. I'd rather not print them to a config file, but if I have to, I guess I could. Ordinarily, I'd just call the method from my testing class and call it good, but this is a separate trigger.

    I'm sorry if I'm very out of it. I've adopted a kitten, I'm not getting much sleep, and I've been working on this pretty much nonstop the past ten days.
     
  5. There are certain situations where making static members is a bad idea, such as situations where making a field static would make code less modular or negatively affect polymorphism. I use static fields and methods for immutable variables or methods that don't modify any states and/or that depend only on the parameters passed to them to return a result. In your situation, you could make some sort of origin point bank somewhere and pass the "gravy" to where it needs to go or stick with what you have.
     
    • Like Like x 1
  6. Wow.
    Now that I think about it, I'm kind of amazed I thought this would work.
    "Yeah. Let me just try to access values in terminated threads."
    I'll use common sense and make the location values for the tool static.
    Thank you.
     
    • Like Like x 1
  7. @Gannam3 Or do it properly instead, and pass the OriginPoint instance from onEnable (I guess it's created there as it is a Listener) to your test class.

    Static is definitely not the way to go here. ( @xMakerx as well)
     
  8. I'm sensing some snark here. And I'm really not sure why.
    As far as passing it from OnEnable, how does that help? Could you elaborate? Is that information during the play session stored somewhere in that instance and not when I call the class itself?
     
  9. A lot of people (usually ones new to programming) tend to just mindlessly smack the static keyword on fields to solve problems, rather than thinking about why they have the problem in the first place.
    You already have an instance (to register it as a listener), and need it in another class. Creating a new instance is not the solution, as it would create a completely new instance with its own Map field.

    The issue which static "solves", is giving both instances the same field memory (since static variables aren't handled by the instance), but that problem only existed because we created a new OriginPoint instance to be able to get access to the setter method.

    If you pass the OriginPoint object from onEnable to the CommandExecutor class, you completely avoid the access issue by giving it the object you're going to read from - read and write share the same piece of memory, and only amongst themselves.
     
    • Like Like x 1
  10. Okay, yeah. Like I said, I was reluctant to use static here, because I know it's typically discouraged.
    I'll work on applying a different fix, by calling it from onEnable() instead, a bit later. I've got a full day of stuff to fix today though.
    Thank you.
     
  11. Right, that's why I nicely suggested passing the gravy around instead and explained in which situations I personally employ static.