Function being called when it shouldn't?

So basically,

bool aCondition = false; bool anotherCondition = true;

void loop() { if (aCondition) { AFunction(); }

if (anotherCondition) { AFunction(); } }

void AFunction() { do something }

output: AFunction is all messed up. It works fine when I comment the call of AFunction out under the condition that shouldn't even be getting triggered. It's as if it is reading "aCondition" as true, but any other type of code under that if statement, such as adding 1 to an int, isn't reached. I think this may be some sort of memory issue or something but I don't have nearly enough experience to know.

I wanted to have one function to do many things, but if simply calling it in an if statement that shouldn't even be triggered messes it up, then what's the point of using functions? As you might be able to tell, I am somewhat of a noob so please use small words thank you.

Sorry, I forgot how to post code. It's quite long and mostly irrelevant to my problem I think anyway.

Post actual code that acts the way you describe.

How? When I just copy and paste it it exceeds the maximum allowable length for this forum. I don't really think it will help you. This seems to be a problem with how C++ works.

Try a sketch that almost looks like your pseudo code above, but make it a complete sketch.

Include the setup() function. Make AFunction() actually do something, maybe just have it print "hello" to the serial monitor.

It does not seem to be a problem with how C++ works; it seems to be a problem with how the code you didn't post works.

Post your code in code tags.

What are code tags? Like this?

Exactly

bool isLookingAtBigMap = false;
bool isLookingAtRecruitmentMenu = true

 void loop() //-------------------------------------------------------------------
{
  if (isLookingAtBigMap == true)
    {
      //MakeSelectionMenu(questionOne,2,0);
    }
  if (isLookingAtRecruitmentMenu == true)
  {
    if (isLookingAtArmy == false)
    {
    MakeSelectionMenu(unitTypes, unitTypesSize, 1); //the function which is being messed up
    }
    if (isLookingAtArmy == true)
    {
    ArmyBattleMenu(numOfInfantry,numOfSpearmen, numOfArchers, numOfCavalry, numOfPeasants);
    }
  }
}

This is the exact loop from the code I am talking about, notice the commented out "MakeSelectionMenu" function. The second call doesn't quite work correctly with it not commented out.

evanmars: It does not seem to be a problem with how C++ works; it seems to be a problem with how the code you didn't post works.

Yes.... I'm sure your code is perfect, and it's FAR more likely there's a problem with the compiler or the language itself. That makes perfect sense...

Really?

Your code has a bug, period. Without seeing your code, nobody here can possibly tell you what you are doing wrong. Your actual code, that compiles and run, and exhibits the actual bug. Not something sort, kinda like your actual code. Not some snippets of your actual code showing the parts YOU think are important. Actual, compilable code that people here can compile and run, and see the bug for themselves.

Regards, Ray L.

Ok, here is the first half:

 #include <Wire.h> 
#include <LiquidCrystal_I2C.h>

LiquidCrystal_I2C lcd(0x3f, 2,1,0,4,5,6,7,3,POSITIVE);

//global varibles
int turn = 0;

//player info

int numOfInfantry =0;
int numOfSpearmen=0;
int numOfArchers=0;
int numOfCavalry=0;
int numOfPeasants=0;

int currentXPos = 0;
int currentYPos = 1;

int money = 0;



//buttons
const int northB;
const int eastB = 9;
const int southB;
const int westB = 8;
const int northWestB;
const int northEastB;
const int southEastB;
const int southWestB;

const int enterB = 7;
const int cancelB = 10;

bool hasPushed = false;
bool makeList = true;


//variables
int cursorPos[2] = {0,0};

bool printBigMapMenu = true;
bool isLookingAtBigMap = false;
bool isLookingAtRecruitmentMenu = true;

bool hasMoved = false;


//various arrays
String unitTypes[6] = {"Infantry", "Spearmen", "Archers", "Cavalry", "Peasants", "see army"};
int unitTypesSize = 6;
char BigMapX[20] = {'A', 'B', 'C', 'D', 'E', 'F', 'G',
     'H', 'I', 'J', 'K', 'L', 'M', 'O', 'P', 'Q', 'R', 'S', 'T'};
int letterIndexer[20] = {0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19};     
int BigMapY[20] = {1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20};

String questionOne[2] = {"Move or", "camp?"};

int lineTrackerX = 0;
int lineTrackerY = 0;
int itemPosCounter = 0;
bool confirmSelect = false;
bool selecting = false;

//BATTLE VARIABLE
bool inBattle = false;
bool printArmy = false;
bool isLookingAtArmy = false;

