trjhtr

This code was reworked and posted here:Text based game “Hunt the Wumpus” Version 2

The following game was a excercise in the PPP book by Stroustrup:

Implement a version of the game "Hunt the Wumpus".
Hunt the Wumpus" (or just "Wump") is a simple (non-graphically) computer game originally invented by Gregory Yob.
The basic premise is that a rather smelly monster lives in a dark cave consisting of connected rooms. Your job is to slay the wumpus using bow and arrow. In addition to the wumpus, the cave has two hazards: bottomless pits and giant bats. If you enter a room with a bat, the bat picks you up and drops you into another room. If you enter a room with a bottomless pit, its the end of the game for you. If you enter the room with the Wumpus he eats you. When you enter a room you will be told if a hazard is nearby:


"I smell the wumpus": It´s in an adjoining room.

"I feel a breeze": One of the adjoining rooms is a bottomless pit.

"I hear a bat": A giant bat is in an adjoining room.



For your convenience, rooms are numbered. Every room is connected by tunnels to three other rooms. When entering a room, you are told something like " You are in room 12; there are tunnels to rooms 1,13, and 4: move or shoot?" Possible answers are m13 ("Move to room 13") and s13-4-3 ("Shoot an arrow through rooms 13,4, and 3"). The range of an arrow is three rooms. At the start of the game, you have five arrows.
The snag about shooting is that it wakes up the wumpus and he moves to a room adjoining the one he was in - that could be your room.
Be sure to have a way to produce a debug output of the state of the cave.

The excercises doesnt say how much bats or pits are in the game. I did some research and found that most implementations made like count_of_rooms / 6 for the amount of bats and pits.

Here is my code:

wumpus.h

#ifndef WUMPUS_GUARD_110720182013
#define WUMPUS_GUARD_110720182013

#include <iostream>
#include <random>
#include <vector>
#include <string>
#include <sstream>

namespace wumpus 

 struct Room 
 Room(int rnum)
 :wumpus false , pit false , bat false , player false , rnumber rnum 
 
 brooms.push_back(nullptr);
 brooms.push_back(nullptr);
 brooms.push_back(nullptr);
 
 int rnumber;
 std::vector <Room*> brooms; //pointer to 3 rooms next to this room
 bool wumpus;
 bool pit;
 bool bat;
 bool player;
 ;

 class Dungeon 
 public:
 Dungeon();
 void indicate_hazards();
 bool shoot_arrow(std::vector<int> tar_rooms);
 bool Dungeon::move_wumpus();
 bool move_player(int tar);
 void debug(); //shows the status of the cave for debug purpose
 std::vector<int> neigbour_rooms(int room);
 int current_room();
 private:
 static constexpr int count_of_arrows = 5;
 static constexpr int count_of_rooms = 12;
 static constexpr int count_of_pits = count_of_rooms / 6;
 static constexpr int count_of_bats = count_of_rooms / 6;
 std::vector <Room> rooms;
 int arrows;
 int wumpus_room;
 int player_room;

 bool connect_rooms();
 bool room_is_full_connected(const int con, const std::vector<int>& crooms);
 ;

 int get_random(int min, int max);
 void hunt_the_wumpus();
 void instructions();
 int select_room_to_move(Dungeon& d1);
 std::vector<int> select_rooms_to_shoot();



#endif

wumpus.cpp

#include "wumpus.h"

namespace wumpus 

Dungeon::Dungeon()
 rooms[player_room - 1].pit == true
 

void Dungeon::indicate_hazards()

 bool found_bat = false;
 bool found_pit = false;

 for (auto& x : rooms[player_room - 1].brooms) 
 if (x->wumpus == true) 
 std::cout << "I smell the wumpusn";
 
 if (x->pit == true && found_pit == false) 
 found_pit = true;
 std::cout << "I feel a breezen";
 
 if (x->bat == true && found_bat == false) 
 found_bat = true;
 std::cout << "I hear a batn";
 
 
 std::cout << "You are in room " << rooms[player_room - 1].rnumber << "n";
 std::cout << "You have "<<arrows<< " arrow(s) leftn";
 std::cout << "Tunnels lead to rooms " << rooms[player_room - 1].brooms[0]->rnumber << ", ";
 std::cout << rooms[player_room - 1].brooms[1]->rnumber << " and ";
 std::cout << rooms[player_room - 1].brooms[2]->rnumber << "n";
 std::cout << "what do you want to do? (M)ove or (S)hoot?n";


