Core/Achievements: Minor fixes

* Corrected criteria and modifier tree logic after implementing their operators
* Account wide achievements will now show progress ingame (even if we still treat them as per-character)
* Fixed more cases of achievements being granted automatically without satisfying modifier trees

5 files changed, 106 insertions, 51 deletions

diff --git a/src/server/game/Achievements/AchievementMgr.cpp b/src/server/game/Achievements/AchievementMgr.cpp
index ce2494c350a..a0b82470eac 100644
--- a/src/server/game/Achievements/AchievementMgr.cpp
+++ b/src/server/game/Achievements/AchievementMgr.cpp
@@ -285,6 +285,7 @@ void PlayerAchievementMgr::LoadFromDB(PreparedQueryResult achievementResult, Pre
             CriteriaProgress& progress = _criteriaProgress[id];
             progress.Counter = counter;
             progress.Date = date;
+            progress.PlayerGUID = _owner->GetGUID();
             progress.Changed = false;
         } while (criteriaResult->NextRow());
     }
@@ -376,6 +377,8 @@ void PlayerAchievementMgr::ResetCriteria(CriteriaCondition condition, int32 fail
 void PlayerAchievementMgr::SendAllData(Player const* /*receiver*/) const
 {
     VisibleAchievementCheck filterInvisible;
+    WorldPackets::Achievement::AllAccountCriteria allAccountCriteria;
+
     WorldPackets::Achievement::AllAchievementData achievementData;
     achievementData.Data.Earned.reserve(_completedAchievements.size());
     achievementData.Data.Progress.reserve(_criteriaProgress.size());
@@ -399,6 +402,8 @@ void PlayerAchievementMgr::SendAllData(Player const* /*receiver*/) const
 
     for (auto itr = _criteriaProgress.begin(); itr != _criteriaProgress.end(); ++itr)
     {
+        Criteria const* criteria = sCriteriaMgr->GetCriteria(itr->first);
+
         WorldPackets::Achievement::CriteriaProgress progress;
         progress.Id = itr->first;
         progress.Quantity = itr->second.Counter;
@@ -408,8 +413,24 @@ void PlayerAchievementMgr::SendAllData(Player const* /*receiver*/) const
         progress.TimeFromStart = 0;
         progress.TimeFromCreate = 0;
         achievementData.Data.Progress.push_back(progress);
+
+        if (criteria->FlagsCu & CRITERIA_FLAG_CU_ACCOUNT)
+        {
+            WorldPackets::Achievement::CriteriaProgress progress;
+            progress.Id = itr->first;
+            progress.Quantity = itr->second.Counter;
+            progress.Player = _owner->GetSession()->GetBattlenetAccountGUID();
+            progress.Flags = 0;
+            progress.Date = itr->second.Date;
+            progress.TimeFromStart = 0;
+            progress.TimeFromCreate = 0;
+            allAccountCriteria.Progress.push_back(progress);
+        }
     }
 
+    if (!allAccountCriteria.Progress.empty())
+        SendPacket(allAccountCriteria.Write());
+
     SendPacket(achievementData.Write());
 }
 
@@ -556,20 +577,40 @@ bool PlayerAchievementMgr::ModifierTreeSatisfied(uint32 modifierTreeId) const
 
 void PlayerAchievementMgr::SendCriteriaUpdate(Criteria const* criteria, CriteriaProgress const* progress, uint32 timeElapsed, bool timedCompleted) const
 {
-    WorldPackets::Achievement::CriteriaUpdate criteriaUpdate;
+    if (criteria->FlagsCu & CRITERIA_FLAG_CU_ACCOUNT)
+    {
+        WorldPackets::Achievement::AccountCriteriaUpdate criteriaUpdate;
 
-    criteriaUpdate.CriteriaID = criteria->ID;
-    criteriaUpdate.Quantity = progress->Counter;
-    criteriaUpdate.PlayerGUID = _owner->GetGUID();
-    criteriaUpdate.Flags = 0;
-    if (criteria->Entry->StartTimer)
-        criteriaUpdate.Flags = timedCompleted ? 1 : 0; // 1 is for keeping the counter at 0 in client
+        criteriaUpdate.Progress.Id = criteria->ID;
+        criteriaUpdate.Progress.Quantity = progress->Counter;
+        criteriaUpdate.Progress.Player = _owner->GetSession()->GetBattlenetAccountGUID();
+        criteriaUpdate.Progress.Flags = 0;
+        if (criteria->Entry->StartTimer)
+            criteriaUpdate.Progress.Flags = timedCompleted ? 1 : 0; // 1 is for keeping the counter at 0 in client
 
-    criteriaUpdate.CurrentTime = progress->Date;
-    criteriaUpdate.ElapsedTime = timeElapsed;
-    criteriaUpdate.CreationTime = 0;
+        criteriaUpdate.Progress.Date = progress->Date;
+        criteriaUpdate.Progress.TimeFromStart = timeElapsed;
+        criteriaUpdate.Progress.TimeFromCreate = 0;
 
-    SendPacket(criteriaUpdate.Write());
+        SendPacket(criteriaUpdate.Write());
+    }
+    else
+    {
+        WorldPackets::Achievement::CriteriaUpdate criteriaUpdate;
+
+        criteriaUpdate.CriteriaID = criteria->ID;
+        criteriaUpdate.Quantity = progress->Counter;
+        criteriaUpdate.PlayerGUID = _owner->GetGUID();
+        criteriaUpdate.Flags = 0;
+        if (criteria->Entry->StartTimer)
+            criteriaUpdate.Flags = timedCompleted ? 1 : 0; // 1 is for keeping the counter at 0 in client
+
+        criteriaUpdate.CurrentTime = progress->Date;
+        criteriaUpdate.ElapsedTime = timeElapsed;
+        criteriaUpdate.CreationTime = 0;
+
+        SendPacket(criteriaUpdate.Write());
+    }
 }
 
 void PlayerAchievementMgr::SendCriteriaProgressRemoved(uint32 criteriaId)
diff --git a/src/server/game/Achievements/CriteriaHandler.cpp b/src/server/game/Achievements/CriteriaHandler.cpp
index 04cdf0ac257..63e681f2273 100644
--- a/src/server/game/Achievements/CriteriaHandler.cpp
+++ b/src/server/game/Achievements/CriteriaHandler.cpp
@@ -23,6 +23,7 @@
 #include "Battleground.h"
 #include "BattlePetMgr.h"
 #include "CollectionMgr.h"
+#include "Containers.h"
 #include "Creature.h"
 #include "DatabaseEnv.h"
 #include "DB2Stores.h"
@@ -1991,6 +1992,7 @@ bool CriteriaHandler::ModifierSatisfied(ModifierTreeEntry const* modifier, uint6
                 return false;
             break;
         case CRITERIA_ADDITIONAL_CONDITION_HAS_ACHIEVEMENT: // 86
+        case CRITERIA_ADDITIONAL_CONDITION_HAS_ACHIEVEMENT_ON_CHARACTER: // 87
             if (!referencePlayer->HasAchieved(reqValue))
                 return false;
             break;
@@ -3208,14 +3210,8 @@ void CriteriaMgr::LoadCriteriaModifiersTree()
 
     // Build tree
     for (auto itr = _criteriaModifiers.begin(); itr != _criteriaModifiers.end(); ++itr)
-    {
-        if (!itr->second->Entry->Parent)
-            continue;
-
-        auto parent = _criteriaModifiers.find(itr->second->Entry->Parent);
-        if (parent != _criteriaModifiers.end())
-            parent->second->Children.push_back(itr->second);
-    }
+        if (ModifierTreeNode* parentNode = Trinity::Containers::MapGetValuePtr(_criteriaModifiers, itr->second->Entry->Parent))
+            parentNode->Children.push_back(itr->second);
 
     TC_LOG_INFO("server.loading", ">> Loaded %u criteria modifiers in %u ms", uint32(_criteriaModifiers.size()), GetMSTimeDiffToNow(oldMSTime));
 }
@@ -3247,12 +3243,6 @@ void CriteriaMgr::LoadCriteriaList()
 {
     uint32 oldMSTime = getMSTime();
 
-    if (sCriteriaTreeStore.GetNumRows() == 0)
-    {
-        TC_LOG_ERROR("server.loading", ">> Loaded 0 criteria.");
-        return;
-    }
-
     std::unordered_map<uint32 /*criteriaTreeID*/, AchievementEntry const*> achievementCriteriaTreeIds;
     for (AchievementEntry const* achievement : sAchievementStore)
         if (achievement->CriteriaTree)
@@ -3299,25 +3289,10 @@ void CriteriaMgr::LoadCriteriaList()
     // Build tree
     for (auto itr = _criteriaTrees.begin(); itr != _criteriaTrees.end(); ++itr)
     {
-        if (!itr->second->Entry->Parent)
-            continue;
+        if (CriteriaTree* parent = Trinity::Containers::MapGetValuePtr(_criteriaTrees, itr->second->Entry->Parent))
+            parent->Children.push_back(itr->second);
 
-        auto parent = _criteriaTrees.find(itr->second->Entry->Parent);
-        if (parent != _criteriaTrees.end())
-        {
-            parent->second->Children.push_back(itr->second);
-            while (parent != _criteriaTrees.end())
-            {
-                auto cur = parent;
-                parent = _criteriaTrees.find(parent->second->Entry->Parent);
-                if (parent == _criteriaTrees.end())
-                {
-                    if (sCriteriaStore.LookupEntry(itr->second->Entry->CriteriaID))
-                        _criteriaTreeByCriteria[itr->second->Entry->CriteriaID].push_back(cur->second);
-                }
-            }
-        }
-        else if (sCriteriaStore.LookupEntry(itr->second->Entry->CriteriaID))
+        if (sCriteriaStore.HasRecord(itr->second->Entry->CriteriaID))
             _criteriaTreeByCriteria[itr->second->Entry->CriteriaID].push_back(itr->second);
     }
 
@@ -3342,14 +3317,14 @@ void CriteriaMgr::LoadCriteriaList()
         Criteria* criteria = new Criteria();
         criteria->ID = criteriaEntry->ID;
         criteria->Entry = criteriaEntry;
-        auto mod = _criteriaModifiers.find(criteriaEntry->ModifierTreeId);
-        if (mod != _criteriaModifiers.end())
-            criteria->Modifier = mod->second;
+        criteria->Modifier = Trinity::Containers::MapGetValuePtr(_criteriaModifiers, criteriaEntry->ModifierTreeId);
 
         _criteria[criteria->ID] = criteria;
 
         for (CriteriaTree const* tree : treeItr->second)
         {
+            const_cast<CriteriaTree*>(tree)->Criteria = criteria;
+
             if (AchievementEntry const* achievement = tree->Achievement)
             {
                 if (achievement->Flags & ACHIEVEMENT_FLAG_GUILD)
@@ -3420,9 +3395,6 @@ void CriteriaMgr::LoadCriteriaList()
             _criteriasByFailEvent[criteriaEntry->FailEvent][criteriaEntry->FailAsset].push_back(criteria);
     }
 
-    for (auto& p : _criteriaTrees)
-        const_cast<CriteriaTree*>(p.second)->Criteria = GetCriteria(p.second->Entry->CriteriaID);
-
     TC_LOG_INFO("server.loading", ">> Loaded %u criteria, %u guild criteria, %u scenario criteria and %u quest objective criteria in %u ms.", criterias, guildCriterias, scenarioCriterias, questObjectiveCriterias, GetMSTimeDiffToNow(oldMSTime));
 }
 
diff --git a/src/server/game/Server/Packets/AchievementPackets.cpp b/src/server/game/Server/Packets/AchievementPackets.cpp
index 058e6d34524..033f4b9ccf6 100644
--- a/src/server/game/Server/Packets/AchievementPackets.cpp
+++ b/src/server/game/Server/Packets/AchievementPackets.cpp
@@ -66,6 +66,15 @@ WorldPacket const* WorldPackets::Achievement::AllAchievementData::Write()
     return &_worldPacket;
 }
 
+WorldPacket const* WorldPackets::Achievement::AllAccountCriteria::Write()
+{
+    _worldPacket << uint32(Progress.size());
+    for (WorldPackets::Achievement::CriteriaProgress const& progress : Progress)
+        _worldPacket << progress;
+
+    return &_worldPacket;
+}
+
 WorldPacket const* WorldPackets::Achievement::RespondInspectAchievements::Write()
 {
     _worldPacket << Player;
@@ -83,6 +92,18 @@ WorldPacket const* WorldPackets::Achievement::CriteriaUpdate::Write()
     _worldPacket.AppendPackedTime(CurrentTime);
     _worldPacket << uint32(ElapsedTime);
     _worldPacket << uint32(CreationTime);
+    _worldPacket.WriteBit(RafAcceptanceID.is_initialized());
+    _worldPacket.FlushBits();
+
+    if (RafAcceptanceID)
+        _worldPacket << uint64(*RafAcceptanceID);
+
+    return &_worldPacket;
+}
+
+WorldPacket const* WorldPackets::Achievement::AccountCriteriaUpdate::Write()
+{
+    _worldPacket << Progress;
 
     return &_worldPacket;
 }
diff --git a/src/server/game/Server/Packets/AchievementPackets.h b/src/server/game/Server/Packets/AchievementPackets.h
index 7b3d3d927ac..e2c6f85dbc0 100644
--- a/src/server/game/Server/Packets/AchievementPackets.h
+++ b/src/server/game/Server/Packets/AchievementPackets.h
@@ -63,6 +63,16 @@ namespace WorldPackets
             AllAchievements Data;
         };
 
