Solved Question about my code

Discussion in 'Spigot Plugin Development' started by JoeyPlayzTV, Jan 9, 2020.

  1. I have this class, to check if are armorstands stacked inside another armorstand.
    A user of my Plugin, reported that the Plugin drastically reduce the TPS. My test results are negative no better and no lower TPS with and without my Plugin. Can someone take look at my code and check it, if the code can causes laggs?

    Code (Java):
    package de.joeyplayztv.armorstandcrashfix.tasks;

    import java.util.ArrayList;
    import java.util.List;

    import org.bukkit.Chunk;
    import org.bukkit.World;
    import org.bukkit.entity.ArmorStand;
    import org.bukkit.entity.Entity;
    import org.bukkit.scheduler.BukkitRunnable;

    import de.joeyplayztv.armorstandcrashfix.ArmorStandCrashFix;

    public class CheckingTask extends BukkitRunnable {
       
        private ArmorStandCrashFix plugin;

        public CheckingTask(ArmorStandCrashFix plugin) {
            this.plugin = plugin;
        }

        private ArmorStandCrashFix getPlugin() {
            return plugin;
        }

        @Override
        public void run() {
            for(World world : getPlugin().getServer().getWorlds()) {
                for(Chunk chunk : world.getLoadedChunks()) {
                    for(Entity entity : chunk.getEntities()) {
                        if(entity instanceof ArmorStand) {
                            List<Entity> nearbys = entity.getNearbyEntities(0.0, 0.0, 0.0);
                            if(nearbys.size() > 0) {
                                ArrayList<ArmorStand> tokill = new ArrayList<>();
                                for(Entity nearby : nearbys) {
                                    if(nearby instanceof ArmorStand) {
                                        ArmorStand found = (ArmorStand) nearby;
                                        if(found.isVisible()) {
                                            tokill.add(found);
                                        }
                                    }
                                }
                                if(tokill.size() >= 1) {
                                    for(ArmorStand stand : tokill) {
                                        stand.remove();
                                    }
                                }
                            }
                        }
                    }
                }
            }
        }
    }
     
    Thanks for help.
     
  2. Choco

    Moderator

    This isn't a cheap operation and I'd hope you're not running this every tick because yea, I'd bet it does lower TPS drastically. You're iterating over every world, every loaded chunk, every entity in said loaded chunks and getting nearby ones. On a large (or even average sized) server, you're looking at thousands of bounding box checks 20 times every second. Why are you doing this to begin with? What is the goal?
     
    • Like Like x 1
    • Agree Agree x 1
  3. MiniDigger

    Supporter

    4 nested for loops and you ask why performance is bad? ^^

    Instead, just use World#getEntitiesByClass instead, saves you 3 loops and is optimized internally.
     
    • Agree Agree x 3
  4. The code blocks stackig ArmorStands together to prenvent laggs.

    I will try to improve it. Has you a soloution to block stacking of ArmorStands that is performanter?
     
  5. MiniDigger

    Supporter

    I literally suggested a better method in the post you quoted.

    And well, even that is not really optimal, if you just want to stack, just listen to spawn events instead of doing costly scans?
     
  6. But i dont get the Entitys per World. I get them per Chunk? So getByClass dont work in my case :(
     
  7. The spawn event does not check if a armorstand get pushed by a piston in another :( I will block pushing ArmorStands into a other.
     
  8. My improved one:

    Code (Java):
    @Override
        public void run() {
            for (World world : getPlugin().getServer().getWorlds()) {
                for (ArmorStand stand : world.getEntitiesByClass(ArmorStand.class)) {
                    List<Entity> nearbys = stand.getNearbyEntities(0.0, 0.0, 0.0);
                    if (nearbys.size() > 0) {
                        ArrayList<ArmorStand> tokill = new ArrayList<>();
                        for (Entity nearby : nearbys) {
                            if (nearby instanceof ArmorStand) {
                                ArmorStand found = (ArmorStand) nearby;
                                if (found.isVisible()) {
                                    tokill.add(found);
                                }
                            }
                        }
                        if (tokill.size() >= 1) {
                            for (ArmorStand as : tokill) {
                                as.remove();
                            }
                        }
                    }
                }

            }
        }
     
  9. Choco

    Moderator

    but you're getting every entity in every loaded chunk for every world... so... you are getting all entities in the world.
     

  10. Yea thanks i have improved this. Have you an improvement for getNearByEntitys? Or a improvement for the complete way. Is it any other way, to block stacking armorstands?
     
  11. Its solved. BlockPistonRetracksEvent is the solution.