Незавершенная ролевая игра

Вот ролевая игра, над которой я работаю, которая использует только стандартные библиотеки C ++. У меня возникли проблемы с концепцией системы координат для персонажей, созданных на карте, а система инвентаря / предметов на данный момент является только оболочкой. Однако я создал небольшую демонстрацию, включающую некоторые функции, которые будут присутствовать в полной версии продукта (Game::Test()). Это моя первая настоящая попытка создать игру, поэтому я хотел бы услышать, что вы думаете о том, что у меня есть на данный момент, и о том, как я могу улучшить.

* Примечание: я не включил свою систему инвентаря, так как для нее почти нет кода, а в любом случае есть вся концепция.

main.cpp

#include <vector>
#include <cstring>
#include <cstdlib>
#include <ctime>

#include "Game.h"

int main() {
    //Seed to use for run;
    //!FIX; OVERHAUL TO HAVE SEED GENERATED BY RUN INSTEAD OF INSTANCE
    srand(time(NULL));
    
    Game myRpg;
    char charInput;
    std::string strInput;
    
    //Run Test Mode (Has Access To Basic Game Mechanics)
    std::cout << "Run test? Enter 'Y' for test." << std::endl;
    std::cin >> charInput;
    
    //Test is all that exists right now. Always enter Y to run game
    if (charInput == 'Y') {
        myRpg.Test();
    }
            
    return 0;
}

Game.h

#ifndef GAME_H
#define GAME_H

#include <iostream>
#include <cstring>
#include <vector>

#include "Characters.h"
#include "Maps.h"
#include "Player.h"

class Game {
    public:
        Game() {Clear(); Intro(); myChar.CharacterCreation();};
        void Clear();
        
        void Intro();
        
        void Test();
        void Run();
    
    private:
        //!FIX; `OUT-OF-RANGE` CRASH WHEN INITIALIZING MAP LIST
        /*===WIP===
        Maps mapLib;
        */
    
        Characters charLib; //!Will use `(mapLib.size())` once Maps fixed
        Characters* myChars = &charLib;
        
        Player myChar;
        Character* player = myChar.p;
        
        //Counts num turns player has made since init; Currently unused
        int time;
};

#endif

Game.cpp

#include <iostream>
#include <fstream>
#include <cstring>
#include <cstdio>
#include <cstdlib>
#include <ctime>
#include <vector>

#include "Game.h"

void Game::Clear() {
    for(unsigned int i = 0; i < 50; i++) {
        std::cout << std::endl;
    }
}

//Displays game intro using text file "intro.txt"
void Game::Intro() {
    //Opens text file "intro.txt"
    std::ifstream intro;
    intro.open("intro.txt");
    
    //Outputs text from file
    if(intro.is_open()) {
        std::string str;
        //While file has not reached the end
        while(!(intro.eof())) {
            getline(intro, str);
            std::cout << str << std::endl;
        }
        std::cout << std::endl << std::endl;
        intro.close();
    }
}

//Mode to run basic battle functions
void Game::Test() {
    //Clear previous screen
    Clear();
    
    /*!===TEST; Test() Start===
    std::cout << "Test Start" << std::endl;
    */
    
    //Create character and single opponent
    Character opponent;
    opponent = charLib.CreateChar("Bandit");
    Character* opp = &opponent;
    
    //Create Default Map
    Map default_map; //!FIX; Find Seg. Fault (Map.cpp)
    default_map.Print();
    
    std::cout << "As you sail across the ocean, you come across a bandit," << std::endl
              << "floating over the water. You pull him aboard, when      " << std::endl
              << "suddenly he jolts up, knife readied in hand. It was a  " << std::endl
              << "trick!" << std::endl;
    std::cout << "Press 'enter' to continue.";
    std::cin.get();
    std::cin.ignore();
    std::cout << std::endl;
    
    player->Battle(opp);
    
    if (player->IsAlive() == 0) {
        std::cout << "The treasure remains lost. GAME OVER" << std::endl;
        return;
    }
    
    std::cout << "You won!- but the treasure is still out there somewhere..." << std::endl;
    
    /*!===TEST; Test() End===
    std::cout << "Test Successful!" << std::endl;
    */
    
}

