Dynamite bug:

[ttk2]

Dynamite bug: When one reinforces a block and dynamite explodes near that block, then the block will be destroyed, but the reinforcement will remain. That is, when a second block is placed in the same location as the first block (2nd block has no reinforcement), then the 2nd block will be reinforced as though it inherits the reinforcement of the 1st block. This is actually two bugs in one.

[ttk2]

No one can replicate the second part of this bug, we think whoever wrote it forgot to turn of ctfortify. Regardless the first part remains relevant, tnt should not destroy a protected block but only lower the reinforcement value.

[ttk2]

jonnyd aka Gu3rr1lla is currently working on this issue.

[JonnyD]

Hey ttk2 here is my attempt at fixing this bug. It works but the thing is it only works when the server is not being over loaded.

Placing a single reinforced block and a single TNT block and you'd think this was working perfectly. However start placing 10 reinforced blocks and about 1+ TNT blocks and you'll soon seen how bad it is.

The problem I believe is that there are way too many calls to the database in too short of a time which the server can't keep up. When the server can't keep up it gives up and afterwards doesn't care if a block is reinforced or not, the TNT will just blow it up like any other normal block.

In order to prevent reinforced blocks from being blown up we first must know which block are actually reinforced blocks. I did this by finding out the blocks that are going to be inside the TNT's blast radius and then comparing each of those blocks to entries in the database. We know a block is reinforced if its return a durability number greater than 0.

Anyway, I got to concentrate most of my time this week on college so I don't know if I can continue helping out. I hope this helps!

@EventHandler
public void checkForExplosion(EntityExplodeEvent event){

    //event.blocklist() returns blocks that were or would have been damaged by TNT explosion
    List<Block> blastRadiusBlocks = event.blockList();

    for(int i = 0; i < blastRadiusBlocks.size(); i++){
        Block block = blastRadiusBlocks.get(i);
        //mock method. needs to be added to DAO. returns current durability from database
        int durability = dao.selectDurability(block);

        //if durability > 0 that means this block is reinforced
        if(durability > 0){
            //.remove(block) removes blocks from being damaged by the upcoming explosion
            event.blockList().remove(block);
            //decrement by 1
            dao.updateReinforcement(block, 1);
        } else {
        //else delete from reinforcements
            dao.deleteReinforcement(block);
        }
    }

}

[ttk2]

I will ask one of my coworkers about this. He is some sort of SQL god and may have a trick for us. In the meantime would could try and delay the block destruction checking, not much just by a few MS, actually i wonder if NoLaggTNT (the TNT buffering component of NoLagg which allows you to set off nearly infinite amounts of TNT by slightly delaying the explosions) would solve this problem.

[Freenor]

Since database calls are expensive, we should offload computation away from the database.

One way to do this is to pull blocks from the database using some inequalities on the location variables. Explosions have a fixed radius (I presume) so we could hardcode a simple cube that's a minimal superset of the sphere (that is, the cube that contains the sphere). All of these blocks could be pulled from the database with one SQL query using inequalities with x, y, and z, and durability > 0. Inequalities over x, y, z define a geometric region, and durability > 0 ensures you only snag reinforced blocks.

Then, you could do the relatively inexpensive check: for each block in the blastRadiusBlocks List above, check to see if it's in the list of blocks you pulled from the database (using the inequalities described above). If so, add it to a special list (and delay it being destroyed). This special list can compile a list of updates to reinforcement values in the list described (propagating the damage of the TNT). This computation can be performed in Java variables and the results (differences to be applied) can then be fed through the DAO interface provided.

I hope that's clear, I'm not always very clear

[Freenor]

When the results of all the computations are fed back to the DAO for commitment to the database, it should be done as a concatenated UPDATE call (the concatenation can be done swiftly in Java). The way I've described limits the number of database calls to 2 ... if I'm reading the code above correctly, it seems as if there's a query for a durability value for each affected block in the explosion. That's expensive.

This way there's a fixed, low number of database calls per TNT explosion (2) as opposed to checking individually for each durability value (from the affected set) and then updating each with an individual call. That, no pun intended, explodes fast.

[ttk2]

Agreed, we can either take our time to optimize database requests or take an approach like this

[JonnyD]

Ok so I got it down to only 2 database calls per explosion! This is a hell of a lot better but it's still not perfect. I think moving operations to RAM could be a good solution. How do we go about that?

