Pong game with 8x8 matrix LED display in Arduino

Question

I was working on programming an 8x8 matrix LED display. In particular, I am trying to make a basic version of Pong, controlled by two switches, one switch for each paddle.

• A ball is generated and moves to one paddle and is reflected upon collision with a paddle.
• If the ball fails to collide with a paddle on the left or right side, the ball is reset to the center of the field.
• If the ball collides with the floor or ceiling, it rebounds to the left or right side.

This game will be more of an enhancement to an encompassing display I will be doing.

Constructive criticism would be of great service, to help a beginner out. It is working as intended, but could perhaps use some tweaking/optimization.

Here is the code I've done so far (done today):

int column[] = {2, 7, 19, 5, 10, 18, 11, 16}; // Anode pins in order they represent as column
int row[] = {6, 14, 15, 3, 17, 4, 8, 9}; // Cathode pins in order they represent as row
//button PINs
const int LEFT_BUTTON = 13;
const int RIGHT_BUTTON = 12;
int leftPad[] = {0,0,128,128,128,0,0,0};
int rightPad[] = {0,0,1,1,1,0,0,0};
int ball[] = {0,0,0,16,0,0,0,0};
//Global variables
int left = 1;
int right = -1;
int rdir; //random direction for ball
int deltaCo = 0; //Change in ball 'co-ordinate'
int ballPosition; //Determines x-position of ball
int leftCounter = 0; //Determinor for change in left paddle direction
int rightCounter = 0; //Determinor for change in right paddle direction
unsigned long firstLeftMillis = 0; 
unsigned long lastLeftMillis = 0;
unsigned long firstRightMillis = 0; 
unsigned long lastRightMillis = 0;
 //Reference variables for array indexes
int leftCo = 2; 
int rightCo = 2;
int ballCo = 3;
 
void setup() {
 Serial.begin(9600); //Generic: Sets the data rate in bauds for serial data transmission 
 randomSeed(analogRead(0)); //Initializes the pseudo-random number generator with random analogue input
 for (int next = 0; next < 8; next++) // sets all I/O used as outputs
 {
 pinMode(column[next], OUTPUT);
 pinMode(row[next], OUTPUT);
 }
 
 pinMode(LEFT_BUTTON, INPUT); //Sets both button I/O as inputs
 pinMode(RIGHT_BUTTON, INPUT);
 int determinant = random(1,10); //Randomy determine ball y-position and initial ball direction
 
 ballPosition = random(3,5); 
 
 if (determinant % 2 == 0)
 {
 rdir = 1;
 }
 else
 {
 rdir = -1;
 }
}
void ClearArray(int toClear[]) //Clears array; sets all array indexes to 0
{
 for (int i = 0; i < 8; i++)
 {
 toClear[i] = 0;
 }
}
void loop() { //Runs pong on the matrix
 int rightButton = digitalRead(RIGHT_BUTTON); //Get states of both buttons
 int leftButton = digitalRead(LEFT_BUTTON);
 
 if (leftButton == HIGH) //For when the left button is pressed
 { 
 firstLeftMillis = millis(); //Sets time in milliseconds from start
 if ((leftPad[0] == 128) || (leftPad[7] == 128)) //If the paddle reaches the ends of the screen, direction is flipped
 {
 left *= -1;
 }
 leftCo += left; //Changes the reference vertical coordinate for the left paddle
 ClearArray(leftPad); //Clears the paddle
 leftPad[leftCo] = 128; //'Turns on' on the display for the paddle with appropriate coordinates
 leftPad[leftCo + 1] = 128;
 leftPad[leftCo + 2] = 128;
 }
 else if ((leftPad[0] == 0) && (leftPad[7] == 0)) //Applies once button is released and if the paddle does not touch the ends 
 {
 lastLeftMillis = millis(); //Sets time in milliseconds from start
 if (lastLeftMillis - firstLeftMillis < 500) //If time pressed is within a sufficiently short timeframe, increments the counter
 {
 leftCounter++;
 }
 if (leftCounter % 2 == 0) //If counter is odd, flips the direction; in effect, every two 'rapid button clicks' changes the direction fo the paddle
 {
 left *= -1;
 }
 
 firstLeftMillis = lastLeftMillis; //Make sure you update the time
 }
 
 if (rightButton == HIGH) //For when the right button is pressed
 {
 firstRightMillis = millis(); //Same shebang; update the time
 
 if ((rightPad[0] == 1) || (rightPad[7] == 1)) //If the paddle reaches the ends of the screen vertically within the column, flip direction
 {
 right *= -1;
 }
 rightCo += right; //Change the vertical reference coordinate of the right paddle
 ClearArray(rightPad); //Clears the array for the right paddle
 rightPad[rightCo] = 1; //'Turns on' the paddle with appropriate coordinates
 rightPad[rightCo + 1] = 1;
 rightPad[rightCo + 2] = 1;
 } 
 else if ((rightPad[0] == 0) && (rightPad[7] == 0)) //Applies once button is released and if the paddle does not touch the ends 
 {
 lastRightMillis = millis(); //Update time
 if (lastRightMillis - firstRightMillis < 500) //Increments counter if button is pressed and released in quick succession
 {
 rightCounter++;
 }
 if (rightCounter % 2 == 0) //If the right counter is even, flip the direction of the paddle
 {
 right *= -1;
 }
 
 firstRightMillis = lastRightMillis; //Updates the time
 }
 if (ball[ballCo] == 64) //Applies when the ball occupies a column adjacent to the column for the left paddle
 {
 if (leftPad[ballCo] != 0) //If the paddle is 'on' in a cell horizontally adjacent to the ball, flip direction of ball
 {
 rdir *= -1;
 if (ballCo == leftCo) //Determines vertical movement of ball if ball comes into contact with edge of paddle; initially no vertical movement
 {
 deltaCo = 1;
 }
 else if (ballCo == leftCo - 2)
 {
 deltaCo = -1;
 }
 else if (ballCo == leftCo + 2)
 {
 deltaCo = 1;
 }
 }
 else
 {
 deltaCo = 0; //Otherwise, reset ball as it has been lost by the left paddle
 ballCo = random(2,4);
 ballPosition = 3;
 }
 }
 else if (ball[ballCo] == 2) //Applies when the ball occupies a column adjacent to the column for the right paddle
 {
 if (rightPad[ballCo] != 0) //If the paddle is 'on' in a cell horizontally adjacent to the ball, flip direction of ball
 {
 rdir *= -1;
 
 if (ballCo == rightCo) //Determines vertical movement of ball if ball comes into contact with edge of paddle; initially no vertical movement
 {
 deltaCo = -1;
 }
 else if (ballCo == rightCo - 2)
 {
 deltaCo = 1;
 }
 else if (ballCo == rightCo + 2)
 {
 deltaCo = -1;
 }
 }
 else
 {
 deltaCo = 0; //Otherwise, reset ball as it has been lost by the right paddle
 ballCo = random(2,4);
 ballPosition = 5;
 }
 }
 if ((ballCo == 0) || (ballCo == 7)) //If the ball touches the cieling or floor of the display, reverses the vertical movement of the ball
 {
 deltaCo *= -1;
 }
 ballPosition += rdir; //Changes ball horizontal position
 ballCo += deltaCo; //Changes ball vertical position; initially 0
 ClearArray(ball); //Clears ball array
 ball[ballCo] = pow(2, ballPosition) + 0.5; //Determines ball horizontal coordinate in relation to vertical coordinate
 
 scanAll(leftPad, rightPad, ball); //Displays the left paddle, right paddle and ball
}
void scanAll(int firstArray[], int secondArray[], int thirdArray[]) 
{
 turnOffAllcolumn();
 unsigned long startTime = millis();
 do {
 for (int currentRow = 0; currentRow < 8; currentRow++)
 {
 outputRow(firstArray[currentRow], currentRow); //increments bits for scan of row.
 outputRow(secondArray[currentRow], currentRow);
 outputRow(thirdArray[currentRow], currentRow);
 }
 } while (millis() - startTime < 500); //delay for how long character is shown.
}
void outputRow(int cols, int curRow) //increments bits for scan of row.
{
 digitalWrite(row[curRow], HIGH); //turns on current row
 
 for (int currentColumn = 0; currentColumn < 8; currentColumn++)
 {
 boolean isAlive = bitRead(cols, currentColumn); //determine if current column is a 1 or 0
 if (isAlive)
 {
 digitalWrite(column[currentColumn], LOW); //if column was a 1 - turns it on and then off again.
 digitalWrite(column[currentColumn], HIGH);
 }
 }
 digitalWrite(row[curRow], LOW); //shuts off current row
}
void turnOffAllcolumn()
{
 for (int currentColumn = 0; currentColumn < 8; currentColumn++)
 {
 digitalWrite(column[currentColumn], HIGH); // reset column to off
 }
}

