3
\$\begingroup\$

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
\$\endgroup\$
7
  • \$\begingroup\$ The code uses a non-standard form of getline(). Note that getline() is declared in stdio.h or cstdio and is not part of C++, it is part of the C programming language. This code does not follow the advice that was given in the original review, specifically game_logic.cpp should be divided into game_logic.h and game_logic.cpp and the same is true of player_stats.hpp. \$\endgroup\$
    – pacmaninbw
    Nov 26, 2023 at 15:27
  • \$\begingroup\$ Good to see that you have seperated input processing from the actual game logic. But this code has a long way to go. As it stands it is confusing to read and maintain. I will take a look and get back to you. \$\endgroup\$ Nov 28, 2023 at 7:48
  • \$\begingroup\$ @pacmaninbw I was not aware that getline was apart of C. Is there a C++ equivalent to storing an entire line into a string (maybe a foreach now that I think about it)? I am aware I didn’t follow all of the things within the post, and I may improve it, given I understand more. One thing I noticed is that I need to change some of the datatypes to better suit their purposes. I thought you split declarations into the header, and logic into another cpp file. \$\endgroup\$
    – Selisine
    Nov 28, 2023 at 18:41
  • \$\begingroup\$ @AjinkyaKamat I appreciate it. \$\endgroup\$
    – Selisine
    Nov 28, 2023 at 18:42
  • \$\begingroup\$ Standard input can be read into a string type. std::cin >> stringVariable. \$\endgroup\$
    – pacmaninbw
    Nov 28, 2023 at 19:48

1 Answer 1

3
\$\begingroup\$

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.

\$\endgroup\$
8
  • \$\begingroup\$ Answers 1. I knew declarations and definitions would go into hpp and cpp files (hence player_stats being my header file with all of the stats and game_logic having more math). That answer you linked makes a lot of sense! I’ll make sure to structure my code like that in the future. Outside of replit, I generally use MinGW on VS Code, but I’ll try and learn how CMake and other build systems work so I can implement them into replit when I have to use them. 2. What are the positives to using a namespace? 3. I think I’ve underestimated the usefulness of constructors. \$\endgroup\$
    – Selisine
    Dec 7, 2023 at 16:45
  • \$\begingroup\$ 4. I’m glad I was at least on the right track. I believe it was you or pacman that had told me to be more verbose with my variable names, so I sought to do so. Thank you for the elaboration! 5. I’ve thought about using Hungarian notation, and I have a couple friends that do. I’m not really a fan of it; why should I use it? 6. I had learned about enumerators, but hadn’t used them a whole lot. That makes sense, though, I could see how returning a string would be error-prone. 7. What’s funny is that’s how my code was originally–I just wasn’t sure what else to put in the main. \$\endgroup\$
    – Selisine
    Dec 7, 2023 at 16:45
  • \$\begingroup\$ 8. That saves me some lines, I hadn’t thought about using modulus in that context or not for that matter. Using not makes sense, but how would I implement modulus in turn_counter_update()? 9. I wasn’t even aware that there was a struct version of an array, but that makes a lot of sense! Consider it implemented. 10. I’ll be honest, I have a lot to learn with PRNGs, most of that code was copied and modified to my needs. Is srand specific to the rand() method? \$\endgroup\$
    – Selisine
    Dec 7, 2023 at 16:46
  • \$\begingroup\$ Architecture I appreciate the changes you made in that fork! There’s definitely a lot I’ve got to learn. I was attempting to make my code styled like OOP, but I think I need to do more research on how to do so. I’ll also need to learn more about pointers and namespaces, as I’ve noticed quite a few of those in the fork. I can see how making actions dynamic could make the game more expandable! Also, how would I make an interface in code? \$\endgroup\$
    – Selisine
    Dec 7, 2023 at 16:46
  • \$\begingroup\$ 2. Namespaces prevent name collisions aka compilation failure because your code and the library code use the same name for functions. Without namespaces, if you write a function named find you might get a error or worse the can compiler silently use the find function in std but you wanted it to use your find. So it is pretty much necessary \$\endgroup\$ Dec 7, 2023 at 16:51

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.