I am having an issue in my code making it not work correctly

fisheyyy · January 18, 2025, 1:03pm

I am making one of those memory games where leds display a sequence and you copy it by pressing the corresponding buttons in order. Problem arises when two of the same led are displayed sequentially at the end of a sequence requiring the user only to press the corresponding button once instead of twice. I am very new to coding arduinos and i have tried to work with ChatGPT to remedy the issue to no avail.

int currentNum = 0;
int randomNum[100];
int counting = 0;
int userPress = 0;

const int led1 = 10;

const int led2 = 9;
  
const int led3 = 8;

const int led4 = 7;
  
void setup()
{
  pinMode(2, INPUT);
  pinMode(3, INPUT);
  pinMode(4, INPUT);
  pinMode(5, INPUT);
  pinMode(7, OUTPUT);
  pinMode(8, OUTPUT);
  pinMode(9, OUTPUT);
  pinMode(10, OUTPUT);
  
  Serial.begin(9600);
  randomSeed(analogRead(A0));
  
  randomNum[currentNum] = random(7,11);
}

void count()
{
	digitalWrite(randomNum[counting], HIGH);
	delay(500);

  	digitalWrite(randomNum[counting], LOW);
	delay(500);

  	counting++;
}

void user(){
  
int btn1 = digitalRead(5);
int btn2 = digitalRead(4);
int btn3 = digitalRead(3);
int btn4 = digitalRead(2);
  
if(btn1 == HIGH && randomNum[userPress] == led1){
    userPress++;
  	delay(50);

}else if(btn2 == HIGH && randomNum[userPress] == led2){
  	userPress++;
  	delay(50);

}else if(btn3 == HIGH && randomNum[userPress] == led3){
  	userPress++;
  	delay(50);
  
}else if(btn4 == HIGH && randomNum[userPress] == led4){
	userPress++;
  	delay(50);

}else if(btn4 == HIGH && randomNum[userPress] == !led4 ||
         btn3 == HIGH && randomNum[userPress] == !led3 ||
         btn2 == HIGH && randomNum[userPress] == !led2 ||
         btn1 == HIGH && randomNum[userPress] == !led1){
  	counting = 0;
  	userPress = 0;
  	currentNum = 1;
  	delay(1000);
 
}
} 


void loop()
{
  
  while(counting < currentNum){
    count();
  }
  
  if(counting == currentNum){
    counting = 0;
      
      while(userPress < currentNum){
      user();
    }
  }
  
  
  if(userPress == currentNum){
    currentNum++;
    userPress = 0;
    randomNum[currentNum] = random(7, 11);  
  }
  
}

Any help or even tips at any level are good as i am trying to learn and improve from this!

UKHeliBob · January 18, 2025, 1:08pm

Welcome to the forum

Surely the user should be required to press the corresponding button twice if the same led is illuminated twice

fisheyyy · January 18, 2025, 1:09pm

Yes, that is the intended function. Which does not work. Or atleast at the end of the sequence, where the user only has time to press it once before instantly showing the next sequence.

sonofcy · January 18, 2025, 4:06pm

Try to design a main loop that mimics the actual scenario. After a brief look at the code I don't see a function to display a random sequence of less (side not not really random either) That should happen once in the loop while the bulk of the time is spent reading the buttons (button library, bounce issues) and that sub loop continues until Solved == true;
Thank you for mentioning ChatGPT, I will let it and you carry on and I will ignore any further posts.

fisheyyy · January 18, 2025, 6:07pm

What is the problem with that? Why should i not use it?

van_der_decken · January 18, 2025, 6:18pm

Because you cannot use a generative AI to do something that you don't know how to do yourself.

Why? Because you won't know when it's just making stuff up that just sounds right, but isn't. You'll waste more of your time with that aspect than you will ever save by trying to get an AI to do the job.

And as you've just now discovered, not many people are going to want to waste their time trying to decipher ChatGPT "it looks right but it's really nonsense" code. Using AI to generate code and then expecting others to debug it for you when it doesn't work is not a good look.

