Text-based RPG game using classes

Question

I am studying for a degree in "Bachelor of Engineering in Information and Communication Technologies." I am currently on vacation, just after we started learning C++ at the end of the semester. I wanted to be ahead of the next semester, so I decided to try to use these classes and to make a text-based game.

I would like to know what I could do better. Should I remove or add some class functions? Should I do something differently, maybe not even related to classes?

Main.cpp

#include "MobClass.h"
#include "Player.h"
#include <iostream>
#include <cstdlib>
#include <ctime>
#include <windows.h>

using namespace std;
player battle(player account);
player calcEXP(player account,classMob monster);
player levelUp(player account);
void death();

int main()
{
    string name;
    int option1;
    cout << "Welcome, please enter your name\n";
    cin >> name;
    string location[4] = {"in a hole","in a cave","in the mauntains","in a castle"};
    player account(name,location[0],1,0);
    cout <<"\nWelcome "<<account.getName() << " you find your self " << account.getArea() << "\nand you are not sure how you ended up here\n";
    while (1)
    {
        Sleep(500);
        cout <<"write 1 to walk forward or 2 to walk left or 3 to walk right\n";
        cin >> option1;
        if (option1 >=1 && option1 <=3)
        {
            Sleep(50*(option1));
            srand(time(NULL));
            if (rand() %3 == option1-1){
                account = battle(account);
            }

        }
        else{
            cout << "\n#@#Error#@# Please enter a number between 1 and 3\n\n";
            cin.clear();
            cin.ignore();
        }
    }
    return 0;

}




player battle(player account)
{
    string option;
    string location[4] = {"in a hole","in a cave","in the mauntains","in a castle"};
    string monsters[5][3] = {{"worm","lizard","snake"},{"rat","snake","trolls"},{"Dragon","Dragon","Dragon"},{"Evil knight","The mad king","Joffrey Baratheon"}};
    Sleep(20);
    srand(time(NULL));
    int ranM = (rand() % 3); //random monster
    int ranD = (rand() % 5)+1; //random diff
    classMob monster(monsters[account.getLevel()-1][ranM],account.getLevel(),account.getArea(),ranD);
    cout <<"Suddently you meet a "<< monster.getName() <<", be ready for battle" << "\n";
    Sleep(2000);
    do 
    {
        cout << "\n\n\n ######################################\nHP:"<< account.getHealth() << "                                         "<< monster.getName()<<"HP:"<<monster.getHealth()<<" difficulty:"<<monster.getDifficulty() << "\n";
        cout << "Write A for attack or R for retreat" << "\n";
        cin >> option;
        srand(time(NULL));
        if (option == "R" || option == "r")
        {
            if ((rand() % 2) == 1){
                cout << "retreat sucessfull" << "\n";
                monster.setHealth(0);
            }
            else{
                cout << "retreat failed, the monster get a free attack and you lose 5 health\n";
                account.setHealth(account.getHealth()-5);
                option ="A";
            }
        }
        if (option == "A" || option == "a")
        {
            int attack =rand()%(account.getDamage());
            srand(time(NULL));
            int mobAttack = rand()%(monster.getDamage());
            monster.setHealth(monster.getHealth()-attack);
            account.setHealth(account.getHealth()-mobAttack);
            cout << "you attack the monster for " << attack << " damage\n";
            Sleep(500);
            cout << "the monster counter attacks for " << mobAttack << " damage\n";
            Sleep(500);
        }
    } while (monster.getHealth() >0 && account.getHealth() > 0);
    cout << "\n\n\n ######################################\nHP:"<< account.getHealth() << "                                         "<< monster.getName()<<"HP:"<<monster.getHealth()<<" difficulty:"<<monster.getDifficulty() << "\n";
    if (account.getHealth() <= 0)
    {
        death();
        exit(0);
    }
    account = calcEXP(account,monster);
    return account;
}



void death()
{
    cout << "Sorry you failed your epic quest\n";
}