Player.h

#ifndef PLAYER_H
#define PLAYER_H

#include <iostream>
#include <cstring>
#include <vector>

#include "Character.h"  

class Player {
    public:
        Player() {p = &player; lv = 1; player.SetSym('@');}
        int GetLv();
        
        void PlayerTurn();
        void Battle(Character *opponent);          //Option 1
        void Defend();          //Option 2
        void OpenInventory();   //Option 3
        void RunAway();         //Option 4
        
        void CharacterCreation();
        
        Character* p;
    
    private:
        Character player;
        int lv;
};

/*===WIP===//
struct Ability {
    std::string name;
    int lv;
    
    int cost(int lv) {
        return (lv);
    }
    
    bool canAfford(int lv, int mp) {
        return cost(lv) <= mp;
    }
        
    void show(int lv) {
        std::cout << "[" << cost() << "] mpn";
    }
};

class Job {
    public:
        void PrintAbilities(int lv);
        
    private:
        std::string name;
        std::vector<Ability> abilities;
        int numAbilities() {return abilities.size();}
        
        Ability const &operator[](int index) const {
            return abilities.at(index);
        }
    
};
//=========*/

#endif

Player.cpp

#include <iostream>
#include <cstring>
#include <cstdio>
#include <vector>
#include <iomanip>

#include "Player.h"

//===WIP===
void Player::PlayerTurn() {
    char usrInput=" ";
    
    std::cout << "Your turn. Press '?' for help." << std::endl;
    usrInput = getchar();
    
    switch(usrInput) {
        case 'w' || 'a' || 's' || 'd':
            break;
        case '?':
            break;
        case 'i':  //!FIX; FINISH INVENTORY/ITEM SYSTEM
            std::cout << "Not functional at this time." << std::endl;
            break;
        case '*':
            break;
    }
    
    if (player.IsAlive() == 0) {
        std::cout << "Your adventure has come to an end." << std::endl;
    }
}

void Player::Battle(Character *opponent) {
    player.Battle(opponent);
}   

void Player::CharacterCreation() {
    std::string usrStr;
    
    //Set Player Name
    std::cout << "What's your name? ";
    getline(std::cin, usrStr);
    player.SetName(usrStr); //Set name as usrStr input
    std::cout << std::endl;
    
    //Set Player Stats
    bool isYes;
    bool isRollOrSet;
    
    do {
        std::cout << "Would you like to 'roll' or 'set' your stats? "
                  << "Please enter 'roll' or 'set'" << std::endl;
        std::cin >> usrStr;
        if ((usrStr != "roll") && (usrStr != "set")) {
            std::cout << "Please enter an appropriate response." << std::endl;
        }
        else {
            do {
                bool setOrRoll = (usrStr == "roll");
                switch (setOrRoll) {
                    case 1: //Rolls stats
                        player.RollStats();
                        break;
                        
                    case 0: //Use a stat pool
                        player.SetStats();
                        break;
                }
                //Confirm with user that they want these stats; Display Stats
                std::cout << "Are these stats okay? Enter 'YES' if so, or enter "
                          << "'roll' / 'set' to redo your stats accordingly.n";
                std::cout << std::setw(2);
                std::cout << "HP: " << player.GetHp() << std::endl;
                std::cout << "MP: " << player.GetMp() << std::endl;
                std::cout << "STR: " << player.GetStr() << std::endl;
                std::cout << "DEF: " << player.GetDef() << std::endl;
                std::cout << "MAG: " << player.GetMag() << std::endl;
                std::cout << "SPD: " << player.GetSpd() << std::endl;
                
                do {
                    std::cin >> usrStr;
                    
                    isYes = (usrStr == "YES");
                    isRollOrSet = ((usrStr == "roll") || (usrStr == "set"));
                    
                    //If they entered "YES"
                    if (usrStr == "YES") {
                        break;
                    }
                    //If they entered something other than "YES", "roll", or "set"
                    else if ((isYes == false) && (isRollOrSet == false)) {
                        std::cout << "Please enter an appropriate answer." << std::endl << std::endl;
                    }
                //Loops input
                } while((isYes == false) && (isRollOrSet == false));
            //Loops stat roll
            } while(isRollOrSet == true);
        }
    //Loops initial question
    } while(isYes == false);
}

