trjhtr

this is a follow up of:Text based game “Hunt the Wumpus” Version 2

I took the suggestions made there and corrected the code. I think now it got a lot more simple.

Please let me know if you still find something smelly in the code. After this is reviewd i will turn this into a GUI game were the dungeon is displayed.

wumpus.h

#pragma once

#include <array> 
#include <vector>

namespace wumpus 

 using Room_number = int;

 class Dungeon 
 public:
 Dungeon();
 void indicate_hazards();
 bool shoot_arrow(std::vector<int> tar_rooms);
 bool move_wumpus();
 bool move_player(Room_number target_room);
 Room_number select_room_to_move();
 std::array<Room_number, 3> get_neighbour_rooms() const;

 void show_state_of_dungeon(); //only used for debug
 private:
 struct Room 
 std::array <Room*, 3> neighbors nullptr ; //pointer to 3 rooms next to this room
 Room_number room_number 0 ;
 bool has_wumpus false ;
 bool has_pit false ;
 bool has_bat false ;
 bool has_player false ;
 ;

 static constexpr int count_of_pits = 3;
 static constexpr int count_of_bats = 3;

 int arrows = 5;

 std::array<Room, 20> rooms
 
 
 &rooms[1] ,&rooms[4], &rooms[19] ,
 &rooms[0] ,&rooms[2], &rooms[17] ,
 &rooms[1] ,&rooms[3], &rooms[15] ,
 &rooms[2] ,&rooms[4], &rooms[13] ,
 &rooms[0] ,&rooms[3], &rooms[5] ,
 &rooms[4] ,&rooms[6], &rooms[12] ,
 &rooms[5] ,&rooms[7], &rooms[19] ,
 &rooms[6] ,&rooms[8], &rooms[11] ,
 &rooms[7] ,&rooms[9], &rooms[18] ,
 &rooms[8] ,&rooms[10], &rooms[16] ,
 &rooms[9] ,&rooms[11], &rooms[14] ,
 &rooms[7] ,&rooms[10], &rooms[12] ,
 &rooms[5] ,&rooms[11], &rooms[13] ,
 &rooms[3] ,&rooms[12], &rooms[14] ,
 &rooms[10] ,&rooms[13], &rooms[15] ,
 &rooms[2] ,&rooms[14], &rooms[16] ,
 &rooms[9] ,&rooms[15], &rooms[17] ,
 &rooms[1] ,&rooms[16], &rooms[18] ,
 &rooms[8] ,&rooms[17], &rooms[19] ,
 &rooms[0] ,&rooms[6], &rooms[18] ,
 

 ;
 ;

 int get_random(int min, int max);
 void hunt_the_wumpus();
 void instructions();

 std::vector<Room_number> select_rooms_to_shoot();

wumpus.cpp

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

#include "wumpus.h"