byte Sword[] = {
  B00000,
  B00100,
  B00100,
  B00100,
  B00100,
  B01110,
  B00100,
  B00000  
};

byte BowAndArrow[] = {
  B00000,
  B00000,
  B00100,
  B01110,
  B10101,
  B01110,
  B00100,
  B00000  
};

byte Spear[] = {
  B00100,
  B01110,
  B00100,
  B00100,
  B00100,
  B00100,
  B00100,
  B00100  
};

byte Horse[] = {
  B00100,
  B01010,
  B01111,
  B01100,
  B11100,
  B11100,
  B11100,
  B11100  
};

byte Human[] = {
  B01110,
  B00100,
  B01110,
  B10101,
  B10101,
  B01010,
  B01010,
  B01010  
};

byte Clock[] = {
  B00000,
  B01110,
  B10101,
  B10111,
  B10001,
  B01110,
  B00000,
  B00000  
};

byte Money[] = {
  B00100,
  B11111,
  B10101,
  B11100,
  B00111,
  B10101,
  B11111,
  B00100  
};


void setup()
{
	// initialize the LCD
	lcd.begin(20,4);

  lcd.createChar(0, Sword);
  lcd.createChar(1, BowAndArrow);
  lcd.createChar(2, Spear);
  lcd.createChar(3, Horse);
  lcd.createChar(4, Human);
  lcd.createChar(5, Clock);
  lcd.createChar(6, Money);
  lcd.home();

 //initialize position
 currentXPos = letterIndexer[0];
 currentYPos = BigMapY[0];

 pinMode(eastB, INPUT_PULLUP);
 pinMode(westB, INPUT_PULLUP);
 pinMode(enterB, INPUT_PULLUP);
 pinMode(cancelB, INPUT_PULLUP);
 
 
//lcd.write(byte(5));
//lcd.write(255);
//DisplayKeyCodes();
}

void loop() //-------------------------------------------------------------------
{
  if (isLookingAtBigMap == true)
    {
      //BigMapMenu();
    }
  if (isLookingAtRecruitmentMenu == true)
  {
    if (isLookingAtArmy == false)
    {
    MakeSelectionMenu(unitTypes, unitTypesSize, 1); //1 to print last element in bottom corner as button. 0 to print array as normal
    }
    if (isLookingAtArmy == true)
    {
    ArmyBattleMenu(numOfInfantry,numOfSpearmen, numOfArchers, numOfCavalry, numOfPeasants);
    }
  }
}

And the second:

void BigMapMenu()
{
  //see army option
  //see territory option
  
  //display money
  //display army size
  //display position
  //display points
  
  //leave or stay in town if in one
  //choose to move or camp
  //choose scout direction
  //see town options if on one/alert player of nearby opponent
  //end turn

  /*if (printBigMapMenu == true)
  {
    
    //bottom line list stats
   lcd.setCursor(0,3);
   lcd.print(BigMapX[currentXPos]);
   lcd.print(currentYPos);

   lcd.setCursor(4,3);
   lcd.write(byte(5));
   lcd.print(turn);

   lcd.setCursor(9,3);
   lcd.write(byte(0));
   lcd.print(numOfInfantry+numOfSpearmen+numOfArchers+numOfCavalry+numOfPeasants);

   lcd.setCursor(14,3);
   lcd.write(byte(6));
   lcd.print(money);

   printBigMapMenu = false;
  } */

   if (hasMoved == false)
  {
     MakeSelectionMenu(questionOne,2,0);
  }
}

char MoveOnBigMap(int xDir, int yDir)
{
  if (currentXPos + xDir > 19 || currentYPos + yDir > 20
     || currentXPos + xDir < 0 || currentYPos + yDir < 1)
     {
      char output[] = "invalid position";
      return output;
     }
    else
    {
      currentXPos += xDir;
      currentYPos += yDir;         
      char output2[2] = {BigMapX[currentXPos],currentYPos};
      return output2;
    }    
}


