1
\$\begingroup\$

I have revised my other question's code which you can find a link to here.

This is just an automated turn based battle simulator which means everything comes out about 1 second at a time, and player and enemies roll dice to determine the amount of damage they have done.

I have updated it thanks to the last answer doing the things below:

  • Added Constructors
  • More Efficient Functions
  • Better Classes (I think)

Here is the code separated into two files below, with one called dependencies.h and the other called main.cpp

dependencies.h

#ifndef __dependencies_H_INCLUDED__
#define __dependencies_H_INCLUDED__
using std::cout;
using std::cin;
using std::vector;
using std::string;
// roll the dice
int rollDice(int diceType) {
  int diceSide = rand() % diceType;
  return diceSide;
}

// player class
class Player {
 public:
  string name;
  string classType;
  int strength;
  int endurance;
  int id;
  int DT = 20;

  int getHP() { return (10 + ((strength + endurance) * 2)); }
  int getHit() { return rollDice(DT); }

  Player(string inpName, string inpClassType, int inpStrength, int inpEndurance,
         int inpId) {
    name = inpName;
    classType = inpClassType;
    strength = inpStrength;
    endurance = inpEndurance;
    id = inpId;
  }
};

// Enemies
class Enemy {
 public:
  // var declaration
  string name;
  double AC;  // armor class ablity to resist hits
  int DT;  // dice used to attack
  int eid;  // id for Enemies (Enemy id)
  int HP = round(HP * (1 + (AC * 10)));
  int getHit() { return rollDice(DT); }

  Enemy(string inpName, int inpHP, double inpAC, int inpDT, int inpEid) {
    name = inpName;
    HP = inpHP;
    AC = inpAC;
    DT = inpDT;
    eid = inpEid;
  }
};

// Get context of the situation
string enemyContxt(int option) {
  int randNum = round(rand() % (9) + 1);

  static const string names[] = {
      "\nBruneor the ", "\nRichard the ", "\nFilbert the ", "\nLodric the ",
      "\nRuuker the ",  "\nKruger the ",  "\nCharles the ", "\nAaarl the ",
      "\nVasiilk the ", "\nGubl the "};

  static const string introductions[] = {
      " hits you with a blunt slinky ", " whacks you with a feather ",
      " pushes you into Tiny Tim ", " stabs you with a lamp ",
      " shoots you with an M16 catapult ", " summons a spirit to pester you ",
      " uses a rune-stothe enchantment ",
      " tries to curse you but explodes an unfourtunate chicken, due to a "
      "terrible mispronuncation of your name ",
      " simply does nothing ", " burps up a gnerm (a miniature knome) "};

  static const string transitions[] = {
      "and says 'Die, filthy swine!' ",
      "then trips on a gruubliyth. ",
      "and then, snarls! ",
      "and then begins to mutter an ancient curse! ",
      "then yells 'You hit like a Kerbb hehe!' ",
      "and says 'Die, filthy swine!' ",
      "then trips on a gruubliyth. ",
      "and then, snarls! ",
      "and then begins to mutter an ancient curse! ",
      "then yells 'You hit like a Uerbb hehe!' "};

  switch (option) {
    case 1:
      return names[randNum - 1];
    case 2:
      return introductions[randNum - 1];
    case 3:
      return transitions[randNum - 1];
  }
}

// Get context of the situation
string playerContxt(Player &player) {
  int randNum = round(rand() % (19) + 1);
  // pretty much the same thing with the finction above
  string name = player.name;

  if (randNum == 1) return "\n" + name + " strikes with an evil Urrgleumbeck ";
  if (randNum == 2)
    return "\n" + name +
           " hits, but epicly fails and hits a wall causing a rupture in time "
           "itself ";
  if (randNum == 3) return "\n" + name + " trips on an explosive turtle ";
  if (randNum == 4) return "\n" + name + " lunges at his enemy ";
  if (randNum == 5)
    return "\n" + name + " sneezes violently causing a worldwide pandemic ";
  if (randNum == 6)
    return "\n" + name + " swiftly hacks at his enemy using a knerm ";
  if (randNum == 7) return "\n" + name + " summons the almighty mega-knerm ";
  if (randNum == 8) return "\n" + name + " summons a crude writhe-golem ";
  if (randNum == 9) return "\n" + name + " casts an ancient curse";
  if (randNum == 10) return "\n" + name + " yells 'AVADA CADABRA!' ";
  if (randNum == 11) return "\n" + name + " falls painfully ";
  if (randNum == 12) return "\n" + name + " throws a strauug gas grenade ";
  if (randNum == 13) return "\n" + name + " fires a portable villkreek mortar ";
  if (randNum == 14)
    return "\n" + name + " strikes with a pirated knerm sword ";
  if (randNum == 15)
    return "\n" + name +
           " drinks a super-enchantment giving him the ability to eat apples "
           "10 times faster than normal ";
  if (randNum == 16)
    return "\n" + name + " summons Tiny Tim who calls upon his  liege ";
  if (randNum == 17)
    return "\n" + name + " calls upon a skeleton to do his bidding";
  if (randNum == 18) return "\n" + name + " strikes with a molten axe";
  if (randNum == 19)
    return "\n" + name + " hits a tree with his head causing it to fall ";
  if (randNum == 20)
    return "\n" + name + " calls upon the ancient curse of Ugaar ";
}

