8
\$\begingroup\$

I am coding a command line based game with 1 player and 2 monsters.

Monster attacks player at specified intervals and player takes input from standard input to attack specific monster.

  1. I would like your feedback on how I can improve class relationships.

  2. I feel there is strong cohesiveness between monster and player classes. How can we reduce it?

  3. Considering this is a multithreaded application, if I make health parameter as atomic it starts complaining that particular Copy Constructor is implicitly deleted. I understand atomic cannot be copied, so should we write copy and move ctor explicitly?

  4. Same is the case if std::thread is a member variable.

Character.h

#ifndef CHARACTER_H_
#define CHARACTER_H_

#include <string>

class Character
{
    std::string name_;
    int health_;
    int attackPower_;

public:
    Character(const std::string& name, int health, int attackPower);
    virtual ~Character();

    virtual void attackOpponent(Character& opponent);

    bool isAlive() const;

    const std::string& getName() const;
    int getHealth() const;
};

#endif //CHARACTER_H_

Character.cpp

#include <iostream>
#include <mutex>

#include "Character.h"

std::mutex mtx;

Character::Character(const std::string& name, int health, int attackPower)
    : name_ {name}
    , health_ {health}
    , attackPower_ {attackPower}
{
}

Character::~Character()
{
}

void Character::attackOpponent(Character& opponent)
{
    std::lock_guard<std::mutex> lock(mtx);
    opponent.health_ = std::max(0, opponent.health_ - attackPower_);
    std::cout << name_ << " hits " << opponent.getName() << ". "; 
    std::cout << opponent.getName() << " health is: " << opponent.getHealth() << std::endl;
}

bool Character::isAlive() const
{
    return health_ > 0;
}

const std::string& Character::getName() const
{
    return name_;
}

int Character::getHealth() const
{
    return health_;
}

Enemy.h

#ifndef ENEMY_H_
#define ENEMY_H_

#include "Character.h"

class Enemy : public Character 
{
    int interval_;

public:
    Enemy(const std::string& name, int initialHealth, int attackPower, int interval);
    ~Enemy();
    virtual void attackOpponent(Character& opponent) override;
};

#endif //ENEMY_H_

Enemy.cpp

#include <thread>
#include <chrono>

#include "Enemy.h"

Enemy::Enemy(const std::string& name, int initialHealth, int attackPower, int interval)
    : Character(name, initialHealth, attackPower)
    , interval_ {interval}
{
}

Enemy::~Enemy()
{
}

void Enemy::attackOpponent(Character& opponent)
{
    std::thread t([&]() {
        while (true)
        {
            std::this_thread::sleep_for(std::chrono::milliseconds(interval_));
            if (isAlive() && opponent.isAlive())
            {
                Character::attackOpponent(opponent);
            }
            else
            {
                break;
            }  
        }
    });
    t.detach();
}

Dragon.h

#ifndef DRAGON_H_
#define DRAGON_H_

#include "Enemy.h"

class Dragon : public Enemy
{
public:
    Dragon(int initialHealth, int attackPower, int interval);
    ~Dragon();
};

#endif //DRAGON_H_

Dragon.cpp

#include "Dragon.h"

Dragon::Dragon(int initialHealth, int attackPower, int interval)
    : Enemy("Dragon", initialHealth, attackPower, interval)
{
}

Dragon::~Dragon()
{
}

Player.h

#ifndef PLAYER_H_
#define PLAYER_H_

#include <string>

#include "Character.h"

class Player : public Character
{
public:
    Player(int initialHealth, int attackPower);
    ~Player();
};

#endif //PLAYER_H_

Player.cpp

#include "Player.h"

Player::Player(int initialHealth, int attackPower)
    : Character("Player", initialHealth, attackPower)
{
}

Player::~Player()
{
}

Game.h

#ifndef GAME_H_
#define GAME_H_

#include <vector>
#include <string>
#include <cassert>
#include <algorithm>

#include "Enemy.h"
#include "Character.h"

class Character;
class Enemy;

class Game
{
    std::vector<Character> players_;
    std::vector<Enemy> enemies_;

public:
    Game();
    ~Game();

    void init();
    void play();
    void startEnemyAttack();
    void printScoreCard();

