9
\$\begingroup\$

I'm working on a text adventure game for a while now, learning more about user input etc. I have a feeling there is something fundamentally wrong about my code, it seems too repetitive.

The use of structs should make things less bulky but can't figure out how to use it properly.

#include <stdio.h>
#include <time.h>
#include <string.h>
#include <ctype.h>
#include <stdlib.h>

// FUNCTIONS
int readLine();
int execute();
void startUp();
void readLocation();
void executeOpen();
void executeOpenDoor();
void executeOpenFridge();
void executeReadSign();
void executeGo();

// LOCATIONS
struct location {
    const char *description;
    const char *name;
}
locs[] = {
    {""},
    {"hallway", "hall"},
    {"kitchen", "kitchen"},
    {"living room", "living room"},
    {"toilet", "toilet room"},
    {"upstairs", "first floor"},
};
void loc_kitchen();
void loc_living();
void loc_hall();
void loc_toilet();
void loc_upstairs();

// INIT
int answer, location;
int bullets, key, gun = 0;
char* current_loc = "hall";
static char input[100];

// MAIN GAME
int main()
{    
    startUp(); // INTRO

    while (readLine() && execute()); // GAME LOOP

    return 0;
}


// FUNCTIONS
// COMMAND & READLINE
int readLine ()
{
    printf(">  ");
    return fgets(input, sizeof(input), stdin) != NULL;
}

int execute()
{
    char *verb = strtok(input, " \n");
    char *noun = strtok(NULL, " \n");

    if (verb != NULL)
    {        
        if (strcasecmp(verb, "open") == 0) 
        {
            executeOpenDoor(noun);
        }
        else if (strcasecmp(verb, "read") == 0) 
        {
            executeReadSign(noun);
        }
        else 
            printf("I don't know the word %s, try again.\n\n", verb);
    }
    return 1;
}

void executeOpenDoor(const char *noun)
{
    if (noun == NULL)
    {
        printf("What do you want to open?\n\n");
    }
    else if (strcasecmp(noun, "door") == 0)
    {        
        printf("You enter the mansion, seems like nobody's been here in years..\n");
        printf("You now have access to the kitchen, toilet, living room & upstairs.\n\n");
        readLocation();
    }
    else
    {
        printf("I don't understand what you want to open.\n\n");
    }
}

void readLocation() 
{
    while (1)
    {
        readLine();

        char *verb = strtok(input, " \n");
        char *noun = strtok(NULL, " \n");

        if (strcasecmp(verb, "go") == 0)
        {
            executeGo(noun);
        }
        else
        {
            printf("I don't understand where you want to go.\n\n");
        }
    }
}

void executeOpenFridge(const char *noun)
{
    if (noun == NULL)
    {
        printf("What do you want to open?\n\n");
    }
    else if (strcasecmp(noun, "fridge") == 0)
    {        
        printf("Oh wish you didnt opened that. Whatever's in it, it's definitely out-of-date.\n\n");
    }
    else
    {
        printf("I don't know what you want to open.\n\n");
    }
}

void executeReadSign(const char *noun)
{
    if (noun == NULL)
    {
        printf("What do you want to read?\n\n");
    }
    else if (strcasecmp(noun, "sign") == 0)
    {
        printf("\"Begone, leave the dead in peace!\"\n\n");
    }
    else
    {
        printf("I don't know what you want to read.\n\n");
    }
}

void executeGo(const char *noun)
{    
    if (strcasecmp(noun, current_loc) == 0)
    {
        printf("You are already standing in the %s.\n\n", current_loc);
    }
    else if (noun == NULL)
    {
        printf("Where do you want to go?\n\n");
    }
    else if (strcasecmp(noun, "kitchen") == 0)
    {
        loc_kitchen();
    }
    else if (strcasecmp(noun, "toilet") == 0)
    {
        loc_toilet();
    }
    else if (strcasecmp(noun, "hall") == 0)
    {
        loc_hall();
    }
    else if (strcasecmp(noun, "living") == 0)
    {
        loc_living();
    }
    else if (strcasecmp(noun, "upstairs") == 0)
    {
        loc_upstairs();
    }
    else
    {
        printf("I don't know where you want to go.\n\n");
    }
}

void loc_hall()
{
    current_loc = "hall"; // ADD LOCATION
    printf("You have access to the kitchen, toilet, living room & upstairs.\n\n");

    while (1)
    {
        readLine();

        char *verb = strtok(input, " \n");
        char *noun = strtok(NULL, " \n");

        if (strcasecmp(verb, "go") == 0) 
        {
            executeGo(noun);
        } 
        else
        {
            printf("I don't know the word %s.\n\n", verb);
        }
    }
}