// fight an Enemy (option 1)
int fightEnemy(Player &player, Enemy &enemy) {
  int eHit = enemy.getHit();
  int pHit = player.getHit();
  int eHP = enemy.HP;
  int pHP = player.getHP();
  int playerLastRoll = pHit;
  int enemyLastRoll = eHit;
  int counter = 0;
  string name = enemyContxt(1);
  cout << "\n->->->->->->->->->->->->->->->->->->->->->->->->->->->->->->\n";
  cout << "Welcome to the Arena!\n";
  cout << "Starting HP: \n" << player.name << "'s HP: " << pHP << "\n"
       << enemy.name << "'s HP: " << eHP << "\n";
  cout << "Begin Battle!\n";
  for (;;) {
    while (playerLastRoll == pHit || enemyLastRoll == eHit && counter != 0) {
      eHit = enemy.getHit();
      pHit = player.getHit();
    }
    sleep(1);
    cout << name << enemy.name << enemyContxt(2) << enemyContxt(3) << "Dealing "
         << eHit << " Damage!\n";
    pHP = pHP - eHit;
    if (pHP <= 0) {
      cout << "\n" << player.name << " is Dead!\n";
      cout << enemy.name << "'s HP left: " << eHP << "\n";
      cout << player.name << "'s HP left: " << pHP << "\n";
      break;
    } else {
      sleep(1);
      cout << "\n" << playerContxt(player) << " Dealing " << pHit
           << " Damage!\n";
      eHP = eHP - pHit;
      if (eHP <= 0) {
        cout << "\n" << enemy.name << " is Dead!\n";
        cout << enemy.name << "'s HP left: " << eHP << "\n";
        cout << player.name << "'s HP left: " << pHP << "\n";
        break;
      }
    }
    playerLastRoll = pHit;
    enemyLastRoll = eHit;
    counter++;
  }

  return 0;
}

// fight a Player (option 2)
int fightPlayer(Player &player1, Player &player2) {
  int pHit1 = player1.getHit();
  int pHit2 = player2.getHit();
  int pHP1 = player1.getHP();
  int pHP2 = player2.getHP();
  int playerLastRoll1 = pHit1;
  int playerLastRoll2 = pHit2;
  int counter = 0;
  cout << "\n->->->->->->->->->->->->->->->->->->->->->->->->->->->->->->\n";
  cout << "Welcome to the Arena!\n";
  cout << "Starting HP: \n" << player1.name << "'s HP: " << pHP1 << "\n"
       << player2.name << "'s HP: " << pHP2 << "\n";
  cout << "Begin Battle!\n";
  for (;;) {
    while (playerLastRoll1 == pHit1 ||
           playerLastRoll2 == pHit2 && counter != 0) {
      pHit1 = player1.getHit();
      pHit2 = player2.getHit();
    }
    sleep(1);
    cout << "\n" << playerContxt(player1) << " Dealing " << pHit1
         << " Damage!\n";
    pHP2 = pHP2 - pHit1;
    if (pHP2 <= 0) {
      cout << "\n" << player2.name << " is Dead!\n";
      cout << player1.name << "'s HP left: " << pHP1 << "\n";
      cout << player2.name << "'s HP left: " << pHP2 << "\n";
      break;
    } else {
      sleep(1);
      cout << "\n" << playerContxt(player2) << " Dealing " << pHit2
           << " Damage!\n";
      pHP1 = pHP1 - pHit2;
      if (pHP1 <= 0) {
        cout << "\n" << player1.name << " is Dead!\n";
        cout << player1.name << "'s HP left: " << pHP1 << "\n";
        cout << player2.name << "'s HP left: " << pHP2 << "\n";
        break;
      }
    }
    playerLastRoll1 = pHit1;
    playerLastRoll2 = pHit2;
    counter++;
  }

  return 0;
}

