Hunt the Wumpus Exercise

Question

I recently completed an exercise in Stroustrup's "Programming Principles and Practices Using C++" with the following specifications: to make a console version of the game Hunt the Wumpus in which the player navigates a cave with multiple rooms in order to find and shoot the wumpus. If the wumpus finds the player, or if the player runs into the wumpus, the player loses. If the player shoots the wumpus, they win. There are rooms with bottomless pits which, if the player enters, they lose. There are rooms with a giant bat which, if the player enters, the bat flies the player to another room. The player has only 5 arrows, and if they run out, the player loses. Any feedback would be appreciated.

Player.h

#include <sstream>
#include <vector>

class Player
{
    static const int max_shots = 5;
    int shots;
    int curr_room;
    void run_out_of_arrows();
public:
    explicit Player(int current_room)
        :shots(0), curr_room{ current_room }{}
    int arrows_left() const { return max_shots - shots; }
    int& current_room() { return curr_room; }
    void shoot(){ ++shots; }
    bool out_of_arrows(){ return shots >= max_shots; }
};

/*Struct used to interpret player's moves from input from std::cin
Examples of valid input: m13 (move to room 13). Can only move to one room at a time
s13-14-5(shoot through rooms 13, 14, and 5). Can shoot through up to three rooms
This does not check if rooms are valid, only if the format is valid
*/
struct Player_move
{
    enum Move_type{move, shoot};
    std::vector<int>rooms;
    Move_type move_type;
    Player_move()
        :move_type{ Move_type::move }{}
    Player_move(int r, Move_type mt = Move_type::move)
        :move_type{ mt }
    {
        rooms.push_back(r);
    }
    Player_move(std::initializer_list<int> il)
        :move_type{ Move_type::shoot }
    {
        rooms = il;
    }
};


std::istream& operator>>(std::istream& is, Player_move& pm);

Player.cpp

#include "Player.h"

std::istream& operator>>(std::istream& is, Player_move& pm)
{
    //read into a string to account for variability of input
    std::string response;
    is >> response;
    int response_sz = response.size();
    if (response_sz < 2 || response_sz > 9) // response is too large or too small to be valid
    {
        is.clear(std::ios_base::failbit);
        return is;
    }

    std::istringstream resp_ist{ response };
    char move_type; int r1;
    resp_ist >> move_type >> r1;
    if (!resp_ist)
    {
        is.clear(std::ios_base::failbit);
        return is;
    }

    switch (move_type)
    {
    case 'm':
        if (resp_ist.eof()) 
        {
            pm = Player_move(r1);
            return is;
        }
        else //there's still content in the string after "m#", bad input
        {
            is.clear(std::ios_base::failbit);
            return is;
        }
    case 's':
        //strategy: if string has not ended, continue to read char separator and int combo until
        //three rooms max have been read.
        if (resp_ist.eof())
        {
            pm = Player_move( r1, Player_move::shoot );
            return is;
        }
        char ch2; int r2;       
        resp_ist >> ch2 >> r2;
        if (!resp_ist)
        {
            is.clear(std::ios_base::failbit);
            return is;
        }
        if (resp_ist.eof())
        {
            pm = Player_move{ r1, r2 };
            return is;
        }
        char ch3; int r3;
        resp_ist >> ch3 >> r3;
        if (!resp_ist)
        {
            is.clear(std::ios_base::failbit);
            return is;
        }
        if (resp_ist.eof())
        {
            pm = Player_move{ r1, r2, r3 };
            return is;
        }
        else
        {
            is.clear(std::ios_base::failbit);
            return is;
        }
    default:
        is.clear(std::ios_base::failbit);
        return is;
    }
}

Cave.h

#include <iostream>
#include "Player.h"

enum class Game_State{ win, lose, player_move, cave_move };

class Room
{
public:
    enum Room_type
    {
        Wumpus, Pit, Empty, Bat
    };
    Room(Room_type rr)
        :rt{ rr }{}
    Room()
        :Room(Room::Empty){}
    int room_type() const { return rt; }
    void set_room_type(Room_type rr) { rt = rr; }
private:
    Room_type rt;
};

