Text-based adventure game with too many conditional statements

Question

For a school project, I am creating a text-based adventure game in C++. I have shown my code to my teacher and he is stating my code is not entirely logical in sequence and that I should make my if statements inside functions because there are so many. I need help as to know how to make these changes.

Note that this is one function of the code, and not it's entirety.

void road() {
    int choice2;
    int choice3;
    int event1;
    system("cls");
    cout << "You are on a road that heads west and east of your position." << endl;
    cout << "Which way will you go?\n" << endl;

    cout << "1. Go West" << endl;
    cout << "2. Go East" << endl;
    cout << "3. Wait for something to happen\n" << endl;

    cin >> choice2;
    cin.ignore();
    system("cls");

    if (choice2 == 1) {
        int decision1;
        cout << "You travel down the road, about only 100 metres and you encounter " << endl;
        cout << "a giant spider with vicious poison coated fangs." << endl;
        cout << "its hideous appearance causes your throat to dry and your knees to shake!" << endl;
        cout << "What on earth will you do?\n\n" << endl;
        cout << "1. Attempt to attack the spider with you sword." << endl;
        cout << "2. Throw your sword in the off chance it might kill it." << endl;
        cout << "3. RUN FOR YOUR LIFE!" << endl;

        cin >> decision1;
        cin.ignore();
        system("cls");

            if (decision1 == 1) {
                cout << "You viscously swing your sword at the spiders general direction." << endl;
                cout << "The swing was so great, your arms jolts out of place, \ncreating a surge of pain." << endl;
                cout << "Your arm is now broken, and you fall to the ground in pain...." << endl;
                cout << "The spider launches 3 metres straight into your body... \nWhat on earth is it doing?" << endl;
                cout << "Oh My God! The spider is devouring everything...." << endl;
                cout << "All that remained was bones of the once mobile adventurer " << name << endl;
                system("pause");
                gameover();
            }
            else if (decision1 == 2) {}
            else if (decision1 == 3) {}



    }
    else if (choice2 == 2) {
        cout << "After a mile walk, you arrive at an old brick house. " << endl;
        cout << "You walk slowly inside." << endl;
        cout << "The door slams behind you and the room lightens up." << endl;
        cout << "What on earth is going on...?\n\n" << endl;
        system("pause");
        cout << "\nUnable to open the door, you look around for anything of use." << endl;
        cout << "Nothing, not a single piece of furniture." << endl;
        cout << "What will you do?\n" << endl;
        cout << "1. Wait for someone to save you." << endl;
        cout << "2. Or Wait for someone to save you." << endl;

        cin >> choice3;
        cin.ignore();
        system("cls");
            if (choice3 == 1) {
                cout << "As you character relaxes his legs and begins to sit" << endl;
                cout << "he leans against the wall, only to hit a hidden switch." << endl;
                system("pause");
                cout << "The floor beneath you disappears, and you start to plummet." << endl;
                cout << "What a trap! You bloody stupid fool!\n\n?" << endl;
                cout << "somehow you never reach the bottom....." << endl;
                cout << "You die of water deprivation, after only 2 weeks of falling...." << endl;
                system("pause");
                gameover();
            }
            else if (choice3 == 2) {
                system("cls");
                cout << "After only 5 hours..." << endl;
                cout << "The doors suddenly swoosh open." << endl;
                cout << "You are free!  " << endl;
                system("pause");
                cout << "You take your first steps into what is now moonlight," << endl;
                cout << "and then suddenly pass out on the ground...." << endl;
                cout << "For some peculiar reason or another, you are now /nback in the creepy dark and gloomy library. You swiftly escape its nostalgia. " << endl;
                road();
            }

    }

    else if (choice2 == 3) {
        int random1;
        cout << "After at least was seemed to be 4 hours " << endl;
        cout << "a dragon swoops down from the heavens...." << endl;
        cout << "and lands right in front of you!\n" << endl;
        system("pause");
        system("cls");

        random1 = rand() % 2; // pseudo random generator
            if (random1 == 0) {
                int choice4;
                cout << "Your character screams at the top of his lungs, " << endl;
                cout << "this causes the dragon to immediately to bow in fear..." << endl;
                cout << "It turns out dragons are very sensitive to hearing....." << endl;
                system("pause");
                cout << "\nIt seems the dragon is requesting you ride it!\n" << endl;
                cout << "Will you ride it?\n" << endl;
                cout << "1. Ride it" << endl;
                cout << "2. Or Wait here." << endl;

            cin >> choice4;
            cin.ignore();
            system("cls");

                if (choice4 == 1){
                    Ending();
                }
                else if (choice4 == 2){
                    road();
                }

        }
        else if (random1 == 1) {
            cout << "Your character screams at the top of his lungs, " << endl;
            cout << "eventually your breath gives out and you die because of lack of oxygen." << endl;
            system("pause");
            gameover();
        }

    }
}