    inline Character& getEnemyByName(const std::string& name)
    {
        auto it = std::find_if(std::begin(enemies_), std::end(enemies_), 
                                [&](auto& o) { return !o.getName().compare(name);});

        assert(it != std::end(enemies_) && "Enemy with matching name not found");

        return *it;
    }
    std::vector<Character>& getPlayers();
    std::vector<Enemy>& getEnemies();
};

#endif //GAME_H_

Game.cpp

#include <iostream>
#include <algorithm>
#include <cassert>

#include "Game.h"
#include "Orc.h"
#include "Player.h"
#include "Dragon.h"


Game::Game()
{
}

Game::~Game()
{
}

void Game::init()
{
    players_.push_back(Player(40, 2));
    enemies_.push_back(Orc(7, 1, 1300));
    enemies_.push_back(Dragon(20, 3, 2600));
}

void Game::play()
{
    startEnemyAttack();

    auto player = std::begin(players_);
    while ((player != std::end(players_)) && player->isAlive())
    {
        if (std::none_of(std::begin(enemies_), std::end(enemies_), 
                [](const auto& o){ return o.isAlive(); }))
        {
            std::cout << "Player wins" << std::endl;
            break;
        }

        std::cout << "Attack enemy:\n1. Orc\n2. Dragon" << std::endl;
        std::cout << "Please choose enemy to be attacked: ";

        int choice = 0;
        std::cin >> choice;

        switch (choice)
        {
            case 1:
                player->attackOpponent(getEnemyByName("Orc"));
                break;
            case 2:
                player->attackOpponent(getEnemyByName("Dragon"));
                break;
            default:
                std::cout << "Wrong option selecetd." << std::endl;
        }
    }

    if ((player != std::end(players_)) && !player->isAlive())
    {
        std::cout << "Player lost" << std::endl;
    }
}

void Game::startEnemyAttack()
{
    auto player = std::begin(players_);
    if (player == std::end(players_))
    {
        return;
    }

    for (auto& e : enemies_)
    {
        e.attackOpponent(*player);
    }
}

std::vector<Character>& Game::getPlayers()
{
    return players_;
}

std::vector<Enemy>& Game::getEnemies()
{
    return enemies_;
}

void Game::printScoreCard()
{
    for (auto& player : players_)
    {
        std::cout << player.getName() << " score is: " << player.getHealth() << std::endl;
    }

    for (auto& enemy : enemies_)
    {
        std::cout << enemy.getName() << " score is: " << enemy.getHealth() << std::endl;
    }
}
\$\endgroup\$
2
\$\begingroup\$

Some observations:

Character::Character(const std::string& name, int health, int attackPower)
    : name_ {name}
    , health_ {health}
    , attackPower_ {attackPower}
{
}

Consider using std::string name to enable move semantics:

Character::Character(std::string name, int health, int attackPower)
    : name_ {std::move(name)}
    , health_ {health}
    , attackPower_ {attackPower}
{
}
Character::~Character()
{
}

This should really be defined directly in-class.

const std::string& Character::getName() const
{
    return name_;
}

int Character::getHealth() const
{
    return health_;
}

Just call them name and health.

Enemy::~Enemy()
{
}

You don't need to explicitly override virtual destructors in derived classes.

void Enemy::attackOpponent(Character& opponent)
{
    std::thread t([&]() {
        while (true)
        {
            std::this_thread::sleep_for(std::chrono::milliseconds(interval_));
            if (isAlive() && opponent.isAlive())
            {
                Character::attackOpponent(opponent);
            }
            else
            {
                break;
            }  
        }
    });
    t.detach();
}

You're introducing data race here. There is no synchronization mechanism.

You are completely changing the semantics of Character::attackOpponent here, so overriding is inappropriate. Leave attackOpponent as-is and rename this to something like start_attack. interval should be stored as a std::chrono::duration to begin with, for clarity. The detach is also prone to lifetime problems.

class Dragon : public Enemy
{
public:
    Dragon(int initialHealth, int attackPower, int interval);
    ~Dragon();
};

class Player : public Character
{
public:
    Player(int initialHealth, int attackPower);
    ~Player();
};

Are these classes really necessary?

class Character;
class Enemy;

These forward declaration are redundant because you have already included the definitions of the classes.

inline Character& getEnemyByName(const std::string& name)
{
    auto it = std::find_if(std::begin(enemies_), std::end(enemies_), 
                            [&](auto& o) { return !o.getName().compare(name);});

    assert(it != std::end(enemies_) && "Enemy with matching name not found");

    return *it;
}

