Ytdyklly

I have made this small text-based RPG in C++ which is based around one quest. I did this to practice what I have learnt so far. How could I improve it? Be as picky as you'd like.

#include <iostream>

#include <stdlib.h>

#include <time.h>



using namespace std;



void riverstead();

void aragornHouse();

void stage1();

void stage2();

void stage3();

void stage4(int &sword, int &gold);

void ratCave();

void attackThief(int &pHealth, int &tHealth);

void thiefDead();

void searchBody();

void questUpdate();

void end();



int input;

int stages[5] = {1, 0, 0, 0, 0};

string qUpdates;

string qStages;



int main() {



    srand (time(NULL));



    system("cls");

    cout << "n Welcome to RPG!" << endl;

    cout << "n 1. Play" << endl;

    cout << "n 2. Exit" << endl;

    cout << "n> ";

    cin >> input;

    switch (input) {



        case 1:

        qUpdates = "Quest begun";

        qStages = "Talk to Aragorn in his house";

        questUpdate();



        riverstead();



        case 2:

        exit(0);



    }

}



void riverstead() {



    system("cls");

    cout << "n You are in the town of Riverstead. Where would you like to go?" << endl;

    cout << "n 1. Aragorn's House" << endl;

    cout << "n 2. Rat Cave" << endl;

    cout << "n> ";

    cin >> input;

    switch (input) {



        case 1:

        aragornHouse();



        case 2:

        ratCave();



    }   

}



void aragornHouse() {



    if (stages[0] == 1) {

        system("cls");

        cout << "n Aragorn: You interested in doing something for me? There's gold in it for you." << endl;

        cout << "n 1. What do you need?" << endl;

        cout << "n 2. I'm kind of busy at the moment." << endl;

        cout << "n> ";

        cin >> input;

        switch (input) {



            case 1:

            stage1();



            case 2:

            system("cls");

            cout << "n If you find the time, I'll be here." << endl;

            cout << "n ";

            system("pause");

            riverstead();



        }

    }



    if (stages[1] == 1) {

        stage2();

    }



    if (stages[2] == 1) {

        stage3();

    }



    else {

        stage4(sword, gold);

    }

}



void stage1() {



    system("cls");

    cout << "n There's a sword that has been in my family for generations and it was recently" << endl;

    cout << "n passed down to me from my father. " << endl;

    cout << "n ";

    system("pause");



    system("cls");

    cout << "n I was walking back from the market the other day and I saw this thief sneaking" << endl;

    cout << "n out of my house with the sword! " << endl;

    cout << "n ";

    system("pause");



    system("cls");

    cout << "n He ran and I followed him to a nearby cave, it's called Rat Cave." << endl;

    cout << "n ";

    system("pause");



    system("cls");

    cout << "n I didn't go inside because I had no way to defend myself." << endl;

    cout << "n 1. I could get that sword for you." << endl;

    cout << "n 2. I don't think I'm the right person for the job." << endl;

    cout << "n> ";

    cin >> input;

    switch (input) {



        case 1:

        stages[0] = 0;

        stages[2] = 1;

        system("cls");

        cout << "n That's great! I'll be waiting right here." << endl;

        cout << "n ";

        system("pause");



        qUpdates = "Quest updated";

        qStages = "Kill the thief in Rat Cave";

        questUpdate();



        riverstead();



        case 2:

        stages[0] = 0;

        stages[1] = 1;

        system("cls");

        cout << "n Maybe I could find someone else to do this for me." << endl;

        cout << "n ";

        system("pause");

        riverstead();



    }

}



void stage2() {



    system("cls");

    cout << "n Have you changed your mind? Will you retrieve my sword?" << endl;

    cout << "n 1. Yes, I'm ready to retrieve the sword." << endl;

    cout << "n 2. I still don't feel like retrieving it." << endl;

    cout << "n> ";

    cin >> input;

    switch (input) {



        case 1:

        stages[0] = 0;

        stages[1] = 0;

        stages[2] = 1;

        system("cls");

        cout << "n That's great! I'll be waiting right here." << endl;

        cout << "n ";

        system("pause");



        qUpdates = "Quest updated";

        qStages = "Kill the thief in Rat Cave";

        questUpdate();



        riverstead();



        case 2:

        system("cls");

        cout << "n That's a shame. Talk to me if you change your mind." << endl;

        cout << "n ";

        system("pause");

        riverstead();



    }

}



void stage3() {



    system("cls");

    cout << "n Have you retrieved the sword yet?" << endl;

    cout << "n 1. I'm working on it." << endl;

    cout << "n> ";

    cin >> input;

    switch (input) {



        case 1:

        system("cls");

        cout << "n Let's hope the thief is still there by the time you get round to doing it." << endl;

        cout << "n ";

        system("pause");

        riverstead();



    }

}



