Problem with Game of Life program

Change all the 25's in your program to SIZE and put this after your includes:
const int SIZE = 25;

Don't make your grids global. Define them in main and pass them to your functions. Then you'll only need one "initGrid" function, for instance.

Code:

---

#include <cstring>  // for memset() and memcpy()

const int SIZE = 25;
typedef char GRID[SIZE][SIZE];

int main() {
    Grid grid, nextGrid;
    initGrid(grid);
    initGrid(nextGrid); // actually, you shouldn't do this here! (see below)
    ...

void initGrid(Grid g) {
    memset(g, '-', SIZE * SIZE);
}

void copyGrid(Grid dest, Grid src) {
    memcpy(dest, src, SIZE * SIZE);
}

---

You don't need to check if a living cell has less than 2 neighbors or more than 3 neighbors because these checks are simply the opposite of checking if it is 2 or 3, i.e., if a living cell does not have 2 or 3 neighbors it dies.

Actually, you shouldn't have these checks in functions. In particular, it's not very smart to be calling gridCheck multiple times when it could just be called once.

And gridCheck is not properly guarding against array overrun. You have to check for every x or y that you add or subtract 1 to/from.

Code:

---

if (x > 0                    && grid[x-1][y]  == '0')  ++count;
if (x < SIZE-1              && grid[x+1][y]  == '0')  ++count;
if (              y > 0      && grid[x]  [y-1] == '0')  ++count;
if (              y < SIZE-1 && grid[x]  [y+1] == '0')  ++count;
if (x > 0      && y > 0      && grid[x-1][y-1] == '0')  ++count;
if (x > 0      && y < SIZE-1 && grid[x-1][y+1] == '0')  ++count;
if (x < SIZE-1 && y > 0      && grid[x+1][y-1] == '0')  ++count;
if (x < SIZE-1 && y < SIZE-1 && grid[x+1][y+1] == '0')  ++count;

---

In Game you need to clear nextGrid every time (just after while(true) ). That will start it off in the state "all cells dead". So you just need to turn on the live cells. I.e., write to nextGrid all living cells that are still living and all dead cells that have just come to life.

EDIT: To the admins: what's wrong with the code blocks? It seems to be double-blocking the code and it is not (for me at least) properly copy/pastable.

Quote:

---

Originally Posted by oogabooga

EDIT: To the admins: what's wrong with the code blocks? It seems to be double-blocking the code and it is not (for me at least) properly copy/pastable.

---

I did have that problem with a recent version of firefox too. Had to restart the browser.
Working with firefox 7 now and don't see that problem any more. ( but it's another OS too )
Kurt

Quote:

---

Originally Posted by oogabooga

post

---

Excellent suggestions, my program is considerably more efficient. Although I tried at first to pass in the arrays as function parameters I couldn't get it to work. Never thought of using a typedef.

But now the program doesn't do anything. I for the most part copied your code and shoehorned it into my program so it's my own fault but I had to edit every single function so I don't really understand what's wrong. Here's the edited main.cpp:

Code:

---

#include <iostream>
#include <cstdlib>
#include <cstring>
#include <Windows.h>

const int s = 25;
typedef char GRID[s][s];

void turnOn(GRID g, int x, int y);
void turnOff(GRID g, int x, int y);
bool lh2o3(GRID g, int x, int y);
bool dhe3(GRID g, int x, int y);
int gridCheck(GRID g, int x, int y);
void printGrid(GRID g);
void initGrid(GRID g);
void copyGrid(GRID dest, GRID src);