player calcEXP(player account,classMob monster)
{
    cout << "#########\ncalculating EXP\n#########\n";
    Sleep(500);
    account.setEXP(account.getEXP() + monster.getEXP());
    cout << "EXP: " <<account.getEXP() << "/" << account.getEXPReq() << "\n";
    if (account.getEXP() >= account.getEXPReq())
    {
        levelUp(account);
    }
    return account;
}

player levelUp(player account)
{
    account.setLevel(account.getLevel()+1);
    account.setEXPReq();
    account.setMaxHealth();
    account.setHealth(account.getMaxHealth());
    cout << "Level up! you are now level: " << account.getLevel() << "!\n";
    return account;
}

Player.h

#include <string>
class player
{
public:
    player(std::string,std::string,int,int);
    void setName(std::string);
    void setArea(std::string);
    void setLevel(int);
    void setEXP(double);
    void setHealth(double);
    void setMaxHealth();
    void setDamage();
    std::string getName();
    std::string getArea();
    int getLevel();
    double getHealth();
    double getMaxHealth();
    int getDamage();
    int getEXP();
    void setEXP(int);
    int getEXPReq();
    void setEXPReq();
private:
    std::string playerName;
    std::string playerArea;
    int playerLevel;
    double playerHealth;
    double playerMaxHealth;
    int playerDamage;
    int EXP;
    int EXPReq;
};

player.cpp

#include <string>
#include "Player.h"
player::player(std::string name,std::string area,int level = 1,int EXP = 0)
{
    setName(name);
    setArea(area);
    setLevel(level);
    setEXP(EXP);
    setMaxHealth();
    setHealth(playerMaxHealth);
    setDamage();
    setEXPReq();
}

void player::setName(std::string name)
{
    playerName = name;
}
void player::setArea(std::string area)
{
    playerArea = area;
}
void player::setLevel(int level)
{
    playerLevel = level;
}
void player::setHealth(double health)
{
    playerHealth = health;
}
void player::setMaxHealth()
{
    playerMaxHealth = (100 * getLevel());
}
void player::setDamage()
{
    playerDamage = (30 * getLevel());
}

std::string player::getName()
{
    return playerName;
}

std::string player::getArea()
{
    return playerArea;
}

int player::getLevel()
{
    return playerLevel;
}
double player::getHealth()
{
    return playerHealth;
}
double player::getMaxHealth()
{
    return playerMaxHealth;
}
int player::getDamage()
{
    return playerDamage;
}

int player::getEXP()
{
    return EXP;
}
void player::setEXP(int _EXP)
{
    EXP = _EXP;
}
int player::getEXPReq()
{
    return EXPReq;
}
void player::setEXPReq()
{
    EXPReq = 70+((getLevel()*getLevel())*35);
}

mobclass.h

#include <string>

class classMob
{
public:
    classMob(std::string,int,std::string,int); // name,lvl,area,difficulty
    void setName(std::string);
    void setLevel(int);
    void setArea(std::string);
    void setDamage();
    void setHealth(double);
    void setMaxHealth();
    void setDifficulty(int);
    std::string getName();
    int getLevel();
    std::string getArea();
    int getDamage();
    double getHealth();
    double getMaxHealth();
    int getDifficulty();
    int getEXP();
    void setEXP();
private:
    std::string mobName;
    std::string mobArea;
    int mobLevel;
    int mobDamage;
    double mobHealth;
    double mobMaxHealth;
    int mobDifficulty;
    int EXP;
};

mobclass.cpp

#include <string>
#include "MobClass.h"
classMob::classMob(std::string name,int lvl,std::string area,int difficulty)
{
    setName(name);
    setLevel(lvl);
    setArea(area);
    setDifficulty(difficulty);
    setDamage();
    setMaxHealth();
    setHealth(mobMaxHealth);
    setEXP();
}

void classMob::setName(std::string name)
{
    mobName = name;
}

void classMob::setLevel(int level)
{
    mobLevel = level;
}

void classMob::setArea(std::string area)
{
    mobArea = area;
}

void classMob::setDifficulty(int difficulty)
{
    mobDifficulty = difficulty;
}

void classMob::setDamage()
{
    mobDamage = (3 *( getLevel())+((getDifficulty()*getLevel())/2));
}