void stage4(int &gold, int &sword) {



    system("cls");

    cout << "n Have you retrieved the sword yet?" << endl;

    cout << "n 1. Yup. Was a piece of cake." << endl;

    cout << "n 2. Yes, at the price of almost getting killed." << endl;

    cout << "n> ";

    cin >> input;



    system("cls");

    cout << "n That's brilliant! I knew you were the right man for the job. Here is the gold," << endl;

    cout << "n as promised." << endl;

    cout << "n ";

    system("pause");



    sword = 0;

    system("cls");

    cout << "n Item removed - Aragorn's Sword" << endl;

    cout << "n ";

    system("pause");



    gold = gold + 100;

    system("cls");

    cout << "n Item added - 100 Gold" << endl;

    cout << "n ";

    system("pause");



    qUpdates = "Quest complete";

    qStages = " ";

    questUpdate();



    end();



}



void ratCave() {



    int pHealth;

    int pDamage;

    int tHealth;

    int tDamage;

    int turn;



    if (stages[0] == 1 || stages[1] == 1) {

        system("cls");

        cout << "n I haven't really got a good reason to go here." << endl;

        cout << "n ";

        system("pause");

        riverstead();

    }



    system("cls");

    cout << "n Thief: I'm warning you, stranger. Leave now!" << endl;

    cout << "n 1. I've come for my friend's sword." << endl;

    cout << "n 2. Okay, okay, I'm leaving!" << endl;

    cout << "n> ";

    cin >> input;

    switch (input) {



        case 1:

        system("cls");

        cout << "n Ha! You won't be leaving with it!" << endl;

        cout << "n ";

        system("pause");



        pHealth = rand() % 40 + 80;

        tHealth = rand() % 20 + 40;



        turn = rand() % 2;



        if (turn == 1) {

            system("cls");

            cout << "n The thief has the first turn." << endl;

            cout << "n ";

            system("pause");



            tDamage = rand() % 5 + 10;



            pHealth = pHealth - tDamage;

            system("cls");

            cout << "n The thief attacks you for " << tDamage << " damage!" << endl;

            cout << "n ";

            system("pause");



        }



        else {

            system("cls");

            cout << "n You have the first turn." << endl;

            cout << "n ";

            system("pause");

        }



        attackThief(pHealth, tHealth);



        case 2:

        system("cls");

        cout << "n That's what I thought." << endl;

        cout << "n ";

        system("pause");

        riverstead();



    }

}



void attackThief(int &pHealth, int &tHealth) {



    int pDamage;

    int tDamage;



    pDamage = rand() % 10 + 10;

    tDamage = rand() % 5 + 10;



    system("cls");

    cout << "n Your health: " << pHealth << endl;

    cout << "n Thief's health:  " << tHealth << endl;

    cout << "n What would you like to do?" << endl;

    cout << "n 1. Attack the thief" << endl;

    cout << "n 2. Attempt to flee" << endl;

    cout << "n> ";

    cin >> input;

    switch (input) {



        case 1:

        tHealth = tHealth - pDamage;

        system("cls");

        cout << "n You attack the thief for " << pDamage << " damage!" << endl;

        cout << "n ";

        system("pause");



        if (tHealth < 1) {

            system("cls");

            cout << "n You have killed the thief!" << endl;

            cout << "n ";

            system("pause");

            thiefDead();

        }



        pHealth = pHealth - tDamage;

        system("cls");

        cout << "n The thief attacks you for " << tDamage << " damage!" << endl;

        cout << "n ";

        system("pause");



        if (pHealth < 1) {

            system("cls");

            cout << "n You have been killed by the thief!" << endl;

            cout << "n ";

            system("pause");

            exit(0);

        }



        attackThief(pHealth, tHealth);



        case 2:

        system("cls");

        cout << "n Your attempt to flee is unsuccessful." << endl;

        cout << "n ";

        system("pause");

        attackThief(pHealth, tHealth);



    }

}



void thiefDead() {



    qUpdates = "Quest updated";

    qStages = "Retrieve Aragorn's Sword";   

    questUpdate();



    system("cls");

    cout << "n What would you like to do?" << endl;

    cout << "n 1. Search the thief's body" << endl;

    cout << "n 2. Leave the cave" << endl;

    cout << "n> ";

    cin >> input;

    switch (input) {



        case 1:

        searchBody();



        case 2:

        riverstead();



    }

}



void searchBody() {



    int gold;

    int sword;



    gold = gold + 20;

    sword = 1;

    stages[2] = 0;

    stages[3] = 1;



    system("cls");

    cout << "n You found 20 gold and Aragorn's Sword!" << endl;

    cout << "n ";

    system("pause");



    qUpdates = "Quest updated";

    qStages = "Return to Aragorn";  

    questUpdate();



    return;



}



void questUpdate() {



    system("cls");

    cout << "n " << qUpdates << " - Aragorn's Sword" << endl;

    cout << "n ";

    system("pause");



    if (qStages != " ") {

        system("cls");

        cout << "n " << qStages << endl;

        cout << "n ";

        system("pause");

    }



    return; 



}



void end() {



    system("cls");

    cout << "n Thank you for playing RPG! A game made by Elliot Morgan." << endl;

    cout << "n ";

    system("pause");

    main();



}

