HashMap Help

Hey, I started a plugin, it was supposed to be VERY quick and easy...
But I keep running into a problem.
I'm trying to make it so that a player can place a hopper, and then ONLY he can break it.
But as it turns out...
My current setup makes it so that the player that placed it, cannot open it.​

I only have me to test it with right now, so Idk about other players.​

Code:java

package com.gmail.xyourfreindx.protectedhoppers;
 
 
import java.util.HashMap;
 
import org.bukkit.Location;
import org.bukkit.Material;
import org.bukkit.event.EventHandler;
import org.bukkit.event.Listener;
import org.bukkit.event.block.Action;
import org.bukkit.event.block.BlockBreakEvent;
import org.bukkit.event.block.BlockPlaceEvent;
import org.bukkit.event.player.PlayerInteractEvent;
import org.bukkit.plugin.PluginDescriptionFile;
import org.bukkit.plugin.java.JavaPlugin;
 
public class ProtectedHoppers extends JavaPlugin implements Listener {
    public HashMap<Location, String> bl = new HashMap<Location, String>();
  public void onEnable()
  {
    PluginDescriptionFile pdfFile = getDescription();
    getServer().getPluginManager().registerEvents(this, this);
    getLogger().info(pdfFile.getName() + " v" + pdfFile.getVersion() + " has been enabled");
}
  public void onDisable() {
    PluginDescriptionFile pdfFile = getDescription();
    getLogger().info(pdfFile.getName() + " v" + pdfFile.getVersion() + " has been disabled");
}
  @EventHandler
  public void onBlockInteract(PlayerInteractEvent e) {
    if ((e.getAction().equals(Action.RIGHT_CLICK_BLOCK)) && (e.getClickedBlock().getType().equals(Material.HOPPER))) {
        Location location = e.getClickedBlock().getLocation();
        String player = e.getPlayer().getUniqueId().toString();
        if (bl.get(location) != null){
            if (bl.get(location) != player && (!(e.getPlayer().hasPermission("ph.d")))) {
                e.setCancelled(true);
            }
        }
    }
}
  @EventHandler
  public void onBlockPlace(BlockPlaceEvent e) {
      if (e.getBlock().getType().equals(Material.HOPPER)) {
          bl.put(e.getBlock().getLocation(), e.getPlayer().getUniqueId().toString());
    }
}
 
@EventHandler
public void onBlockBreak(BlockBreakEvent e) {
      if (e.getBlock().getType().equals(Material.HOPPER)) {
          bl.remove(e.getBlock().getLocation());
    }
}
}
 

​

 

You need to create a getter for the hashmap. (I think) You could shortcut, and make the hashmap static, but that isn't good practice.

 

JeremyPark Unfortunately, I do not believe this is the issue.

 

xYourFreindx
In line 36 of your code:

Code:java

 
if(bl.get(location)!= player && (!(e.getPlayer().hasPermission("ph.d")))){
 

you're doing a comparison with a String. In Java, if you want to make sure an object "equals" another object, not that it's the same instance of said object, you need to use

Code:java

 
objectA.equals(objectB)
 

otherwise, you're creating a new object and checking if the object is the same instance as the String object in your hash map, which in this listener, will never equal.

I'm not sure if that will fix the issue entirely, but I'm pretty sure that if you're doing this in many places, you'll have issues.
To fix, rewrite that line as so:

Code:java

 
if(player.equalsIgnoreCase(bl.get(location)) && (!(e.getPlayer().hasPermission("ph.d")))){
 

The reason why I use player. instead of bl.get(location). is because bl.get() may return null and you'd have an NPE, whereas player is guaranteed to be instantiated by the lines above.

Furthermore, when putting a Location into the HashMap, I'd recommend cloning the location of the block instead of just storing the instance of the location the block has. The reason for this is that you're defensively linking a location in your own HashMap instead of linking the Location belonging to a Block which is regarded to be still in use by the GC and might have a memory leak.

Then again, I might be completely wrong in that assumption, but for me, I defensively clone objects like this to avoid having modifications performed on that object while it's still in the HashMap or other Collection.

 

The scope of the HashMap is the whole class, it works how OP is doing it. (Although it doesn't follow encapsulation ) Though getters and setters would be a good idea if you ever want to have another class that looks at that HashMap and modifies it.

gabizou I think you're actually spot on with that as the issue. Line 35 is the big issue, since you're using ==/!= comparison it's comparing memory addresses rather than lexicographically (isn't theory fun?) and with strings a == will never be equal. Gabizou's solution should fix your issues nicely.

 

Nateb1121 gabizou Gosh darn it!
You're completely right.
I cannot believe I hadn't noticed that.
That was something I was taught 15 billion times in basic java...
Thanks so much.