5
\$\begingroup\$

The ‘skeleton’ of the game works, making it playable, so I’m mainly just looking for renditions and feedback on both the code, and the actual game.

Here’s the things I’m already working to fix/add:

  • Add color to text
  • Add startup menu
  • Find a way to put names in commander fights OR alternatively have player name their cmdr
  • Balance the stats like a lot oh my god
  • Make integer inputs non-integer capable
  • Loop the current player's turn until they input an appropriate number
  • Equipment for commander
  • List current price for each technology

Currently, the game’s files have a combined 562 lines, so to make it a little easy on you, here are the links to the project on the compiler I use, and the Github repository:

Compiler – https://replit.com/@Selisine/CBPR?v=1

Github – https://github.com/aidenpike/CBPR

Here is the actual code: main.cpp

#include <iostream> 
#include "Country.h"

using namespace std;

int main(){
    Player *currentPlayer;
    
    getNames(&p1);
    getNames(&p2);

    while (true) {
        //Player One Turn
        playerTurn(&p1);
        
        //Player Two Turn
        playerTurn(&p2);
    } 
}

Country.h


#include <iostream>
#include <algorithm>
#include <string>
#include "Commander.h"

using std::cout;
using std::string;
using std::cin;
using std::endl;

struct Player {
    string name = " ";
    int territories = 2;
    int armies = 10;
    int armySkill = 0;
    int armyEndurance = 0;
    int weaponComplexity = 0;
    int passiveIncome = 0; 
    int money = 49900;
    bool weaponFailure = false;
    int player = 0;
} p1, p2;

Player *currentPlayer;

//Rolls, chances, and lists
//Dice Roll
int battleRoll(int dice){
    srand(int(time(0)));
    
    return rand()%dice + 1;
}

void getNames(Player *currentPlayer){
    cout << "Player, enter your name: ";
    getline(cin, currentPlayer->name);

    if (currentPlayer->name == "Oasis"){
        cout << "Welcome back, old friend.\n";
        currentPlayer->armySkill += 100;
        currentPlayer->weaponComplexity += 10;
        currentPlayer->passiveIncome += 1000;
        currentPlayer->armies += 100;
    }
}

//Comma Seperation Math (100% not stolen)
string commaSeperate(int num){
    auto src = std::to_string(num);
    auto dest = std::string();
    auto count = 3;
    
    for(auto i = src.crbegin() ; i != src.crend() ; ++i) {
        if (count == 0)
        {
            dest.push_back(',');
            count = 3;
        }
        if (count--) {
            dest.push_back(*i);
        }
    }
    reverse(dest.begin(), dest.end());

    return dest;   
}

//Tech List
void techList(){
    cout << "[A] Army Skill\n";
    cout << "[B] Army Endurance\n";
    cout << "[C] Weapon Upgrade\n";
    cout << "[D] Level Up Commander\n";
    cout << "[H] Passive Income\n";
}

//Stat List
void listStats(Player *currentPlayer){
    string passiveIncome = commaSeperate(currentPlayer->passiveIncome * 50 + 100);
    string money = commaSeperate(currentPlayer->money);
    
    cout << "It is currently " << currentPlayer->name << "'s turn. Here are their stats:\n";
    cout << "\t      \x1B[33m"         << currentPlayer->territories              << "\033[0m Territories\n"; //Yellow
    cout << "\t      \x1B[30m"         << currentPlayer->armies                   << "\033[0m Armies\n"; //Light gray maybe 
    cout << "\t\x1B[34mLevel "         << currentPlayer->armySkill                << "\033[0m Army Skill\n"; //Blue
    cout << "\t\x1B[31mLevel "         << currentPlayer->armyEndurance            << "\033[0m Army Endurance\n"; //Red 
    cout << "\t\x1B[35mLevel "         << currentPlayer->weaponComplexity         << "\033[0m Weapon Complexity\n"; //Purple 
    cout << "\t     \x1B[32m$"         << passiveIncome                           << "\033[0m Dollars in passive income\n"; //Green
    cout << "\t     \x1B[32m$"         << money                                   << "\033[0m Dollars\n\n\n"; //Green 
}

