From 67bbe4c1f0475c3ba62f9942be59244e1d9f7603 Mon Sep 17 00:00:00 2001 From: David Markowitz <39972741+EmosewaMC@users.noreply.github.com> Date: Sun, 17 May 2026 12:21:08 -0700 Subject: [PATCH] fix(Missions): mission progression undefined behavior (#1973) * fix: mission progression undefined behavior defer the sub calls until after the loop has finished, that way no ub happens. tested that mission progression all the way up until joining a faction still works and meta missions still function. * default initialize * Update MissionComponent.h --- dGame/dComponents/MissionComponent.cpp | 112 ++++++------------------- dGame/dComponents/MissionComponent.h | 22 ++++- 2 files changed, 47 insertions(+), 87 deletions(-) diff --git a/dGame/dComponents/MissionComponent.cpp b/dGame/dComponents/MissionComponent.cpp index 6480e74d..e99b1f5e 100644 --- a/dGame/dComponents/MissionComponent.cpp +++ b/dGame/dComponents/MissionComponent.cpp @@ -144,13 +144,22 @@ void MissionComponent::RemoveMission(uint32_t missionId) { void MissionComponent::Progress(eMissionTaskType type, int32_t value, LWOOBJID associate, const std::string& targets, int32_t count, bool ignoreAchievements) { LOG("Progressing missions %s %i %llu %s %s", StringifiedEnum::ToString(type).data(), value, associate, targets.c_str(), ignoreAchievements ? "(ignoring achievements)" : ""); - std::vector acceptedAchievements; - if (count > 0 && !ignoreAchievements) { - acceptedAchievements = LookForAchievements(type, value, true, associate, targets, count); + + // If we are already iterating m_Missions, defer this call to avoid iterator invalidation + // from re-entrant insertions (e.g. a completing achievement triggering LookForAchievements). + if (m_IsProgressing) { + m_PendingProgress.push_back({ type, value, associate, targets, count, ignoreAchievements }); + return; } + std::vector acceptedAchievements; + if (count > 0 && !ignoreAchievements) { + acceptedAchievements = LookForAchievements(type, value, associate, targets, count); + } + + m_IsProgressing = true; for (const auto& [id, mission] : m_Missions) { - if (!mission || std::ranges::find(acceptedAchievements, mission->GetMissionId()) != acceptedAchievements.end()) continue; + if (!mission) continue; if (mission->IsAchievement() && ignoreAchievements) continue; @@ -158,6 +167,18 @@ void MissionComponent::Progress(eMissionTaskType type, int32_t value, LWOOBJID a mission->Progress(type, value, associate, targets, count); } + m_IsProgressing = false; + + // Drain any Progress() calls that were deferred during the loop above. + // Each call here may itself defer further calls, which are drained recursively + // before returning, so the while loop only needs one pass in practice. + while (!m_PendingProgress.empty()) { + auto pending = std::move(m_PendingProgress); + m_PendingProgress.clear(); + for (const auto& p : pending) { + Progress(p.type, p.value, p.associate, p.targets, p.count, p.ignoreAchievements); + } + } } void MissionComponent::ForceProgress(const uint32_t missionId, const uint32_t taskId, const int32_t value, const bool acceptMission) { @@ -280,10 +301,7 @@ bool MissionComponent::GetMissionInfo(uint32_t missionId, CDMissions& result) { return true; } -#define MISSION_NEW_METHOD - -const std::vector MissionComponent::LookForAchievements(eMissionTaskType type, int32_t value, bool progress, LWOOBJID associate, const std::string& targets, int32_t count) { -#ifdef MISSION_NEW_METHOD +const std::vector MissionComponent::LookForAchievements(eMissionTaskType type, int32_t value, LWOOBJID associate, const std::string& targets, int32_t count) { // Query for achievments, using the cache const auto& result = QueryAchievements(type, value, targets); @@ -310,85 +328,9 @@ const std::vector MissionComponent::LookForAchievements(eMissionTaskTy instance->Accept(); acceptedAchievements.push_back(missionID); - - if (progress) { - // Progress mission to bring it up to speed - instance->Progress(type, value, associate, targets, count); - } } return acceptedAchievements; -#else - auto* missionTasksTable = CDClientManager::GetTable(); - auto* missionsTable = CDClientManager::GetTable(); - - auto tasks = missionTasksTable->Query([=](const CDMissionTasks& entry) { - return entry.taskType == static_cast(type); - }); - - std::vector acceptedAchievements; - - for (const auto& task : tasks) { - if (GetMission(task.id) != nullptr) { - continue; - } - - const auto missionEntries = missionsTable->Query([=](const CDMissions& entry) { - return entry.id == static_cast(task.id) && !entry.isMission; - }); - - if (missionEntries.empty()) { - continue; - } - - const auto mission = missionEntries[0]; - - if (mission.isMission || !MissionPrerequisites::CanAccept(mission.id, m_Missions)) { - continue; - } - - if (task.target != value && task.targetGroup != targets) { - auto stream = std::istringstream(task.targetGroup); - std::string token; - - auto found = false; - - while (std::getline(stream, token, ',')) { - try { - const auto target = std::stoul(token); - - found = target == value; - - if (found) { - break; - } - } catch (std::invalid_argument& exception) { - LOG("Failed to parse target (%s): (%s)!", token.c_str(), exception.what()); - } - } - - if (!found) { - continue; - } - } - - auto* instance = new Mission(this, mission.id); - - m_Missions.insert_or_assign(mission.id, instance); - - if (instance->IsMission()) instance->SetUniqueMissionOrderID(++m_LastUsedMissionOrderUID); - - instance->Accept(); - - acceptedAchievements.push_back(mission.id); - - if (progress) { - instance->Progress(type, value, associate, targets, count); - } - } - - return acceptedAchievements; -#endif } const std::vector& MissionComponent::QueryAchievements(eMissionTaskType type, int32_t value, const std::string targets) { @@ -497,7 +439,7 @@ bool MissionComponent::RequiresItem(const LOT lot) { } } - const auto required = LookForAchievements(eMissionTaskType::GATHER, lot, false); + const auto required = LookForAchievements(eMissionTaskType::GATHER, lot); return !required.empty(); } diff --git a/dGame/dComponents/MissionComponent.h b/dGame/dComponents/MissionComponent.h index 7ac4b108..f31fac8e 100644 --- a/dGame/dComponents/MissionComponent.h +++ b/dGame/dComponents/MissionComponent.h @@ -134,13 +134,12 @@ public: * Checks if there's any achievements we might be able to accept for the given parameters * @param type the task type for tasks in the achievement that we wish to progress * @param value the value to progress by - * @param progress if we can accept the mission, this will apply the progression * @param associate optional associate related to mission progression * @param targets optional multiple targets related to mission progression * @param count the number of values to progress by (differs by task type) * @return true if a achievement was accepted, false otherwise */ - const std::vector LookForAchievements(eMissionTaskType type, int32_t value, bool progress = true, LWOOBJID associate = LWOOBJID_EMPTY, const std::string& targets = "", int32_t count = 1); + const std::vector LookForAchievements(eMissionTaskType type, int32_t value, LWOOBJID associate = LWOOBJID_EMPTY, const std::string& targets = "", int32_t count = 1); /** * Checks if there's a mission active that requires the collection of the specified LOT @@ -208,6 +207,25 @@ private: * In live this value started at 745. */ uint32_t m_LastUsedMissionOrderUID = 746U; + + /** + * Holds arguments for a Progress() call that arrived re-entrantly while m_Missions was + * being iterated, so it can be replayed after the active loop finishes. + */ + struct PendingProgress { + eMissionTaskType type{}; + int32_t value{}; + LWOOBJID associate{}; + std::string targets; + int32_t count{}; + bool ignoreAchievements{}; + }; + + /// True while the m_Missions range-for loop in Progress() is executing. + bool m_IsProgressing = false; + + /// Re-entrant Progress() calls deferred here for replay after the active loop finishes. + std::vector m_PendingProgress; }; #endif // MISSIONCOMPONENT_H