In-class definitions are already inline. Missing const. Use the == operator instead of directly calling the compare function:

auto it = std::find_if(
    enemies_.begin(), enemies_.end(),
    [&] (const auto& o) {
        return o.getName() == name;
    }
);
Game::Game()
{
}

Game::~Game()
{
}

void Game::init()
{
    players_.push_back(Player(40, 2));
    enemies_.push_back(Orc(7, 1, 1300));
    enemies_.push_back(Dragon(20, 3, 2600));
}

Remove the destructor. The init function is probably what you should be doing in the constructor:

Game::Game()
    : players_{Player{40, 2}}
    , enemies_{Orc{7, 1, 1300}, Dragon{20, 3, 2600}}
{
}
auto player = std::begin(players_);
while ((player != std::end(players_)) && player->isAlive())

player doesn't change, so ...

\$\endgroup\$
1
  • \$\begingroup\$ I understand Dragon, Orc, Player classes are not necessary. I am having difficulty in providing synchronization. If I introduce start_attack function, it will take opponent object and invoke attack_opponent in Character class. I guess start_attack will be present in Character as well? How should I use mutex or any other exclusion technique on health of individual characters? When I join on threads it blocks until Dragon kills player or himself gets killed, so I used detach. Is there a way to redesign this and avoid detach? \$\endgroup\$
    – Daemon
    May 9 '20 at 13:37
2
\$\begingroup\$

Here are some things that may help you improve your program. The other review gives some good suggestions about the code itself, so this review will mostly focus on fundamental design issues.

Think carefully about threads

How many threads are really needed here? At the moment, the program launches one thread per enemy which seems rather extravagant. I think it's more likely that all enemies could operate on a single thread. One way to do that would be to push enemy objects (or std::shared_ptrs to them) onto a std::priority_queue with their calculated deadline time as the ordering such that the shortest remaining duration is always at the front of the queue. Generically, this is called Earliest Deadline First (EDF) scheduling.

Think carefully about classes

At the moment, there is a base Character class. Then a Player class that derives from that, and also an Enemy class which is further derived as Orc and Dragon classes. However, there really isn't anything special about any of them in that they operate 99% the same way. I'd suggest instead that you have one underlying Character and then derive a computer-controlled character from that. They could be either enemies or players. Differentiate them via data members rather than by class types. It will lead to a much cleaner design and better flexibility as well.

Think of the user

The code somewhat optimistically contains a std::vector<Character> (emphasizing that the Player object is probably useless as mentioned above) but how will multiple players actually play? Do you anticipate having five human players sharing a single keyboard? The answer to this is important because it helps to determine how threading might be done. If the answer is that there's really only one human-controlled player, then it's quite simple. However, if there are really intended to be more than one player sharing the keyboard, you will almost certainly need to use a mechanism other than std::cin >> choice to get input.

Avoid object slicing

Right now, the code contains a std::vector<Enemy>. However, it is populated with further derived Dragon and Orc classes. In this case, as mentioned immediately above, the classes are functionally identical so it doesn't cause a problem but if the classes were somehow different, we could easily encounter all of the problems of object slicing. See this answer for how to use polymorphism and std::unique_ptr to preserve derived object behavior.

A worked example

See Multithreaded console-based monster battle with earliest-deadline-first scheduler for a worked example.

\$\endgroup\$
0
1
\$\begingroup\$