@EventHandler
public void checkForExplosion(EntityExplodeEvent event){

    //Store blocks that are inside explosion's blast radius
    List<Block> blastRadiusBlocks = event.blockList();

    //Initialize min & max X,Y,Z coordinates
    int smallestX = blastRadiusBlocks.get(0).getX();
    int largestX = smallestX;
    int smallestY = blastRadiusBlocks.get(0).getY();
    int largestY = smallestY;
    int smallestZ = blastRadiusBlocks.get(0).getZ();
    int largestZ = smallestZ;

    //World Name
    String worldName = blastRadiusBlocks.get(0).getWorld().getName();

    //Find min & max X,Y,Z coordinates
    for(int i = 0; i < blastRadiusBlocks.size(); i++){
        Block block = blastRadiusBlocks.get(i);
        int blockX = block.getX();
        int blockY = block.getY();
        int blockZ = block.getZ();

        if(blockX < smallestX){
            smallestX = blockX;
        }

        if(blockX > largestX){
            largestX = blockX;
        }

        if(blockY < smallestY){
            smallestY = blockY;
        }

        if(blockY > largestY){
            largestY = blockY;
        }

        if(blockZ < smallestZ){
            smallestZ = blockZ;
        }

        if(blockZ > largestZ){
            largestZ = blockZ;
        }

    }

    try {
        //Query database for any reinforced blocks that may be in the blast radius
        //Reinforced blocks should have a durability > 0 (aka >= 1)
        PreparedStatement ask = this.conn.prepareStatement(
            "SELECT X, Y, Z FROM REINFORCEMENTS WHERE DURABILITY >= 1 " +
            "AND x<=? AND x>=? AND y<=? AND y>=? AND z<=? AND z>=? AND world=?");
        ask.setInt(1, largestX);
        ask.setInt(2, smallestX);
        ask.setInt(3, largestY);
        ask.setInt(4, smallestY);
        ask.setInt(5, largestZ);
        ask.setInt(6, smallestZ);
        ask.setString(7, worldName);
        ask.execute();
        ResultSet result = ask.getResultSet();

        //If there were none found close and return
        if(!result.next()){
            result.close();
            ask.close();
            return;
        }

        //If there was some found, loop through each one
        while(result.next()){
            //Get X,Y,Z coords of reinforced block
            int x = result.getInt(1);
            int y = result.getInt(2);
            int z = result.getInt(3);

            //Then compare to the blocks in the blast radius list
            for(int i = 0; i < blastRadiusBlocks.size(); i++){
                Block block = blastRadiusBlocks.get(i);

                //Compare coords
                if(block.getX() == x && block.getY() == y && block.getZ() == z){
                    //If they equal, then remove block from affected blocks in explosion
                    event.blockList().remove(block);
                    break;
                }
            }
        }

        result.close();
        ask.close();

    } catch (SQLException e) {
        // TODO Auto-generated catch block
        e.printStackTrace();
    }   

    //Update reinforcements to set durability of blocks that are within blast radius
    //Explosion should decrement durability by 1
    try {
        PreparedStatement update = this.conn.prepareStatement(
                    "UPDATE REINFORCEMENTS SET DURABILITY = DURABILITY - 1 WHERE " +
                    "x<=? AND x>=? AND y<=? AND y>=? AND z<=? AND z>=? AND world=?");
            update.setInt(1, largestX);
            update.setInt(2, smallestX);
            update.setInt(3, largestY);
            update.setInt(4, smallestY);
            update.setInt(5, largestZ);
            update.setInt(6, smallestZ);
            update.setString(7, worldName);
            update.execute();
            update.close();
    } catch (SQLException e) {
        // TODO Auto-generated catch block
        e.printStackTrace();
    }
}

[JonnyD]

I should add that in order to limit database calls I took out the 'delete' query so when a blocks durability reaches 0 it doesn't delete from the database even though the block still goes away in the game. The danger of this is that if the player tries to add a new reinforcement block on the same coord (s)he will get an error. I could be wrong but I think the responsibility for this should rest on the applyReinforcement method? When a player adds a new reinforcement block the applyReinforcement method should check to see if the block's placement coords already exists. If yes, then 'update' query, else, 'insert' query.

[ttk2]

Good job, if you could get me a .jar file with this implemented that would be great. As for moving operations to ram i dont really know, my idea was to load the database entries as objects into an array list as chunks where loaded. If there is a function that allows you to see which blocks are loaded at any given time then it would be perfect. Otherwise it may be troublesome to identify what should be in ram or not. An alternate consideration would be to simply load the entire database into an array list on every start up, this would take a couple of seconds but probably work pretty well. The LWC database on the old server never exceeded 2mb despite all the furnace bunkers so ram usage should not be too much of a problem.

[JonnyD]

I have updated the method to add a HashMap and Coordinate Class to get rid of an unnecessary loop. It will save time because say there are 10 protected blocks in the area, and 50 blocks affected by the blast, that means it will have to loop through 10*50 items. Instead, this way, it only has to loop 50 items by doing a quicker HashMap lookup.

