Solved Efficienty problems and structure generation

Hi!
In my plugin, i have a populator that spawns a house thats 8*6*9.
It looks like this:


It checks that the ground is not air, water, lava etc, and it checks that the sides is air, grass whatever. That way it doesn't spawn inside a mountain or something stupid like that, but rather in a spot that is flat and empty. Because of this the generation is very slow, and eats up alot of capacity on the server, and i wondered if anyone know a more effiecient way to do this?

Here's the code (its pretty messy, but it works):

Populator:

Code (open) Code (close)

Code:

public class populator extends BlockPopulator {

    @Override
    public void populate(World world, Random random, Chunk chunk) {
        int noAir = 0;
        int cx = chunk.getX() * 16;
        int cz = chunk.getZ() * 16;
        int cZOff = cz + random.nextInt(10);
        int cXOff = cx + random.nextInt(10);

        if (random.nextInt(9) < 3) {
            if (world.getBiome(cx, cz) == Biome.PLAINS || world.getBiome(cx, cz) == Biome.BEACH) {
                Bukkit.broadcastMessage("Checking for places to build in biome: " + world.getBiome(cx, cz));
                Location loc = new Location(world, cXOff, world.getHighestBlockYAt(cXOff, cZOff), cZOff);
                Location block = null;
              
                //check ground
                for (int x = 0; x < 8; x++){
                    int bx = loc.getBlock().getX() + x;
                    for (int z = 0; z < 9; z++){
                        int bz = loc.getBlock().getZ() + z;
                        int by = loc.getBlock().getY() - 1;
                        block = new Location(loc.getBlock().getWorld(), bx, by, bz);
                        if (block.getBlock().getType() == Material.AIR || block.getBlock().getType() == Material.STATIONARY_WATER || block.getBlock().getType() == Material.WATER || block.getBlock().getType() == Material.LAVA || block.getBlock().getType() == Material.STATIONARY_LAVA) {
                            noAir--;
                            return;
                        }
                        noAir++;
                    }
                }
              
                //check west side
                for (int x = 0; x < 8; x++){
                    int bx = loc.getBlock().getX() + x;
                    for (int y = 0; y < 6; y++){
                        int by = loc.getBlock().getY() + y;
                        int bz = loc.getBlock().getZ() - 1;
                        block = new Location(loc.getBlock().getWorld(), bx, by, bz);
                        if (block.getBlock().getType() != Material.AIR) {
                            noAir--;
                            return;
                        }
                        noAir++;
                    }
                }
              
                //check south side
                for (int z = 0; z < 9; z++){
                    int bz = loc.getBlock().getZ() + z;
                    for (int y = 0; y < 6; y++){
                        int by = loc.getBlock().getY() + y;
                        int bx = loc.getBlock().getX() - 1;
                        block = new Location(loc.getBlock().getWorld(), bx, by, bz);
                        if (block.getBlock().getType() != Material.AIR) {
                            noAir--;
                            return;
                        }
                        noAir++;
                    }
                }
              
                //check north side
                for (int x = 0; x < 8; x++){
                    int bx = loc.getBlock().getX() + x;
                    for (int y = 0; y < 6; y++){
                        int by = loc.getBlock().getY() + y;
                        int bz = loc.getBlock().getZ() + 9;
                        block = new Location(loc.getBlock().getWorld(), bx, by, bz);
                        if (block.getBlock().getType() != Material.AIR) {
                            noAir--;
                            return;
                        }
                        noAir++;
                    }
                }
              
                //check east side
                for (int z = 0; z < 9; z++){
                    int bz = loc.getBlock().getZ() + z;
                    for (int y = 0; y < 6; y++){
                        int by = loc.getBlock().getY() + y;
                        int bx = loc.getBlock().getX() + 8;
                        block = new Location(loc.getBlock().getWorld(), bx, by, bz);
                        if (block.getBlock().getType() != Material.AIR) {
                            noAir--;
                            return;
                        }
                        noAir++;
                    }
                }
              
                //Build houses
                if(noAir == 276) {
                    Bukkit.broadcastMessage(Integer.toString(noAir));
                    generateQuestHouse(loc);
                }
            }
          
        }
    }
}

GenerateHouse:

Code (open) Code (close)