Answer 1

I have found a couple of things that could help you improve your code.

Don't abuse using namespace std

Putting using namespace std at the top of every program is a bad habit that you'd do well to avoid. I don't know that you've actually done that, but it's an alarmingly common thing for new C++ programmers to do.

Avoid the use of global variables

I see that name is used but it's neither declared within the function nor passed to it, implying a global variable. It's generally better to explicitly pass variables your function will need rather than using the vague implicit linkage of a global variable.

Eliminate unused variables

This code declares a variable event1 but then does nothing with it. Your compiler is smart enough to help you find this kind of problem if you know how to ask it to do so.

Don't use system("cls")

There are two reasons not to use system("cls") or system("pause"). The first is that it is not portable to other operating systems which you may or may not care about now. The second is that it's a security hole, which you absolutely must care about. Specifically, if some program is defined and named cls or pause, your program will execute that program instead of what you intend, and that other program could be anything. First, isolate these into a seperate functions cls() and pause() and then modify your code to call those functions instead of system. Then rewrite the contents of those functions to do what you want using C++.

Fix your newline characters

The string starting with "For some peculiar reason" contains /n which is two characters, but it's clear from the context that you intended \n which is a single newline character.

Use a menu object or at least a common menu function

In a number of places in your code, you have something like a menu. Your code presents a couple of options and then asks the user to pick one based on an input number. Rather than repeating that code in many places, it would make sense to make it generic. Only the prompt strings actually change, but the underlying logic of presenting the choices and asking for input are all the same. It looks like you're a beginning programmer, and so perhaps you haven't learned about objects yet, but this kind of repeated task with associated data is really well-suited to object-oriented programming and that's something that C++ is very good at expressing.

Specifically, here's how I might approach this. I'd create a Menu object:

class Menu
{
public:
    Menu(const string &name, const string &prompt, 
        const std::vector<std::pair<string, string> > &choices 
        = std::vector<std::pair<string, string> >{});
    virtual ~Menu();
    const string& getChoice() const;
    bool operator==(const string &name) const;
private:
    static const string menuend;
    string _name, _prompt;
    std::vector<std::pair<string, string> > _choices;
};

Implementations are here:

Menu::Menu(const string &name, const string &prompt, 
        const std::vector<std::pair<string, string> > &choices) 
    : _name(name), _prompt(prompt), _choices(choices) 
{}

bool Menu::operator==(const string &name) const
{
    return name==_name;
}

const string& Menu::getChoice() const
{ 
    if (_choices.size() == 0) {
        cout << _prompt;
        return menuend;
    }
    unsigned choice; 
    int i;
    do { 
        cout << _prompt;
        i = 1;
        for (auto ch : _choices)
            cout << i++ << ": " << ch.first << '\n';
        cin >> choice; 
        --choice;
    } while (choice >= _choices.size()); 
    return _choices[choice].second; 
}

Menu::~Menu() 
{}

const string Menu::menuend{"END"};

Finally, we can construct the game itself as a std::vector of these Menu objects:

std::vector<Menu> game{  
    Menu("mainroad", 
            "You are on a road that heads west and east of your position.\n" 
            "Which way will you go?\n", std::vector<std::pair<string,string> >{
                {"Go West", "spider"}, 
                {"Go East", "brickhouse"}, 
                {"Wait for something to happen", "dragon"}}),
    Menu("spider", 
            "You travel down the road, about only 100 metres and you encounter \n" 
            "a giant spider with vicious poison coated fangs.\n" 
            "its hideous appearance causes your throat to dry and your knees to shake!\n"
            "What on earth will you do?\n\n", std::vector<std::pair<string, string> >{
                {"Attempt to attack the spider with your sword.","spiderattack"},
                {"Throw your sword in the off chance it might kill it.","throwsword"},
                {"RUN FOR YOUR LIFE!", "running"}}),
    Menu("spiderattack",
            "You viscously swing your sword at the spiders general direction.\n" 
            "The swing was so great, your arms jolts out of place,\n"
            "creating a surge of pain.\n" 
            "Your arm is now broken, and you fall to the ground in pain....\n" 
            "The spider launches 3 metres straight into your body...\n"
            "What on earth is it doing?\n" 
            "Oh My God! The spider is devouring everything....\n" 
            "All that remained was bones of the once mobile adventurer.\n"), 
    Menu("brickhouse",
            "After a mile walk, you arrive at an old brick house.\n" 
            "You walk slowly inside.\n" 
            "The door slams behind you and the room lightens up.\n" 
            "What on earth is going on...?\n\n" 
            "Unable to open the door, you look around for anything of use.\n" 
            "Nothing, not a single piece of furniture.\n" 
            "What will you do?\n", std::vector<std::pair<string, string> >{
                {"Wait for someone to save you.", "trapdoor"}, 
                {"Or Wait for someone to save you.", "library"}})
};

The game itself becomes entirely data-driven:

void road() {
    auto menu = std::find(game.begin(), game.end(), "mainroad"); 
    while (menu != game.end())
        menu = std::find(game.begin(), game.end(), menu->getChoice());
}

If you need more than the plain Menu class provides, you can simply derive a new kind of class and put that into the vector. It should also be obvious that this could very easily all be read as a script from a file, simply by defining an ostream extractor for the Menu class.

Consider using a better random number generator

You are currently using

random1 = rand() % 2;

There are two problems with this approach. One is that the low order bits of the random number generator are not particularly random, so neither with random1 be. On my machine, there's a slight but measurable bias toward 0 with that. The second problem is that it's not thread safe because rand stores hidden state. A better solution, if your compiler and library supports it, would be to use the C++11 std::uniform_int_distribution. It looks complex, but it's actually pretty easy to use. One way to do that (from Stroustrup) is like this:

int rand_int(int low, int high)
{
    static std::default_random_engine re {};
    using Dist = std::uniform_int_distribution<int>;
    static Dist uid {};
    return uid(re, Dist::param_type{low,high});
}

Note that this still stores state, but at least the distribution is correct.

Answer 2

I guess what your teacher meant is this:

if (choice == 1) {
    spider();

But there is a better way to write this. You adventure game is actually a Finite State Machine. You could implement it with a simple loop:

#include <iostream>

struct  state;
struct  transition;

struct transition {
    char *text;
    struct state *next_state;
};

struct state {
    char *text;
    struct transition transitions[8];
};

extern struct state start;
extern struct transition start_transitions[];
extern struct state spider;
extern struct transition spider_transitions[];

struct state start = {
    "You are on a road that heads west and east of your position.\n"
        "Which way will you go?",
    {
        {"Go West", &spider},
        {"Go East", NULL},
        {"Wait for something to happen", NULL},
        { NULL }
    }
};

struct state spider = {
    "You travel down the road, about only 100 metres and you encounter\n"
        "a giant spider with vicious poison coated fangs.\n"
        "Its hideous appearance causes your throat to dry and your knees to shake!\n"
        "What on earth will you do?",
    {
        { "Attempt to attack the spider with your sword.", NULL },
        { "Throw your sword in the off chance it might kill it.", NULL },
        { "RUN FOR YOUR LIFE!", NULL },
        { NULL }
    }
};


int main(void)
{
    state *cur = &start;
    while (cur) {
        // Print text
        std::cout << cur->text << std::endl;

        // List available choices
        unsigned trans = 0;
        while (cur->transitions[trans].text) {
            std::cout << trans << ". " << cur->transitions[trans].text << std::endl;
            trans += 1;
        }

        // Read input
        unsigned choice;
        std::cin >> choice;
        std::cin.ignore();

        // Check input is valid
        if (choice < trans) {
            // Move to new state
            cur = cur->transitions[choice].next_state;
        }    
    }
    return 0;
}

Of course, a more mature version of the game would read the states and transitions from a data file rather than include them directly in the code.

Answer 3

I would say we need to look at design of your game.

To me an adventure game is moving through a set of locations. At each locatoin you can move to another location (that is linked to the current location) or interact with objects at that location. So it seems to me that you want to build a graph of locations.

So first we have a game object.

 class Game
 {
      std::string  currentLocation;                // Assuming a one player game
                                                   // You only need to store that
                                                   // players location.

      std::map<std::string, Location>   locations; // All locations in the game
                                                   // You can look them up by name.
 };

So we have a game with locations. But how does that fit together.

 class Location
 {
     std::string                           description;
     std::map<std::string, std::string>    linked;       // Places linked to from
                                                         // here. The action is the key
                                                         // the location is the value
                                                         // and you can get the location
                                                         // details by looking up the 
                                                         // location in the map.
     std::vector<std::string>              thingsLyingHere;
 };

 Example:
 ========
   description: "You are on a road that heads west and east of your position."
   linked["Go West"]  = "TheCreepyCrossroads";
   linked["Go East"]  = "TheLongRoad";
   linked["Wait for something to happen"] = "Waiting At Start";

Now your main program looks at your current location. Looks that location up in the Game object and can print out the description and options for that location. When your user types in one of the available actions you look that up in linked and update the current location. Then you start the loop again.

Answer 4

Your code does a very poor job in separating code from data. I would suggest you split your projects into two parts: a data file to define your adventure and code which parses the data file and lets you play the game.

The following is a suggestion how it might work. You can come up with your own more sophisticated "grammar" for the data file. There are also tools around to do the parsing for your, but as this project is for learning, I would suggest your write your own parsing code.

If you want some custom code to run, like doing a fight or rolling a random choice, you can also add that to your grammar and link tags to functions you want to execute (for example using if statements to select the correct piece of code).

Have fun!

---

Data file

A normal text file, e.g. "the_dragon_tale.txt", which has all your text messages and the possible choices. The text file is annotated to indicate the program flow.

Example:

[road]
You are on a road that heads west and east of your position.
Which way will you go?
#west Go West
#east Go East
#roadwait Wait for something to happen

[west]
A spider!!
#fightspider

[east]
A dragon!
#endofgame

[roadwait]
Very boring.
#road

---

Code

Your program now looks completely different. First write a function which reads and parses your adventure text file. For each [tag] section it creates an instances of a class and links and the possible choices marked by the # sections.

Example:

struct Choice {
    std::string text;
    std::string target;
}

struct Encounter {
    std::string tag;
    std::string message;
    std::vector<Choice> choices;
}

std::string ReadFile(const std::string& filename) {
    /** check online resources */
}

Encounter ParseEncounter(const std::string& text) { ... }

std::map<std::string,Encounter> ParseEncounters(const std::string& text) {
    std::map<std::string,Encounter> encounters;
    size_t pos = 0;
    while(true) {
        // find next [
        size_t next = text.find("[");
        if(next == std::string::npos) break;
        // get the encounter text
        std::string enc_str = text.substr(pos,next);
        // parse encounter
        Encounter enc = ParseEncounter(enc_str);
        // store encounter
        encounters[enc.tag] = enc;
        // next 
        pos = next;
    }
    return encounters;
}

std::string RunEncounter(const std::map<std::string,Encounter>& encounters, const std::string& tag) {
    system("cls");
    // get encounter
    Encounter node = encounters[tag];
    // print message and choices
    std::cout << node.message << std::endl;
    for(size_t i=0; i<node.choices.size(); i++) {
        std::cout << i << ". " << node.choices[i].text << std::endl;
    }
    // get correct choice from user
    int choice = -1;
    while(!(0 <= choice && choice < node.choices.size())) {
        std::cin >> choice;
    }
    cin.ignore();
    // return choice tag
    return node.choices[choice].target;
}

int main() {
    std::string str = ReadFile("adventure.txt");
    std::map<std::string,Encounter> encounters = ParseEncounters(str);
    std::string tag = "road";
    while(tag != "endofgame") {
        tag = RunEncounter(tag);
    }
}

Answer 5

I'd start breaking your code up into chunks. There are a couple of ways you could do this, such as by giving each room its own function (and I can remember doing this with my first text-adventure type game, way back when!) or alternatively by defining a room as a data structure, like this:

struct Choice
{
  int nextroom;
  std::string text;

  Choice(int n, std::string t) : nextroom(n), text(t) {}
};

struct Room
{
  const std::string text;
  std::vector<Choice> choices;
  bool deathroom;

  Room(std::string t, std::vector<Choice> c, bool d = false)
   : text(t), choices(c), deathroom(d) {}
};

Now, you can define a collection of rooms:

std::vector<Room> rooms;
Room r1(
    "You are in room 1.\n",
    std::vector<Choice>{ Choice(2, "Option 1"), Choice(3, "Option 2") });
rooms.push_back(r1);

With this, you can turn your game into a single loop:

int room = 0;
while (rooms[room].deathroom == false)
{
    std::cout << rooms[room].text;
    room = get_choice(room);
}
std::cout << "Game Over!\n";

You'll need to implement get_choice yourself, of course ;)

This makes it much simpler to add and remove rooms, or even in the future read room definitions from a text file, and so on.

Answer 6

I agree with your teacher that breaking the code into functions will help the readability of your code.

For example, put the code, for the result of each decision, in its own function.

cout << "1. Go West" << endl;
...

if (choice2 == 1) {
    go_west();
} else if (choice2 == 2) {
    go_east();
} else if (choice2 == 3) {
    wait();
}

If I am reading the code this makes it much easier to figure out what is happening. What happens if I input 2? I do not need to skip over all the go west or wait for something to happen code because they are in functions somewhere else. If I am interested in that code then I look at the function.

This also helps if two routes lead to the same result, you do not need to copy and paste the code you simply call the same function.

If you have learned about switch statements then it might get you some more points to use a switch instead of if/else.

switch (choice2) {

case 1:
    go_west();
    break;
case 2:
    go_east();
    break;
case 3:
    wait();
    break;
}

This is not very important in the real world though. Use whichever you think looks better.

It would also be an idea to use enums or constant values when processing the decision points. If I look at the if/else statements I need to remember: does 1 mean east or west?

#define WEST 1
#define EAST 2
#define WAIT 3

if (choice2 == WEST) {
    go_west();
} else if (choice2 == EAST) {
    go_east();
} else if (choice2 == WAIT) {
    wait();
}

A more advanced strategy would be to use a jump table.

void(*tbl[4])(void) = { 
    &invalid_input,
    &go_west,
    &go_east,
    &wait
};

cin >> choice2;

if (choice2 > 3)
    choice2 = 0;

tbl[choice2]();

This may not be a viable option for you at your current stage but it might help other people or yourself later on in your education.

Answer 7

A good general rule is that if the code takes more than two screens of text, it should be split into functions. Sure you can read your spaghetti code, but always remember that you're coding for other people to read it. These people may be your teacher (at school), your co-workers (when you leave school), or you in 6 months time (when you look at it and go "WTF?!").

The big thing with functions though is that you may have more than one way of getting somewhere. If you're on a flat featureless plain and you go north then west, you'd usually expect to get to same place as if you go west then north. Your code simply doesn't allow this to happen though. Do you want to copy-and-paste code to do the same thing twice? (If your answer is anything other than variations on the theme of "hell no!" then you've just failed your programming aptitude test. ;) Functions will let you do this very easily, so use them.

