Util PlayerStorage - Store players the safe way!

Discussion in 'Resources' started by megamichiel, Feb 23, 2015.

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

    megamichiel

    I've seen many people come by that store players in stuff like arraylists, but that's not good at all because it can cause huge server leaks. And if you want to fix it you need to create a map/list of strings/uuids and then adding all their uuid's/names. But with this simple util you can easily store players without any worries about leaks! Here's the two classes I made for this: (FYI: It uses UUD's, so it won't work for versions < 1.7.5.

    EDIT: Changed hashmap to weakhashmap

    PlayerList.java (open)

    Code:
    package your.package.name;
    
    import java.util.ArrayList;
    import java.util.Collection;
    import java.util.Iterator;
    import java.util.List;
    import java.util.UUID;
    
    import org.bukkit.Bukkit;
    import org.bukkit.entity.Player;
    
    public class PlayerList implements Iterable<Player> {
      
        private final List<UUID> list;
      
        public PlayerList(Player... contents) {
            list = new ArrayList<UUID>();
            if(contents != null && contents.length > 0) {
                for(Player p : contents)
                    list.add(p.getUniqueId());
            }
        }
      
        @Override
        public Iterator<Player> iterator() {
            List<Player> players = new ArrayList<Player>();
            for(UUID uuid : list.toArray(new UUID[0]))
                players.add(Bukkit.getPlayer(uuid));
            return players.iterator();
        }
      
        public void add(Player p) {
            if(!list.contains(p.getUniqueId())) list.add(p.getUniqueId());
        }
      
        public void remove(Player p) {
            if(list.contains(p.getUniqueId())) list.remove(p.getUniqueId());
        }
      
        public boolean contains(Player p) {
            return list.contains(p.getUniqueId());
        }
      
        public boolean containsAll(Collection<Player> players) {
            for(Player p : players.toArray(new Player[0]))
                if(!list.contains(p.getUniqueId())) return false;
            return true;
        }
      
        public Player get(int index) {
            return Bukkit.getPlayer(list.get(index));
        }
      
        public boolean isEmpty() {
            return list.isEmpty();
        }
      
        public Player[] toArray() {
            List<Player> players = new ArrayList<Player>();
            for(UUID uuid : list.toArray(new UUID[0]))
                players.add(Bukkit.getPlayer(uuid));
            return players.toArray(new Player[0]);
        }
      
        public void addAll(Player... players) {
            for(Player p : players)
                if(!list.contains(p.getUniqueId())) list.add(p.getUniqueId());
        }
      
        public void clear() {
            list.clear();
        }
      
        public int size() {
            return list.size();
        }
    }

    If you want to create a new list you just create a PlayerList instance with either no parameters or the players you want to add. It works just like a normal ArrayList. I also let it implement Iterable<Player> so you can use a for statement on this so you can easily get every player of the list.

    PlayerMap.java (open)

    Code:
    package your.package.name;
    
    import java.util.Collection;
    import java.util.WeakHashMap;
    import java.util.HashSet;
    import java.util.Set;
    import java.util.UUID;
    
    import org.bukkit.Bukkit;
    import org.bukkit.entity.Player;
    
    public class PlayerMap<T> {
      
        private final WeakHashMap<UUID, T> info;
      
        public PlayerMap() {
            info = new WeakHashMap<UUID, T>();
        }
      
        public void put(Player p, T value) {
            info.put(p.getUniqueId(), value);
        }
      
        public void remove(Player p) {
            info.remove(p.getUniqueId());
        }
      
        public T get(Player p) {
            return info.get(p.getUniqueId());
        }
      
        public boolean containsKey(Player p) {
            return info.containsKey(p.getUniqueId());
        }
      
        public boolean containsValue(T value) {
            return info.containsValue(value);
        }
      
        public boolean isEmpty() {
            return info.isEmpty();
        }
      
        public int size() {
            return info.size();
        }
      
        public void clear() {
            info.clear();
        }
      
        public Set<Player> keySet() {
            Set<Player> out = new HashSet<Player>();
            for(UUID uuid : info.keySet().toArray(new UUID[0]))
                out.add(Bukkit.getPlayer(uuid));
            return out;
        }
      
        public Collection<T> values() {
            return info.values();
        }
    }
    

    This class works just like a hashmap, so you can use it like you usually would.

    Example usage:
    Example (open)

    Code:
    PlayerList list = new PlayerList(Bukkit.getOnlinePlayers());
    for(Player p : list) {
      if(p.getHealthScale() < 3) {
        p.sendMessage("You're almost dead, better heal up!");
        p.getInventory().addItem(new ItemStack(Material.COOKIE, 64));
    }
    }
     
    Last edited: Feb 24, 2015
    ProStriker123 likes this.
  2. Offline

    ProStriker123

  3. Offline

    DJSkepter

    @megamichiel Awesome - this is pretty much a better version of my Player Hashmap. I suggest having it so that it removes the player from the map when the player leaves if you no longer want to reference that player from the map; hence saving possible memory leaks.
     
  4. Offline

    LordVakar

    Nice resource, but I find it unnecessary at some bits, the map is nice though.
    You can actually store players in arraylists without causing server leaks (although I don't know why you would, a hashset, or maybe a treemap would be better, depending on your situation). Server leaks happen when you don't know how to effectively handle the player object, and when to store the player object.
    A collection or hashset with UUIDs would work as an alternative.
     
    gal0511Dev likes this.
  5. Offline

    teej107

    Where are you storing the players? I don't see any Players being stored but rather their UUID! This is just a simple wrapper class that you could alternatively achieve by overriding the add or put method.
    Something like this:
    Code:
    if(list.contains(p.getUniqueId())) list.remove(p.getUniqueId());
    is unnecessary. No errors will be thrown if the Object is not in the Collection/Map. The remove method will just return a different value depending on if the Object was removed.
    Also things like this are terrible:
    Code:
    info.keySet().toArray(new UUID[0]))
    There is no need to get an Array of the keySet. You can still use a for-loop over anything that implements an Iterable. Also specifying an Array with a length of 0 is worse. What's so hard about getting the size of a Map or Collection?

    The fact that you are copying methods from Map/Collection but aren't even extending it as one is also a bad thing. Polymorphism is a great thing!

    If you are using a UUID to represent a Player, then you are still somewhat doing what you are trying to prevent. It won't nearly be as big a problem that a Player causes that's for sure, but you aren't going to use that UUID if that Player leaves, right? Storing Players in Collections or Maps isn't a bad thing. What matters is how you handle it when the player leaves. Using a Weak References is an easy way to handle when a Player leaves eg: WeakHashMap, "WeakSet".
     
    werter318 likes this.
  6. Offline

    CaptainBern Retired Staff

    Could also just use a map or list that makes use of weak-references.
     
  7. Offline

    megamichiel

    @teej107
    I used the toArray(new UUID[0]) because it can sometimes throw a concurrentmodificationexception on a for statement, which doesn't happen in an array. I didn't override a class because if I did, I couldn't achieve some stuff like add(Player) cause then it would still be stored as a player. The containskey check on remove is because a while ago it gave me an exception on that, but I didn't know that was gone. I also haven't known about Weak references before, but I'll have a look at it.
     
  8. Offline

    teej107

    @megamichiel You can still avoid a ConcurrentModificationException without using an Array. I never said to override a class, I said to override methods. You should at least have the class implement a Map/Collection. For ArrayLists and HashMaps, removing without checking to see if the object is inside should never throw an exception. If it did, it was an error in your part.
     
  9. Offline

    megamichiel

    @teej107
    Ah, I see what you mean. But about the removing, I think it was when I added an object to a list with the object already in it when I got exception.
     
  10. Offline

    teej107

    @megamichiel Still don't know what you are trying to do because adding an Object to an ArrayList should never throw an exception.
     
Thread Status:
Not open for further replies.

Share This Page