//Weapon Failure
void weaponFailure(Player *currentPlayer){
    int weaponFail =  battleRoll(20) + 1 * currentPlayer->weaponComplexity;
    if ((weaponFail - currentPlayer->armySkill) <= 5){
        cout << "\x1B[31m" << currentPlayer->name << ", your weapons failed during battle!\033[0m\n";
        switch (rand()%3 + 1){
            case 1: 
                cout << "\x1B[33m" << currentPlayer->name << " loses this battle, and flees!\033[0m\n";
                
                (battleRoll(10) + currentPlayer->armyEndurance <= 5) ? currentPlayer->armies-- : currentPlayer->armies = currentPlayer->armies;
                
                currentPlayer->money -=  rand()%1000 + 500;
                currentPlayer->weaponFailure = true;
            break;

            case 2: 
                cout << "\x1B[32m" << currentPlayer->name << ", you escape by a thread!\033[0m\n";
                currentPlayer->money -= rand()%1000 + 500;
                currentPlayer->weaponFailure = true;
            break;

            case 3:
                cout << "\x1B[31m" << currentPlayer->name << ", the battle was immensely devastating.\033[0m\n";
                currentPlayer->money -= rand()%2000 + 1300;

                (battleRoll(10) + currentPlayer->armyEndurance <= 10) ? currentPlayer->armies -= 2 : currentPlayer->armies = currentPlayer->armies;
                
                currentPlayer->weaponComplexity--;
                currentPlayer->weaponFailure = true;
        }
    }
}

//Expand territory
void expandTerritory(Player *currentPlayer){
    int territoryPrice = 10000;

    if (currentPlayer->money >= territoryPrice){
            cout << currentPlayer->name << " expanded their territory!\n";
            currentPlayer->money -= territoryPrice;
            currentPlayer->territories++;
        }
    else {
        cout << "You don't have enough money!\n\n";
    }
}

//Actual function for tech choices
void upgradeTechnology(Player *currentPlayer){
    p1.player = 1;
    p2.player = 2;
    
    int techPrice = 8000;
    char techBranch = ' ';
    
    cout << "Which technology would you like to upgrade?\n";
    techList();
    cin >> techBranch;

    techBranch = toupper(techBranch);

    switch (techBranch){
        //Army Skill
        case 'A':
            if (techPrice + (800 * currentPlayer->armySkill) > currentPlayer->money){
                cout << "You don't have enough money!\n\n";
            }
            else {
                cout << "After a long day, your armies' training is done. It pays off an they gain one skill level!\n\n";
                currentPlayer->armySkill++; 
                currentPlayer->money -= techPrice + (800 * currentPlayer->armySkill);
            }
        break;

        case 'B':
            if (techPrice + (1000 * currentPlayer->armyEndurance > currentPlayer->money)){
                cout << "Not enough money!\n\n";
            }
            else {
                cout << "Your armies train hard, focusing more towards their endurance. Their endurance gains on level!\n\n";
                currentPlayer->armyEndurance++;
                currentPlayer->money -= techPrice + (1000 * currentPlayer->armyEndurance);
            }
        break;

        case 'C':
            if (2000 + techPrice + (1000 * currentPlayer->weaponComplexity > currentPlayer->money)){
                cout << "Not enough money!\n\n";
            }
            else {
                cout << "Using your ingenious military scientists, you manage to develop a newer, more complex weapon. Your weapon complexity level increases!\n\n";
                currentPlayer->weaponComplexity++;
                currentPlayer->money -= 2000 + techPrice + (1000 * currentPlayer->weaponComplexity);
            }
        break;

        case 'D':
            if (techPrice + (500 * returnLevel(currentPlayer->player)) > currentPlayer->money){
                cout << "Not enough money!\n\n";
            }
            else {
                cmdrLVLRedirect(currentPlayer->player);
                currentPlayer->money -= techPrice + (500 * returnLevel(currentPlayer->player));
            }
        break;

        case 'H':
            if (techPrice + (800 * currentPlayer->passiveIncome) > currentPlayer->money){
                cout << "Not enough money!\n\n";
            }
            else if (currentPlayer->passiveIncome <= currentPlayer->territories){
                cout << "You add more mines to your territories.\n\n";
                currentPlayer->passiveIncome++;
                currentPlayer->money -= techPrice + (800 * currentPlayer->passiveIncome);
            }
            else {
                cout << "Not enough territories to add mines to!\n\n";
                currentPlayer->passiveIncome = currentPlayer->territories;
            }
        }
}