void print_room_details(Room& r);

class Cave
{
public:
    //one "tunnel" per room, each contains room numbers of rooms that are connected via tunnel
    using Tunnel = std::vector<int>; 
    Cave();
    const Game_State get_game_state() const { return game_state; }
    void alert_player();
    void get_player_move();
private:
    std::vector<Room>rooms; //each room's number is determined by its index in vector rooms
    std::vector<Tunnel>tunnels; 
    static const int number_of_rooms = 12;
    Player player{ Player{ 0 } };
    void connect_rooms();
    bool valid_move(const Player_move& pm);
    void wumpus_move();
    int wumpus_index() const;
    int pit_index() const;
    int bat_index() const;
    void player_kills_wumpus();
    void player_lose(const std::string& s);
    void bat_move_player();
    void move_player(Player_move& pm);
    Game_State game_state{ Game_State::player_move };
};

inline int randint(int min, int max)
{
    return (rand() % max) + min;
}

inline int randint(int max)
{
    return randint(0, max);
}

Cave.cpp

#include "Cave.h"

Cave::Cave()
    :rooms(number_of_rooms), tunnels(number_of_rooms)
{
    //initialize rooms with wumpus, bat, and bottomless pit, and player rooms
    int wump_ind = randint(number_of_rooms);
    int pit_ind = randint(number_of_rooms);
    // ensure that each room is unique
    while (pit_ind == wump_ind) pit_ind = randint(number_of_rooms); 
    int bat_ind = randint(number_of_rooms);
    while (bat_ind == wump_ind || bat_ind == pit_ind) bat_ind = randint(number_of_rooms);
    int player_room_ind = randint(number_of_rooms);
    while (player_room_ind == wump_ind || player_room_ind == pit_ind || player_room_ind == bat_ind)
        player_room_ind = randint(number_of_rooms);
    player = Player{ player_room_ind };
    rooms[wump_ind] = Room{ Room::Wumpus };
    rooms[pit_ind] = Room{ Room::Pit };
    rooms[bat_ind] = Room{ Room::Bat };
    //all other rooms are empty by default
    connect_rooms();
}

//each room is connected to three other rooms by a "tunnel"(alias for vector<int>)
void Cave::connect_rooms()
{
    for (int i = 0; i < rooms.size(); ++i)
    {
        int adj1 = randint(number_of_rooms);
        //ensure that no connected rooms are the same
        while (adj1 == i) adj1 = randint(number_of_rooms);
        int adj2 = randint(number_of_rooms);
        while (adj2 == adj1 || adj2 == i) adj2 = randint(number_of_rooms); 
        int adj3 = randint(number_of_rooms);
        while (adj3 == adj1 || adj3 == adj2 || adj3 == i) adj3 = randint(number_of_rooms);
        //connect current room w/ 3 random rooms
        tunnels[i].push_back(adj1);
        tunnels[i].push_back(adj2);
        tunnels[i].push_back(adj3);
    }
}

void Cave::alert_player()
{
    std::cout << "You are currently in room " << player.current_room()
        << "; There are tunnels to rooms ";
    Tunnel& t = tunnels[player.current_room()];
    for (int i = 0; i < t.size(); ++i)
    {
        std::cout << t[i];
        if (i != t.size() - 1) std::cout << ", ";
    }
    std::cout << "; move or shoot?\n";

    //give player hints of surroundings
    for (int i : t)
    {
        print_room_details(rooms[i]);
    }
}

void print_room_details(Room& r)
{
    switch (r.room_type())
    {
    case Room::Wumpus:
        std::cout << "I smell the wumpus\n";
        break;
    case Room::Pit:
        std::cout << "I feel a breeze\n";
        break;
    case Room::Bat:
        std::cout << "I hear a bat\n";
        break;
    default:
        break;
    }
}

//return wumpus' room number
int Cave::wumpus_index() const
{
    int wump_index = 0;
    for (const Room& r : rooms)
    {
        if (r.room_type() == Room::Wumpus) break;
        ++wump_index;
    }
    return wump_index;
}

