Refactor Bot engine Queue (#895)

* Update Queue.cpp

* Update Queue.h

src/strategy/Queue.cpp

@@ -4,72 +4,93 @@
  */
 
 #include "Queue.h"
-
 #include "AiObjectContext.h"
 #include "Log.h"
 #include "PlayerbotAIConfig.h"
 
 void Queue::Push(ActionBasket* action)
 {
-    if (action)
+    if (!action)
     {
-        for (std::list<ActionBasket*>::iterator iter = actions.begin(); iter != actions.end(); iter++)
+        return;
+    }
+
+    for (ActionBasket* basket : actions)
     {
-            ActionBasket* basket = *iter;
         if (action->getAction()->getName() == basket->getAction()->getName())
         {
-                if (basket->getRelevance() < action->getRelevance())
+            updateExistingBasket(basket, action);
-                    basket->setRelevance(action->getRelevance());
-
-                if (ActionNode* actionNode = action->getAction())
-                    delete actionNode;
-
-                delete action;
-
             return;
         }
     }
 
     actions.push_back(action);
 }
-}
 
 ActionNode* Queue::Pop()
 {
-    float max = -1;
+    ActionBasket* highestRelevanceBasket = findHighestRelevanceBasket();
-    ActionBasket* selection = nullptr;
+    if (!highestRelevanceBasket)
-
-    for (std::list<ActionBasket*>::iterator iter = actions.begin(); iter != actions.end(); iter++)
     {
-        ActionBasket* basket = *iter;
-        if (basket->getRelevance() > max)
-        {
-            max = basket->getRelevance();
-            selection = basket;
-        }
-    }
-
-    if (selection != nullptr)
-    {
-        ActionNode* action = selection->getAction();
-        actions.remove(selection);
-        delete selection;
-        return action;
-    }
-
         return nullptr;
     }
 
+    return extractAndDeleteBasket(highestRelevanceBasket);
+}
+
 ActionBasket* Queue::Peek()
 {
-    float max = -1;
+    return findHighestRelevanceBasket();
+}
+
+uint32 Queue::Size()
+{
+    return actions.size();
+}
+
+void Queue::RemoveExpired()
+{
+    if (!sPlayerbotAIConfig->expireActionTime)
+    {
+        return;
+    }
+
+    std::list<ActionBasket*> expiredBaskets;
+    collectExpiredBaskets(expiredBaskets);
+    removeAndDeleteBaskets(expiredBaskets);
+}
+
+// Private helper methods
+void Queue::updateExistingBasket(ActionBasket* existing, ActionBasket* newBasket)
+{
+    if (existing->getRelevance() < newBasket->getRelevance())
+    {
+        existing->setRelevance(newBasket->getRelevance());
+    }
+
+    if (ActionNode* actionNode = newBasket->getAction())
+    {
+        delete actionNode;
+    }
+
+    delete newBasket;
+}
+
+ActionBasket* Queue::findHighestRelevanceBasket() const
+{
+    if (actions.empty())
+    {
+        return nullptr;
+    }
+
+    float maxRelevance = -1.0f;
     ActionBasket* selection = nullptr;
-    for (std::list<ActionBasket*>::iterator iter = actions.begin(); iter != actions.end(); iter++)
+
+    for (ActionBasket* basket : actions)
     {
-        ActionBasket* basket = *iter;
+        if (basket->getRelevance() > maxRelevance)
-        if (basket->getRelevance() > max)
         {
-            max = basket->getRelevance();
+            maxRelevance = basket->getRelevance();
             selection = basket;
         }
     }
@@ -77,21 +98,30 @@ ActionBasket* Queue::Peek()
     return selection;
 }
 