bool Dungeon::shoot_arrow(std::vector<int> tar_rooms)
 //trys to shoot in the supplied tar rooms an arrow
 //if the wumpus is hit returns true to indicate victory
 //moves the wumpus on fail

 --arrows;
 int curr_room = player_room - 1;
 for (const auto& tr : tar_rooms)
 bool room_reached = false;
 for (const auto& x : rooms[curr_room].brooms) //check if neigbour room is one of the vectors
 if (x->rnumber == tr) 
 room_reached = true;
 curr_room = x->rnumber - 1;
 if (rooms[curr_room].wumpus) //wumpus hit
 std::cout << "!!!!!!YOU WON!!!!!!: You killed the Wumpus in room " << rooms[curr_room].rnumber << "n";
 return true;
 
 break;
 
 
 if (!room_reached) //if not end 
 std::cout << "Room " << tr << " could not be reached from arrown";
 return false;
 
 
 if (arrows == 0) 
 std::cout << "You lost: You ran out of arrows";
 return true;
 


bool Dungeon::move_wumpus() 
 //moves the wumpus with a chance of 75% to a new room
 //if player room is entered true is returned for game over

 if (get_random(1, 4) == 4) // no movement on 4
 return false;
 else 
 rooms[wumpus_room - 1].wumpus = false;
 wumpus_room = rooms[wumpus_room - 1].brooms[get_random(0, 2)]->rnumber;
 rooms[wumpus_room - 1].wumpus = true;
 if (rooms[wumpus_room - 1].player) 
 std::cout << "You lost: Wumpus enters youre room and eats youn";
 return true;
 
 
 return false;


bool Dungeon::move_player(int tar)
 //trys to move player to the selected room
 //if deadly hazard like pit or wumpus is found return game over = true;
 //if bat is found choose new random room free from hazards to put the player

 for (auto& x : rooms[player_room - 1].brooms) 
 if (x->rnumber == tar) 
 if (x->wumpus == true) 
 std::cout << "You lost: You got eaten by the Wumpusn";
 return true;
 
 else if (x->pit == true) 
 std::cout << "You lost: You fell in a bottomless pitn";
 return true;
 
 else if (x->bat) rooms[rnd_room - 1].bat == true 
 else 
 rooms[player_room - 1].player = false;
 player_room = tar;
 rooms[player_room - 1].player = true;
 return false;
 
 
 
 std::cerr << "Dungeon::move_player: Unknown target room entered";
 return false;


void Dungeon::debug()

 for (const auto&x : rooms) 
 std::cout << "Room " << x.rnumber << " connects to: ";
 for (const auto&y : x.brooms) 
 if (y != nullptr) std::cout << y->rnumber << " ";
 else std::cout << "np" << " ";
 
 std::cout << " ";
 if(x.wumpus) std::cout << "wumpus:" << x.wumpus << " ";
 if(x.pit) std::cout << "pit:" << x.pit << " ";
 if(x.bat) std::cout << "bat:" << x.bat << " ";
 if(x.player) std::cout << "player:" << x.player << " ";
 std::cout << "n";
 


std::vector<int> Dungeon::neigbour_rooms(int room)

 std::vector<int> ret;
 ret.push_back(rooms[room - 1].brooms[0]->rnumber);
 ret.push_back(rooms[room - 1].brooms[1]->rnumber);
 ret.push_back(rooms[room - 1].brooms[2]->rnumber);
 return ret;


int Dungeon::current_room()

 return player_room;


//-------------------------------------------------------------
//Private functions
//-------------------------------------------------------------

bool Dungeon::room_is_full_connected(const int con, const std::vector<int>& crooms)
 //checks if the room has already 3 connections so it is on the black list

 for (const auto &x : crooms) // room is on the black list
 if (x == con)
 return true;
 return false;