/*===WIP===//
Job::PrintAbilities(int lv) {
    for(unsigned int i = 0; i < numAbilities; i++) {
        std::cout << "[" << i << "]";
        abilites[i].show(lv);
    }
}
//=========*/

Characters.h

#ifndef CHARACTERS_H
#define CHARACTERS_H

#include <iostream>
#include <vector>

#include "Character.h"

class Coord {
    public:
        Coord() {};
        void SetData(int x, int y) {
            this->x = x;
            this->y = y;
        }
        
        void XShift(int a) {x += a;}
        void YShift (int b) {y += b;}
        
        int GetX() {return x;}
        int GetY() {return y;}
        
    private:
        int x;
        int y;
};

/********************************************************************/

class Characters {
    public:
        Characters() {SetArchList();}
        Characters(int numMaps) {SetArchList(); charList.resize(numMaps);}
        void AddCharArch(Character _char, std::vector<Character> _chars);
        Character CreateChar(std::string archName);
        
    private:
        //Stores character archetypes
        std::vector<Character> archList;
        std::vector<std::vector<Character>> charList;
        /*!FIX; DYNAMICALLY ADD CHARACTERS FROM CURR MAP TO LIST
         * Reference == Map.cpp, `void Map::SpawnEntities() {};`  */
        
        void SetArchList();
};

#endif

Characters.cpp

#include <iostream>
#include <cstring>
#include <vector>

#include "Characters.h"

//Adds character archetypes to characterList
void Characters::AddCharArch(Character _char, std::vector<Character> _chars = {{"null"}}) {
    if (_chars.at(0).GetName() == "null") {
        //Copies character data _char
        archList.push_back(_char);
    }
    else {
        //Copies data from character vector _chars
        for(unsigned int i = 0; i < _chars.size(); i++) {
            archList.push_back(_chars.at(i));
        }
    }
}

//Retrieves character archetype data from characterList
Character Characters::CreateChar(std::string archName) {
    Character archType;
    for(unsigned int i = 0; i < archList.size(); i++) {
        Character* charListArch = &archList.at(i); //Creates a pointer for archetype in charList at i
        if(charListArch->GetName() == archName) {
            archType = charListArch->GetName();
        }
        else {
            continue;
        }
    }
    
    return archType;
}

void Characters::SetArchList() {
    //Archetype Info |Name     | hp| mp|str|def|mag|spd|sym|
    archList =      {{"Bandit" ,  15,  0,  5,  2,  0,  2,'b'},
                     {"Crew"   ,  24,  4,  8,  5,  2,  4,'c'},
                     {"Captain",  32,  8, 10,  8,  5,  6,'C'}};
};

Character.h

#ifndef CHARACTER_H
#define CHARACTER_H

#include <iostream>
#include <cstring>
#include <vector>

#include "Inventory.h"

class Character {
    public:
        Character() : name("Default"), hp(6), mp(6), str(6), def(6), mag(6), spd(6) {};
        Character(std::string name) : hp(12), mp(6), str(6), def(6), mag(6), spd(6) {this->name = name;}
        Character(std::string name, int hp, int mp, int str, int def, int mag, int spd, char symbol) {
            this->name = name;
            this->hp = hp * 3;
            this->mp = mp;
            this->str = str;
            this->def = def;
            this->mag = mag;
            this->spd = spd;
            this->symbol = symbol;
        }
        
        void SetName(std::string);                //Create character name
        std::string GetName() {return name;}      //Get character's name
        
        void SetStats();      //Set stat values using point pool
        void RollStats();     //Roll d12 for character's start stats
        
        void SetHp(int hp) {this->hp = hp;}
        void SetMp(int mp) {this->mp = mp;}
        void SetStr(int str) {this->str = str;}
        void SetDef(int def) {this->def = def;}
        void SetMag(int mag) {this->mag = mag;}
        void SetSpd(int spd) {this->spd = spd;}
        void SetSym(char symbol) {this->symbol = symbol;}
        
