[Class] No more "Don't store players" crap: PlayerKeyHashMap<V> & PlayerValueHashMap<K>

Hi guys,

I wrote these 2 little classes to enable storing players without having to worry about removing players from it.

You use a normal HashMap like this:
HashMap<Key,Value> - HashMap<String,Integer>

They are used in the following way:

Code:

PlayerKeyHashMap<String> simplePlayerNameStringMap = new PlayerKeyHashMap<String>(plugin);
PlayerKeyHashMap<String> playerDisplayNameMap = new PlayerKeyHashMap<String>(plugin, PlayerKeyHashMap.PlayerSyncField.DISPLAY_NAME,true);

Setting the last boolean to false causes the class to not register the listener and therefore - Doesn't automatically remove the Entry when a Player Disconnects

Code:java

 
import java.util.HashMap;
import org.bukkit.entity.Player;
import org.bukkit.event.EventHandler;
import org.bukkit.event.Listener;
import org.bukkit.event.player.PlayerQuitEvent;
import org.bukkit.plugin.Plugin;
 
/**
*
* @author Thomas Nairn
* @author TomFromCollege
*/
public class PlayerKeyHashMap<V> extends HashMap<String, V> implements Listener {
 
private final PlayerSyncField playerSyncField;
 
/**
*
* @param Plugin The plugin used to register the listener
* @param playerSyncField An Enum contained within this class that allows
* you to sync more than just the Player Name
*/
public PlayerKeyHashMap(Plugin plugin, PlayerSyncField playerSyncField, boolean autoRemove) {
if (playerSyncField == null) {
throw new NullPointerException("PlayerSyncField must be set.");
}
this.playerSyncField = playerSyncField;
if (autoRemove) {
plugin.getServer().getPluginManager().registerEvents(this, plugin);
}
}
 
/**
* <p>This constructor should be used only when wanting to sync the
* "PlayerName" field </p>
*/
public PlayerKeyHashMap(Plugin plugin) {
this(plugin, PlayerSyncField.NAME, true);
}
 
@EventHandler
private void onPlayerDisconnect(PlayerQuitEvent e) {
synchronized (this) {
remove(getPlayerField(e.getPlayer()));
}
}
 
public synchronized V put(Player player, V value) {
return put(getPlayerField(player), value);
}
 
public synchronized V get(Player player) {
return get(getPlayerField(player));
}
 
private String getPlayerField(Player player) {
switch (playerSyncField) {
case NAME:
return player.getName();
case DISPLAY_NAME:
return player.getDisplayName();
case LIST_NAME:
return player.getPlayerListName();
}
return player.getName();
}
 
/**
*
*/
public static enum PlayerSyncField {
 
NAME, DISPLAY_NAME, ADDRESS, LIST_NAME;
}
}
 

This next one I wouldn't really suggest using it as any player disconnect will cause it loop through the players in the List, it was really more for completion and for people requiring the use of a custom Key.

Code:java

 
 
import java.util.HashMap;
import java.util.Iterator;
import java.util.Map.Entry;
import org.bukkit.entity.Player;
import org.bukkit.event.EventHandler;
import org.bukkit.event.Listener;
import org.bukkit.event.player.PlayerQuitEvent;
import org.bukkit.plugin.Plugin;
 
/**
*
* I Wouldn't really suggest using this one, as removing the values can be a
* little slow due to the necessity of looping through the values to find key
*
* @author Thomas Nairn
* @author TomFromCollege
*
*
*/
public class PlayerValueHashMap<K> extends HashMap<K, String> implements Listener {
 
private final PlayerKeyHashMap.PlayerSyncField playerSyncField;
 
/**
*
* @param Plugin The plugin used to register the listener
* @param playerSyncField An Enum contained within this class that allows
* you to sync more than just the Player Name
*/
public PlayerValueHashMap(Plugin plugin, PlayerKeyHashMap.PlayerSyncField playerSyncField, boolean autoRemove) {
if (playerSyncField == null) {
throw new NullPointerException("PlayerSyncField must be set.");
}
this.playerSyncField = playerSyncField;
if (autoRemove) {
plugin.getServer().getPluginManager().registerEvents(this, plugin);
}
}
 
/**
* <p>This constructor should be used only when wanting to sync the
* "PlayerName" field </p>
*/
public PlayerValueHashMap(Plugin plugin) {
this(plugin, PlayerKeyHashMap.PlayerSyncField.NAME, true);
}
 
@EventHandler
private void onPlayerDisconnect(PlayerQuitEvent e) {
synchronized (this) {
remove(get(e.getPlayer()));
}
}
 
public synchronized String put(K key, Player player) {
return put(key, getPlayerField(player));
}
 
/**
* I wouldn't really use this, it's only really here for completion...
*/
public synchronized K get(Player player) {
Iterator<Entry<K, String>> it = entrySet().iterator();
String field = getPlayerField(player);
while (it.hasNext()) {
Entry<K, String> val = it.next();
if (val.getValue().equals(field)) {
return val.getKey();
}
}
return null;
}
 
private String getPlayerField(Player player) {
switch (playerSyncField) {
case NAME:
return player.getName();
case DISPLAY_NAME:
return player.getDisplayName();
case LIST_NAME:
return player.getPlayerListName();
}
return player.getName();
}
 
/**
*
*/
public static enum PlayerSyncField {
 
NAME, DISPLAY_NAME, ADDRESS, LIST_NAME;
}
}
 

 

