Battleship game help

[Aliun]

So, this is my version of battleship with single space ships instead of a ship with 2,3,4, and 5. The only issue is the cpu I programmed isn't always choosing five, or it's choosing spots not on the map, I am completely lost if you can offer any help at all that would be great.

edit: I have the cpu's ships marked as L's for debugging reasons

#include <iostream>
#include <cstdlib>
#include <ctime>
#include <iomanip>
#include <string>
#include <fstream>
#include <stdio.h>

using namespace std;
int convert(char num)
{
	int nom = 0;
	if(num == 'A'||num == 'a')
	{
		nom = 1;
	}
	if(num == 'B'||num == 'b')
	{
		nom = 2;
	}
	if(num == 'C'||num == 'c')
	{
		nom = 3;
	}
	if(num == 'D'||num == 'd')
	{
		nom = 4;
	}
	if(num == 'E'||num == 'e')
	{
		nom = 5;
	}
	if(num == 'F'||num == 'f')
	{
		nom = 6;
	}
	if(num == 'G'||num == 'g')
	{
		nom = 7;
	}
	if(num == 'H'||num == 'h')
	{
		nom = 8;
	}
	return nom;
}

void grid(string userShips[][8], string compShips[][8])
{
	string dot = "॰";
	cout<<"    Your Grid           His Grid\n";
	for(int x = 7; x >= 0; x--)
	{
		cout<<x+1;
		for(int y = 0; y <= 7; y++)
		{
			if(userShips[x][y] == "0")
			{
				cout<<" "<<dot;
			}
			if(userShips[x][y] == "H")
			{
				cout<<" H";
			}
			if(userShips[x][y] == "M")
			{
				cout<<" M";
			}
			if(userShips[x][y] == "1")
			{
				cout<<" S";
			}
		}
		cout<<"  ";
		cout<<x+1;
		for(int o = 0; o <= 7; o++)
		{
			if(compShips[x][o] == "0")
			{
				cout<<" "<<dot;
			}
			if(compShips[x][o] == "H")
			{
				cout<<" H";
			}
			if(compShips[x][o] == "M")
			{
				cout<<" M";
			}
			if(compShips[x][o] == "1")
			{
				cout<<" L";
			}
			//cout<<compShips[x][o];
		}
		cout<<endl;
	}
	cout<<"  A B C D E F G H    A B C D E F G H\n";
}
void compShip(string compShips[][8])
{
	for(int x = 0; x < 5; x++)
	{
		int random = 0;
		random = (rand()%8);
		int randm = 0;
		randm = (rand()%8);
		//cout<<random<<endl<<randm<<endl<<endl;
		compShips[random][randm] = "1";
	}
}
void playerChoose(string userShips[][8], string compShips[][8])
{
	grid(userShips, compShips);
	string one = "";
	int onez = 0;
	int two = 0;
	for(int i = 1; i<=5; i++)
	{
		cout<<"Choose the position of ship #"<<i<<": ";
		cin>>one;
		onez = convert(one[0]);
		two = atoi(&one[1]);
		userShips[two-1][onez-1] = "1";
	}
}
int compChoose(string userShips[][8], int userShipz)
{
	int x = 1;
	while(x == 1)
	{
		int r = (rand()%8)-1;
		int r1 = (rand()%8)-1;
		if(userShips[r][r1] == "1")
		{
			userShips[r][r1] = "H";
			userShipz--;
			x = 0;
		}
		if(userShips[r][r1] == "0")
		{
			userShips[r][r1] = "M";
			x = 0;
		}
	}
	return userShipz;
}
void play(string userShips[][8], string compShips[][8], int userShipz, int compShipz)
{
	string one = "";
	int onez = 0;
	int two = 0;
	int x = 1;
	while(userShipz > 0 && compShipz > 0)
	{
		grid(userShips,compShips);
		while(x == 1)
		{
			cout<<"Where would you like to hit: ";
			cin>>one;
			onez = convert(one[0]);
			two = atoi(&one[1]);
			onez--;
			two--;
			if(compShips[two][onez] == "1")
			{
				cout<<"HIT!\n";
				compShips[two][onez] = "H";
				compShipz--;
				break;
			}
			if(compShips[two][onez] == "0")
			{
				cout<<"MISS!\n";
				compShips[two][onez] = "M";
				break;
			}
		}
		userShipz = compChoose(userShips, userShipz);
	}
	if(userShipz == 0)
	{
		grid(userShips,compShips);
		cout<<"HAHA YOU LOST!\n";
	}
	if(compShipz == 0)
	{
		grid(userShips,compShips);
		cout<<"GRATZ YOU WON!\n";
	}
}
int main()
{
	srand(time(0));
	int userShipz = 5;
	int compShipz = 5;
	string userShips[8][8] = {
	{"0", "0", "0", "0", "0", "0", "0", "0"},
	{"0", "0", "0", "0", "0", "0", "0", "0"},
	{"0", "0", "0", "0", "0", "0", "0", "0"},
	{"0", "0", "0", "0", "0", "0", "0", "0"},
	{"0", "0", "0", "0", "0", "0", "0", "0"},
	{"0", "0", "0", "0", "0", "0", "0", "0"},
	{"0", "0", "0", "0", "0", "0", "0", "0"},
	{"0", "0", "0", "0", "0", "0", "0", "0"}};
	string compShips[8][8] = {
	{"0", "0", "0", "0", "0", "0", "0", "0"},
	{"0", "0", "0", "0", "0", "0", "0", "0"},
	{"0", "0", "0", "0", "0", "0", "0", "0"},
	{"0", "0", "0", "0", "0", "0", "0", "0"},
	{"0", "0", "0", "0", "0", "0", "0", "0"},
	{"0", "0", "0", "0", "0", "0", "0", "0"},
	{"0", "0", "0", "0", "0", "0", "0", "0"},
	{"0", "0", "0", "0", "0", "0", "0", "0"}};
	compShip(compShips);
	playerChoose(userShips, compShips);
	play(userShips,compShips, userShipz, compShipz);
}