One thing in your code that you want to fix is re-entrant function calls. What I mean by this is that you're calling road() from within road(). This is almost always a bad thing to do. If there are any changes to global resources during the function (hint: cls() changes the state of a global resource...) then your re-entry may do things you didn't want it to. You'd need to keep track of how each re-entry affects the ones before it, which gets epicly complex very quickly. Worse, each time you re-enter, you use up a new chunk of stack space. On really small systems (like Arduino) you can easily use up more memory than the system has free, and it all crashes. Even on big systems like PCs, if you accidentally set up something which re-enters in a loop and keeps going for a while then you can kill things that way too. You really need to fix this, because it's very bad practice.

Oh, and have a play of some of the classic 80's text adventure games. "Planetfall" or the "Zork" series are top tips.

Answer 8

An important rule of thumb when programming is to make function blocks small enough to fit entirely on the screen, let's say, no more than about 20 lines of code. Otherwise you easily lose overview when writing really big programs.

So, split your code into chunks you could give a meaningful name, put each chunk into a subfunction you give just that name and replace the code chunk in your original function by just calling that subfunction. In case any local variables are needed in your code chunk pass them as parameters of your subfunction and return any values needed further.

Answer 9

Grouping functionality in functions would be a first step. You could add functions like that:

void printStoryText(std::string storyText) { /* impl */ }

