Easy way to assign variables to players.

Discussion in 'Plugin Development' started by scavenger, Feb 28, 2012.

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

    scavenger

    So i want to set a bunch of variables to a player, but instead of doing a heap of e.g. "Hashtable<Player, Integer>" i do this:
    Code:
    public final ArrayList<PPlayer> PPlayers = new ArrayList<PPlayer>();
    and then on join and on leave:
    Code:
        @EventHandler
        public void onJoin(PlayerJoinEvent e) {
            synchronized (plugin.PPlayers) {
                plugin.PPlayers.add(new PPlayer(e.getPlayer()));
            }
        }
       
        @EventHandler
        public void onQuit(PlayerQuitEvent e) {
            int i = 0;
            synchronized (plugin.PPlayers) {
                for (PPlayer play : plugin.PPlayers) {
                    if (e.getPlayer().getName().equalsIgnoreCase(play.player.getName())){
                        plugin.PPlayers.remove(i);
                        break;
                    }
                    i++;
                }
            }
        }
    then in the PPlayer class i can set my variables:
    Code:
    public class PPlayer {
       
        public Player player;
        public int Block;
        public boolean canuse;
        //etc...
       
        public PPlayer (Player player) {
            this.player = player;
            this.Block = 0;
            this.canuse = false;
        }
    }
    And i can use this to get/set the variables:
    Code:
        public PPlayer GetPPlayerbyName(String name) {
            for (PPlayer ply : plugin.PPlayers) {
                if (ply.player.getName().equalsIgnoreCase(name))
                    return ply;
            }
            return null;
        }
    Is there an easyer way?
     
  2. Offline

    Atomic Fusion

    Why not a Hashmap<Player,PPlayer>? If player doesn't work, you could always use a Hashmap<String,PPlayer>, but it would be uglyish.
    Code:
    public final Hashmap<Player,PPlayer> PPlayers = new Hashmap<Player,PPlayer>();
    Code:
        @EventHandler
        public void onJoin(PlayerJoinEvent e) {
            synchronized (plugin.PPlayers) {
                plugin.PPlayers.put(e.getPlayer(),new PPlayer(e.getPlayer()));
            }
        }
       
        @EventHandler
        public void onQuit(PlayerQuitEvent e) {
            synchronized (plugin.PPlayers) {
            plugin.PPlayers.remove(e.getPlayer());
            }
        }
    Shouldn't even need this bit, unless you want a shortcut method:
    Code:
        public PPlayer GetPPlayerbyName(String name) {
            return plugin.PPlayers.get(plugin.getServer().getPlayer(name));
        }
    You might want a more descriptive title in the future.

    Actually it's HashMap, not Hashmap, sorry. Or you could use Hashtable and remove the synchronisation blocks.

    EDIT by Moderator: merged posts, please use the edit button instead of double posting.
     
    Last edited by a moderator: May 24, 2016
  3. Offline

    scavenger

    I guess, but how do other people do it (dont just improve my method)

    Edit: Also, i changed the title xD.
     
  4. Offline

    desht

    A Map (HashMap) is the right way to do this, like Atomic Fusion says. It's much more efficient than using a List.

    Otherwise your general approach looks alright.

    Have a read of http://www.javapractices.com/topic/TopicAction.do?Id=65 for a general overview of when to use which Collection type.

    As for the map key type, I'd strongly recommend using a String (the player name - player.getName() ) rather than a Player object as the key. Player objects are much more heavyweight than Strings.
     
  5. Offline

    Atomic Fusion

    With a Player though, isn't it just a reference to an existing instance, whereas with a String, isn't a new instance created for every get/set? Or does Java handle Strings differently from other Objects? Or am I just full of crap?
     
  6. Offline

    desht

    player.getName() just returns the existing entity's name - it doesn't create any new String object. Which is perfectly safe, since String is immutable. That's another good reason not to use Player objects (which are mutable) as map keys - it's just generally better practice to use immutable objects as keys where possible.
     
  7. Offline

    JayEffKay

    desht
    Is there a reason you're not using player.getEntityID(), thus having int instead of string as a key?
     
  8. Offline

    desht

    You could do that as long as you're certain you'll never want to persist your map or track players who log out and in again. I tend to use the name because I know that (by definition) a name always uniquely refers to the same player, while an entity ID can change.

    This is a case of a trade-off between performance and safety/maintainability:
    • Using the entity ID is faster than the name, but it may make your code more fragile. In the example in this post, it's probably OK to use the ID, since the map only tracks players while they're online, and it doesn't need to be persisted.
    • Using the name is slower, but more robust. In many cases, the performance lost will be negligible - if you're only accessing the map occasionally, it's no big deal. But if you needed to access the map hundreds or thousands of times a second, a String might not be your best choice.
     
    JayEffKay likes this.
Thread Status:
Not open for further replies.

Share This Page