Game of Life - help/suggestions

I am having an issue with advancing the iterations in my effort for the Game of Life project and hoping to get some help or suggestions. I am new to React, but familiar enough to have completed the 3 ReactJS projects prior to this.

I am going via the rules mentioned in the Wikipedia entry for GOL. I plan to stick to a pure ReactJS implementation all through, if possible.

Rough flow:

I have a <board> component(or class) that holds the main logic. Inside this multiple nested <cell> components are generated per some variables (well, i have it declared in the state for now but perhaps they will change).

In the <board> component i have a ‘board’ state consisting of an array of length 400(or whatever number - this is a calculated value and is filled in from the componentDidMount). The render method iterates over this number and generates the necessary cells.

All cells are dead by default(‘x’) except a few that are alive(‘j’). These alive cells are the seed and come from a state declaration - an array consisting of numbers that point to an index in the board state array. The cells have props that trigger changes (including a shouldComponetMount condition where i compare the previous and current states). It works fine. My board is generated and the dead/alive cells display as intended.

Functions for calculating the position of a cell (i.e is it an edge etc etc) and getting the ‘neighbours’ are in place. Standalone, they work fine (except a glitch for the bottom left corner cell - which i will be fixing later since it is not pertinent to my immediate problem. In fact the edge functions as a whole are not pertinent as well for the immediate problem on hand). Given a number(an index mostly since mostly they will be called inside a loop) the functions reliably return neighbour cells.

A Start button in my <board> component calls a function that consists the rules. A thought-flow follows:

  • i create copies of the board and seed states
  • i then have my wrapper setInterval that will run the rules
  • Inside this setInterval, i begin a loop over the seed array(the ‘alive’ cells). I do it backward since i will be modifying the array in-place - removing entries if they are ‘dead’. For example for the seed array that consists of [48, 49, 50] - a blinker, the loop begins at 48, and checks if it needs to be alive/dead. If dead, the entry is removed from the array. I then loop over its neighbours (mostly 8 but it can vary depending on the position since i have a finite board - i will be displaying a shorter viewport however for a better user experience. No plans to mirror. Mostly i will be making the cells dead if they collide with an out-of-view edge) and run the same rule-set for each of them.

My loop runs through but all end up dead. Typically, i should have at least one alive cell on a single setTimeout/setIntreval iteration, going by the rules.

My first goal is a working blinker from a seed. I am not sure if my overall approach is right even though i do think it is definitely viable and the things that do work as of now do so without any problems.

Any help/suggestions here would be definitely welcome. Ideally, something that follows my thought flow(even if it seems not too optimal). Since i am quite new to ReactJS i thought ill follow through with my flow in order to avoid confusion when i try changing core approaches. But if a re-think is really required, i will go that path.

I have a JSfiddle link with a WIP code at https://jsfiddle.net/arunmenon/bL0qczbd/1/ so that the current code can be seen in entirety. Using JS Fiddle here instead of Code Pen since the latter is currently reserved for more complete/publishable code. The following function/snippet is where things start off:

       startClicked(){ 
            var boardCopy=this.state.board.slice();
            var seedSlice = this.state.seed.slice();
            
            this.interval = window.setInterval(function(){
                //seedSlice.forEach(function(obj,ind){
                for(var i=seedSlice.length;i>0;i--){
                    var obj = seedSlice[i-1];
                    this.neighbours = this.getNeighbours(obj);
                    let population = this.populationDynamics(obj);
                    if(population==="under"){
                        boardCopy[obj]="x";
                        console.log(obj, seedSlice);
                        seedSlice.splice(seedSlice.indexOf(obj),1);
                        console.log(obj, seedSlice);
                    }
                    else if(population==="over"){
                        boardCopy[obj]="x";
                        this.state.seed.splice(obj,1);
                    }
                    else if(population==="equable"){
                        boardCopy[obj]="j";
                    }
                    else /*if(population==="resurrect")*/{
                        boardCopy[obj]="j";
                    }
/*  I will be using a second/nested loop here that goes through the neighbours (this.neighbours) here. 
Finally i will set state. 
Note:I will be optimizing the repetitive calls below by separating them to a function as well as have &&/|| where necessary. 
This is just WIP   */
  
		for(var j=this.neighbours.length;j>0;j--){
                    	let ob = this.neighbours[j-1];
                    	let population = this.populationDynamics(ob);
                      if(population==="under"){
                          boardCopy[ob]="x";
                      }
                      else if(population==="over"){
                          boardCopy[ob]="x";
                      }
                      else if(population==="equable"){
                          boardCopy[ob]="j";
                      }
                      else /*if(population==="resurrect")*/{
                          boardCopy[ob]="j";
                      }
                    
                  }
                    this.setState({
		    seed: seedSlice,
                        board: boardCopy   
                    });
                }
                //}.bind(this),1000);
                
            }.bind(this),1000);
        };

