invitation to hunt_the_wumpus refactoring

[nuderobmonkey]

I have re-coded the console based game Hunt_the_wumpus, just for fun. There I had several considerations made about sparse use of stl libraries, at other side about easiness of the code as such, also about easy readibility.

So, if someone is interested in looking over the code, maybe refactoring it to its own goal (maybe fewer use of libraries, readibility, shortness, a.s.o), or just about discussing the design, feel free to dig in.

The size of the source is about 400 lines, hosted at
https://github.com/NicoSchumann/wumpus

Have fun.

[jonnin]

I think it looks fine, apart from rand(). If you do not care to use <random> then you can find or make a better random generation tool. Given the nature of the project, it may not matter that much.

There may be some way to better organize the hard coded string data, but I am not sure it matters. It seems like there could be some consolidation if you wanted it smaller.

The text in a few places is not correct english, maybe it needs a proofreader, or maybe its for historical reasons. one example:
os << "Your shoot killed the Wumpus.\n"; //shot, not shoot.


if you allow basic stl (2011 or 2017 ish?) you could toss the dynamic memory/graph stuff.

[AbstractionAnon]

As jonnin mentioned it need a proofreader.

The only significant change I would make would be to read the CONNECTIONS data in from a file. This would allow reconfiguring the cave without recompiling the program.

[seeplus]

const int could be replaced with constexpr int as these would then be evaluated at compile time rather than run-time.

operator<<() L48 - 54 could have a look-up array rather than multiple if statements - EMPTY is 0, PIT is 3.

L70 & 72 could be:

Cave() : rooms(new Room[CAVE_SIZE])
    {

idx is defined on L340 and then also within that scope on L345 - just a warning.

When playing and an invalid room number is entered, who might want to display a suitable message (eg Whoops you just tried to walk through solid rock)

Where a variable is initialised but it's value doesn't change then it can be marked as const. If the type of the variable can be obtained from it's initialisation value, then auto can be used.

Also in modern C++, {} is preferred to = for variable definition initialisation.

eg L84 would be

const auto i {std::rand() % CAVE_SIZE};

Also since C++17 you can define a variable as part of an if statement (also with switch). So eg L84-89 could be:

if (const auto i {std::rand() % CAVE_SIZE}; rooms[i].content == EMPTY) {
    rooms[i].content = WUMPUS;
    wumpus_room = &rooms[i];
    break;
}

[deleted account xyzzy]

seeplus wrote:

Also in modern C++, {} is preferred to = for variable definition initialisation.

I happen to agree with that, but......

There are people who nay-say that. Preferring to cling to the old ways.

http://www.cplusplus.com/forum/beginner/280882/

[lastchance]

Also in modern C++, {} is preferred to = for variable definition initialisation.

I happen to agree with that, but......

There are people who nay-say that. Preferring to cling to the old ways.

#include <iostream>
using namespace std;

const double TWOPI = 6.28319;

int main()
{
   // How many complete revolutions?
   int u{ 15.0 / TWOPI   };    // This warns in g++ and refuses in cl
   int v{ 15.0 / 6.28319 };    // This refuses
   int w = 15.0 / TWOPI;       // This does exactly what I wanted it to
}

Sometimes the old ways are best.

[jonnin]

that is the point of {} -- to fuss if your assignment isn't type matched, for better or worse. For objects, that can be a good warning to have. For numbers, its just an aggravation.

you could let the user put in a seed to get the same map or a new one etc. Take that a step farther: old school console save style.. user puts in a value to get back to where they were :)

[nuderobmonkey]

Thank you all so far for your hints and suggestions. I tried to use all of the suggestions for where I thought that it would make the code shorter, faster and easier to read.

It would be nice, if someone could provide some code which shows me how to substituting the Room * ptrs with unique/shared ptrs. Thanks.

[seeplus]

include memory, then simply just:

//Room* rooms;
    const std::unique_ptr<Room[]> rooms {std::make_unique<Room[]>(CAVE_SIZE)};

        //rooms = new Room[CAVE_SIZE]; 

 //~Cave() { delete[] rooms; } 