int decide(std::string decisionText,
  const std::vector<std::string>& choices) { /* impl */ }

Put the cout and pause statements in the printStoryText. Decisions should be done with the decide function which accepts a string containing the decision description and a vector of strings with the possible choices. It returns the index of the chosen element.

Add individual functions for each step of your story and call them as result of the decision outcome. Make separate functions for the brick house, the dragon, etc.

Of course this strategy won't be sufficient as your adventure game grows bigger. I would suggest to abstract the game logic into a data structure suitable for that task. A tree would be the best choice I think. The following code samples should illustrate the idea, they are not complete (most importantly they are missing most of the public interface):

struct Choice
{
  std::string choice_text;
  std::shared_ptr<DecisionNode> next_node;

  Choice(const std::string& choice, std::shared_ptr<DecisionNode> next)
    : choice_text(choice)
    , next_node(next)
  {}
};

class DecisionNode
{
public:
  DecisionNode(const std::string& text)
    : story_text_(text)
  {}

  void AddChoice(const std::string& text, std::shared_ptr<DecisionNode> nextNode)
  {
    child_nodes_.push_back(Choice(text, nextNode));
  }

  std::shared_ptr<DecisionNode> Decide() const
  {
    std::vector<Choice>::const_iterator it = choices_.begin();
    for(unsigned int i = 1; it != choices_.end(); ++it, ++i)
    {
      std::cout << i << ". " << it->choice_text << std::endl;
    }

    unsigned int selected;
    std::cin >> selected;

    return choices_.at(selected);
  }

private:
  std::string story_text_;
  std::vector<Choice> choices_;
}

You would then build a tree first before actually starting the game logic by creating a root node with the top level story and the following decision. Each choice of a decision has a choice text and a next node which will be selected when chosen. The Decide() function returns the next node. This will absorb the many if/else statements that you would need otherwise into the tree structure.

Answer 10

Just looking at it, I have to say: this seems like an engaging game. I especially like the visual image of "viscously swinging a sword". I envision something very gooey, like a limb made from molasses, or anything else with that viscosity. ;-]

In my opinion the if statements are fine and natural. Functions have a lot of limitations. In a game that might end up involving something like an Inventory or a Health Bar functions can be dead ends, unless you write a lot of intricate code to make them work.

On the other hand, you have different fingers. Relying upon a decision tree such as you have done, representing the branches as nested conditional statements, can be an even greater headache to follow. That being said, only you can account for your own biological memory. An outsider looking at it, paid to educate you, might be hypercritical, but his or her confusion need not impede the development of your own, personal style. The most important question is this: if you are developing a game independently, coding alone, will YOU be able to remember what you were thinking when you wrote it? Will it make sense in a month? A year? A decade?

Your functions, if you choose to use them, ought to print an outcome and then return some sort of variable that, once returned, will direct your program in such a way that either restricts or expands upon the player's freedom. For instance, you could split your game into several rooms, and each function might return a number that will direct you into one room rather than another. The same can be done for things like situations, events, and health bars. Just so long as it can be represented by an integer (or some other form of generic data.).

Really, it's all up to you. In your place, I would only use functions for things like Inventory. But that's just me. I only found this forum owing to my own experience in programming this sort of thing in C.