Hey there!

I went through your code on JSFiddle and did some tests. If I’m not mistaken, the logic in startClicked() is actually not quite right because you are only looking at the seeded cells; the cells above and below cell 50 are not changing in the second generation simply because they never get looked at.

A couple of things that may help:

  • The problem above is not the only problem. I tried tracing the problem and it seems that populationDynamics() never returns the expected response. For example, countNeigbourAlive always returns 1 for cell 50
  • I don’t think you are supposed to change a state directly like you do with this.state.seed.splice(obj,1). The recommended way is to always use setState I believe
  • Personally, I find it unintuitive to not use Cartesian coorindates for a simple grid system that the Game of Life uses. One benefits of using Cartesian coordinates as reference that I can think of is that you can use for loops to iterate through the 3 x 3 area around a given cell/tile
  • In my opinion, the code is difficult to read because of inconsistent spacing and, in many cases, a lack of space
  • Using meaningful variables would be very helpful—particular if you are going to ask someone to look at your code. aaaa makes no sense, it puts people off and makes it less likely that you will get a response
  • I personally find the code difficult to follow. I think it’s because of how variables and functions are not named meaningfully. Together with the absence of comments, it’s very difficult for someone else to figure anything out without having need to go through the same process of going through every single line and following every single variable and function to see what they do

I hope that helps. Good luck!

Hi honmanyau,

Thanks for the comments and suggestions. Jumping straight into point 5 and 6: i fully agree with you there. In fact whenever i read other code and i find non-meaningful names, i find that i quickly lose interest. It should be double-stressed whenever encountered.

I was quickly trying out some code, and due to momentary lack of inspiration these make-shift names came about(i had ‘abc’ and ‘def’ as well but i removed them before pushing it in. I guess i missed a few). Please note that these efforts are very ‘raw’ so it will be definitely sub-optimal in many cases.

Regarding point #2, it was a slip before publishing. I have it as seedSlice.push(obj) in my local copy - i am quite aware (from some lost time in my initial 2 ReactJS projects) that setState is the way to go.

Regarding point #4, my current editor doesn’t recognize <script type="text/babel".. and so no easy way to format the code - has to be done manually (in fact even the color-codes don’t get rendered correctly’). I take the blame for the laziness there :slight_smile:

Thanks again for the comments. I am going through them in detail. For starters perhaps the Cartesian system needs to be implemented. I did read about it, but i missed understanding the significance.

Absolutely no problem. :slight_smile: I used to write things that I couldn’t even follow myself, and I completely understand that there are sometimes moments where it’s just impossible to come up with any meaningful names.

Using cartesian coordinates to refer to cells may not necessarily be faster or slower in terms of how the code runs, but I personally just find that easier to work with and debug. Given that you have already implemented logic for accessing neighbour tiles, you can likely leave that for a future project if rewriting is not an option at this moment.

The way I implemented mine in the Game of Life and Dungeon Crawler projects is to use an array of rows. This way you can refer to each tile as, for example, board[row][column]—because that’s naturally how the array is structured, you know that a tile to the top left of this given tile is always board[row - 1][column -1]. Edge cases will also automatically return undefined (which you need to take into account of, of course), and give you a consistent response across the board (no pun intended).

Once again, good luck!

Thanks for the tips especially the last paragraph. I am ok for a re-write. The Cartesian coordinates approach is definitely optimal and makes sense in such scenarios. While mine does work, it is rather roundabout when something like this already exists.