[WaltP]

Looks to me like you need to explain in detail what the problems are -- and where in the code you're having problems. I don't see a problem in compShip() which it sounds like you're complaining about.

[Aliun]

Yes, I think the problem is in compShip() where the cpu is choosing it's ships. When I run the program most of the time only 4 ships are displayed that the comp chooses, when there should be 5, clearly. So my thoughts are maybe it's choosing a spot outside the board with the rands, but that would probably cause an error or something.

[mitrmkar]

I think that there is no protection against adding a ship several times to same location (neither for the user nor the computer).

Then are you sure that you have gotten the usage of rand() right, i.e. what does (rand()%8)-1; do?

Then try to type in some unexpected input and watch out for convert() .

PS. Kudos for great formatting.

[Aliun]

Then are you sure that you have gotten the usage of rand() right, i.e. what does (rand()%8)-1; do?

Well (rand()%8)-1; is simply because I was too lazy to change 8 to 7 because the arrays are 0 indexed which it turns out I didn't change 8 to 7 or add a -1 to my rand's in the cpu ship chooser which fixed the problem except sometimes it does choose the same spot twice, any tips on how to prevent that?

PS. Kudos for great formatting.

PS. Thanks, I try :)

[Fbody]

Well (rand()%8)-1; is simply because I was too lazy to change 8 to 7 because the arrays are 0 indexed which it turns out I didn't change 8 to 7 or add a -1 to my rand's in the cpu ship chooser which fixed the problem except sometimes it does choose the same spot twice, any tips on how to prevent that?

PS. Thanks, I try :)

Huh??? :-O:confused:

The range of answers you get from int%8 is [0,7], which should match the indexes for an 8x8 array exactly. With the '-1' the range of answers is modified to [-1, 6], which is completely useless. It would also explain why you're losing ships, if you get a '-1' coordinate, you're off your grid. In addition, the highest index you can generate is 6, which means a ship can never be placed in column/row 7.

Essentially, what you have done is shifted the ship placement area 1 unit left and 1 unit up from the valid play area causing 17 out of 64 possible spaces (appx 26% of your play area) to be wasted/inaccessible. Not to mention, you run the risk of crashing your system due to modifying memory outside the boundaries of your program/array(s).

[mitrmkar]

>> except sometimes it does choose the same spot twice, any tips on how to prevent that?

Have a loop in which the computer chooses, so if the location is already used, keep looping until a proper one is found.

Note that the indexes generated by rand() must be within 0..7, inclusive, otherwise you are in trouble.

[Aliun]

Ah you're right, Fbody. I guess that did help and I'm realizing now it's pretty easy to fix my two-ships-choosing-the-same-spot issue I mentioned earlier. Post solved, thanks much guys :).

[mitrmkar]

Post solved, thanks much guys :).

Once you've fixed what's mentioned above, test the program by trying to place your ship at 'A0' for example.

[tetron]

There are good general prinicples of OO you are adopting. And when your code gets long you want to test things a function at a time.

I think there is a potential issue with the player choose function.
you might find it simpler to have a function to process the input
seperately as there is no error checking

atoi would normally take a c_string rather than a single char
so this might cause problems putting in the address of ones[1] but it might handle it ok but it would be safer to take everything but the first letter

string ones;
int col_id, row_id
cout << "enter a number " << endl;
cin >> ones;
if(ones.size() >= 2)
{
col_id = convert(ones[0]);
//get all but the first letter of ones
ones = ones.substr(1);
row_id = atoi(ones.c_str())
}
else
{
//error
}
//now check col_id & row_id are valid

so you might prefer putting in letter then a number
with two inputs that way if the user only entered one letter
your code will not fail.

also v[two - 1][onez -1] might go out of range if convert finds a non-letter if this happens your code will fail
as v[-1] does not exist
so you should check

this is the issue with your rand %8 is in range 0-7
%8 - 1 in range -1 to 6

a couple of minor things to consider

you have if() followed by if()

else if() is more appropriate as you are checking for cases where
only on condition can be true;

your convert function can be made shorter:
either use a to lower function or you can use chars directly:

//i.e. convert

if(c >= 'A' && c <= 'H')
{
c = c - 'A' + 1; //offset A to 0, b to 1 then + 1
}
else if(c >= 'a' && c <= 'h')
{
c  =  c  - 'a' + 1;
}
else
{
//error you wanted to return 1
return 0;
}
int ret(c); //convert to int only a-h or A-H
return ret;

[Aliun]

That's all helpful, very helpful but at the moment I'm still having trouble with the stupid cpu ship chooser, I can't find a way to prevent it from choosing the same spot twice.
Here's what I have so far:

void compShip(string compShips[][8])
{
	int s[5] = {9,9,9,9,9};
	int m[5] = {9,9,9,9,9};
	int x = 1;
	for(int x = 0; x < 5; x++)
	{
		int random = 0;
		random = (rand()%8);
		int randm = 0;
		randm = (rand()%8);
		for(int i = 0; i<5; i++)
		{
			if(s[i] == random&&m[i] == randm)
			{
				random = (rand()%8);
				randm = (rand()%8);
				i = 0;
				i-=1;
			}
		}
		s[x] == random;
		m[x] == randm;
		cout<<"\n["<<random<<","<<randm<<"]\n";
		compShips[random][randm] = "1";
	}
}

[Fbody]

Here is a basic algorithm that should give you an idea.

do
  set row to a random number 0-7
  set col to a random number 0-7
loop while ( grid[row][col] is occupied )

set grid[row][col] to occupied

[Aliun]

edit: nvm