Simon Says Project - Can't go from random lights to user input

Question

I'm new to using an Arduino and I am attempting the Simon Says game. Right now, I do not understand why my program will not go from activating the random lights to the user input. Currently it is flashing all the lights three times like I want it to, but then it will flash three random lights and then go straight to flashing every light, right to left (which is the indicator that the wrong button was pushed).

I am working in Tinkercad/circuits.io and have had the physical implementation looked at, and it is good. I just can't figure out the code. Anything will help, thanks!

// Defining LEDs and buttons.
int leds[] = {12, 10, 7, 4};
int buttons[] = {11, 9, 6, 3};
const int max = 40;
int lightArr[max]; // Array with full patterns of LEDs.
int currArr[max];
int gameState = 0;
int level = 1;
int count = 0; // Counter for every sequence of iterating over the 
random array of ints (LEDs).
int buttonPressed = -1;
int correct = 1;
void setup() {
 pinMode(leds[0], OUTPUT);
 pinMode(leds[1], OUTPUT);
 pinMode(leds[2], OUTPUT);
 pinMode(leds[3], OUTPUT);
 pinMode(buttons[0], INPUT);
 pinMode(buttons[1], INPUT);
 pinMode(buttons[2], INPUT);
 pinMode(buttons[3], INPUT);
 randomSeed(analogRead(0));
 allOn();
}
void loop() {
 if (gameState == 0) {
 resetGame();
 } else if (gameState == 1) {
 play();
 } else if (gameState == 2) {
 gameOver(); 
 } 
}
void resetGame() {
 if (level == 1) {
 // Setting up a sequence of random lights.
 for (int i = 0; i < 40; i++) {
 lightArr[i] = random(0,4);
 }
 }
 level = 1; 
 gameState = 1;
}
void play() {
 while (correct == 1) {
 showPattern();
 int correct = userInput();
 }
}
void showPattern() {
 for (int i = 0; i < level; i++) {
 delay(500);
 digitalWrite(leds[lightArr[i]], 1);
 delay(500);
 digitalWrite(leds[lightArr[i]], 0);
 delay(500);
 }
}
int userInput() {
 for (int i = 0; i < level; i++) {
 int buttonPressed = readOneButton();
 digitalWrite(leds[buttonPressed], 1);
 delay(500);
 digitalWrite(leds[buttonPressed], 0);
 delay(500);
 if (lightArr[i] != buttonPressed) {
 // User button pressed.
 gameState = 2;
 correct = 0;
 }
 }
}
void gameOver() {
 for (int i = 0; i < 3; i++) {
 digitalWrite(leds[0], 1);
 delay(150);
 digitalWrite(leds[0], 0); 
 delay(150);
 digitalWrite(leds[1], 1);
 delay(150);
 digitalWrite(leds[1], 0);
 delay(150);
 digitalWrite(leds[2], 1);
 delay(150);
 digitalWrite(leds[2], 0);
 delay(150);
 digitalWrite(leds[3], 1);
 delay(150);
 digitalWrite(leds[3], 0);
 delay(150);
 }
 gameState = 0;
}
int readOneButton() {
 if (digitalRead(buttons[0])) {
 return 0; // Button: Red
 } else if (digitalRead(buttons[1])) {
 return 1; // Button: Green
 } else if (digitalRead(buttons[2])) {
 return 2; // Button: Blue
 } else if (digitalRead(buttons[3])) {
 return 3; // Button: Yellow
 }
}
void allOn() {
 for (int i = 0; i < 3; i++) {
 digitalWrite(leds[0], 1);
 digitalWrite(leds[1], 1);
 digitalWrite(leds[2], 1);
 digitalWrite(leds[3], 1);
 delay(500); 
 digitalWrite(leds[0], 0); 
 digitalWrite(leds[1], 0); 
 digitalWrite(leds[2], 0); 
 digitalWrite(leds[3], 0);
 delay(500);
 }
}

Answer 1

void play() {
 while (correct == 1) {
 showPattern();
 int correct = userInput();
 }
}

Why do you create a new variable called correct and put the value form userInput in that? Why don't you put the return from userInput into the variable named correct that you already have? Two variables with the same name in the same scope is NEVER a good idea. The original correct will never be changed here so this becomes an infinite loop.

Next, look at readOneButton. It will take the Arduino just a few microseconds to run through that for loop and the whole userInput function. Unless you are super duper fast at getting that button pressed, it is just going to decide that you didn't press any buttons. You don't have a case in there for no button pressed, so it is going to return 0. You should probably wait until the user presses a button before you decide whether he is right or wrong.