Code:

    public void generateQuestHouse(Location loc) {
        int layloc;
        for (int y = 0; y < 5; y++){
            int by = loc.getBlock().getY() + y;
            layloc = 0;
            for (int x = 0; x < 8; x++){
                int bx = loc.getBlock().getX() + x;
                for (int z = 0; z < 9; z++){
                    int bz = loc.getBlock().getZ() + z;
                    Location block = new Location(loc.getBlock().getWorld(), bx, by, bz);
                    block.getBlock().setType(Material.matchMaterial(layers(y, layloc)));
                    if (block.getBlock().getType() == Material.REDSTONE_BLOCK) {
                        block.getBlock().setType(Material.AIR);
                        block = new Location(loc.getBlock().getWorld(), bx + 0.5, by, bz + 0.5);
                        customEntityZombie.Spawn(block);
                    }
                    layloc++;
                }
            }
        }   
    }

    public String layers(int y, int layloc){
        String m1 = "AIR";
        String m2 = "17"; //Spruce log
        String m3 = "5"; //Spruce wood plank
        String m5 = "126"; //Spruce wood slab
        String m6 = "REDSTONE_BLOCK";
        String m7 = "GLASS";

        String[] layer1 = {
                /*1*/m1, m1, m5, m5, m5, m5, m5, m1, m1,
                /*2*/m1, m2, m2, m3, m3, m3, m2, m2, m1,
                /*3*/m1, m2, m3, m3, m3, m3, m3, m2, m1,
                /*4*/m1, m2, m3, m3, m3, m3, m3, m2, m1,
                /*5*/m1, m2, m3, m3, m3, m3, m3, m2, m1,
                /*6*/m1, m2, m3, m3, m3, m3, m3, m2, m1,
                /*7*/m1, m2, m2, m2, m2, m2, m2, m2, m1,
                /*8*/m1, m1, m1, m1, m1, m1, m1, m1, m1
        };

        String[] layer2 = {
                /*1*/m1, m1, m1, m1, m1, m1, m1, m1, m1,
                /*2*/m1, m2, m2, m1, m1, m1, m2, m2, m1,
                /*3*/m1, m2, m2, m2, m1, m2, m2, m2, m1,
                /*4*/m1, m2, m1, m1, m1, m1, m1, m2, m1,
                /*5*/m1, m2, m1, m1, m1, m1, m1, m2, m1,
                /*6*/m1, m2, m1, m1, m6, m1, m1, m2, m1,
                /*7*/m1, m2, m2, m2, m2, m2, m2, m2, m1,
                /*8*/m1, m1, m1, m1, m1, m1, m1, m1, m1
        };

        String[] layer3 = {
                /*1*/m1, m1, m1, m1, m1, m1, m1, m1, m1,
                /*2*/m1, m2, m2, m1, m1, m1, m2, m2, m1,
                /*3*/m1, m2, m2, m2, m1, m2, m2, m2, m1,
                /*4*/m1, m2, m1, m1, m1, m1, m1, m2, m1,
                /*5*/m1, m7, m1, m1, m1, m1, m1, m7, m1,
                /*6*/m1, m2, m1, m1, m1, m1, m1, m2, m1,
                /*7*/m1, m2, m2, m2, m7, m2, m2, m2, m1,
                /*8*/m1, m1, m1, m1, m1, m1, m1, m1, m1
        };

        String[] layer4 = {
                /*1*/m1, m1, m1, m1, m1, m1, m1, m1, m1,
                /*2*/m1, m2, m2, m1, m1, m1, m2, m2, m1,
                /*3*/m1, m2, m2, m2, m2, m2, m2, m2, m1,
                /*4*/m1, m2, m1, m1, m1, m1, m1, m2, m1,
                /*5*/m1, m2, m1, m1, m1, m1, m1, m2, m1,
                /*6*/m1, m2, m1, m1, m1, m1, m1, m2, m1,
                /*7*/m1, m2, m2, m2, m2, m2, m2, m2, m1,
                /*8*/m1, m1, m1, m1, m1, m1, m1, m1, m1
        };

        String[] layer5 = {
                /*1*/m5, m5, m5, m5, m5, m5, m5, m5, m5,
                /*2*/m5, m3, m3, m3, m3, m3, m3, m3, m5,
                /*3*/m5, m3, m3, m3, m3, m3, m3, m3, m5,
                /*4*/m5, m3, m3, m3, m3, m3, m3, m3, m5,
                /*5*/m5, m3, m3, m3, m3, m3, m3, m3, m5,
                /*6*/m5, m3, m3, m3, m3, m3, m3, m3, m5,
                /*7*/m5, m3, m3, m3, m3, m3, m3, m3, m5,
                /*8*/m5, m5, m5, m5, m5, m5, m5, m5, m5
        };

        if (y == 0){return layer1[layloc];}
        if (y == 1){return layer2[layloc];}
        if (y == 2){return layer3[layloc];}
        if (y == 3){return layer4[layloc];}
        if (y == 4){return layer5[layloc];}
        return null;
    }

 