bool Dungeon::connect_rooms()
 //connects the rooms with random algorithm
 //it can happen that the last 2 rooms connected are in the same room 
 //e.g Room 2 connected to 4 np np. the last to connections are in the same room
 //in this case it is returned false to re run the connection algorithm
 //other case to restart 2 rooms left with one unconnected node because they are already connected
 //with each other

 std::vector <int> conn_rooms;
 while (conn_rooms.size() < count_of_rooms) source_rnd_room == target_rnd_room) //check if rnd room is on black list
 target_rnd_room = get_random(min_room, max_room);

 if (conn_rooms.size() == count_of_rooms - 2 //special case for last 2 numbers to prevent infinite loop
 && room_is_full_connected(target_rnd_room, conn_rooms)
 && room_is_full_connected(source_rnd_room, conn_rooms)
 && source_rnd_room != target_rnd_room) 
 conn_rooms.push_back(source_rnd_room);
 conn_rooms.push_back(target_rnd_room);
 
 if (conn_rooms.size() == count_of_rooms - 1) 
 return false;
 
 

 bool conn_full = true;
 bool tar_already_conn = false;
 for (size_t i = 0; i < rooms[source_rnd_room - 1].brooms.size(); ++i) 
 if (rooms[source_rnd_room - 1].brooms[i] == nullptr) 
 conn_full = false;
 rooms[source_rnd_room - 1].brooms[i] = &rooms[target_rnd_room - 1];
 if (i == rooms[source_rnd_room - 1].brooms.size() - 1)
 conn_full = true;
 break;
 
 else if (rooms[source_rnd_room - 1].brooms[i]->rnumber == target_rnd_room) //skip if room already leads to source_rnd_room
 tar_already_conn = true;
 break; 
 
 

 if (tar_already_conn && conn_rooms.size() == count_of_rooms - 2)
 return false;

 if (tar_already_conn)
 continue;

 if (conn_full)
 conn_rooms.push_back(source_rnd_room);

 conn_full = true;
 for (size_t i = 0; i < rooms[target_rnd_room - 1].brooms.size(); ++i) 
 if (rooms[target_rnd_room - 1].brooms[i] == nullptr) 
 conn_full = false;
 rooms[target_rnd_room - 1].brooms[i] = &rooms[source_rnd_room - 1];
 if (i == rooms[target_rnd_room - 1].brooms.size() - 1)
 conn_full = true;
 break;
 
 

 if (conn_full)
 conn_rooms.push_back(target_rnd_room);

 
 return true;


//-------------------------------------------------------------
//Helper functions
//-------------------------------------------------------------

int get_random(int min, int max)

 std::random_device rd;
 std::mt19937 mt(rd());
 std::uniform_int_distribution<int> distribution(min, max);
 return distribution(mt);


void hunt_the_wumpus()

 instructions();
 while (true)
 


void instructions()

 std::cout <<
 "Welcome to "Hunt the Wumpus"!n"
 "The wumpus lives in a cave of rooms.Each room has 3 tunnels leading ton"
 "other rooms. (Look at a dodecahedron to see how this works - if you don't known"
 "what a dodecahedron is, ask someone).n"
 "n"
 "Hazardsn"
 "Bottomless pits - two rooms have bottomless pits in them.If you go there, youn"
 "fall into the pit(and lose!)n"
 "Super bats - two other rooms have super bats.If you go there, a bat grabs youn"
 "and takes you to some other room at random. (Which may be troublesome).n"
 "n"
 "Wumpusn"
 "The wumpus is not bothered by hazards(he has sucker feet and is too big for an"
 "bat to lift).Usually he is asleep.Two things wake him up : you shooting ann"
 "arrow or you entering his room.n"
 "n"
 "If the wumpus wakes he moves(p = .75) one room or stays still(p = .25).Aftern"
 "that, if he is where you are, he eats you up and you lose!n"
 "n"
 "Youn"
 "Each turn you may move or shoot a crooked arrow.n"
 "Moving: you can move one room(thru one tunnel).n"
 "Arrows : you have 5 arrows.You lose when you run out.Each arrow can go from 1n"
 "to 3 rooms.You aim by telling the computer the rooms you want the arrow to gon"
 "to.If the arrow can't go that way (if no tunnel) it moves at random to then"
 "next room.If the arrow hits the wumpus, you win.If the arrow hits you, youn"
 "lose.n"
 "n"
 "Warningsn"
 "When you are one room away from a wumpus or hazard, the computer says :n"
 "n"
 "Wumpus: "I smell the wumpus"n"
 "Bat : "I hear a bat"n"
 "Pit : "I feel a breeze"n"
 "nn"
 "Press any key to startn";
 char c;
 std::cin.get(c);


int select_room_to_move(Dungeon& d1)

 int in;
 while (true) 