        int GetHp() const {return hp;}
        int GetMp() const {return mp;}
        int GetStr() const {return str;}
        int GetDef() const {return def;}
        int GetMag() const {return mag;}
        int GetSpd() const {return spd;}
        char GetSym() const {return symbol;}
        
        void Battle(Character *opponent);
        
        bool IsAlive() const {return hp > 0;}        //Check if character's alive
        void TakeDamage(Character *opponent);        //Take damage
        int GetDamage(Character* opponent) const;
        
    private:
        std::string name;  //Character name
        int hp;            //Health of character
        int mp;            //Magic points of character
        int str;           //Strength of character
        int def;           //Defense of character
        int mag;           //Magic skill of character
        int spd;           //Speed of character
        
        char symbol;
};

#endif

Character.cpp

#include <iostream>
#include <cstring>
#include <cstdlib>

#include "Character.h"

void Character::SetName(std::string name) {
    this->name = name;
}

//Allows the user to freely set their character's stats from a pool of 36
void Character::SetStats() {
    int pointPool = 36;
    
    for(unsigned int i = 0; i < 6; i++) {
        int usrInt = 0;
        bool goodStat = false;
        
        do {
            std::cout << "You have " << pointPool << " points remaining.n";
            std::cout << "How many points for ";
            switch(i) {
                case 0:
                    std::cout << "HP? ";
                    std::cin >> usrInt;
                    break;
                    
                case 1:
                    std::cout << "MP? ";
                    std::cin >> usrInt;
                    break;
                    
                case 2:
                    std::cout << "STR? ";
                    std::cin >> usrInt;
                    break;
                    
                case 3:
                    std::cout << "DEF? ";
                    std::cin >> usrInt;
                    break;
                    
                case 4:
                    std::cout << "MAG? ";
                    std::cin >> usrInt;
                    break;
                    
                case 5:
                    std::cout << "SPD? ";
                    std::cin >> usrInt;
                    break;
            }
            // Check if player choice was appropriate
            if (usrInt > pointPool) {
                std::cout << "Not enough points." << std::endl;
            }
            else if (usrInt < 0) {
                std::cout << "Can't allocate less than 0 points." << std::endl;
            }
            //Set Stats
            else {
                switch(i) {
                    case 0:
                        SetHp(usrInt * 3);
                        break;
                        
                    case 1:
                        SetMp(usrInt);
                        break;
                        
                    case 2:
                        SetStr(usrInt);
                        break;
                        
                    case 3:
                        SetDef(usrInt);
                        break;
                        
                    case 4:
                        SetMag(usrInt);
                        break;
                        
                    case 5:
                        SetSpd(usrInt);
                        break;
                }
                goodStat = true;
            }       
        } while(goodStat == false);
        
        //Subtract player choice from point pool
        pointPool -= usrInt;
        
        /*!===TEST; SetStats() Interval===
        std::cout << std::endl;
        std::cout << "Test: Interval " << i << std::endl;
        */
    }
}

void Character::RollStats() {
    int pointPool = 36;
    
    for(unsigned int i = 0; i < 6; i++) {
        int randNum;
        
        //Get random number 1-8 for stat allocation
        do {
            randNum = (rand() % 8) + 1;
            if(randNum <= pointPool) {
                break;
            }
        } while(randNum > pointPool);
        
        //Set stat using randNum
        switch(i) {
            case 0:
                if (randNum <= 3) {
                    randNum = 4;
                }
                SetHp(randNum * 3); //Set Health
                break;
            case 1:
                SetMp(randNum); //Set Magic Points
                break;
            case 2:
                SetStr(randNum); //Set Strength
                break;
            case 3:
                SetDef(randNum); //Set Defense
                break;
            case 4:
                SetMag(randNum); //Set Magic Skill
                break;
            case 5:
                SetSpd(randNum); //Set Speed
                break;
        }
        pointPool -= randNum; //Remove randNum from pointPool
    }
}