//return bat's room number
int Cave::bat_index() const
{
    int bat_indx = 0;
    for (const Room& r : rooms)
    {
        if (r.room_type() == Room::Bat) break;
        ++bat_indx;
    }
    return bat_indx;
}

//return pit room number
int Cave::pit_index() const
{
    int pit_indx = 0;
    for (const Room& r : rooms)
    {
        if (r.room_type() == Room::Pit) break;
        ++pit_indx;
    }
    return pit_indx;
}

//wumpus moves after hearing player shoot an arrow
void Cave::wumpus_move()
{
    game_state = Game_State::cave_move;

    int bat_ind = bat_index();
    int pit_ind = pit_index();
    int wump_index = wumpus_index();

    Tunnel& t = tunnels[wump_index];
    int new_wump_index = t[randint(t.size())];
    //ensure wumpus does not move to a room w/ bat or pit
    while (new_wump_index == bat_ind || new_wump_index == pit_ind) new_wump_index = t[randint(t.size())];
    rooms[wump_index].set_room_type(Room::Empty);
    rooms[new_wump_index].set_room_type(Room::Wumpus);

    if (new_wump_index == player.current_room())
    {
        player_lose("The Wumpus has found you! Game over.");
    }
}

void Cave::player_lose(const std::string& s)
{
    std::cout << s << '\n';
    game_state = Game_State::lose;
}

bool Cave::valid_move(const Player_move& pm)
{
    if (pm.rooms.size() == 0) return false; //empty vector
    Tunnel& t = tunnels[player.current_room()];
    for (int i : pm.rooms)
    {
        if (i < 0 || i >(number_of_rooms - 1)) return false;
        if (std::find(t.begin(), t.end(), i) == t.end()) return false; //room is not connected to players current room
    }
    return true;
}

void Cave::get_player_move()
{
    game_state = Game_State::player_move;
    Player_move pm;
    while (!(std::cin >> pm) || !valid_move(pm))
    {
        std::cout << "Please enter a valid move. Ex: m# or s#-#-#(shoot through 3 rooms max).\n";
        std::cin.clear();
    }
    if (pm.move_type == Player_move::move)
    {
        move_player(pm);
    }
    else //shot awakens the wumpus
    {
        player.shoot();
        if (std::find(pm.rooms.begin(), pm.rooms.end(), wumpus_index()) != pm.rooms.end()) player_kills_wumpus();
        if (player.out_of_arrows()) player_lose("You have no more arrows left! Game over.\n");
        else wumpus_move();
    }
}

void Cave::player_kills_wumpus()
{
    std::cout << "You shot the Wumpus! You win.\n";
    game_state = Game_State::win;
}

void Cave::bat_move_player()
{
    game_state = Game_State::cave_move;
    std::cout << "You have entered the bat room! The bat will now move you to a random room.\n";
    move_player(Player_move( randint(number_of_rooms) ));
}

void Cave::move_player(Player_move& pm)
{
    player.current_room() = pm.rooms[0];
    if (player.current_room() == wumpus_index()) player_lose("The Wumpus has found you! Game over.");
    else if (player.current_room() == pit_index()) player_lose("You fell down the pit! Game over.");
    else if (player.current_room() == bat_index()) bat_move_player();
}

Main.cpp

#include <ctime>
#include "Cave.h"


void game_loop(Cave& cave)
{

    while (int(cave.get_game_state()) != int(Game_State::win) && int(cave.get_game_state()) != int(Game_State::lose))
    {
        cave.alert_player();
        cave.get_player_move();
    }
}

int main()
try{
    srand(time(0));
    Cave main_cave;
    game_loop(main_cave);
}
catch (std::exception& e)
{
    std::cerr << e.what() << '\n';
}
catch (...)
{
    std::cerr << "Something went wrong\n";
}

Answer 1

I see a number of things here that may help you improve your program.

Fix the bug

While this might compile on your platform, this line in Cave::bat_move_player() is definitely a bug:

move_player(Player_move( randint(number_of_rooms) ));

The reason is that move_player is defined to take a reference, and we can't bind a non-const lvalue reference to the temporary rvalue created by the call to Player_move's constructor. To fix this, either break the line into two pieces

