Making Simon Says

Projects Programming
1

I figured I's make a Simon Says just because why not. But It isn't working and I don't know why. Here is a schematic: https://123d.circuits.io/circuits/1290892-simon-sucks

Do note that in the schematic, there are four switches. That is because the sim would pause when reading them I guess to keep from being an infinite loop.

Here is my code:

/*const byte ledRed = 2;
const byte ledGrn = 3;
const byte ledBlu = 4;
const byte ledRng = 5;*/

const byte buttonRed = 6;
const byte buttonGrn = 7;
const byte buttonBlu = 8;
const byte buttonRng = 9;

byte pattern[35]; //pattern
byte button[35]; //correct button order
byte location; //this is for the pattern playback
byte counter; //this is for our button presses

unsigned long then;
unsigned long wait;

void setup() {
  for (byte i = 2; i < 6; i++) {
    pinMode(i, OUTPUT); digitalWrite(i,0);
  }

  for (byte i = 6; i < 10; i++) {
    pinMode(i, INPUT_PULLUP);
  }

  clearPattern();
}

void loop() {
  boolean lose = 0;

  //make sure there's something there
  if (pattern[0] == 0) {
    generate();
  }

  //cycle through the pattern
  while (pattern[counter] != 0) {
    digitalWrite(pattern[counter], 1);
    then = millis();
    while (millis() - then < 250) {
      //do nothing)
    }
    digitalWrite(pattern[counter], 0);
    counter++;
  }

  //buttonary input
  counter = 0;
  while (button[counter] != 0 && lose == 0) { //we are going to do the following code once for each step in the pattern. it will also stop if the wrong button is pressed

    while (digitalRead(buttonRed) & digitalRead(buttonGrn) & digitalRead(buttonBlu) & digitalRead(buttonRng) == 1) {
      //do nothing until a button is pressed
    }

    while (millis() - wait < 1) {
      //debouncing
    }

    if (digitalRead(buttonRed) | digitalRead(buttonGrn) | digitalRead(buttonBlu) | digitalRead(buttonRng) == 0) { //makes sure a button was pressed
      if (digitalRead(button[counter]) != 0) { //if it was not the correct button
        lose = 1; //will stop the search
        clearPattern(); //clear current pattern
      }
      else { //but if it was the correct one
        counter++;
        generate();

      }
      while(digitalRead(buttonRed) | digitalRead(buttonGrn) | digitalRead(buttonBlu) | digitalRead(buttonRng) == 0){
        //wait
      }
    }
  }
  

}

void clearPattern() {
  memset(pattern, 0, sizeof(pattern));
  memset(button, 0, sizeof(button));
  counter = 0;
}

void generate() {
  byte number1;
  byte number2;
  number1 = random(2, 5);
  number2 = number1+4;

  pattern[counter] = number1;
  button[counter] = number2;
}

The 123D Emulator was doing a weird thing that the random number part of the code always 'generates' the same number, which migrates into the green LED lighting up first. I found that pressing the correct button counts as being wrong. Thinking that it was just the emulator being stupid, I made it for real... it behaves exactly the same way. So going back to the emulator, one of the arrays (pattern[])goes from being full of 0's to being completely blank.

I just have no idea what's going on. If I use the sim's debugger, I can pause the code at specific lines (82, 83, 84, 90, 94) to read the variable's values.

Mid-Post Edit
Both arrays act dumb. Actually, it's like the whole generate() function is wrong.

2 
    while (millis() - then < 250) {
      //do nothing)
    }

