Looking for help to improve my code

Hi everyone, i hope everything is allright for all of you.

I’m looking for advices and/or help to improve a specific part of my code.

So here’s my code so you can run it:

first the main class:

import java.util.Iterator;
int locX, locY;
int offsetX, offsetY;
 
final int unit = 20;
byte[][] terrain = new byte[30][30]; //30

ArrayList<GrassL> grassl;
ArrayList<GrassD> grassd;
ArrayList<Water>  water;

void setup() {
  size(601, 601); //601 601
  
  grassl = new ArrayList<GrassL>();
  grassd = new ArrayList<GrassD>();
  water = new ArrayList<Water>();  
 
  randomizeNoise();
}

void draw() {
  
  for (int i = 0; i < 30; i++) {
    for (int j = 0; j < 30; j++) {
      offsetX = i;
      offsetY = j;

      PVector l = new PVector();
      l.x = offsetX*unit;
      l.y = offsetY*unit;
      
      boolean TgrassL = false;
      boolean TgrassD = false;
      boolean Twater = false;
      
      // Out of bounds
      if (offsetX < 0 || offsetX > 29 || offsetY < 0 || offsetY > 29) fill(0xff808080); //grey
      // Draw based on terrain
      else {
        if (terrain[offsetX][offsetY]>25){ //25 //22
          TgrassD = true;
        }
        if (terrain[offsetX][offsetY]>22){ //25 //22
          TgrassD = true;
        }
        else if (terrain[offsetX][offsetY]>18){ //18 //14
          TgrassL = true;
        }
        else if (terrain[offsetX][offsetY]>14){ //18 //14
          TgrassL = true;
        }
        else {
          Twater = true;
        }
      }
      
      if(TgrassL) grassl.add(new GrassL(l));
      if(TgrassD) grassd.add(new GrassD(l));
      if(Twater)  water.add(new Water(l));
      
    }
  }
  
  //collision detection
  for(int a = 0; a < water.size(); a++){
    Water w1 = water.get(a);
    for(int b = 0; b < water.size(); b++){
      Water w2 = water.get(b);
      for(int c = 0; c < water.size(); c++){
        Water w3 = water.get(c);
        for(int d = 0; d < water.size(); d++){
          Water w4 = water.get(d);
          for(int e = 0; e < grassl.size(); e++){
            GrassL gl1 = grassl.get(e);
            for(int f = 0; f < grassl.size(); f++){
              GrassL gl2 = grassl.get(f);
              
              boolean left = false;
              boolean right = false;
              boolean top = false;
              boolean bottom = false;
              boolean leftBottom = false;
              boolean rightBottom = false;
              boolean leftTop = false;
              boolean rightTop = false;
              
              float dist = dist(w1.position.x,w1.position.y,gl1.position.x,gl1.position.y);
              if(dist == 20 ){
                gl1.skin = 1;
                
                if(w1.position.x +20 == w2.position.x &&
                   w1.position.x -20 == gl1.position.x &&
                   w1.position.y +20 == w3.position.y &&
                   w1.position.y -20 == w4.position.y) left = true;
                
                if(w1.position.x +20 == gl1.position.x &&
                   w1.position.x -20 == w2.position.x &&
                   w1.position.y +20 == w3.position.y &&
                   w1.position.y -20 == w4.position.y) right = true; 
                
                if(w1.position.x +20 == w2.position.x &&
                   w1.position.x -20 == w3.position.x &&
                   w1.position.y +20 == w4.position.y &&
                   w1.position.y -20 == gl1.position.y) top = true;
                
                if(w1.position.x +20 == w2.position.x &&
                   w1.position.x -20 == w3.position.x &&
                   w1.position.y +20 == gl1.position.y &&
                   w1.position.y -20 == w4.position.y) bottom = true;   
                   
                if(w1.position.x +20 == w2.position.x &&
                   w1.position.x -20 == gl1.position.x &&
                   w1.position.y +20 == gl2.position.y &&
                   w1.position.y -20 == w3.position.y) leftBottom = true; 
                
                if(w1.position.x +20 == w2.position.x &&
                   w1.position.x -20 == gl1.position.x &&
                   w1.position.y +20 == w3.position.y &&
                   w1.position.y -20 == gl2.position.y) rightBottom = true;
                
              }
              
              if(left == true ) w1.skinWater = 1;
              if(right == true ) w1.skinWater = 2;
              if(top == true ) w1.skinWater = 3;
              if(bottom == true) w1.skinWater = 4;
              if(leftBottom == true) w1.skinWater = 5;
              if(rightBottom == true) w1.skinWater = 6;
              if(leftTop == true) w1.skinWater = 7;
              if(rightTop == true) w1.skinWater = 8;
              
            }
          }
        }
      }
    }
  }
  
  Iterator<GrassL> GL = grassl.iterator();
  while(GL.hasNext()){
    GrassL gl = GL.next();
    gl.display();
  }
      
  Iterator<GrassD> GD = grassd.iterator();
  while(GD.hasNext()){
    GrassD gd = GD.next();
    gd.display();
  }
      
  Iterator<Water> WT = water.iterator();
  while(WT.hasNext()){
    Water wt = WT.next();
    wt.display();
  }
  
}

