   bool function not running all the way through, automatically returning true
  Howdy everyone. I have recently got back into programming and I'm trying to learn c++ in a lot more depth than I ever have before(which wasn't very much)Basically what I'm doing is reading a couple books and as I learn new things applying the to an ongoing program I'm working on. Currently I'm having trouble with a function I'm calling not "going away" after its done being executed. It also not doing what it should and returning a value it should not. By "going away" I mean clearing from memory. I'm guessing this is what the problem is but maybe someone out there can see an obvious flaw that I'm not catching. //movement.cpp//both of these functions are declared in movement.h#include <iostream>#include <stdlib.h>#include <time.h>#include <iomanip>#include <stdio.h>#include "globalTWILIGHT.h" #include "movement.h"//function to see if asteroid is present returns 0 if sobool asteroidPRESENT (int moveFROM){std::cout << "RUNNING asteroidPRESENT FUNCTION" << std::endl;/*looprun through each tile placement spot besides the one givencheck xz against move from to see if adjacent to eachotherthen check to see if adjacent location has asteroid or supernova(if there is no tile there it will be empty and set to 0)if asteroid or supernova presentthenc check to see if any of the tiles in hand are not a supernova or asteroid (this is done back in board placement function)if so then cant place tile hereif all tiles in hand are supernova or asteroid then success and move on to next check*/for (int xz = 1; xz < 38; xz++) //run through all tiles on board, except the one that is being questioned{if (xz == moveFROM) continue; if (adjacentCHECK(moveFROM, xz)) //checks to see if the tile sent to the function is adjacent to x(tile being tested) {//tileCONTENT is a global not declared here!!! if (tileCONTENT[xz][4] == 1 || tileCONTENT[xz][5] == 1)//if the tile sent to this function is adjacent to the tile in question(x) this line makes sure its not an asteroid or nova {std::cout << xz << "Here is function asteroid present right before the return of true" << std::endl; return true; //if it is i returns 0 } } std::cout << "RUNNING FOR LOOP " << xz << std::endl;}std::cout << "ENDING asteroidPRESENT FUNCTION" << std::endl;return false; //otherwise return false as there is no asteroid present}//function to see if the spot requested is adjacent returns false if cant be donebool adjacentCHECK (int spotFROM, int spotTO){std::cout << "RUNNING adjacentCHECK function with coordinates " << spotFROM << " " << spotTO << std::endl;// need to setup ifs for if there are 2 or 3 players   //totalPLAYERS is a global not declared here!if (totalPLAYERS == 4 || totalPLAYERS == 5 || totalPLAYERS == 6){//this should all be a big switch here with spot from, but I changed it to a bunch of if's with worries that //the ifs inside each case statement would be more nested ifs then the compiler could handle. Probably not the case//but I was desperate to find out what was wrong. if (spotFROM ==1) {if (spotTO == 5 || spotTO == 6 || spotTO == 2)std::cout<< "Returning " << spotFROM << " and " << spotTO << std::endl;return true;}if (spotFROM ==2){if (spotTO == 1 || spotTO == 6 || spotTO == 7 || spotTO == 3)std::cout<< "Returning " << spotFROM << " and " << spotTO << std::endl;return true;}if (spotFROM ==3){if (spotTO == 2 || spotTO == 7 || spotTO == 8 || spotTO == 4)std::cout<< "Returning " << spotFROM << " and " << spotTO << std::endl;return true;}if (spotFROM ==4){if (spotTO == 8 || spotTO == 0)std::cout<< "Returning " << spotFROM << " and " << spotTO << std::endl;return true;}if (spotFROM ==5){if (spotTO == 1 || spotTO == 6 || spotTO == 11 || spotTO == 10)std::cout<< "Returning " << spotFROM << " and " << spotTO << std::endl;return true;}if (spotFROM ==6){if (spotTO == 1 || spotTO == 5 || spotTO == 11 || spotTO == 12 || spotTO == 7 || spotTO == 2)std::cout<< "Returning " << spotFROM << " and " << spotTO << std::endl;return true;}if (spotFROM ==7){if (spotTO == 2 || spotTO == 6 || spotTO == 12 || spotTO == 13 || spotTO == 8 || spotTO == 3)std::cout<< "Returning " << spotFROM << " and " << spotTO << std::endl;return true;}if (spotFROM ==8){if (spotTO == 3 || spotTO == 7 || spotTO == 13 || spotTO == 14 || spotTO == 9 || spotTO == 4)std::cout<< "Returning " << spotFROM << " and " << spotTO << std::endl;return true;}if (spotFROM ==9){if (spotTO == 4 || spotTO == 8 || spotTO == 14 || spotTO == 15)std::cout<< "Returning " << spotFROM << " and " << spotTO << std::endl;return true;}if (spotFROM ==10){if (spotTO == 5 || spotTO == 11 || spotTO == 17 || spotTO == 16)std::cout<< "Returning " << spotFROM << " and " << spotTO << std::endl;return true;}if (spotFROM ==11){if (spotTO == 5 || spotTO == 10 || spotTO == 17 || spotTO == 18 || spotTO == 12 || spotTO == 6)std::cout<< "Returning " << spotFROM << " and " << spotTO << std::endl;return true;}if (spotFROM ==12){if (spotTO == 6 || spotTO == 11 || spotTO == 18 || spotTO == 19 || spotTO == 13 || spotTO == 7)std::cout<< "Returning " << spotFROM << " and " << spotTO << std::endl;return true;}if (spotFROM ==13){if (spotTO == 7 || spotTO == 12 || spotTO == 19 || spotTO == 20 || spotTO == 14 || spotTO == 8)std::cout<< "Returning " << spotFROM << " and " << spotTO << std::endl;return true;}if (spotFROM ==14){if (spotTO == 8 || spotTO == 13 || spotTO == 20 || spotTO == 21 || spotTO == 15 || spotTO == 9)std::cout<< "Returning " << spotFROM << " and " << spotTO << std::endl;return true;}if (spotFROM ==15){if (spotTO == 9 || spotTO == 14 || spotTO == 21 || spotTO == 22)std::cout<< "Returning " << spotFROM << " and " << spotTO << std::endl;return true;}if (spotFROM ==16){if (spotTO == 10 || spotTO == 17 || spotTO == 23)std::cout<< "Returning " << spotFROM << " and " << spotTO << std::endl;return true;}if (spotFROM ==17){if (spotTO == 10 || spotTO == 16 || spotTO == 23 || spotTO == 24 || spotTO == 18 || spotTO == 11)std::cout<< "Returning " << spotFROM << " and " << spotTO << std::endl;return true;}if (spotFROM ==18){if (spotTO == 11 || spotTO == 17 || spotTO == 24 || spotTO == 25 || spotTO == 19 || spotTO == 12)std::cout<< "Returning " << spotFROM << " and " << spotTO << std::endl;return true;}if (spotFROM ==19){if (spotTO == 12 || spotTO == 18 || spotTO == 25 || spotTO == 26 || spotTO == 20 || spotTO == 13)std::cout<< "Returning " << spotFROM << " and " << spotTO << std::endl;return true;}if (spotFROM ==20){if (spotTO == 13 || spotTO == 19 || spotTO == 26 || spotTO == 27 || spotTO == 21 || spotTO == 14)std::cout<< "Returning " << spotFROM << " and " << spotTO << std::endl;return true;}if (spotFROM ==21){if (spotTO == 14 || spotTO == 20 || spotTO == 27 || spotTO == 28 || spotTO == 22 || spotTO == 15)std::cout<< "Returning " << spotFROM << " and " << spotTO << std::endl;return true;}if (spotFROM ==22){if (spotTO == 15 || spotTO == 21 || spotTO == 28)std::cout<< "Returning " << spotFROM << " and " << spotTO << std::endl;return true;}if (spotFROM ==23){if (spotTO == 16 || spotTO == 17 || spotTO == 24 || spotTO == 29)std::cout<< "Returning " << spotFROM << " and " << spotTO << std::endl;return true;}if (spotFROM ==24){if (spotTO == 17 || spotTO == 23 || spotTO == 29 || spotTO == 30 || spotTO == 25 || spotTO == 18)std::cout<< "Returning " << spotFROM << " and " << spotTO << std::endl;return true;}if (spotFROM ==25){if (spotTO == 18 || spotTO == 24 || spotTO == 30 || spotTO == 31 || spotTO == 26 || spotTO == 19)std::cout<< "Returning " << spotFROM << " and " << spotTO << std::endl;return true;}if (spotFROM ==26){if (spotTO == 19 || spotTO == 25 || spotTO == 31 || spotTO == 32 || spotTO == 27 || spotTO == 20)std::cout<< "Returning " << spotFROM << " and " << spotTO << std::endl;return true;}if (spotFROM ==27){if (spotTO == 20 || spotTO == 26 || spotTO == 32 || spotTO == 33 || spotTO == 28 || spotTO == 21)std::cout<< "Returning " << spotFROM << " and " << spotTO << std::endl;return true;}if (spotFROM ==28){if (spotTO == 22 || spotTO == 21 || spotTO == 27 || spotTO == 33)std::cout<< "Returning " << spotFROM << " and " << spotTO << std::endl;return true;}if (spotFROM ==29){if (spotTO == 23 || spotTO == 24 || spotTO == 30 || spotTO == 34)std::cout<< "Returning " << spotFROM << " and " << spotTO << std::endl;return true;}if (spotFROM ==30){if (spotTO == 24 || spotTO == 29 || spotTO == 34 || spotTO == 35 || spotTO == 31 || spotTO == 25)std::cout<< "Returning " << spotFROM << " and " << spotTO << std::endl;return true;}if (spotFROM ==31){if (spotTO == 25 || spotTO == 30 || spotTO == 35 || spotTO == 36 || spotTO == 32 || spotTO == 26)std::cout<< "Returning " << spotFROM << " and " << spotTO << std::endl;return true;}if (spotFROM ==32){if (spotTO == 26 || spotTO == 31 || spotTO == 36 || spotTO == 37 || spotTO == 33 || spotTO == 27)std::cout<< "Returning " << spotFROM << " and " << spotTO << std::endl;return true;}if (spotFROM ==33){if (spotTO == 28 || spotTO == 27 || spotTO == 32 || spotTO == 37)std::cout<< "Returning " << spotFROM << " and " << spotTO << std::endl;return true;}if (spotFROM ==34){if (spotTO == 29 || spotTO == 30 || spotTO == 35)std::cout<< "Returning " << spotFROM << " and " << spotTO << std::endl;return true;}if (spotFROM ==35){if (spotTO == 34 || spotTO == 30 || spotTO == 31 || spotTO == 36)std::cout<< "Returning " << spotFROM << " and " << spotTO << std::endl;return true;}if (spotFROM ==36){if (spotTO == 35 || spotTO == 31 || spotTO == 32 || spotTO == 37)std::cout<< "Returning " << spotFROM << " and " << spotTO << std::endl;return true;}if (spotFROM ==37){if (spotTO == 36 || spotTO == 32 || spotTO == 33)std::cout<< "Returning " << spotFROM << " and " << spotTO << std::endl;return true;}  return false;}return false;}What happens is:I'll call the asteroidPRESENT function. xz will only get as high as the first time asteroidPRESENT returnedTRUE. basicallywhenever I call the asteroidPRESENT function again, the tileCONTENT function will always stop at the same place it stopped the very first time(xz) asteroidPRESENT was called. So asteroid present will always return true. The weird thing is if you see inside the adjacentCHECK function it will print two lines. The first being the coordinates and the second being what is being returned. If it does not print the second line, it should return false. What is actually happening is when it gets to that same xz point it stopped at the very first time asteroidPRESENT was called it displays the following output. I'll use asteroidPRESENT(26)RUNNING asteroidPRESENT FUNCTIONRUNNING adjacentCHECK function with coordinates 26 1RUNNING FOR LOOP 1RUNNING adjacentCHECK function with coordinates 26 2RUNNING FOR LOOP 2RUNNING adjacentCHECK function with coordinates 26 3RUNNING FOR LOOP 3RUNNING adjacentCHECK function with coordinates 26 4RUNNING FOR LOOP 4RUNNING adjacentCHECK function with coordinates 26 5RUNNING FOR LOOP 5RUNNING adjacentCHECK function with coordinates 26 6RUNNING FOR LOOP 6RUNNING adjacentCHECK function with coordinates 26 7RUNNING FOR LOOP 7RUNNING adjacentCHECK function with coordinates 26 8RUNNING FOR LOOP 8RUNNING adjacentCHECK function with coordinates 26 9RUNNING FOR LOOP 9RUNNING adjacentCHECK function with coordinates 26 10RUNNING FOR LOOP 10RUNNING adjacentCHECK function with coordinates 26 11RUNNING FOR LOOP 11RUNNING adjacentCHECK function with coordinates 26 1212See this is like my 10th run trying this function during this debug session. Ten runs ago I entered 13 instead of 26 into the asteroidPRESENT function. This would have returned true since inside adjacentCHECK:if (spotFROM ==12){if (spotTO == 6 || spotTO == 11 || spotTO == 18 || spotTO == 19 || spotTO == 13 || spotTO == 7)std::cout<< "Returning " << spotFROM << " and " << spotTO << std::endl;return true;}That is okay, but given the above output its like it never changed the spotFROM value when the function went away. IDK I may be over explaining at this point. If this doesnt make sense let me know and I'll try to make it clearer. Thanks for any help you can give   
  I think your problem is those if blocks, the ones that look like this:if (spotFROM ==1) {if (spotTO == 5 || spotTO == 6 || spotTO == 2)std::cout<< "Returning " << spotFROM << " and " << spotTO << std::endl;return true;}If you indent them properly then you can see what is going on:if (spotFROM == 1) {    if (spotTO == 5 || spotTO == 6 || spotTO == 2)   std::cout << "Returning " << spotFROM << " and " << spotTO << std::endl;    return true;}Since there are no {}s after the second if block, the "if (spotTO == 5 || spotTO == 6 || spotTO == 2)" part only applies to the next statement (the one that prints some output). Then no matter what, you return true if spotFROM is 1. Ditto for the other blocks like that.So I'd recommend that you learn to be really strict about good indentation. This is something that all programmers do. This bug would have been a lot more obvious with good indentation.Also, some people have a rule that they always put {}s after an 'if', 'while' or 'for', because omitting them often leads to bugs (like this one). I don't follow that rule myself but you might consider it.Some other things..//totalPLAYERS is a global not declared here!One common thing is to have a special way to name things that are globals, so that it's obvious and you don't have to write comments about it. You could put a lower-case g in front of them: gTotalPlayers.//this should all be a big switch here with spot from, but I changed it to a bunch of if's with worries that //the ifs inside each case statement would be more nested ifs then the compiler could handle. Probably not the case//but I was desperate to find out what was wrong. Yeah that is crazy talk. The compiler can handle whatever you throw at it.   ;  Im just taking a stab at this without having really dug too deep butif(spotFROM ==12){  if(spotTO == 6 || spotTO == 11 || spotTO == 18 || spotTO == 19 || spotTO == 13 || spotTO == 7)std::cout<< "Returning " << spotFROM << " and " << spotTO << std::endl;return true;}first make sure totalPLAYERS is 4, 5 or 6, or all these ifs will do nothing. You seem to have handled each possible value of spotFROM individually (brutal..) but assuming they are all handled, as soon as the "if" for spotFROM is reached, it will always return true, regardless of whether the inner if condition is hit (which it seemingly never is for any number from what your saying) so double check your inner "if" statements and make sure they are correct. Maybe you want something more like if(spotFROM == 12){if(spotTO == 6 || spotTO == 11 || spotTO == 18 || spotTO == 19 || spotTO == 13 || spotTO == 7) {std::cout<< "Returning " << spotFROM << " and " << spotTO << std::endl;return true;}return false;};  Man that was it. Thanks so for much for the coding tips as well. I will definately practice good indentation and naming of the globals. I skipped right over that bug when I changed the switch to a bunch of ifs because I didnt even look at the if spotTO part of things. I had a hunch it was something like this the whole time, but I swore I went over it in depth enough. Guess the lesson here is is to use proper indentation and ask for help 6 hours earlier :)Thanks so much again!!!EDIT: Also thanks NumberXaero for the heads up on the totalPLAYERS.   ;  Stop.What exactly is the logic here? What do all these possible values for spotFROM and spotTO mean?And why, when you're checking whether two spots are adjacent to each other, does it matter how many players there are?Also:1) There is no such thing as stdlib.h, time.h or stdio.h in C++. You want cstdlib, ctime and cstdio respectively. Except that you don't want cstdlib or cstdio, because you aren't going to use them for anything (because you're already properly using iostream to handle I/O, and aren't doing any text manipulation).2) Array indices start at 0, not 1. It is highly likely, based on the given code, that you are handling memory in an invalid way. (Although I don't see the tileCONTENT declaration, so I can't verify that).3) Where does the number 38 come from?4) Instead of 'continue'ing every time through the loop past a certain point (moveFROM), you could 'break'. Or better yet, only loop up to moveFROM in the first place. If I ask you to count to a hundred, you don't mentally count to a million and just stop saying the numbers after 100, right?5) Have you considered using some kind of structure to represent tile data instead of an array?   ;Quote:Original post by Zahlman4) Instead of 'continue'ing every time through the loop past a certain point (moveFROM), you could 'break'. Or better yet, only loop up to moveFROM in the first place. If I ask you to count to a hundred, you don't mentally count to a million and just stop saying the numbers after 100, right?To be fair to him he's only continuing on the moveFROM item, not every one afterwards as well.   ;  Er, sorry, misread that. :)   ;  Hey Zahlman thanks for the reply. First off the reason I need the amount of players to see if two tiles are adjacent is because the board changes in size depending on players(for game balance). spotFROM and spotTO are named that way because this is actually just in the setu phase of the game. After that I will use this function to check and make sure a unit can move from the spot its on(spotFROM0 to the spot requested(spotTO)Thanks for the heads up on the headers. I'll take em out. Still a little confused on what include files to use, but basically I'm sticking to if I dont use it in that file dont use the includeAs for my Arrays- your prolly gonna hate me for this. I initialize a lot of my arrays in this program assuming I'm not even going to use positions [0] or [0,0]From my what I have experienced this prevents me from having to edit the user input to - 1, if I'm using that number from user input to access a certain array slot. For example in playerRACE[7] , playerRACE[1] = any of the available races. Since its in [1] the program knows that is player 1. Basically its a memory wasting workaround to get away from the fence post bug. 37 is the maximum amount of tiles that could be checked. You just helped me realize it would make more sense to have a standard way of writing these 'for' loops to say <= 37. That way I'll recognize it better. As far as the structure question: Right now I'm getting into classes (which are like structures for c++?). I'm not having so much trouble on how to declare and use classes, I just really cant see any reason why. I gotta read through the chapters again. Maybe something will hit more that will make it very clear why classes will make my life easier. Thanks for all the questions and reviewing my code in depth. Sorry for the late reply, was up all night continuing the code after my lil bug got fixed.   ;Quote:As for my Arrays- your prolly gonna hate me for this. I initialize a lot of my arrays in this program assuming I'm not even going to use positions [0] or [0,0]From my what I have experienced this prevents me from having to edit the user input to - 1, if I'm using that number from user input to access a certain array slot. For example in playerRACE[7] , playerRACE[1] = any of the available races. Since its in [1] the program knows that is player 1. Basically its a memory wasting workaround to get away from the fence post bug.Don't do this. Array indexing in C++ is zero-based; if you try to use one-based indexing instead, you'll just end up fighting the language (and the standard library), introducing bugs, and confusing anyone who tries to read your code.You can still use one-based indexing for, say, user input, but instead of keeping the one-based index and jumping through a lot of hoops to make the rest of your code work with it, convert the input immediately to a zero-based index, and write the rest of your code naturally. That way, there's only a single point where you have to adjust for differing conventions, instead of many.(One other thing: your idea that using one-based indexing will help you avoid fencepost errors is somewhat confused. The idioms that programmers - and the standard library - use to deal with indexing and iterating over ranges in C++ reflect the fact that the language uses zero-based indexing. So really, all you have to do to avoid fencepost errors in C++, generally speaking, is write idiomatic code.)   