David Longnecker

I’ve been working on a new release for a game server I operate on the side and, with any new release, comes opportunities to weep at refactor your older code.

The current adventure is diving into our menuing system–allowing players to interact and engage with various systems using their keyboard or controllers. One menu system we have has several areas that users can flip through and teleport around. Easy enough, but, was originally written as an if/then statement.

Here’s the gist:

void show (CEntity* PEntity, int page) {
  // null checks skipped
  
  std::ostringstream menu;
  std::vector<string_t> options;
  
  if (page == 0) {
    options.emplace_back("\"Option 1\"");
    options.emplace_back("\"Option 2\"");
    options.emplace_back("\"Option 3\"");
    options.emplace_back("\"Option 4\"");
  else if (page == 1) {
    // more options
  }
  
  std::copy(options.begin(), options.end(), std::ostream_iterator<std::string>(menu, " ");
  PEntity->pushPacket(new CMenuPacket(PEntity, menu.str());
}

void respond (CEntity* PEntity, int location) {
  // null checks skipped
  
  if (location == 0) {
    custom_menu::delay();
    custom_menu::teleportPlayer(PEntity, -18.31f, 3.37f, -256.42f, 30, 205);
    PChar->pushPacket(new CChatPacket(PEntity, MSG::SYSTEM, "Welcome to the jungle!"));
  }   
  else if (location == 1) {
    // do more things
  end
  ...
}

So let’s get this out of the way…

why didn’t you use a switch?

1. Most of the // do things are fairly long and call other methods.
2. The compiled performance gain vs. readability wasn’t worth the squeeze.
3. What started as 3-4 locations 5 years ago… has grown to nearly 60 today.

I’ve recently rolled off another small project where I dug a bit into using maps and composite keys in vectors (similar to the work demonstrated in this post) and thought they’d make a more readable example. Let’s dig in!

Maps everywhere!

If you’re familiar with using typed tuples or dynamics in C#, this feels pretty similar! There’s two different types in this example: unordered_map and map.

They’re both associative containers that store key-value pairs, but vary in how they store, performance, and ordering.

How do we update our code?

Let’s start with our show() method above. Each if/then added options back into a vector (the menu items) based on which page they were on. Since we care about the order here, we’ll want a map for this job.

void show (CEntity* PEntity, int page) {
  // null checks skipped
  
  std::ostringstream menu;
  std::vector<string_t> options;
  
  const std::map<int, std::vector<std::string>> pages = {
    { 1, { "\"Option 1\"", "\"Option 2\"", "\"Option 3\"", "\"Option 4\"" },
    { 2, { "\"Option 5\"", "\"Option 6\"", "\"Option 7\"", "\"Option 8\"" },
    ...
  }
  
  if (pages.count(page) > 0) {
    for (const auto& option : pages.at(page)) {
       options.emplace_back(option);
    }
  }

  // packet stuff here
}

Here, we’re doing the following:

• creating a map that has each of our pages, in order that we want to iterate them later on,
• using pages.count(page) to ensure that our passed in page exists in our vector,
  • NOTE: since a map can only have one instance of a key at compile time, this is a safe check (though, on a larger map, pages.find(page) != pages.end() may be quicker as it stops once it finds a match)
• iterate through the pages found in that vector to emplace back into our options vector.

From a readability perspective, it’s a lot easier to see the options on each page rather than having them embedded in the if/thens!

Next, let’s take a look at our respond() method. For this one, since we’re just doing matches to the maps, not outputs, order doesn’t matter, so we’ll stick to unordered_map and shrinking down all of those individual calls to teleport the player and tell them where they landed (in case they forgot…).

void respond (CEntity* PEntity, int location)
  // null checks skipped
  
  // all the multiple dimensions!
  const std::unordered_map<std::string, std::tuple<float, float, float, int, int, std::string>> locations = {
    { "Option 1", { -18.31f, 3.37f, -256.42f, 30, 205, "Welcome to the Jungle!" } },
    { "Option 2", { -28.31f, 4.37f, -356.42f, 30, 206, "Welcome to the Forest!" } },
    ...
  }
  
  // necessary try/catch wraps, of course, should be here!
  
  if (locations.count(locations) > 0) {
    const auto action = locations.at(location);
    custom_menu::teleportPlayer(PEntity, std::get<0>(action), std::get<1>(action), std::get<2>(action), 
      std::get<3>(action), std::get<4>(action));
    PEntity->pushPacket(new CChatPacket(PEntity, MSG::SYSTEM, std::get<5>(action));
  }
}

Here, we’re doing the following:

• creating an unordered_map that has each location, but in a single location for easier maintenance,
• again using our locations.count(location) to ensure the location exists in the vector,
• a quick auto for our tuple so the std::get doesn’t get out of hand later
• and then using our values.

The type-safe tuples also add a layer of safety to ensure we’re passing the right data values to the packets. Extra bonus!

Overall, while this shaved about 137 lines of code off the page, the big win was readability and maintainability as the menus grow.

Other ideas for optimization:

• pull the const maps out of each method and put them in the .h file,
• see if there’s a way to add another dimension to the map and have a single map for everything,
• now that there’s a standard format, can it be ingested in real time, so that you could build these front-end via lua or other scripting language?

Hmm. =]

What about you? Do you have any unwieldy if/then or switch statements that maps could help with?