Pardon my ignorance, but, why would you need to store players in a hashmap?

 

That's nice and all, but if people were thinking about going through this much trouble to store a player object, they'd probably more satisfied with the name storage and getting the player back from the string.

I'd also like to know, why would you need to store player objects?

 

Well I think this is cool lol. Seems pretty useful

 

TheGreenGamerHD
I don't think you realize what this does. Or even read it?
It handles ALL that for you. I mean come on. You have to have wanted to save a value per player? Be it cooldown or I-Don't-Know-What

It allows you to store an object per player, without having to go about the whole storing names thing.
It also ensures you don't bloat your server with player disconnects and connects. I can save a value for a player (A config value, permission maybe?) and not have to worry about removing it when the player disconnects

 

I know what it does -_-

I'm just saying it's silly to go through all that work when you really don't have to.


Regardless, nice I guess

 

Haha I see what you're saying. But.. I use PlayerName HashMaps through about 9 different plugins and it occurred to me today that this would make my life easier, it took about 20 minutes and the hard work is done!

 

If you need a concurrent map of players, why not use Guava's MapMaker and store the players with weak references?

Code:java

import java.util.concurrent.ConcurrentMap;
 
import org.bukkit.entity.Player;
 
import com.google.common.collect.ForwardingConcurrentMap;
import com.google.common.collect.MapMaker;
 
/**
 * Represents a concurrent hash-map using a Bukkit player object as key. The player keys are 
 * referenced weakly, and will be properly collected once each player has logged out.
 * @author Kristian
 * @param <TValue> Type of the stored value.
 */
public class PlayerHashMap<TValue> extends ForwardingConcurrentMap<Player, TValue> {
    protected ConcurrentMap<Player, TValue> delegate;
 
    /**
     * Construct a new player-based concurrent hash map.
     */
    public PlayerHashMap() {
        this.delegate = new MapMaker().weakKeys().makeMap();
    }
 
    /**
     * Construct a new player-based concurrent hash map.
     * @param concurrencyLevel - guides the allowed concurrency among update operations. 
     */
    public PlayerHashMap(int concurrencyLevel) {
        this.delegate = new MapMaker().weakKeys().concurrencyLevel(concurrencyLevel).makeMap();
    }
 
    // You could also register a listener and remove players explicitly.
    @Override
    protected ConcurrentMap<Player, TValue> delegate() {
        return delegate;
    }
}

I don't think there's a very compelling reason to use player names as keys when you can use weak references. Though, keep in mind that the key-values will not be immediately evicted after a player has logged out, but for most uses this shouldn't be an issue. You can always check if a player has logged out, or register an event listener like in TomFromCollege's version.

I would much prefer to use a concurrent hash map over a synchronized map, as they won't block during retrieval operations, avoiding the dreaded deadlock, and they're generally faster. You can get better consistency with blocking, but that requires proper care and use of concurrency primitives. Besides, you can use putIfAbsent and get atomic insertion.

But of course, that's only if need to worry about concurrency. Most plugins won't run on multiple threads (async event/async tasks), so the synchronized key word will just waste performance. In those cases, especially if performance is critical, you may use the second constructor that allows you to specify a concurrency level. Use one (1) if you know you'll never use it in a different thread.

 

And today I learnt something!
Never even heard of weak keys. I literally implemented a work around for the issue I was having.
I know this is going to sound stupid due to Bukkits dependency on Guava, but I really do hate the idea of depending on it.

 

You're welcome.

It's definitely something more people should be aware of. I don't really see a point to the whole Map<String, Object> pattern, except that it's very easy for beginners to understand. But it could be abstracted away in a PlayerHashMap class.

Well, that's fine, but Guava will always be bundled with Bukkit, or any Bukkit derivatives. There's nothing bad about taking advantage of it. The more you use it, the more you can't be without it.

The only thing you should watch out for is using depreciated methods or beta classes. While CraftBukkit itself may not upgrade Guava from version 10, other custom implementations have done so already.

 

Oh my god. Guava is beautiful. Comphenix, post your code in a resources snippet and allow me to scrap my horrendous code?

 

Hm. Perhaps you could just link to my post somewhere in the first thread post? It seems a bit wasteful to make another thread.

By the way, I should probably mention that MapMaker (and ConcurrentHashMap in Java 7 and earlier) does use locking as well, but it can accomodate multiple writers simultaneously by locking different sections of the hash table. However, I wouldn't recommend using synchronized methods, as someone else could synchronize against the same object and cause a deadlock. It's very unlikely, but it's better to be safe than sorry.