highscore for a game

This ‘game’ shows a random number between 0 and 15 as binary and the goal is to press the button the same amount as the number the LEDS display as fast as you can.
When the number of presses is equal to the random number it must print out how fast it was.
Everything works fine, except for the part where its supposed to print the time it took to press the button as many times as the number.

What i would like is the game to start as mode 1
if the counter is equal to the number go to mode 2
in mode 2 print the time it took and go back to mode 1

I tried making 2 gamemodes where 1 is the actual game and 2 is where it prints the time when the number of presses is correct and then resets to mode 1, but it keeps looping the part where the counter is equal to the number. It doesnt go to mode 2.
In the monitor it prints out two numbers, the first is 9-11 and the second is 19-21 which i don’t know why it prints those and after that it prints all zero’s.

#include <TrueRandom.h>

int number = TrueRandom.random(0, 16);
const int buttonPin = 7; 
int ledPin[] = {2,3,4,5}; 
boolean buttonState = LOW;
int counter = 0;
long startTime, stopTime, beginTime, endTime;
int gameMode = 1;

void setup() {
  Serial.begin(9600);
  pinMode(buttonPin, INPUT); 
  showBinary(number);  
   
  for (int i = 0; i < 4; i++) {
    pinMode(ledPin[i], OUTPUT);
  }
}

void showBinary(byte binaryNumber) {    
    for (int i = 0; i < 4; i++) {
      if (bitRead(binaryNumber, i) == 1) {
        digitalWrite(ledPin[i], HIGH);
      }
      else {
        digitalWrite(ledPin[i], LOW);
      }
    }
}

void loop() {
    
  if (gameMode == 1) {
    beginTime = millis();
        
    if (debounceButton(buttonState) == HIGH && buttonState == LOW) {     
      counter++;
      buttonState = HIGH;
      stopTime = millis() - startTime;
    }
    else if (debounceButton(buttonState) == LOW && buttonState == HIGH) {
      buttonState = LOW;
      startTime = millis();
    }     
    if (stopTime > 2000) {
      digitalWrite(2, HIGH);  
      delay(100);
      digitalWrite(2, LOW);
      digitalWrite(3, LOW);        
      digitalWrite(4, LOW);
      digitalWrite(5, HIGH);
      delay(100);
      digitalWrite(5, LOW);   
    }
    else if (counter > number) {  
      digitalWrite(2, HIGH);  
      delay(100);
      digitalWrite(2, LOW);
      digitalWrite(3, LOW);        
      digitalWrite(4, LOW);
      digitalWrite(5, HIGH);
      delay(100);
      digitalWrite(5, LOW);   
    }     
    else if (counter == number) {  
      endTime = millis() - beginTime;
      digitalWrite(2, LOW);
      digitalWrite(3, HIGH);
      delay(100);
      digitalWrite(3, LOW);
      digitalWrite(4, HIGH);
      delay(100);
      digitalWrite(4, LOW);
      digitalWrite(5, LOW); 
      gameMode = 2;    
    }  
  }
  else if (gameMode == 2) {
    Serial.println(endTime);
    gameMode = 1;   
  }
}

boolean debounceButton(boolean state) {
  boolean stateNow = digitalRead(buttonPin);
  if (state != stateNow) {
    delay(10);
    stateNow = digitalRead(buttonPin);
  }
  return stateNow;
}

Could someone give me some pointers?

Sure.

char *stupidStatement = "but I just can't get it to work.";
char *reasonableStatements = {
     "The code does something. You need to explain what is actually does",
     "That somehow differs from what you want. You need to explain how" };

aapie:
Could someone give me some pointers?

Yes, how about starting with a better description of what you're expecting the code to do, and what is it actually doing? Just saying it doesn't work is very ambiguous and could mean just about anything.

A couple things I notice right off the bat:

  1. In setup(), you first write out a random number to the LEDs, and then you set the LED pins to outputs. This will likely not accomplish what you want. The pins are inputs at powerup. When you do a digitalWrite() to an input pin, you are actually turning the internal pullup resistor on or off. You are not setting what the future state of the pin will be when you do eventually make it an output. Set the pins to outputs first, then write the random number to the pins.

  2. You set the beginTime value to the current millis() value on each pass through loop() whenever gameMode is 1. You probably only want to do that when you first start a game sequence, not on every pass. When your count finally does equal number, you will calculate an end time, but that will just be the time is took to debounce the last switch press and update/test the counter. It will not be the time since the game started.

  3. You have an IF condition checking whether counter > number, but the IF condition of counter == number will always evaluate first and end the game. Did you want it to end as soon as the count is correct, or were you planning on checking for additional button presses to make sure the button wasn't pressed too many times? If the latter, your logic won't work the way it is now: you will probably want to save the time when the count did become equal, but then give a little more time to see if there are additional button presses before declaring victory.

  4. You only generate a random number at the top of the file, and never again. By setting gameMode back to one, it seems like you want to repeat the game, but it will always be with the same number. Right now, the only time that number will change is on power on or reset.

  5. Same thing with counter: it is set to zero at the top, but never reset between games. Right now, the only time that number be zero is on power on or reset. You will certainly want to clear it when starting a new game.

  6. As soon as the correct number of button presses are found, it switches to gameMode 2, prints the time, and goes immediately back to gameMode1. You might want to stay in mode 2 for a while, perhaps until another button press, to give the player time to get ready for the next game?