fisheyyy · January 18, 2025, 6:24pm

That makes perfect sense but i guess ive never really thought about it that way. I had many examples of it just spewing bullshit at me but yeah. I get that. I will try and fix this without AI from now. As someone new to programming it is quite useful for finding simple coding errors though, which is what i use it for mainly.

LarryD · January 18, 2025, 7:32pm

alto777 · January 18, 2025, 7:58pm

A valiant effort. I can't make heads nor tails of your logic, but it is sound. I have enjoyed playing the game without difficulty.

I built it for myself. There was one basic issue, that is that you are acting only on the fact that a button is pressed.

Since you only delay 50 milliseconds after seeing a pressed button, it comes around again and sees the same button still being pressed. So that explains part of it.

When the game ends or (not sure) you make a mistake, there is no delay before it immediately starts over.

Here's your code, changed for the way I like to wire my buttons and with the addition of a delay here and some print statements there. No changes to the logic, which as I said I do not have what it takes at the moment to make the least sense of. I'll take half the blame.

I increased the delay after reading a pressed button to 133 milliseconds, long enough so the game logic functions if you quick-stab the buttons, that is to say hold them down for less than 133 milliseconds.

I commented out the randomizer you have in setup(). For now it is useful to have the exact same sequence play out every time.

You did

  Serial.begin(9600);

but made no use of Serial printing. This is an essential tool for seeing if the values of key variables is what you think and are properly informing the flow through your code. It costs nothing to be chatty.

Play with it in a different simulator here.

Your code, my mods:

This text will be hidden

// https://wokwi.com/projects/420448199508170753

int currentNum = 0;
int randomNum[100];
int counting = 0;
int userPress = 0;

const int led1 = 10;
const int led2 = 9;
const int led3 = 8;
const int led4 = 7;

const int kDelay = 133;

void setup()
{
  pinMode(2, INPUT_PULLUP);
  pinMode(3, INPUT_PULLUP);
  pinMode(4, INPUT_PULLUP);
  pinMode(5, INPUT_PULLUP);

  pinMode(7, OUTPUT);
  pinMode(8, OUTPUT);
  pinMode(9, OUTPUT);
  pinMode(10, OUTPUT);

  Serial.begin(9600);

// for now, use the sane sequence every time
// randomSeed(analogRead(A0));

  randomNum[currentNum] = random(7, 11);
}

void count()
{
  digitalWrite(randomNum[counting], HIGH);
  delay(500);

  digitalWrite(randomNum[counting], LOW);
  delay(500);

  counting++;
}

void user() {
# define PRESST LOW // my buttons are wired differently
  bool btn1 = digitalRead(5) == PRESST;
  bool btn2 = digitalRead(4) == PRESST;
  bool btn3 = digitalRead(3) == PRESST;
  bool btn4 = digitalRead(2) == PRESST;

  if (btn1 && randomNum[userPress] == led1) {
    userPress++;
    delay(kDelay);

  } else if (btn2 && randomNum[userPress] == led2) {
    userPress++;
    delay(kDelay);

  } else if (btn3 && randomNum[userPress] == led3) {
    userPress++;
    delay(kDelay);

  } else if (btn4 && randomNum[userPress] == led4) {
    userPress++;
    delay(kDelay);

  } else if (btn4 && randomNum[userPress] == !led4 ||
             btn3 && randomNum[userPress] == !led3 ||
             btn2 && randomNum[userPress] == !led2 ||
             btn1 && randomNum[userPress] == !led1) {
    counting = 0;
    userPress = 0;
    currentNum = 1;
Serial.println("Game Over? Restarting.");
    delay(1000);
  }
}

void loop()
{
  while (counting < currentNum) {
    count();
  }

  if (counting == currentNum) {
    counting = 0;

    while (userPress < currentNum) {
      user();
    }

Serial.println("now here, so...");
delay(777);

  }

  if (userPress == currentNum) {
    currentNum++;
    userPress = 0;
    randomNum[currentNum] = random(7, 11);
  }
}