[JLBorges]

> include memory, then simply just:

Why is a smart pointer required for this? What is wrong with using std::vector<>?

[kbw]

Typo, missing the reference:

std::ostream & operator<<( std::ostream & os, const Content c )

Also, if you didn't use std::cin for your input, you could navigate with arrow keys.

[nuderobmonkey]

Also, if you didn't use std::cin for your input, you could navigate with arrow keys.

What input stream/fileptr should I then use as an alternative way?
Consider that I want to use merely std::, no external libraries.

@JLBorges

Why is a smart pointer required for this? What is wrong with using std::vector<>?

Asking about using of smart ptrs was merely a technical question. That's because I yet know how to use it with std::vector/array. But anyway, I don't want to change my code, also not for using smart ptrs.

[kbw]

What input stream/fileptr should I then use as an alternative way?

You'll have to go for something platform specific.

On Microsoft platforms, there's getch().

You can do similar things on posix, https://newbedev.com/what-is-the-equivalent-to-getch-getche-in-linux

[mbozzi]

Attempted this myself, obviously it's heavily based on @nuderobmonkey's

#include <cstdlib>
#include <ctime>
#include <iostream>
#include <sstream>
#include <string>

int constexpr n_rooms     = 20;
int constexpr n_pits      = 2;
int constexpr n_bats      = 2;
int constexpr max_args    = 5;
int constexpr arrow_range = 3;

struct room { int a, b, c; enum { empty, wumpus, bat, pit } content = empty; };
struct command { char c; int n_args; int args[max_args]; };

room cave[n_rooms + 1] =
{ {  0,  0,  0 }, { 14, 17, 20 }, {  3,  9,  6 },
  {  2,  4, 11 }, {  3,  5, 13 }, {  4,  6, 15 },
  {  2,  5,  7 }, {  6,  8, 16 }, {  7,  9, 18 },
  {  2,  8, 10 }, {  9, 11, 19 }, {  3, 10, 12 },
  { 11, 13, 20 }, {  4, 12, 14 }, { 13, 15,  1 },
  {  5, 14, 16 }, {  7, 15, 17 }, { 16, 18,  1 },
  {  8, 17, 19 }, { 10, 18, 20 }, { 12, 19,  1 } };

[[nodiscard]] static room& random_room() { return cave[1 + (rand() % n_rooms)]; }
[[nodiscard]] static room& random_empty_room()
{
  room* r; do r = &random_room(); while (r->content != room::empty); return *r;
}

[[nodiscard]] static room& random_adjacent_room(room const& r)
{
  switch (rand() % 3) { case 0: return cave[r.a]; case 1: return cave[r.b]; default: return cave[r.c]; };
}

[[nodiscard]] static bool adjacent(room const& r, int n)
{ return (r.a == n) || (r.b == n) || (r.c == n); }

[[nodiscard]] static int count_adjacent_rooms_with_content(room const& r, int c)
{
  return (cave[r.a].content == c) + (cave[r.b].content == c) + (cave[r.c].content == c);
}

[[nodiscard]] static command prompt_read_command()
{
  command result {};
  
  for (std::string line; ;)
  {
    std::cout << "Move or shoot (m-s)? ";
    
    std::getline(std::cin, line);
    std::istringstream iss{line};

    if (std::string word; iss >> word)
    {
      result.c = word[0];
      while (result.n_args < max_args && iss >> result.args[result.n_args])
        ++result.n_args;
      
      break; // succeed, ignoring string->int conversion failures
    }
  }

  return result;
}