void keyPressed() {
  // SPACE BAR or any key 
  randomizeNoise();
}

void randomizeNoise() {
  int seed;
  seed = int(random(11111));
  //noiseSeed( seed ); 
  //println(int(seed) );
  noiseSeed( 179 );  //179
  for (int i = 0; i < 30; i++) {
    for (int j = 0; j < 30; j++) {
      terrain[i][j] = byte(ceil(noise(i*0.05,j*0.05)*60));
    }
  }
}

and the three tabs for the tiles:

class GrassD{
  ArrayList<PVector> grassd;
  public PVector position;
  
  int r;
  
  GrassD(PVector l){
    position = l.get();
    r = 20;
    grassd = new ArrayList<PVector>();
  }
  
  void add(PVector l){
    grassd.add(l.get());
  }
  
  void display(){
    fill(0xff005000);
    rect(position.x,position.y,r,r);
  }
  
  ArrayList<PVector> getGrassD(){
    return grassd;
  }
  
  PVector position(){
    return position;
  }
}
class GrassL{
  ArrayList<PVector> grassl;
  public PVector position;
  
  int r;
  
  int skin;
  
  GrassL(PVector l){
    position = l.get();
    r = 20;
    grassl = new ArrayList<PVector>();
  }
  
  void add(PVector l){
    grassl.add(l.get());
  }
  
  void display(){
    fill(0xff008000);
    rect(position.x,position.y,r,r);
    switch(skin){
      case 0:
      break;
      case 1:
        fill(255,0,0);
        text("X",position.x+5,position.y+15);
      break;
    }
  }
  
  ArrayList<PVector> getGrassL(){
    return grassl;
  }
  
  PVector position(){
    return position;
  }
}
class Water{
  ArrayList<PVector> water;
  public PVector position;
  
  int r;
  
  int skinWater;
  
  Water(PVector l){
    position = l.get();
    r = 20;
    water = new ArrayList<PVector>();
  }
  
  void add(PVector l){
    water.add(l.get());
  }
  
  void display2(){
    fill(255,0,0);
    text("O",position.x+5,position.y+15);
  }

  
  void display(){
    fill(0xff0000c0);
    rect(position.x,position.y,r,r);
    switch(skinWater){
      case 0: //<>//
        fill(0,0,0);
        text("X",position.x+5,position.y+15);
      break;
      case 1:
        fill(0,0,0);
        text("1",position.x+5,position.y+15);
      break;
      case 2:
        fill(0,0,0);
        text("2",position.x+5,position.y+15);
      break;
      case 3:
        fill(0,0,0);
        text("3",position.x+5,position.y+15);
      break;
      case 4:
        fill(0,0,0);
        text("4",position.x+5,position.y+15);
      break;
      case 5:
        fill(0,0,0);
        text("5",position.x+5,position.y+15);
      break;
      case 6:
        fill(0,0,0);
        text("6",position.x+5,position.y+15);
      break;
      case 7:
        fill(0,0,0);
        text("7",position.x+5,position.y+15);
      break;
      case 8:
        fill(0,0,0);
        text("8",position.x+5,position.y+15);
      break;
      case 9:
        fill(000000);
        text("9",position.x+5,position.y+15);
      break;
      case 10:
        fill(0,0,0);
        text("10",position.x+5,position.y+15);
      break;
    }
  }
  
  ArrayList<PVector> getWater(){
    return water;
  }
  
  PVector position(){
    return position;
  }
}