+        class AllAccountCriteria final : public ServerPacket
+        {
+        public:
+            AllAccountCriteria() : ServerPacket(SMSG_ALL_ACCOUNT_CRITERIA) { }
+
+            WorldPacket const* Write() override;
+
+            std::vector<CriteriaProgress> Progress;
+        };
+
         class RespondInspectAchievements final : public ServerPacket
         {
         public:
@@ -88,6 +98,17 @@ namespace WorldPackets
             time_t CurrentTime = time_t(0);
             uint32 ElapsedTime = 0;
             uint32 CreationTime = 0;
+            Optional<uint64> RafAcceptanceID;
+        };
+
+        class AccountCriteriaUpdate final : public ServerPacket
+        {
+        public:
+            AccountCriteriaUpdate() : ServerPacket(SMSG_ACCOUNT_CRITERIA_UPDATE) { }
+
+            WorldPacket const* Write() override;
+
+            CriteriaProgress Progress;
         };
 
         class CriteriaDeleted final : public ServerPacket
diff --git a/src/server/game/Server/Protocol/Opcodes.cpp b/src/server/game/Server/Protocol/Opcodes.cpp
index 076e0c55d08..a8f0be7874e 100644
--- a/src/server/game/Server/Protocol/Opcodes.cpp
+++ b/src/server/game/Server/Protocol/Opcodes.cpp
@@ -895,7 +895,7 @@ void OpcodeTable::Initialize()
     ValidateAndSetServerOpcode(opcode, #opcode, status, con)
 
     DEFINE_SERVER_OPCODE_HANDLER(SMSG_ABORT_NEW_WORLD,                         STATUS_UNHANDLED,    CONNECTION_TYPE_REALM);
-    DEFINE_SERVER_OPCODE_HANDLER(SMSG_ACCOUNT_CRITERIA_UPDATE,                 STATUS_UNHANDLED,    CONNECTION_TYPE_REALM);
+    DEFINE_SERVER_OPCODE_HANDLER(SMSG_ACCOUNT_CRITERIA_UPDATE,                 STATUS_NEVER,        CONNECTION_TYPE_REALM);
     DEFINE_SERVER_OPCODE_HANDLER(SMSG_ACCOUNT_DATA_TIMES,                      STATUS_NEVER,        CONNECTION_TYPE_REALM);
     DEFINE_SERVER_OPCODE_HANDLER(SMSG_ACCOUNT_HEIRLOOM_UPDATE,                 STATUS_NEVER,        CONNECTION_TYPE_INSTANCE);
     DEFINE_SERVER_OPCODE_HANDLER(SMSG_ACCOUNT_MOUNT_UPDATE,                    STATUS_NEVER,        CONNECTION_TYPE_INSTANCE);
@@ -913,7 +913,7 @@ void OpcodeTable::Initialize()
     DEFINE_SERVER_OPCODE_HANDLER(SMSG_AE_LOOT_TARGETS,                         STATUS_NEVER,        CONNECTION_TYPE_INSTANCE);
     DEFINE_SERVER_OPCODE_HANDLER(SMSG_AE_LOOT_TARGET_ACK,                      STATUS_NEVER,        CONNECTION_TYPE_INSTANCE);
     DEFINE_SERVER_OPCODE_HANDLER(SMSG_AI_REACTION,                             STATUS_NEVER,        CONNECTION_TYPE_INSTANCE);
-    DEFINE_SERVER_OPCODE_HANDLER(SMSG_ALL_ACCOUNT_CRITERIA,                    STATUS_UNHANDLED,    CONNECTION_TYPE_INSTANCE);
+    DEFINE_SERVER_OPCODE_HANDLER(SMSG_ALL_ACCOUNT_CRITERIA,                    STATUS_NEVER,        CONNECTION_TYPE_INSTANCE);
     DEFINE_SERVER_OPCODE_HANDLER(SMSG_ALL_ACHIEVEMENT_DATA,                    STATUS_NEVER,        CONNECTION_TYPE_INSTANCE);
     DEFINE_SERVER_OPCODE_HANDLER(SMSG_ALL_GUILD_ACHIEVEMENTS,                  STATUS_NEVER,        CONNECTION_TYPE_REALM);
     DEFINE_SERVER_OPCODE_HANDLER(SMSG_ARCHAEOLOGY_SURVERY_CAST,                STATUS_UNHANDLED,    CONNECTION_TYPE_REALM);