From 74f5df99db12bd111785fd6e3cfc49ca5b460ab9 Mon Sep 17 00:00:00 2001 From: Stephen Baynham Date: Thu, 17 Nov 2016 00:11:15 -0800 Subject: [PATCH 1/8] Add job remove method Job remove eliminates a job's worker & holder references, if any, puts the worker on cd, if appropriate, removes the job's postings, eliminates the job from the global linked list, and then finally deletes it. This code was tested by incorporating it into autochop and it does make the plugin work. However, chop jobs don't have holder building references, and anyway, with DF being 90% edge case by volume, this could use a heck of a lot more testing. I saw elsewhere code that prevented worker removal if the job was a special job, and that made me feel funny so I made the job remove method not work if the job is a special job. --- library/include/modules/Job.h | 5 +++ library/modules/Job.cpp | 65 ++++++++++++++++++++++++++++++++++- 2 files changed, 69 insertions(+), 1 deletion(-) diff --git a/library/include/modules/Job.h b/library/include/modules/Job.h index fbbed9ff3..78a694b6f 100644 --- a/library/include/modules/Job.h +++ b/library/include/modules/Job.h @@ -66,6 +66,11 @@ namespace DFHack DFHACK_EXPORT void setJobCooldown(df::building *workshop, df::unit *worker, int cooldown = 100); DFHACK_EXPORT bool removeWorker(df::job *job, int cooldown = 100); + // Delete a job & remove all refs from everywhere. + // This method DELETES the job object! Everything related to it will be wiped + // clean from the earth, so make sure you pull what you need out before calling this! + DFHACK_EXPORT void removeJob(df::job *job); + // Instruct the game to check and assign workers DFHACK_EXPORT void checkBuildingsNow(); DFHACK_EXPORT void checkDesignationsNow(); diff --git a/library/modules/Job.cpp b/library/modules/Job.cpp index 2678eadba..d187989a0 100644 --- a/library/modules/Job.cpp +++ b/library/modules/Job.cpp @@ -212,7 +212,7 @@ void DFHack::Job::printJobDetails(color_ostream &out, df::job *job) out.color(job->flags.bits.suspend ? COLOR_DARKGREY : COLOR_GREY); out << "Job " << job->id << ": " << ENUM_KEY_STR(job_type,job->job_type); if (job->flags.whole) - out << " (" << bitfield_to_string(job->flags) << ")"; + out << " (" << bitfield_to_string(job->flags) << ")"; out << endl; out.reset_color(); @@ -304,6 +304,68 @@ void DFHack::Job::setJobCooldown(df::building *workshop, df::unit *worker, int c } } +void DFHack::Job::removeJob(df::job *job) { + using df::global::world; + CHECK_NULL_POINTER(job); + + if (job->flags.bits.special) //I don't think you can cancel these, because DF wasn't build to expect it? + return; + + //As far as I know there are only two general refs jobs have, the unit assigned to work it (if any) + //and the workshop it was created at (if any). It's possible there are others, so we go ahead and wipe all + //refs, but these two are the only ones that we really handle with any intelligence. If other refs + //exist that might have return-references that need to be cleared, that needs to be implemented!!!! + auto holderRef = getGeneralRef(job, general_ref_type::BUILDING_HOLDER); + auto workerRef = getGeneralRef(job, general_ref_type::UNIT_WORKER); + df::building *holder = NULL; + df::unit *worker = NULL; + + if (holderRef) holder = holderRef->getBuilding(); + if (workerRef) worker = workerRef->getUnit(); + + //removeWorker() adds a job cd about right now, but I chose not to do that because I'm pretty sure + //that's only to stop removed workers from immediately reclaiming the job before doing something + //else, and this job is gonna be dead in a second. + + //Remove return-refs from the holder & worker + if (holder) { + int jobIndex = linear_index(holder->jobs, job); + if (jobIndex >= 0) + vector_erase_at(holder->jobs, jobIndex); + } + + if (worker) { + if (worker->job.current_job == job) + worker->job.current_job = NULL; + } + + //Wipe all refs out + while (job->general_refs.size() > 0) { + auto ref = job->general_refs[0]; + vector_erase_at(job->general_refs, 0); + delete ref; + } + + //Remove job from job board + Job::removePostings(job, true); + + //Remove job from global list + if (job->list_link) { + auto prev = job->list_link->prev; + auto next = job->list_link->next; + + if (prev) + prev->next = next; + + if (next) + next->prev = prev; + + delete job->list_link; + } + + delete job; +} + bool DFHack::Job::removeWorker(df::job *job, int cooldown) { CHECK_NULL_POINTER(job); @@ -397,6 +459,7 @@ bool DFHack::Job::removePostings(df::job *job, bool remove_all) { if ((**it).job == job) { + (**it).job = NULL; (**it).flags.bits.dead = true; removed = true; } From 67af9f5e82ff0c45043aa2508f8346b17e77d5a9 Mon Sep 17 00:00:00 2001 From: Stephen Baynham Date: Thu, 17 Nov 2016 23:04:48 -0800 Subject: [PATCH 2/8] Add lua bindings for removeJob --- library/LuaApi.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/library/LuaApi.cpp b/library/LuaApi.cpp index 712ccb541..4416a9066 100644 --- a/library/LuaApi.cpp +++ b/library/LuaApi.cpp @@ -1469,6 +1469,7 @@ static const LuaWrapper::FunctionReg dfhack_job_module[] = { WRAPM(Job,getName), WRAPM(Job,linkIntoWorld), WRAPM(Job,removePostings), + WRAPM(Job,removeJob), WRAPN(is_equal, jobEqual), WRAPN(is_item_equal, jobItemEqual), { NULL, NULL } From fba32f2e2fb2256037a358e2d0170c0607b65f6c Mon Sep 17 00:00:00 2001 From: Stephen Baynham Date: Thu, 17 Nov 2016 23:25:48 -0800 Subject: [PATCH 3/8] Also disconnect the job from its items. --- library/modules/Job.cpp | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/library/modules/Job.cpp b/library/modules/Job.cpp index d187989a0..f8bf00240 100644 --- a/library/modules/Job.cpp +++ b/library/modules/Job.cpp @@ -346,6 +346,33 @@ void DFHack::Job::removeJob(df::job *job) { delete ref; } + //Detach all items from the job + while (job->items.size() > 0) { + auto itemRef = job->items[0]; + df::item *item = NULL; + + if (itemRef) { + item = itemRef->item; + + if (item) { + item->flags.bits.in_job = false; + + //Work backward through the specific refs & remove/delete all specific refs to this job + int refCount = item->specific_refs.size(); + for(int refIndex = refCount-1; refIndex >= 0; refIndex--) { + auto ref = item->specific_refs[refIndex]; + if (ref->type == df::specific_ref_type::JOB && ref->job == job) { + vector_erase_at(item->specific_refs, refIndex); + delete ref; + } + } + } + + delete itemRef; + } + vector_erase_at(job->items, 0); + } + //Remove job from job board Job::removePostings(job, true); From 8b964ca2dcdd7bbf33d099d4916a185fde8dd8ca Mon Sep 17 00:00:00 2001 From: Stephen Baynham Date: Mon, 21 Nov 2016 06:51:21 -0800 Subject: [PATCH 4/8] Wipe job_items vector --- library/modules/Job.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/library/modules/Job.cpp b/library/modules/Job.cpp index f8bf00240..0ee17ddd1 100644 --- a/library/modules/Job.cpp +++ b/library/modules/Job.cpp @@ -376,6 +376,15 @@ void DFHack::Job::removeJob(df::job *job) { //Remove job from job board Job::removePostings(job, true); + //Clean up job_items + while (job->job_items.size() > 0) { + auto jobItem = job->job_items[0]; + vector_erase_at(job->job_items, 0); + if (jobItem) { + delete jobItem; + } + } + //Remove job from global list if (job->list_link) { auto prev = job->list_link->prev; From e490afdf0058da782f6d9b8c1c09f559fcd2c81f Mon Sep 17 00:00:00 2001 From: Stephen Baynham Date: Thu, 24 Nov 2016 22:36:11 -0800 Subject: [PATCH 5/8] Rebuilt slightly to offer bool return val We fail on unknown general ref types now, without modifying the job at all yet --- library/LuaApi.cpp | 2 + library/include/modules/Job.h | 12 +++- library/modules/Job.cpp | 126 ++++++++++++++++++++-------------- 3 files changed, 89 insertions(+), 51 deletions(-) diff --git a/library/LuaApi.cpp b/library/LuaApi.cpp index 4416a9066..7f0e629a7 100644 --- a/library/LuaApi.cpp +++ b/library/LuaApi.cpp @@ -1469,6 +1469,8 @@ static const LuaWrapper::FunctionReg dfhack_job_module[] = { WRAPM(Job,getName), WRAPM(Job,linkIntoWorld), WRAPM(Job,removePostings), + WRAPM(Job,disconnectJobItem), + WRAPM(Job,disconnectJobGeneralRef), WRAPM(Job,removeJob), WRAPN(is_equal, jobEqual), WRAPN(is_item_equal, jobItemEqual), diff --git a/library/include/modules/Job.h b/library/include/modules/Job.h index 78a694b6f..1448b6656 100644 --- a/library/include/modules/Job.h +++ b/library/include/modules/Job.h @@ -66,10 +66,20 @@ namespace DFHack DFHACK_EXPORT void setJobCooldown(df::building *workshop, df::unit *worker, int cooldown = 100); DFHACK_EXPORT bool removeWorker(df::job *job, int cooldown = 100); + // This helpful method only removes the backref from the item to the job, but it doesn't + // remove the item ref from the job's vector, or delete it or anything. Think of it as a method + // that does all the needful to make an item ref ready to delete. + DFHACK_EXPORT void disconnectJobItem(df::job_item_ref *item, df::job *job); + // This helpful method only removes the backref from whatever the general_ref points to, + // it doesn't remove the general_ref from the job's vector, or delete it or anything. + // Think of it as a method that does all the needful to make a ref ready to delete. + // If it returns false, you've found a ref that the method doesn't know how to handle. Congratulations! + // You should report that and/or check in a fix. + DFHACK_EXPORT bool disconnectJobGeneralRef(df::general_ref *ref, df::job *job); // Delete a job & remove all refs from everywhere. // This method DELETES the job object! Everything related to it will be wiped // clean from the earth, so make sure you pull what you need out before calling this! - DFHACK_EXPORT void removeJob(df::job *job); + DFHACK_EXPORT bool removeJob(df::job *job); // Instruct the game to check and assign workers DFHACK_EXPORT void checkBuildingsNow(); diff --git a/library/modules/Job.cpp b/library/modules/Job.cpp index 0ee17ddd1..b1f262f09 100644 --- a/library/modules/Job.cpp +++ b/library/modules/Job.cpp @@ -304,73 +304,98 @@ void DFHack::Job::setJobCooldown(df::building *workshop, df::unit *worker, int c } } -void DFHack::Job::removeJob(df::job *job) { +void DFHack::Job::disconnectJobItem(df::job_item_ref *ref, df::job *job) { + if (!ref) return; + + auto item = ref->item; + if (!item) return; + + //Work backward through the specific refs & remove/delete all specific refs to this job + int refCount = item->specific_refs.size(); + bool stillHasJobs = false; + for(int refIndex = refCount-1; refIndex >= 0; refIndex--) { + auto ref = item->specific_refs[refIndex]; + + if (ref->type == df::specific_ref_type::JOB) { + if (ref->job == job) { + vector_erase_at(item->specific_refs, refIndex); + delete ref; + } else { + stillHasJobs = true; + } + } + } + + if (!stillHasJobs) item->flags.bits.in_job = false; +} + +bool DFHack::Job::disconnectJobGeneralRef(df::general_ref *ref, df::job *job) { + if (ref == NULL) return true; + + switch (ref->getType()) { + case general_ref_type::BUILDING_HOLDER: + auto building = ref->getBuilding(); + + if (building != NULL) { + int jobIndex = linear_index(building->jobs, job); + if (jobIndex >= 0) { + vector_erase_at(building->jobs, jobIndex); + } + } + break; + case general_ref_type::UNIT_WORKER: + auto unit = ref->getUnit(); + + if (unit != NULL) { + if (unit->job.current_job == job) { + unit->job.current_job = NULL; + } + } + break; + default: + return false; + } + + return true; +} + +bool DFHack::Job::removeJob(df::job *job) { using df::global::world; CHECK_NULL_POINTER(job); if (job->flags.bits.special) //I don't think you can cancel these, because DF wasn't build to expect it? - return; + return false; - //As far as I know there are only two general refs jobs have, the unit assigned to work it (if any) - //and the workshop it was created at (if any). It's possible there are others, so we go ahead and wipe all - //refs, but these two are the only ones that we really handle with any intelligence. If other refs - //exist that might have return-references that need to be cleared, that needs to be implemented!!!! - auto holderRef = getGeneralRef(job, general_ref_type::BUILDING_HOLDER); - auto workerRef = getGeneralRef(job, general_ref_type::UNIT_WORKER); - df::building *holder = NULL; - df::unit *worker = NULL; - - if (holderRef) holder = holderRef->getBuilding(); - if (workerRef) worker = workerRef->getUnit(); - - //removeWorker() adds a job cd about right now, but I chose not to do that because I'm pretty sure - //that's only to stop removed workers from immediately reclaiming the job before doing something - //else, and this job is gonna be dead in a second. - - //Remove return-refs from the holder & worker - if (holder) { - int jobIndex = linear_index(holder->jobs, job); - if (jobIndex >= 0) - vector_erase_at(holder->jobs, jobIndex); - } + //We actually only know how to handle BUILDING_HOLDER and UNIT_WORKER refs- there's probably a great + //way to handle them, but until we have a good example, we'll just fail to remove jobs that have other sorts + //of refs, or any specific refs + if (job->specific_refs.size() > 0) + return false; - if (worker) { - if (worker->job.current_job == job) - worker->job.current_job = NULL; + for (auto genRefItr = job->general_refs.begin(); genRefItr != job->general_refs.end(); ++genRefItr) { + auto ref = *genRefItr; + if (ref != NULL && (ref->getType() != general_ref_type::BUILDING_HOLDER && ref->getType() != general_ref_type::UNIT_WORKER)) + return false; } - //Wipe all refs out + //Disconnect, delete, and wipe all general refs while (job->general_refs.size() > 0) { auto ref = job->general_refs[0]; + + //Our code above should have ensured that this won't return false- if it does, there's not + //a great way of recovering since we can't properly destroy the job & we can't leave it + //around. Better to know the moment that becomes a problem. + assert(disconnectJobGeneralRef(ref, job)); vector_erase_at(job->general_refs, 0); - delete ref; + if (ref != NULL) delete ref; } //Detach all items from the job while (job->items.size() > 0) { auto itemRef = job->items[0]; - df::item *item = NULL; - - if (itemRef) { - item = itemRef->item; - - if (item) { - item->flags.bits.in_job = false; - - //Work backward through the specific refs & remove/delete all specific refs to this job - int refCount = item->specific_refs.size(); - for(int refIndex = refCount-1; refIndex >= 0; refIndex--) { - auto ref = item->specific_refs[refIndex]; - if (ref->type == df::specific_ref_type::JOB && ref->job == job) { - vector_erase_at(item->specific_refs, refIndex); - delete ref; - } - } - } - - delete itemRef; - } + disconnectJobItem(itemRef, job); vector_erase_at(job->items, 0); + if (itemRef != NULL) delete itemRef; } //Remove job from job board @@ -400,6 +425,7 @@ void DFHack::Job::removeJob(df::job *job) { } delete job; + return true; } bool DFHack::Job::removeWorker(df::job *job, int cooldown) From de0e211e07dbc0e044096efbaf783d31d9759046 Mon Sep 17 00:00:00 2001 From: Stephen Baynham Date: Thu, 24 Nov 2016 23:35:03 -0800 Subject: [PATCH 6/8] Figured I could like test my code. --- library/modules/Job.cpp | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/library/modules/Job.cpp b/library/modules/Job.cpp index b1f262f09..531c85773 100644 --- a/library/modules/Job.cpp +++ b/library/modules/Job.cpp @@ -332,9 +332,12 @@ void DFHack::Job::disconnectJobItem(df::job_item_ref *ref, df::job *job) { bool DFHack::Job::disconnectJobGeneralRef(df::general_ref *ref, df::job *job) { if (ref == NULL) return true; + df::building *building = NULL; + df::unit *unit = NULL; + switch (ref->getType()) { case general_ref_type::BUILDING_HOLDER: - auto building = ref->getBuilding(); + building = ref->getBuilding(); if (building != NULL) { int jobIndex = linear_index(building->jobs, job); @@ -344,7 +347,7 @@ bool DFHack::Job::disconnectJobGeneralRef(df::general_ref *ref, df::job *job) { } break; case general_ref_type::UNIT_WORKER: - auto unit = ref->getUnit(); + unit = ref->getUnit(); if (unit != NULL) { if (unit->job.current_job == job) { @@ -385,7 +388,9 @@ bool DFHack::Job::removeJob(df::job *job) { //Our code above should have ensured that this won't return false- if it does, there's not //a great way of recovering since we can't properly destroy the job & we can't leave it //around. Better to know the moment that becomes a problem. - assert(disconnectJobGeneralRef(ref, job)); + bool success = disconnectJobGeneralRef(ref, job); + assert(success); + vector_erase_at(job->general_refs, 0); if (ref != NULL) delete ref; } From 7920f71517622ef2f28cdd1cab6be2d0ec25d191 Mon Sep 17 00:00:00 2001 From: Stephen Baynham Date: Fri, 25 Nov 2016 00:25:18 -0800 Subject: [PATCH 7/8] Bad formatting --- library/include/modules/Job.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/library/include/modules/Job.h b/library/include/modules/Job.h index 1448b6656..439a4484d 100644 --- a/library/include/modules/Job.h +++ b/library/include/modules/Job.h @@ -68,16 +68,16 @@ namespace DFHack // This helpful method only removes the backref from the item to the job, but it doesn't // remove the item ref from the job's vector, or delete it or anything. Think of it as a method - // that does all the needful to make an item ref ready to delete. + // that does all the needful to make an item ref ready to delete. DFHACK_EXPORT void disconnectJobItem(df::job_item_ref *item, df::job *job); // This helpful method only removes the backref from whatever the general_ref points to, // it doesn't remove the general_ref from the job's vector, or delete it or anything. - // Think of it as a method that does all the needful to make a ref ready to delete. + // Think of it as a method that does all the needful to make a ref ready to delete. // If it returns false, you've found a ref that the method doesn't know how to handle. Congratulations! // You should report that and/or check in a fix. DFHACK_EXPORT bool disconnectJobGeneralRef(df::general_ref *ref, df::job *job); // Delete a job & remove all refs from everywhere. - // This method DELETES the job object! Everything related to it will be wiped + // This method DELETES the job object! Everything related to it will be wiped // clean from the earth, so make sure you pull what you need out before calling this! DFHACK_EXPORT bool removeJob(df::job *job); From 595f3857b6b11cc42adc518fce787e914e54536c Mon Sep 17 00:00:00 2001 From: Stephen Baynham Date: Thu, 1 Dec 2016 20:13:49 -0800 Subject: [PATCH 8/8] Reverse the param order of these two methods The current way doesn't match other Job module methods --- library/include/modules/Job.h | 4 ++-- library/modules/Job.cpp | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/library/include/modules/Job.h b/library/include/modules/Job.h index 439a4484d..814f1e062 100644 --- a/library/include/modules/Job.h +++ b/library/include/modules/Job.h @@ -69,13 +69,13 @@ namespace DFHack // This helpful method only removes the backref from the item to the job, but it doesn't // remove the item ref from the job's vector, or delete it or anything. Think of it as a method // that does all the needful to make an item ref ready to delete. - DFHACK_EXPORT void disconnectJobItem(df::job_item_ref *item, df::job *job); + DFHACK_EXPORT void disconnectJobItem(df::job *job, df::job_item_ref *item); // This helpful method only removes the backref from whatever the general_ref points to, // it doesn't remove the general_ref from the job's vector, or delete it or anything. // Think of it as a method that does all the needful to make a ref ready to delete. // If it returns false, you've found a ref that the method doesn't know how to handle. Congratulations! // You should report that and/or check in a fix. - DFHACK_EXPORT bool disconnectJobGeneralRef(df::general_ref *ref, df::job *job); + DFHACK_EXPORT bool disconnectJobGeneralRef(df::job *job, df::general_ref *ref); // Delete a job & remove all refs from everywhere. // This method DELETES the job object! Everything related to it will be wiped // clean from the earth, so make sure you pull what you need out before calling this! diff --git a/library/modules/Job.cpp b/library/modules/Job.cpp index 531c85773..3f12ea2c2 100644 --- a/library/modules/Job.cpp +++ b/library/modules/Job.cpp @@ -304,7 +304,7 @@ void DFHack::Job::setJobCooldown(df::building *workshop, df::unit *worker, int c } } -void DFHack::Job::disconnectJobItem(df::job_item_ref *ref, df::job *job) { +void DFHack::Job::disconnectJobItem(df::job *job, df::job_item_ref *ref) { if (!ref) return; auto item = ref->item; @@ -329,7 +329,7 @@ void DFHack::Job::disconnectJobItem(df::job_item_ref *ref, df::job *job) { if (!stillHasJobs) item->flags.bits.in_job = false; } -bool DFHack::Job::disconnectJobGeneralRef(df::general_ref *ref, df::job *job) { +bool DFHack::Job::disconnectJobGeneralRef(df::job *job, df::general_ref *ref) { if (ref == NULL) return true; df::building *building = NULL; @@ -388,7 +388,7 @@ bool DFHack::Job::removeJob(df::job *job) { //Our code above should have ensured that this won't return false- if it does, there's not //a great way of recovering since we can't properly destroy the job & we can't leave it //around. Better to know the moment that becomes a problem. - bool success = disconnectJobGeneralRef(ref, job); + bool success = disconnectJobGeneralRef(job, ref); assert(success); vector_erase_at(job->general_refs, 0); @@ -398,7 +398,7 @@ bool DFHack::Job::removeJob(df::job *job) { //Detach all items from the job while (job->items.size() > 0) { auto itemRef = job->items[0]; - disconnectJobItem(itemRef, job); + disconnectJobItem(job, itemRef); vector_erase_at(job->items, 0); if (itemRef != NULL) delete itemRef; }