int enemyComboCheck(int id1, int id2, vector<Player> &allPlayers,
                    vector<Enemy> &allEnemies) {
  vector<Player>::iterator player_iter = allPlayers.begin();
  vector<Enemy>::iterator enemy_iter;
  bool found = false;

  // try to find player
  while (player_iter != allPlayers.end() && !found) {
    if (player_iter->id == id1)
      found = true;
    else
      player_iter++;
  }

  // when player has been found, try to find enemy
  if (found) {
    found = false;
    enemy_iter = allEnemies.begin();

    while (enemy_iter != allEnemies.end() && !found) {
      if (enemy_iter->eid == id2)
        found = true;
      else
        enemy_iter++;
    }

    // if both have been found call function fightEnemy
    if (found) fightEnemy(*player_iter, *enemy_iter);
    return 0;
  }
  return 0;
}

int playerComboCheck(int id1, int id2, vector<Player> &allPlayers) {
  vector<Player>::iterator player_iter1 = allPlayers.begin();
  vector<Player>::iterator player_iter2;
  bool found = false;

  // try to find player 1
  while (player_iter1 != allPlayers.end() && !found) {
    if (player_iter1->id == id1)
      found = true;
    else
      player_iter1++;
  }

  // when player has been found, try to find player 2
  if (found) {
    found = false;
    player_iter2 = allPlayers.begin();

    while (player_iter2 != allPlayers.end() && !found) {
      if (player_iter2->id == id2)
        found = true;
      else
        player_iter2++;
    }

    // if both have been found call function fightPlayer
    if (found) fightPlayer(*player_iter1, *player_iter2);
    return 0;
  }
  return 0;
}

#endif

main.cpp

// all the libraries
#include <iostream>
#include <string>
#include <time.h>
#include <stdlib.h>
#include <stdio.h>
#include <unistd.h>
#include <math.h>
#include <vector>
#include "dependencies.h"

using std::cout;
using std::cin;
using std::vector;
using std::string;

// main function
int main() {
  srand(time(NULL));
  vector<Player> allPlayers;
  vector<Enemy> allEnemies;

  // player declaration
  allPlayers.push_back(Player("Peter", "Scribe", 3, 6, 1));
  allPlayers.push_back(Player("Cole", "Cursed", 3, 3, 2));
  allPlayers.push_back(Player("Mason", "Thief", 3, 1, 3));
  allPlayers.push_back(Player("Miles", "Paladin", 9, 5, 4));
  allPlayers.push_back(Player("Ashton", "Hobo", 3, 7, 4));

  // Enemy declaration below
  allEnemies.push_back(Enemy("Amatuer Guard", 10, 0.05, 4, 1));
  allEnemies.push_back(Enemy("Mediocre Guard", 15, 0.1, 8, 2));
  allEnemies.push_back(Enemy("Knight", 20, 0.2, 12, 3));
  allEnemies.push_back(Enemy("Okachi Raider", 10, 0.15, 6, 4));
  allEnemies.push_back(Enemy("Okachi Paladin", 25, 0.1, 10, 5));
  allEnemies.push_back(Enemy("Okachi Mage", 15, 0, 20, 6));
  allEnemies.push_back(Enemy("Frost Giant", 30, 0.2, 10, 7));
  allEnemies.push_back(Enemy("Dune Viper", 10, 0.15, 10, 8));
  allEnemies.push_back(Enemy("Burrowurm", 10, 0.25, 8, 9));
  allEnemies.push_back(Enemy("Behemoth", 65, 0.25, 6, 10));
  allEnemies.push_back(Enemy("Rabid Squirrel", 10, 0, 20, 11));
  allEnemies.push_back(Enemy("Wolf", 15, 0.2, 8, 12));
  allEnemies.push_back(Enemy("Dire Wolf", 20, 0.2, 12, 13));
  allEnemies.push_back(Enemy("Insane Hobo", 10, 0.05, 12, 14));
  allEnemies.push_back(Enemy("Forest Archer", 15, 0.15, 10, 15));
  allEnemies.push_back(Enemy("Forest Knight", 20, 0.1, 8, 16));
  allEnemies.push_back(Enemy("Forest Mage", 10, 0.15, 10, 17));
  allEnemies.push_back(Enemy("Murder Of Crows", 10, 0.1, 10, 18));

  // core
  int choice = 0;
  cout << "Please select a function: \n";
  cout << "1) Fight Against an Enemy\n";
  cout << "2) Fight Against Another Player\n";
  cout << "Response: ";
  cin >> choice;

  switch (choice) {
    case 1:
      int uid1;
      int uid2;

      cout << "\nEnter your Id (1-5): ";
      cin >> uid1;
      cout << "\nEnter your Enemies Id (1-18): ";
      cin >> uid2;
      enemyComboCheck(uid1, uid2, allPlayers, allEnemies);
      break;

    case 2:
      cout << "\nEnter Player 1's Id (1-5): ";
      cin >> uid1;
      cout << "\nEnter Player 2's Id (1-5): ";
      cin >> uid2;

      playerComboCheck(uid1, uid2, allPlayers);
      break;

    default:
      cout << "Input invalid. Bye!\n";
      return 0;
      break;
  }
  // ask user if they want to run the program again
  string endornot;
  sleep(1);
  cout << "\nRun again y/n: ";
  cin >> endornot;
  if (endornot == "y" || endornot == "Y") return main();
  if (endornot == "n" || endornot == "N") cout << "Shutting Down...\n";
  return 0;
}
\$\endgroup\$
2
\$\begingroup\$