void loc_kitchen()
{
    current_loc = "kitchen"; // ADD LOCATION
    printf("There are several cupboards and drawers ajar, there's also a weird\n");
    printf("smell coming from the fridge.\n\n");

    while (1)
    {
        readLine();

        char *verb = strtok(input, " \n");
        char *noun = strtok(NULL, " \n");

        if (strcasecmp(verb, "search") == 0) 
        {
            if (gun == 1) {
                gun++;
                printf("You filled your shotgun with bullets.\n");
                printf("When you put the bullets in the gun, you hear a door being slammed shut upstairs.\n\n");
            }
            else if (gun == 2 || bullets == 1){
                printf("You already found ammo in the drawers.\n\n");
            }
            else 
            {
                printf("In one of the drawers you found some salt bullets. These might come in handy!\n\n");
                bullets++;
            }
        } 
        else if (strcasecmp(verb, "open") == 0) 
        {
            executeOpenFridge(noun);
        }
        else if (strcasecmp(verb, "go") == 0) 
        {
            executeGo(noun);
        } 
        else
        {
            printf("I don't know the word %s.\n\n", verb);
        }
    }
}

void loc_living()
{
    current_loc = "living"; // ADD LOCATION
    printf("The furniture is covered with white cloth, but the colour has become\n");
    printf("yellow out of age. The carpet has blood and dirt stains on it.\n");
    if (!gun)
    {
        printf("Above the fireplace you see a double-barreled shotgun.\n");
    }
    printf("\n");

    while (1)
    {
        readLine();

        char* verb = strtok(input, " \n");
        char* noun = strtok(NULL, " \n");

        if (strcasecmp(verb, "take") == 0) 
        {
            if (bullets) {
                gun = 2;
                printf("You got yourself a gun, you filled it up with the salt bullets you found in the kitchen.\n");
                printf("When you put the bullets in the gun, you hear a door being slammed shut upstairs.\n\n");
            }
            else if (gun > 0)
            {
                printf("You already have the gun.\n\n");
            }
            else 
            {
                gun++;
                printf("You took the gun, empty.. We need some find some bullets.\n\n");
            }
        }
        else if (strcasecmp(verb, "go") == 0) 
        {
            executeGo(noun);
        } 
        else
        {
            printf("I don't know the word %s.\n\n", verb);
        }
    }
}

void loc_toilet()
{
    current_loc = "toilet"; 
    printf("You sure have a small bladder, couldn't you go before we started playing?\n\n");
    readLocation();
}

void loc_upstairs()
{
    current_loc = "upstairs"; 
    if (gun != 2) {
        printf("Maybe we need to find something to defend ourself first.\n\n");
    }
    else
    {
        printf("There are 2 doors, which one do you want to take? Left or right?\n\n");
    }
}