The "right" way to handle this would mean many changes, bu the first thing to learn is how to react to the fact that a button became pressed,the act of pressing a button, not the fact that the button is pressed, which may just mean the player has left her fat finger on there too long. If you handle the button becoming pressed, it will solve the problem because it can only get pressed (again) if a finger leaves the button and returns.

This is called "state change detection" or "edge detection"; there is an example on off in the IDE and you can find a competent article that accompanies it.

Or I can

https://docs.arduino.cc/built-in-examples/digital/StateChangeDetection/

a7

fisheyyy · January 18, 2025, 8:04pm

Appreciate it man!

alto777 · January 18, 2025, 9:52pm

I forgot to post the link, it was in the code and I meant to

I will try to utnagle your logic and see how you made this function. I am completely serious when I say it is impressive and if you did write this, I think it means you have a talent for this.

It is sad that these days anything the least bit creative seems to be being blamed on chatGPT or other AI.

Please keep a copy of this. Visit it over as you learn more about programming and marvel.

Now - the feature I am missing is having the LEDs blink as I enter what the sequence has become…

And naturally you want punishment and reward to have their rolls.

It may get harder and harder to grow this, but keep figuring things out and start read read reading other ppls' code to see how some o]things work.

a7

fisheyyy · January 18, 2025, 10:37pm

Somehting like this, i still need more cosmetic features but that can wait

int currentRound = 0;
int randomNum[100];
int counting = 0;
int userPress = 0;

int lastbtn1State;
int lastbtn2State;
int lastbtn3State;
int lastbtn4State;

const int led1 = 10;
const int led2 = 9;
const int led3 = 8;
const int led4 = 7;
  
void setup()
{
  pinMode(2, INPUT);
  pinMode(3, INPUT);
  pinMode(4, INPUT);
  pinMode(5, INPUT);
  
  pinMode(7, OUTPUT);
  pinMode(8, OUTPUT);
  pinMode(9, OUTPUT);
  pinMode(10, OUTPUT);
  
  Serial.begin(9600);
  randomSeed(analogRead(A0));
  
  randomNum[currentRound] = random(7,11);
}

void count()
{	
  	delay(500);
	digitalWrite(randomNum[counting], HIGH);
  
	delay(500);
  	digitalWrite(randomNum[counting], LOW);
	

  	counting++;
}

void user()
{
  
	int btn1 = digitalRead(5);
	int btn2 = digitalRead(4);
	int btn3 = digitalRead(3);
	int btn4 = digitalRead(2);
  
	if(btn1 == HIGH && lastbtn1State == LOW && randomNum[userPress] == led1)
	{
    userPress++;

	}else if(btn2 == HIGH && lastbtn2State == LOW && randomNum[userPress] == led2)
	{
  	userPress++;

	}else if(btn3 == HIGH && lastbtn3State == LOW && randomNum[userPress] == led3)
	{
  	userPress++;
  
	}else if(btn4 == HIGH && lastbtn4State == LOW && randomNum[userPress] == led4)
	{
	userPress++;
    
	}else if((btn4 == HIGH && lastbtn4State == LOW && randomNum[userPress] != led4) ||
         (btn3 == HIGH && lastbtn3State == LOW && randomNum[userPress] != led3) ||
         (btn2 == HIGH && lastbtn2State == LOW && randomNum[userPress] != led2) ||
         (btn1 == HIGH && lastbtn1State == LOW && randomNum[userPress] != led1))
	{
  	counting = 0;
  	userPress = 0;
  	currentRound = 0;
    randomNum[currentRound] = random(7,11);
    Serial.println("game over");
  	delay(1000);
	}
  
    lastbtn1State = btn1;
  	lastbtn2State = btn2;
  	lastbtn3State = btn3;
  	lastbtn4State = btn4;
} 


void loop()
{
	while(counting < currentRound){
	count();
	}
  
	if(counting == currentRound)
    {
    counting = 0;
      
    while(userPress < currentRound){
    user();
  	}
 	}	
  
  	if(userPress == currentRound)
    {
    currentRound++;
    userPress = 0;
    randomNum[currentRound] = random(7, 11);  
  	} 
}

