ensure refs are cleaned up when we remove a job (#2184)

* ensure job items are disassociated from the job

when the job is removed. the new df-provided ``cancel_job()`` doesn't do
this for us whereas the old custom implementation did.
ref: #2028

* remove trailing whitespace

* Clean up general refs before removing job

Because the game method doesn't do it itself

* Fix typo in var name

* clean up code

* update changelog

docs/changelog.txt

@@ -39,6 +39,7 @@ changelog.txt uses a syntax similar to RST, with a few special sequences:
 
 ## Fixes
 - ``widgets.CycleHotkeyLabel``: allow initial option values to be specified as an index instead of an option value
+- ``job.removeJob()``: fixes regression in DFHack 0.47.05-r5 where items/buildings associated with the job were not getting disassociated when the job is removed. Now `build-now` can build buildings and `gui/mass-remove` can cancel building deconstruction again
 
 ## Misc Improvements
 - `confirm`: added a confirmation dialog for removing manager orders

library/modules/Job.cpp

@@ -298,10 +298,10 @@ void DFHack::Job::setJobCooldown(df::building *workshop, df::unit *worker, int c
     }
 }
 
-void DFHack::Job::disconnectJobItem(df::job *job, df::job_item_ref *ref) {
-    if (!ref) return;
+void DFHack::Job::disconnectJobItem(df::job *job, df::job_item_ref *item_ref) {
+    if (!item_ref) return;
 
-    auto item = ref->item;
+    auto item = item_ref->item;
     if (!item) return;
 
     //Work backward through the specific refs & remove/delete all specific refs to this job
@@ -360,10 +360,36 @@ bool DFHack::Job::removeJob(df::job* job) {
     using df::global::world;
     CHECK_NULL_POINTER(job);
 
-    // call the job cancel vmethod graciously provided by The Toady One.
-    // job_handler::cancel_job calls job::~job, and then deletes job (this has been confirmed by disassembly)
-    // this method cannot fail; it will either delete the job or crash/corrupt DF
+    // cancel_job below does not clean up refs, so we have to do that first
+
+    // 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;
+    }
 
+    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;
+    }
+    job->general_refs.resize(0);
+
+    // clean up item refs
+    for (auto &item_ref : job->items) {
+       disconnectJobItem(job, item_ref);
+       if (item_ref) delete item_ref;
+    }
+    job->items.resize(0);
+
+    // call the job cancel vmethod graciously provided by The Toady One.
+    // job_handler::cancel_job calls job::~job, and then deletes job (this has
+    // been confirmed by disassembly).
     world->jobs.cancel_job(job);
 
     return true;