Why bother, when delay() does exactly the same thing? No bonus points for not using delay() when you replace it with blocking code that accomplishes nothing.

    while (digitalRead(buttonRed) & digitalRead(buttonGrn) & digitalRead(buttonBlu) & digitalRead(buttonRng) == 1) {

& and && do different things. Only one of them is appropriate here. And, not the one you chose.

    if (digitalRead(buttonRed) | digitalRead(buttonGrn) | digitalRead(buttonBlu) | digitalRead(buttonRng) == 0) { //makes sure a button was pressed

| and || do different things. Only one of them is appropriate here. And, not the one you chose.

I really do not like your code style. You seem to try to get as much as possible on one line, and then do stupid shit like:

  byte number1;
  byte number2;
  number1 = random(2, 5);
  number2 = number1+4;

  pattern[counter] = number1;
  button[counter] = number2;

using 7 lines to do what could be done in 2.

There are no comments to explain what this code is supposed to be doing. Generating a random number between 2 and 4 doesn't make sense, when most Simon Says games have 4 LEDs and 4 switches. Are the numbers supposed to be pin numbers? If so, it would seem that the random number should be between 2 and 5.

3

I did change the code a lot, so what is in the post is outdated. It is sloppy because I was babysitting a 1 y/o and only had five second windows to type anything before he was climbing on counters or trying to grab firewood. It took me about four hours just to get all of that typed. But now on to what you was saying.

I didn't use delay because I get yelled at by other members when I do. Changed to use delay() anyway.

I wasn't sure about using | or || and & or && so I took my best guess. Changed to what you suggested. Did you want me to change the other line that had | too? Because I did.

I also removed all blocks of code I ended up not using, so instead of the original 100+ lines, it's down to about 70.

const byte buttonRed = 6;
const byte buttonGrn = 7;
const byte buttonBlu = 8;
const byte buttonRng = 9;

byte pattern[35]; //pattern
byte button[35]; //correct button order
byte counter; //this is to keep track of the pattern
byte number; //just temporary storage

void setup() {
  for (byte i = 2; i < 6; i++) {
    pinMode(i, OUTPUT); digitalWrite(i, 0);
  }

  for (byte i = 6; i < 10; i++) {
    pinMode(i, INPUT_PULLUP);
  }

  clearPattern(); //gets rid of junk data and start a pattern
}

void loop() {

  //cycle through the pattern
  while (pattern[counter] != 0) {
    digitalWrite(pattern[counter], 1);
    delay(250);
    digitalWrite(pattern[counter], 0);
    counter++;
  }

  //buttonary input
  counter = 0;
  while (button[counter] != 0) { //we are going to do the following code once for each step in the pattern. it will also stop if the wrong button is pressed

    while (digitalRead(buttonRed) && digitalRead(buttonGrn) && digitalRead(buttonBlu) && digitalRead(buttonRng) == 1) {
      //do nothing until a button is pressed
    }

    delay(1);

    if (digitalRead(buttonRed) || digitalRead(buttonGrn) || digitalRead(buttonBlu) || digitalRead(buttonRng) == 0) { //makes sure a button was pressed
      if (digitalRead(button[counter]) != 0) { //if it was not the correct button
        clearPattern(); //clear current pattern
      }
      else { //but if it was the correct one, insert a new piece of the pattern.
        counter++;
        generate();
      }
      while (digitalRead(buttonRed) || digitalRead(buttonGrn) || digitalRead(buttonBlu) || digitalRead(buttonRng) == 0) {
        //wait for the button to be depressed. No debouncing because buttons are not read for at least 0.25 seconds
      }
    }
  }
}

void clearPattern() { //clears junk or old data.
  memset(pattern, 0, sizeof(pattern));
  memset(button, 0, sizeof(button));
  counter = 0;
  generate();
}

void generate() {
  number = random(2, 6);
  pattern[counter] = number;
  button[counter] = number + 4;
}

It still shows the same behavior, but this time it's the orange LED that always lights up first.

4

The random() function will always return the same sequence of numbers, unless you call randomSeed() with some random value. There are a lot of discussions about what constitutes a good value to pass to randomSeed(). There are no universally acceptable solutions.

5

free-bee:
It still shows the same behavior, but this time it's the orange LED that always lights up first.

If I were you I would:

  • not try to create a fully working program from scratch all-in-one
  • not try to create working code without possibility of debugging
  • start with simple things and then do a "stepwise refinement" of the code

First I'd put the button pins and the led pins in arrays.

Then I'd think about creating a starting value for the random number generator. The Arduino pseudo-random function will always generate the same sequence of numbers, if you do not set a randomSeed value first.

In an interactive Game it would be a good idea to wait for the first key pressed and take the time of the micros() function as a starting value for randomSeed.

That way, the random sequence will always be different, because the first button press in a game will hardly occur in the same microsecond.

So first let's concentrate on led pins, button pins, waiting for some button pressed, initialize the randomSeed value and generate a sequence of random numbers.

Try this code:

const byte ledPins[] = {2,3,4,13};
const byte buttonPins[] = {6,7,8,9};
const byte NUMCOLORS= sizeof(ledPins);

void blinkLEDs()
{ // just some fancy LED blinking
  for (int i=0; i<NUMCOLORS; i++)
  {
    bool onState= (millis()/1000)%2;
    digitalWrite(ledPins[i], onState);
  }
}

void offLEDs()
{ // switch all LEDs off
  for (int i=0; i<NUMCOLORS; i++)
  {
    digitalWrite(ledPins[i], LOW);
  }
}

void setup() {
  Serial.begin(9600); // Prepare for Serial debugging
  Serial.println("Simon Says Game");
  Serial.println("Press any button to start...");
  for (int i=0; i<NUMCOLORS; i++)
  {
    pinMode(ledPins[i],OUTPUT);
    pinMode(buttonPins[i],INPUT_PULLUP);
  }

  // now do some busy waiting until first button is pressed
  while(1)
  {
    if (digitalRead(buttonPins[0])==LOW) break; // leave loop if button-0 was pressed
    if (digitalRead(buttonPins[1])==LOW) break; // leave loop if button-1 was pressed
    if (digitalRead(buttonPins[2])==LOW) break; // leave loop if button-2 was pressed
    if (digitalRead(buttonPins[3])==LOW) break; // leave loop if button-3 was pressed
    blinkLEDs();
  }

  // then randomize the starting value for the pseudo-random number generator
  randomSeed(micros());
  // let's generate some Random numbers
  Serial.println("First 10 random numbers:");
  for (int i=0;i<10; i++) 
  {
    offLEDs();
    byte randomLED=random(NUMCOLORS);
    Serial.println(randomLED);
    digitalWrite(ledPins[randomLED], HIGH);
    delay(1000);
  }
  offLEDs();
}

void loop() {
  // put your main code here, to run repeatedly:
}

(Code is only tested for compile and debug output lines on Serial, as I don't have the circuit)

Open the Serial monitor at 9600 baud and watch!

On each reset you should get a different "random sequence" after pressing a button to start the game.

From that, "stepwise refinement" of the code can begin.