I'm sorry, I just started coding and it's my first time coding with arduino.
I edit my first post. I hope it explains what it does now and what i would like it to do.

Thank you for the feedback, ShapeShifter. That is very helpful

I edit my first post.

Never again. Add details in a reply.

aapie:
I'm sorry, I just started coding and it's my first time coding with arduino.

Everybody was a first-time sometime. You've got to start somewhere, nobody was born a coding expert.

aapie:
In the monitor it prints out two numbers, the first is 9-11 and the second is 19-21 which i don't know why it prints those and after that it prints all zero's.

Clear your mind about what the program does. Print out the code, and manually walk through the code making pencil notes about what is in the various variables. Don't assume anything that you intended, just look at what is actually coded.

You start out with gameMode as 1, and counter as zero. As you press the button, the counter is incremented. Now, assume that counter is one less than number, and start at the top of loop(). gameMode is one, so you enter the first IF statement, and beginTime gets set to the current millis() value. Then, you call debounceButton: the buttonState is LOW, and the button is currently pressed.

debounceButton reads the button input, sees that the state is different, and delays 10 ms. It then reads the button state (still high) and returns it. The else clause is not executed because the IF clause was. So the next thing is stopTime is compared to 2000, which is false. The next else if is tested, and counter is not greater than number (it is equal) so that is false. The next else if is evaluated, and that is true because counter is now equal to number. End time is calculated by subtracting the beginTime from the current millis() value. beginTime was set at the top of this pass to loop: most of the code will execute quite quickly, and the longest operation was the 10 ms delay in debounceButton, so the endTime will be around 10 ms (depending on where these events lined up withing the current millisecond, the actual value could be 9, 10, or 11.) The code then flashes a pattern on the LEDs, and sets gameMode to 2. The final else if is skipped because the original gameMode test was true, and loop exits().

On the next pass, the initial IF is false (gameMode is now 2, not 1) so the whole long alternative is skipped. The else if at the end of loop is true because gameMode is 2) so the endTime (9 to 11) is printed, and gameMode is set to 1.

Back at the top of the loop, gameMode is once again 1, so the initial if is true. beginTime is once again set to the current millis() value. Then you hit the next if and call debounceButton: at this point, the button is still down (everything that has happened since the last button press has been so quick that your finger hasn't even started to release the button yet.) So, debounceButton does a 10 ms delay and returns, but the buttonState is HIGH, so the first IF is false. Therefore it goes to the else if, and calls debounceButton again, which takes another 10 ms. Finally, the next series of if statements are evaluated, and the counter == number expression is still true because counter was not cleared. At this point, the endTime is calculated (it was 19 to 21 now because of the two 10 ms delays) gameMode becomes 2, the result is printed, etc.

The next time you press the button, count is now > number and things are really falling apart...

Doing a paper walkthrough of the code like that can be very helpful, but only if you make no assumptions about what you intended the code to do - ONLY DO WHAT IS ACTUALLY CODED! This will often show up the cause of your problems.

If that fails, a common tactic is to sprinkle a lot of print statements throughout the code. Inside each IF statement, print out a message saying you entered that block of code. Print out important variable values at key points. Run the code and compare the printed output with what you intended to get, and with what you got when you did your paper walkthrough. Wherever there is a difference, try to understand WHY it is different than what you expected - that will usually lead to the problem.

PaulS:
Never again. Add details in a reply.

Very good advice. Materially changing a post makes it harder for others to follow a thread, because it changes the context for replies that follow it. It also makes it harder for others to learn from your post.

There are a few valid reasons for editing a post, but not many: fixing grammar or spelling, adding code tags around code listings to make it easier to read, changing the subject name by adding "SOLVED:" to the begging, etc. These are mostly housekeeping type activities that don't really change the meaning.

I solved some of the problems now, thanks to your feedback and going through the code to see where it might go wrong.
It goes through the different gameModes, prints the time and resets to gameMode 1.

Nr. 3) about the delay before it checks if its the right number or not is something I might look at later.
I’m now stuck at nr. 4), like you said it generates the same number when it resets to gameMode 1.
I’m not sure where to put that line. I tried placing it in gameMode 1, but then at the part where it compares the counter with the number it tells me it was not declared.

