.

.

 

I would change it so that when a player uses /lms join it adds them to an ArrayList then sends them a message saying "You will be teleported shortly" or whatever.

First you would declare an ArrayList outside the methods so that would look like:

Code:java

private ArrayList<Player> players = new ArrayList<Player>;

Then whenever a player uses /lms join add them to the list:

Code:java

players.add(sender);

Finally whenever the counter reaches 0 and you are ready to teleport the players instead of using a for each loop for the online players, use it for the players you have added onto the list:

Code:java

for (Player player : players)
{
    player.teleport(lms);
}

 

I was going to say use an ArrayList, but didn't want to comment just incase it was wrong.

 

elementalgodz11

Firstly, don't use Players in an ArrayList. Use their names instead. To answer your question, check if they are in the list first by using players.contains(player).

 

If you're going to call players.contains() you *might* want to use a TreeSet instead (e.g. Set<String> players = new TreeSet<String>()).

Just an efficiency thing, though, shouldn't matter for a list of a handful of players, but something to keep in mind- you don't want to be calling ArrayList.contains() on a big list.

 

elementalgodz11 might as well get my MiniEvents plugin
Does this exact thing

 

If you're going for efficiency and speed, surely you mean a HashSet?

 

Why do you iterate over all the players when sending the message?

Don't add the player to 'players' until you've confirmed the player isn't already in the List (do it in the else statement).

EDIT by Moderator: merged posts, please use the edit button instead of double posting.

 

elementalgodz11

Code:java

// In case you use a Set<String> with playernames
Set<String> players = new HashSet<String>();
 
// Then this is the onCommand part
if (args[0].equalsIgnoreCase("join"))
{
Player player = (Player)sender;
if (players.contains(player.getName()))
{
player.sendMessage("You are in the event already");
}
else
{
players.add(player.getName())
p.sendMessage(ChatColor.RED + "You are now in the " + ChatColor.WHITE + "LMS" + ChatColor.RED + " event");
}
}
 
// Or if you really, really need to use Player objects
Map<Player, Boolean> players = new WeakHashMap<Player, Boolean>();
 
// Then this is the onCommand part
if (args[0].equalsIgnoreCase("join"))
{
Player player = (Player)sender;
if (players.contains(player))
{
player.sendMessage("You are in the event already");
}
else
{
players.put(player.getName(), Boolean.TRUE);
p.sendMessage(ChatColor.RED + "You are now in the " + ChatColor.WHITE + "LMS" + ChatColor.RED + " event");
}
}

If you use a Set<String>, for more efficiency on acquiring the Player objects.
(Note: do not store the list, then it -should- be ready for GC after the method is done)

Code:java

List<Player> temp = new ArrayList<Player>();
for(Player player : Bukkit.getOnlinePlayers())
{
if(players.contains(player.getName()))
{
temp.add(player);
}
}
 
for(Player player : temp)
{
// Do whatever
}

(On a second note, I use a WeakHashMap so it uses WeakReferences, which means it -shouldn't- stop the GC from freeing the memory. Any people who wonder about not using a Set:
1. There is no (as far as I am aware) WeakSet, feel free to make a HashSet<WeakReference<Player>> though.
2. Regarding the first point - and the main note in general - take a peek in the HashSet source, then you will realize that Sets are basically wrappers using HashMaps :3.)

 

If the 'running' boolean indicates whether there is an event started, use 'if(running) {' to check before you add the player or anything like that.

 

Instead of using arraylists, how about using configs. When I make a minigame, I always make an Arena and ArenaManager class, best way of making a minigame,.

Check if the lms is already started, and if it is, message to player saying "lms is already started"

EDIT by Moderator: merged posts, please use the edit button instead of double posting.

 

Good call, yes! Sorry, I normally care about order- but if you don't, absolutely right a HashSet is much more efficient.

But both are still much better options than an ArrayList Those are not for searching.

 

Where do you remove 1 from the countdown?

 

(plus that you won't have duplicates)

 

More similar to Lists, they both implement Collection- if you're not looking things up by an index, then it should be a drop in replacement (just switch ArrayList to HashSet and see if anything turns red). They've got add, remove, contains, an iterator, etc. Oh, one other difference is they won't store duplicates (which you don't want in this case, but worth mentioning). It is a "set" in the mathematical sense- if you add the same String to it twice, it will still only appear in the set once.

Anyway, maybe not worth worrying about for a list of just a few players, but if you want to experiment with it now, it's a good tool to have!

I recently switched a bunch of ArrayLists in my code to TreeSets (similar thing, but ordered and not quite as efficient) and I didn't have to change anything but the variable types.

EDIT: I looked at your code, and I think you should be fine! If you check the docs, you'll see it's got removeAll and everything else you're using:

http://docs.oracle.com/javase/6/docs/api/java/util/HashSet.html

I mean if you do:

Set<String> mySet = new HashSet<String>();
mySet.add("NathanWolf");
mySet.add("NathanWolf");
System.out.println(mySet.size());

The size will be 1. You can't have the same thing (by equality test) in a Set more than once.

Fortunately, this is probably what you want for a list of players- but in some cases might cause confusion.

EDIT by Moderator: merged posts, please use the edit button instead of double posting.

 

elementalgodz11 Don't use an

Code:

ArrayList<Player>

If you use Lists or Sets, use Strings for the playernames.

elementalgodz11

Thus,

Code:

Set<String>
or
List<String>

Storing Players in regular Sets and Lists creates memory leaks when players disconnect.

elementalgodz11 Bukkit.getPlayer(name)

EDIT by Moderator: merged posts, please use the edit button instead of double posting.