@veggis
Once you have a List<Block> (or List<Location>, anything to save a list of something) of all the blocks you wan't to set, you can make an bukkitrunnable that will set 5 or so blocks at a time, May I see your generateQuestHouse method?

This is assuming that the lag is caused by setting the blocks, and not calculating whether to generate it or not, this could be fixed by running the task asyncly + delaying it.

Code:

final Runnable newThread = new Runnable() {
public void run() {
try {
Thread.sleep(10);//calculate your stuff in here, make sure to add thread.sleep(1-10) wherever you can, it'll delay generation of the structure, but won't put
//much strain on the server
//In your code you put "return;" to save memory, here you will want to: break;
//After this, you will want to use generateQuestHouse()
} catch (InterruptedException e) {
e.printStackTrace();
}
};new Thread(newThread).run(); //Run the thread.

 

@veggis

So the idea here is that you're not going to anything remotely similar to what @finalblade1234 said. It's some of the most atrocious code I've seen all week, I've got to say. "Run it asynchronously" is about the worst way to solve your problem, especially when you obviously have no clue what Threading actually is, or how to make thread-safe code. At all. Not to mention not understanding BlockPopulators.

Instead, I think your issue is that you're running your populator too often. What biomes are you running it in for a start? In addition, your Random is probably too lenient and it performs those checks way too often. Try upping the threshold (a lot) and then we can work on making the checks go faster.

 

@mythbusterma Fair enough, I rarely thread or use populators, As I wasn't aware that the populate method was ever called after the chunk was generated. However I still stand by what I said about the bukkitrunnable setting blocks in pairs of 5 to reduce strain.

edit: Is it also possible you could link me to an article, or tell me why "runing it asyncronously" is the wrost way, I'm not saying it isn't, I would just like to know why.

 

@finalblade1234

That's fine, do it synchronously.

The point of a populator is that it is the final step in chunk generation. It comes after the terrain is generated and it's used to create things like trees and villages. It is a separate class specifically meant for that, and setting blocks in the populator doesn't require special changes to be sent to the client, like setting them after does. It is therefore extremely fast to set blocks in the populator itself, as it's just modifying a byte array.

If you were setting the blocks otherwise, spreading the changes out over time is a good way to do it, but if it's in a populator, the populator should take care of it as quickly as possible.

 

@mythbusterma Okay, that makes sense, however wouldn't the constant getting/comparing of variables be best handled in a separate thread, then modifying the byte array with the new values? What I was saying to do in the bukkitrunnable was basically delaying how often a method was called to reduce strain on the server thread, would it be okay to use the bukkitrunnable method I was talking about instead of running it in a separate thread?

 

@finalblade1234

No, comparisons take negligible amounts of time, and so does variable assignment. Plus, you run in to a lot of issues when you involve more than one thread. For example, if you check the value of an array, then there is no guarantee that the value has not changed since you checked it.

Unless you truly understand "atomicity" and "visibility" as it pertains to threading in Java, you shouldn't be using another thread at all.

Also, NONE of the Bukkit API methods are thread-safe. So you can't safely manipulate the world at all in a different thread.

 

@finalblade1234 As myth said, threading is something that if can, should be avoided, specially in this matter. But I got your point. Thanks.

@mythbusterma Thanks for your reply.

I agree, as of now the starting block location is only from the southwest corner. Then it checks conditions from that location. Ideall would be if it checks from every corner with the conditions (or even more possible locations), then I wouldnt rely on populator running that much. What biome it is running for is irrelevant, it will tear on the server when it checks in a specific biome anyway. Right now it checks in plans and desert biome.

Yes, this is the thing and I've tried with a bigger random treshold. It checks about 40% chance for every new chunk. But to actually Check for a place to generate house, with that many conditions, it has to check Alot of places. If not the house will be a very rare spawn. To have a bigger treshold it has to check for more possible starting locations to build. If that makes sense

PS: Added updated code in first thread

@mythbusterma Ok so i think i got the "check all places in biome" working, but it evolves having another for loop around the whole thing.

Code (open) Code (close)

Code:

@Override
    public void populate(World world, Random random, Chunk chunk) {
        int noAir = 0;
        int cx = chunk.getX() * 16;
        int cz = chunk.getZ() * 16;
        int offSet = 0;
        int cZOff = cz + offSet;
        int cXOff = cx + offSet;
        int checkChance = 12;//random.nextInt(20);

        if (checkChance == 12) {
            for (int i = 1; i < 16; i++) {
                offSet++;
                //if (world.getBiome(cx, cz) == Biome.PLAINS || world.getBiome(cx, cz) == Biome.BEACH) {
                    Location loc = new Location(world, cXOff, world.getHighestBlockYAt(cXOff, cZOff), cZOff);
                    Location block = null;

                    //check ground
                    for (int x = 0; x < 8; x++){
                        int bx = loc.getBlock().getX() + x;
                        for (int z = 0; z < 9; z++){
                            int bz = loc.getBlock().getZ() + z;
                            int by = loc.getBlock().getY() - 1;
                            block = new Location(loc.getBlock().getWorld(), bx, by, bz);
                            if (block.getBlock().getType() == Material.AIR || block.getBlock().getType() == Material.STATIONARY_WATER || block.getBlock().getType() == Material.WATER || block.getBlock().getType() == Material.LAVA || block.getBlock().getType() == Material.STATIONARY_LAVA) {
                                noAir--;
                                return;
                            }
                            noAir++;
                        }
                    }

                    //check west side
                    for (int x = 0; x < 8; x++){
                        int bx = loc.getBlock().getX() + x;
                        for (int y = 0; y < 6; y++){
                            int by = loc.getBlock().getY() + y;
                            int bz = loc.getBlock().getZ() - 1;
                            block = new Location(loc.getBlock().getWorld(), bx, by, bz);
                            if (block.getBlock().getType() != Material.AIR) {
                                noAir--;
                                return;
                            }
                            noAir++;
                        }
                    }

                    //check south side
                    for (int z = 0; z < 9; z++){
                        int bz = loc.getBlock().getZ() + z;
                        for (int y = 0; y < 6; y++){
                            int by = loc.getBlock().getY() + y;
                            int bx = loc.getBlock().getX() - 1;
                            block = new Location(loc.getBlock().getWorld(), bx, by, bz);
                            if (block.getBlock().getType() != Material.AIR) {
                                noAir--;
                                return;
                            }
                            noAir++;
                        }
                    }

                    //check north side
                    for (int x = 0; x < 8; x++){
                        int bx = loc.getBlock().getX() + x;
                        for (int y = 0; y < 6; y++){
                            int by = loc.getBlock().getY() + y;
                            int bz = loc.getBlock().getZ() + 9;
                            block = new Location(loc.getBlock().getWorld(), bx, by, bz);
                            if (block.getBlock().getType() != Material.AIR) {
                                noAir--;
                                return;
                            }
                            noAir++;
                        }
                    }

                    //check east side
                    for (int z = 0; z < 9; z++){
                        int bz = loc.getBlock().getZ() + z;
                        for (int y = 0; y < 6; y++){
                            int by = loc.getBlock().getY() + y;
                            int bx = loc.getBlock().getX() + 8;
                            block = new Location(loc.getBlock().getWorld(), bx, by, bz);
                            if (block.getBlock().getType() != Material.AIR) {
                                noAir--;
                                return;
                            }
                            noAir++;
                        }
                    }

                    //Build houses
                    if(noAir == 276) {
                        generateQuestHouse(loc);
                    //}
                }
            }         
        }
    }

To test this, i used a flat world to check that the house spawns at the same location (first possible location) in that chunk. It looked something like this:


Since i had to check EVERY freakin' block highest at Y in every chunk, i got a bunch of errors in console. But that didn't happen when i sorrounded the code with smaller chance of checking conditions, and since it was Much better for performance than it originally was, I guess it came from overloading or something like that. And as you can see, it now check every block in chunk, and then spawns the house at first possible location.

If we use the same conditions as before (40% chance)

you can see that it spawns much frequent, which means we can now start upping the treshold, and cut strain on the server.

It still needs optimization and ill work on that and post it when I get there. Don't hasitate to comment if theres anything that can be improved.

EDIT by Moderator: merged posts, please use the edit button instead of double posting.

 

@veggis

You may want to look at some examples of how other BlockPopulators work, that might be a bit more helpful.

I'm not quite sure how most of them work, but I feel you're doing something a little wrong.

 

@mythbusterma Yeah, I think so too. I'll check out some other stuff. Thanks