From 64bc20bf35de3207d7dac16de2b46497688e6b8c Mon Sep 17 00:00:00 2001 From: Shauren Date: Mon, 7 Sep 2020 21:36:13 +0200 Subject: [PATCH] Core/Players: Directly store PlayerSpell in m_spells, not as pointer --- .../game/Achievements/AchievementMgr.cpp | 12 +- src/server/game/Entities/Player/Player.cpp | 157 +++++++++--------- src/server/game/Entities/Player/Player.h | 2 +- src/server/game/Entities/Unit/Unit.cpp | 2 +- .../game/Spells/Auras/SpellAuraEffects.cpp | 4 +- src/server/game/Spells/SpellHistory.cpp | 2 +- 6 files changed, 83 insertions(+), 96 deletions(-) diff --git a/src/server/game/Achievements/AchievementMgr.cpp b/src/server/game/Achievements/AchievementMgr.cpp index b5e93f1a68a..6eb48e147fa 100644 --- a/src/server/game/Achievements/AchievementMgr.cpp +++ b/src/server/game/Achievements/AchievementMgr.cpp @@ -1366,11 +1366,9 @@ void AchievementMgr::UpdateAchievementCriteria(AchievementCriteriaTypes type, case ACHIEVEMENT_CRITERIA_TYPE_LEARN_SKILLLINE_SPELLS: { uint32 spellCount = 0; - for (PlayerSpellMap::const_iterator spellIter = referencePlayer->GetSpellMap().begin(); - spellIter != referencePlayer->GetSpellMap().end(); - ++spellIter) + for (std::pair const& spellIter : referencePlayer->GetSpellMap()) { - SkillLineAbilityMapBounds bounds = sSpellMgr->GetSkillLineAbilityMapBounds(spellIter->first); + SkillLineAbilityMapBounds bounds = sSpellMgr->GetSkillLineAbilityMapBounds(spellIter.first); for (SkillLineAbilityMap::const_iterator skillIter = bounds.first; skillIter != bounds.second; ++skillIter) { if (skillIter->second->SkillLine == achievementCriteria->learn_skillline_spell.skillLine) @@ -1392,11 +1390,9 @@ void AchievementMgr::UpdateAchievementCriteria(AchievementCriteriaTypes type, case ACHIEVEMENT_CRITERIA_TYPE_LEARN_SKILL_LINE: { uint32 spellCount = 0; - for (PlayerSpellMap::const_iterator spellIter = referencePlayer->GetSpellMap().begin(); - spellIter != referencePlayer->GetSpellMap().end(); - ++spellIter) + for (std::pair const& spellIter : referencePlayer->GetSpellMap()) { - SkillLineAbilityMapBounds bounds = sSpellMgr->GetSkillLineAbilityMapBounds(spellIter->first); + SkillLineAbilityMapBounds bounds = sSpellMgr->GetSkillLineAbilityMapBounds(spellIter.first); for (SkillLineAbilityMap::const_iterator skillIter = bounds.first; skillIter != bounds.second; ++skillIter) if (skillIter->second->SkillLine == achievementCriteria->learn_skill_line.skillLine) spellCount++; diff --git a/src/server/game/Entities/Player/Player.cpp b/src/server/game/Entities/Player/Player.cpp index 4a3852e250d..6f9d07a9e5d 100644 --- a/src/server/game/Entities/Player/Player.cpp +++ b/src/server/game/Entities/Player/Player.cpp @@ -382,9 +382,6 @@ Player::~Player() for (uint8 i = 0; i < PLAYER_SLOTS_COUNT; ++i) delete m_items[i]; - for (PlayerSpellMap::const_iterator itr = m_spells.begin(); itr != m_spells.end(); ++itr) - delete itr->second; - delete _talentMgr; //all mailed items should be deleted, also all mail should be deallocated @@ -2633,10 +2630,10 @@ void Player::SendInitialSpells() for (PlayerSpellMap::const_iterator itr = m_spells.begin(); itr != m_spells.end(); ++itr) { - if (itr->second->state == PLAYERSPELL_REMOVED) + if (itr->second.state == PLAYERSPELL_REMOVED) continue; - if (!itr->second->active || itr->second->disabled) + if (!itr->second.active || itr->second.disabled) continue; data << uint32(itr->first); @@ -2662,10 +2659,10 @@ void Player::SendUnlearnSpells() for (PlayerSpellMap::const_iterator itr = m_spells.begin(); itr != m_spells.end(); ++itr) { - if (itr->second->state == PLAYERSPELL_REMOVED) + if (itr->second.state == PLAYERSPELL_REMOVED) continue; - if (itr->second->active || itr->second->disabled) + if (itr->second.active || itr->second.disabled) continue; auto skillLineAbilities = sSpellMgr->GetSkillLineAbilityMapBounds(itr->first); @@ -2913,7 +2910,7 @@ bool Player::AddSpell(uint32 spellId, bool active, bool learning, bool dependent PlayerSpellMap::iterator itr = m_spells.find(spellId); // Remove temporary spell if found to prevent conflicts - if (itr != m_spells.end() && itr->second->state == PLAYERSPELL_TEMPORARY) + if (itr != m_spells.end() && itr->second.state == PLAYERSPELL_TEMPORARY) RemoveTemporarySpell(spellId); else if (itr != m_spells.end()) { @@ -2933,33 +2930,33 @@ bool Player::AddSpell(uint32 spellId, bool active, bool learning, bool dependent } // not do anything if already known in expected state - if (itr->second->state != PLAYERSPELL_REMOVED && itr->second->active == active && - itr->second->dependent == dependent && itr->second->disabled == disabled) + if (itr->second.state != PLAYERSPELL_REMOVED && itr->second.active == active && + itr->second.dependent == dependent && itr->second.disabled == disabled) { if (!IsInWorld() && !learning) // explicitly load from DB and then exist in it already and set correctly - itr->second->state = PLAYERSPELL_UNCHANGED; + itr->second.state = PLAYERSPELL_UNCHANGED; return false; } // dependent spell known as not dependent, overwrite state - if (itr->second->state != PLAYERSPELL_REMOVED && !itr->second->dependent && dependent) + if (itr->second.state != PLAYERSPELL_REMOVED && !itr->second.dependent && dependent) { - itr->second->dependent = dependent; - if (itr->second->state != PLAYERSPELL_NEW) - itr->second->state = PLAYERSPELL_CHANGED; + itr->second.dependent = dependent; + if (itr->second.state != PLAYERSPELL_NEW) + itr->second.state = PLAYERSPELL_CHANGED; dependent_set = true; } // update active state for known spell - if (itr->second->active != active && itr->second->state != PLAYERSPELL_REMOVED && !itr->second->disabled) + if (itr->second.active != active && itr->second.state != PLAYERSPELL_REMOVED && !itr->second.disabled) { - itr->second->active = active; + itr->second.active = active; if (!IsInWorld() && !learning && !dependent_set) // explicitly load from DB and then exist in it already and set correctly - itr->second->state = PLAYERSPELL_UNCHANGED; - else if (itr->second->state != PLAYERSPELL_NEW) - itr->second->state = PLAYERSPELL_CHANGED; + itr->second.state = PLAYERSPELL_UNCHANGED; + else if (itr->second.state != PLAYERSPELL_NEW) + itr->second.state = PLAYERSPELL_CHANGED; if (active) { @@ -2983,24 +2980,23 @@ bool Player::AddSpell(uint32 spellId, bool active, bool learning, bool dependent return active; // learn (show in spell book if active now) } - if (itr->second->disabled != disabled && itr->second->state != PLAYERSPELL_REMOVED) + if (itr->second.disabled != disabled && itr->second.state != PLAYERSPELL_REMOVED) { - if (itr->second->state != PLAYERSPELL_NEW) - itr->second->state = PLAYERSPELL_CHANGED; - itr->second->disabled = disabled; + if (itr->second.state != PLAYERSPELL_NEW) + itr->second.state = PLAYERSPELL_CHANGED; + itr->second.disabled = disabled; if (disabled) return false; disabled_case = true; } - else switch (itr->second->state) + else switch (itr->second.state) { case PLAYERSPELL_UNCHANGED: // known saved spell return false; case PLAYERSPELL_REMOVED: // re-learning removed not saved spell { - delete itr->second; m_spells.erase(itr); state = PLAYERSPELL_CHANGED; break; // need re-add @@ -3009,7 +3005,7 @@ bool Player::AddSpell(uint32 spellId, bool active, bool learning, bool dependent { // can be in case spell loading but learned at some previous spell loading if (!IsInWorld() && !learning && !dependent_set) - itr->second->state = PLAYERSPELL_UNCHANGED; + itr->second.state = PLAYERSPELL_UNCHANGED; return false; } @@ -3043,20 +3039,23 @@ bool Player::AddSpell(uint32 spellId, bool active, bool learning, bool dependent LearnSpell(prev_spell, true, fromSkill); } - PlayerSpell* newspell = new PlayerSpell; - newspell->state = state; - newspell->active = active; - newspell->dependent = dependent; - newspell->disabled = disabled; + std::pair inserted = m_spells.emplace(std::piecewise_construct, std::forward_as_tuple(spellId), std::forward_as_tuple()); + PlayerSpell& newspell = inserted.first->second; + // learning a previous rank might have given us this spell already from a skill autolearn, most likely with PLAYERSPELL_NEW state + // we dont want to do double insert if this happened during load from db so we force state to CHANGED, just in case + newspell.state = inserted.second ? state : PLAYERSPELL_CHANGED; + newspell.active = active; + newspell.dependent = dependent; + newspell.disabled = disabled; bool needsUnlearnSpellsPacket = false; // replace spells in action bars and spellbook to bigger rank if only one spell rank must be accessible - if (newspell->active && !newspell->disabled && !spellInfo->IsStackableWithRanks() && spellInfo->IsRanked()) + if (newspell.active && !newspell.disabled && !spellInfo->IsStackableWithRanks() && spellInfo->IsRanked()) { for (PlayerSpellMap::iterator itr2 = m_spells.begin(); itr2 != m_spells.end(); ++itr2) { - if (itr2->second->state == PLAYERSPELL_REMOVED) + if (itr2->second.state == PLAYERSPELL_REMOVED) continue; SpellInfo const* i_spellInfo = sSpellMgr->GetSpellInfo(itr2->first); @@ -3065,7 +3064,7 @@ bool Player::AddSpell(uint32 spellId, bool active, bool learning, bool dependent if (spellInfo->IsDifferentRankOf(i_spellInfo)) { - if (itr2->second->active) + if (itr2->second.active) { if (spellInfo->IsHighRankOf(i_spellInfo)) { @@ -3076,9 +3075,9 @@ bool Player::AddSpell(uint32 spellId, bool active, bool learning, bool dependent } // mark old spell as disable (SMSG_SUPERCEDED_SPELL replace it in client by new) - itr2->second->active = false; - if (itr2->second->state != PLAYERSPELL_NEW) - itr2->second->state = PLAYERSPELL_CHANGED; + itr2->second.active = false; + if (itr2->second.state != PLAYERSPELL_NEW) + itr2->second.state = PLAYERSPELL_CHANGED; superceded_old = true; // new spell replace old in action bars and spell book. } else @@ -3090,22 +3089,20 @@ bool Player::AddSpell(uint32 spellId, bool active, bool learning, bool dependent } // mark new spell as disable (not learned yet for client and will not learned) - newspell->active = false; - if (newspell->state != PLAYERSPELL_NEW) - newspell->state = PLAYERSPELL_CHANGED; + newspell.active = false; + if (newspell.state != PLAYERSPELL_NEW) + newspell.state = PLAYERSPELL_CHANGED; } } } } } - m_spells[spellId] = newspell; - if (needsUnlearnSpellsPacket) SendUnlearnSpells(); // return false if spell disabled - if (newspell->disabled) + if (newspell.disabled) return false; } @@ -3219,12 +3216,11 @@ void Player::AddTemporarySpell(uint32 spellId) // spell already added - do not do anything if (itr != m_spells.end()) return; - PlayerSpell* newspell = new PlayerSpell; + PlayerSpell* newspell = &m_spells[spellId]; newspell->state = PLAYERSPELL_TEMPORARY; newspell->active = true; newspell->dependent = false; newspell->disabled = false; - m_spells[spellId] = newspell; } void Player::RemoveTemporarySpell(uint32 spellId) @@ -3234,9 +3230,8 @@ void Player::RemoveTemporarySpell(uint32 spellId) if (itr == m_spells.end()) return; // spell has other state than temporary - do not change it - if (itr->second->state != PLAYERSPELL_TEMPORARY) + if (itr->second.state != PLAYERSPELL_TEMPORARY) return; - delete itr->second; m_spells.erase(itr); } @@ -3282,8 +3277,8 @@ void Player::LearnSpell(uint32 spell_id, bool dependent, uint32 fromSkill /*= 0* { PlayerSpellMap::iterator itr = m_spells.find(spell_id); - bool disabled = (itr != m_spells.end()) ? itr->second->disabled : false; - bool active = disabled ? itr->second->active : true; + bool disabled = (itr != m_spells.end()) ? itr->second.disabled : false; + bool active = disabled ? itr->second.active : true; bool learning = AddSpell(spell_id, active, true, dependent, false, false, fromSkill); @@ -3302,7 +3297,7 @@ void Player::LearnSpell(uint32 spell_id, bool dependent, uint32 fromSkill /*= 0* if (uint32 nextSpell = sSpellMgr->GetNextSpellInChain(spell_id)) { PlayerSpellMap::iterator iter = m_spells.find(nextSpell); - if (iter != m_spells.end() && iter->second->disabled) + if (iter != m_spells.end() && iter->second.disabled) LearnSpell(nextSpell, false, fromSkill); } @@ -3310,7 +3305,7 @@ void Player::LearnSpell(uint32 spell_id, bool dependent, uint32 fromSkill /*= 0* for (SpellsRequiringSpellMap::const_iterator itr2 = spellsRequiringSpell.first; itr2 != spellsRequiringSpell.second; ++itr2) { PlayerSpellMap::iterator iter2 = m_spells.find(itr2->second); - if (iter2 != m_spells.end() && iter2->second->disabled) + if (iter2 != m_spells.end() && iter2->second.disabled) LearnSpell(itr2->second, false, fromSkill); } } @@ -3328,7 +3323,7 @@ void Player::RemoveSpell(uint32 spell_id, bool disabled, bool learn_low_rank) if (itr == m_spells.end()) return; - if (itr->second->state == PLAYERSPELL_REMOVED || (disabled && itr->second->disabled) || itr->second->state == PLAYERSPELL_TEMPORARY) + if (itr->second.state == PLAYERSPELL_REMOVED || (disabled && itr->second.disabled) || itr->second.state == PLAYERSPELL_TEMPORARY) return; // unlearn non talent higher ranks (recursive) @@ -3347,26 +3342,23 @@ void Player::RemoveSpell(uint32 spell_id, bool disabled, bool learn_low_rank) if (itr == m_spells.end()) return; // already unleared - bool giveTalentPoints = disabled || !itr->second->disabled; + bool giveTalentPoints = disabled || !itr->second.disabled; - bool cur_active = itr->second->active; - bool cur_dependent = itr->second->dependent; + bool cur_active = itr->second.active; + bool cur_dependent = itr->second.dependent; if (disabled) { - itr->second->disabled = disabled; - if (itr->second->state != PLAYERSPELL_NEW) - itr->second->state = PLAYERSPELL_CHANGED; + itr->second.disabled = disabled; + if (itr->second.state != PLAYERSPELL_NEW) + itr->second.state = PLAYERSPELL_CHANGED; } else { - if (itr->second->state == PLAYERSPELL_NEW) - { - delete itr->second; + if (itr->second.state == PLAYERSPELL_NEW) m_spells.erase(itr); - } else - itr->second->state = PLAYERSPELL_REMOVED; + itr->second.state = PLAYERSPELL_REMOVED; } RemoveOwnedAura(spell_id, GetGUID()); @@ -3459,17 +3451,17 @@ void Player::RemoveSpell(uint32 spell_id, bool disabled, bool learn_low_rank) PlayerSpellMap::iterator prev_itr = m_spells.find(prev_id); if (prev_itr != m_spells.end()) { - if (prev_itr->second->dependent != cur_dependent) + if (prev_itr->second.dependent != cur_dependent) { - prev_itr->second->dependent = cur_dependent; - if (prev_itr->second->state != PLAYERSPELL_NEW) - prev_itr->second->state = PLAYERSPELL_CHANGED; + prev_itr->second.dependent = cur_dependent; + if (prev_itr->second.state != PLAYERSPELL_NEW) + prev_itr->second.state = PLAYERSPELL_CHANGED; } // now re-learn if need re-activate - if (!prev_itr->second->active && learn_low_rank) + if (!prev_itr->second.active && learn_low_rank) { - if (AddSpell(prev_id, true, false, prev_itr->second->dependent, prev_itr->second->disabled)) + if (AddSpell(prev_id, true, false, prev_itr->second.dependent, prev_itr->second.disabled)) { // downgrade spell ranks in spellbook and action bar SendSupercededSpell(spell_id, prev_id); @@ -3740,8 +3732,8 @@ void Player::DestroyForPlayer(Player* target, bool onDeath /*= false*/) const bool Player::HasSpell(uint32 spell) const { PlayerSpellMap::const_iterator itr = m_spells.find(spell); - return (itr != m_spells.end() && itr->second->state != PLAYERSPELL_REMOVED && - !itr->second->disabled); + return (itr != m_spells.end() && itr->second.state != PLAYERSPELL_REMOVED && + !itr->second.disabled); } bool Player::HasTalent(uint32 spell, uint8 spec) const @@ -3753,8 +3745,8 @@ bool Player::HasTalent(uint32 spell, uint8 spec) const bool Player::HasActiveSpell(uint32 spell) const { PlayerSpellMap::const_iterator itr = m_spells.find(spell); - return (itr != m_spells.end() && itr->second->state != PLAYERSPELL_REMOVED && - itr->second->active && !itr->second->disabled); + return (itr != m_spells.end() && itr->second.state != PLAYERSPELL_REMOVED && + itr->second.active && !itr->second.disabled); } /** @@ -7800,7 +7792,7 @@ void Player::ApplyItemDependentAuras(Item* item, bool apply) PlayerSpellMap const& spells = GetSpellMap(); for (auto itr = spells.begin(); itr != spells.end(); ++itr) { - if (itr->second->state == PLAYERSPELL_REMOVED || itr->second->disabled) + if (itr->second.state == PLAYERSPELL_REMOVED || itr->second.disabled) continue; SpellInfo const* spellInfo = sSpellMgr->GetSpellInfo(itr->first); @@ -20476,7 +20468,7 @@ void Player::_SaveSpells(CharacterDatabaseTransaction& trans) for (PlayerSpellMap::iterator itr = m_spells.begin(); itr != m_spells.end();) { - if (itr->second->state == PLAYERSPELL_REMOVED || itr->second->state == PLAYERSPELL_CHANGED) + if (itr->second.state == PLAYERSPELL_REMOVED || itr->second.state == PLAYERSPELL_CHANGED) { stmt = CharacterDatabase.GetPreparedStatement(CHAR_DEL_CHAR_SPELL_BY_SPELL); stmt->setUInt32(0, itr->first); @@ -20485,25 +20477,24 @@ void Player::_SaveSpells(CharacterDatabaseTransaction& trans) } // add only changed/new not dependent spells - if (!itr->second->dependent && (itr->second->state == PLAYERSPELL_NEW || itr->second->state == PLAYERSPELL_CHANGED)) + if (!itr->second.dependent && (itr->second.state == PLAYERSPELL_NEW || itr->second.state == PLAYERSPELL_CHANGED)) { stmt = CharacterDatabase.GetPreparedStatement(CHAR_INS_CHAR_SPELL); stmt->setUInt32(0, GetGUID().GetCounter()); stmt->setUInt32(1, itr->first); - stmt->setBool(2, itr->second->active); - stmt->setBool(3, itr->second->disabled); + stmt->setBool(2, itr->second.active); + stmt->setBool(3, itr->second.disabled); trans->Append(stmt); } - if (itr->second->state == PLAYERSPELL_REMOVED) + if (itr->second.state == PLAYERSPELL_REMOVED) { - delete itr->second; itr = m_spells.erase(itr); continue; } - if (itr->second->state != PLAYERSPELL_TEMPORARY) - itr->second->state = PLAYERSPELL_UNCHANGED; + if (itr->second.state != PLAYERSPELL_TEMPORARY) + itr->second.state = PLAYERSPELL_UNCHANGED; ++itr; } diff --git a/src/server/game/Entities/Player/Player.h b/src/server/game/Entities/Player/Player.h index e5ad5a637f4..f1c6aa8862a 100644 --- a/src/server/game/Entities/Player/Player.h +++ b/src/server/game/Entities/Player/Player.h @@ -213,7 +213,7 @@ struct PlayerCurrency }; typedef std::unordered_map PlayerTalentMap; -typedef std::unordered_map PlayerSpellMap; +typedef std::unordered_map PlayerSpellMap; typedef std::unordered_set SpellModContainer; typedef std::unordered_map PlayerCurrenciesMap; diff --git a/src/server/game/Entities/Unit/Unit.cpp b/src/server/game/Entities/Unit/Unit.cpp index 313a60e0723..6a01e12a5d2 100644 --- a/src/server/game/Entities/Unit/Unit.cpp +++ b/src/server/game/Entities/Unit/Unit.cpp @@ -5833,7 +5833,7 @@ void Unit::ModifyAuraState(AuraStateType flag, bool apply) PlayerSpellMap const& sp_list = ToPlayer()->GetSpellMap(); for (PlayerSpellMap::const_iterator itr = sp_list.begin(); itr != sp_list.end(); ++itr) { - if (itr->second->state == PLAYERSPELL_REMOVED || itr->second->disabled) + if (itr->second.state == PLAYERSPELL_REMOVED || itr->second.disabled) continue; SpellInfo const* spellInfo = sSpellMgr->GetSpellInfo(itr->first); if (!spellInfo || !spellInfo->IsPassive()) diff --git a/src/server/game/Spells/Auras/SpellAuraEffects.cpp b/src/server/game/Spells/Auras/SpellAuraEffects.cpp index 3a247e4458a..7639f6a4dc4 100644 --- a/src/server/game/Spells/Auras/SpellAuraEffects.cpp +++ b/src/server/game/Spells/Auras/SpellAuraEffects.cpp @@ -1301,7 +1301,7 @@ void AuraEffect::HandleShapeshiftBoosts(Unit* target, bool apply) const PlayerSpellMap const& sp_list = plrTarget->GetSpellMap(); for (PlayerSpellMap::const_iterator itr = sp_list.begin(); itr != sp_list.end(); ++itr) { - if (itr->second->state == PLAYERSPELL_REMOVED || itr->second->disabled) + if (itr->second.state == PLAYERSPELL_REMOVED || itr->second.disabled) continue; if (itr->first == spellId || itr->first == spellId2) @@ -1929,7 +1929,7 @@ void AuraEffect::HandleAuraModShapeshift(AuraApplication const* aurApp, uint8 mo PlayerSpellMap const& sp_list = target->ToPlayer()->GetSpellMap(); for (PlayerSpellMap::const_iterator itr = sp_list.begin(); itr != sp_list.end(); ++itr) { - if (itr->second->state == PLAYERSPELL_REMOVED || itr->second->disabled) + if (itr->second.state == PLAYERSPELL_REMOVED || itr->second.disabled) continue; SpellInfo const* spellInfo = sSpellMgr->GetSpellInfo(itr->first); diff --git a/src/server/game/Spells/SpellHistory.cpp b/src/server/game/Spells/SpellHistory.cpp index 5758cc45b0a..db59681ca5b 100644 --- a/src/server/game/Spells/SpellHistory.cpp +++ b/src/server/game/Spells/SpellHistory.cpp @@ -558,7 +558,7 @@ void SpellHistory::LockSpellSchool(SpellSchoolMask schoolMask, uint32 lockoutTim if (Player* plrOwner = _owner->ToPlayer()) { for (auto const& p : plrOwner->GetSpellMap()) - if (p.second->state != PLAYERSPELL_REMOVED) + if (p.second.state != PLAYERSPELL_REMOVED) knownSpells.insert(p.first); } else if (Pet* petOwner = _owner->ToPet())