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`](https://en.cppreference.com/w/cpp/algorithm/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 `assert`s 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, use `std::cin`'s `operator bool`: `if (!std::cin) /*fail*/;`.
16. (I'm not sure about this point). Isn't `distribution` in `get_random` not supposed to be `static` too?
17. `if (game_over == true)` can be simplified to `if (game_over)`.
18. 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.
19. 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.
20. Instead of having a `has_wumpus`, `has_pit`, `has_whatever`, ..., it's better to use one `enum` and `has_pit`:
enum class RoomInhabitant {
Nobody,
Player,
Wumpus,
Bat
};
Then you can also use a `switch` instead of an if cascade if you'd like. This has also an additional advantage is that you cannot break invariants easily: In your posted code, you can have a room with `has_pit == has_bat == true`, which is not supposed to be possible. With an enum you do not have this problem.
21. 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.
22. `Dungeon::shoot_arrow` has a bug. If I want to shoot an arrow into `3-5-9`, then I only lose one arrow. I don't think that this intended. Also, if I shoot my last arrow into a non-existent room, then I can illegally continue ;).
23. `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! :)