1.16.5 How to optimize Land Protection plguin?

Discussion in 'Spigot Plugin Development' started by MrHardyCZ, Sep 25, 2020.

  1. Good day, everyone!

    I'm updating my old and poorly written land protection plugin called ForceFields.
    I want to make the plugin as optimized as possible, because now it is just a mess.

    Each time every player does something, the plugin iterates between all ForceFields (the claimed areas) and checks whether the player is inside or not.

    I'm including small snipped of code to demonstrate how the plugin basically works.
    Code (Java):

    public class Plugin implements Listener {

        private ArrayList<ForceField> forceFields;

        public Plugin() {
            forceFields = new ArrayList<>();
            getServer().getPluginManager().registerEvents(this, ...);
        }

        @EventHandler
        public void OnBlockBreak(BlockBreakEvent event) {
            for(ForceField forceField : forceFields) {
                if(forceField.isPlayerInside(event.getPlayer())) {
                    if(!forceField.isPlayerOwner(event.getPlayer())) {
                        event.setCancelled(true);
                        event.getPlayer().sendMessage("Not your ForceField!");
                    }
                }
            }
        }

        @EventHandler
        public void OnBlockPlace(BlockPlaceEvent event) {
            for(ForceField forceField : forceFields) {
                if(forceField.isPlayerInside(event.getPlayer())) {
                    if(!forceField.isPlayerOwner(event.getPlayer())) {
                        event.setCancelled(true);
                        event.getPlayer().sendMessage("Not your ForceField!");
                    }
                }
            }
        }
    }
     

    This seems to me like a completely invalid solution, for the performance suffers more and more with increasing number of Force Fields.
    I would really like to know how I can optimize such a plugin.
     
  2. Store the Force Field(s) that the player is currently inside of, and does not own, on the player somehow. Then on the events, check if the player is still in those specific force field(s) and if any of those fields block whatever action the player is performing. If the player is still in at least one of the fields that blocks the action, simply cancel the event. If the player is not in any force fields that they don't own, then iterate over all force fields that you haven't already checked, store all force fields on the player, and cancel the event accordingly.

    There are probably better solutions, that's just one I thought of quickly. Basically it minimizes the need to search over every field by assuming the player will perform many actions close together, so having the fields that they are currently in persist makes it much faster to check actions performed close together.

    Something else you could do, have a way to get the force fields in a radius (for bonus performance points(tm) use distanceSquared rather than distance, though unless you have an absurd number of force fields it wont really make much of a difference). Instead of checking whether the player is inside all force fields, get all force fields within whatever biggest possible force field radius is. then check if the player is inside those fields (and persist them on the player like described above). Depending on how you currently implement checking if a player is in a field, this could improve performance substantially.
     
  3. For starters, break out of the loop if the field the player is in is found (unless the fields can overlap).
    Then, you should consider grouping, for example by chunks, so Map<Chunk, Field> or if multiple fields can be in a single chunk Multimap<Chunk, Field>, then you'd only have get the player's chunk and call Map#get, or Multimap#get and iterate though the chunks.
    Also, using Stream#anyMatch is a good idea in such a case.
     
  4. You would have to iterate over the chunks bordering the player as well, since if the force field extends into an adjacent chunk it would not be detected. This would still be much faster than getting the distances of all fields, though.
     
  5. I should have mentioned it in the first post.
    If a field is spread over multiple chunks you'd have an Entry in the Map (or Multimap) for each Chunk it is in.
    So if a field contains two chunks the map will look something like that: [chunk1=field1, chunk2=field1].
    So you wouldn't have to get nearby chunks.