Problem with Boolean and While loop.

[Kabooss]

Im writing a Battleships games, and everything is working (of which I know anyway) as intended, EXCEPT, my ship placement function... which sort of works?
The code works fine up to line 9, however once it gets to the while loop, the program closes with no errors or anything...

bool valid;
int shipplacement(int &Xcoordinate, int &Ycoordinate);

  int shipplacement(int &Xshipcoord, int &Yshipcoord){
	int a=3;
	valid=false;
	char direction;
	Getshipcoord(Xcoord, Ycoord);
	while(valid=false){  
	cout << "Which direction would you like your ship to point?" << endl;
	cin >> direction;
	
	switch (direction){

  case 'L':
		
	shipboard[Xcoord][Ycoord]='s';
		for(a=3; a!=0; a--){
			--Xcoord;
			shipboard[Xcoord][Ycoord]='s';
	}
	return true;
  break;
	
  case 'R':
		shipboard[Xcoord][Ycoord]='s';
			for(a=3; a!=0; a--){
				++Xcoord;
				shipboard[Xcoord][Ycoord]='s';
		}

		return true;
  break;
	
  case 'U':
		shipboard[Xcoord][Ycoord]='s';
			for(a=3; a!=0; a--){
				++Ycoord;
				shipboard[Xcoord][Ycoord]='s';
		}

		return true;
  break;
	
  case 'D':
		shipboard[Xcoord][Ycoord]='s';
			for(a=3; a!=0; a--){
				--Xcoord;
				shipboard[Xcoord][Ycoord]='s';
		}

		return true;
  break;
	
  default:
		cout << "No valid direction was given, the choice is either: up    (U), down (D), left (L), right (R)." << endl;
		return false;
  break;
	}
	}
	return true;
  }

Thank you for reading and any help is appreciated! :)

[Marcos Modenesi]

valid = false

gives variable valid a false state.

valid == false

compares valid with false.

= is not the same as ==, the first case is an assignment, second one is comparrison (however it´s spelled!)

You could also use:

while (!valid) {

since ! means NOT, the opposite of

[Kazekan]

Your while loop is looking for the specific boolean variable "valid", but you never actually change that variable to anything other than false. Your cases may return true, but that's not setting "valid" to true.

Also, from the way you've posted the code it's almost impossible to tell how you have things organized in the rest of the program, which makes it quite difficult to say why it's ending all of a sudden. You declare a function prototype and then a definition right below it, with what looks like a global variable of "valid" right above it. But, why are you returning a true or false after each case? If you want the loop statement to end at these points, you should be using valid = true; and not return true;. Return true; will just tell the function that it's supposed to end, and the final result was a value of "true" or since this is an int function, 1.

Hope that helps.

[Kabooss]

Tha nk you very much marcos!
@kazaken, the main program literally just has, cin.getline, and outputs some text, then calls the Shipplacement function, and it never made it past the shipplacement, so i figured it was unnecessary. I could copy it in, but well, its 315 lines long and i assumed that the most of it was not needed as the issue iI knew was directly in the shipplacement func...
But thank you for the explanation of the whole Return true/false, because in all honesty I didn't actually know that haha.

[Kabooss]

Sorry, just realised that it was a blatenly obvious error and just took me a while to see it!

** edited the wrong comment, should have been the bottom ones...

[Kazekan]

I have to say, your usage of brackets and tabs makes this very hard to read. Unless you're paying very close attention to the number of brackets, you can't even tell when the while loop ends. It would be extremely helpful if you used tabs to create logical breaks in your code and to organize your code within each set of brackets. Having code that's further to the left than it should be creates unnecessary confusion.

[Kabooss]

Sorry about that, it was at one point, but I emailed it from one computer to another and well, it got weird, and i never sorted it out... ill do it now...

[Kazekan]

I've cleaned it up as well, and moved something that should stop your program from ending. Namely, the very last return true; statement which is trying to run at the end of each iteration of the loop. Putting this outside the loop will ensure it only returns true once the while loop has finished going through all 5 ships.

int shipplacement(int &Xshipcoord, int &Yshipcoord)
{
    int a=3;
    ShipsToPlace=5;
    valid=false;
    char direction;
    while(ShipsToPlace !=0)
    {
        --ShipsToPlace;
        cout << "You have " << ShipsToPlace << " Ships to place." << endl;
        Getshipcoord(Xcoord, Ycoord);
        while(valid == false)
        {
            cout << "Which direction would you like your ship to point?" << endl;
            cin >> direction;
        
            switch (direction)
            {
                case 'L':
                {
                    shipboard[Xcoord][Ycoord]='s';
                    for(a=3; a!=0; a--){
                        --Xcoord;
                        shipboard[Xcoord][Ycoord]='s';
                    }
                    valid = true;
                    break;
                }
                    
                case 'R':
                {
                    shipboard[Xcoord][Ycoord]='s';
                    for(a=3; a!=0; a--){
                        ++Xcoord;
                        shipboard[Xcoord][Ycoord]='s';
                    }
                    valid = true;
                    break;
                }
                    
                case 'U':
                {
                        shipboard[Xcoord][Ycoord]='s';
                        for(a=3; a!=0; a--){
                            ++Ycoord;
                            shipboard[Xcoord][Ycoord]='s';
                        }

                        valid = true;
                    break;
                }
                case 'D':
                {
                    shipboard[Xcoord][Ycoord]='s';
                    for(a=3; a!=0; a--){
                        --Xcoord;
                        shipboard[Xcoord][Ycoord]='s';
                    }

                    valid = true;
                    break;
                }
                    
                default:
                {
                    cout << "No valid direction was given, the choice is either: up (U), down (D), left (L), right (R)." << endl;
                    valid = false;
                    break;
                }
            }
        }
        
        shipplacement2(Xcoord, Ycoord);
    }
    return true;
}

I of course haven't really tested this, but it should be fine. :) Hope that helps!

[Kabooss]

Thank you very much, works wonders!

[Kabooss]

Ok, so. I have done quiet a few additions, the program now will not allow for covering other ships, and placing ships off edge (0 to 9) It also output some values (so i cant see what its doing when its running(I.e Undo)) and for some reason, when it goes and clears a ship placement if it covers another ship... it goes up to like 11... when it should only ever get to 4? Im sorry if its messy.

Looking at likes 31 and 58 for example (as code is applied to every case statement and is the same (other than things such as the Coordinate values...)

int shipplacement(int &Xshipcoord, int &Yshipcoord){
	int undo=0;
	int a=4;
    ShipsToPlace=6;
    valid=false;
    char direction;
    while(ShipsToPlace !=1)
    {
		
		if(!valid)
		{
		--ShipsToPlace;
		Displayshipboard(shipboard);
		}
		valid=false;
	
		cout << "You have " << ShipsToPlace << " Ships to place." << endl;
        Getshipcoord(Xcoord, Ycoord);
		  
            cout << "Which direction would you like your ship to point?" << endl;
			cout << "Up (U), Down (D), Left (L) or Right (R)." << endl;
            cin >> direction;
        
            switch (direction)
            {
                case 'L':
                {
					cout << "You chose to point your ship Left" << endl;
					    for(a=4; a!=0; a--)
						{
							++undo;  //here is the adding of Undo (so it equals the same as 'a' which is the ship length counter... this is in every direction case.)
							while (shipboard[Xcoord][Ycoord] =='s')
							{
								cout << "You cannot place a ship here as a ship is already in this location: " << Xcoord << ", " << Ycoord << endl;
								--undo;
								valid=true;
								break;
							}	
							while(Xcoord<0 || Ycoord<0)
							{
									cout << "Your ship must be within the borders of the battle zone. Ships are 4 coordinates long. " << Xcoord << ", " << Ycoord <<  endl;
									valid=true;
									break;
							}	
							while(Xcoord>=10 || Ycoord>=10)
							{
									cout << "Your ship must be within the borders of the battle zone. Ships are 4 coordinates long. " << Xcoord << ", " << Ycoord <<  endl;
									valid=true;
									break;
							}
								
							if(!valid)
							{
								shipboard[Xcoord][Ycoord]='s';
								--Xcoord;
								cout << "Xcoord: " << Xcoord << " a: " << a << endl;
							}
							else // this is where it is supposed to work out how many times it needs to undo...again in every direction case.
							{
								while(undo!=0)
								{
									++Xcoord;
									
									shipboard[Xcoord][Ycoord]=' ';
									cout << "Xcoord: " << Xcoord << " Undo: "  << undo << endl;
									--undo;
								}
							}
						}
						
                    break;
                }
                    
                case 'R':
                {
					cout << "You chose to point your ship Right" << endl;
					    for(a=4; a!=0; a--)
						{
							++undo;
							while (shipboard[Xcoord][Ycoord] =='s')
							{
								cout << "You cannot place a ship here as a ship is already in this location: " << Xcoord << ", " << Ycoord << endl;
								--undo;
								valid=true;
								break;
							}	
							while(Xcoord<0 || Ycoord<0)
							{
									cout << "Your ship must be within the borders of the battle zone. Ships are 4 coordinates long. " << Xcoord << ", " << Ycoord <<  endl;
									valid=true;
									break;
							}	
							while(Xcoord>=10 || Ycoord>=10)
							{
									cout << "Your ship must be within the borders of the battle zone. Ships are 4 coordinates long. " << Xcoord << ", " << Ycoord <<  endl;
									valid=true;
									break;
							}
							
							if(!valid)
							{
								shipboard[Xcoord][Ycoord]='s';
								++Xcoord;
								cout << "Xcoord: " << Xcoord << " a: " << a << endl;
							}
							else
							{
								while(undo!=0)
								{
									--Xcoord;
									
									shipboard[Xcoord][Ycoord]=' ';
									cout << "Xcoord: " << Xcoord << " Undo: "  << undo << endl;
									--undo;
								}
							}
						}
						
                    break;
                }
                    
                case 'U':
                {
					cout << "You chose to point your ship Up" << endl;
					    for(a=4; a!=0; a--)
						{
							++undo;
							while (shipboard[Xcoord][Ycoord] =='s')
							{
								cout << "You cannot place a ship here as a ship is already in this location: " << Xcoord << ", " << Ycoord << endl;
								--undo;
								valid=true;
								break;
							}	
							while(Xcoord<0 || Ycoord<0)
							{
									cout << "Your ship must be within the borders of the battle zone. Ships are 4 coordinates long. " << Xcoord << ", " << Ycoord <<  endl;
									valid=true;
									break;
							}	
							while(Xcoord>=10 || Ycoord>=10)
							{
									cout << "Your ship must be within the borders of the battle zone. Ships are 4 coordinates long. " << Xcoord << ", " << Ycoord <<  endl;
									valid=true;
									break;
							}
						
							if(!valid)
							{
								shipboard[Xcoord][Ycoord]='s';
								--Ycoord;
								cout << "Ycoord: " << Ycoord << " a: " << a << endl;
							}
							else
							{
								while(undo!=0)
								{
									++Ycoord;

									shipboard[Xcoord][Ycoord]=' ';
									cout << "Ycoord: " << Ycoord << " Undo: "  << undo << endl;
									--undo;
								}
							}
						}
						
                    break;
                }
                case 'D':
                {
					cout << "You chose to point your ship Down" << endl;
					    for(a=4; a!=0; a--)
						{
							++undo;
							while (shipboard[Xcoord][Ycoord] =='s')
							{
								cout << "You cannot place a ship here as a ship is already in this location: " << Xcoord << ", " << Ycoord << endl;
								--undo;
								valid=true;
								break;
							}	

							while(Xcoord<0 || Ycoord<0)
							{
									cout << "Your ship must be within the borders of the battle zone. Ships are 4 coordinates long. " << Xcoord << ", " << Ycoord <<  endl;
									valid=true;
									break;
							}	
							while(Xcoord>=10 || Ycoord>=10)
							{
									cout << "Your ship must be within the borders of the battle zone. Ships are 4 coordinates long. " << Xcoord << ", " << Ycoord <<  endl;
									valid=true;
									break;
							}
								
							if(!valid)
							{
								shipboard[Xcoord][Ycoord]='s';
								++Ycoord;
								cout << "Ycoord: " << Ycoord << " a: " << a << endl;
							}
							else
							{
								while(undo!=0)
								{
									--Ycoord;
									
									shipboard[Xcoord][Ycoord]=' ';
									cout << "Ycoord: " << Ycoord << " Undo: "  << undo << endl;
									--undo;
								}
							}
						}
						
                    break;
                }
                    
                default:
                {
                    cout << "No valid direction was given, the choice is either: up (U), down (D), left (L), right (R)." << endl;
                    break;
                }
            }
        }
    
	 shipplacement2(Xcoord, Ycoord);
    return true;
}

Ok, so, ive sorted out the problem, again, thank you all so much for the support you've given me!