I have been writing Hunt the Wumpus game in C++, and i think it's finished. It's first non-trivial project, and it DOES work properly (at least 20 hours of debugging showed so). How can the code be more clean or simply better ? Rules are in the game.cpp.

Any criticism or suggestions are welcome.

game.cpp

#pragma once #include "heart.h" #include <iostream> #include <ctime> void rules() { { std::cout << "Rules:" << std::endl << "1. You are in a cave system(a dodecahedron) such that each cave connects to three other caves.\n" << "2. Somewhere in the cave lives a dreaful monster - Wumpus.\nHe has sucker feet to cling to the walls of bottomless pits and is too heavy to be picked up by super - bats.\nHe also has has big teet thand will eat you if you are in the same room with him.\n" << "3. In the cave system, two caves are bottomless pits. Step in cave with one = you are scattered into pieces (Game Over).\n" << "4. Also, two caves contain super - bats, which will carry you off to a random cave.\nBats can carry you to any room, including Pit room and Wampus room, which will result in death (Game Over).\n" << "5. If the adjacent room to your position contains any of three events (wampus, bat, pit), you will be notified.\n" << "6. Only one instance of event can spawn at a room, but Wampus can move to both bat and pit rooms and remain there.\n" << "7. You have 5 arrows.\nWhen you think you know where the Wumpus is(or even if you don't) shoot an arrow into a room.\nIf you hit the Wumpus, he will die and you win the game (Game WON).\n" << "8. If you miss, the Wumpus, who is also a very light sleeper:\n1)Will wakeup and move(75%) to one of his adjucent rooms \n2) Will continue sleeping (25%). \nWampus can move in your room, so you would become a delicious snack (Game Over).\n" << "9. The arrows are magical, so you choose three destinations, knowing where they would fly.\n" << "10. If all arrows are gone, you are defencelence and therefore die of panic (Game Over).\n" << endl << " Notes: " << std::endl << "1. If Wumpus moved to a bat room, then player also moved to this room (containing both bat and wampus already) -\nbat logic activates, so you are saved...probably.\n" << "GOOD LUCK, YUMMY HUMAN...\n" << std::endl; } } int main() { rules(); while (true) { //srand(time(0)); Heart heart; heart.play(); std::cout << "Want to play again? y / n" << std::endl; char answer; cin >> answer; if (answer == 'y') continue; else if (answer == 'n') return static_cast<int>(Heart::EXIT); } } 

heart.h

#pragma once #include "Room.h" #include "Player.h" class Heart { public: enum CONFIG { SHUT_DOWN = 1, EXIT, LOST, VICTORY, CONTINUE }; vector<vector<int> >roomsInitializeArray { //Room 1 Room 2 Room 3 Room 4 Room 5 {2,5,8}, {1,3,10}, {2,4,12}, {3,5,14}, {1,4,6}, //Room 6 Room 7 Room 8 Room 9 Room 10 {5,7,15}, {6,8,17}, {1,7,9}, {8,10,18}, {2,9,11}, // Room 11 Room 12 Room 13 Room 14 Room 15 {10,12,19}, {3,11,13}, {12,14,20}, {4,13,15}, {6,14,16}, // Room 16 Room 17 Room 18 Room 19 Room 20 {15,17,20}, {7,16,18}, {9,17,19}, {11,18,20}, {13,16,19}, }; Heart(); Player p; void initializeRooms(); int validRoom(); void fillRooms(); void moveWumpus(); void batEncounter(); CONFIG playerMove(int); CONFIG moveResult(); CONFIG playerShoot(int, int, int); bool flyCheck(int, int); CONFIG input(); CONFIG inputError(); void hints(); void debug(); CONFIG play(); }; 

Room.h

#pragma once #include <vector> using namespace std; const int ROOMS = 20; class Room { private: static int roomCount; int roomNumber; bool player; bool bat; bool pit; bool wumpus; public: vector<int>adjRooms = vector<int>(3); Room(vector<int>); bool hasBats(); bool hasPit(); bool hasWumpus(); bool hasPlayer(); int getRoomNumber(); bool empty(); void setBats(bool); void setPit(); void setWumpus(bool); void setPlayer(bool); }; 

Player.h

#pragma once class Player { int arrows = 5; int currRoom; public: Player(); int currPlayer() const; void setLocation(int); void fired(); int getArrows(); }; 

Heart.cpp