void MakeSelectionMenu(String stringArray[], int sizeOfArray, int separateLastItem)
{
  int itemPlacesX[sizeOfArray];
  int itemPlacesY[sizeOfArray];
  
  if (makeList == true)
  {
   for (int i = 0; i< sizeOfArray; i++)
   {
    if (i<sizeOfArray-separateLastItem)//to print the last item in the bottom corner as a "see army" button
    {
     if (stringArray[i].length() + 2 + lineTrackerX >20)
     {
      lineTrackerX = 0;
      lineTrackerY +=1;
     }
     lcd.setCursor(lineTrackerX,lineTrackerY);
     lcd.print(stringArray[i]);
     itemPlacesX[i] = lineTrackerX;
     itemPlacesY[i] = lineTrackerY;
     lineTrackerX += stringArray[i].length()+2;
     if (i<(sizeOfArray-separateLastItem)-1)
     {
      lcd.print(", ");
     }
    }
   else if (i<sizeOfArray && separateLastItem >0)//print the see army button
    {
      itemPlacesX[i] = 12;
      itemPlacesY[i] = 3;
      lcd.setCursor(12,3);
      lcd.print(stringArray[i]);
    }
   }
   makeList = false;
   lcd.blink();
  } 

  
  //-------------------------------------------------------------------------------------------
  
  
  if (digitalRead(eastB) == LOW && hasPushed == false && confirmSelect == false)
  {
    //-------------------------------------------------------------
    if (itemPosCounter<sizeOfArray-1)
    {
      hasPushed = true;
      itemPosCounter+=1;
      cursorPos[0] = itemPlacesX[itemPosCounter];
      cursorPos[1] = itemPlacesY[itemPosCounter];
      delay(175);
      hasPushed = false;
    }
  }
  //---------------------------------------------------------------------
   if (digitalRead(westB) == LOW && hasPushed == false && confirmSelect == false)
  {
    if (itemPosCounter>0)
    {
      hasPushed = true;
      itemPosCounter-=1;
      cursorPos[0] = itemPlacesX[itemPosCounter];
      cursorPos[1] = itemPlacesY[itemPosCounter];
      delay(175);   
      hasPushed = false;    
    }
  }
//----------------------------------------------------------
  if (digitalRead(enterB) == LOW && hasPushed == false && confirmSelect == false 
     && itemPosCounter < sizeOfArray-1)
  {
    hasPushed = true;
    confirmSelect = true;
    lcd.clear();
    lcd.noBlink();
    lcd.noCursor();
    
    lcd.setCursor(0,0);
    lcd.print("Are you sure you "); 
    lcd.setCursor(0,1);
    lcd.print("want to recruit "); 
    lcd.setCursor(0,2);
    lcd.print(stringArray[itemPosCounter] + "?"); 
    
    delay(175);   
    hasPushed = false;
  }
  //--------------------------------------------------------------
  if (digitalRead(enterB) == LOW && hasPushed == false && confirmSelect == false 
     && itemPosCounter == sizeOfArray-1) //if it's the last element in array
  {
    hasPushed = true;
    confirmSelect = true;
    lcd.clear();
    lcd.noBlink();
    lcd.noCursor();
    
    isLookingAtArmy = true;//specifically for looking at army
    
    delay(175);   
    hasPushed = false;
  }
 //---------------------------------------------------------------- 
  if (digitalRead(cancelB) == LOW && hasPushed == false && confirmSelect == true)
    {
      hasPushed = true;
      lcd.clear();
      lineTrackerX = 0;
      lineTrackerY = 0;
      makeList = true;
      delay(175);   
      hasPushed = false;
      confirmSelect = false;
    }
//-----------------------------------------------
    if (digitalRead(enterB) == LOW && hasPushed == false && confirmSelect == true)
    {
      hasPushed = true;
      
      if (stringArray[itemPosCounter] == "Infantry")
      {
        numOfInfantry +=1;
      }
      if (stringArray[itemPosCounter] == "Spearmen")
      {
        numOfSpearmen +=1;
      }
      if (stringArray[itemPosCounter] == "Archers")
      {
        numOfArchers +=1;
      }
      if (stringArray[itemPosCounter] == "Cavalry")
      {
        numOfCavalry +=1;
      }
      if (stringArray[itemPosCounter] == "Peasants")
      {
        numOfPeasants +=1;
      }
      lcd.clear();
      lineTrackerX = 0;
      lineTrackerY = 0;
      makeList = true;
      delay(175);   
      hasPushed = false;
      confirmSelect = false;
    }
    //----------------------------------------------
  lcd.setCursor(cursorPos[0],cursorPos[1]);
}

//---------------------------------------------------------------------------------------------