#include <TrueRandom.h>

int number = TrueRandom.random(0, 16);
const int buttonPin = 7; 
int ledPin[] = {2,3,4,5}; 
boolean buttonState = LOW;
int counter = 0;
long startTime, stopTime, beginTime, endTime;
int gameMode = 1;

void setup() {
  Serial.begin(9600);
  pinMode(buttonPin, INPUT);  
   
  for (int i = 0; i < 4; i++) {
    pinMode(ledPin[i], OUTPUT);
  } 
}

void showBinary(byte binaryNumber) {    
    for (int i = 0; i < 4; i++) {
      if (bitRead(binaryNumber, i) == 1) {
        digitalWrite(ledPin[i], HIGH);
      }
      else {
        digitalWrite(ledPin[i], LOW);
      }
    }
}

void loop() {
  if (gameMode == 1) {
    counter = 0;
    showBinary(number);
    beginTime = millis();
    gameMode = 2;
  }
  
   else if (gameMode == 2) {       
    if (debounceButton(buttonState) == HIGH && buttonState == LOW) {     
      counter++;
      buttonState = HIGH;
      stopTime = millis() - startTime;
    }
    else if (debounceButton(buttonState) == LOW && buttonState == HIGH) {
      buttonState = LOW;
      startTime = millis();
    }     
    if (stopTime > 2000) {
      digitalWrite(2, HIGH);  
      delay(100);
      digitalWrite(2, LOW);
      digitalWrite(3, LOW);        
      digitalWrite(4, LOW);
      digitalWrite(5, HIGH);
      delay(100);
      digitalWrite(5, LOW);   
    }
    else if (counter > number) {  
      digitalWrite(2, HIGH);  
      delay(100);
      digitalWrite(2, LOW);
      digitalWrite(3, LOW);        
      digitalWrite(4, LOW);
      digitalWrite(5, HIGH);
      delay(100);
      digitalWrite(5, LOW);   
    }     
    else if (counter == number) {  
      digitalWrite(2, LOW);
      digitalWrite(3, HIGH);
      delay(100);
      digitalWrite(3, LOW);
      digitalWrite(4, HIGH);
      delay(100);
      digitalWrite(4, LOW);
      digitalWrite(5, LOW);
      gameMode = 3;        
    }  
  }
  else if (gameMode == 3) {
    endTime = millis() - beginTime;
    Serial.println(endTime);
    gameMode = 1;  
  }
}

boolean debounceButton(boolean state) {
  boolean stateNow = digitalRead(buttonPin);
  if (state != stateNow) {
    delay(10);
    stateNow = digitalRead(buttonPin);
  }
  return stateNow;
}

aapie:
I'm now stuck at nr. 4), like you said it generates the same number when it resets to gameMode 1.
I'm not sure where to put that line. I tried placing it in gameMode 1, but then at the part where it compares the counter with the number it tells me it was not declared.

That's because you are likely confused about the difference between declaring a variable, and setting its value.

For example, this is declaring four variables, all of which are longs:

long startTime, stopTime, beginTime, endTime;

By the way, hey should be unsigned longs to match the value returned by millis() and so that your time calculations work properly.

This is an example of declaring a variable, and setting it's value at the same time:

int gameMode = 1;

And this is an example of setting the value of a previously declared variable:

gameMode = 2;

Notice the difference? This one does not include a type name. If it has a type name before the variable name, it is declaring the variable, whether or not a value is being assigned to it. With no type name before it, it's simply assigning to the previously declared variable.

Now, you are currently declaring number at the top of the file. That's good: it needs to be declared before it is used, and you want to to remember the value through multiple passes of loop90 (which means you don't want to declare it inside of loop() because the previous value goes away between calls.) So the location of the declaration is good. It's also good to assign an initial value there by calling TrueRandom.random() and assigning the result to number.

It sounds like you tried moving both the declaration and assignment of number down into the code. That would be bad, because it won't be defined for the earlier access of it, and because it will not be remembered between iterations of loop(). So you can't move the declaration.

But you want to make another assignment to it. By now, you hopefully see the answer, just leave off the type name when you want to assign a value:

number = TrueRandom.random(0, 16);

Thanks a lot for your help and quick responses, ShapeShifter.

Everything works so far.
I'll have a go at adding that timer to check if there are not too many button presses.