Conway's Game of life overpopulation?

Question

I was working on a Conway's game of life clone because it is good practice, but I ran into a problem. I see there are pixels deleting and rebirthing, but all the pixels just spread out the very end of the screen and then other pixels are rebirthing, but it is just idling at that point.

Here are some screenshots: enter image description here

I'll show you some of my code for the logic of this. It is all handled in the change method:

package main;

import java.awt.Color;
import java.awt.Graphics;

public class Functions {

  public static int pixelsize=6,gridwidth=800/6,gridheight=600/6;
  static int[][] pixels = new int[gridwidth][gridheight];
  static boolean first = true;

  public static void change(){

    for(int i = 0; i < gridwidth; i++){
      for(int j = 0; j < gridheight; j++){
        int neighbors = 0;
        //check each cell

        try{

          if(pixels[i+1][j] == 1){neighbors++;}
          if(pixels[i-1][j] == 1){neighbors++;}
          if(pixels[i+1][j-1] == 1){neighbors++;}
          if(pixels[i][j+1] == 1){neighbors++;}
          if(pixels[i][j-1] == 1){neighbors++;}
          if(pixels[i+1][j+1] == 1){neighbors++;}
          if(pixels[i-1][j-1] == 1){neighbors++;}
          if(pixels[i-1][j+1] == 1){neighbors++;}

        }catch(ArrayIndexOutOfBoundsException e){

        }

        if(neighbors == 3 || neighbors == 2 ){
          pixels[i][j] = 1;
        }else if(neighbors < 2 || neighbors >= 4){
          pixels[i][j] = 0;
        }
      }
    }
  }

  public static void render(Graphics g){

    for(int i = 0; i < gridwidth;i++){
      for(int j = 0; j < gridheight; j++){
        if(pixels[i][j] == 1){

          g.setColor(Color.red);
          g.fillRect(i*6, j*6, 6, 6);
        }
      }
    }
  }
}

thanks for all the help. Sadly it still isn't working correctly.
Now it is doing the same thing but in diamond formation like so:
enter image description here

why ignore the AIOOBE? Consider using nested for loops for your check above, but ignoring the case of i == 0 && j == 0, and using `Math.min(...)` and `Math.max(...)` for handling the edge cases so you don't have to worry about AIOOBE. — Hovercraft Full Of Eels, Feb 08 '13 at 17:39
`catch(ArrayIndexOutOfBoundsException e)` this is an atrocity! Never do something like that. Solution to your problem is actually in my other answer [here](http://stackoverflow.com/questions/14439109/java-search-words-in-two-dimensional-array/14439402#14439402) — Dariusz, Feb 08 '13 at 17:39

score 6 · Accepted Answer · edited Jan 24 '19 at 21:00

The main problem I do see here is the fact that you are updating values while discovering them.

You should cache your whole grid (or at least neighbours count) before updating, otherwise, while you update element at (x, y), you are alterating the neighbours count for element that are successive as (x+1,y), (x+1,y+1), (x,y+1) by counting the outcome of current iteration.

For example you could update a separate array called cachedPixels like so:

for(int i = 0; i < gridwidth; i++){
    for(int j = 0; j < gridheight; j++){
        int neighbors = 0;
        // find the proper boundaries
        int minI = Math.max(0, i - 1);
        int maxI = Math.min(gridwidth, i + 2)
        int minJ = Math.max(0, j - 1);
        int maxJ = Math.min(gridheight, j + 2)

        for (int i2 = minI; i2 < maxI; i2++) {
           for (int j2 = minJ; j2 < maxJ; j2++) {
              if (i2 != i || j2 != j) {
                 if (pixels[i2][j2] == 1) {
                    neighbors++;
                 }
              }
           }
        }

        if (neighbors == 2 || neighbors == 3) {
           cachedPixels[i][j] = 1;
        } else {
           cachedPixels[i][j] = 0; // probably not even necessary as 0 is default value
        }
    }   
}

Then after completing this process for the entire array set using the arraycopy function:

for (int i = 0; i < length; i++) {
    System.arraycopy(cachedPixels[i], 0, pixels[i], 0, cachedPixels[i].length);
}

Simply setting pixels = cachedPixels would point "pixels" to the "cachedPixels" array and so changing one would change the other, and the system would collapse.

P.S. The rules you're using for your GoL are not the same as John H. Conway's. Cells always live on the next time step if they have 3 neighbours, and live on the next time step with 2 neighbours only if they are alive in this time step, else they die:

cachedPixels[i][j] = 0; // Default value - death.
if (neighbors == 3) {
   cachedPixels[i][j] = 1;
} else if (thisCell == 1 && neighbors == 2) {
   cachedPixels[i][j] = 1;
}

Jack: I added code example of what you suggested. If OK, leave it, but please delete it if not. — Hovercraft Full Of Eels, Feb 08 '13 at 17:49
@HovercraftFullOfEels: It's ok, don't worry. I was just too lazy to find any implementation around my codebase ;) — Jack, Feb 08 '13 at 17:52
Thanks so much! The only problem now is when I place start it, it spreads out in a diamond shape and then fills the screen. — 36redsoxfan, Feb 08 '13 at 17:54

score 2 · Answer 2 · answered Feb 08 '13 at 17:41

Your newborn pixels should not be counted as neighbours for this move, and your dying pixels should be counted. It can be achieved by having another values, say 2 and 3, for newborn and dying pixels. After processing the field, you commit its state by replacing a value for newborn with 1 and a value for dying with 0.

Conway's Game of life overpopulation?

2 Answers2