void Character::Battle(Character *opponent) {
    bool att; //Attacker
    bool opp; //Opponent
    
    do {
        bool whoseFaster = GetSpd() > opponent->GetSpd();
        
        switch(whoseFaster) {
            case 1: //Player's faster
                opponent->TakeDamage(this);
                std::cout << name << " hits " << opponent->GetName()
                          << " for " << opponent->GetDamage(this) << " points." << std::endl;
                
                //If the opponent died from attack, end battle
                if (IsAlive() == false) {
                    break;
                }
                
                TakeDamage(opponent);
                std::cout << opponent->GetName() << " hits " << name
                          << " for " << GetDamage(opponent) << " points." << std::endl;
                break;
            case 0: //Opponent's faster
                TakeDamage(opponent);
                std::cout << opponent->GetName() << " hits " << name
                          << " for " << GetDamage(opponent) << " points." << std::endl;
                          
                //If player died from attack, end battle
                if (IsAlive() == false) {
                    break;
                }
                
                //Else, player attacks
                opponent->TakeDamage(this);
                std::cout << name << " hits " << opponent->GetName()
                          << " for " << opponent->GetDamage(this) << " points." << std::endl;
                break;
        }
        
        std::cout << GetName() << " has " << GetHp() << " HP remaining." << std::endl
                  << opponent->GetName() << " has " << opponent->GetHp() << " hp remaining." << std::endl
                  << std::endl;
        
        att = IsAlive();
        opp = opponent->IsAlive();
    } while( (att == true) && (opp == true) );
    
    switch(att) {
        case 1:
            std::cout << opponent->GetName() << " was slain." << std::endl;
            break;
        case 0:
            std::cout << name << " was cut down." << std::endl;
            break;
    }
}

//Character takes damage from parameter
//  |Recipient|            |Attacker|
void Character::TakeDamage(Character *opponent) {
    int dmg = GetDamage(opponent);
    
    hp -= dmg;
}

//Returns damage value
//  |Recipient|          |Attacker|
int Character::GetDamage(Character* opponent) const {
    int dmg = (opponent->GetStr() - def);
    if (dmg < 1) {
        dmg = 1;
    }
    
    return dmg;
}

Maps.h

#ifndef MAPS_H
#define MAPS_H

#include <iostream>
#include <cstring>
#include <vector>

#include "Map.h"

class Maps {
    public:
        Maps() {SetMapTypes(); SetMapNumTypes(); GenMaps(1);}
        Maps(int numMaps) {SetMapTypes(); SetMapNumTypes(); GenMaps(numMaps);}
        void SetCurrMap();
        void PrintCurrMap() const;
        
    private:
        //Map types and num of maps per type
        std::vector<std::string> mapTypes;
        std::vector<int> mapNumTypes;
        
        //Collection of Map
        std::vector<Map> maps;
        
        //Copy of map for gameplay 
        Map currMap;
        
        void SetMapTypes();
        void SetMapNumTypes();
        void GenMaps(int numMaps);
};

#endif

Maps.cpp

#include <iostream>
#include <cstring>
#include <vector>
#include <cstdlib>

#include "Maps.h"
#include "Map.h"

void Maps::SetMapTypes() {
    mapTypes = {"test_area", "ship_battle"};
}

void Maps::SetMapNumTypes() {
    //             test_area, ship_battles
    mapNumTypes = {        1,            1};
}

void Maps::GenMaps(int numMaps) {
    srand(time(NULL));
        
    for(int k = 0; k < numMaps; k++) {  
        //Set random map type from library
        int randType = rand() % mapTypes.size();
        std::string mapType = mapTypes.at(randType);
            
        //Get num of maps for type; Set random map variation
        int mapVars = mapNumTypes.at(randType);
        int randMapVar = rand() % mapVars;
            
        //Set map name
        std::string mapName = mapType;
        mapName.push_back(randMapVar);
        mapName.append(".txt");
            
        //Generate and store map copy
        Map map(mapName);
        maps.push_back(map);
    }
}

Map.h

#ifndef MAP_H
#define MAP_H

#include <iostream>
#include <cstring>
#include <vector>

#include "Characters.h"