std::vector<int> select_rooms_to_shoot()

 for(;;)
 std::cout << "Enter rooms you want to shoot the arrow (e.g. 2-3-12, eg 4-5, eg 2)n";

 std::string in;
 std::cin >> in;

 std::istringstream ist in ;
 std::vector<int> tar_rooms;

 bool bad_input = false;
 while (!ist.eof()) 
 int in_int;
 char in_char;
 ist >> in_int;
 if (ist.fail()) 
 std::cin.clear();
 std::cin.ignore(999, 'n');
 bad_input = true;
 break;
 
 tar_rooms.push_back(in_int);

 if (ist.eof()) break;

 ist >> in_char;
 if (ist.fail()) 
 std::cin.clear();
 std::cin.ignore(999, 'n');
 bad_input = true;
 break;
 
 if (in_char != '-') bad_input = true; break; 
 if (tar_rooms.size() > 3) bad_input = true;break; 
 if (ist.eof()) bad_input = true;break; ;//to prevent 2-3-12- last sign must be integer
 
 if (bad_input) continue;
 if (tar_rooms.size() <= 3)
 return tar_rooms;
 

main.cpp

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


int main()
try 
 wumpus::hunt_the_wumpus();

catch (std::runtime_error& e) 
 std::cerr << e.what() << "n";
 std::cin.get();

catch (...) 
 std::cerr << "unknown errorn";
 std::cin.get();

I wonder what can be improved to clean up the code. Is it a good idea to split the code into smaller classes?

Please let me know if the code is readable or if it needs improvements.

I want the code as clean as possible beause in the next step i want to make it a GUI game (using FLTK).

Your code is too big for me to do a full review, but here are some random observations:

First, the standard miscellany.

• Good on you for not using namespace std; anywhere!


• Consider using #pragma once instead of the #ifndef/#define dance you're currently doing. (It's not standard but it's a lot more foolproof. Numbers such as your 110720182013 may change — and noise up your git logs when they do — but #pragma once endures.)


• Ending every program with std::cin.get(); is a MSVC-ism. Learn to run your programs in the Command Prompt (a.k.a. Terminal) and you won't need to mess around with cin.get() anymore.

Now the naming. Most of your identifiers are inappropriate for a real codebase. For example, you have a vector that in a real program would be called something like neighbors or outgoing_passages; you call it brooms.

Some of your identifiers are at least plausibly related by abbreviation to their actual meanings; for example int tar, which from context clearly means something like int target. But you should still write them out! Remember, we write our C++ code for people, not for machines. (Machines are totally happy reading machine code.) So rnd_room should be random_room; rnumber should be room_number (or just room?), tar_rooms should be target_rooms (or path?), and so on.

Consider introducing a type alias

using RoomNumber = int;

so that you don't have to use unexplained int for so many different purposes.

Your helpdoc could maybe productively be rewritten to use a R"(raw string)" literal, which first appeared in C++11. I intensely dislike raw strings because of the R".*(regex).*" issue, but I also dislike having to manually place n and " in page-long C strings, so... in this case I'd consider it.

Speaking of your helpdoc, it's a copy of Gregory Yob's original, which is cool and all, but that means it is not an accurate description of what your program actually does!

The wumpus lives in a cave of rooms. Each room has 3 tunnels leading to other rooms. (Look at a dodecahedron to see how this works — if you don't know what a dodecahedron is, ask someone).

Your code doesn't actually do this! To see one way to implement "randomly numbered rooms on a dodecahedron," you could do this. (Step 1: hard-code a map of a dodecahedron. Step 2: assign the numbers 1 through 20 to the vertices, at random.)

int get_random(int min, int max)

 std::random_device rd;
 std::mt19937 mt(rd());
 std::uniform_int_distribution<int> distribution(min, max);
 return distribution(mt);

This is an antipattern. Better would be:

int get_random(int min, int max)

 static std::mt19937 mt = () 
 std::random_device rd;
 return std::mt19937(rd());
 ();
 std::uniform_int_distribution<int>(min, max);
 return distribution(mt);

This way you only create the random_device once per run of the program, instead of once per random number generated. Your intuition here should be that creating a random_device is equivalent to open("/dev/urandom"); so you don't want to do it often if you don't need to.

neigbour_rooms

Spell-check! (Protip: If you use full English words, instead of tar and rnd and such, then you can pretty much literally run spell-check on your code.)

struct Room 
 Room(int rnum)
 :wumpus false , pit false , bat false , player false , rnumber rnum 
 
 brooms.push_back(nullptr);
 brooms.push_back(nullptr);
 brooms.push_back(nullptr);
 
 int rnumber;
 std::vector <Room*> brooms; //pointer to 3 rooms next to this room
 bool wumpus;
 bool pit;
 bool bat;
 bool player;
;

Two things about this struct. First, every constructor you write (regardless of number of parameters) should be explicit, unless you deliberately want to enable implicit conversion for some reason. Second, C++11 and later support in-line initializers for member data, so you should actually write it like this:

struct Room 
 explicit Room(int r) : room_number(r), neighbors(3, nullptr) 
 int room_number;
 std::vector<Room*> neighbors;
 bool has_wumpus = false;
 bool has_pit = false;
 bool has_bat = false;
 bool has_player = false;
;

Notice that with the vector renamed to neighbors, we no longer need the long comment explaining about "brooms"! This is what people mean by "self-documenting code." I've renamed the other members accordingly. Notice that some_room.pit is relatively clear, but some_room.has_pit is 100% unambiguous. Strive for 100%!

int Dungeon::current_room()

 return player_room;

This is an interesting bit of code, because here we have a one-line function that claims to return "dungeon current room" but actually returns "player room." One or the other of these names is lying to us!

I think what it's actually returning is the room the player is currently in. (Why the Dungeon should care what room the player is in, I'm not sure.) The method should probably be named player_room, at which point we realize that the member variable player_room might as well be public.