void classMob::setHealth(double health)
{
    mobHealth = health;
}
void classMob::setMaxHealth()
{
    mobMaxHealth = (15 *(getDifficulty() + getLevel()));
}
std::string classMob::getName()
{
    return mobName;
}

int classMob::getLevel()
{
    return mobLevel;
}
std::string classMob::getArea()
{
    return mobArea;
}
int classMob::getDifficulty()
{
    return mobDifficulty;
}
int classMob::getDamage()
{
    return mobDamage;
}
double classMob::getHealth()
{
    return mobHealth;
}
double classMob::getMaxHealth()
{
    return mobMaxHealth;
}

int classMob::getEXP()
{
    return EXP;
}

void classMob::setEXP()
{
    EXP = (getLevel() * 35);
}

Answer 1

• Try not to get in the habit of using using namespace std. Read this for more information.

• For clarity, have your #includes organized. Read this blog post or this answer for more information.

• Add a newline between each section of code. For instance, separate all user input and loops. For variables, it's best to initialize them late as late as possible in case the function needs to terminate prematurely. Again, keep them with the corresponding code.

• mobclass.h already includes <string>, so you don't need to include it again in the .cpp file.

• You have a lot of accessors and mutators. Since these are short one-line implementations, you can define them in the header like this:

  void setEXP() {EXP = (getlevel() * 35;}
  int getEXP() const {return EXP;}
  

  As such, you will no longer need to implement these in the .cpp file. When they're in the header, they'll automatically be inline. It should also make it easier if you ever need to implement newer functions. In the header, you could also keep the accessors and mutators together for clarity.

• I like what @Kaivo Anastetiks said about classMob's constructor, but I would like to add on that a bit. You have a few options for this:

  1. keep it in the .cpp file (with those changes)
  2. put it in the header (with the classMob:: part removed)
  3. put it under the class declaration in the same file

• It's better to use getline() instead of cin for getting an std::string value from the user:

  getline(std::cin, name);
  

• srand() should ONLY be called ONCE in the program, preferably at the top of main(). If you keep it as is, rand() will be "not-so-random" because the seed will keep resetting to 0.

• That really long output line in battle()'s do-while loop could be wrapped so that it doesn't extend out that far.

• For death(): there's no need to have a function just output a message. Either have it do something else relevant, or just remove it.

• For the player's death (in general): I would prefer the function to fall back to main() instead of explicitly exiting. This is because:

  1. it's clearer to let main() terminate the program whenever possible
  2. it could be hard to tell where the player's death is determined

  You would then need to change main()'s loop to handle this. You could even have battle() return a bool to indicate the battle outcome (the player has won or has lost). Try this at the end:

  account = calcEXP(account, monster);
  
  if (account.getHealth() <= 0)
      return false;
  
  return true;
  

  You could even create a bool member function for determining if the player's health was depleted:

  bool healthDepleted() const {return playerHealth <= 0;}
  

• If you're just mutating data members in calcEXP() and levelUp(), they don't need to return anything. Just make those functions void.

• Your "saving" problem is due to your functions receiving the objects by value. It should be received by reference instead. You were only passing in a copy and modifying it, only to have those changes discarded each time those functions ended. This change will allow you to modify the original objects:

  bool battle(player &account);
  void calcEXP(player &account, classMob &monster);
  

• After looking at mobClass's definition, it appears that you may not need those mutators. You should consider a mobClass instance as an individual monster, just as a player is just one player. As such, you just need to construct each mobClass once with the default stats. The accessors are still okay.

• Once again, I forgot about this: create a Game class. Since the human player doesn't need to know how the game's internal mechanisms work, you would no longer need those extra functions in the driver. Instead, main() will create a Game and the class will handle the rest. Here's (roughly) what main() could look like:

  int main()
  {
      std::srand(std::time(NULL));
  
      Game game;
      game.play();
  }
  

  That may be too little for main(), but the idea is that Game will handle everything. Every function in Game, except for play(), should be private. Game should contain a player as a data member and instantiated in Game's constructor. If you end up implementing your map idea (or anything similar), that would be instantiated in the constructor as well. You may still keep those extra driver functions, but they should be called in play() instead.

  As for classMob, you could create an std::vector of objects (you cannot predict how many monsters will "spawn" before the player dies). New monsters would be added and, when killed by the player, removed. If you maintain a counter, you could even track the number of monsters killed before defeat.

Answer 2

Your classes are designed badly:

class player
{
public:
    player(std::string,std::string,int,int);
    void setName(std::string);
    void setArea(std::string);
    void setLevel(int);
    void setEXP(double);
    void setHealth(double);
    void setMaxHealth();
    void setDamage();
    std::string getName();
    std::string getArea();
    int getLevel();
    double getHealth();
    double getMaxHealth();
    int getDamage();
    int getEXP();
    void setEXP(int);
    int getEXPReq();
    void setEXPReq();
private:
    std::string playerName;
    std::string playerArea;
    int playerLevel;
    double playerHealth;
    double playerMaxHealth;
    int playerDamage;
    int EXP;
    int EXPReq;
};

You are exposing all the internal members (they just happen to be behind shallow get/set functions). But you are tightly coupling your class to the actual types you use for storage. This tight coupling makes your code very brittle; any change is going to ripple through your code requiring any code that uses your class to also change.

Your class methods should be "VERBS" that describe actions that happen on your obeject (thus not exposing the internal types).

Example Leveling up:
The only reason you have a bunch of these getters is so that an external function can get update then update the value.

player levelUp(player account)
{
    account.setLevel(account.getLevel()+1);
    account.setEXPReq();
    account.setMaxHealth();
    account.setHealth(account.getMaxHealth());
    cout << "Level up! you are now level: " << account.getLevel() << "!\n";
    return account;
}

The problem here is that you loose control of leveling up. With this technique anybody can write an alternative way of leveling up. Then if things change you need to find and modify all the techniques when you change how leveling up is done.

This should all be part of the player class that way leveling up is controlled as part of the player. It is done in one place (and only one place):

player Player::levelUp()
{
    playerLevel++;
    EXPReq          = 70  + (playerLevel * playerLevel) * 35;
    playerMaxHealth = 100 * playerLevel;
    playerHealth    = playerMaxHealth;

    cout << "Level up! you are now level: " << account.getLevel() << "!\n";
}

You can basically remove all the getter/setter methods and put in methods that actual manipulate the object.

Edit Based on Comment:

hm I am getting one problem though, so in the function calcEXP I am using monster.getEXP(); and i know that in the function battle I use some of the get functions to get information about the classMob class. How can i acess this without the use of get functions ?

Couple of ways. But I think the simplest is to make a common base class that handles this stuff:

 class LifeForm
 {
      // Traits that are common to both players and monsters.
      int EXP;
      int EXPReq;

           void levelUp() {}
       public:
           void hasKilled(LifeForm const& deadLifeForm)
           {
                  // Absorb the experience of the foe.
                  EXP += deadLifeForm.EXP;

                  if (EXP >= EXPReq)
                  {
                      levelUp();
                  }
           }
           
      
 };

 class Person: public LifeForm
 {
 };

 class Monster: public LifeForm
 {
 };

The LifeForm class understands the concept of experience (maybe other stuff) and handles the interaction between two lifeforms. Since an object has access to the private members of other objects of the same class it works fine.

Answer 3

First off, decide whether, and to what degree, you want object orientedness.

If you want to keep the current design, you may want to use structs rather than classes. At the moment this isn't very object-oriented at all, and getting rid of the need for getters and setters for everyfreakingthing would reduce the amount of code by like 2/3. Making something private and adding a getter and setter that just blindly set the variable...that's not encapsulation. That's busywork. You're not really protecting anything, you end up having to declare and define the getter/setter, and you turn player.health -= 10; into player.setHealth(player.getHealth() - 10);, which is harder to read. :P

If you want to be more object-oriented about all this, the first thing to do is let the classes take more responsibility for their own internal state. As many as possible of the actions that involve modifying player attributes, should be in the player class. Same with monster attributes and classMob, of course.

Consider your current fight logic:

player battle(player account) {
    ...
    do {
        ...
        if (option == "A" || option == "a")
        {
            int attack =rand()%(account.getDamage());
            srand(time(NULL));
            int mobAttack = rand()%(monster.getDamage());
            monster.setHealth(monster.getHealth()-attack);
            account.setHealth(account.getHealth()-mobAttack);
            cout << "you attack the monster for " << attack << " damage\n";
            Sleep(500);
            cout << "the monster counter attacks for " << mobAttack << " damage\n";
            Sleep(500);
        }
    } while (monster.getHealth() >0 && account.getHealth() > 0);

    ...

    if (account.getHealth() <= 0)
    {
        death();
        exit(0);
    }

    ...

    return account;
}

A huge amount of this is stuff that classMob and player could be handling. You could easily have something like

int player::attack(classMob& mob) {
    int attack = rand() % damage;
    mob.takeDamage(attack);
    return attack;
}

void player::takeDamage(int damage) {
    health -= damage;
    if (health < 0) health = 0;
}

bool player::isAlive() { return health > 0; }



int classMob::attack(player& p) {
    int attack = rand() % damage;
    p.takeDamage(attack);
    return attack;
}

void classMob::takeDamage(int damage) {
    health -= damage;
    if (health < 0) { health = 0; }
}

bool classMob::isAlive() { return health > 0; }

Now your player and mob can attack each other, and the above main-manages-the-attack code turns into

player battle(player account) {
    ...
    do {
        ...
        if (option == "A" || option == "a")
        {
            int attack = account.attack(monster);
            cout << "you attack the monster for " << attack << " damage\n";
            Sleep(500);

            int mobAttack = monster.attack(account);
            cout << "the monster counter attacks for " << mobAttack << " damage\n";
            Sleep(500);
        }
    } while (monster.isAlive() && account.isAlive());

    ... show status lines again ...

    if ( !account.isAlive() )
    {
        death();
        exit(0);
    }

    ...
}

(By the way, if we take a look, we will see that monsters and players attack each other in the same way. In fact, they share a whole lot of the same attributes. It's starting to look like classMob and player should have a common ancestor, at least.)

It might not look that much better, and as of yet, it's still not great. But you gain a few things by giving players and mobs some autonomy:

• Clarity. When you want a mob to attack, you don't have to set the player's health to some lower amount determined by the mob's damage. You just tell it to attack. The resulting code is much more self-descriptive, which makes it much easier to follow.

• Scalability. Eventually, you're going to want to add features -- like, say, health potions. Or spiffy weapons that increase the damage dealt. Or debuffs that occasionally make you or the mob miss a turn. If you leave the classes dumb and keep main doing all the manipulation, there will be a point where it's too painful to add more stuff.

  With OOP done right, on the other hand, you get stuff done by telling an object to do something -- which often causes it to give orders to another object, which causes that other object to send messages to a bunch of others, and so on. The work's spread out, much more granular, and thanks to polymorphism, hot-swappable -- you can change the program's behavior on the fly by simply replacing some object with a compatible one that does things the way you want it to.

---

As far as the game goes, you might want to change how fleeing works. Right now, if i run away from a fight, the monster dies? And i get as much EXP as if i had stayed and killed it? Something's not right about that.

Answer 4

For the classes, the get functions could have a const keyword in their prototype since they aren't supposed to change any value. That will just make it impossible for them to do so and will make it possible to use them in other const functions.

In the .h

int getDamage() const;

In the .cpp

int player::getDamage() const {...}

Also, the get and maybe even set functions could be declared inline, placed at the end of the .h file, to avoid function calls. Inline functions are not compiled as functions. Instead, the body of the function kind of replaces the function call when compiled, similar to Macros. This way, on runtime, there's no actual function call. It's perfect when returning values.

In the class declaration:

class player
{
public:
    int getDamage() const;
}

In the .h after the class declaration:

inline int player::getDamage() const {return damage;}

For the constructor, instead of calling the set functions, you could add the values before the function body:

classMob::classMob(std::string name,int lvl,std::string area,int difficulty)
: mobName(name), mobLevel(lvl), mobArea(area), mobDifficulty(difficulty)
{
    // ...
}