class Map {
    public:
        Map() {this->name = "default"; GenMap(); SpawnEntities();}
        Map(std::string name) {this->name = name; GenMap(); SpawnEntities();}
        void Print() const;
    
    private:
        std::string name;
        std::vector<std::vector<char>> map;

        void GenMap();
        void SpawnEntities();
};

#endif

Map.cpp

#include <iostream>
#include <fstream>
#include <cstring>
#include <cstdlib>
#include <ctime>
#include <vector>

#include "Map.h"

void Map::GenMap() {
    /*!===TEST; GenMap() Start===
    std::cout << "Map Gen Start" << std::endl;
    */
    
    int width = 0;
    int height = 0;
    
    //If name is "default", generate pre-made test map
    if (name == "default") {
        map = {{'~','~','~','~','~','~','~','~','~'},
               {'~','~','~','~','~','~','~','~','~'},
               {'~','~','~','~','-','~','~','~','~'},
               {'~','~','~','|','*','|','~','~','~'},
               {'~','~','~','|','b','|','~','~','~'},
               {'~','~','~','|','@','|','~','~','~'},
               {'~','~','~','~','-','~','~','~','~'},
               {'~','~','~','~','~','~','~','~','~'},
               {'~','~','~','~','~','~','~','~','~'}};
    }
    else {
        //Open map file of the same name as parameter
        std::ifstream mapFile;
        mapFile.open(name);
        
        //Copy contents of map to 2D vector
        if (mapFile.is_open()) {
            std::string str;
            getline(mapFile, str);
            
            
        } //Closes if statement
        mapFile.close();
        
    } //Closes else statement
    /*!===TEST; GenMap() END===
    std::cout << "Map Gen Successful!" << std::endl;
    */
}

//===WIP===
void Map::SpawnEntities() {
    //Do not spawn entities when map is default
    if (name == "default") {
        return;
    }
    
    int randNum1 = rand() % 16;
    
    //Spawn Player
    for(size_t i = 0; i < map.size(); i++) { //Row size
        bool placedPlayer = false;
        for(size_t k = 0; k < map.at(i).size(); k++) { //Column size
            int thisRandNum = rand() % 16;
            if((thisRandNum == randNum1) && (map[i][k] == '*')) {
                map[i][k] = '@';
                placedPlayer = true;
                break;
            }
        }
        if(placedPlayer == true) {
            break;
        }
    }
    
    //Spawn enemies
    int mapSeverity = (rand() % 5) + 1; //Enemy # multiplier between 1-5
    int numEntities = ((rand() % 4) + 1) * mapSeverity; //# of enemies spawned
    
    std::vector<char> includeEntities;
    std::vector<int> numSpawns;
    
    //!===TEST; mapSeverity Setting===
    if(name == "default") {
        mapSeverity = 1;
        numEntities = 3;
    }
    
    switch(mapSeverity) {
        case 5:
        case 4:
            includeEntities.push_back('C');
        case 3:
            includeEntities.push_back('c');
        case 2:
        case 1:
            includeEntities.push_back('b');
    }
    
    int randNum2 = rand() % 16;
    
    for(size_t i = 0; i < map.size(); i++) { //Row
        for(size_t k = 0; k < map.at(i).size(); k++) { //Column
            int thisRandNum = rand() % 16;
            if((thisRandNum == randNum2) && (map[i][k] == '*')) {
                
                    /*!===Finish Coding Entitiy Spawner===
                    //Needs: - Spawn number of each entity
                    //         using int numEntities
                    */
                    
            }
        }
    }
}

void Map::Print() const {
    for(size_t i = 0; i < map.size(); i++) {
        for(size_t k = 0; k < map.at(i).size(); k++) {
            std::cout << map[i][k];
        }
        std::cout << std::endl;
    }
}               

Вот несколько вещей, которые могут помочь вам улучшить ваш код.

Используйте соответствующий #includes

Код использует std::string но использует #include <cstring>. Это ошибка, потому что std::string в <string> а не в устаревшем C <cstring> заголовок. По аналогии, main не нужен и не должен #include <vector> потому что ничего из этого заголовка там не используется.

Обозначьте преднамеренное падение

Код включает этот переключатель:

switch(mapSeverity) {
    case 5:
    case 4:
            includeEntities.push_back('C');
    case 3:
            includeEntities.push_back('c');
    case 2:
    case 1:
            includeEntities.push_back('b');
}

Это имеет ряд провалов, когда выполнение продолжается из предыдущих случаев. Неясно, намеренно это или нет. Если их нет, это ошибка, и вам нужно вставить break; заявления. Если это так, и если ваш компилятор совместим с C ++ 17, вы должны использовать [[fallthrough]] атрибут. По крайней мере, комментарий был бы полезен.

Подумайте об использовании лучшего генератора случайных чисел

Код содержит несколько попыток генерации случайных чисел, которые выглядят так:

int mapSeverity = (rand() % 5) + 1; //Enemy # multiplier between 1-5
int numEntities = ((rand() % 4) + 1) * mapSeverity; //# of enemies spawned

Если вы используете компилятор, который поддерживает как минимум C ++ 11, подумайте об использовании лучшего генератора случайных чисел. В частности, вместо rand, возможно, вы захотите взглянуть на std::uniform_int_distribution и друзья в <random> заголовок.

Не использовать std::endl если тебе это действительно не нужно

Разница между std::endl и 'n' в том, что 'n' просто испускает символ новой строки, а std::endl фактически сбрасывает поток. Это может занять много времени в программе с большим количеством операций ввода-вывода и на самом деле редко требуется. Лучше всего Только использовать std::endl когда у вас есть веская причина для очистки потока, а это не очень часто требуется для простых программ, таких как эта. Избегайте привычки использовать std::endl когда 'n' будет делать, принесет дивиденды в будущем, поскольку вы будете писать более сложные программы с большим количеством операций ввода-вывода и в которых необходимо максимизировать производительность.

Не зацикливайтесь eof()

Код в настоящее время содержит это в Game.cpp:

void Game::Intro() {
        //Opens text file "intro.txt"
        std::ifstream intro;
        intro.open("intro.txt");
        
        //Outputs text from file
        if(intro.is_open()) {
                std::string str;
                //While file has not reached the end
                while(!(intro.eof())) {
                        getline(intro, str);
                        std::cout << str << std::endl;
                }
                std::cout << std::endl << std::endl;
                intro.close();
        }
}

Зацикливание почти всегда неверно eof() при чтении файла. Причина в том, что eof индикация устанавливается только тогда, когда делается попытка прочитать что-то из файла, когда мы уже в конце. Видеть этот вопрос для более подробной информации о том, зачем использовать eof обычно ошибается.

Вот как я бы переписал эту функцию:

void Game::Intro() {
    std::ifstream intro{"intro.txt"};
    std::string str;
    while (std::getline(intro, str)) {
        std::cout << str << 'n';
    }
    std::cout << "nn";
}

Еще лучше:

void Game::Intro() {
    std::ifstream intro{"intro.txt"};
    std::cout << intro.rdbuf() << "nn";
}

Не пишите геттеры и сеттеры для каждого класса

C ++ — это не Java, и писать функции получения и установки для каждого класса C ++ — не лучший стиль. Вместо этого переместите функции сеттера в конструкторы и очень внимательно подумайте, нужен ли вообще геттер. В этом коде Character класс завален таким кодом:

void SetHp(int hp) {this->hp = hp;}
int GetHp() const {return hp;}

Если что-то может изменить значение, значит, оно не является инвариантом, поэтому наличие этих функций не дает никаких преимуществ. Просто объявите такие элементы данных public вместо. Видеть C.131 для подробностей.

Предпочитайте инициализаторы членов класса конструкторам

В Character class имеет эти три конструктора:

Character() : name("Default"), hp(6), mp(6), str(6), def(6), mag(6), spd(6) {};
Character(std::string name) : hp(12), mp(6), str(6), def(6), mag(6), spd(6) {this->name = name;}
Character(std::string name, int hp, int mp, int str, int def, int mag, int spd, char symbol) { 
    this->name = name;
    this->hp = hp * 3;
    this->mp = mp;
    // etc. 
}

