Confusion with Bukkit Scheduler

Hey,

So, I'm making a simple plugin that prevents people from teleporting for 5 seconds after they have taken damage.

Code:

public void taskCombat() {
        plugin.getServer().getScheduler()
                .scheduleSyncDelayedTask(plugin, new Runnable() {
                    public void run() {
                        String player = plugin.playerName;
                        btcombat.inCombat.remove(player);
                    }
                }, 100L);
    }

I have an onEntityDamage listener that checks to see if the entity is a player, and if the cause of the damage was Entity_Damage, or Projectile. If it is, they are added to the HashMap inCombat.

Then it starts the task will remove the player from the HashMap. If the player tries to teleport while they are in the HashMap, then the onPlayerTeleport event is cancelled.

Im not sure if the correct players are being removed from the HashMap. I don't really know how it works, so I'm wondering if its just removing the latest player that took damage, or if its actually removing the players in the order I want.

Any help/advice is much appreciated!

 

That should give each player an equal delay. The scheduler will create a new instance each time you call that method, so no need to worry about overlapping.

 

@DirtyStarfish Don't forget you can also use any class implementing Runnable. Right now you do the same, but then in a more flexible way. I recommend adding a separate task for every player instead:

Code:

public class PlayerCoolDownTask implements Runnable {
    private final String playername;
    public PlayerCoolDownTask(final Player player) {
        this(player.getName());
    }
    public PlayerCoolDownTask(final String playername) {
        this.playername = playername;
    }
    public void run() {
        btcombat.inCombat.remove(this.playername);
    }
}

Then just call that with delay whenever you need it:

Code:

public static void coolDown(String playername, long delay) {
    schedule(new PlayerCoolDownTask(playername), delay);
}
public static void schedule(Runnable runnable, long delay) {
    plugin.getServer().getScheduler().scheduleSyncDelayedTask(plugin, runnable, delay);
}

Also, 'plugin.playerName' will change before the task even executed, causing some players to never get a cool down. (only if multiple players are causing it simultaneously)

 

Keep in mind that if a player is damaged during the 5 second "cooldown", your current code won't reset the timer. The timer will still expire after 5 seconds, which means that the player could teleport away then, even if they received additional damage just 1 second before.

 

I recommend just saving the currentTimeInMillis and comparing that.

 

Thanks for the responses! I decided to go ahead and try @bergerkiller 's way as it looks quite interesting. I'm getting a NullPointerExcepion though, and I'm not really sure what the cause is.

Player join event in PlayerListener

Code:

public void onPlayerJoin(PlayerJoinEvent event) {
        Player player = event.getPlayer();
        String playername = player.getName();
        btcombat.spawned.add(playername);
        PlayerCoolDownTask.coolDown(playername, 300L);
    }

PlayerCoolDownTask class

Code:

public class PlayerCoolDownTask implements Runnable {
    public static btcombat plugin;
    private final String playername;
    public PlayerCoolDownTask(final Player player) {
        this(player.getName());
    }
    public PlayerCoolDownTask(final String playername) {
        this.playername = playername;
    }
    @Override
    public void run() {
        btcombat.inCombat.remove(this.playername);
        plugin.log.info(playername + " removed from inCombat.");

    }
    public static void coolDown(String playername, long delay) {
        schedule(new PlayerCoolDownTask(playername), delay);
    }
    public static void schedule(Runnable runnable, long delay) {
        plugin.getServer().getScheduler().scheduleSyncDelayedTask(plugin, runnable, delay);
    }

}

This is the line with that has the exception (5th line up in the PlayerListener).

Code:

        schedule(new PlayerCoolDownTask(playername), delay);

Would be great if someone could point me in the right direction.

 

The only thing that could be null on the stated line is "playername", however that would mean that "player.getName()" returns null which is extremely unlikely unless you have been messing around with player creation elsewhere.

Are you sure that was the line it stated? The previous line is a risky one, as it would create a NullPointerException if "btcombat.spawned" had not been initialised. It is a very common mistake to forget to initialise List type objects, however I would need to see the btcombat class to be sure.

P.S. It is good practice to use camel-case, starting with a capital letter for class names in Java e.g. BTCombat, PlayerCoolDownTask. The btcombat class at first glance would appear to be an instance because you started with a lower-case letter and this will cause confusion when showing your code to others, especially when invoking static methods of the class.

Also, Pandemoneus is right, recording the time of last hit for each player and checking it on teleporting would be a lot more efficient than scheduling a new task every time a player is hit.

 

All seems a little complex. Why not just do this:

• Maintain a map of player name -> last time damaged in your plugin (i.e. Map<String,Long> lastDamaged)
• In onEntityDamage(), when a player takes damage, update the last time damaged for the player in your map: lastDamaged.put(playerName, System.currentTimeMillis())
• In onPlayerTeleport(), check if System.currentTimeMillis - lastDamaged.get(playerName) > cooldownTime and cancel the teleport event if the last damage for the player was too recent.

Update: just noticed @Pandemoneus post - consider this as support for that