So the part of my code i’m looking to improve is (i bet you allready seen it) in the collision detection:
It’s all the for(int…) i use.
I use them because i don’t know a better way to do it.
I need them to check the four adjacents tile for each water tile and then put the correct number on it (see the example)
Untitled Untitled2 .
But it’s clearly not effective.

I imagine that is a noob question but i didn’t find a better way to do it alone.
Thanks for your times.

ok i found a way to improve my code:

i do this :

  //collision detection
  for(int a = 0; a < water.size(); a++){
    Water w1 = water.get(a);
    for(int b = 0; b < grassl.size(); b++){
      GrassL gl1 = grassl.get(b);
      
      float dist = dist(w1.position.x,w1.position.y,gl1.position.x,gl1.position.y);
      if(dist == 20 ){
        gl1.skin = 1;
        
        boolean left = false;
        boolean right = false;
        boolean top = false;
        boolean bottom = false;
        boolean leftBottom = false;
        boolean rightBottom = false;
        boolean leftTop = false;
        boolean rightTop = false;
        
        for(int c = 0; c < water.size(); c++){
          Water w2 = water.get(c);
          for(int d = 0; d < water.size(); d++){
            Water w3 = water.get(d);
            for(int e = 0; e < water.size(); e++){
              Water w4 = water.get(e);
              for(int f = 0; f < grassl.size(); f++){
                 GrassL gl2 = grassl.get(f);   
                 
                 
                 
                 if(w1.position.x +20 == w2.position.x &&
                   w1.position.x -20 == gl1.position.x &&
                   w1.position.y +20 == w3.position.y &&
                   w1.position.y -20 == w4.position.y) left = true;
                   
                 if(w1.position.x +20 == gl1.position.x &&
                   w1.position.x -20 == w2.position.x &&
                   w1.position.y +20 == w3.position.y &&
                   w1.position.y -20 == w4.position.y) right = true; 
                 
                 if(w1.position.x +20 == w2.position.x &&
                   w1.position.x -20 == w3.position.x &&
                   w1.position.y +20 == w4.position.y &&
                   w1.position.y -20 == gl1.position.y) top = true;
                   
                 if(w1.position.x +20 == w2.position.x &&
                   w1.position.x -20 == w3.position.x &&
                   w1.position.y +20 == gl1.position.y &&
                   w1.position.y -20 == w4.position.y) bottom = true;
                 
                 if(w1.position.x +20 == w2.position.x &&
                   w1.position.x -20 == gl1.position.x &&
                   w1.position.y +20 == gl2.position.y &&
                   w1.position.y -20 == w3.position.y) leftBottom = true;
                 
                 if(w1.position.x +20 == w2.position.x &&
                   w1.position.x -20 == gl1.position.x &&
                   w1.position.y +20 == w3.position.y &&
                   w1.position.y -20 == gl2.position.y) rightBottom = true;  
                 
              }
            }
          }
        }
        
        if(left == true ) w1.skinWater = 1;
        if(right == true ) w1.skinWater = 2;
        if(top == true ) w1.skinWater = 3;
        if(bottom == true) w1.skinWater = 4;
        if(leftBottom == true) w1.skinWater = 5;
        if(rightBottom == true) w1.skinWater = 6;
        if(leftTop == true) w1.skinWater = 7;
        if(rightTop == true) w1.skinWater = 8;
        
      }
    }
  }

clearly its an improvement, what do you think ? is there a better way to do it ?

not sure if you have forgotten to check leftTop and rightTop (in the section with if with 4 lines)

Remark

This