int main(){
    GRID grid, nextGrid;
    initGrid(grid);
    turnOn(grid, 10,15);
    turnOn(grid, 10,16);
    turnOn(grid, 9,15);
    turnOn(grid, 11,15);
    turnOn(grid, 10,14);

    //Game
    while (true){
        printGrid(grid);
        initGrid(nextGrid);
        for(int x=0;x<s;++x){
            for(int y=0;y<s;++y){
                if((grid[x][y] == '0') and (lh2o3(grid, x, y) == true)){
                    turnOn(nextGrid, x, y);
                }
                else if((grid[x][y] == '-') and (dhe3(grid, x, y) == true)){
                    turnOn(nextGrid, x, y);
                } else {
                    turnOff(nextGrid, x, y);
                }

            }
        }
        copyGrid(nextGrid, grid);
        system("PAUSE");
        system("CLS");
    }
}
// Activate cell
void turnOn(GRID g, int x, int y){
    g[x][y] = '0';
}
// Kill cell
void turnOff(GRID g, int x, int y){
    g[x][y] = '-';
}
// Check whether living cell has two or three neighbours
bool lh2o3(GRID g, int x, int y){
    if(gridCheck(g, x, y) == 2 or gridCheck(g, x, y) == 3){
        return true;
    } else {
        return false;
    }
}
// Check whether dead cell has exactly three neighbours
bool dhe3(GRID g, int x, int y){
    if(gridCheck(g, x, y) == 3){
        return true;
    } else {
        return false;
    }
}
// Scan adjacent cells for life
int gridCheck(GRID g, int x, int y){
    int count = 0;

    //To avoid array underrun and overrun
    if (x > 0                    && g[x-1][y]  == '0')  ++count;
    if (x < s-1              && g[x+1][y]  == '0')  ++count;
    if (              y > 0      && g[x]  [y-1] == '0')  ++count;
    if (              y < s-1 && g[x]  [y+1] == '0')  ++count;
    if (x > 0      && y > 0      && g[x-1][y-1] == '0')  ++count;
    if (x > 0      && y < s-1 && g[x-1][y+1] == '0')  ++count;
    if (x < s-1 && y > 0      && g[x+1][y-1] == '0')  ++count;
    if (x < s-1 && y < s-1 && g[x+1][y+1] == '0')  ++count;

    return count;
}

void printGrid(GRID g){
    for(int x=0;x<s;++x){
        for(int y=0;y<s;++y){
            std::cout << g[x][y] << " ";
        }
        std::cout << std::endl;
    }
}

void initGrid(GRID g) {
    memset(g, '-', s * s);
}

void copyGrid(GRID dest, GRID src) {
    memcpy(dest, src, s * s);
}

---

Your copyGrid() call is the wrong way round. You want to copy nextGrid into grid.
Kurt

You could simplify all the checking by doing
for ( i = 1 ; i < s-1 ; i++ )

Make your outer for loops miss the first and last rows, and first and last columns.

Or do the tests just once inside the function
if ( x < 1 || x > s-1 || y < 1 || y > s-1 ) return 0;

Alternatively you could put a border around the grid, making it (SIZE+2)*(SIZE+2) elements in size. The edges would be initialized with an otherwise-unused value (e.g., ascii-nul). Then your loops would be like for (x = 1; x <= SIZE x++) and you wouldn't need any tests at all.

Quote:

---

Originally Posted by ZuK

Your copyGrid() call is the wrong way round. You want to copy nextGrid into grid.
Kurt

---

Well that's embarrassing. My original copy function was "copy this to this" whereas this one is "copy to this from this". It works perfectly now, though I have a lot more work to do (UI-wise). Thanks so much for all the help everyone.

EDIT:
@Salem and oogabooga: Good ideas, I'll implement them.

Quote:

---

But now the program doesn't do anything.

---

I have not looked at your code but be aware that simple implementations of game of life ie with random seed cells may well lead to 'nothing' happening - you might see your starting config, and then a blank screen.. = extinction. when you start to see familiar oscilators and static cell arrangements from your seed cells you know you have nailed it

While you are at it, use std::array instead of C arrays.

Quote:

---

Originally Posted by rogster001

I have not looked at your code but be aware that simple implementations of game of life ie with random seed cells may well lead to 'nothing' happening - you might see your starting config, and then a blank screen.. = extinction. when you start to see familiar oscilators and static cell arrangements from your seed cells you know you have nailed it

---

Yeah, I know. I was only testing initial configurations for sequences I am familiar with. I have the simplelife iPhone app and I also checked a web-based Game of Life and when I said nothing was happening I meant there was no evolution when there should have been.

Quote:

---

Originally Posted by Elysia

While you are at it, use std::array instead of C arrays.

---

Never heard of it, but I'll check it out. I presume std::arrays are safer to use than C-style arrays.