@EventHandler
public void checkForExplosion(EntityExplodeEvent event){

    //Store blocks that are inside explosion's blast radius
    List<Block> blastRadiusBlocks = event.blockList();
    HashMap<Coordinate, Block> affectedBlocks = new HashMap<Coordinate, Block>();

    //Initialize min & max X,Y,Z coordinates
    int smallestX = blastRadiusBlocks.get(0).getX();
    int largestX = smallestX;
    int smallestY = blastRadiusBlocks.get(0).getY();
    int largestY = smallestY;
    int smallestZ = blastRadiusBlocks.get(0).getZ();
    int largestZ = smallestZ;

    //World Name
    String worldName = blastRadiusBlocks.get(0).getWorld().getName();
    World world = this.myPlugin.getServer().getWorld(worldName);

    //Find min & max X,Y,Z coordinates
    for(int i = 0; i < blastRadiusBlocks.size(); i++){
        Block block = blastRadiusBlocks.get(i);
        int blockX = block.getX();
        int blockY = block.getY();
        int blockZ = block.getZ();

        if(blockX < smallestX){
            smallestX = blockX;
        }

        if(blockX > largestX){
            largestX = blockX;
        }

        if(blockY < smallestY){
            smallestY = blockY;
        }

        if(blockY > largestY){
            largestY = blockY;
        }

        if(blockZ < smallestZ){
            smallestZ = blockZ;
        }

        if(blockZ > largestZ){
            largestZ = blockZ;
        }

        //Instantiate Coordinate class passing in parameters
        Coordinate coordinate = new Coordinate(world, blockX, blockY, blockZ);
        //Put a new entry of type Coordinate as key and type Block as value
        affectedBlocks.put(coordinate, block);
    }

    try {
        //Query database for any reinforced blocks that may be in the blast radius
        //Reinforced blocks should have a durability > 0 (aka >= 1)
        PreparedStatement ask = this.conn.prepareStatement(
            "SELECT X, Y, Z FROM REINFORCEMENTS WHERE DURABILITY >= 1 " +
            "AND x<=? AND x>=? AND y<=? AND y>=? AND z<=? AND z>=? AND world=?");
        ask.setInt(1, largestX);
        ask.setInt(2, smallestX);
        ask.setInt(3, largestY); 
        ask.setInt(4, smallestY);
        ask.setInt(5, largestZ);
        ask.setInt(6, smallestZ);
        ask.setString(7, worldName);
        ask.execute();
        ResultSet result = ask.getResultSet();

        //If there was some found, loop through each one
        while(result.next()){
            //Get X,Y,Z coords of reinforced block
            int x = result.getInt(1);
            int y = result.getInt(2);
            int z = result.getInt(3);

            //Pass in x, y, z of reinforced block into affectedBlocks HashMap to instantiate a Block
            Block protectedBlock = affectedBlocks.get(new Coordinate(world, x, y, z));
            //Then remove the protectedBlock from explosion list
            event.blockList().remove(protectedBlock);
        }

        result.close();
        ask.close();

    } catch (SQLException e) {
        // TODO Auto-generated catch block
        e.printStackTrace();
    }   

    //Update reinforcements to set durability of blocks that are within blast radius
    //Explosion should decrement durability by 1
    try {
        PreparedStatement update = this.conn.prepareStatement(
                    "UPDATE REINFORCEMENTS SET DURABILITY = DURABILITY - 1 WHERE " +
                    "x<=? AND x>=? AND y<=? AND y>=? AND z<=? AND z>=? AND world=?");
            update.setInt(1, largestX);
            update.setInt(2, smallestX);
            update.setInt(3, largestY);
            update.setInt(4, smallestY);
            update.setInt(5, largestZ);
            update.setInt(6, smallestZ);
            update.setString(7, worldName);
            update.execute();
            update.close();
    } catch (SQLException e) {
        // TODO Auto-generated catch block
        e.printStackTrace();
    }
}

Coordinate class was created to override the Location's .equals method for when we are comparing affectedBlocks and protectedBlocks in the HashMap. This may not be needed though.

public class Coordinate extends Location {

    public Coordinate(World world, double x, double y, double z) {
        super(world, x, y, z);
        // TODO Auto-generated constructor stub
    }

}

[JonnyD]

The 'update' query in this method needs to be enhanced because blocks outside the blast radius lose durability. In the first loop it is looping through the blocks to get the bounds of explosion. The problem with this is that the bounds are cubic where as the blast radius is a sphere. This is not much of an issue for the 'select' query because after the query is executed we are comparing the entries against the blastRadiusBlocks (blocklist()). This is a problem when we get to the 'update' query. We should change the query to take into account that the blast radius is a sphere.

[Freenor]

Excellent work Jonny