for(int c = 0; c < water.size(); c++){
          Water w2 = water.get(c);

can be shorter

for ( Water w2 : water) {

1 Like

ho i just don’t write it :slight_smile:

Thanks for the advice

1 Like

Hello,

I replaced all occurrences of position with pos.

And also:

if(dist == 20 )
  {
  gl1.skin = 1;
  
  float w1xp20 = w1.pos.x +20;
  float w1xm20 = w1.pos.x -20;
  float w1yp20 = w1.pos.y +20;
  float w1ym20 = w1.pos.y -20;
  
  if(w1xp20 == w2.pos.x  && w1xm20 == gl1.pos.x && w1yp20 == w3.pos.y  && w1ym20 == w4.pos.y)  left = true;
  
  if(w1xp20 == gl1.pos.x && w1xm20 == w2.pos.x  && w1yp20 == w3.pos.y  && w1ym20 == w4.pos.y)  right = true; 
  
  if(w1xp20 == w2.pos.x  && w1xm20 == w3.pos.x  && w1yp20 == w4.pos.y  && w1ym20 == gl1.pos.y) top = true;
  
  if(w1xp20 == w2.pos.x  && w1xm20 == w3.pos.x  && w1yp20 == gl1.pos.y && w1ym20 == w4.pos.y)  bottom = true;   
     
  if(w1xp20 == w2.pos.x  && w1xm20 == gl1.pos.x && w1yp20 == gl2.pos.y && w1ym20 == w3.pos.y)  leftBottom = true; 
  
  if(w1xp20 == w2.pos.x  && w1xm20 == gl1.pos.x && w1yp20 == w3.pos.y  && w1ym20 == gl2.pos.y) rightBottom = true;
  }

That should boost performance!

:)

1 Like

Hi, i need yours help again.
My collision detection gave me wrong output but i’m pretty sure my if() are good.
If you test my code with these if() : (sorry @glv i don’t use your float in this example)

                if(w1.position.x +20 == w2.position.x &&
                   w1.position.x -20 == gl2.position.x &&
                   w1.position.y +20 == w3.position.y &&
                   w1.position.y -20 == w4.position.y ) left = true;
                   
                if(w1.position.x +20 == gl1.position.x &&
                   w1.position.x -20 == w2.position.x &&
                   w1.position.y +20 == w3.position.y &&
                   w1.position.y -20 == w4.position.y ) right = true; 
                 
                 if(w1.position.x +20 == w2.position.x &&
                    w1.position.x -20 == w3.position.x &&
                    w1.position.y +20 == w4.position.y &&
                    w1.position.y -20 == gl1.position.y ) top = true;
                   
                 if(w1.position.x +20 == w2.position.x &&
                    w1.position.x -20 == w3.position.x &&
                    w1.position.y +20 == gl2.position.y &&
                    w1.position.y -20 == w4.position.y) bottom = true;
                
                 if(w1.position.x +20 == w2.position.x &&
                    w1.position.x -20 == gl1.position.x &&
                    w1.position.y +20 == gl2.position.y &&
                    w1.position.y -20 == w3.position.y) leftBottom = true;
                 
                 if(w1.position.x +20 == w2.position.x &&
                    w1.position.x -20 == gl1.position.x &&
                    w1.position.y +20 == w3.position.y &&
                    w1.position.y -20 == gl2.position.y) leftTop = true;  
                 
                 if(w1.position.x +20 == gl1.position.x &&
                    w1.position.x -20 == w2.position.x &&
                    w1.position.y +20 == w3.position.y &&
                    w1.position.y -20 == gl2.position.y) rightTop = true;  
                 
                 if(w1.position.x +20 == gl1.position.x &&
                    w1.position.x -20 == w2.position.x &&
                    w1.position.y +20 == gl2.position.y &&
                    w1.position.y -20 == w3.position.y) rightBottom = true;  

you can see, that there is a lot of error in the output and i can figure why.
here is a list a all the output in a picture (i don’t use all the 14 possibilities in this example, i don’t think it’s relevant)
Untitled

By the way don’t watch the results on the top right on the map, i know i need to make a different version of my collision detection for the edge of the map

Thanks for your help and sorry if i don’t make a new topic for this question. I did not want to saturate the forum

i completly lost, i can’t figure what is wrong in my code and it’s drive me crazy.

I though that my first try generate conflict with my position, so i tried this:

in the Water tab i add:

  boolean onlyWater(float w1x, float w1y, float w2x, float w3x, float w4y, float w5y){
    boolean left = false;
    boolean right = false;
    boolean top = false;
    boolean bottom = false;
    if(w1x +20 == w2x) right = true;
    if(w1x -20 == w3x) left = true;
    if(w1y +20 == w4y) bottom = true;
    if(w1y -20 == w5y) top = true;
    if(right == true && left == true && bottom == true && top == true) return true;
    else return false;
  }
  
  boolean testLeft(float w1x, float w1y, float w2x, float w3y, float w4y){
    boolean left = false;
    boolean right = false;
    boolean top = false;
    boolean bottom = false;
    for ( GrassL gl1 : grassl){
      if(w1x +20 == w2x) right = true;
      if(w1x -20 == gl1.position.x) left = true;
      if(w1y +20 == w3y) bottom = true;
      if(w1y -20 == w4y) top = true;
    }
    if(right == true && left == true && bottom == true && top == true) return true;
    else return false;
  }
  
  
  boolean testRight(float w1x, float w1y, float w2x, float w3y, float w4y){
    boolean left = false;
    boolean right = false;
    boolean top = false;
    boolean bottom = false;
    for ( GrassL gl1 : grassl){
      if(w1x +20 == gl1.position.x) right = true;
      if(w1x -20 == w2x) left = true;
      if(w1y +20 == w3y) bottom = true;
      if(w1y -20 == w4y) top = true;
    }
    if(right == true && left == true && bottom == true && top == true) return true;
    else return false;
  }
  
  boolean testTop(float w1x, float w1y, float w2y, float w3x, float w4x){
    boolean left = false;
    boolean right = false;
    boolean top = false;
    boolean bottom = false;
    for ( GrassL gl1 : grassl){
      if(w1x +20 == w3x) right = true;
      if(w1x -20 == w4x) left = true;
      if(w1y +20 == w2y) bottom = true;
      if(w1y -20 == gl1.position.y) top = true;
    }
    if(right == true && left == true && bottom == true && top == true) return true;
    else return false;
  }

  boolean testBottom(float w1x, float w1y, float w2y, float w3x, float w4x){
    boolean left = false;
    boolean right = false;
    boolean top = false;
    boolean bottom = false;
    for ( GrassL gl1 : grassl){
      if(w1x +20 == w3x) right = true;
      if(w1x -20 == w4x) left = true;
      if(w1y +20 == gl1.position.y) bottom = true;
      if(w1y -20 == w2y) top = true;
    }
    if(right == true && left == true && bottom == true && top == true) return true;
    else return false;
  }
  
  boolean testLeftBottom(float w1x, float w1y,float w2x,float w3y,float gl1x,float gl2y){
    boolean left = false;
    boolean right = false;
    boolean top = false;
    boolean bottom = false;
    if(w1x +20 == w2x ) right = true;
    if(w1x -20 == gl1x ) left = true;
    if(w1y +20 == gl2y ) bottom = true;
    if(w1y -20 == w3y ) top = true;
    if(right == true && left == true && bottom == true && top == true) return true;
    else return false;
  }

and so in the Main i tried this:

  for(Water w1 : water){
    for(Water w2 : water){
      for(Water w3 : water){
        for(Water w4 : water){
          if(w1.testLeft(w1.position.x, w1.position.y,w2.position.x,w3.position.y,w4.position.y) == true)w1.skinWater = 1; //a voir
        }
      }
    }
  }
  
  
  for(Water w1 : water){
    for(Water w2 : water){
      for(Water w3 : water){
        for(Water w4 : water){
          if(w1.testRight(w1.position.x, w1.position.y,w2.position.x,w3.position.y,w4.position.y) == true)w1.skinWater = 2;
        }
      }
    }
  }
  
  for(Water w1 : water){
    for(Water w2 : water){
      for(Water w3 : water){
        for(Water w4 : water){
          if(w1.testTop(w1.position.x, w1.position.y,w2.position.y,w3.position.x,w4.position.x) == true)w1.skinWater = 3;
        }
      }
    }
  }

  for(Water w1 : water){
    for(Water w2 : water){
      for(Water w3 : water){
        for(Water w4 : water){
          if(w1.testBottom(w1.position.x, w1.position.y,w2.position.y,w3.position.x,w4.position.x) == true)w1.skinWater = 4;
        }
      }
    }
  }
  
  for(Water w1 : water){
    for(Water w2 : water){
      for(Water w3 : water){
        for(GrassL gl1 : grassl){
          for(GrassL gl2 : grassl){
            if(w1.testLeftBottom(w1.position.x, w1.position.y,w2.position.x,w3.position.y,gl1.position.x,gl2.position.y) == true)w1.skinWater = 5;
          }
        }
      }
    }
  }
  
  
  for(Water w1 : water){
    for(Water w2 : water){
      for(Water w3 : water){
        for(Water w4 : water){
          for(Water w5 : water){
            if(w1.onlyWater(w1.position.x, w1.position.y,w2.position.x,w3.position.x,w4.position.y,w5.position.y) == true)w1.skinWater = 10;
          }
        }
      }
    }
  }

Same problem, it give me wrong output (wrong numbre on the tile).
does anyone have any idea where the problem is ?

Thanks for your time.