//Recruit an army
void recruitArmy(Player *currentPlayer){
    int armyPrice = 8000;

    if (currentPlayer->money >= armyPrice){
        cout << "A new army is hired.\n\n";
        currentPlayer->armies++;
        currentPlayer->money -= armyPrice;
    }
    else {
        cout << "Not enough money to recruit another army!\n\n";
    }
}

void battleInitiation(){
    int outcomeChoice = 0;
    int pOneATKRoll = battleRoll(8) + battleRoll(8) + p1.armySkill * 2 + p1.weaponComplexity * 3 + p1.armies + cmdr1.cmdrMOR;
    int pTwoATKRoll = battleRoll(8) + battleRoll(8) + p2.armySkill * 2 + p2.weaponComplexity * 3 + p2.armies + cmdr2.cmdrMOR;
    bool cmdrFight = false;


    cout << "What is your goal for battle?\n";
    cout << "1. Devastate\n";
    cout << "2. Conquer\n";
    cin >> outcomeChoice;

    (rand()%2 + 1 == 2) ? cmdrFight = true : cmdrFight = false;

    p1.weaponFailure = false;
    p2.weaponFailure = false;
    
    weaponFailure(&p1);
    weaponFailure(&p2);
    
    switch (outcomeChoice){
        case 1:
            if (p1.weaponFailure && p2.weaponFailure){
                cout << "It's a tie!\n\n";
            
                p1.money -= 5;
                p2.money -= 5;    
                p1.weaponFailure = false;
                p2.weaponFailure = false;
                cmdrFight = false;
            }
            else if (cmdrFight){
                if (cmdrBattle() == "p1win"){
                   cout << p1.name << " has devastated this territory.\n\n";
                    
                    p2.territories--;
                    p2.money -= rand()%750 + 500;
                }
                else {
                    cout << p2.name << " has devastated this territory.\n\n";
                    
                    p1.territories--;
                    p1.money -= rand()%750 + 500;
                }
            }
            else if (pOneATKRoll > pTwoATKRoll || p2.weaponFailure){
                cout << p1.name << " has devastated this territory.\n\n";
                
                p2.territories--;
                p2.money -= rand()%750 + 500;
            }
            else if (pOneATKRoll == pTwoATKRoll){
                cout << "Both players' weapons fail, it's a tie!\n\n";
                
                p1.money -= 5;
                p2.money -= 5;
            }
            else if (pOneATKRoll < pTwoATKRoll || p1.weaponFailure){
                cout << p2.name << " has devastated this territory.\n\n";
                
                p1.territories--;
                p1.money -= rand()%750 + 500;
            }
        break;

        case 2:
            if (p1.weaponFailure && p2.weaponFailure){
                cout << "It's a tie!\n\n";
                
                p1.money -= 5;
                p2.money -= 5;
            }
            else if (cmdrFight){
                if (cmdrBattle() == "p1win"){
                    cout << p1.name << " has conquered this territory.\n\n";
                    
                    p1.territories++;
                    p2.territories--;
                    p1.money += rand()%500 + 250;
                    p2.money -= rand()%500 + 250;
                }
                else {
                    
                    cout << p2.name << " has conquered this territory.\n\n";
                   
                    p2.territories++;
                    p1.territories--;
                    p2.money += rand()%500 + 250;
                    p1.money -= rand()%500 + 250;
                }
            }
            else if (pOneATKRoll > pTwoATKRoll || p2.weaponFailure){
                cout << p1.name << " has conquered this territory.\n\n";
                
                p1.territories++;
                p2.territories--;
                p1.money += rand()%500 + 250;
                p2.money -= rand()%500 + 250;
            }
            else if (pOneATKRoll == pTwoATKRoll || (p1.weaponFailure && p2.weaponFailure)){
                cout << "It's a tie!\n\n";
                p1.money -= 5;
                p2.money -= 5;
            }
            else if (pOneATKRoll < pTwoATKRoll || p1.weaponFailure){
                cout << p2.name << " has conquered this territory.\n\n";
               
                p2.territories++;
                p1.territories--;
                p2.money += rand()%500 + 250;
                p1.money -= rand()%500 + 250;
            }
    }

    p1.weaponFailure = false;
    p2.weaponFailure = false;
}