auto pm = Player_move( randint(number_of_rooms) );
move_player(pm);

Or define an additional, alternate form of the move_player function using move semantics:

void Cave::move_player(Player_move&& pm);

Fix the other bug

What happens if the player kills the wumpus with the last arrow?

player.shoot();
if (std::find(pm.rooms.begin(), pm.rooms.end(), wumpus_index()) != pm.rooms.end()) player_kills_wumpus();
if (player.out_of_arrows()) player_lose("You have no more arrows left! Game over.\n");
else wumpus_move();

What happens is that we get this:

You shot the Wumpus! You win.

You have no more arrows left! Game over.

I suppose both are objectively true, but the first statement sets the state to win and the second sets the state to lose which doesn't seem right. It also leads directly to the next suggestion.

Always use braces with compound statements

I'd also add that the compound statement should not be put on the same line. Following both of these pieces of advices transforms the code mentioned above into this:

if (std::find(pm.rooms.begin(), pm.rooms.end(), wumpus_index()) != pm.rooms.end()) {
    player_kills_wumpus();
}
if (player.out_of_arrows()) {
    player_lose("You have no more arrows left! Game over.\n");
} else {
    wumpus_move();
}

Then we can fix it like this:

if (std::find(pm.rooms.begin(), pm.rooms.end(), wumpus_index()) != pm.rooms.end()) {
    player_kills_wumpus();
} else if (player.out_of_arrows()) {
    player_lose("You have no more arrows left! Game over.\n");
} else {
    wumpus_move();
}

Make sure to #include all required headers

This program calls std::find from within Cave.cpp but does not include the corresponding header. Fix that by adding this line:

#include <algorithm>

There may be other missing headers, but I haven't gone through thoroughly. Even if the code compiles as is, it is best (most portable and durable) if the documented headers are included.

Avoid doing extra work

Any time we want to find the bat or the wumpus, the code linearly searches through the entire room vector. It makes more sense to me to simply store the index and simplify the lookup.

Simplify your code

The game_state variable is not very much used and the value is only checked to determine when the game is over with this unsightly bit of code:

while (int(cave.get_game_state()) != int(Game_State::win) && int(cave.get_game_state()) != int(Game_State::lose))

There are a number of problems with that, starting with the ugly old-style C casts and the fact that the state is fetched twice. What I'd suggest instead would be to write this:

while (!cave.isGameOver()) { ... }

The isGameOver could be as simple as this:

bool isGameOver() const { return player_won || player_lost; }

and having player_won and player_lost being two simple bool variables.

Use a better random number generator

You are currently using

inline int randint(int min, int max)
{
    return (rand() % max) + min;
}

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 is this function. On my machine, there's a slight but measurable bias toward 0 with that. A better solution would be to use the C++11 `std::uniform_int_distribution. It looks complex, but it's actually pretty easy to use.

// random number generator from Stroustrup: 
// http://www.stroustrup.com/C++11FAQ.html#std-random
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});
}

Use standard functions and algorithms where practical

The Cave constructor has many lines of code to choose four rooms in which to place the player, wumpus, pit and bat. Here's another way to write that constructor:

Cave::Cave() :
    rooms(number_of_rooms), 
    tunnels(number_of_rooms),
    wumpus{randomEmptyRoomIndex()},
    bat{randomEmptyRoomIndex()},
    pit{randomEmptyRoomIndex()},
    player{randomEmptyRoomIndex()}
{
    rooms[wumpus] = Room{ Room::Wumpus };
    rooms[pit] = Room{ Room::Pit };
    rooms[bat] = Room{ Room::Bat };
    //all other rooms are empty by default
    connect_rooms();
}

Then all we need is this function:

int Cave::randomEmptyRoomIndex() const {
    int i;
    do {
        i = randint(rooms.size()); 
    } while (rooms[i].isEmpty());
    return i;
}

This also has a new isEmpty() convenience function for the Room class and assumes, per the earlier suggestion, that the indices of each of the hazards is a Cave class member. There are many other such simplifications possible, and I'd encourage you too look for and implement them.