From ba629b8e0a022e8891d7177ebc58f53cc923244b Mon Sep 17 00:00:00 2001
From: Myk <myk002@yahoo.com>
Date: Tue, 21 Jun 2022 16:38:04 -0700
Subject: [PATCH] manually handle DestroyBuilding jobs (#2209)

* don't delete general refs from jobs that we cancel

though we still disconnect the refs if we can

* get job remove working in all cases

we apparently need to manually handle DestroyBuilding jobs
everything else we should let cancel_job handle

* update changelog
---
 docs/changelog.txt      |  1 +
 library/modules/Job.cpp | 31 ++++++++++++++-----------------
 2 files changed, 15 insertions(+), 17 deletions(-)

diff --git a/docs/changelog.txt b/docs/changelog.txt
index 35f344adb..6499a1fb5 100644
--- a/docs/changelog.txt
+++ b/docs/changelog.txt
@@ -38,6 +38,7 @@ changelog.txt uses a syntax similar to RST, with a few special sequences:
 ## New Tweaks
 
 ## Fixes
+- ``job.removeJob()``: ensure jobs are removed from the world list when they are canceled
 
 ## Misc Improvements
 
diff --git a/library/modules/Job.cpp b/library/modules/Job.cpp
index 749be199e..ff158caa0 100644
--- a/library/modules/Job.cpp
+++ b/library/modules/Job.cpp
@@ -360,27 +360,24 @@ bool DFHack::Job::removeJob(df::job* job) {
     using df::global::world;
     CHECK_NULL_POINTER(job);
 
-    // cancel_job below does not clean up refs, so we have to do that first
+    // cancel_job below does not clean up all refs, so we have to do some work
 
-    // clean up general refs
-    for (auto genRef : job->general_refs) {
-        if (!genRef) continue;
-
-        // disconnectJobGeneralRef only handles buildings and units
-        if (genRef->getType() != general_ref_type::BUILDING_HOLDER &&
-                genRef->getType() != general_ref_type::UNIT_WORKER)
-            return false;
-    }
+    // manually handle DESTROY_BUILDING jobs (cancel_job doesn't handle them)
+    if (job->job_type == df::job_type::DestroyBuilding) {
+        for (auto &genRef : job->general_refs) {
+            disconnectJobGeneralRef(job, genRef);
+            if (genRef) delete genRef;
+        }
+        job->general_refs.resize(0);
 
-    for (auto genRef : job->general_refs) {
-        // this should always succeed because of the check in the preceding loop
-        bool success = disconnectJobGeneralRef(job, genRef);
-        assert(success); (void)success;
-        if (genRef) delete genRef;
+        // remove the job from the world
+        job->list_link->prev->next = job->list_link->next;
+        delete job->list_link;
+        delete job;
+        return true;
     }
-    job->general_refs.resize(0);
 
-    // clean up item refs
+    // clean up item refs and delete them
     for (auto &item_ref : job->items) {
        disconnectJobItem(job, item_ref);
        if (item_ref) delete item_ref;