//Lose Check
void hasLost(){
    if (p1.territories <= 0 && p2.territories <= 0){
        cout << "Both empires go down in flames. The war is a tie.\n\n";
        exit(0);
    }
    
    if (p1.money <= 0 && p2.money <= 0){
        cout << "Both empires go down in flames. The war is a tie.\n\n";
        exit(0);
    }

    if (p1.money <= 0){
        cout << p1.name << " has been devastated by " << p2.name << "!\n\n";
        exit(0);
    }
    else if (p1.territories <= 0){
        cout << p1.name << " has been conquered by " << p2.name << "!\n\n";
        exit(0);
    }
    
    if (p2.money <= 0){
        cout << p2.name << " has been devastated by " << p1.name << "!\n\n";
        exit(0);
    }
    else if (p2.territories <= 0){
        cout << p2.name << " has been conquered by " << p1.name << "!\n\n";
        exit(0);
    }
}

void playerTurn(Player *currentPlayer){
    hasLost();
    
    //Passive Income
    currentPlayer->money += 100 + (50 * currentPlayer->passiveIncome);
    //List stats for current player
    listStats(currentPlayer);
    int choice = 0;
    
    cout << "What do you wish to do?" << endl;
    cout << "\t1. Grow Territory ($10,000)\n";
    cout << "\t2. Upgrade Technology\n";
    cout << "\t3. Recruit Army ($8,000)\n";
    cout << "\t4. Attack\n";
    cout << "\t5. Pass\n\n";

    cin >> choice;
    
    switch (choice){
        case 1:
            expandTerritory(currentPlayer);
        break;
            
        case 2:
            upgradeTechnology(currentPlayer); 
        break;

        case 3:
            recruitArmy(currentPlayer);
        break;

        case 4:            
            battleInitiation();
        break;  
    }
} 

Commander.h


 #include <iostream>

using std::cout;
using std::endl;
using std::string;

struct Commander {
    int cmdrLVL = 1;
    int cmdrATK = 2;
    int cmdrDEF = 5;
    int cmdrMDEF = 5;
    int cmdrHP = 10;
    int cmdrMHP = 10;
    int cmdrMOR = 3;
} cmdr1, cmdr2;

Commander *currentCmdr;

void cmdrLevelUp(Commander *currentCmdr, int player){
    cout << "Player " << player << ", you leveled up your commander!\n";
    currentCmdr->cmdrLVL++;
    currentCmdr->cmdrHP *= 1.2 * currentCmdr->cmdrLVL + 1;
    currentCmdr->cmdrMHP *= 1.2 * currentCmdr->cmdrLVL + 1;
    currentCmdr->cmdrDEF *= 1.2 * currentCmdr->cmdrLVL + 1;
    currentCmdr->cmdrMDEF *= 1.2 * currentCmdr->cmdrLVL + 1;
    currentCmdr->cmdrATK *= 1.1 * currentCmdr->cmdrLVL + 1;
    currentCmdr->cmdrMOR *= 1.05 * currentCmdr->cmdrLVL + 1;
}

void cmdrLVLRedirect(int player){
    (player == 1) ? cmdrLevelUp(&cmdr1, player) : cmdrLevelUp(&cmdr2, player);
}

void cmdrChoiceList(){
    cout << "1. Attack\n";
    cout << "2. Repair Armor\n";
    cout << "3. Forfeit\n";
}

int returnLevel(int player){
    return (player == 1) ? cmdr1.cmdrLVL : cmdr2.cmdrLVL;
}