(Or, if we're going the full OOP getters-and-setters route here, the private member variable should be named with a sigil such as player_room_ or m_player_room, and orthogonally, the member function int player_room() const should have that const.

There's more, but that's a good stopping point for now, anyway.

Here are some things that may help you improve your code. It's definitely improved over the last version, but there's more you could do.

Omit the class name from member function declarations

It was probably just a typo or cut-and-paste error, but within the Dungeon class in wumpus.h we have this:

bool Dungeon::move_wumpus();

As you know (because this is the only place it's done otherwise) the extra qualification is not necessary here. Instead, that should be:

bool move_wumpus();

Always return an appropriate value

Your shoot_array() routine has control paths that cause it to end without returning any bool value. This is an error and should be fixed, probably most easily by simply adding return false; to the end.

Prefer std::array to std::vector when fixed size is known at compile time

The way this game is constructed, there will always be 20 room, each with 3 neighbors. For that reason, the get_neighbor_room should probably return a std::array<Room_number, 3> instead of a std::vector<int>.

Don't create the random device every time

The get_random code is currently this:

int get_random(int min, int max)

 std::random_device rd;
 std::mt19937 mt(rd());
 std::uniform_int_distribution<int> distribution(min, max);
 return distribution(mt);

But we don't really need a new rd or a new mt each time. I'd make both of these static.

Don't hide loop exit conditions

In the hunt_the_wumpus routine, there are two nested loops both of which are while (true) but neither really continues forever. Rather than hide the exit condition, state it up front to make it easier to read and understand. For instance, the inner loop could be this:

for (bool gamme_over = false; ) 

Becomes this:

 else if (in == "s" || in == "S" || in == "Shoot" || in == "shoot") 
 game_over = d1.shoot_arrow(select_rooms_to_shoot());
 if (!game_over) 
 game_over = d1.move_wumpus();
 
 
}

Avoid a bunch of confusing break and continue statements and simplify and shorten the code at the same time.

Simplify the room connection code

The connect_rooms() code is quite long and complex. It can be much simplified by using a better algorithm. Select two rooms at a time, connect them to each other (if they're not already connected) and move those to another vector. Continue until the first vector is empty. Then do that three more times.

Make your classes work harder

The room_is_full_connected should be a member function of Room. Here are functions I'd add to Room:

// return true if the passed room is already connected to this one
bool isConnected(const Room& other) const;
// return the number of connections from this room
bool connectionCount() const;
// return true if the passed room is this one or is connected
bool isSelfOrConnected(const Room& other) const;
// connect this room to the other
bool connect(Room& other);
// hazard checks
bool nearWumpus() const;
bool nearBat() const;
bool nearPit() const;

Look out for member functions

There is a function in the current code with this prototype:

int select_room_to_move(Dungeon& d1);

Since it's being passed a non-const reference to a Dungeon anyway, it seems likely that this should instead be a member function.

Simplify user input

The select_rooms_to_shoot is much longer and much more convoluted than necessary. Consider a cleaner approach:

std::vector<int> select_rooms_to_shoot()

 std::vector<int> tar_rooms;
 for(bool bad_inputtrue; bad_input; ) 
 std::cout << "Enter rooms you want to shoot the arrow (e.g. 2-3-12, eg 4-5, eg 2)n";

 std::string in;
 if (std::getline(std::cin, in)) tar_rooms.size() > 3;
 
 
 return tar_rooms;