I'm not that great with threads, parallel programming so I won't comment about them... However, as for design decisions; I've taken your code and wrote my own version. I wouldn't say that the code that I'm about to present to you as an example is 100% bug proof. It does currently run and compile producing some interesting results! This has more to do with class hierarchies, the relationships between them, and their differences. I used a CRTP structure. I gave the Character classes the ability to assign function pointers for attack and defending which can be created by the user to be able to customize how they want a specific character or enemy type to perform an attack or to defend. I did not program any "game logic" but I have randomly generated a player where the user can choose between 3 types (no need to inherit here from the player class), and to randomly generate 3 different kinds of enemies (which are inherited)... Here's my version of the code that you can use for a reference and build off of. As for writing the game logic (that's for you to build and design), as for threading and parallel programming, it's beyond what I can explain. I can write some code that does it, but explaining it is not my strong suit! The code is quite long so I split it into 2 files due to some of the overloaded operators that I had to write for easier printing. Make sure to read the comments in the code too... it shows where I had made some design decisions, and where I modified the input values.

main.cpp

#include "Game.h"

int main() {
    try {
        Game game;          
        game.run();           
    }
    catch (const std::exception& e) {
        std::cerr << e.what() << std::endl;
        return EXIT_FAILURE;
    }
    return EXIT_SUCCESS;
}

Here's a potential output since the number of enemies is randomly generated... Output

Welcome to Dungeon Raid!
Choose your character type
1 - Human:
2 - Dwarven:
3 - Elven:
3

Main Player Info:
Elven
level:45
health: 5493.489746
attack: 919.298096
defense: 970.807129
response: 91.92981
delay: 97.08071
recover: 746.64215

Orc_000
level:71
health: 9015.84277
attack: 2291.32764
defense: 2364.90454
response: 229.13277
delay: 236.49046
recover: 1225.37927

Orc_001
level:58
health: 7145.38623
attack: 1581.78455
defense: 1630.08899
response: 158.17845
delay: 163.00890
recover: 971.15802

Orc_002
level:36
health: 5356.60059
attack: 723.04858
defense: 653.49048
response: 72.30486
delay: 65.34905
recover: 728.03699

Troll_000
level:29
health: 4248.76318
attack: 458.06143
defense: 453.84021
response: 45.80614
delay: 45.38402
recover: 577.46637

Troll_001
level:92
health: 13566.97852
attack: 4404.49219
defense: 4765.45508
response: 440.44922
delay: 476.54550
recover: 1843.94226

Orc_003
level:74
health: 9432.72852
attack: 2752.44165
defense: 2504.81201
response: 275.24417
delay: 250.48120
recover: 1282.03979

Orc_004
level:29
health: 4301.33301
attack: 426.52374
defense: 492.74667
response: 42.65237
delay: 49.27467
recover: 584.61139

Troll_002
level:100
health: 14677.85352
attack: 5369.20947
defense: 5856.85938
response: 536.92096
delay: 585.68597
recover: 1994.92578

Troll_003
level:47
health: 6805.82422
attack: 1253.68689
defense: 1255.42249
response: 125.36869
delay: 125.54225
recover: 925.00677

...and now for the actual classes to make it all work.

Game.h

#pragma once

// error handling
#include <exception>

// numerics, algorithms, properties, limits
#include <algorithm>
#include <cstdint>
#include <limits>
#include <numeric>
#include <random>
#include <type_traits>

// string and stream libraries
#include <fstream>
#include <iostream>
#include <iomanip>
#include <sstream>
#include <string>

// containers
#include <array>
//#include <vector>
#include <concurrent_vector.h>
#include <concurrent_priority_queue.h>


// memory, threads, etc.
#include <memory>
#include <mutex>
#include <thread>

// When adding a new type: must update operators
enum class PlayerType {
    HUMAN = 1,
    DWARVEN,
    ELVEN
};
std::istream& operator>>(std::istream& in, PlayerType& type);
std::ostream& operator<<(std::ostream& in, const PlayerType& type);

// When adding a new type: must update operators
enum class EnemyType {
    GOBLIN = 1,
    ORC,
    TROLL
};
std::istream& operator>>(std::istream& in, EnemyType& type);
std::ostream& operator<<(std::ostream& out, const EnemyType& type);

struct CharacterInfo {
    std::uint32_t level_;          
    float health_;          
    float attack_power_;   
    float defense_;        

    float time_response_;          // range [0,1]  // same as speed or how fast they can attack
    float time_delay_;             // range [0,1]  // time delay before next attack
    float recovery_rate_;          // range [0,1]  // how fast they can recover, regain health, etc...

    CharacterInfo();
    CharacterInfo(std::uint32_t level, float health, float attackPower, float defense,
        float timeResponse, float timeDelay, float recoverRate);
   
    CharacterInfo(const CharacterInfo& other);
    CharacterInfo& operator=(const CharacterInfo& other);

};
std::istream& operator>>(std::istream& in, CharacterInfo& info);
std::ostream& operator<<(std::ostream& out, const CharacterInfo& info);

class Character;
typedef float(*AttackFunc)(Character* attacker, Character* defender, float time_response);
typedef void(*DefendOrBlockFunc)(Character* defender, Character* attacker, float time_response);

template<class T>
class EntityID {
protected:
    explicit EntityID(std::string& id) {
        static int i = 0;
        std::stringstream strValue;
        strValue << std::setw(3) << std::setfill('0') << std::to_string(i++);
        id.append("_" + strValue.str());
    }
    virtual ~EntityID() = default;
};

class Character {
protected:
    std::string id_ = "";
    AttackFunc attack_;
    CharacterInfo info_;
    DefendOrBlockFunc defend_;

    explicit Character(CharacterInfo& info, const std::string& id = "")
        : info_{ info }, id_{std::move(id)} {}

public:
    virtual ~Character() = default;
    Character* operator()() { return this; }
    CharacterInfo* info() { return &info_; }

    std::string& id() { return id_; }
    float health() const { return info_.health_; }
    float attackPower() const { return info_.attack_power_; }
    float defense() const { return info_.defense_; }
    float response() const { return info_.time_response_; }
    float delay() const { return info_.time_delay_; }
    float recovery() const { return info_.recovery_rate_; }

    void assignAttack(AttackFunc func) { attack_ = func; }
    void assignBlock(DefendOrBlockFunc func) { defend_ = func; }

    inline virtual void healthModifier(const Character& other) {
        auto power = other.attackPower();
        this->info_.health_ -= (power - this->info_.defense_);
    }

    float attack(Character* defender, float time_response) { return attack_(this, defender, time_response); }
    void defend(Character* attacker, float time_response) { return defend_(this, attacker, time_response); }
 };

template<typename Ty>
const std::string nameOfCharacterType(Ty type) {
    std::stringstream name;
    name << type;
    return name.str();
}

class Player final : public Character, EntityID<Player> {
    PlayerType player_type_;
public:
    Player(CharacterInfo& info, PlayerType type, const std::string& name = "Player" )
        : Character(info, std::move(name)), EntityID(id()), player_type_{ type } 
    {}
    virtual ~Player() = default;
    
    Player* operator()() { return this; }
    
    inline virtual void healthModifier(const Character& other) override {
        // modify as desired... leave blank for default
    }
    
    PlayerType type() const { return player_type_; }
};

class Enemy : public Character {
    EnemyType enemy_type_;
protected:
    Enemy(CharacterInfo& info,EnemyType type, const std::string& name = "Enemy")
        : Character(info, std::move(name)), enemy_type_{ type } {}
  
public:
    virtual ~Enemy() = default;
    inline virtual void healthModifier(const Character& other) override {
        // modify as desired... leave blank for default
    }  
    Enemy* operator()() { return this; }
    EnemyType type() const { return enemy_type_; }
};

class Goblin final : public Enemy, EntityID<Goblin> { // remove final if you want to derive from Goblin
public:
    Goblin(CharacterInfo& info, EnemyType type, const std::string& name = "Goblin")
        : Enemy(info, type, std::move(name)), EntityID(id()) {}
    virtual ~Goblin() = default;
    Goblin* operator()() { return this; }
    virtual void healthModifier(const Character& other) override {
        // modify as desired... leave blank for default
    }
};

class Orc final : public Enemy, EntityID<Orc> { // remove final if you want to derive from Orc
public:
    Orc(CharacterInfo& info, EnemyType type, const std::string& name = "Orc")
        : Enemy(info, type, std::move(name)), EntityID(id()) {}
    virtual ~Orc() = default;
    Orc* operator()() { return this; }
    virtual void healthModifier(const Character& other) override {
        // modify as desired... leave blank for default
    }
};

class Troll final : public Enemy, EntityID<Troll> { // remove final if you want to derive from Troll
public:
    Troll(CharacterInfo& info, EnemyType type, const std::string& name = "Troll")
        : Enemy(info, type, std::move(name)), EntityID(id()) {}
    virtual ~Troll() = default;
    Troll* operator()() { return this; }
    virtual void healthModifier(const Character& other) override {
        // modify as desired... leave blank for default
    }
};

std::ostream& operator<<(std::ostream& out, Player& player);
std::ostream& operator<<(std::ostream& out, Orc& orc);
std::ostream& operator<<(std::ostream& out, Troll& troll);

// define your attack and defense functions for the function pointers here! Or create a lambda within the initialize function.

class Game {
    std::unique_ptr<Player> player_;
    std::vector<std::unique_ptr<Enemy>> enemies_;

    std::random_device rd;
    std::mt19937 gen{ rd() };

public:
    Game() {
        initialize();
    }

    void initialize() {
        std::cout << "Welcome to Dungeon Raid!\n";
        createPlayer();
        generateRandomEnemies();

        // here is where you would assign the attack and defence function pointers:
        // player_->assignAttack();
        // player_->assignBlock();

        /*for (auto& e : enemies_) {
            e->assignAttack();
            e->assignBlock();
        }*/

    }

    void run() {
        // main loop, user input, game logic here
        
        // for testing purposes, let's just print out our player and enemy info as lists:
        std::cout << "\nMain Player Info:\n" << player_->type() << '\n' << *player_->info() << '\n';

        for (auto& e : enemies_) {
            std::cout << e->id() << '\n' << *e->info() << std::endl;
        }

    }

private:
    void createPlayer() {
        PlayerType type;
        CharacterInfo playerInfo;
  
        retrievePlayerInfo(type, playerInfo);
        this->player_.reset(new Player{ playerInfo, type, nameOfCharacterType(type) });
    }

    void showPlayerChoice(PlayerType& type) {
        std::cout << "Choose your character type\n";
        std::cout << "1 - Human:\n"
            << "2 - Dwarven:\n"
            << "3 - Elven:\n";
        std::string str;
        std::getline(std::con, str);
        std::uint32_t val = std::stoi(str); // can improve this to fix invalid input types...
        type = static_cast<PlayerType>(val);
    }

    void retrievePlayerInfo(PlayerType& type, CharacterInfo& playerInfo) {
        bool properType = false;

        do {
            if (!properType)
                showPlayerChoice(type);

            switch (type) {
                case PlayerType::HUMAN: {
                    playerInfo = generateRandomStats(); // CharacterInfo{ 1, 10, 4, 3, 0.7f, 0.5f, 0.4f };
                    properType = true; break;
                }
                case PlayerType::DWARVEN: {
                    playerInfo = generateRandomStats(); // CharacterInfo{ 1, 12, 4, 4, 0.5f, 0.3f, 0.6f };
                    properType = true; break;
                }
                case PlayerType::ELVEN: {
                    playerInfo = generateRandomStats(); // CharacterInfo{ 1, 9, 3, 3, 0.8f, 0.2f, 0.7f };
                    properType = false; break;
                }
                default: {
                    properType = false; break;
                }
            }
        } while (!properType);
    }

    void generateRandomEnemies() {
        std::uniform_int_distribution<std::uint32_t> enemyCountDist{ 5, 20 }; // 5 to 20 enemies
        std::uint32_t enemyCount = enemyCountDist(gen);
        enemies_.resize(enemyCount);

        std::uniform_int_distribution<std::uint32_t> enemyTypeDist{ 1, 3 }; // 1 = Goblin, 2 = Orc, 3 = Troll
        
        EnemyType type;
        CharacterInfo enemyInfo;
        for (unsigned i = 0; i < enemyCount; i++) {
            type = static_cast<EnemyType>( enemyTypeDist(gen) );

            switch (type) {
                case EnemyType::GOBLIN: {
                    enemyInfo = generateRandomStats(); // CharacterInfo{ 1, 5, 2, 3, 0.9f, 0.2f, 0.9f };
                    this->enemies_[i].reset(new Goblin{ enemyInfo, type });
                }
                case EnemyType::ORC: {
                    enemyInfo = generateRandomStats(); // CharacterInfo{ 1, 7, 5, 8, 0.3f, 0.4f, 0.6f };
                    this->enemies_[i].reset(new Orc{ enemyInfo, type });
                    break;
                }
                case EnemyType::TROLL: {
                    enemyInfo = generateRandomStats(); // CharacterInfo{ 1, 14, 5, 8, 0.3f, 0.4f, 0.6f };
                    this->enemies_[i].reset(new Troll{ enemyInfo, type });
                    break;
                }
            }            
        }
    }

    CharacterInfo generateRandomStats() {
        // Generate a Random level in the range of [1,20] for the player
        std::uniform_int_distribution<std::uint32_t> randomLevelDist(1, 100);
        std::uint32_t randomLevel = randomLevelDist(gen);

        // Character states will be based on the curve of the level
        // Generate Random Stats: Level 1 base health = 100
        // Health Range = ((Base Health * Multiplyer) + (Base Health * Level)) / BaseHealth         
        const float baseHealth = 10.0f;        
        auto baseMinMultiplyer = 1.2f;
        auto baseMaxMultiplyer = 1.5f;
        auto baseLevelHealth = (baseHealth * randomLevel);   

        auto lowerRange = baseHealth * baseMinMultiplyer * baseLevelHealth;
        auto upperRange = baseHealth * baseMaxMultiplyer * baseLevelHealth;
        std::uniform_real_distribution<float> dist(lowerRange, upperRange);
        auto randomHealth = dist(gen);

        // Attack & Defense Range = 50% of health    
        auto healthPercentage = randomHealth * 0.5f;
        lowerRange /= randomLevel;
        upperRange /= randomLevel;
      
        std::uniform_real_distribution<float> randomAttackDefenceDist(healthPercentage / upperRange, healthPercentage / lowerRange);  
        auto randomAttack = randomAttackDefenceDist(gen) * randomLevel;
        auto randomDefense = randomAttackDefenceDist(gen) * randomLevel;

        // Time Response and Delay is based off of attack and defense where recovery is based off of health
        auto randomResponse = randomAttack * 0.1f;
        auto randomDelay = randomDefense * 0.1f;
        auto randomRecovery = randomHealth * 0.271828f * 0.5f;  // 0.271828 approximate e/10

        // Create our Info
        return CharacterInfo{ randomLevel, randomHealth, randomAttack, randomDefense, randomResponse, randomDelay, randomRecovery };
    }
};

Game.cpp

#include "Game.h"

std::istream& operator>>(std::istream& in, PlayerType& type) {
    std::uint32_t val{ 0 };
    in >> val;
    type = static_cast<PlayerType>(val);
    return in;
}

std::ostream& operator<<(std::ostream& out, const PlayerType& type) {
    std::string str;
    switch (type) {
        case PlayerType::HUMAN:
            str = "Human";
            break;
        case PlayerType::DWARVEN:
            str = "Dwarven";
            break;
        case PlayerType::ELVEN:
            str = "Elven";
            break;
        default:
            str = "Unknown";
            break;
        }
    return out << str;
}

std::istream& operator>>(std::istream& in, EnemyType& type) {
    std::uint32_t val{ 0 };
    in >> val;
    type = static_cast<EnemyType>(type);
    return in;
}

std::ostream& operator<<(std::ostream& out, const EnemyType& type) {
    std::string str;
    switch (type) {
        case EnemyType::GOBLIN:
            str = "Goblin";
            break;
        case EnemyType::ORC:
            str = "Orc";
            break;
        case EnemyType::TROLL:
            str = "Troll";
            break;
        default:
            str = "Unknown";
            break;
        }
    return out;
}

CharacterInfo::CharacterInfo() :    
    level_{ 0 },
    health_{ 0 },
    attack_power_{ 0 },
    defense_{ 0 },
    time_response_{ 0.0f },
    time_delay_{ 0.0f },
    recovery_rate_{ 0.0f }
{}

CharacterInfo::CharacterInfo( std::uint32_t level, float health, 
                              float attackPower, float defense, 
                              float timeResponse, float timeDelay, float recoveryRate) :
    level_{level},
    health_{health},
    attack_power_{attackPower},
    defense_{defense},
    time_response_{timeResponse},
    time_delay_{timeDelay},
    recovery_rate_{recoveryRate}
{}

CharacterInfo::CharacterInfo(const CharacterInfo& other) {
    this->level_ = other.level_;
    this->health_ = other.health_;
    this->attack_power_ = other.attack_power_;
    this->defense_ = other.defense_;
    this->time_response_ = other.time_response_;
    this->time_delay_ = other.time_delay_;
    this->recovery_rate_ = other.recovery_rate_;
}

CharacterInfo& CharacterInfo::operator=(const CharacterInfo& other) {
    this->level_ = other.level_;
    this->health_ = other.health_;
    this->attack_power_ = other.attack_power_;
    this->defense_ = other.defense_;
    this->time_response_ = other.time_response_;
    this->time_delay_ = other.time_delay_;
    this->recovery_rate_ = other.recovery_rate_;
    return *this;
}

std::istream& operator>>(std::istream& in, CharacterInfo& info) {
    in >> info.level_;
    in >> info.health_;
    in >> info.attack_power_;
    in >> info.defense_;
    in >> info.time_response_;
    in >> info.time_delay_;
    in >> info.recovery_rate_;
    return in;
}

std::ostream& operator<<(std::ostream& out, const CharacterInfo& info) {        
    out << "level:" << info.level_ << '\n';
    out.setf(std::ios::floatfield, std::ios::fixed);
    out << "health: " << std::setw(3) << std::setfill('0') << info.health_ << '\n'
        << "attack: " << std::setw(3) << std::setfill('0') << info.attack_power_ << '\n'
        << "defense: " << std::setw(3) << std::setfill('0') << info.defense_ << '\n'
        << "response: " << std::setprecision(5) << std::setfill('0') << info.time_response_ << '\n'
        << "delay: " << std::setprecision(5) << std::setfill('0') << info.time_delay_ << '\n'
        << "recover: " << std::setprecision(5) << std::setfill('0') << info.recovery_rate_ << std::endl;
    return out;
}

std::ostream& operator<<(std::ostream& out, Player& player) {
    out << "ID: " << player.id() << '\n'
        << player.info() << std::endl;
    return out;
}

std::ostream& operator<<(std::ostream& out, Orc& orc) {
    out << "ID: " << orc.id() << '\n'
        << orc.info() << std::endl;
    return out;
}

std::ostream& operator<<(std::ostream& out, Troll& troll) {
    out << "ID: " << troll.id() << '\n'
        << troll.info() << std::endl;
    return out;
}

If you look at the generated output, you can see that Goblin, Orc, Troll are their own class, but through inheritance, they are all enemy objects and enemy objects are a form of a Character. Even the Player is a Character. However, Character and Enemy themselves are abstract types. The CRTP that is used helps in the aide of generating unique IDs for them in the form of an std::string. Every time a new Orc is generated the id value is incremented based on the number of orcs, but this doesn't affect the number id for the Trolls or Goblins, however, we can easily store all of these into a single vector. Using unique_ptr's helps with cleaning up memory. Now it's just a matter of making sure that your threads and access to read/write within your containers are synchronized and concurrent. Intel has a nice threading library, you can just search for tbb as it's a part of their IDEs, but they do have an open-source - free version of the tbb library via apache's licenses for download. At the time of this post, here is their active link: Intel:TBB. This should be able to help you with your threading and parallel programming concerns.

\$\endgroup\$
5
  • \$\begingroup\$ "I've taken your code and wrote my own version." I think you should post this answer as a new review instead. \$\endgroup\$
    – G. Sliepen
    Jul 21 '20 at 12:05
  • \$\begingroup\$ @G. Sliepen I wasn't asking for it to be reviewed. It's not 100% without bugs, it can be optimized and improved for readability, it should be wrapped in a namespace, etc... I'm currently limited to C++17 since I don't have access to a C++20 compiler with its latest features. My intent is centered around the underlying CRTP structure that I used to illustrate my thought process in the design decisions. With C++20, a good 30% of this can be refactored and simplified. This wasn't meant to "critique" all of their concerns. It's specific to the coupling and decoupling of the related classes... \$\endgroup\$ Jul 21 '20 at 12:53
  • \$\begingroup\$ @G.Sliepen Basically I covered points 1 & 2 from the OP's concerns using my example as one way of resolving this issue! The Player and Enemy Type classes are related as in that they are all Characters, however, Player and Enemy are two distinct types... Now, as for a Troll, Golbin, Orc, Dragon, etc... they are each their own types, but they are all enemies. Now, I didn't implement one, but an NPC would be another type that would inherit from Character but would be separate from Player and Enemy or Monster types... The CRTP pattern helps with this shown through the designated IDs. \$\endgroup\$ Jul 21 '20 at 12:59
  • 1
    \$\begingroup\$ Sure, it is a good way of showing an alternative way to implement OP's game. But this answer itself is worthy of review. I see some issues, like no input validation, that could be addressed, which would turn it into an even better answer. \$\endgroup\$
    – G. Sliepen
    Jul 21 '20 at 14:06
  • \$\begingroup\$ @G.Sliepen True. I had worked on that for a few hours just to get the boilerplate framework functioning... I'm sure there are plenty of corners and edge-case issues... I don't think it would be something that I would invest a lot of time in. I do have other projects with a much higher priority... Who knows, maybe one day I might have it reviewed... but as for right now it has to take a back seat... \$\endgroup\$ Jul 21 '20 at 15:29

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.