int main() try
{
#ifdef DETERMINISTIC_RANDOM_SEED
  std::srand(42);
#else
  std::srand(static_cast<unsigned>(std::time(0)));
#endif
  std::cin.exceptions(std::ios::failbit | std::ios::badbit | std::ios::eofbit);

  int n_arrows = 5;
  int wumpus_turns_awake = 0;
  bool wumpus_killed = false;
  bool player_killed = false;

  room* wumpus_room = &random_room();
  for (int i = 0; i < n_pits; ++i) random_empty_room().content = room::pit;
  for (int i = 0; i < n_bats; ++i) random_empty_room().content = room::bat;
  room* player_room = &random_empty_room();

  while (true)
  {
    // Is it "wumpuses" or "wumpi"?
    auto const [a, b, c, _] = *player_room;
    std::cout << "You're in room " << player_room - cave << ".\n";
    std::cout << "There are tunnels leading to rooms "
              << a << ", " << b << ", and " << c << ".\n";
    if (n_arrows > 0) std::cout << "You have " << n_arrows
                                << (n_arrows == 1? " arrow": " arrows") << " left.\n";
    else std::cout << "You're entirely out of arrows.  It's probably time to escape.\n";
    int const nearby_wumpuses = count_adjacent_rooms_with_content(*player_room, room::wumpus); 
    int const nearby_pits = count_adjacent_rooms_with_content(*player_room, room::pit);
    int const nearby_bats = count_adjacent_rooms_with_content(*player_room, room::bat);
    
    if (nearby_pits == 1) std::cout << "You feel a cold draught in the air.\n";
    if (nearby_pits >= 2) std::cout << "An icy wind blows from adjacent rooms.  Your torch flickers.\n";
    if (nearby_bats == 1) std::cout << "You hear a light rustle in the air.\n";
    if (nearby_bats >= 2) std::cout << "Quiet clicking can be heard through two of the nearby tunnels.\n";
    if (nearby_wumpuses >= 1) std::cout << "It smells somewhat. Maybe a wumpus is near.\n";

    command const cmd = prompt_read_command();

    if (cmd.c == 'q') { std::cout << "bye...\n"; return 0; }
    if (cmd.c == 'm')
    {
      int const r = cmd.n_args? cmd.args[rand() % cmd.n_args]: 0;
      
      if (cmd.n_args && adjacent(*player_room, r))
      {
        player_room = cave + r;
      }
      else
      {
        std::cout << "Disorientation has taken hold.  You squeeze into a nearby tunnel at random.\n";
        player_room = &random_adjacent_room(*player_room);
      }
    }
  
    if (cmd.c == 's')
    {
      if (++wumpus_turns_awake == 1)
        std::cout << "A thunderous roar echoes through the tunnels.\n";

      if (n_arrows == 0)
      {
        std::cout << "You reach for an arrow, but you have none.\n";
      }
      else
      {
        std::cout << "You loose an arrow.\n";
        --n_arrows;
    
        room* arrow_room = player_room;
        for (int i = 0; i < arrow_range; ++i)
        {
          arrow_room = (i < cmd.n_args && adjacent(*arrow_room, cmd.args[i]))
            ? cave + cmd.args[i]: &random_adjacent_room(*arrow_room);
      
          if (arrow_room == player_room) { player_killed = true; break; }
          if (arrow_room == wumpus_room) { wumpus_killed = true; break; }
        }
      }
    }

    if (wumpus_turns_awake >= 1)
    {
      wumpus_room->content = room::empty;
      wumpus_room = &random_adjacent_room(*wumpus_room);
      wumpus_room->content = room::wumpus;
    }

    if (player_room->content == room::bat)
    {
      std::cout << "Claws dig into your shoulders: a huge creature grabs you and carries you through the cave.\n";
      player_room->content = room::empty;
      player_room = &random_room();
      random_empty_room().content = room::bat;
    }
    if (player_room->content == room::pit)
    {
      std::cout << "The ground gives out, and you plummet to your death.\n";
      return 0;
    }
    if (player_room == wumpus_room)
    {
      std::cout << "The wumpus is upon you.  Your death is painful and swift.\n";
      return 0;
    }
    if (player_killed)
    {
      std::cout << "Your own arrow strikes you in the back.\nYou fall to the ground, dead.\n";
      return 0;
    }
    if (wumpus_killed)
    {
      std::cout << "You hear a mighty scream as your arrow strikes the wumpus.  You have slain your prey.\n";
      return 0;
    }
    std::cout << '\n';
  }
} catch (std::ios::failure const&) { return 0; }