void startUp()
{
    printf("You stand in front of the mansion, there is a sign on the door.\n\n");
}
```
\$\endgroup\$
  • \$\begingroup\$ locs[] is not used. Why is it in the code? \$\endgroup\$ – chux - Reinstate Monica May 23 at 21:20
  • \$\begingroup\$ I meant to used it in the function ExecuteGo, i wanted to use a ‘for’ loop To go through all the values of the locs struct instead of using every if else statement. \$\endgroup\$ – Cuypers Michel May 23 at 22:23
  • \$\begingroup\$ In terms of a "cleanup", recommend to leave out meant to use code. \$\endgroup\$ – chux - Reinstate Monica May 24 at 12:44
5
\$\begingroup\$

Static functions

Given that this is a one-file program, all of your functions (except main) and global variables should be marked static as they will not be used in other translation units.

stdbool

Functions like readLine should return bool (from stdbool.h), and not int.

This will also allow

while (1)

to change to

while (true)

Global state

Most of your global variables after // INIT should be moved. input should just be a local variable. The others could be moved into a game state structure that gets passed around, to enable re-entrance.

Simple output

I prefer puts to printf when you are only outputting a string literal with no formatting. Note that puts includes a newline, so

printf("What do you want to open?\n\n");

would turn into

puts("What do you want to open?\n");

but printf("> "); would stay as-is.

The reasons I prefer this change:

  • it produces more terse code;
  • if we were to assume a non-optimizing compiler, printf would be slower; and
  • puts is constrained to a much simpler set of behaviour.

executeGo

Rather than representing this as a long list of if statements, you could factor it out into an array of string/function-pointer pairs. Iterate through them until you find a matching string and call the appropriate function. If this list gets longer as you add to the game, consider using a dictionary library.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Maybe for a beginner you should explain why you prefer puts over printf. \$\endgroup\$ – pacmaninbw May 24 at 10:36
  • \$\begingroup\$ Didn't expect so much advice, much appreciated @Reinderien! I do have some questions. I declared static functions at the top, should the function itself also need static in front of it? With stdbool should my return 0 also be return false or do you keep it to return 0? (int main). If 'input' is a local variable, do i use a pointer to get it to be used in the other functions? \$\endgroup\$ – Cuypers Michel May 24 at 11:53
  • \$\begingroup\$ Note: To get the same functionality printf("> "); is not replaceable with puts(). Could use fputs("> ", stdout); "prefer puts to printf" does not discuss the '\n' added by puts() so sounds like you are advocating a simple switch of functions without taking the '\n' into account. \$\endgroup\$ – chux - Reinstate Monica May 24 at 12:39
  • 1
    \$\begingroup\$ I declared static functions at the top, should the function itself also need static in front of it? - For clarity I always recommend that function signatures in the declaration and definition be exactly the same. \$\endgroup\$ – Reinderien May 24 at 13:40
  • 1
    \$\begingroup\$ With stdbool should my return 0 also be return false - yes. \$\endgroup\$ – Reinderien May 24 at 13:41
4
\$\begingroup\$

There are a few ways that struct types, and restructuring in general, could clean up your code.

Stack Overflow

First, I'll point out that your code is infinitely mutually recursive. If a player goes from the kitchen to the hall and back again, over and over, the stack will overflow.

As such, you should first concentrate on eliminating this recursion. Understand why you felt the need to encode it, and move that reason into some kind of data structure that is independent of the stack.

The three most important things in real estate:

Your various loc_ functions seem to have a similar structure. First they set a location string, then they mostly print a static message (with one exception), then they maybe print some extra text depending on the player's inventory or past actions.

Some of the locations then enter a nested command loop, but that should be addressed above.

So if you had a data structure that encoded those data items, you could process all the loc_ code with a single function. Something like:

typedef struct LOCATION {
    const char * name;
    const char * enter_msg;
    struct CONDITIONAL_MESSAGE {
        int    item_id;
        const char * per_item_msg;
    } * conditional_messages;
} LOCATION;

If your maze grows to require it, you might include a function pointer for really complex rooms. Also, you might want to have a "first time" entry message and a "every other time" entry message, so the game doesn't get too verbose.

Sic transit gloria mundi!

English verbs are divided between transitive and intransitive forms. Transitive verbs take an object while intransitive verbs do not.

An example of an intransitive verb would be "quit" -- the command you should always implement first. A transitive verb would be something like "go kitchen" or "read note".

Most of your verbs are transitive, which is fine. But the transitive verbs have a very similar structure when you process them:

1. Was there an object specified? If not, snark.
2. Is the object valid for this verb? If so, do something.
3. If not, snark.

So that leads to the suggestion that you move as much of this structure as possible into your parsing engine, and clean up the rest of your code.

struct VERB {
    unsigned flags;
    const char * word;
    const char * no_object_msg;
    const char * bogus_object_msg;
    // maybe a helper function?
};

Lots and lots of lists and lists

When thinking about transitive verbs, there are three obvious sources for objects. First, there is the map itself. The various "go XXX" commands will change based on where a player is standing. So it makes sense for there to be a list of rooms that is currently reachable.

The "take" verb, and the "open" verb, both suggest that there should be a list of items in the room. Some of those items are take-able, like the gun or the ammo. If taken, these items will leave the room and move into the player's inventory. Other items are permanently in the room, like a door or the fridge. You can still open them, but cannot have them in inventory. (A bit flag would be sensible for this. CAN_TAKE, CAN_OPEN, etc.)

Finally, there are the items in the player's inventory. These items will "always" be available, regardless of what room the player is in. You have used global variables for this, which IMO is a mistake. Better to create an array or a list.

Once you have all these lists figured out, you can search them for verb-objects. If the player enters, "take gun", it makes sense to check her inventory for the gun and print "you already have that", then check the room inventory for a gun that is take-able.

Hope this helps.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Advice is great, i've been working on it, especially the Stack overflow part but i have to admit my knowledge isn't advanced yet to implement everything. Is it possible to check what I have done for now? github link \$\endgroup\$ – Cuypers Michel May 25 at 20:35
  • \$\begingroup\$ As a starting point, try this: create a struct for your verbs with only two elements, the word (string) and a function pointer to the handler function. The code in your execute function where you use if/else to determine which verb has been input can be replaced by a loop, where you compare the word and invoke the function pointer when a match is found. \$\endgroup\$ – Austin Hastings May 26 at 2:54
  • \$\begingroup\$ It took my a while before i understood what you mean with a handler function and how it works. I managed to implement it twice, but i will cleanup some more because half of the function in the locations are just returns. link \$\endgroup\$ – Cuypers Michel May 28 at 8:23
  • \$\begingroup\$ I can see you're making progress. I'd still suggest putting more data into the verb structure, but you clearly can see how to do that. You might want to get in the habit of using a pointer, instead of an index -- it's easier to pass that around to other functions. \$\endgroup\$ – Austin Hastings May 28 at 22:51
  • \$\begingroup\$ I added a quit statement, more structs but i've got stuck. I can't seem to connect everything properly (locations, items,..) which makes my code unorganized & messy. The IF statements are way too long right now. link \$\endgroup\$ – Cuypers Michel May 30 at 19:14

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.