Resource PvPLogger (CombatLog) Resource.

Discussion in 'Spigot Plugin Development' started by PvPNiK, Mar 14, 2018.

  1. first of all just want to say sorry if there is any mistakes with grammar, English is not my native language.
    and second of all just wanted to share a little code of mine, nothing special and if i have not posted the resource well enough so sorry i'm will learn for the next time.

    oh and i will glad to have some criticism about the code, where i can improve it or change it, thanks.
    (the code was written a year ago on spigot 1.8.9)

    First Class:
    Code (Text):

    public class Logger {
        private static HashMap<UUID, TimerLogger> Timer = new HashMap<UUID, TimerLogger>();
        public static void add(UUID uuid) {
            if (hasTimer(uuid))
            Timer.put(uuid, new TimerLogger(uuid));
        public static boolean hasTimer(UUID uuid) {
            return Timer.containsKey(uuid);

        public static boolean remove(UUID uuid) {
            if (hasTimer(uuid)) {
                return true;
            return false;

    first of all there is not a lot to explain here,
    we have a list called "Timer" that saves all the players that in combat log status, and their timer.
    if we want to add a player to the "combat log" status we will just write: "Logger.add(uuid)".

    now for the real part, the timer:

    Second Class:
    Code (Text):
    public class TimerLogger extends BukkitRunnable {
        private int time;
        private UUID uuid;
        private Player player;
        private Plugin plugin;
        public TimerLogger(UUID uuid) {
            this.time = 7;
            this.uuid = uuid;
            player = Bukkit.getPlayer(uuid);
            this.plugin = Main.getInstance();
            this.runTaskTimer(plugin, 0, 20L);
        public void run() {
            if (!player.isOnline()) {
                if (Logger.hasTimer(uuid))
            if (time <= 0) {
                Message.sendMessage(Bukkit.getPlayer(uuid), "§7[§6PVP Logger§7] §cOFF");
                if (Logger.hasTimer(uuid))

    so i will explain a little bit of the code:
    time, is the combat log status length, in this example the combat log will stay for 7 seconds.
    plugin, it is the "JavaPlugin" class, mine called Main and it has a singelton function thats why i wrote "Main.getInstance();"
    those were the parts that u will need change and so on..
    after we got all the variables in the place let's move on to the timer.

    first of all we are starting the timer at the end of the constructor, so it will be executed every second.
    and what code will be executed? the code that written in run() method.

    first of all in this method i'm checking if the player is still online, if not just stop all, why to waist memory?
    i wrote this part for backup, for example if i forgot to remove the player when he leaving the server so the code will not continue..
    but you can change it to whatever you want, if you have a minigame and when player dies you do not want the code to continue, so player#isOnline() will not help you here. you got my point right?

    after we checking if the player is online we are moving to our second if, the end part.
    if the timer "run out of time" that means that means the combat log is finished and player is safe to go.
    so when time <= 0, i'm sending a message to the player, removes him from the list and canceling the timer so the timer will stop.
    and pretty much thats it.. easy right?

    after all of the if and stuff there is "time--;" the code will get to there if:
    a. player is online
    b. there is still time.
    so remove 1 seconds, and loop all it again.

    hope i explained myself good and you guys understood how all works here.


    very very easy,

    every time player gets a damage, use the add method, it will stop the last timer and start a new one.
    so if the player had already a timer and there was 2 seconds left, it will start again from 7.

    if the player quits the server, check if he is combat log using:
    Logger#hasTimer(uuid), if it returns true so the player in combat log status, now do with him whatever you want, for example kill him?

    to remove player's combat log status just use Logger#remove(uuid) and that's it..


    Logger#add(uuid), set player's combat log status on.
    Logger#hasTimer(uuid), returns true if a player is in combat log status.
    Logger#remove(uuid), removes player combat log status.


    Code (Text):
    public class LoggerListener implements Listener {
        public void onPlayerQuit(PlayerQuitEvent e) {
            UUID uuid = e.getPlayer().getUniqueId();
            if (!Logger.hasTimer(uuid))
        public void onPlayerDeath(PlayerDeathEvent e) {
            UUID uuid = e.getEntity().getUniqueId();
            if (Logger.hasTimer(uuid))
        public void onPlayerDamage(EntityDamageEvent e) {
            if (!(e.getEntity() instanceof Player))
    • Like Like x 1
    • Optimistic Optimistic x 1
  2. Why do people come up with such stupid things. You think that everythjing you make is a resource? What is the difference between your useless ‘resource’ and people making their OWN
    • Like Like x 1
  3. just to confess here for real i just was a little bit bored and i saw a post that someone need help with combat log, and a guy posted a code that was messy so i wanted to post my code, and to hear some criticism and if there are any way the code/i can improve.
  4. Make it in one class
  5. Why ?
    Like that makes no difference what so ever

    At the moment you have it working of a timer this is useful if you want to send a countdown to the player or update them live on there tag time.
    How ever if you only need to send a message on them doing something Eg if your blocking cmds.
    It could be a good idea to use time stamps so you do not have a task running.
    Or add both systems.
  6. When using an api, it's a convenience factor to keep it in one file unless it would degrade it. In this case, these classes could have been inner classes.
    Why static? And why not abstract to the map interface? This is a very bad practice. What if multiple plugins use this? You are aren't using static in the way it's supposed to be used in situations like this.

    Why? This is an API! Use dependency injection not this.
    • Agree Agree x 5
    • map shouldnt be static
    • build against interfaces (Map instead of HashMap)
    • use dependency injection to access other class references
    • instead of using scheduler resources, compare a stored System.currentTimeMillis() and a current one, and check the time between.
    • why store the player object AND their uuid? Just store the player object. don't fall into the misconception that you shouldn't store solely player objects. you can store anything as long as you clean it up properly when you're finished with it.

    what..? no. this entirely protests against OOP
    • Agree Agree x 3
  7. Actually, I think this can be appropriate. This thing is not OOP either way and those static methods are ugly. So you could just move all those methods, make them private and the map can stay as private static. Rrrright???
  8. I wouldn't consider it a misconception, but more of a safety guideline. It is recommended to not store direct player objects, in the event that the programmer does indeed forget to clean up after themselves. Now, this wouldn't normally be an issue, but since this is technically an API, storing UUIDs would be the more generically safe thing to do, so that cleanup isn't required on the users end.
  9. What makes you think that API can't do the cleanup and users have to do that by themselves? Users shouldn't even be able to do such a thing...
  10. Many issues with this.
    • The class names shouldn't be "Logger" because in programming that usually refers to the system that writes log messages during the runtime.
    • As people have said already, the use of static is not good at all in this situation. It does not allow people to create their own implementations of your system that are independent from eachother.
    • Since you are only executing meaningful code when the countdown timer hits 0, it would be better to just schedule 1 single task to be run in 7 seconds.
    • More, but other people have already stated all that I could see in the time I have to write this.

    It should also be noted that storing just the UUID as a Map key means that you don't need an actual Player object when querying the Map which can be very useful.
  11. Users in the sense of users of the API. Making the API to clean itself up (without using listeners and checks) is the simplest way of doing it, and saves lines of code, although it should be noted that cleaning up manually is always a good idea.
  12. The best way to do it for me is doing cleaning at the end of the method that does some job which needs cleaning. User of the API shouldn't have to worry about this. Saving lines of code is not better than simplicity.

Share This Page