¿ This code is efficient ?

Discussion in 'Plugin Development' started by MaTaMoR_, Feb 27, 2015.

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

    MaTaMoR_

    Hi, im trying to check if a player is inside an area with this :

    Border class :
    Code:java
    1.  
    2. public class Border {
    3.  
    4. private Vector p1;
    5. private Vector p2;
    6.  
    7. public Border(Vector p1, Vector p2) {
    8. int x1 = Math.min(p1.getBlockX(), p2.getBlockX());
    9. int y1 = Math.min(p1.getBlockY(), p2.getBlockY());
    10. int z1 = Math.min(p1.getBlockZ(), p2.getBlockZ());
    11. int x2 = Math.max(p1.getBlockX(), p2.getBlockX());
    12. int y2 = Math.max(p1.getBlockY(), p2.getBlockY());
    13. int z2 = Math.max(p1.getBlockZ(), p2.getBlockZ());
    14. this.p1 = new Vector(x1, y1, z1);
    15. this.p2 = new Vector(x2, y2, z2);
    16. }
    17.  
    18. public boolean contains(Location loc) {
    19. if(loc == null) {
    20. return false;
    21. }
    22. return loc.getBlockX() >= p1.getBlockX() && loc.getBlockX() <= p2.getBlockX()
    23. && loc.getBlockY() >= p1.getBlockY() && loc.getBlockY() <= p2.getBlockY()
    24. && loc.getBlockZ() >= p1.getBlockZ() && loc.getBlockZ() <= p2.getBlockZ();
    25. }
    26.  
    27. }
    28.  

    Event class :
    Code:java
    1.  
    2. @EventHandler
    3. public void onMove(PlayerMoveEvent event) {
    4. if(event.getFrom().getX() == event.getTo().getX() && event.getFrom().getBlockY() == event.getTo().getBlockY() && event.getFrom().getBlockZ() == event.getTo().getZ()) {
    5. return;
    6. }
    7. if(Koth.koths.isEmpty()) { return; }
    8.  
    9. Player player = event.getPlayer();
    10. Boolean IsIn = false;
    11. Koth koth = null;
    12. for(Koth k : Koth.koths) {
    13. for(UUID uuid : k.getPlayers()) {
    14. if(uuid == player.getUniqueId()) {
    15. IsIn = true;
    16. koth = k;
    17. break;
    18. }
    19. }
    20. }
    21.  
    22. if(IsIn && koth != null) {
    23. Location l1 = koth.getFirstLocation();
    24. Location l2 = koth.getSecondLocation();
    25.  
    26. Border border = new Border(new Vector(l1.getX(), l1.getY(), l1.getZ()), new Vector(l2.getX(), l2.getY(), l2.getZ()));
    27. if(!border.contains(event.getTo())) {
    28. KothsManager.getManager().removePlayer(player, koth);
    29. s(player, KothMain.prefix + "&7Leaving the koth &b"+koth.getName());
    30. }
    31. } else {
    32. for(Koth k : Koth.koths) {
    33. Location l1 = k.getFirstLocation();
    34. Location l2 = k.getSecondLocation();
    35. Border border = new Border(new Vector(l1.getX(), l1.getY(), l1.getZ()), new Vector(l2.getX(), l2.getY(), l2.getZ()));
    36. if(border.contains(event.getTo())) {
    37. KothsManager.getManager().addPlayers(player, k);
    38. s(player, KothMain.prefix + "&7Entering koth &b"+k.getName());
    39. break;
    40. }
    41. }
    42. }
    43. }
    44.  

    I wonder if there's any other way to do this more efficient.
     
    Last edited: Feb 27, 2015
  2. Offline

    mine-care

    Hmm generally speaking, move event is not the most efficient thing :3 but in your case it is the most decent way to deal with regions.
    What is Koth#arenaObjects ?
     
  3. Offline

    MaTaMoR_

    That returns all Koths == Arenas, ¿ i should use a runnable instead of PlayerMoveEvent ?
     
  4. Offline

    mine-care

    @MaTaMoR_ i dont think so, runnable runs regardless to if player is moving. the thing that matters now is the case. in a server with 50 players afk (or moving just by yaw and pitch) then the runnable will be a pain with no reason, whereas in a server with 50 players running arround it might be better than an event called about a milion tims :/ idk what to say. Smarter guys like @teej107 might have a better solution :3
    Okay, genera;;y im suspicius of static xD i have seen it miss used a bilion times with people puting static in arena classes and woops all objects (arenas) have the same names and characteristics -_- xD
    good.
     
  5. Offline

    rickydaan

    @MaTaMoR_, @mine-care PlayerMoveEvent gets runned alot. You are probably better off making your own class that implements Runnable. That has a "run" section. Every so-many ticks you call that event. I'd call it every second to two seconds.

    !!! UNTESTED CODE !!!
    Code:
    public class CheckPlayers implements Runnable{
       @Override
       public void run( ){
          Iterator it = Bukkit.getServer().getOnlinePlayers().Iterator();
          while( it.hasNext( ) ){
             Player player = it.next();
             // Run your code to check if player is in border here
             // Make sure to use "player" and not it.next(), as that would break the loop and may cause NPE and could skip players
          }
       }
    }
    
    Then in your main class (DEPRECATED CODE (Works though))
    Code:
        @Override
        public void onEnable(){
           long checkPlayersTickDelay = 20L;
           getServer().getScheduler().runTaskTimer(this, new CheckPlayers( ), 0L, checkPlayerTickDelay);
           // Your normal onEnable code of course
        }
    
     
  6. Offline

    mine-care

    @rickydaan ^
     
    tomudding likes this.
  7. Offline

    rickydaan

    @mine-care You can add in a hashmap with Player and Location, and check if the positions are the same. Uses more RAM and saves CPU.
     
  8. Offline

    mine-care

    @rickydaan Player? then it is the ultimate ram leak.... especially when not handled properly :/ And what is the reason for this if he adds the proper checks to "filter" the move event as he already did
    Then there is no reason to mess with runnables.
     
  9. Offline

    Konato_K

    When I made a KingOfTheHill-like game I used a runnable every 2 seconds, I just checked the players in game and if they were in or not, I found it better since it was for a server with lots of people, and lost of people moving it's more overkill than a simple runnable in this case. I'd go with runnable for this.
     
  10. Offline

    teej107

    @MaTaMoR_
    1. Your Border class is not needed. Everything you need can be found in the Vector class. http://docs.codelanx.com/Bukkit/1.7...rg.bukkit.util.Vector-org.bukkit.util.Vector-
    2. PlayerMoveEvent does get called a lot. Sometimes it is better to schedule a task because the users won't see a difference, but it will be easier on the server. However, if you value precision, use the PlayerMoveEvent. Just don't do any heavy tasks in the PlayerMoveEvent.
    If it was up to me, I would probably go with using a scheduler since it seems you are looping through quite a bit of things.
     
  11. Offline

    Tecno_Wizard

    Hmm. Now if you are okay with using round regions, this can be done a lot easier using the distance from the center squared.
     
  12. Offline

    Konato_K

Thread Status:
Not open for further replies.

Share This Page