#include "heart.h" #include <random> #include <iostream> using namespace std; vector<Room> labyrinth; Heart::Heart() { initializeRooms(); fillRooms(); } void Heart::initializeRooms() { //copy from hardcoded vector for (int i = 0; i < ROOMS; i++) { // labyrinth[i] = roomsInitializeArray[i]; either way Room newRoom = roomsInitializeArray[i]; labyrinth.push_back(newRoom); } } int Heart::validRoom() { // checks random room for emptiness; if empty = return index, else recursion int room = rand() % ROOMS; if (labyrinth[room].empty()) return room; else validRoom(); } void Heart::fillRooms() { // set up everything (bool true in valid rooms) p.setLocation(validRoom()); labyrinth[p.currPlayer()].setPlayer(true); labyrinth[validRoom()].setBats(true); labyrinth[validRoom()].setBats(true); labyrinth[validRoom()].setPit(); labyrinth[validRoom()].setPit(); labyrinth[validRoom()].setWumpus(true); } void Heart::moveWumpus() { // 25% chance to move to one of three rooms or remain in the same int tmp = rand() % 4; int currWumpus; for (int i = 0; i < ROOMS; ++i) { if (labyrinth[i].hasWumpus()) { currWumpus = i; break; } } labyrinth[currWumpus].setWumpus(false); if (tmp == 0 or tmp == 1 or tmp == 2) { labyrinth[labyrinth[currWumpus].adjRooms[tmp]].setWumpus(true); cout << "Wampus moved to another room..." << endl; } else if (tmp == 3) { labyrinth[currWumpus].setWumpus(true); cout << "Wampus felt no danger & didn't move to another room" << endl; } } Heart::CONFIG Heart::inputError() { // Called if input is invalid. Clears buffer and passes continue instruction to play() loop (simply re-calls input()). cout << "Wrong input" << endl; cin.clear(); cin.ignore(32767, '\n'); return CONTINUE; }; bool Heart::flyCheck(int target, int start) { // returns true if destination can be chosen // may be odd, however used to simplify shootArrow() return(target == labyrinth[start].adjRooms[0] || target == labyrinth[start].adjRooms[1] || target == labyrinth[start].adjRooms[2]); } Heart::CONFIG Heart::input() { // might be rewrited // asks for input, checks if input is valid, finally laucnhes logic according to input ; if wrong - calls inputError() cout << "Move(m) or Shoot(s) ?" << endl; cout << "To terminate the game enter (T)" << endl; char action; cin >> action; if (action == 'T') return SHUT_DOWN; int room; int target1; int target2; int target3; if (action == 'm') { cout << "Where to ?" << endl; cin >> room; if (cin.fail()) inputError(); else return playerMove(--room); // f.e. player inputs room 1, but program works with it as room[0], therefore room is decremented by 1 } else if (action == 's') { cout << "Where at ?" << endl; cin >> target1; if (cin.fail()) inputError(); --target1; // lowering by one just as the 'm' case if (flyCheck(target1, p.currPlayer())) { cout << "Arrow in room # " << target1 + 1 << ". Enter next destination room: " << labyrinth[target1].adjRooms[0] + 1 << ", " //+1 is made for proper cout (logic works with [index] << labyrinth[target1].adjRooms[1] + 1 << ", " // cout is index[1] (Room[0] = Room#1) << labyrinth[target1].adjRooms[2] + 1 << endl; } else inputError(); cin >> target2; // we get there only after we do flyCheck() with previous target if (cin.fail()) inputError(); --target2; // do it for each cin if (flyCheck(target2, target1)) { cout << "Arrow in room # " << target2 + 1 << ". Enter final destination room: " << labyrinth[target2].adjRooms[0] + 1 << ", " << labyrinth[target2].adjRooms[1] + 1 << ", " << labyrinth[target2].adjRooms[2] + 1 << endl; } else inputError(); cin >> target3; if (cin.fail()) inputError(); --target3; if (flyCheck(target3, target2)) return playerShoot(target1, target2, target3); // if all 3 targets are valid, perform a shot. else inputError(); } else inputError(); } void Heart::hints() { // +1 is used for proper cout, Room[0] represented as Room # 1. cout << "You are in the room # " << p.currPlayer() + 1 << endl; cout << "Adjacent rooms are: " << labyrinth[p.currPlayer()].adjRooms[0] + 1 << " , " << labyrinth[p.currPlayer()].adjRooms[1] + 1 << " , " << labyrinth[p.currPlayer()].adjRooms[2] + 1 << endl; cout << "You have " << p.getArrows() << " arrows left" << endl << endl; // if more than two same objects, received only one message (f.e. two pit rooms = only one message) if (labyrinth[labyrinth[p.currPlayer()].adjRooms[0]].hasBats() || labyrinth[labyrinth[p.currPlayer()].adjRooms[1]].hasBats() || labyrinth[labyrinth[p.currPlayer()].adjRooms[2]].hasBats()) cout << "You hear flapping wings..." << endl << endl; if (labyrinth[labyrinth[p.currPlayer()].adjRooms[0]].hasPit() || labyrinth[labyrinth[p.currPlayer()].adjRooms[1]].hasPit() || labyrinth[labyrinth[p.currPlayer()].adjRooms[2]].hasPit()) cout << "You feel wind..." << endl << endl; if (labyrinth[labyrinth[p.currPlayer()].adjRooms[0]].hasWumpus() || labyrinth[labyrinth[p.currPlayer()].adjRooms[1]].hasWumpus() || labyrinth[labyrinth[p.currPlayer()].adjRooms[2]].hasWumpus()) cout << "You feel the dread within..." << endl << endl; } Heart::CONFIG Heart::playerMove(int newLocation) { // This ?can? be somehow made into Player member function. if (newLocation != labyrinth[p.currPlayer()].adjRooms[0] && newLocation != labyrinth[p.currPlayer()].adjRooms[1] && newLocation != labyrinth[p.currPlayer()].adjRooms[2]) { cout << "Wrong movement!";// << labyrinth[p.currPlayer()].adjRooms[0] << ', ' << labyrinth[p.currPlayer()].adjRooms[1] << ', ' << labyrinth[p.currPlayer()].adjRooms[2] << endl; return CONTINUE; } labyrinth[p.currPlayer()].setPlayer(false); labyrinth[newLocation].setPlayer(true); p.setLocation(newLocation); moveResult(); } void Heart::batEncounter() { // change player position randomly (no emptyness check) int r = rand() % ROOMS; labyrinth[p.currPlayer()].setPlayer(false); labyrinth[p.currPlayer()].setBats(false); labyrinth[r].setPlayer(true); p.setLocation(r); debug(); moveResult(); } Heart::CONFIG Heart::moveResult() { // event reaction if (labyrinth[p.currPlayer()].hasBats()) { cout << "You moved to a room with Huge Bat.\nYou are flying! But where...\n" << endl; batEncounter(); } if (labyrinth[p.currPlayer()].hasPit()) { cout << "You moved to a room with Pit.\nVZUH!\nYou are now many pieces.\n" << endl; return LOST; } if (labyrinth[p.currPlayer()].hasWumpus()) { cout << "You moved to a room with Wumpus.\nMhm... Yummy Human...\nYOU ARE EATEN\n" << endl; return LOST; } return CONTINUE; } Heart::CONFIG Heart::playerShoot(int target1, int target2, int target3) { // shoots an arrow, checks if wampus was hit, WIN if true, else check if Wampus moved to player, LOST if true, else check remaining arrows, LOST if == 0, else continue play() loop p.fired(); if (labyrinth[target1].hasWumpus() || labyrinth[target2].hasWumpus() || labyrinth[target3].hasWumpus()) { cout << "Target practice!\nWampus dead!\nCongratulations!" << endl; return VICTORY; } else { cout << "A miss... what a pity." << endl; moveWumpus(); if (labyrinth[p.currPlayer()].hasWumpus()) { cout << "YOUR ROOM!\nToo late to run, yummy human" << endl; return LOST; } if (p.getArrows() == 0) { cout << "No more arrows...\nYou are defenceless!" << endl; return LOST; } return CONTINUE; } } void Heart::debug() { // only for debug, unused logic for (int i = 0; i < ROOMS; ++i) { cout << "Room# " << i << ":\n"; cout << "Bats: " << labyrinth[i].hasBats() << endl; cout << "Wumpus: " << labyrinth[i].hasWumpus() << endl; cout << "Pits: " << labyrinth[i].hasPit() << endl; cout << "Player: " << labyrinth[i].hasPlayer() << endl; cout << endl << endl; } } Heart::CONFIG Heart::play() { // check input results due to Heart::CONFIG enum, finishes game in LOST / VICTORY / SHUT_DOWN cases while (true) { hints(); CONFIG result = input(); if (result == CONTINUE) continue; else if (result == LOST) { cout << "YOU LOST" << endl << endl; break; } else if (result == VICTORY) { cout << "YOU WON" << endl << endl; break; } else if (result == SHUT_DOWN) return EXIT; } } 

