From f3b546bcf63648aa925e30c7ea64eaf2105523ac Mon Sep 17 00:00:00 2001
From: davewiththenicehat
 <54369722+davewiththenicehat@users.noreply.github.com>
Date: Sun, 18 Apr 2021 08:51:18 -0400
Subject: [PATCH] TaskHandler.remove() made functional

TaskHandler.remove method now functions. Previous it would have removed the task from the TaskHandler.tasks dictionary, but never canceled the task. Making the "remove a persistent task without executing it" incorrect. Previous there was no method to get a persistent tasks's deferral instance, which was likely why TaskHandler.remove was not used within the module.

Added unit tests to test TaskHandler.remove
---
 evennia/scripts/taskhandler.py    | 27 ++++++++++++++++++++-------
 evennia/utils/tests/test_utils.py | 14 ++++++++++++++
 2 files changed, 34 insertions(+), 7 deletions(-)

diff --git a/evennia/scripts/taskhandler.py b/evennia/scripts/taskhandler.py
index b865a74185..2eb8623670 100644
--- a/evennia/scripts/taskhandler.py
+++ b/evennia/scripts/taskhandler.py
@@ -212,22 +212,35 @@ class TaskHandler(object):
         return task_id
 
     def remove(self, task_id):
-        """Remove a persistent task without executing it.
+        """
+        Remove a task without executing it.
+        Deletes the instance of the task's deferral.
 
         Args:
             task_id (int): an existing task ID.
 
-        Note:
-            A non-persistent task doesn't have a task_id, it is not stored
-            in the TaskHandler.
+        Returns:
+            True (bool), if the removal completed successfully.
+            None, if there was a raise error
 
         """
+        d = None
+        # delete the task from the tasks dictionary
         if task_id in self.tasks:
-            del self.tasks[task_id]
+            # if the task has not been run, cancel it
+            d = self.get_deferred(task_id)
+            if d:  # it is remotely possible for a task to not have a deferral
+                if not d.called:
+                    d.cancel()
+            del self.tasks[task_id]  # delete the task from the tasks dictionary
+        # remove the task from the persistent dictionary and ServerConfig
         if task_id in self.to_save:
             del self.to_save[task_id]
-
-        self.save()
+            self.save()  # remove from ServerConfig.objects
+        # delete the instance of the deferral
+        if d:
+            del d
+        return True
 
     def do_task(self, task_id):
         """Execute the task (call its callback).
diff --git a/evennia/utils/tests/test_utils.py b/evennia/utils/tests/test_utils.py
index ae4d49e90f..6d9ae97d04 100644
--- a/evennia/utils/tests/test_utils.py
+++ b/evennia/utils/tests/test_utils.py
@@ -351,3 +351,17 @@ class TestDelay(EvenniaTest):
         _TASK_HANDLER.clock.advance(timedelay)  # make time pass
         self.assertEqual(self.char1.ndb.dummy_var, False)
         self.char1.ndb.dummy_var = False
+        # test removing an active task
+        task_id = utils.delay(timedelay, dummy_func, self.char1.dbref)
+        success = _TASK_HANDLER.remove(task_id)
+        _TASK_HANDLER.clock.advance(timedelay)  # make time pass
+        self.assertEqual(self.char1.ndb.dummy_var, False)
+        self.char1.ndb.dummy_var = False
+        # test removing a canceled active task
+        task_id = utils.delay(timedelay, dummy_func, self.char1.dbref)
+        deferal_inst = _TASK_HANDLER.get_deferred(task_id)
+        deferal_inst.cancel()
+        success = _TASK_HANDLER.remove(task_id)
+        _TASK_HANDLER.clock.advance(timedelay)  # make time pass
+        self.assertEqual(self.char1.ndb.dummy_var, False)
+        self.char1.ndb.dummy_var = False