void ArmyBattleMenu(int inf, int spr, int arc, int cav, int pes)//used both during gameplay to
//review army size and during battle to keep track of your units.
{
  if (printArmy == false)
  {
    printArmy = true;
    lcd.clear();
    lcd.setCursor(0,0);
    lcd.print(inf);
    lcd.print(" INF ");
    lcd.write(byte(0));
    
    lcd.setCursor(11, 0);
    lcd.print(spr);
    lcd.print(" SPR ");
    lcd.write(byte(2));
    
    lcd.setCursor(0,1);
    lcd.print(arc);
    lcd.print(" ARC ");
    lcd.write(byte(1));
    
    lcd.setCursor(11,1);
    lcd.print(cav);
    lcd.print(" CAV ");
    lcd.write(byte(3));
    
    lcd.setCursor(0,2);
    lcd.print(pes);
    lcd.print(" PES ");
    lcd.write(byte(4));
  }
  
  if (inBattle == true)
  {
    
  }
}

To be quite specific, when the calling of “MakeSelectionMenu” under the “if (isLookingAtBigMap == true)” is NOT commented out, either the cursorPos or one of the itemPLaces is off. Everything else works fine, it will even select the right object depending on the itemPos. Works fine with it commented out.

There are several places you do something like this:

    char someArray[2] = {};
    return someArray;

That array is local variable to the function in which it is created. It is created on the stack, with the functions stack frame. When the return statement is executed, the stack frame is discarded, and the variable can no longer be guaranteed to be valid as soon as the return statement is executed, as that area of the stack can be clobbered by any interrupt.

If you need to return an array, or other complex data type, from a function, you must either create it on the heap, or, better, let the caller provide the array, and pass it as either a pointer or reference as an argument to the function.

Also, you cannot declare a function to return a char, but then actually return a pointer to an array of char...

Regards, Ray L.

Thank you for the reply. I don't get it. None of my functions are returning anything, they are all void, they just change variables. And it works fine, it is only when I call it a second time under an if statement that shouldn't even get reached. And I can't define the array in the function itself or else it will constantly be reset in the loop.

How would one put it in the "heap?"

EDIT: Oh that function? I'm not even using that right now and haven't really looked at it or tested it enough to know that's it's not right. This is why I didn't want to show the whole thing, there's a lot going on that is completely irrelevant to my problem and I'm sure I'm making bad practices all over the place. The function I am having a problem with is void.

I’ve simplified the code so none of you get sidetracked by my many other bad practices and mistakes. I simplified the problem down to its most essential elements and the problem is still there.

You will see only relevant variables, the start and loop, and the singular function that is being messed up. Yes, I tested this. If theoretically you have a 20x4 lcd display and a button in pin 9 this will work with no modification. The loop is where the problem is.

 #include <Wire.h> 
#include <LiquidCrystal_I2C.h>

LiquidCrystal_I2C lcd(0x3f, 2,1,0,4,5,6,7,3,POSITIVE);


const int eastB = 9;
const int westB = 8;
const int enterB = 7;
const int cancelB = 10;

int cursorPos[2] = {0,0};

bool hasPushed = false;
bool makeList = true;

String unitTypes[6] = {"Infantry", "Spearmen", "Archers", "Cavalry", "Peasants", "see army"};
int unitTypesSize = 6;
String questionOne[2] = {"Move or", "camp?"};

int lineTrackerX = 0;
int lineTrackerY = 0;
int itemPosCounter = 0;
bool confirmSelect = false;
bool selecting = false;

bool isLookingAtBigMap = false;
bool isLookingAtRecruitmentMenu = true;
bool isLookingAtArmy = false;

int numOfInfantry =0;
int numOfSpearmen=0;
int numOfArchers=0;
int numOfCavalry=0;
int numOfPeasants=0;

void setup()
{
 lcd.begin(20,4);
  
 pinMode(eastB, INPUT_PULLUP);
 pinMode(westB, INPUT_PULLUP);
 pinMode(enterB, INPUT_PULLUP);
 pinMode(cancelB, INPUT_PULLUP);

 lcd.home();
}




void loop() //-------------------------------------------------------------------
{
  if (isLookingAtBigMap == true) //isLookingAtBigMap is FALSE
    {
      MakeSelectionMenu(questionOne, 2, 0); //if this call is commented out, everything works fine
    }
    
  if (isLookingAtRecruitmentMenu == true)
    { 
    MakeSelectionMenu(unitTypes, unitTypesSize, 1); //function being slightly messed up
    }
}