void cmdrStats(Commander *currentCmdr){
    cout << "[Current Commander Stats]\n";
    cout << "\tHP: "     << currentCmdr->cmdrHP  << "/" << currentCmdr->cmdrMHP  << endl;
    cout << "\tDEF: "    << currentCmdr->cmdrDEF << "/" << currentCmdr->cmdrMDEF << endl;
    cout << "\tATK: "    << currentCmdr->cmdrATK << endl;
    cout << "\tMorale: " << currentCmdr->cmdrMOR << endl;
}

void cmdrResetStats(Commander *currentCmdr){
    currentCmdr->cmdrHP = currentCmdr->cmdrMHP;
    currentCmdr->cmdrDEF = currentCmdr->cmdrMDEF;
}
//One Fight Cycle 
void cmdrFight(Commander *currentCmdr, Commander *opposingCmdr){
    int choice = 0;
    
    if (currentCmdr->cmdrDEF < 0){
        currentCmdr->cmdrDEF = 0;
    }
    
    cmdrStats(currentCmdr);
    cmdrChoiceList();
    std::cin >> choice;

    switch (choice){
        case 1:
            cout << "This commander takes a swift strike at his opponent!\n";

            if (rand()%20 + 1 < 10){
                cout << "He hits!\n";
                if (currentCmdr->cmdrDEF > 0){
                    currentCmdr->cmdrDEF -= opposingCmdr->cmdrATK;
                }
                else if (currentCmdr->cmdrDEF <= 0){
                    currentCmdr->cmdrHP -= opposingCmdr->cmdrATK;
                }
                
            }
            else {
                cout << "He misses!\n";
            }
        break;

        case 2:
            if (cmdr1.cmdrDEF < cmdr1.cmdrMDEF){
                cout << "This commander repairs his armor!\n'";
                currentCmdr->cmdrDEF += (rand()%3 + 1) * currentCmdr->cmdrLVL;
            }
            else if (cmdr1.cmdrDEF >= cmdr1.cmdrMDEF){
                cout << "Armor already at max!\n";
                currentCmdr->cmdrDEF = currentCmdr->cmdrMDEF;
            }
        break;

        case 3:
            cout << "This commander forfeits this skirmish!\n";
            currentCmdr->cmdrHP = 0;
        break;
    }
}

//Entire Fight Loop
string cmdrBattle(){
    cout << "A fight between both commanders ensues!\n\n";

    cmdrResetStats(&cmdr1);
    cmdrResetStats(&cmdr2);
    
    while (cmdr1.cmdrHP > 0 || cmdr2.cmdrHP > 0){    
        cout << "Player 1, it's your move. What do you wish to do?\n";
        cmdrFight(&cmdr1, &cmdr2);

        if (cmdr1.cmdrHP <= 0){
            cout << "Player 1's commander has fallen!\n\n";
            return "p2win";
            break;
        }

        cout << "Player 2, it's your move. What do you wish to do?\n";
        cmdrFight(&cmdr2, &cmdr1);

        if (cmdr2.cmdrHP <= 0){
            cout << "Player 2's commander has fallen!\n\n";
            return "p1win";
            break;
        }
    }
}
\$\endgroup\$

2 Answers 2

4
\$\begingroup\$

General Guidelines

  1. Separate declaration and definitions into .cpp and .h (or .hpp, etc) files.
  2. Never use using xyz in header files. It may cause unexpected namespace scoping in files that include the header.
  3. Never use using namespace std.
  4. Never define global variables in .h files. It may lead to ODR (One Definition Rule) violation if the file is included in multiple files.
  5. Header files must have include guard at the top and bottom.
  6. Prefer int*_t, uint*_t and size_t over int, long etc. Using size_t for quantities. Like number of xyz.

Player Class

There is no need to make the player class a struct and then have free functions to operate over the player class. Encapsulate the data correctly. I would suggest something like this. You may further break the player class if that makes sense but the below example is the minimal required change.

class Player {
public:

    void expandTerritory();
    void upgradeTechnology();
    ...

private:

    size_t numTerritories = 2;
    size_t numArmies = 10;
    ...

};