Illegal Code

Reserved Identifiers

#ifndef __dependencies_H_INCLUDED__
#define __dependencies_H_INCLUDED__

The above two lines contain identifiers that are reserved by the implementation (this means you are not allowed to use them). You can not have double underscore anywhere in your identifier. Also avoid using a leading underscore (the rules are complex).

Also macros (things defined with #define are traditionally all upper case. This will avoid clashes with identifiers that have more restricted scope rules.

Personally I use my namespace and file name as part of the macro guards. Since I own the "ThorsAnvil.com" domain I use this as my top level namespace to make it unique.

#ifndef THORSANVIL_BATTLE_SIM_DEPENDENCIES_H
#define THORSANVIL_BATTLE_SIM_DEPENDENCIES_H

namespace ThorsAnvil    // PS Get your own domain.
{
    namespace BattleSim
    {
        class Bob{public: Bob();};
        // Your Stuff
    }
}

#endif

Main is Special

Main is special and has a couple of restrictions. One of these restrictions is that it is not reentrant. This means you can not call main() recursively (even indirectly) from within your own code.

So this is also illegal:

if (endornot == "y" || endornot == "Y") return main();

The simplest solution is to move all your code into a new function main2() and then simply add this main.

int main(int argc) {
    srand(time(nullptr));
    main2();
}

Note. By recursively calling main you are re-seeding the random number generator which is bad and result in even worse random numbers. Only ever call srand() once in an application. Thus in my code I pulled stand() from main2() back into main() so it is only called once.

OK. Moving the code to main2() and calling it recursively may be the simplest solution. But it is not the best. It would be better to use a loop rather than recursion to get your code working.

Let me introduce you to the concept of Self Documenting Code.

This is where you break the code into small functions with meaningful names. So the reader can guess what is happening before they read the contained code, and the part where it is used reads like it is a document.

void playGame(vector<Player>& allPlayers, vector<Enemy>& allEnemies)
{
    bool finished = false;
    while(!finished)
    {
        int choice = getUserChoice();
        playRound(choice, allPlayers, allEnemies);
        finished   = askUserIfWeAreFinished();
    }
}
int main()
{
    srand(time(nullptr));
    vector<Player> allPlayers(getPLayers());
    vector<Enemy>  allEnemies(getEnemies());

    playGame(allPlayers, allEnemies);
}

Bad Ideas

Last Time I told you not to use using namespace std; and you have taken it out and replaced it with:

using std::cout;
using std::cin;
using std::vector;
using std::string;

This is better if used in the correct context. BUT you can never do this in a header file. If you ever want to write code for a shared project (or a project with two or more people) then this will eventually cause a breakage.

This is because including your header file can potentially change the meaning of my code. You never want your code to change the behavior of other peoples code. This will get your code banned from projects and your hand slapped.

Now I know the next thing you say will be. But this is my project and nobody will ever use this code. First never say never (stranger things have happened). But the main point is that this leads to bad habits. You may do it here because you are trying to be lazy and cut down typing. Then it will become the normal thing you do then when you work on other projects you will do it without thinking (muscle memory). So it is best to break the habit early.

Now you can use the above in a source file (but I would advice against it). Use the using clause in as tight as scope as possible.

The exception to the rule.

As always there are exceptions to the rule. When writing your code. When you declare your types in a namespace declaration in the header file there is no prefixes. But when you then start to define your types in the source file you end up adding all the prefixes.

Example

#include "dependencies.h"
ThorsAnvil::BattleSim::Bob::Bob()
{
    std::cout << "Bob built\n";
}

This is the only time I use the using clause (in my source file) where I am defining the types for my class.

#include "dependencies.h"
using namespace ThorsAnvil::BattleSim;
Bob::Bob()
{
    std::cout << "Bob built\n";
}

This is what it was actually designed for.

There is one time where saving typing is OK. But it uses namespace alias to shorten a deeply nested set of namepaces into a single name.

namespace foo {
    namespace bar {
         namespace baz {
             class FantasticMagic {};
         }
    }
}

// In source file:
namespace fbz = foo::bar::baz;

int main()
{
    fbz::FantasticMagic  value;  // notice it still has a prefix.
}

You will see this a lot around the IO code in boost (as it at least two namespace deep).

Rand

int rollDice(int diceType) {
  int diceSide = rand() % diceType;
  return diceSide;
}

You need to re-read me comments. This dice is lopsided. Some of the lower numbers are more likely than the higher numbers. But better yet go back and implement it using the modern C++ version.

Code Review

Classes

Your class are all public:

// player class        // Still think this comment is redundant.
class Player {
    public:
    // All declarations.
};

There are a lot of similarities between a Player and an Enemy. You should probably pull all the same code into a common class so that you are not repeating code in two places.

Reducing Code

You fixed up string enemyContxt(int option) nicely. You should probably move the rand number dye roll down close to the point it was used.

But you can apply the same solution to string playerContxt(Player &player).

The functions fightEnemy(Player &player, Enemy &enemy) and fightPlayer(Player &player1, Player &player2) look very similar. Is there no way this code can be generalized so that it works the same if you are fighting an enemy or another player?

The functions enemyComboCheck(int id1, int id2, vector<Player> &allPlayers) and playerComboCheck(int id1, int id2, vector<Player> &allPlayers) also look very similar. Again can you not generalize these function into a single function?

In fact I am pretty sure that if you have a common base class for Person and Enemy then all the above functions will very quickly simplify themselves.

class Combatant
{
  public:
    bool isAlive();
    void attack(Comatant& other);
    virtual std::string context() = 0;
    virtual void        print(std::ostream& out = std::cout) = 0;
    friend std::ostream& operator<<(std::ostream& str, Comatant const& value)
    {
        value.print(str);
    }
};
class Player: public Combatant
{
  public:
    virtual std::string context() override;
    virtual void        print(std::ostream& out = std::cout) override;
};
class Enemy: public Combatant
{
  public:
    virtual std::string context() override;
    virtual void        print(std::ostream& out = std::cout) override;
};

void fight(Combatant& p1, Combatant& p2)
{
    std::cout << p1 << " Vs " << p2 << "\n";
    std::cout << p1.context() << "\n";
              << p2.context() << "\n";
    while(p1.isAlive() && p2.isAlive()) {
       p1.attack(p2);
       p2.attack(p1);
    }
    if (p1.isAlive() || p2.isAlive()) {
        Combatant& winer = p1.isAlive() ? p1 : p2;

        std::cout << "The Winner: " << winner << "\n";
    }
    else {
        std::cout << "Both combatants killed each other simultaneously\n");
}

Don't encode data into code.

// player declaration
allPlayers.push_back(Player("Peter", "Scribe", 3, 6, 1));
allPlayers.push_back(Player("Cole", "Cursed", 3, 3, 2));

// Enemy declaration below
allEnemies.push_back(Enemy("Amatuer Guard", 10, 0.05, 4, 1));
allEnemies.push_back(Enemy("Mediocre Guard", 15, 0.1, 8, 2));

For the players and enemies you are hard-coding them into the code. If you want to update your simulator with new players or enemies you need to update the code rebuild and redistribute the executable.

If on the other hand you put this data into configuration file and load this dynamically at run time then you can easily upgrade (or sell expansions) just be sending out updated configuration files.

vector<Player> allPlayers(getPlayers());
vector<Enemy>  allEnemies(getEnemies());

Then you can write these as:

std::vector<Player> getPlayers()
{
    std::ifstream                 playerFile("Player.cfg");
    std::istream_iterator<Player> playerIter(playerFile);
    std::istream_iterator<Player> end;

    return std::vector<Player>(playerIter, end);
}

Note: The above code relies on you writting the input operator for player.

std::istream& operator>>(std::istream& str, Player& player)
{
    std::string line;
    if (std::getline(str, line)) {
       std::stringstream lineStream(line);

       std::string name;
       std::string classType;
       int         strength;
       int         endurance;
       int         id;

       if (lineStream >> name >> classType >> strength >> endurance >> id) {
           // Set up the player object.
           player = Player(name, clasType, strength, endurance, id);
       }
       else {
           // Error when reading player characteristics from line.
           // Need to set the errror state of the stream.
           str.setstate(std::ios::failbit);
       }
    }
    return str;
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.