Need help writing a fun game

[GhostCas]

So I keep getting stuck while writing this Battleship game... I think that i am trying to do too much at once which is confusing me, even though I did try to break it up into parts. My main problem is setting the ships to a value and then allowing them to occupy a spot on the board. I just want to hardcode the ships into certain spots (can be the same spots for both players) After I get that to work then I'll try to go more in depth with the game. Any help or advice would be appreciated. I didn't get far into my target.cpp but my battle board.cpp is getting somewhere.

 #ifndef TARGET_H
#define TARGET_H
class Target 
{
private:
int *length; 
length = new int;
string ship1, ship2, ship3, ship4, ship5;

public:
void placeship (); // set ships to a value on the gameboard 
bool ship (string name, int length); // create ships
int play (int gameboard [50]); // Start the game loop through players etc
};

#endif

Target.cpp
#include <iostream>
#include "boardconst.h"
#include "battleboard.h"
#include "target.h"
using namespace std;

bool Target ::  ship (string name, int length)
{

  ship1.length == 2;
  ship2.length == 3;
  ship3.length == 4;
  ship4.length == 4;
  ship5.length == 5;
}

#ifndef battleboard_h
#define battleboard_h
using namespace std;
#include "boardconst.h"

class BattleBoard 
{
public:
void display (int gameboard [position]);
bool checkforwin (int gameboard [position]); 
void player1(int gameboard[position]);
void player2 (int gameboard[position]);
};

#endif 

BattleBoard.cpp

#include <iostream>
#include "boardconst.h"
#include "battleboard.h"
#include "target.h"
using namespace std;

void BattleBoard :: display(int gameboard [position])
{
  
// Gameboard Trying to convert a two dimensonal gameboard to a one dimensonal array game board
int row = 0, column = 0, location=0;
 cout << "Show empty board." << endl;
     for (int x = 0; x < 50; x++)
{
cout << gameboard[x] << " ";// Making the board easier to see by putting a space between every  number
 if ((x + 1) % 5 == 0)
   {
cout << endl; // Start a new line
   }
}
    cout << "Which row [1 to 5] ? " << endl; 
        cin >> row;
    cout << "Which column [1 to 5] ? " << endl; 
	cin >> column;
    cout << "Putting the number 2, at location "<<row << ", " << column << endl;
	location = ((row - 1) * 5) + (column-1);
	// Subtract 1 from row and column, since array starts at zero
	gameboard[location] = 2;
	for (int x = 0; x < 50; x++)
{
cout << gameboard[x] << " ";// Putting a space between numbers
 if ((x + 1) % 5 == 0)
   {
   cout << endl; // Start a new line
   }
 }

}


void BattleBoard :: player1(int gameboard[position])
{
  int attack;
  attack == gameboard[position]
  cout << "Player 1 enter the position that you want to hit!" << endl; // The position should map back to a position on the gameboard 
 cin >> attack;

 while (attack <= 0)
    {
      cout << " Invalid position try again" << endl; 
      cin >> attack;
    }
}

void Battleboard :: player2 (int gameboard [position])
  { 
    int attack;
    attack = gameboard[position];
    cout << "Player 1 enter the position that you want to hit!" << endl; // The position should map back to a position on the gameboard
    cin >> attack; 
    while (attack <= 0)
      {
	cout << "Invalid position try again" << endl;
	cin >> attack;
      }
  }

[MatthewRock]

I'd suggest that you read some basic stuff about OOP - how to design classes - then come back.

For example, BattleBoard shouldn't have methods such as player1 or player2. Moreover, even if it had these methods, they should be more generic(like PlayerN(int numPlayer)).

You should have distinction between each game phase(get input, update logic, draw).

The basic patterns are Expert, High Cohesion, Low Coupling, and more - google for "GRASP OOP".

This knowledge should help you improve your code - now we'd have to rewrite it from scratch to not look like a hack.
Also, google for "Game Loop" to see how it's usually structured.


Back to the topic - it's normal that at the beginning you're overwhelmed by how many things you've got to remember at one time. However, after some time and experience you'll have no problem with such things.

What I find useful is that I identify the problem(program) and divide it into some abstract parts. Abstract things away as much as you can, and write smaller parts.

I hardly ever think of the problem as of whole of the functionalities it has. I don't think about the code, I think about the algorithm and logic in natural language. It's much simpler to say that "I check each neighbouring tile and see if it has some property" especially that this can result in (relatively) complex code.

The idea is actually hard enough to explain face to face, so I'm powerless here, but the point is: don't think about too much at once, and separate what you're thinking about. Write code so that it isn't a tangled spaghetti, so you don't have to worry about some thing breaking because of other thing. Read about bottom-up and top-down design and use them when you think they make sense. In programming knowledge comes mostly from experience, and you'll get that.

But first try to read about these things, they should help you with writing clearer code.

[AbstractionAnon]

I know you had a previous thread about representing the game board in a one dimensional array. You didn't make clear in that thread why you wanted to do that. For a battleship game, a 2D array is much more intuitive.

One of the more challenging functions for a beginner in a battleship game is placing the ships on the board since you generally want to place some ships horizontally and some vertically.

lines 6-7: Why is length dynamically allocated? You don't have a destructor for Target that frees length, therefore you have a memory leak.

lines 28-32: These statements don't do anything. You're using the comparison operator (==) and ignoring the result of the comparison. I suspect you were trying to use the assignment operator (=), however, you can't change the length of a string that way. length() is a const function.

Line 111: You refer to player 1 in the cout.

Lines 93-118: As MathewRock stated, these functions should be a single generic function. The only difference between them is the player number.

Not clear where you declared gameboard. Logically, this should be an attribute of BattleBoard. This would eliminate passing gameboard to every BattleBoard function.

Your Target class seems to me to represent a player, not a target.

[MatthewRock]

AbstractionAnon proves my point. Much of your code needs refactoring. We could do it Anon style and point out each mistake we find, but it will probably be (quite) a long list - and we would probably end up with ugly program that works; we don't want that. We want a good program. Therefore I strongly suggest that you go through the tutorials and read about these things. I know that writing games or anything that you *want* to write is much more interesting than reading tutorials, but without tutorials you'll learn a lot slower.

For example, some of the classes I'd create would be Game, Board, Player, maybe InputHandler. Some of the methods would be* Player::Move(), Game::Draw(), Board::Draw(), Board::IsGameFinished(), Game::GetInput(), Board::VerifyInput(). Maybe I'd change my mind, but you see what I'm going at.

* The methods arguments are unknown now, but they would most likely end up taking some arguments.