Здесь можно многое улучшить. В частности, простая установка значений для каждого элемента данных в объявлении делает это намного более читаемым. Видеть C.45 для подробностей. Кстати, зачем вам утроить только полученное значение для hp? И почему у вас есть два разных значения по умолчанию для hp?

Избегайте путаницы в именах файлов

Есть несколько файлов, которые отличаются только одной буквой. Есть Character.h и Characters.h и аналогичная ситуация для Map.h и Maps.h. Было бы немного добрее по отношению к вашим товарищам-программистам (и к вам самому!), Если бы вы переименовали их, чтобы путаница была менее вероятной. Я бы предложил CharacterCollection Например.

Не возвращать указатели на private данные

В Player класс в настоящее время содержит это:

class Player {
public:
    Player() {p = &player; lv = 1; player.SetSym('@');}
    // other stuff
    Character* p;
private:
    Character player;
    int lv;
};

В результате у нас есть публичный указатель. p к личным данным player! Это плохо, потому что это означает, что любой код может проникнуть в якобы личные данные внутри класса и вмешаться в них, что является нарушением скрытие информации принцип. Видеть C.9 для подробностей.

Основная проблема, по-видимому, заключается в некоторой путанице в отношении структуры классов. Если Player это Character, что, по-видимому, и является целью здесь, то лучший способ сделать это — иметь Character быть базовым классом для Player. Вот как это выглядит после частичного рефакторинга вашего кода:

// the Player class is derived from the Character class
class Player : public Character {
public:
    Player() : Character() { symbol="@"; }
    int GetLv() const;
    void PlayerTurn();
    void Defend();
    void OpenInventory();
    void RunAway();
    void CharacterCreation();
private:
    int lv = 1;
};

Здесь нужно отметить пару моментов. Сначала я оставил GetLv() функция, но сделал это const. Во-вторых, я удалил Battle функция, потому что базовый класс Character уже реализует то, что нам нужно. В-третьих, потому что я удалил все тривиальные геттеры и сеттеры из Character класс, symbol устанавливается непосредственно в конструкторе.

Упростите свой код

В Battle() функция-член Character класс намного сложнее, чем должен быть, особенно если вы примете предыдущий совет и Player быть производным классом Character. Вот переписанный Battle() функция:

void Character::Battle(Character *opponent) {
    Character *attacker = this;
    Character *defender = opponent;
    // For the first round, the faster Character is attacker
    // Thereafter, they alternate
    if (attacker->spd < opponent->spd) {
        std::swap(attacker, defender);
    }
    while (defender->IsAlive()) {
        int damage = defender->TakeDamage(attacker);
        std::cout << attacker->name << " hits " << defender->name
                          << " for " << damage << " points.n";
        std::swap(attacker, defender);
        // always report the player status
        std::cout << name << " has " << hp << " HP remaining.n"
                  << opponent->name << " has " << opponent->hp 
                  << " hp remaining.nn";
    }
    // somebody is dead -- who was it?
    if (IsAlive()) {
        std::cout << opponent->name << " was slain.n";
    } else {
        std::cout << name << " was cut down.n";
    }
}

Понять перегрузку

Код в настоящее время содержит эту функцию-член:

void Characters::AddCharArch(Character _char, std::vector<Character> _chars = {{"null"}}) {
    if (_chars.at(0).GetName() == "null") {
        //Copies character data _char
        archList.push_back(_char);
    }
    else {
        //Copies data from character vector _chars
        for(unsigned int i = 0; i < _chars.size(); i++) {
            archList.push_back(_chars.at(i));
        }
    }
}

Здесь происходит много всего, что действительно требует рефакторинга! Во-первых, похоже, что вы собираетесь разрешить добавление одного символа или их коллекции к внутреннему archList. Если это так, сделайте две функции. Вот как бы я это сделал:

void Characters::AddCharArch(Character ch) {
    archList.emplace_back(ch);
}

void Characters::AddCharArch(const std::vector<Character>& chars) {
    archList.insert(archList.end(), chars.begin(), chars.end());
}

Также обратите внимание, что мы используем emplace_back для первой версии с ch уже передано по значению, и мы используем insert добавить список пропущенных символов в конец archList.