Re: C++ 4X Game

Question

This post is regarding the revamp of this post/project I did about 8 months ago. I took a lot of the advice from the suggestions provided (thank you by the way), and fixed the project up, so I'm reposting it. @Ajinkya Kamat wanted to see the revisions I made, so here they are:

(From https://replit.com/@Selisine/CBPR?v=1)

main.cpp

#include <iostream> 
#include "game_logic.cpp"


int main(){
    game_logic game_logic;
    bool repeater = false;
    
    game_logic.assign_stats();
    game_logic.name_entry();

    //Begin game loop
    while (game_logic.victor_status() == "continue"){
        game_logic.check_non_zeroes();
        game_logic.stat_rundown();
        game_logic.passive_income();

        do {
            switch (game_logic.turn_choice()){
                case 1: 
                    game_logic.expand_territory();
                    repeater = false;
                break;
    
                case 2: 
                    game_logic.upgrade_technology();
                    repeater = false;
                break;
    
                case 3: 
                    game_logic.recruit_army();
                    repeater = false;
                break;
    
                case 4: 
                    game_logic.battle_initiation();
                    repeater = false;
                break;
    
                default:
                    std::cout << "Invalid input! Try again...";
                    repeater = true;
                break;
            }
        } while (repeater);

        game_logic.turn_counter_update();
    }
}

player_stats.hpp

#ifndef CBPR_PLAYER_STATS_HPP
#define CBPR_PLAYER_STATS_HPP
#include <iostream>

class player_stats {
    public:
        player_stats(); //Setter
        int stat_table[2][5];
        int return_values(int, int);
        void assign_values();

    private:
        int territory_count;
        int army_count;
        int money;

        //Tech Levels
        int army_skill_level;
        int passive_income_level;
};

void player_stats::assign_values(){
    int base_stat_table[5] = {territory_count, army_count, money, army_skill_level, passive_income_level};

    for (int x = 0; x < 2; x++){
        for (int y = 0; y < 5; y++){
            stat_table[x][y] = base_stat_table[y];
        }
    }    
}

//Setter
player_stats::player_stats(){
    territory_count = 2;
    army_count = 5;
    money = 50'000;
    army_skill_level = 0;
    passive_income_level = 0;    

    assign_values();
}

//Getter
int player_stats::return_values(int turn_counter, int stat_type){
    return stat_table[turn_counter][stat_type];
}
#endif //CBPR_PLAYER_STATS_HPP

game_logic.cpp

#ifndef CBPR_GAME_LOGIC_CPP
#define CBPR_GAME_LOGIC_CPP
#include <iostream>
#include <random>
#include "player_stats.hpp"

class game_logic {
    public:
        //Misc
        void assign_stats();
        void name_entry();
        void stat_rundown();
        //Choices
        int turn_choice();
        void expand_territory();
        void upgrade_technology();
        void recruit_army();
        void battle_initiation();
        //Passives
        void passive_income();
        int turn_counter = 0;
        void turn_counter_update();
        std::string victor_status();
        void check_non_zeroes();
    private:
        std::string names[2]; 
        int player_stats_malleable[2][5];
};

/*
-----PREPROCESSED-----
*/
void game_logic::assign_stats(){
    player_stats player_stats; 

    for (int x = 0; x < 2; x++){
        for (int y = 0; y < 5; y++){
            player_stats_malleable[x][y] = player_stats.return_values(x, y);
        }
    }
}

/*
-----MISC-----  
*/
//Gets all player names
void game_logic::name_entry() {
    std::cout << "Player 1, enter your name: ";
    std::getline(std::cin, names[0]);
    std::cout << "Player 2, enter your name: ";
    std::getline(std::cin, names[1]);
}

//Lists all stats
void game_logic::stat_rundown(){
    std::string stat_type[5] = {"Territories: ", "Armies: ", "Money: $", "Army Skill Level: ", "Passive Income Level: "};

    for (int x = 0; x < 2; x++){
        std::cout << "[" << names[x] << "'s stats]\n";
        for (int y = 0; y < 5; y++){
            std::cout << stat_type[y];
            std::cout << player_stats_malleable[x][y] << "\n";
        }
        std::cout << "\n";
    }
}

//Dice Roll
int will_of_tyche(){
   static std::default_random_engine rng(std::random_device{}());
   static std::uniform_int_distribution<> dist{};
   
   return dist(rng, decltype(dist)::param_type{1, 100});
}

//Checks if the player's choice can be bought
bool can_be_bought(int player_money, int price){
    if (player_money - price < 0){
        return false;
    }
    return true;
}

/*
-----CHOICES-----  
*/
//Lists each choice
void choice_list(){
    game_logic game_logic;
    std::string choice_list[5] = {"Expand Territory [$10,000]", "Upgrade Technology", "Recruit Army [$8,000]", "Battle"};

    for (int x = 0; x < 4; x++){
        std::cout << "[" << x + 1 << "] ";
        std::cout << choice_list[x] << "\n";
    }
}

//Gets and returns the player's choice
int game_logic::turn_choice(){
    int player_choice; 

    std::cout << "It is your turn " << names[turn_counter] << ". Here are your options:\n";

    choice_list();

    std::cout << "Choice: ";
    std::cin >> player_choice;

    return player_choice;
}

//+1 Territories if can_be_bought() returns true
void game_logic::expand_territory(){    
    int price = 10'000;
    
    if (!can_be_bought(player_stats_malleable[turn_counter][2], price)){
        std::cout << "Not enough money!\n";
    }
    else {
        std::cout << names[turn_counter] << " expands their territory! [-10,000]\n"; 
        player_stats_malleable[turn_counter][0] += 1;
        player_stats_malleable[turn_counter][2] -= 10'000;
    }    
}

/*
---TECHNOLOGY---
*/
//List of technologies to choose from
void tech_list(){
    std::string tech_list[2] = {"Army Skill [$8,000]", "Passive Income Level [$8,000]"};

    for (int x = 0; x < 2; x++){
        std::cout << "[" << x + 1 << "] ";
        std::cout << tech_list[x]  << "\n";
    }
}

//+1 of chosen technology
void game_logic::upgrade_technology(){
    int technology_index;
    int price = 8'000;

    std::cout << names[turn_counter] << ", what technology would you like to upgrade?\n";
    tech_list();
    std::cin >> technology_index;

    if (!can_be_bought(player_stats_malleable[turn_counter][2], price)){
        std::cout << "Not enough money!\n"; 
    }
    else {
        player_stats_malleable[turn_counter][technology_index + 2] += 1;
        player_stats_malleable[turn_counter][2] -= price;
    }
}

//+1 army
void game_logic::recruit_army(){
    int price = 8000;
    
    if (player_stats_malleable[turn_counter][1] == player_stats_malleable[turn_counter][0]){
        std::cout << names[turn_counter] << " recruits another army! [-8,000]\n"; 
        player_stats_malleable[turn_counter][1] += 1;
        player_stats_malleable[turn_counter][2] -= 8'000;
    }
    else if (!can_be_bought(player_stats_malleable[turn_counter][2], price)){
        std::cout << "Not enough money!\n";
    }
    else {
        std::cout << "Territories must be as much or higher than armies!\n";
    }
}

void game_logic::battle_initiation(){
    int player_one_boosts = .3 * player_stats_malleable[0][1] * player_stats_malleable[0][3];
    int player_two_boosts = .3 * player_stats_malleable[1][1] * player_stats_malleable[1][3];
    srand( time(NULL) );
    int player_one_roll;
    int player_two_roll;

    std::cout << "A skirmish commences!\n\n";

    player_one_roll = will_of_tyche() + player_one_boosts;
    player_two_roll = will_of_tyche() + player_two_boosts;

    if (std::max(player_one_roll, player_two_roll) == player_one_roll){
        std::cout << player_one_roll << " > " << player_two_roll << "\n"; 
        std::cout << names[0] << " wins!\n";
        player_stats_malleable[0][2] += 1'000;
        player_stats_malleable[1][2] -= 1'000;
    } 
    else if (std::max(player_one_roll, player_two_roll) == player_two_roll){
        std::cout << player_one_roll << " < " << player_two_roll << "\n"; 
        std::cout << names[1] << " wins!\n";
        player_stats_malleable[0][2] -= 1'000;
        player_stats_malleable[1][2] += 1'000;
    }
    else {
        std::cout << "Tie!\n";
    }

}


/*
-----PASSIVES-----
*/
void game_logic::passive_income(){
    int upgrade_addition = 1'000 * (.1 * player_stats_malleable[turn_counter][4]);
    player_stats_malleable[turn_counter][2] += 1'000 + upgrade_addition;
}

std::string game_logic::victor_status(){
    if (player_stats_malleable[0][0] <= 0 || player_stats_malleable[0][2] <= 0){
        std::cout << names[1] << " has won the war! Congratulations!\n";
        return "player_two_win";
    }
    else if (player_stats_malleable[1][0] <= 0 || player_stats_malleable[1][2] <= 0){
        std::cout << names[1] << " has won the war! Congratulations!\n";
        return "player_one_win";
    }
    else {
        return "continue";
    }
}

void game_logic::turn_counter_update(){
    if (turn_counter + 1 == 1){
        turn_counter++;
    }
    else if (turn_counter + 1 == 2){
        turn_counter--;
    }
    else {
        std::cout << "Something is wrong.\n";
    }
}

//Checks if armies is below zero (potential to lose one during battle)
void game_logic::check_non_zeroes(){
    if (player_stats_malleable[turn_counter][1] < 0){
        player_stats_malleable[turn_counter][1] = 0;
    }
}
#endif //CBPR_GAME_LOGIC_CPP

Answer 1

Just seperating the game_logic from input processing has made a huge difference in terms of code clarity. In this review, I will start with nitpicks with the current code and then suggest a new architecture to make the code easier to extend. I can easily see this project as a good jumping off point into good software architecture.

In this review, I will first go through the code and review it. Since you are a new coder, I will suggest a better architecture, so as to introduce to this key aspect of software engineering.

Review

I have forked your repl and made changes. You can see them here.

1. Never #include a cpp file. Always seperate declarations and definition into hpp and cpp files. This answer goes into details. This means you will have to use a build system. I have used CMake in my reviewed code. I had to modify the hidden .replit and replit.nix files to make this work.
2. Always use your own namespace for your code.
3. Prefer constructors over delayed initialization. You dont need a seperate assign_logic() and name_entry() functions in game_logic class.
4. Prefer <verb>_<noun> pattern for naming functions. name_entry() can be input_names().
5. Use m_ prefix to identify class member variables. So game_logic::names becomes game_logic::m_names
6. Prefer enum classes over std::string to return status. This reduces errors as the status can only take values listed in the enum. This also makes the code more readable because the reader does not have to look at the implementation. game_logic::victory_status() should return enums.
7. Since, you are already doing input processing in game_logic. The switch statement after turn_choice can be moved inside game_logic.
8. turn_counter_update can be implemented using modulus operator instead of if statements. Similarly can_be_bought can be implement by using not operator.
9. Prefer structs over arrays for storing named parameters like player_stats.
10. In will_of_tyche you dont need to have decltype incantation. You can just initialize std::uniform_distribution with the right parameters aka 1, 100. C and C++ random number generators are not the same, you don't have to call srand before using C++ random number generators

New Architecture

The repl for this can be found here.

Your code is written in procedural style. There is nothing wrong with procedural style. But your problem statement will benefit if we introduce object oriented paradigm here. In this iteration, I will focus on encapsulating player choice. I will call player choices Actions from now on. My goal here is to make every action a standalone item. Each Player has a list of Actions that they can perform. At the start of their turn, they are presented with this list. The player chooses and action which our code executes. Upon execution, the player stats change. I will also keep these actions dynamic. This can allow for complex behavior such as the cost of a tech upgrade might increase as the tech levels up. Keeping the player dynamic actions and storing them in a list means that a concept of civilizations similar to Age of Empires can easily be added in the future.

Also, making Game an interface we can easily add features to save and replay games. This will make it super easy to balance the game.