void MakeSelectionMenu(String stringArray[], int sizeOfArray, int separateLastItem)
{
  int itemPlacesX[sizeOfArray];
  int itemPlacesY[sizeOfArray];
  
  if (makeList == true)
  {
   for (int i = 0; i< sizeOfArray; i++)
   {
    if (i<sizeOfArray-separateLastItem)//to print the last item in the bottom corner as a "see army" button
    {
     if (stringArray[i].length() + 2 + lineTrackerX >20)
     {
      lineTrackerX = 0;
      lineTrackerY +=1;
     }
     lcd.setCursor(lineTrackerX,lineTrackerY);
     lcd.print(stringArray[i]);
     itemPlacesX[i] = lineTrackerX;
     itemPlacesY[i] = lineTrackerY;
     lineTrackerX += stringArray[i].length()+2;
     if (i<(sizeOfArray-separateLastItem)-1)
     {
      lcd.print(", ");
     }
    }
   else if (i<sizeOfArray && separateLastItem >0)//print the see army button
    {
      itemPlacesX[i] = 12;
      itemPlacesY[i] = 3;
      lcd.setCursor(12,3);
      lcd.print(stringArray[i]);
    }
   }
   makeList = false;
   lcd.blink();
  } 

  
  //-------------------------------------------------------------------------------------------
  
  
  if (digitalRead(eastB) == LOW && hasPushed == false && confirmSelect == false)
  {
    //-------------------------------------------------------------
    if (itemPosCounter<sizeOfArray-1)
    {
      hasPushed = true;
      itemPosCounter+=1;
      cursorPos[0] = itemPlacesX[itemPosCounter];
      cursorPos[1] = itemPlacesY[itemPosCounter];
      delay(175);
      hasPushed = false;
    }
  }
  //---------------------------------------------------------------------
   if (digitalRead(westB) == LOW && hasPushed == false && confirmSelect == false)
  {
    if (itemPosCounter>0)
    {
      hasPushed = true;
      itemPosCounter-=1;
      cursorPos[0] = itemPlacesX[itemPosCounter];
      cursorPos[1] = itemPlacesY[itemPosCounter];
      delay(175);   
      hasPushed = false;    
    }
  }
//----------------------------------------------------------
  if (digitalRead(enterB) == LOW && hasPushed == false && confirmSelect == false 
     && itemPosCounter < sizeOfArray-1)
  {
    hasPushed = true;
    confirmSelect = true;
    lcd.clear();
    lcd.noBlink();
    lcd.noCursor();
    
    lcd.setCursor(0,0);
    lcd.print("Are you sure you "); 
    lcd.setCursor(0,1);
    lcd.print("want to recruit "); 
    lcd.setCursor(0,2);
    lcd.print(stringArray[itemPosCounter] + "?"); 
    
    delay(175);   
    hasPushed = false;
  }
  //--------------------------------------------------------------
  if (digitalRead(enterB) == LOW && hasPushed == false && confirmSelect == false 
     && itemPosCounter == sizeOfArray-1) //if it's the last element in array
  {
    hasPushed = true;
    confirmSelect = true;
    lcd.clear();
    lcd.noBlink();
    lcd.noCursor();
    
    isLookingAtArmy = true;//specifically for looking at army
    
    delay(175);   
    hasPushed = false;
  }
 //---------------------------------------------------------------- 
  if (digitalRead(cancelB) == LOW && hasPushed == false && confirmSelect == true)
    {
      hasPushed = true;
      lcd.clear();
      lineTrackerX = 0;
      lineTrackerY = 0;
      makeList = true;
      delay(175);   
      hasPushed = false;
      confirmSelect = false;
    }
//-----------------------------------------------
    if (digitalRead(enterB) == LOW && hasPushed == false && confirmSelect == true)
    {
      hasPushed = true;
      
      if (stringArray[itemPosCounter] == "Infantry")
      {
        numOfInfantry +=1;
      }
      if (stringArray[itemPosCounter] == "Spearmen")
      {
        numOfSpearmen +=1;
      }
      if (stringArray[itemPosCounter] == "Archers")
      {
        numOfArchers +=1;
      }
      if (stringArray[itemPosCounter] == "Cavalry")
      {
        numOfCavalry +=1;
      }
      if (stringArray[itemPosCounter] == "Peasants")
      {
        numOfPeasants +=1;
      }
      lcd.clear();
      lineTrackerX = 0;
      lineTrackerY = 0;
      makeList = true;
      delay(175);   
      hasPushed = false;
      confirmSelect = false;
    }
    //----------------------------------------------
  lcd.setCursor(cursorPos[0],cursorPos[1]);
}

Again, while scrolling through the list, either the cursorPos or one of the itemPos is getting thrown off. When I comment out the other call in the loop (that shouldn’t even be reached), it all works perfectly.