Room.cpp

#include "Room.h" #include <vector> using namespace std; int Room::roomCount = 1; Room::Room(vector<int> v) // accept vector in case of adjucent changes, however only works with dodecahedrone hardcode; :player(false), bat(false), wumpus(false), pit(false), adjRooms{v[0]-1, v[1]-1, v[2]-1}, roomNumber(roomCount++)//Off by one problem sollution //(initializing vector begins with Room#1 but [0]) {} bool Room::hasBats() { return bat; } bool Room::hasPit() { return pit; } bool Room::hasWumpus() { return wumpus; } bool Room::hasPlayer() { return player; } int Room::getRoomNumber() { return roomNumber; } bool Room::empty() { return (!hasBats() && !hasPit() && !hasPlayer() && !hasWumpus()); } void Room::setBats(bool status) { bat = status; } void Room::setPit() { pit = true; } void Room::setWumpus(bool status) { wumpus = status; } void Room::setPlayer(bool status) { player = status; } 

Player.cpp

#include "Player.h" Player::Player() :arrows(5), currRoom(-1) {} int Player::currPlayer() const { return currRoom; } void Player::setLocation(int newLocation) { currRoom = newLocation; } void Player::fired() { arrows--; } int Player::getArrows() { return arrows; } 

As I see the improvements, they might be:

1. Make labyrinth[x].method() into a function with accepted x as room number and method as method. However [x] is sometimes a function, and method() is often an adjRooms[], what is basically and integer.
2. CONST, references& and pointers*, everywhere. Everything is being copied over and over again, it's a long work, but can be done (if it's better).
3. As far as I know, the Interface should be separate with game logic, and input() function should NOT std::cout, however I have no idea how to do it (if it's needed of course).

BOTH CRITICISM AND RECOMMENDATIONS ARE HIGLY APPRECIATED