alto777 · January 18, 2025, 11:36pm

I see what you are doing there.

To be extra sure your methods will continue to work well, I suggest you to place an harmless delay into the code that looks at the buttons, viz:

void user()
{
  delay(50);  // poor man's debouncing

  int btn1 = digitalRead(5);

You are probably relying on delays elsewhere in you sketch to mask the indeterminate period a pushbutton can exhibit very briefly (milliseconds) when it is transitioning from one state to the other. This is called bounce and debouncing a switch goes hand in hand with state change detection.

Doing this way is a convenient hack, thus its name, and in some circumstances poor man's is perfectly viable. You may have seen it in the sketch at the site I linked:

    // Delay a little bit to avoid bouncing
    delay(50);

They don't cop to it being a bit kludgy. It does get the matter off the table, though.

a7

alto777 · January 19, 2025, 1:23pm

Now I have worked some on your sketch. I can't share it anymore as I made some extensive changes and don't want to deprive you of the fun of doing.

Suffice it to say arrays are your friend: whenever you have four of something and growing or maintaining your code involves copy/paste/editing, like to change the names of variables to make a code pattern do the same thing as one that exsits, you should start to think about arrays and functions.

I did not alter the basic structure of your sketch, which is sound. It is a bit hard to follow as the control is quite thoroughly distributed in the sketch and depends on global variables.

But... I did do two things I think yo umight consider.

The first is to completely uncouple the code (0, 1, 2, 3) from the pin numbers that the code numbers want to light up, and that the code numbers are correct by the buttons.

So my random element is selected to be 0 .. 3

  whatever = random(0, 4);

and all references to it adjusted.

So I compare the second button to the code being 1, not to the led2.

In a teeny sketch, it makes no difference. But in bigger projects , you should def never exploit the fact that four output pins are to be found at four consecutive integers.

I used an arrays

const byte leds[] = {7, 8, 9, 10};
const byte buttons[] = {2, 3, 4, 5};

or was it the other way around? In any case, everywhere else the buttons are numbered 0 .. 3, the lamps are number 0 .. 3 and the logic is less tightly bound to the implementation.

The other big but not small change was to move the LED blinker into a function, and arrange for that function to be called from your count() sequence playback mechanism.

Then, wait for it, I can call the same function from your user() solicit and confirm the correct choice code.

So I get what I wanted, feedback of the sequence as I enter it.

// blink LED at pin nn
void lampIt(byte nn)
{
  digitalWrite(nn, HIGH);
  delay(500);    // or speed, see below
  
  digitalWrite(nn, LOW);
  delay(500);
}

You could put a sound maker into that same function and add tones to the sequence. I assume you are familiar with Simon, that 20th century hit still going strong.

I put a few longer delays in some places to give some breathing room between things, especially when a game is over before the new game starts. Stuff like that.

I used a manifest constant for the delay in the LED blinker. I like it to go a bit faster, so this

const int speed = 333;

and delaying by that variable gives you the option to easily tune it to taste.

If instead you use a regular variable (no const qualifier), it would be easy to make the sequence go a bit faster now and again.

This has been fun, or at least what qualifies as fun in my empty life. I did note with some amusement the provision for sequences reaching 100 steps... you should have a test for that in the code and heap huge rewards and praise on anyone who succeeds in breaking the sketch by outdoing its fundamental limitation.

HTH

a7

system · July 18, 2025, 1:23pm

This topic was automatically closed 180 days after the last reply. New replies are no longer allowed.

Topic		Replies	Views
Adding buttons to my Simon Says game & general questions. Programming	24	6137	May 5, 2021
Alternating Pattern and user input in memory game Syntax & Programs	3	1950	May 6, 2021
Need help with game! Frequently-Asked Questions	13	3688	May 6, 2021
Memory game project Syntax & Programs	20	6229	May 6, 2021
2 LED Sequence Programming	19	5714	May 5, 2021

I am having an issue in my code making it not work correctly

Related topics