You might further want to remove UI logic from the player class. Take the void recruitArmy() function for example. Instead of logging to cout inside the function. You can make the function return bool and then outside the function do the UI part. This allows you to change the UI logic without touching the player. It is also correct as per the Single Responsibility Principle. The player class now only deals with player data and not UI.

Game Class

Consider creating a new class called Game. You can then move the playerTurn(), hasLost() functions into that class. This would further improve the code as per SRP. You can also dependency inject the UI class into it making it possible for you to change the UI easily and also change the game rules if required.

Commander Class

Again encapsulate the commander class instead of having it as a plain old struct. Also, there is no need to prefix the members of Commander with cmdr. Instead use more verbose names. It is not clear what cmdrMOR does. Do not hardcode numbers into functions; prefer constexpr static const member variables to give them the right names.

\$\endgroup\$
10
  • \$\begingroup\$ At first player was a class, was recommended to use a struct instead because i should until I learn about something called access modifiers. Yes, you shouldn't use library references, the reason I did that was because I didn't want to put std:: next to every single keyword that needed it. cmdrMOR is a morale stat that is sent to battleInitiation(). Global variables are a bad call I was being lazy. What is constexpr static const? I understand them individually, but I've never seen them strung together. I somewhat know the concept of inheritance, I've just never found a good application for it \$\endgroup\$
    – Selisine
    Commented Mar 14, 2023 at 11:41
  • \$\begingroup\$ I generally agree with your General Guidelines but it would be helpful to others if you explained why for your points, it would also be helpful to others if you provided links to the SRP, ODR and other things you point out so that users really know they are best practices. \$\endgroup\$
    – pacmaninbw
    Commented Mar 14, 2023 at 12:24
  • \$\begingroup\$ @Selisine Putting std:: next to every single standard language object, function tempate etc is a fairly standard C++ practice. Not writing it will ensure your code will fail code review everywhere. So for better or worse get used to it if u want to use c++ in the future. constexpr static const means this member variable is a constant variable whose value is used at compile time (think of this as a #define VARIABLE VALUE but with better scoping). You need the static keyword because variable is the same throughout the class regardless of which object of the class is being refered too. \$\endgroup\$ Commented Mar 15, 2023 at 0:27
  • \$\begingroup\$ @AjinkyaKamat Oh okay, so does that mean putting std:: next to everything that needs it is better practice in a career as well? Also, thank you for explaining constexpr static const. \$\endgroup\$
    – Selisine
    Commented Mar 16, 2023 at 13:01
  • \$\begingroup\$ @Selisine Yes. In fact, if you don't then the senior developer in your team will make you add it during code review. The rationale behind this is simple. The namespace std has a lot of functions that have common names like std::next. Imagine your codebase or a library that you use has a function called next, it now becomes confusing if you are using your next or std::next. Generally, using namespace xyz at file level is frowned upon. Instead you can have it at function level. Or alias longer namespaces like namespace sc = boost::statechart at file scope(But never in header files) \$\endgroup\$ Commented Mar 16, 2023 at 17:29
3
\$\begingroup\$

General Observations

The review by Ajinkya Kamat is quite good and covers many of the major points. This review emphasizes some of those points as well as covering some other items that could be improved.

The program lacks modualarity it needs some serious redesign including object oriented design. It looks like a procedural programming language implementation rather than an object oriented language implementation. Modularity is important to procedural programming languages as well because it breaks the program into smaller problems that can be solved more easily.

Top down design, sometimes known as stepwise refinement is a design methodology for procedural languages. It is what I learned at the university in the 1980s for designing software. Object oriented design usually involves design patterns. Object oriented design should also be SOLID.

SOLID is 5 object oriented design principles. SOLID is a mnemonic acronym for five design principles intended to make software designs more understandable, flexible and maintainable. This will help you design your objects and classes better.

  1. The Single Responsibility Principle - A class should only have a single responsibility, that is, only changes to one part of the software's specification should be able to affect the specification of the class.
  2. The Open–closed Principle - states software entities (classes, modules, functions, etc.) should be open for extension, but closed for modification.
  3. The Liskov Substitution Principle - Objects in a program should be replaceable with instances of their subtypes without altering the correctness of that program.
  4. The Interface segregation principle - states that no client should be forced to depend on methods it does not use.
  5. The Dependency Inversion Principle - is a specific form of decoupling software modules. When following this principle, the conventional dependency relationships established from high-level, policy-setting modules to low-level, dependency modules are reversed, thus rendering high-level modules independent of the low-level module implementation details.

Some design patterns you may want to look into are separating program logic from the display, this is a common design pattern, and there are multiple design patters that implement it such as Model View Controller (MVC) and Model View Viewmodel (MVVM). The benefits of this are that different graphic systems could be used to display the game using the same game logic. This is a good reason to follow Ajinkya Kamat's suggestion about creating the game class.

Use Compiler Warnings

What ever compiler you are using can probably improve your coding if you use the warnings. My compiler (Visual Studio C++ 2022) provided the following warnings when I compiled the code:

Commander.h(22,55): warning C4244: '=': conversion from 'double' to 'int', possible loss of data
Commander.h(23,56): warning C4244: '
=': conversion from 'double' to 'int', possible loss of data
Commander.h(24,56): warning C4244: '=': conversion from 'double' to 'int', possible loss of data
Commander.h(25,57): warning C4244: '
=': conversion from 'double' to 'int', possible loss of data
Commander.h(26,56): warning C4244: '=': conversion from 'double' to 'int', possible loss of data
Commander.h(27,57): warning C4244: '
=': conversion from 'double' to 'int', possible loss of data
main.cpp(7,13): warning C4101: 'currentPlayer': unreferenced local variable
Commander.h(131): warning C4715: 'cmdrBattle': not all control paths return a value

The last warning is very important because it implies there are logic errors in the cmdrBattle() function.

The gcc/g++ Gnu compilers also provide these warnings when the proper compiler flags are used:
-Wall
-Wextra
-pedantic
-Werror

The Gnu compilers are free compilers that follow the programming standards, they are the default compilers on Linux and they are available for Windows as well if you install cygwin.

Free compilers on the internet don't necessarily follow the language standards.

Prefer Not to Use Raw Pointers

It the C programming language it is often necessary to use pointers to objects, in the C++ programming language it is much better not to use raw pointers and quite often not to use pointers at all. Global pointer variables such as Commander* currentCmdr; are very dangerous. In C++ you can have an std::array or an std::vector of Commanders instead. The currentCMDR variable could be an index into the array or vector.

Use references rather than pointers in most cases.

Prefer \n over std::endl

For performance reasons it is better to use \n in output rather than std::endl. The std::endl flushes the output which is a system call. System calls are very expensive and the can cause the program to temporarily stop executing while the operating system is servicing the system call.

C++ Random Number Generators

The srand() and rand() functions are from the standard C library, C++ provides better random number generators. Please see this Stack Overflow Question on C++ random number generation.

It is also customary to call srand() only once in a program during set up.

The Commander struct Should be a Class

In addition to the fields in the Commander struct there are all the cmdr functions. Each one of these functions addresses the struct. All of these should be turned into a class. I do NOT guarantee that the logic or the code is correct here. I can say that I doubt the game is currently working as you want it to based on some of the following code. This does show how much typing could be saved if it was a class. This is purely a possible class definition and not a good one:

class Commander
{
public:
    int cmdrLVL = 1;
    int cmdrATK = 2;
    int cmdrDEF = 5;
    int cmdrMDEF = 5;
    int cmdrHP = 10;
    int cmdrMHP = 10;
    int cmdrMOR = 3;
    void LevelUp()
    {
        std::cout << "Player you leveled up your commander!\n";
        cmdrLVL++;
        cmdrHP *= 1.2 * cmdrLVL + 1;
        cmdrMHP *= 1.2 * cmdrLVL + 1;
        cmdrDEF *= 1.2 * cmdrLVL + 1;
        cmdrMDEF *= 1.2 * cmdrLVL + 1;
        cmdrATK *= 1.1 * cmdrLVL + 1;
        currentCmdr->cmdrMOR *= 1.05 * currentCmdr->cmdrLVL + 1;
    }
    int ChoiceList() {
        int choice;
        std::cout << "1. Attack\n";
        std::cout << "2. Repair Armor\n";
        std::cout << "3. Forfeit\n";
        std::cin >> choice;
        return choice;
    }
    int getLevel()
    {
        return cmdrLVL;
    }
    void Stats() {
        std::cout << "[Current Commander Stats]\n";
        std::cout << "\tHP: " << cmdrHP << "/" << cmdrMHP << "\n";
        std::cout << "\tDEF: " << cmdrDEF << "/" << cmdrMDEF << "\n";
        std::cout << "\tATK: " << cmdrATK << "\n";
        std::cout << "\tMorale: " << cmdrMOR << "\n";
    }
    void ResetStats() {
        cmdrHP = cmdrMHP;
        cmdrDEF = cmdrMDEF;
    }
    //One Fight Cycle 
    void Fight(Commander& opposingCmdr) {
        int choice = 0;

        if (cmdrDEF < 0) {
            cmdrDEF = 0;
        }

        Stats();
        choice = ChoiceList();

        switch (choice) {
        case 1:
            std::cout << "This commander takes a swift strike at his opponent!\n";

            if (rand() % 20 + 1 < 10) {
                std::cout << "He hits!\n";
                if (currentCmdr->cmdrDEF > 0) {
                    currentCmdr->cmdrDEF -= opposingCmdr.cmdrATK;
                }
                else if (currentCmdr->cmdrDEF <= 0) {
                    currentCmdr->cmdrHP -= opposingCmdr.cmdrATK;
                }

            }
            else {
                std::cout << "He misses!\n";
            }
            break;

        case 2:
            if (cmdrDEF < cmdrMDEF) {
                std::cout << "This commander repairs his armor!\n'";
                cmdrDEF += (rand() % 3 + 1) * cmdrLVL;
            }
            else if (cmdrDEF >= cmdrMDEF) {
                std::cout << "Armor already at max!\n";
                cmdrDEF = cmdrMDEF;
            }
            break;

        case 3:
            std::cout << "This commander forfeits this skirmish!\n";
            cmdrHP = 0;
            break;
        }
    }
    //Entire Fight Loop
    std::string cmdrBattle(Commander& opposingCmdr) {
       std:: cout << "A fight between both commanders ensues!\n\n";

        ResetStats();
        opposingCmdr.ResetStats();

        while (cmdrHP > 0 || opposingCmdr.cmdrHP > 0) {
            std::cout << "Player 1, it's your move. What do you wish to do?\n";
            Fight(opposingCmdr);

            if (cmdrHP <= 0) {
                std::cout << "Player 1's commander has fallen!\n\n";
                return "p2win";
                break;
            }

            std::cout << "Player 2, it's your move. What do you wish to do?\n";
            opposingCmdr.Fight(cmdr1);

            if (cmdrHP <= 0) {
                std::cout << "Player 2's commander has fallen!\n\n";
                return "p1win";
                break;
            }
        }
    }
};

Don't Be Lazy

In a comment on another answer you mentioned that you were being lazy by using global variables. This code is also lazy and a bad practice:

using std::cout;
using std::string;
using std::cin;
using std::endl;

Especially in a header file.

Since this code is on GitHub you might be considering using it as part of a portfolio for jobs, and software manager or software engineer that looks at this code won't hire you because there are too many bad practices being used.

Avoid Global Variables

It is very difficult to read, write, debug and maintain programs that use global variables. Global variables can be modified by any function within the program and therefore require each function to be examined before making changes in the code. In C and C++ global variables impact the namespace and they can cause linking errors if they are defined in multiple files. The answers in this stackoverflow question provide a fuller explanation.

Avoid using namespace std;

If you are coding professionally you probably should get out of the habit of using the using namespace std; statement. The code will more clearly define where cout and other identifiers are coming from (std::cin, std::cout). As you start using namespaces in your code it is better to identify where each function comes from because there may be function name collisions from different namespaces. The identifiercout you may override within your own classes, and you may override the operator << in your own classes as well. This stack overflow question discusses this in more detail.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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