Repeating task

Discussion in 'Spigot Plugin Development' started by Eliminator, May 20, 2015.

  1. I want to have a repeating task in my plugin that runs once a second.

    But the one I have currently is causing lagg:

    Code (Text):
            @SuppressWarnings("unchecked")
            private void timer(){
                try{
                    //Stuff
                }catch(Exception x){x.printStackTrace();}
                Bukkit.getServer().getScheduler().scheduleSyncDelayedTask(this, new Runnable() {
                     public void run() {
                         timer();
                     }
                }, (20 ));
            }
    Is there a better way to do this?
     
  2. runTaskTimer maybe?
     
    • Agree Agree x 1
  3. You want to use runTaskTimer for a repeating task.
     
    • Agree Agree x 1
  4. How does a runTaskTimer Work? Where do I specify the code for it to run?
     
  5. * don't catch Exception in your catch block, only catch specific exceptions like IOException
    * runTaskTimer is preferred over scheduleSyncRepeatingTask as it returns a BukkitRunnable rather than a task id. @NinjaWaffles @nikmanG however, both work the same, causing the same lag
    * to reduce lag, either run the task in greater intervals, (if possible) run it async or decrease the amount of stuff you do in the task
     
  6. The problem with this is that he isn't experiencing lag because he's using a repeating timer, it's because he's creating a dozen, if not more, delayed timers. If you look back at his original post, you'll see what I mean. Lastly, I see no reason for him to run an asynchronous timer, as long as he isn't performing tasks like MySQL queries, which are known to lock the main thread until it completes its task. If he's just seeing how many players are online at any given time, or something menial like that, running a repeating task (on the main thread) will work just fine.
     
  7. No, if you look at his code again, you'll see that he calls timer() in the runnable, but doesn't create a *repeating* task (#scheduleSyncRepeatingTask), but instead only delays the call (#scheduleSyncDelayedTask). As long as he don't call timer() multiple times, there will only be one running task. You don't know what he does, when he writes "//Stuff", it might be MySQL or other IO operations, calculations etc., if it can be done async, it should be done async.
     
  8. In my "stuff" I loop through all the entities near each player and make them display particles and a few other things.

    Would making it async make the plugin cause less lagg?
     
  9. * Sounds like you would call bukkit methods, which should NEVER done async
    * please post your updated code with the //stuff; the only possibility to optimize your code seems to be there
     
    • Agree Agree x 1
  10. Here is what is in there:

    Code (Text):
                try{
                    //GUI Bars And Stuff
                    scoreCheck();
                    //Mob Stuff
                    ArrayList<Mob> tmp = (ArrayList<Mob>) infernalList.clone();
                    for(Mob m : tmp){
                        final Entity mob = m.entity;
                        UUID id = mob.getUniqueId();
                        Player p = null;
                        boolean foundMob = false;
                        for(Entity entity : mob.getNearbyEntities(64, 64, 64))
                            if(entity instanceof Player)
                                p = (Player) entity;
                        for(Entity entity : p.getNearbyEntities(64, 64, 64))
                            if(entity.getUniqueId() == id)
                                foundMob = true;
                        int index = idSearch(id);
                        if ((foundMob == true) && (!mob.isDead()) && (mob.getLocation() != null) && (index != -1)){
                            Location feet = mob.getLocation();
                            Location head = mob.getLocation();
                            head.setY(head.getY() +1);
                            if ((getConfig().getBoolean("enableParticles") == true) && (p != null)){
                                displayEffect(feet, m.effect);
                                //mob.getWorld().playEffect(feet, Effect.ENDER_SIGNAL, 1);
                                if (isSmall(mob) == false){
                                    displayEffect(head, m.effect);
                                    //mob.getWorld().playEffect(head, Effect.ENDER_SIGNAL, 1);
                                }
                                if ((mob.getType().equals(EntityType.ENDERMAN)) || (mob.getType().equals(EntityType.IRON_GOLEM))){
                                    head.setY(head.getY() +1);
                                    displayEffect(head, m.effect);
                                    //mob.getWorld().playEffect(head, Effect.ENDER_SIGNAL, 1);
                                }
                            }
                            //Ability's
                            ArrayList<String> abilityList = findMobAbilities(id);
                        }else if (foundMob == false){
                            try{
                                if(mob.isDead()){
                                    try {
                                        removeMob(index);
                                    } catch (IOException e) {}
                                }
                                infernalList.remove(index);
                            }catch(Exception x){}
                        }
                    }
                }catch(Exception x){x.printStackTrace();}
     
  11. * remove all these try-catch-blocks
    * your code does currently not work and will throw NullPointerException
    * use brackets { } for your loops and if, otherwise your code won't work
    * decrease the values for Entity#getNearbyEntities. 64 x 64 x 64 will collect maaany entities
     
  12. Whats wrong with try catches?

    Loops work fine without brackets...
     
    #12 Eliminator, May 20, 2015
    Last edited: May 20, 2015
  13. So my first question is based on this code segment:
    Code (Text):
     for(Entity entity : mob.getNearbyEntities(64, 64, 64))
                    if(entity instanceof Player)
                                p = (Player) entity;
    for(Entity entity : p.getNearbyEntities(64, 64, 64))
                    if(entity.getUniqueId() == id)
                                foundMob = true;
    - I may be reading this wrong but it seems that you are searching all entities around a mob to see if a player exists and then (assuming you found a player) you search 64 blocks around that player to see if that exact mob, that you just searched around to find the player you are currently checking for, exists? If that is the case it seems pointless to do the second check and not just set
    Code (Text):
    foundMob = true
    when u found a player around the mob which would save you a crap ton of operations.

    Random thoughts and questions:
    - Why not just comment things when u are posting to a forum for help. Instead of having to take the time to break down your code and understand what you are doing u could comment on what your doing. Not only is this best practice in the real world but when ur plugins start getting larger it will be easier to keep organized.

    - Im not fully versed on the InfernalMobs plugin but if they happen to only have mobs existing in a particular radius around players is it even useful to check for a player and not just display the particles?

    - It seems you display the particles twice if the mob is a small mob or the tall mob, that seems like it isn't needed and can be fixed by doing:
    Code (Text):

    if ((getConfig().getBoolean("enableParticles") == true) && (p != null)){
        //mob.getWorld().playEffect(feet, Effect.ENDER_SIGNAL, 1);
        if (isSmall(mob) == false){
             displayEffect(head, m.effect);
             //mob.getWorld().playEffect(head, Effect.ENDER_SIGNAL, 1);
        }
        else if ((mob.getType().equals(EntityType.ENDERMAN)) ||  (mob.getType().equals(EntityType.IRON_GOLEM))){
             head.setY(head.getY() +1);
             displayEffect(head, m.effect);
             //mob.getWorld().playEffect(head, Effect.ENDER_SIGNAL, 1);
        }
        else{
             displayEffect(feet, m.effect);
        }
    }
     
    • Informative Informative x 1
  14. The mobs can be anywhere in the world, not just near players, but I guess I could do:
    Code (Text):
    (mob.getLocation().getChunk().isLoaded())
    Would that be faster than looping through nearby entities?

    Small mob is bat, chicken, ect.
    So, it auto plays the effect at the mobs feet, as all mobs are at least 1 block tall.
    If the mob is not small, like a zombie, it displays it on its head.
    If the mob is really tall, (3 blocks) then its displayed a 3rd time.
    Code (Text):

                                displayEffect(feet, m.effect);
                                if (isSmall(mob) == false){
                                    displayEffect(head, m.effect);
                                }
                                if ((mob.getType().equals(EntityType.ENDERMAN)) || (mob.getType().equals(EntityType.IRON_GOLEM))){
                                    head.setY(head.getY() +1);
                                    displayEffect(head, m.effect);
                                }
     
    I fail to see how it displays twice?

    Thanks for the other stuff.
     
    #14 Eliminator, May 21, 2015
    Last edited: May 21, 2015
  15. I feel like it would be faster, but even then u could look at comparing the locations of players to the locations of the mobs and see how far they are, or you just display the particles, or any other way to see if a player is close to one of those mobs. and your 100% sure that InfernalMobs will have mobs in unloaded chunks right, because that is not the case with vanilla mechanics?

    To the second portions lets look at your code that you show:
    Code (Text):

    displayEffect(feet, m.effect);  //this will display an effect - Display count up to 1

    //this is going to test to see if a mob is small and if so it will display particles  - Displaying particles count up to 2 (if the mob is small)
    if (isSmall(mob) == false){
       displayEffect(head, m.effect);
    }

    //this is going to test to see if a mob is an Enderman or Iron Golem and if so display particles - Displaying particles up to at least 2 if conditions are met.
     
    always go through code line by line and think about what happens at that line and you should be able to catch mistakes like that. Plus it would just be better to use if, else if, and else in that situation because that is exactly what your logic states. Read your own logic out and you will see what i mean.
     
  16. (isSmall(mob) == false) displays them if they are NOT small.

    So the only time is displays multiple is when I want them to, e.g. 2 block tall mobs.
     
  17. Ahh missed that when i was skimming. so u just want to do particles and head and feet of mobs that are not small. that works, but just so you know with the way it is set up you will have 3 particle displays on enderman and irongolems due to using 2 if statements instead of if and else if, but i get it now on the small mobs :D. You wouldn't even need the if statement for enderman and irongolems since i am assuming they meet the condition isSmall == false
     
  18. Yah, but they are 3 tall, so they need 3 particles.