-uint32 Queue::Size() { return actions.size(); }
+ActionNode* Queue::extractAndDeleteBasket(ActionBasket* basket)
-
-void Queue::RemoveExpired()
 {
-    std::list<ActionBasket*> expired;
+    ActionNode* action = basket->getAction();
-    for (std::list<ActionBasket*>::iterator iter = actions.begin(); iter != actions.end(); iter++)
+    actions.remove(basket);
-    {
+    delete basket;
-        ActionBasket* basket = *iter;
+    return action;
-        if (sPlayerbotAIConfig->expireActionTime && basket->isExpired(sPlayerbotAIConfig->expireActionTime))
-            expired.push_back(basket);
 }
 
-    for (std::list<ActionBasket*>::iterator iter = expired.begin(); iter != expired.end(); iter++)
+void Queue::collectExpiredBaskets(std::list<ActionBasket*>& expiredBaskets)
+{
+    uint32 expiryTime = sPlayerbotAIConfig->expireActionTime;
+    for (ActionBasket* basket : actions)
+    {
+        if (basket->isExpired(expiryTime))
+        {
+            expiredBaskets.push_back(basket);
+        }
+    }
+}
+
+void Queue::removeAndDeleteBaskets(std::list<ActionBasket*>& basketsToRemove)
+{
+    for (ActionBasket* basket : basketsToRemove)
     {
-        ActionBasket* basket = *iter;
         actions.remove(basket);
 
         if (ActionNode* action = basket->getAction())

src/strategy/Queue.h

@@ -3,26 +3,92 @@
  * and/or modify it under version 2 of the License, or (at your option), any later version.
  */
 
-#ifndef _PLAYERBOT_QUEUE_H
+#ifndef PLAYERBOT_QUEUE_H
-#define _PLAYERBOT_QUEUE_H
+#define PLAYERBOT_QUEUE_H
 
 #include "Action.h"
 #include "Common.h"
 
+/**
+ * @class Queue
+ * @brief Manages a priority queue of actions for the playerbot system
+ * 
+ * This queue maintains a list of ActionBasket objects, each containing an action
+ * and its relevance score. Actions with higher relevance scores are prioritized.
+ */
 class Queue
 {
 public:
-    Queue(void) {}
+    Queue() = default;
-    ~Queue(void) {}
+    ~Queue() = default;
 
+    /**
+     * @brief Adds an action to the queue or updates existing action's relevance
+     * @param action Pointer to the ActionBasket to be added
+     * 
+     * If an action with the same name exists, updates its relevance if the new
+     * relevance is higher, then deletes the new action. Otherwise, adds the new
+     * action to the queue.
+     */
     void Push(ActionBasket* action);
+
+    /**
+     * @brief Removes and returns the action with highest relevance
+     * @return Pointer to the highest relevance ActionNode, or nullptr if queue is empty
+     * 
+     * Ownership of the returned ActionNode is transferred to the caller.
+     * The associated ActionBasket is deleted.
+     */
     ActionNode* Pop();
+
+    /**
+     * @brief Returns the action with highest relevance without removing it
+     * @return Pointer to the ActionBasket with highest relevance, or nullptr if queue is empty
+     */
     ActionBasket* Peek();
+
+    /**
+     * @brief Returns the current size of the queue
+     * @return Number of actions in the queue
+     */
     uint32 Size();
+
+    /**
+     * @brief Removes and deletes expired actions from the queue
+     * 
+     * Uses sPlayerbotAIConfig->expireActionTime to determine if actions have expired.
+     * Both the ActionNode and ActionBasket are deleted for expired actions.
+     */
     void RemoveExpired();
 
 private:
-    std::list<ActionBasket*> actions;
+    /**
+     * @brief Updates existing basket with new relevance and cleans up new basket
+     */
+    void updateExistingBasket(ActionBasket* existing, ActionBasket* newBasket);
+
+    /**
+     * @brief Finds the basket with the highest relevance score
+     * @return Pointer to the highest relevance basket, or nullptr if queue is empty
+     */
+    ActionBasket* findHighestRelevanceBasket() const;
+
+    /**
+     * @brief Extracts action from basket and handles basket cleanup
+     */
+    ActionNode* extractAndDeleteBasket(ActionBasket* basket);
+
+    /**
+     * @brief Collects all expired baskets into the provided list
+     */
+    void collectExpiredBaskets(std::list<ActionBasket*>& expiredBaskets);
+
+    /**
+     * @brief Removes and deletes all baskets in the provided list
+     */
+    void removeAndDeleteBaskets(std::list<ActionBasket*>& basketsToRemove);
+
+    std::list<ActionBasket*> actions; /**< Container for action baskets */
 };
 
 #endif