Want to improve a simple game program

Projects Programming
1

This program works but I want to improve it. Two players press down and hold down a pb. A red led flashes 7 times and becomes solid. To first player to release the pb once the led becomes solid wins. To show which pb one a green led will flash beside the winners pb. The green led will flash for 5 sec and stop. Then a new game can start. One thing I want to improve is the creation of a "led light show" to celebrate the winner. Any other improvement I can do? By the way the help from this forum is great, and a special thank you to Larry!

int redled = 8;  //pin  8 is the red led
int greenled1 = 4; // pin 4 is the green led 1
int pushbutton1 = 3; //pin 3 is the pushbutton 1
int greenled2 = 11; // pin 11 is the green led 2
int pushbutton2 = 10; // pin 10 is the pushbutton 2

void setup ()  {
 pinMode (3,INPUT); //pin 3 is setup as an input
 pinMode (4,OUTPUT); // pin 4 is setup as an output
 pinMode (8,OUTPUT); // pin 8 is setup as an output
 pinMode (10,INPUT); // pin 10 is setup as an imput
 pinMode (11,OUTPUT); // pin 11 is setup as an output

}

void loop () {
 digitalWrite(redled,HIGH); // the red led will be turn on and solid to start the game
 digitalWrite(greenled1,LOW); // the green led 1 will be off
 digitalWrite(greenled2,LOW); // the green led 2 will be off

 if(digitalRead(10) && digitalRead(3) == HIGH){ //if both push buttons are pressed and held down
   delay(30);
 }
   if(digitalRead(10) == HIGH && digitalRead(3) == HIGH){ //rechecks if buttons are pressed
   delay(30);

   for(int i=1;i<=7;i++){ // the red led will flash 7 times at a 2Hz frequency
   digitalWrite(redled, HIGH);
   delay(250);
   digitalWrite(redled, LOW);
   delay(250);
   check();      // this is to check if someone let go of the button before the 7th flash.

   }

while(digitalRead(10) && digitalRead(3) == HIGH){ //if both buttons are still press after the 7th flash the red led will become solid
  digitalWrite(redled,HIGH);

}

if(digitalRead(10) == HIGH && digitalRead(3) == LOW){  // if pb2 release first as soon as redled comes solid then he wins.
  digitalWrite(greenled1,HIGH);                        // this light show is to indicate the winner using the led next to pb2
  for(int i=1;i<=10;i++){ 
   digitalWrite(greenled1, HIGH);                // I HAVE TO MAKE A "LIGHT SHOW" THAT LAST 5 SECONDS, ANY RECOMMENDATION TO MAKE THIS NICER?
   delay(250);
   digitalWrite(greenled1, LOW);
   delay(250);

  loop();

  }}
 else if(digitalRead(10) == LOW && digitalRead(3) == HIGH){   //if pb1 release first as soon as redled comes solid then he wins
  digitalWrite(greenled2,HIGH);                              //this led is to indicate the winner using the led next to pb1

  for(int i=1;i<=10;i++){                                  // I HAVE TO MAKE A "LIGHT SHOW" THAT LAST 5 SECONDS, ANY RECOMMENDATION TO MAKE THIS NICER?
   digitalWrite(greenled2, HIGH);
   delay(250);

   digitalWrite(greenled2, LOW);
   delay(250);
  loop();
}}}}

  void check(){
    if(digitalRead(3) == HIGH && digitalRead(10) == LOW){   // if player 2 release too early then player 1 wins and is winner "light show" will start
  digitalWrite(greenled1,HIGH);

  for(int i=1;i<=10;i++){           // the green led will blink 10 times for a total of 5 seconds
   digitalWrite(greenled1, HIGH);
   delay(250);
   digitalWrite(greenled1, LOW);
   delay(250);


  }}
 else if(digitalRead(3) == LOW && digitalRead(10) == HIGH){ //if player 1 release too early then player 2 wins and his winner "light show" will start
  digitalWrite(greenled2,HIGH);

  for(int i=1;i<=10;i++){            // the green led 2 will blink 10 times for a total of 5 seconds, then the next game can start
   digitalWrite(greenled2, HIGH);
   delay(250);

   digitalWrite(greenled2, LOW);
   delay(250);
   loop();

  }}}
2

can start by replacing all your 10's and 3's for your reads and writes with the int you declared pre void setup()

3

I will do that.

4 
int redled = 8;  //pin  8 is the red led
int greenled1 = 4; // pin 4 is the green led 1
int pushbutton1 = 3; //pin 3 is the pushbutton 1
int greenled2 = 11; // pin 11 is the green led 2
int pushbutton2 = 10; // pin 10 is the pushbutton 2

Don't waste space. Make all these variable const bytes instead of ints

 pinMode (3,INPUT); //pin 3 is setup as an input
  pinMode (4,OUTPUT); // pin 4 is setup as an output
  pinMode (8,OUTPUT); // pin 8 is setup as an output
  pinMode (10,INPUT); // pin 10 is setup as an imput
  pinMode (11,OUTPUT); // pin 11 is setup as an output

You have given the pins names. Why not use them ?
Use INPUT_PULLUP and avoid the need for external pullup resistors.

while(digitalRead(10) && digitalRead(3) == HIGH){ //if both buttons are still press after the 7th flash the red led will become solid

This is only doing what the comment says by accident. You cannot compare 2 values to one other like that.

while(digitalRead(10) == HIGH && digitalRead(3) == HIGH)

Would be better. You use that format later in the code.

for(int i=1;i<=10;i++)

Use spaces to make things like this easier to read

for(int i = 1; i <= 10; i++)

Make the code more readable by putting each brace on its own line and using Auto Format to indent the code. As it is at the moment it is difficult to follow the logic.

5

Good for you for sticking with your project.
Listen to UKHeliBob suggestions.

You could have the yellow led flash a random number of times say 3-11.
When the flashes stop have a blue led come or make a piezo buzzer beep.
The person who responds to the blue led or beep wins.
See:
http://arduino.cc/en/Reference/RandomSeed
And
http://arduino.cc/en/Reference/Random

6

Thanks everyone for the recommendation, I will make the changes. I want to modify the led winner "light show" with only the led I already have. I tried to had the red and the other green led to the light show but it somehow screwed the rest of the code.

Thank you

7

I don't understand why you are calling loop() within within loop(). Also, you can use Ctrl-T to format your code in a standard format that most of us will find easier to read.