• Tunaki: Hi, welcome to Code Review! Is your code working as intended or is there some issue left?
• A Coder: It is working as intended, but could perhaps use some tweaking/optimization.
• user272752: "an 8x8 matrix LED display" ... Hmmm... (Product Number???) In (roughly) 2019, I experimented with some 8x8 matrix LED displays that made available DIN/DOUT/Clock... This 'game' code is pretty simple... If it were more advanced, it would be invaluable to strive to separate "game" logic from "display control" (aka I/O)... Think in terms of 'functional units' and strive to isolate each with a well defined interface....

Answer 1

Documentation

Add a header comment to summarize the purpose of the code:

/*
Programming an 8x8 matrix LED display. 
A basic version of Pong, controlled by two switches, one switch for each paddle.
*/

Layout

The structure of the code is hard to understand using only 2 spaces per indent level:

void ClearArray(int toClear[])
{
 for (int i = 0; i < 8; i++)
 {
 toClear[i] = 0;
 }
}

It is more common to use 4 spaces:

void ClearArray(int toClear[])
{
 for (int i = 0; i < 8; i++)
 {
 toClear[i] = 0;
 }
}

Comments

The comments in the code are helpful, but their placement is inconsistent and too far from the code they are intended to explain. For example:

int deltaCo = 0; //Change in ball 'co-ordinate'
int ballPosition; //Determines x-position of ball

This layout is more sensible:

int deltaCo = 0; // Change in ball 'co-ordinate'
int ballPosition; // Determines x-position of ball

Long lines are hard to understand:

Serial.begin(9600); //Generic: Sets the data rate in bauds for serial data transmission 

This can be split into 2 lines, with the comment above the code:

// Generic: Sets the data rate in bauds for serial data transmission 
Serial.begin(9600);

Simpler

if/else statements like this:

if (determinant % 2 == 0)
{
 rdir = 1;
}
else
{
 rdir = -1;
}

can be simplified using the ternary operator:

rdir = (determinant % 2 == 0) ? 1 : -1;

Magic number

Repeated numbers in the code, such as 128, can be replaced by a named constant.

Naming

Long iterator variable names:

for (int currentColumn = 0; currentColumn < 8; currentColumn++)

are unnecessary. It is more common to use a short name, as you've done elsewhere in your code:

for (int i = 0; i < 8; i++)