namespace wumpus 

 Dungeon::Dungeon()
 
 // create room numbers
 std::array<Room_number,20> random_room_numbers;

 for (size_t i = 0; i < rooms.size(); ++i) 
 random_room_numbers[i] = i + 1;
 

 //generate random numbers to use to put room numbers random
 std::random_device rd;
 std::mt19937 g(rd());
 std::shuffle(random_room_numbers.begin(), random_room_numbers.end(),g);

 // add room numbers randomly
 for (size_t i = 0; i < rooms.size(), i < random_room_numbers.size(); ++i) 
 rooms[i].room_number = random_room_numbers[i];
 

 std::size_t i 0 ;
 rooms[i++].has_player = true;
 rooms[i++].has_wumpus = true;

 for (auto pits count_of_pits ; pits; --pits) 
 rooms[i++].has_pit = true;
 

 for (auto bats count_of_bats ; bats; --bats) 
 rooms[i++].has_bat = true;
 

 std::shuffle(rooms.begin(), rooms.end(), g);
 

 void Dungeon::indicate_hazards()
 
 bool is_first_bat = true;
 bool is_first_pit = true;

 // find the player
 auto player_room std::find_if(rooms.begin(), rooms.end(), (const Room &r) return r.has_player; ) ;

 for (auto& x : player_room->neighbors) 
 if (x->has_wumpus) 
 std::cout << "I smell the wumpusn";
 
 if (is_first_pit && x->has_pit) 
 is_first_pit = false;
 std::cout << "I feel a breezen";
 
 if (is_first_bat && x->has_bat) 
 is_first_bat = false;
 std::cout << "I hear a batn";
 
 

 std::cout << "You are in room " << player_room->room_number << "n"
 << "You have "<<arrows<< " arrow(s) leftn"
 << "Tunnels lead to rooms " 
 << player_room->neighbors[0]->room_number << ", "
 << player_room->neighbors[1]->room_number << " and "
 << player_room->neighbors[2]->room_number << "n"
 << "what do you want to do? (M)ove or (S)hoot?n";
 

 bool Dungeon::shoot_arrow(std::vector<int> target_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;

 // find the player
 auto player_room std::find_if(rooms.begin(), rooms.end(), (const Room &r) return r.has_player; ) ;

 for (const auto& target : target_rooms)

 bool room_reached = false;

 for (const auto& neigbour : player_room->neighbors) 

 if (neigbour->room_number == target) 
 room_reached = true;

 if (rooms[neigbour->room_number - 1].has_wumpus) 
 std::cout << "!!!!!!YOU WON!!!!!!: You killed the Wumpus in room " << rooms[neigbour->room_number - 1].room_number << "n";
 return true;
 
 break;
 
 
 if (!room_reached) 
 std::cout << "Room " << target << " could not be reached from arrown";
 return false;
 
 
 if (arrows == 0) 
 std::cout << "You lost: You ran out of arrows";
 return true;
 
 return false;
 

 bool Dungeon::move_wumpus() 
 
 auto direction = get_random(0, 3);
 if (direction == 3) // 25% chance that wumpus won't move
 return false;
 

 // find the wumpus
 auto wumpus_room std::find_if(rooms.begin(), rooms.end(), (const Room &r) return r.has_wumpus; ) ;

 // move him
 wumpus_room->has_wumpus = false;
 auto new_room = wumpus_room->neighbors[direction];
 new_room->has_wumpus = true;

 if (new_room->has_player) 
 std::cout << "You lost: Wumpus enters your room and eats youn";
 return true;
 
 return false;
 

 bool Dungeon::move_player(Room_number target_room_number)
 //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
 

 // find the player
 auto player_room std::find_if(rooms.begin(), rooms.end(), (const Room &r) return r.has_player; ) ;

 for (auto& x : player_room->neighbors) 

 if (x->room_number == target_room_number) 
 if (x->has_wumpus) 
 std::cout << "You lost: You got eaten by the Wumpusn";
 return true;
 
 else if (x->has_pit) 
 std::cout << "You lost: You fell in a bottomless pitn";
 return true;
 
 else if (x->has_bat) bat_destionation_room->has_bat 
 else 
 player_room->has_player = false;
 auto target_room = &rooms[target_room_number];
 target_room->has_player = true;
 return false;
 
 
 
 std::cerr << "Dungeon::move_player: Unknown target room entered";
 return false;
 

 Room_number Dungeon::select_room_to_move()
 
 for (;;) 

 std::cout << "To where??n";

 Room_number target = 0;
 std::cin >> target;

 if (std::cin.fail()) 
 std::cin.clear();
 std::cin.ignore(999, 'n');
 continue;
 

 auto neighbor = get_neighbour_rooms();

 if (target == neighbor[0] 
 

 std::array<Room_number, 3> Dungeon::get_neighbour_rooms() const
 
 // find the player
 auto player_room std::find_if(rooms.begin(), rooms.end(), (const Room &r) return r.has_player; ) ;

 return std::array<Room_number, 3>
 player_room->neighbors[0]->room_number,
 player_room->neighbors[1]->room_number,
 player_room->neighbors[2]->room_number
 ;
 

 void Dungeon::show_state_of_dungeon()
 
 auto print_rooms = rooms;

 std::sort(print_rooms.begin(), print_rooms.end(), (const Room &a, const Room &b) return b.room_number > a.room_number; );

 for (const auto&room : print_rooms) 
 std::cout << "Room " << room.room_number << " connects to: ";

 for (const auto&neighbor : room.neighbors) 
 if (neighbor != nullptr) 
 std::cout << neighbor->room_number << " ";
 
 else 
 std::cout << "np" << " ";
 
 

 std::cout << " ";
 if (room.has_wumpus) 
 std::cout << "wumpus:" << room.has_wumpus << " ";
 
 if (room.has_pit) 
 std::cout << "pit:" << room.has_pit << " ";
 
 if (room.has_bat) 
 std::cout << "bat:" << room.has_bat << " ";
 
 if (room.has_player) 
 std::cout << "player:" << room.has_player << " ";
 
 std::cout << "n";
 
 

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

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

 void hunt_the_wumpus()
 
 instructions();

 for (;;) // restart game
 
 

 void instructions()
 
 std::cout <<R"(Welcome to "Hunt the Wumpus"!
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).

Hazards
Bottomless pits - two rooms have bottomless pits in them. If you go there, you
fall into the pit(and lose!)
Super bats - two other rooms have super bats.If you go there, a bat grabs you
and takes you to some other room at random. (Which may be troublesome).

Wumpus
The wumpus is not bothered by hazards(he has sucker feet and is too big for a
bat to lift).Usually he is asleep.Two things wake him up : you shooting an
arrow or you entering his room."

If the wumpus wakes he moves(p = .75) one room or stays still(p = .25).After
that, if he is where you are, he eats you up and you lose!"

Each turn you may move or shoot a crooked arrow.
Moving: you can move one room(thru one tunnel).
Arrows : you have 5 arrows.You lose when you run out.Each arrow can go from 1
to 3 rooms.You aim by telling the computer the rooms you want the arrow to go
to.If the arrow can't go that way (if no tunnel) it moves at random to the
next room.If the arrow hits the wumpus, you win.If the arrow hits you, you lose.

Warnings
When you are one room away from a wumpus or hazard, the computer says :

Wumpus: "I smell the wumpus"
Bat : "I hear a bat"
Pit : "I feel a breeze"


"Press any key to start")";

 char c;
 std::cin.get(c);
 

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

 std::string input;
 std::cin >> input;

 std::istringstream ist input ;

 std::vector<int> target_rooms;

 bool bad_input = false;

 while (!ist.eof()) 

 int room_number;
 ist >> room_number;

 if (ist.fail()) 
 bad_input = true;
 break;
 

 target_rooms.push_back(room_number);

 if (target_rooms.size() == 3 

 if (bad_input) 
 continue;
 
 else 
 return target_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 highly recommend you compile your code with clang and use all of its facilities (like sanitizer, which didn't find anything, so good job). Compiling your code with -Wall -Wextra -pedantic -pedantic-errors results in 81 warnings!!

To be fair, some of those warnings are stylistic (-Wmissing-braces), but some of them are correct (-Wunitialized) and there is even a bug in your code uncovered by a warning:

for (size_t i = 0; i < rooms.size(), i < random_room_numbers.size(); ++i)
// ^^^^^^^^^^^^^^^^
// discarded expression

It's not actually a bug, because rooms and random_room_numbers have the same size. More on that later.

As a side note, it's very interesting that you're using try-catch function bodies; I've never seen one of those "in the wild".

1. Don't catch exceptions by reference, catch them by const& or your handler will not get called if I throw v; where v is an lvalue.




2. I'd recommend to use single character literals instead of string literals if you're only going to put in one character (see main.cpp:10); that's a stylistic issue however (feel free to ignore those).




3. You should be consistent. Some things are not (space before < with templates for example). An automated tool (like clang-format) helps a lot for me.




4. Typos: seperator => separator; menue => menu;




5. You don't actually need to catch any exceptions, because not of part of your code throws an exception.




6. (opinion) You should mark class that aren't supposed to be inherited from as final.




7. The -Wuninitialized warning is pretty useful: &rooms[1] dereferences uninitialized memory. rooms[1] returns the first element of rooms, but at that point rooms hasn't been initialized yet. You'll need to defer initialization.



8. 
   

   The initialization of random_room_numbers can be simplified using std::iota:

   
   
   

   std::iota(random_room_numbers.begin(), random_room_numbers.end(), 1);
   

   
   


9. 
   

   To guarantee that random_room_numbers and rooms always have the same size, use a constant:

   
   
   

   static constexpr int total_rooms = 20;
   

   
   



10. (opinion) I'd suggest to name the member variable room_number just number, because the class is already a room, so it's implied.




11. Use asserts to guard against (accidental) precondition violations. For example, in Dungeon::indicate_hazards, you are assuming that player_room != rooms.end(). I'd put an assert(player_room != rooms.end() && "..."); afterwards just to be sure. (In C++20 you'd use contracts for this).




12. Because you're not going to modify target_rooms (in Dungeon::shoot_arrow), take it by const&.




13. Sometimes you use Room_number (+1) for a room number, but you also use just an int :(. Consistency!



14. 
    

    std::cin.ignore(std::numeric_limits<std::streamsize>::max(), 'n'); is safer than some random 999. It also fixes a bug that you had: Try to enter a whitespace followed by a newline. This results in 999 characters been ignored and the game isn't playable anymore.

    
    
    

    There is an exception for the maximum amount, which is why it works in that case, and not in the 999 case.

    
    



15. Instead of checking for stream failure (in Dungeon::select_room_to_move) using fail, usestd::cin'soperator bool:if (!std::cin) /fail/;`.




16. if (game_over == true) can be simplified to if (game_over).




17. The introduction text is not up-to-date. There are three pits and super bats, not two. Might want to use the actual variables instead of hardcoding text.




18. It's my understanding that neighbors cannot ever be nullptr, so the extra if statement for it is unnecessary (wumpus.cpp:226). Might want to consider using an assert instead.




19. It seems like you are using player_room a lot. You might want to consider using a member variable for that so that you avoid to iterate over (potentially) the whole room container.




20. Dungeon::shoot_arrow has a bug. If I shoot my last arrow into a non-existent room, then I can illegally continue ;)




21. Dungeon::move_player has a bug, it moves the player into the wrong room. That's because rooms is not sorted, so rooms[target_room_number] doesn't get you the room with room_number == target_room_number. Use std::find again :)

Apart from that, I really like your game so good job! :)