Don't do using namespace std in global scope, instead use it inside the functions or even better only on the things you are using:

int main()

{

  using std::cout;

  ...

}

In your main, you have exit(0), instead you should do a return statement.

return EXIT_SUCCESS;

You don't have a default case in your switch statements, so if a user enters the wrong number nothing happens and the program ends without notice. It is better to have some kind of error handling. You should also do a function that shows a number of options and lets the user enter a value that is returned if correct, that way you save some typing.

e.g.

/**

 * show a number choices and lets the user choose one

 * @returns 1-n

 */

int promptUser( const std::vector<std::string>& options );

Your program has the structure of a C program, you should use classes to encapsulate functionality. Identify the objects in the story and create appropriate classes.

e.g. Aragorn, Thief, Player have some common traits

system("pause"); 

Calling external programs like that is not a good thing, it opens up a security hole in your application instead use std::getline or similar.

You forgot to initialize some variables e.g.

int gold;   <---

int sword;



gold = gold + 20;

Always make it a habit to initialize variables when you declare them.

Don't call main(). It makes the program flow difficult to follow, instead have a loop in main() if you want to allow restart of the game.

There is a lot to be improved here. Number one is that you have huge blocks of code in super methods that handle entire moves. You should split that up into smaller logical groups for ease of reading, debugging, maintenance, and additions. One example I noticed is that you have this a lot:

cout << "n 1. Attack the thief" << endl;

cout << "n 2. Attempt to flee" << endl;

Maybe you should make a method to print the options:

void printOptions(std::vector<std::string> ops)

{

    for(const auto& opt : ops) {

        std::cout << &elem - &v[0] << opt << std::endl;

    }

}

Then, you can just call this:

printOptions({"Attack the thief", "Attempt to flee"});

This is cleaner than the other version, and is reusable, so it will shorten your code. You can also have as many arguments as you wish, so if you want three options sometime, no problem.

Another problem you have is you are using namespace std;. This is bad, because later you may define your own namespace or use a different one that has some methods named the same, which could cause problems.

Your switch statements should not look like this:

switch (input) {



    case 1:

    searchBody();



    case 2:

    riverstead();



}

Instead, the case statements should be indented, like this:

switch (input) {

    case 1:

        searchBody();



    case 2:

        riverstead();

}

Also, I believe you have an error here. In C++, and most other languages, once a matching case statement is reached, you continue executing all lower case statements. If case 1: is matched and searchBody(); is executed, that means riverstead(); is also executed. You should add a break; statement to the end of each case block:

switch (input) {

    case 1:

        searchBody();

        break;



    case 2:

        riverstead();

        break;        // unnecessary here because it is the last statement, but good practice.

}

I prefer if/else statements to switch statements, but switch statements are sometimes helpful.

Again, you have huge methods that should be split up into smaller blocks. I'm sure there are other things you can clean up, but this will be a good start

If you have C++11, use nullptr instead of NULL.

srand (time(nullptr));

In general though, you want to avoid using rand() and instead take advantage of the improved random facilities from <random>. A much better generator is std::mt19937. And instead of rand() % N + M, which gives non-uniform results, you can use std::uniform_int_distribution<> dist(N, M);. For a seed, instead of time(nullptr), you can use std::random_device.

std::mt19937 gen(std::random_device{}());

std::uniform_int_distribution<> dist(1, 10);

dist(gen); // Returns number in [1, 10]

Don't use std::default_random_engine because that might use rand(). Always prefer std::mt19937 as a good default.

If you don't have a C++11 capable compiler, use Konrad Rudolph's
suggestion:

unsigned result;

do {

    result = rand();

} while (result > N);

Of course, that method is slow but it does produce a good
distribution. A slightly smarter way of doing this is to find the
largest multiple of N that is smaller than RAND_MAX and using that
as the upper bound. After that, one can safely take the result % (N + 1).

Required reading:

rand() Considered Harmful - Stephan T. Lavavej, Going Native 2013

Using rand() - Julienne Walker

How do I scale down numbers from rand()?

If you've managed to ditch rand(), you can get rid of <stdlib.h> and <time.h>. Otherwise, prefer the headers added by the C++ standard, <cstdlib> and <ctime>, because <xxx.h>-style headers are deprecated. A long and comprehensive answer found in std::transform() and toupper(), no matching function by Lightness Races in Orbit explains the caveats you need to be aware of regarding these headers. Since it is unspecified whether or not C library functions like toupper appear in the global namespace when using <xxx>-style headers, you should always prefix those functions with std::.

In general, system() is considered bad practice. To quote R..:

system is a bad idea for several reasons:

• Your program is suspended until the command finishes.


• It runs the command through a shell, which means you have to worry about making sure the string you pass is safe for the shell to
  evaluate.


• If you try to run a backgrounded command with &, it ends up being a grandchild process and gets orphaned and taken in by the init
  process (pid 1), and you have no way of checking its status after
  that.


• There's no way to read the command's output back into your program.

Aside from those reasons, it's just bad user experience. Clearing the screen and requiring the user to press ENTER several times throughout the program is really annoying.