HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppModerate

Foundation for 2D dungeon crawler game

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
foundationgameforcrawlerdungeon

Problem

A game I've created to implement everything I've learned and to further build upon. I just recently learned classes so I'm probably using them wrongly.
Every bit of criticism is appreciated, I don't want to create bad habits.

Edit: My game draws up a field / map of symbols and characters which represents the playing field. It's turn-based and the goal of the game is to move the player ('P') to the exit on the other side of the playing field while avoid monsters ('M') who randomly walks around.

```
#include
#include
#include
#include

class fieldinfo
{
public:
static int fieldLength;
static int fieldWidth;
static int fieldExitAmount;
static std::vector xPositionFieldExit;
static std::vector yPositionFieldExit;
};

class playerinfo
{
public:
static int xPosition;
static int yPosition;
};

class monsterinfo
{
public:
static std::vector xPosition;
static std::vector yPosition;
static int monsterAmountDesired;
static int monsterAmountCurrent;
};

int fieldinfo::fieldLength = 10; // Default fieldsize.
int fieldinfo::fieldWidth = 10; // Default fieldsize.
int fieldinfo::fieldExitAmount;
std::vector fieldinfo::xPositionFieldExit;
std::vector fieldinfo::yPositionFieldExit;

int playerinfo::xPosition;
int playerinfo::yPosition;

int monsterinfo::monsterAmountDesired = 0;
int monsterinfo::monsterAmountCurrent = 0;
std::vector monsterinfo::xPosition;
std::vector monsterinfo::yPosition;

void GameRules( );
void SetFieldSize( );
void SetFieldExits( );
void SetPlayerPosition( );
void SetMonsterAmount( );
void RandomizeMonsterPositions( );
bool CheckForSpawnNearPlayer( int xPos, int yPos );
bool CheckForFieldExit( int xPos, int yPos );
bool CheckForFieldWall( int xPos, int yPos );
bool CheckForPlayerPosition( int xPos, int yPos );
bool CheckForMonsterPosition( int xPos, int yPos );
void CheckForWinCondition( );
void CheckForLoseCondition( );
void DrawField( );
void TurnO

Solution

I just recently learned classes so I'm probably using them wrongly.

Yes. For this review, I intend to focus on that aspect of your code--your usage of classes.

First of all, if every member of our class is static, we almost certainly don't need a class. If everything is static, essentially what you want is a namespace. In this case, however, using a class might be correct, but using static for everything definitely is not.

I say that using a class might be correct, because some might argue that in your case you actually want a struct. I've seen the argument made that in C++, passive objects that just carry data should be structs while class should be reserved for object which define method you might call on them. I'm not a C++ master, and I'm not sure how many people follow this convention. For the sake of this answer, I'm going to assume we're okay using a class here rather than a struct.

A class should represent a thing (not info about a thing, but the thing). We should capitalize the names of classes, and the info suffix is unnecessary. For example, we want:

class Player {
    // stuff
}


An instance of this class represents a "player" and the "info" about that player is held in any instance variables we stick on here.

Timeout. Pitstop. We're using a lot of "xPosition" and "yPosition". Let's just make a "position" struct real quick, shall we? We'll also make a "size" struct while we're at it. This will simplify some things moving forward.

struct Position {
    int x;
    int y;
};

struct Size {
    int length;
    int width;
};


Okay, now that that's done, let's take a look out how our Field class might look:

class Field {
public:
    std::vector fieldExits;
    Size fieldSize;
    int exitAmount;
}


And our Player class?

class Player {
public:
    Position position;
}


Now, you will notice, nothing is static, right? So how do we use these?

When you declare a variable as static, that means only one copy of that variable exists for the class. Instead, what we want are "instance variables" (without the static keyword). What this means is we're going to create instances of our classes. The position variable on the Player class will be unique to each "player" we create.

For instance:

Player player1 = Player();
player1.position = {0,1};

Player player2 = Player();
player2.position = {5,7};

Player player3 = Player();
player3.position = {3,4};

Player player4 = Player();
player4.position = {2,7};


What have we done here?

Well, we've created four different variables of type Player. Each of these Player instances keep track of a Position. They each have their own unique position, and we've set this position for all four players individually.

Before though, with static prepended to all of our variables, we essentially had global variables that could be accessed through out the entire program. Now, we don't have that. You can only access player1's position if you can access the player1 object. So we'll need to pass the player1 object around... this will require a pretty major restructuring of your program however... just as a warning. This answer has just shown you the basics of the proper usage of classes--there is still much more to learn.

Code Snippets

class Player {
    // stuff
}
struct Position {
    int x;
    int y;
};

struct Size {
    int length;
    int width;
};
class Field {
public:
    std::vector<Position> fieldExits;
    Size fieldSize;
    int exitAmount;
}
class Player {
public:
    Position position;
}
Player player1 = Player();
player1.position = {0,1};

Player player2 = Player();
player2.position = {5,7};

Player player3 = Player();
player3.position = {3,4};

Player player4 = Player();
player4.position = {2,7};

Context

StackExchange Code Review Q#85449, answer score: 10

Revisions (0)

No revisions yet.