patterncppModerate
Text-based RPG game using classes
Viewed 0 times
textgameusingbasedrpgclasses
Problem
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
Player.h
```
#include
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()
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
#include
#include
#include
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 > name;
string location[4] = {"in a hole","in a cave","in the mauntains","in a castle"};
player account(name,location[0],1,0);
cout > option1;
if (option1 >=1 && option1 > option;
srand(time(NULL));
if (option == "R" || option == "r")
{
if ((rand() % 2) == 1){
cout 0 && account.getHealth() > 0);
cout = 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
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()
Solution
-
Try not to get in the habit of using
-
For clarity, have your
-
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.
-
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 `, 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:
- keep it in the .cpp file (with those changes)
- put it in the header (with the
classMob:: part removed)
- 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:
- it's clearer to let
main() terminate the program whenever possible
- 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.Code Snippets
void setEXP() {EXP = (getlevel() * 35;}
int getEXP() const {return EXP;}getline(std::cin, name);account = calcEXP(account, monster);
if (account.getHealth() <= 0)
return false;
return true;bool healthDepleted() const {return playerHealth <= 0;}bool battle(player &account);
void calcEXP(player &account, classMob &monster);Context
StackExchange Code Review Q#27986, answer score: 16
Revisions (0)
No revisions yet.