From c4a99aabd07ccc43f0c2fb33e4ad1f0e33fb6935 Mon Sep 17 00:00:00 2001 From: hg <4683435+hgtw@users.noreply.github.com> Date: Sun, 31 Jul 2022 14:24:21 -0400 Subject: [PATCH] [Shared Tasks] Avoid erasing shared tasks while iterating (#2348) This wasn't safe since the erase would invalidate iterators used internally by the range loops. Shared tasks with no members are now also cleaned up Make GetMembers return reference instead of copy (this is used a lot) Add rule for shared task terminate time for easier debugging --- common/ruletypes.h | 1 + common/shared_tasks.cpp | 2 +- common/shared_tasks.h | 2 +- world/shared_task_manager.cpp | 45 +++++++++++++++++++++-------------- world/shared_task_manager.h | 1 - 5 files changed, 30 insertions(+), 21 deletions(-) diff --git a/common/ruletypes.h b/common/ruletypes.h index 9bbabab02..8a2f1f30d 100644 --- a/common/ruletypes.h +++ b/common/ruletypes.h @@ -556,6 +556,7 @@ RULE_BOOL(TaskSystem, KeepOneRecordPerCompletedTask, true, "Keep only one record RULE_BOOL(TaskSystem, EnableTaskProximity, true, "Enable task proximity system") RULE_INT(TaskSystem, RequestCooldownTimerSeconds, 15, "Seconds between allowing characters to request tasks (live-like default: 15 seconds)") RULE_INT(TaskSystem, SharedTasksWorldProcessRate, 6000, "Timer interval (milliseconds) that shared tasks are processed in world") +RULE_INT(TaskSystem, SharedTasksTerminateTimerMS, 120000, "Delay (milliseconds) until a shared task is terminated if requirements are no longer met after member removal (default: 2 minutes)") RULE_CATEGORY_END() RULE_CATEGORY(Range) diff --git a/common/shared_tasks.cpp b/common/shared_tasks.cpp index ea8884fef..894f9c280 100644 --- a/common/shared_tasks.cpp +++ b/common/shared_tasks.cpp @@ -7,7 +7,7 @@ std::vector SharedTask::GetActivityState() const return m_shared_task_activity_state; } -std::vector SharedTask::GetMembers() const +const std::vector& SharedTask::GetMembers() const { return m_members; } diff --git a/common/shared_tasks.h b/common/shared_tasks.h index b300268d0..0d16ce8ec 100644 --- a/common/shared_tasks.h +++ b/common/shared_tasks.h @@ -183,7 +183,7 @@ public: SharedTaskMember FindMemberFromCharacterName(const std::string& character_name) const; SharedTaskMember GetLeader() const; std::vector GetActivityState() const; - std::vector GetMembers() const; + const std::vector& GetMembers() const; // getters const std::vector &GetTaskActivityData() const; diff --git a/world/shared_task_manager.cpp b/world/shared_task_manager.cpp index fa91f647f..b5d514439 100644 --- a/world/shared_task_manager.cpp +++ b/world/shared_task_manager.cpp @@ -241,20 +241,6 @@ void SharedTaskManager::RemoveEveryoneFromSharedTask(SharedTask *t, uint32 reque PrintSharedTaskState(); } -void SharedTaskManager::Terminate(SharedTask* s) -{ - LogTasksDetail("[Process] Terminating shared task [{}]", s->GetDbSharedTask().id); - - for (const auto& member : s->GetMembers()) - { - SendRemovePlayerFromSharedTaskPacket(member.character_id, s->GetTaskData().id, true); - client_list.SendCharacterMessageID(member.character_id, Chat::Yellow, TaskStr::HAS_ENDED, {s->GetTaskData().title}); - } - - RemoveAllMembersFromDynamicZones(s); - DeleteSharedTask(s->GetDbSharedTask().id); -} - void SharedTaskManager::DeleteSharedTask(int64 shared_task_id) { LogTasksDetail( @@ -1679,14 +1665,21 @@ void SharedTaskManager::SetSharedTasks(const std::vector &shared_tas SharedTaskManager *SharedTaskManager::PurgeExpiredSharedTasks() { + std::vector delete_tasks; + auto now = std::time(nullptr); for (auto &s: m_shared_tasks) { if (s.GetDbSharedTask().expire_time > 0 && s.GetDbSharedTask().expire_time <= now) { LogTasksDetail("[PurgeExpiredSharedTasks] Deleting expired task [{}]", s.GetDbSharedTask().id); - DeleteSharedTask(s.GetDbSharedTask().id); + delete_tasks.push_back(s.GetDbSharedTask().id); } } + for (int64_t shared_task_id : delete_tasks) + { + DeleteSharedTask(shared_task_id); + } + return this; } @@ -1764,7 +1757,7 @@ void SharedTaskManager::HandleCompletedTask(SharedTask* s) void SharedTaskManager::StartTerminateTimer(SharedTask* s) { - s->terminate_timer.Start(120000); // 2min + s->terminate_timer.Start(RuleI(TaskSystem, SharedTasksTerminateTimerMS)); SendMembersMessageID(s, Chat::Red, TaskStr::REQS_TWO_MIN); } @@ -1775,11 +1768,27 @@ void SharedTaskManager::Process() return; } + std::vector delete_tasks; for (auto& shared_task : m_shared_tasks) { - if (shared_task.terminate_timer.Check()) + if (shared_task.GetMembers().empty() || shared_task.terminate_timer.Check()) { - Terminate(&shared_task); + LogTasksDetail("[Process] Terminating shared task [{}]", shared_task.GetDbSharedTask().id); + for (const auto& member : shared_task.GetMembers()) + { + SendRemovePlayerFromSharedTaskPacket(member.character_id, shared_task.GetTaskData().id, true); + client_list.SendCharacterMessageID(member.character_id, Chat::Yellow, TaskStr::HAS_ENDED, {shared_task.GetTaskData().title}); + } + + RemoveAllMembersFromDynamicZones(&shared_task); + + // avoid erasing from m_shared_tasks while iterating it + delete_tasks.push_back(shared_task.GetDbSharedTask().id); } } + + for (int64_t shared_task_id : delete_tasks) + { + DeleteSharedTask(shared_task_id); + } } diff --git a/world/shared_task_manager.h b/world/shared_task_manager.h index 803a17245..a456a7139 100644 --- a/world/shared_task_manager.h +++ b/world/shared_task_manager.h @@ -132,7 +132,6 @@ protected: void RecordSharedTaskCompletion(SharedTask *s); void RemoveAllMembersFromDynamicZones(SharedTask *s); void StartTerminateTimer(SharedTask* s); - void Terminate(SharedTask* s); // memory search std::vector FindCharactersInSharedTasks(const std::vector &find_characters);