diff --git a/src/fileio/fileio_party.cpp b/src/fileio/fileio_party.cpp index 01919f0a..6cf46b07 100644 --- a/src/fileio/fileio_party.cpp +++ b/src/fileio/fileio_party.cpp @@ -299,8 +299,6 @@ bool load_party_v2(fs::path file_to_load, cUniverse& real_univ){ return false; } univ.party[i].readFrom(fin); - // Do this to make sure the PC's internal party reference is correct - univ.party[i].join_party(univ.party); } // Including stored PCs @@ -309,9 +307,8 @@ bool load_party_v2(fs::path file_to_load, cUniverse& real_univ){ long next_uid; while(fin >> next_uid) { std::string fname = "save/pc~" + std::to_string(next_uid) + ".txt"; - cPlayer* stored_pc = new cPlayer(univ.party); + cPlayer* stored_pc = new cPlayer(no_party); stored_pc->readFrom(partyIn.getFile(fname)); - univ.stored_pcs[next_uid] = stored_pc->leave_party(); } } @@ -427,7 +424,7 @@ bool save_party(fs::path dest_file, const cUniverse& univ) { // And stored PCs if(univ.stored_pcs.size()) { std::ostream& fout = partyOut.newFile("save/stored_pcs.txt"); - for(auto p : univ.stored_pcs) { + for(const auto& p : univ.stored_pcs) { std::string fname = "save/pc~" + std::to_string(p.first) + ".txt"; p.second->writeTo(partyOut.newFile(fname)); fout << p.first << '\n'; diff --git a/src/fileio/fileio_scen.cpp b/src/fileio/fileio_scen.cpp index 2cc4ddf8..99fe6f17 100644 --- a/src/fileio/fileio_scen.cpp +++ b/src/fileio/fileio_scen.cpp @@ -581,7 +581,7 @@ static void readShopFromXml(ticpp::Element& data, cShop& shop) { entry->GetText(&num); shop.addItem(num, dummy_item, amount, chance); } else if(type == "special") { - int amount, node, cost = 0, icon; + int amount = 0, node = 0, cost = 0, icon = 0; std::string title, descr; std::set reqs = {"quantity", "node", "icon", "name", "description"}; Iterator attr; @@ -609,7 +609,7 @@ static void readShopFromXml(ticpp::Element& data, cShop& shop) { throw xMissingElem("special", *reqs.begin(), entry->Row(), entry->Column(), fname); shop.addSpecial(title, descr, icon, node, cost, amount); } else { - eShopItemType itype; + eShopItemType itype{}; int n = 0; if(type == "mage-spell") { itype = eShopItemType::MAGE_SPELL; diff --git a/src/game/boe.specials.cpp b/src/game/boe.specials.cpp index 1b38ef3b..cc07dddc 100644 --- a/src/game/boe.specials.cpp +++ b/src/game/boe.specials.cpp @@ -2705,7 +2705,7 @@ void affect_spec(const runtime_state& ctx) { for(auto& p : univ.stored_pcs) { if(p.first == pc && p.second->unique_id == pc) { can_pick = true; - found = p.second; + found = p.second.get(); break; } } @@ -3174,7 +3174,7 @@ void affect_spec(const runtime_state& ctx) { univ.party.stuff_done[spec.sd1][spec.sd2] = univ.party[pc_num].unique_id - 1000; if(spec.ex1a == 1) break; who->main_status += eMainStatus::SPLIT; - univ.stored_pcs[who->unique_id] = who->leave_party(); + univ.stored_pcs[who->unique_id] = univ.party.remove_pc(pc_num); univ.party.new_pc(pc_num); } break; @@ -3191,7 +3191,7 @@ void affect_spec(const runtime_state& ctx) { check_mess = false; break; } - univ.party.replace_pc(pc_num, univ.stored_pcs[spec.ex1a]); + univ.party.replace_pc(pc_num, std::move(univ.stored_pcs[spec.ex1a])); ctx.cur_target = &univ.get_target(pc_num); univ.party[pc_num].main_status -= eMainStatus::SPLIT; univ.stored_pcs.erase(spec.ex1a); diff --git a/src/gfx/render_text.hpp b/src/gfx/render_text.hpp index 0671c891..24ce3806 100644 --- a/src/gfx/render_text.hpp +++ b/src/gfx/render_text.hpp @@ -31,7 +31,7 @@ enum eFont { struct TextStyle { bool italic = false, underline = false; eFont font = FONT_BOLD; - int pointSize = 10, lineHeight; + int pointSize = 10, lineHeight = 10; sf::Color colour; TextStyle() : colour(sf::Color::Black) {} void applyTo(sf::Text& text); diff --git a/src/pcedit/pc.editors.cpp b/src/pcedit/pc.editors.cpp index b5ec7491..1e2e9985 100644 --- a/src/pcedit/pc.editors.cpp +++ b/src/pcedit/pc.editors.cpp @@ -166,6 +166,7 @@ static bool pick_race_select_led(cDialog& me, std::string item_hit, bool, const case 1: race = eRace::NEPHIL; break; case 2: race = eRace::SLITH; break; case 3: race = eRace::VAHNATAI; break; + default: return false; } if(store_trait_mode == 0) pc->race = race; diff --git a/src/universe/party.cpp b/src/universe/party.cpp index 9dd36778..05e60666 100644 --- a/src/universe/party.cpp +++ b/src/universe/party.cpp @@ -40,18 +40,11 @@ cParty::cParty(ePartyPreset party_preset) { for(int i = 0; i < 10; i++) out_c[i].exists = false; for(int i = 0; i < 6; i++) - adven[i] = new cPlayer(*this, party_preset, i); + adven[i].reset(new cPlayer(*this, party_preset, i)); for(auto& monst : imprisoned_monst) monst = 0; } -cParty::~cParty() { - for(int i = 0; i < 6; i++) { - delete adven[i]; - adven[i] = nullptr; - } -} - cParty::cParty(const cParty& other) : iLiving(other) , next_pc_id(other.next_pc_id) @@ -109,8 +102,7 @@ cParty::cParty(const cParty& other) memcpy(stuff_done, other.stuff_done, sizeof(stuff_done)); memcpy(setup, other.setup, sizeof(setup)); for(int i = 0; i < 6; i++) { - adven[i] = new cPlayer(*other.adven[i]); - adven[i]->join_party(*this); + adven[i].reset(new cPlayer(*this, *other.adven[i])); } } @@ -184,12 +176,8 @@ void cParty::swap(cParty& other) { memcpy(temp_setup, setup, sizeof(setup)); memcpy(setup, other.setup, sizeof(setup)); memcpy(other.setup, temp_setup, sizeof(setup)); - // Fixup the references of PCs to their party for(size_t i = 0; i < adven.size(); i++) { - adven[i]->join_party(*this); - } - for(size_t i = 0; i < other.adven.size(); i++) { - other.adven[i]->join_party(other); + std::swap(adven[i], other.adven[i]); } } @@ -351,22 +339,27 @@ void cParty::cEncNote::import_legacy(int16_t(& old)[2], const cScenario& scenari void cParty::import_legacy(legacy::pc_record_type(& old)[6]) { for(int i = 0; i < 6; i++) { - delete adven[i]; - adven[i] = new cPlayer(*this); + adven[i].reset(new cPlayer(*this)); adven[i]->import_legacy(old[i]); } } +std::unique_ptr cParty::remove_pc(size_t spot) { + if(spot >= 6) return nullptr; + adven[spot]->party = nullptr; + return std::move(adven[spot]); +} + void cParty::new_pc(size_t spot) { - replace_pc(spot, new cPlayer(*this)); + std::unique_ptr new_pc{new cPlayer(*this)}; + replace_pc(spot, std::move(new_pc)); adven[spot]->main_status = eMainStatus::ALIVE; } -void cParty::replace_pc(size_t spot, cPlayer* with) { +void cParty::replace_pc(size_t spot, std::unique_ptr with) { if(spot < 6 && with != nullptr) { - with->join_party(*this); - delete adven[spot]; - adven[spot] = with; + with->party = this; + adven[spot] = std::move(with); } } diff --git a/src/universe/party.hpp b/src/universe/party.hpp index e5d31c9c..a07679d1 100644 --- a/src/universe/party.hpp +++ b/src/universe/party.hpp @@ -123,7 +123,7 @@ public: long long total_m_killed, total_dam_done, total_xp_gained, total_dam_taken; std::string scen_name; private: - std::array adven; + std::array,6> adven; public: unsigned short setup[4][64][64]; // formerly setup_save_type std::array,3> stored_items; // formerly stored_items_list_type @@ -178,8 +178,9 @@ public: int get_level() const; int calc_day() const; + std::unique_ptr remove_pc(size_t spot); void new_pc(size_t spot); - void replace_pc(size_t spot, cPlayer* with); + void replace_pc(size_t spot, std::unique_ptr with); size_t free_space(); size_t count(eMainStatus type = eMainStatus::ALIVE); void void_pcs(); @@ -219,7 +220,6 @@ public: typedef std::vector::iterator journalIter; typedef std::vector::iterator talkIter; cParty(ePartyPreset party_preset = PARTY_DEFAULT); - ~cParty(); // Copy-and-swap void swap(cParty& other); cParty(const cParty& other); diff --git a/src/universe/pc.cpp b/src/universe/pc.cpp index 47e78123..003877a8 100644 --- a/src/universe/pc.cpp +++ b/src/universe/pc.cpp @@ -943,7 +943,12 @@ void cPlayer::finish_create() { } } -cPlayer::cPlayer(cParty& party) : party(&party), weap_poisoned(*this) { +cPlayer::cPlayer(cParty& party) : cPlayer(no_party) { + this->party = &party; + unique_id = party.next_pc_id++; +} + +cPlayer::cPlayer(no_party_t) : weap_poisoned(*this) { main_status = eMainStatus::ABSENT; name = "\n"; @@ -967,8 +972,6 @@ cPlayer::cPlayer(cParty& party) : party(&party), weap_poisoned(*this) { race = eRace::HUMAN; direction = DIR_N; combat_pos = {-1,-1}; - - unique_id = party.next_pc_id++; } cPlayer::cPlayer(cParty& party,ePartyPreset key,short slot) : cPlayer(party) { @@ -1115,9 +1118,13 @@ cPlayer::cPlayer(cParty& party,ePartyPreset key,short slot) : cPlayer(party) { } } -cPlayer::cPlayer(const cPlayer& other) +cPlayer::cPlayer(cParty& party, const cPlayer& other) : cPlayer(no_party, other) { + this->party = &party; + unique_id = party.next_pc_id++; +} + +cPlayer::cPlayer(no_party_t, const cPlayer& other) : iLiving(other) - , party(other.party) , main_status(other.main_status) , name(other.name) , skills(other.skills) @@ -1136,7 +1143,6 @@ cPlayer::cPlayer(const cPlayer& other) , weap_poisoned(*this, other.weap_poisoned.slot) , traits(other.traits) , race(other.race) - , unique_id(party->next_pc_id++) , last_cast(other.last_cast) , combat_pos(other.combat_pos) , parry(other.parry) @@ -1148,7 +1154,7 @@ cPlayer::cPlayer(cPlayer&& other) : weap_poisoned(*this, other.weap_poisoned.slo } void cPlayer::swap(cPlayer& other) { - std::swap(party, other.party); + // Don't swap the party reference! std::swap(main_status, other.main_status); std::swap(name, other.name); std::swap(skills, other.skills); diff --git a/src/universe/pc.hpp b/src/universe/pc.hpp index aacfbc23..59ea7cc8 100644 --- a/src/universe/pc.hpp +++ b/src/universe/pc.hpp @@ -26,6 +26,8 @@ namespace legacy { struct pc_record_type; }; +static struct no_party_t {} no_party; + enum class eBuyStatus {OK, NO_SPACE, NEED_GOLD, TOO_HEAVY, HAVE_LOTS}; enum ePartyPreset {PARTY_BLANK, PARTY_DEFAULT, PARTY_DEBUG}; @@ -67,9 +69,13 @@ inline bool operator!=(const cInvenSlot& a, const cInvenSlot& b) { } class cPlayer : public iLiving { - cParty* party; + cParty* party = nullptr; template cInvenSlot find_item_matching(Fcn fcn); + // Allow the party to overwrite the PC's party reference + // It only does this (and should ever do this) in remove_pc and replace_pc! + // Sadly there is no good way to friend just those two functions. + friend class cParty; static const int INVENTORY_SIZE = 24; public: // A nice convenient bitset with just the low 30 bits set, for initializing spells @@ -166,19 +172,19 @@ public: short stat_adj(eSkill skill) const; eBuyStatus ok_to_buy(short cost,cItem item) const; - void join_party(cParty& p) {party = &p;} - cPlayer* leave_party() {party = nullptr; return this;} - void import_legacy(legacy::pc_record_type old); + cPlayer(no_party_t); cPlayer(cParty& party); cPlayer(cParty& party,ePartyPreset key,short slot); + cPlayer(no_party_t, const cPlayer& other); + cPlayer(cParty& party, const cPlayer& other); short get_tnl(); void writeTo(std::ostream& file) const; void readFrom(std::istream& file); virtual ~cPlayer() = default; // Copy-and-swap void swap(cPlayer& other); - cPlayer(const cPlayer& other); + cPlayer(const cPlayer& other) = delete; cPlayer(cPlayer&& other); // For now, not assignable because of an issue of how to handle the unique_id cPlayer& operator=(cPlayer other) = delete; diff --git a/src/universe/universe.cpp b/src/universe/universe.cpp index e48c1276..feab380a 100644 --- a/src/universe/universe.cpp +++ b/src/universe/universe.cpp @@ -912,7 +912,6 @@ cUniverse::cUniverse(const cUniverse& other) , cur_pc(other.cur_pc) , scenario(other.scenario) , party(other.party) - , stored_pcs(other.stored_pcs) , town(*this) , out(*this) , file(other.file) @@ -920,11 +919,12 @@ cUniverse::cUniverse(const cUniverse& other) , ghost_mode(other.ghost_mode) , node_step_through(other.node_step_through) { - for(auto& p : stored_pcs) { - p.second = new cPlayer(*p.second); - } town.copy(other.town); out.copy(other.out); + for(const auto& pc : other.stored_pcs) { + std::unique_ptr clone(new cPlayer(no_party, *pc.second)); + stored_pcs.emplace(pc.first, std::move(clone)); + } } cUniverse::cUniverse(cUniverse&& other) : town(*this), out(*this) { @@ -1305,13 +1305,7 @@ short cUniverse::difficulty_adjust() const { return adj; } -cUniverse::~cUniverse() { - clear_stored_pcs(); -} - void cUniverse::clear_stored_pcs() { - for(auto& p : stored_pcs) - delete p.second; stored_pcs.clear(); } diff --git a/src/universe/universe.hpp b/src/universe/universe.hpp index c3316cfd..15de3941 100644 --- a/src/universe/universe.hpp +++ b/src/universe/universe.hpp @@ -198,7 +198,7 @@ public: cScenario scenario; cParty party; - std::map stored_pcs; + std::map> stored_pcs; cCurTown town; cCurOut out; fs::path file; @@ -214,7 +214,6 @@ public: void generate_job_bank(int which, job_bank_t& bank); short difficulty_adjust() const; explicit cUniverse(ePartyPreset party_type = PARTY_DEFAULT); - ~cUniverse(); // Copy-and-swap void swap(cUniverse& other); cUniverse(const cUniverse& other); diff --git a/test/pc_read.cpp b/test/pc_read.cpp index 004d0c28..6576a49c 100644 --- a/test/pc_read.cpp +++ b/test/pc_read.cpp @@ -16,9 +16,7 @@ using namespace std; TEST_CASE("Loading player character from file") { ifstream fin; fin.exceptions(ios::badbit); - cParty party; - cPlayer pc(party); - pc.leave_party(); + cPlayer pc(no_party); // Fill in some junk data pc.cur_sp = 27; pc.max_sp = 38;