From 9fcb20888ff1774bff2e3d70ed8c7cdcec1f8543 Mon Sep 17 00:00:00 2001 From: Kelly Kinkade Date: Sun, 20 Nov 2022 11:57:52 -0600 Subject: [PATCH 1/6] realign autohauler and autolabor with each other also clean up C++ code smells in both --- plugins/autolabor/autohauler.cpp | 280 +++++++------------------------ plugins/autolabor/autolabor.cpp | 116 ++++++------- 2 files changed, 113 insertions(+), 283 deletions(-) diff --git a/plugins/autolabor/autohauler.cpp b/plugins/autolabor/autohauler.cpp index 691dc2e8e..c9b746581 100644 --- a/plugins/autolabor/autohauler.cpp +++ b/plugins/autolabor/autohauler.cpp @@ -1,6 +1,3 @@ -/** - * All includes copied from autolabor for now - */ #include "Core.h" #include #include @@ -45,21 +42,15 @@ #include "laborstatemap.h" -// Not sure what this does, but may have to figure it out later -#define ARRAY_COUNT(array) (sizeof(array)/sizeof((array)[0])) - -// I can see a reason for having all of these -using std::string; -using std::endl; -using std::vector; using namespace DFHack; using namespace df::enums; -// idk what this does DFHACK_PLUGIN("autohauler"); REQUIRE_GLOBAL(ui); REQUIRE_GLOBAL(world); +#define ARRAY_COUNT(array) (sizeof(array)/sizeof((array)[0])) + /* * Autohauler module for dfhack * Fork of autolabor, DFHack version 0.40.24-r2 @@ -86,37 +77,29 @@ REQUIRE_GLOBAL(world); * autolabor.plug.dll as this plugin is mutually exclusive with it. */ -// Yep...don't know what it does DFHACK_PLUGIN_IS_ENABLED(enable_autohauler); -// This is the configuration saved into the world save file -static PersistentDataItem config; - -// There is a possibility I will add extensive, line-by-line debug capability -// later static bool print_debug = false; -// Default number of frames between autohauler updates +static std::vector state_count(NUM_STATE); + const static int DEFAULT_FRAME_SKIP = 30; -// Number of frames between autohauler updates -static int frame_skip; +static PersistentDataItem config; -// Don't know what this does command_result autohauler (color_ostream &out, std::vector & parameters); -// Don't know what this does either +static int frame_skip; + static bool isOptionEnabled(unsigned flag) { return config.isValid() && (config.ival(0) & flag) != 0; } -// Not sure about the purpose of this enum ConfigFlags { CF_ENABLED = 1, }; -// Don't know what this does static void setOptionEnabled(ConfigFlags flag, bool on) { if (!config.isValid()) @@ -128,49 +111,34 @@ static void setOptionEnabled(ConfigFlags flag, bool on) config.ival(0) &= ~flag; } -// This is a vector of states and number of dwarves in that state -static std::vector state_count(NUM_STATE); - -// Mode assigned to labors. Either it's a hauling job, or it's not. enum labor_mode { ALLOW, HAULERS, FORBID }; -// This is the default treatment of a particular labor. -struct labor_default -{ - labor_mode mode; - int active_dwarfs; -}; - -// This is the current treatment of a particular labor. -// This would have been more cleanly presented as a class struct labor_info { - // It seems as if this is the means of accessing the world data PersistentDataItem config; - // Number of dwarves assigned this labor int active_dwarfs; - // Set the persistent data item associated with this labor treatment - // We Java folk hate pointers, but that's what the parameter will be - void set_config(PersistentDataItem a) { config = a; } - - // Return the labor_mode associated with this labor labor_mode mode() { return (labor_mode) config.ival(0); } - // Set the labor_mode associated with this labor void set_mode(labor_mode mode) { config.ival(0) = mode; } + + void set_config(PersistentDataItem a) { config = a; } + + }; + +struct labor_default +{ + labor_mode mode; + int active_dwarfs; }; -// This is a vector of all the current labor treatments static std::vector labor_infos; -// This is just an array of all the labors, whether it should be untouched -// (DISABLE) or treated as a last-resort job (HAULERS). static const struct labor_default default_labor_infos[] = { /* MINE */ {ALLOW, 0}, /* HAUL_STONE */ {HAULERS, 0}, @@ -257,36 +225,24 @@ static const struct labor_default default_labor_infos[] = { /* BOOKBINDING */ {ALLOW, 0} }; -/** - * Reset labor to default treatment - */ -static void reset_labor(df::unit_labor labor) -{ - labor_infos[labor].set_mode(default_labor_infos[labor].mode); -} - -/** - * This is individualized dwarf info populated in plugin_onupdate - */ struct dwarf_info_t { - // Current simplified employment status of dwarf dwarf_state state; - // Set to true if for whatever reason we are exempting this dwarf - // from hauling bool haul_exempt; }; -/** - * Disable autohauler labor lists - */ static void cleanup_state() { enable_autohauler = false; labor_infos.clear(); } +static void reset_labor(df::unit_labor labor) +{ + labor_infos[labor].set_mode(default_labor_infos[labor].mode); +} + static void enable_alchemist(color_ostream &out) { if (!Units::setLaborValidity(unit_labor::ALCHEMIST, true)) @@ -297,29 +253,18 @@ static void enable_alchemist(color_ostream &out) } } -/** - * Initialize the plugin labor lists - */ static void init_state(color_ostream &out) { - // This obtains the persistent data from the world save file config = World::GetPersistentData("autohauler/config"); - // Check to ensure that the persistent data item actually exists and that - // the first item in the array of ints isn't -1 (implies disabled) if (config.isValid() && config.ival(0) == -1) config.ival(0) = 0; - // Check to see if the plugin is enabled in the persistent data, if so then - // enable the local flag for autohauler being enabled enable_autohauler = isOptionEnabled(CF_ENABLED); - // If autohauler is not enabled then it's pretty pointless to do the rest if (!enable_autohauler) return; - // First get the frame skip from persistent data, or create the item - // if not present auto cfg_frameskip = World::GetPersistentData("autohauler/frameskip"); if (cfg_frameskip.isValid()) { @@ -327,126 +272,78 @@ static void init_state(color_ostream &out) } else { - // Add to persistent data then get it to assert it's actually there cfg_frameskip = World::AddPersistentData("autohauler/frameskip"); cfg_frameskip.ival(0) = DEFAULT_FRAME_SKIP; frame_skip = cfg_frameskip.ival(0); } + labor_infos.resize(ARRAY_COUNT(default_labor_infos)); - /* Here we are going to populate the labor list by loading persistent data - * from the world save */ - - // This is a vector of all the persistent data items from config std::vector items; - - // This populates the aforementioned vector World::GetPersistentData(&items, "autohauler/labors/", true); - // Resize the list of current labor treatments to size of list of default - // labor treatments - labor_infos.resize(ARRAY_COUNT(default_labor_infos)); - // For every persistent data item... - for (auto p = items.begin(); p != items.end(); p++) + for (auto& p : items) { - // Load as a string the key associated with the persistent data item - string key = p->key(); - - // Translate the string into a labor defined by global dfhack constants + std::string key = p.key(); df::unit_labor labor = (df::unit_labor) atoi(key.substr(strlen("autohauler/labors/")).c_str()); - - // Ensure that the labor is defined in the existing list if (labor >= 0 && size_t(labor) < labor_infos.size()) { - // Link the labor treatment with the associated persistent data item - labor_infos[labor].set_config(*p); - - // Set the number of dwarves associated with labor to zero + labor_infos[labor].set_config(p); labor_infos[labor].active_dwarfs = 0; } } // Add default labors for those not in save for (size_t i = 0; i < ARRAY_COUNT(default_labor_infos); i++) { - - // Determine if the labor is already present. If so, exit the for loop if (labor_infos[i].config.isValid()) continue; - // Not sure of the mechanics, but it seems to give an output stream - // giving a string for the new persistent data item std::stringstream name; name << "autohauler/labors/" << i; - // Add a new persistent data item as it is not currently in the save labor_infos[i].set_config(World::AddPersistentData(name.str())); - // Set the active counter to zero labor_infos[i].active_dwarfs = 0; - - // Reset labor to default treatment reset_labor((df::unit_labor) i); } - // Allow Alchemist to be set in the labor-related UI screens so the player - // can actually use it as a flag without having to run Dwarf Therapist enable_alchemist(out); } -/** - * Call this method to enable the plugin. - */ static void enable_plugin(color_ostream &out) { - // If there is no config persistent item, make one if (!config.isValid()) { config = World::AddPersistentData("autohauler/config"); config.ival(0) = 0; } - // I think this is already done in init_state(), but it can't hurt setOptionEnabled(CF_ENABLED, true); enable_autohauler = true; + out << "Enabling the plugin." << std::endl; - // Output to console that the plugin is enabled - out << "Enabling the plugin." << endl; - - // Disable autohauler and clear the labor list cleanup_state(); - - // Initialize the plugin init_state(out); } -/** - * Initialize the plugin - */ DFhackCExport command_result plugin_init ( color_ostream &out, std::vector &commands) { - // This seems to verify that the default labor list and the current labor - // list are the same size if(ARRAY_COUNT(default_labor_infos) != ENUM_LAST_ITEM(unit_labor) + 1) { out.printerr("autohauler: labor size mismatch\n"); return CR_FAILURE; } - // Essentially an introduction dumped to the console commands.push_back(PluginCommand( "autohauler", "Automatically manage hauling labors.", autohauler)); - // Initialize plugin labor lists init_state(out); return CR_OK; } -/** - * Shut down the plugin - */ DFhackCExport command_result plugin_shutdown ( color_ostream &out ) { cleanup_state(); @@ -454,9 +351,6 @@ DFhackCExport command_result plugin_shutdown ( color_ostream &out ) return CR_OK; } -/** - * This method responds to the map being loaded, or unloaded. - */ DFhackCExport command_result plugin_onstatechange(color_ostream &out, state_change_event event) { switch (event) { @@ -474,59 +368,38 @@ DFhackCExport command_result plugin_onstatechange(color_ostream &out, state_chan return CR_OK; } -/** - * This method is called every frame in Dwarf Fortress from my understanding. - */ DFhackCExport command_result plugin_onupdate ( color_ostream &out ) { - // This makes it so that the plugin is only run every 60 steps, in order to - // save FPS. Since it is static, this is declared before this method is called static int step_count = 0; + if(!world || !world->map.block_index || !enable_autohauler) + { + return CR_OK; + } - // Cancel run if the world doesn't exist or plugin isn't enabled - if(!world || !world->map.block_index || !enable_autohauler) { return CR_OK; } - - // Increment step count - step_count++; - - // Run aforementioned step count and return unless threshold is reached. - if (step_count < frame_skip) return CR_OK; - - // Reset step count since at this point it has reached 60 + if (++step_count < frame_skip) + return CR_OK; step_count = 0; - // Create a vector of units. This will be populated in the following for loop. std::vector dwarfs; - // Scan the world and look for any citizens in the player's civilization. - // Add these to the list of dwarves. - // xxx Does it need to be ++i? - for (size_t i = 0; i < world->units.active.size(); ++i) + for (auto& cre : world->units.active) { - df::unit* cre = world->units.active[i]; if (Units::isCitizen(cre)) { dwarfs.push_back(cre); } } - // This just keeps track of the number of civilians from the previous for loop. int n_dwarfs = dwarfs.size(); - // This will return if there are no civilians. Otherwise could call - // nonexistent elements of array. if (n_dwarfs == 0) return CR_OK; - // This is a matching of assigned jobs with a dwarf's state - // xxx but wouldn't it be better if this and "dwarfs" were in the same object? std::vector dwarf_info(n_dwarfs); - // Reset the counter for number of dwarves in states to zero state_count.clear(); state_count.resize(NUM_STATE); - // Find the activity state for each dwarf for (int dwarf = 0; dwarf < n_dwarfs; dwarf++) { /* Before determining how to handle employment status, handle @@ -551,11 +424,9 @@ DFhackCExport command_result plugin_onupdate ( color_ostream &out ) // Scan a dwarf's miscellaneous traits for on break or migrant status. // If either of these are present, disable hauling because we want them // to try to find real jobs first - for (auto p = dwarfs[dwarf]->status.misc_traits.begin(); p < dwarfs[dwarf]->status.misc_traits.end(); p++) - { - if ((*p)->id == misc_trait_type::Migrant) - is_migrant = true; - } + auto v = dwarfs[dwarf]->status.misc_traits; + auto test_migrant = [](df::unit_misc_trait* t) { return t->id == misc_trait_type::Migrant; }; + is_migrant = std::find_if(v.begin(), v.end(), test_migrant ) != v.end(); /* Now determine a dwarf's employment status and decide whether * to assign hauling */ @@ -583,7 +454,6 @@ DFhackCExport command_result plugin_onupdate ( color_ostream &out ) { dwarf_info[dwarf].state = IDLE; } - // If it gets to this point we look at the task and assign either BUSY or OTHER else { int job = dwarfs[dwarf]->job.current_job->job_type; @@ -591,7 +461,6 @@ DFhackCExport command_result plugin_onupdate ( color_ostream &out ) dwarf_info[dwarf].state = dwarf_states[job]; else { - // Warn the console that the dwarf has an unregistered labor, default to BUSY out.print("Dwarf %i \"%s\" has unknown job %i\n", dwarf, dwarfs[dwarf]->name.first_name.c_str(), job); dwarf_info[dwarf].state = BUSY; } @@ -639,19 +508,13 @@ DFhackCExport command_result plugin_onupdate ( color_ostream &out ) // There was no reason to put potential haulers in an array. All of them are // covered in the following for loop. - // Equivalent of Java for(unit_labor : labor) - // For every labor... FOR_ENUM_ITEMS(unit_labor, labor) { - // If this is a non-labor continue to next item if (labor == unit_labor::NONE) continue; - - // If this is not a hauling labor continue to next item if (labor_infos[labor].mode() != HAULERS) continue; - // For every dwarf... for(size_t dwarf = 0; dwarf < dwarfs.size(); dwarf++) { if (!Units::isValidLabor(dwarfs[dwarf], labor)) @@ -676,21 +539,30 @@ DFhackCExport command_result plugin_onupdate ( color_ostream &out ) { labor_infos[labor].active_dwarfs++; } - // CHILD ignored } - - // Let's play a game of "find the missing bracket!" I hope this is correct. } - - // This would be the last statement of the method return CR_OK; } -/** - * xxx Isn't this a repeat of enable_plugin? If it is separately called, then - * passing a constructor should suffice. - */ +void print_labor (df::unit_labor labor, color_ostream &out) +{ + std::string labor_name = ENUM_KEY_STR(unit_labor, labor); + out << labor_name << ": "; + for (int i = 0; i < 20 - (int)labor_name.length(); i++) + out << ' '; + if (labor_infos[labor].mode() == ALLOW) out << "allow" << std::endl; + else if(labor_infos[labor].mode() == FORBID) out << "forbid" << std::endl; + else if(labor_infos[labor].mode() == HAULERS) + { + out << "haulers, currently " << labor_infos[labor].active_dwarfs << " dwarfs" << std::endl; + } + else + { + out << "Warning: Invalid labor mode!" << std::endl; + } +} + DFhackCExport command_result plugin_enable ( color_ostream &out, bool enable ) { if (!Core::getInstance().isWorldLoaded()) { @@ -707,36 +579,12 @@ DFhackCExport command_result plugin_enable ( color_ostream &out, bool enable ) enable_autohauler = false; setOptionEnabled(CF_ENABLED, false); - out << "Autohauler is disabled." << endl; + out << "Autohauler is disabled." << std::endl; } return CR_OK; } -/** - * Print the aggregate labor status to the console. - */ -void print_labor (df::unit_labor labor, color_ostream &out) -{ - string labor_name = ENUM_KEY_STR(unit_labor, labor); - out << labor_name << ": "; - for (int i = 0; i < 20 - (int)labor_name.length(); i++) - out << ' '; - if (labor_infos[labor].mode() == ALLOW) out << "allow" << endl; - else if(labor_infos[labor].mode() == FORBID) out << "forbid" << endl; - else if(labor_infos[labor].mode() == HAULERS) - { - out << "haulers, currently " << labor_infos[labor].active_dwarfs << " dwarfs" << endl; - } - else - { - out << "Warning: Invalid labor mode!" << endl; - } -} - -/** - * This responds to input from the command prompt. - */ command_result autohauler (color_ostream &out, std::vector & parameters) { CoreSuspender suspend; @@ -761,13 +609,13 @@ command_result autohauler (color_ostream &out, std::vector & param { int newValue = atoi(parameters[1].c_str()); cfg_frameskip.ival(0) = newValue; - out << "Setting frame skip to " << newValue << endl; + out << "Setting frame skip to " << newValue << std::endl; frame_skip = cfg_frameskip.ival(0); return CR_OK; } else { - out << "Warning! No persistent data for frame skip!" << endl; + out << "Warning! No persistent data for frame skip!" << std::endl; return CR_OK; } } @@ -775,7 +623,7 @@ command_result autohauler (color_ostream &out, std::vector & param { if (!enable_autohauler) { - out << "Error: The plugin is not enabled." << endl; + out << "Error: The plugin is not enabled." << std::endl; return CR_FAILURE; } @@ -826,7 +674,7 @@ command_result autohauler (color_ostream &out, std::vector & param { if (!enable_autohauler) { - out << "Error: The plugin is not enabled." << endl; + out << "Error: The plugin is not enabled." << std::endl; return CR_FAILURE; } @@ -834,14 +682,14 @@ command_result autohauler (color_ostream &out, std::vector & param { reset_labor((df::unit_labor) i); } - out << "All labors reset." << endl; + out << "All labors reset." << std::endl; return CR_OK; } else if (parameters.size() == 1 && (parameters[0] == "list" || parameters[0] == "status")) { if (!enable_autohauler) { - out << "Error: The plugin is not enabled." << endl; + out << "Error: The plugin is not enabled." << std::endl; return CR_FAILURE; } @@ -855,9 +703,9 @@ command_result autohauler (color_ostream &out, std::vector & param out << state_count[i] << ' ' << state_names[i]; need_comma = true; } - out << endl; + out << std::endl; - out << "Autohauler is running every " << frame_skip << " frames." << endl; + out << "Autohauler is running every " << frame_skip << " frames." << std::endl; if (parameters[0] == "list") { @@ -876,7 +724,7 @@ command_result autohauler (color_ostream &out, std::vector & param { if (!enable_autohauler) { - out << "Error: The plugin is not enabled." << endl; + out << "Error: The plugin is not enabled." << std::endl; return CR_FAILURE; } diff --git a/plugins/autolabor/autolabor.cpp b/plugins/autolabor/autolabor.cpp index 198a588cc..aaa1f5053 100644 --- a/plugins/autolabor/autolabor.cpp +++ b/plugins/autolabor/autolabor.cpp @@ -42,9 +42,6 @@ #include "laborstatemap.h" -using std::string; -using std::endl; -using std::vector; using namespace DFHack; using namespace df::enums; @@ -81,20 +78,34 @@ REQUIRE_GLOBAL(world); DFHACK_PLUGIN_IS_ENABLED(enable_autolabor); -static bool print_debug = 0; +static bool print_debug = false; -static std::vector state_count(5); +static std::vector state_count(NUM_STATE); static PersistentDataItem config; +command_result autolabor (color_ostream &out, std::vector & parameters); + +static bool isOptionEnabled(unsigned flag) +{ + return config.isValid() && (config.ival(0) & flag) != 0; +} + enum ConfigFlags { CF_ENABLED = 1, }; +static void setOptionEnabled(ConfigFlags flag, bool on) +{ + if (!config.isValid()) + return; + + if (on) + config.ival(0) |= flag; + else + config.ival(0) &= ~flag; +} -// Here go all the command declarations... -// mostly to allow having the mandatory stuff on top of the file and commands on the bottom -command_result autolabor (color_ostream &out, std::vector & parameters); static void generate_labor_to_skill_map(); @@ -104,7 +115,6 @@ enum labor_mode { AUTOMATIC, }; - struct labor_info { PersistentDataItem config; @@ -113,6 +123,7 @@ struct labor_info int active_dwarfs; labor_mode mode() { return (labor_mode) config.ival(0); } + void set_mode(labor_mode mode) { config.ival(0) = mode; } int minimum_dwarfs() { return config.ival(1); } @@ -273,22 +284,6 @@ struct dwarf_info_t bool diplomacy; // this dwarf meets with diplomats }; -static bool isOptionEnabled(unsigned flag) -{ - return config.isValid() && (config.ival(0) & flag) != 0; -} - -static void setOptionEnabled(ConfigFlags flag, bool on) -{ - if (!config.isValid()) - return; - - if (on) - config.ival(0) |= flag; - else - config.ival(0) &= ~flag; -} - static void cleanup_state() { enable_autolabor = false; @@ -330,13 +325,13 @@ static void init_state() std::vector items; World::GetPersistentData(&items, "autolabor/labors/", true); - for (auto p = items.begin(); p != items.end(); p++) + for (auto& p : items) { - string key = p->key(); + std::string key = p.key(); df::unit_labor labor = (df::unit_labor) atoi(key.substr(strlen("autolabor/labors/")).c_str()); if (labor >= 0 && size_t(labor) < labor_infos.size()) { - labor_infos[labor].config = *p; + labor_infos[labor].config = p; labor_infos[labor].is_exclusive = default_labor_infos[labor].is_exclusive; labor_infos[labor].active_dwarfs = 0; } @@ -397,7 +392,7 @@ static void enable_plugin(color_ostream &out) setOptionEnabled(CF_ENABLED, true); enable_autolabor = true; - out << "Enabling autolabor." << endl; + out << "Enabling autolabor." << std::endl; cleanup_state(); init_state(); @@ -412,7 +407,6 @@ DFhackCExport command_result plugin_init ( color_ostream &out, std::vector status.souls[0]->skills.begin(); s < dwarfs[dwarf]->status.souls[0]->skills.end(); s++) + for (auto s : dwarfs[dwarf]->status.souls[0]->skills) { - if ((*s)->id == skill) + if (s->id == skill) { - skill_level = (*s)->rating; - skill_experience = (*s)->experience; + skill_level = s->rating; + skill_experience = s->experience; break; } } @@ -713,10 +707,8 @@ DFhackCExport command_result plugin_onstatechange(color_ostream &out, state_chan DFhackCExport command_result plugin_onupdate ( color_ostream &out ) { static int step_count = 0; - // check run conditions if(!world || !world->map.block_index || !enable_autolabor) { - // give up if we shouldn't be running' return CR_OK; } @@ -730,9 +722,8 @@ DFhackCExport command_result plugin_onupdate ( color_ostream &out ) bool has_fishery = false; bool trader_requested = false; - for (size_t i = 0; i < world->buildings.all.size(); ++i) + for (auto& build : world->buildings.all) { - df::building *build = world->buildings.all[i]; auto type = build->getType(); if (building_type::Workshop == type) { @@ -756,9 +747,8 @@ DFhackCExport command_result plugin_onupdate ( color_ostream &out ) } } - for (size_t i = 0; i < world->units.active.size(); ++i) + for (auto& cre : world->units.active) { - df::unit* cre = world->units.active[i]; if (Units::isCitizen(cre)) { if (cre->burrows.size() > 0) @@ -787,9 +777,8 @@ DFhackCExport command_result plugin_onupdate ( color_ostream &out ) df::historical_figure* hf = df::historical_figure::find(dwarfs[dwarf]->hist_figure_id); if(hf!=NULL) //can be NULL. E.g. script created citizens - for (size_t i = 0; i < hf->entity_links.size(); i++) + for (auto& hfelink : hf->entity_links) { - df::histfig_entity_link* hfelink = hf->entity_links.at(i); if (hfelink->getType() == df::histfig_entity_link_type::POSITION) { df::histfig_entity_link_positionst *epos = @@ -822,9 +811,8 @@ DFhackCExport command_result plugin_onupdate ( color_ostream &out ) // identify dwarfs who are needed for meetings and mark them for exclusion - for (size_t i = 0; i < ui->activities.size(); ++i) + for (auto& act : ui->activities) { - df::activity_info *act = ui->activities[i]; if (!act) continue; bool p1 = act->unit_actor == dwarfs[dwarf]; bool p2 = act->unit_noble == dwarfs[dwarf]; @@ -838,13 +826,11 @@ DFhackCExport command_result plugin_onupdate ( color_ostream &out ) } } - for (auto s = dwarfs[dwarf]->status.souls[0]->skills.begin(); s != dwarfs[dwarf]->status.souls[0]->skills.end(); s++) + for (auto& skill : dwarfs[dwarf]->status.souls[0]->skills) { - df::job_skill skill = (*s)->id; - - df::job_skill_class skill_class = ENUM_ATTR(job_skill, type, skill); + df::job_skill_class skill_class = ENUM_ATTR(job_skill, type, skill->id); - int skill_level = (*s)->rating; + int skill_level = skill->rating; // Track total & highest skill among normal/medical skills. (We don't care about personal or social skills.) @@ -935,10 +921,8 @@ DFhackCExport command_result plugin_onupdate ( color_ostream &out ) // Handle DISABLED skills (just bookkeeping). // Note that autolabor should *NEVER* enable or disable a skill that has been marked as DISABLED, for any reason. // The user has told us that they want manage this skill manually, and we must respect that. - for (auto lp = labors.begin(); lp != labors.end(); ++lp) + for (auto& labor: labors) { - auto labor = *lp; - if (labor_infos[labor].mode() != DISABLE) continue; @@ -956,10 +940,8 @@ DFhackCExport command_result plugin_onupdate ( color_ostream &out ) // Handle all skills except those marked HAULERS - for (auto lp = labors.begin(); lp != labors.end(); ++lp) + for (auto& labor : labors) { - auto labor = *lp; - assign_labor(labor, n_dwarfs, dwarf_info, trader_requested, dwarfs, has_butchers, has_fishery, out); } @@ -1043,19 +1025,19 @@ DFhackCExport command_result plugin_onupdate ( color_ostream &out ) } } - print_debug = 0; + print_debug = false; return CR_OK; } void print_labor (df::unit_labor labor, color_ostream &out) { - string labor_name = ENUM_KEY_STR(unit_labor, labor); + std::string labor_name = ENUM_KEY_STR(unit_labor, labor); out << labor_name << ": "; for (int i = 0; i < 20 - (int)labor_name.length(); i++) out << ' '; if (labor_infos[labor].mode() == DISABLE) - out << "disabled" << endl; + out << "disabled" << std::endl; else { if (labor_infos[labor].mode() == HAULERS) @@ -1063,7 +1045,7 @@ void print_labor (df::unit_labor labor, color_ostream &out) else out << "minimum " << labor_infos[labor].minimum_dwarfs() << ", maximum " << labor_infos[labor].maximum_dwarfs() << ", pool " << labor_infos[labor].talent_pool(); - out << ", currently " << labor_infos[labor].active_dwarfs << " dwarfs" << endl; + out << ", currently " << labor_infos[labor].active_dwarfs << " dwarfs" << std::endl; } } @@ -1083,7 +1065,7 @@ DFhackCExport command_result plugin_enable ( color_ostream &out, bool enable ) enable_autolabor = false; setOptionEnabled(CF_ENABLED, false); - out << "Autolabor is disabled." << endl; + out << "Autolabor is disabled." << std::endl; } return CR_OK; @@ -1110,7 +1092,7 @@ command_result autolabor (color_ostream &out, std::vector & parame { if (!enable_autolabor) { - out << "Error: The plugin is not enabled." << endl; + out << "Error: The plugin is not enabled." << std::endl; return CR_FAILURE; } @@ -1122,7 +1104,7 @@ command_result autolabor (color_ostream &out, std::vector & parame { if (!enable_autolabor) { - out << "Error: The plugin is not enabled." << endl; + out << "Error: The plugin is not enabled." << std::endl; return CR_FAILURE; } @@ -1186,7 +1168,7 @@ command_result autolabor (color_ostream &out, std::vector & parame { if (!enable_autolabor) { - out << "Error: The plugin is not enabled." << endl; + out << "Error: The plugin is not enabled." << std::endl; return CR_FAILURE; } @@ -1194,14 +1176,14 @@ command_result autolabor (color_ostream &out, std::vector & parame { reset_labor((df::unit_labor) i); } - out << "All labors reset." << endl; + out << "All labors reset." << std::endl; return CR_OK; } else if (parameters.size() == 1 && (parameters[0] == "list" || parameters[0] == "status")) { if (!enable_autolabor) { - out << "Error: The plugin is not enabled." << endl; + out << "Error: The plugin is not enabled." << std::endl; return CR_FAILURE; } @@ -1215,7 +1197,7 @@ command_result autolabor (color_ostream &out, std::vector & parame out << state_count[i] << ' ' << state_names[i]; need_comma = 1; } - out << endl; + out << std::endl; if (parameters[0] == "list") { @@ -1234,7 +1216,7 @@ command_result autolabor (color_ostream &out, std::vector & parame { if (!enable_autolabor) { - out << "Error: The plugin is not enabled." << endl; + out << "Error: The plugin is not enabled." << std::endl; return CR_FAILURE; } From 8d95d20852c7fa75c10a86f07be19c297a9f5922 Mon Sep 17 00:00:00 2001 From: Kelly Kinkade Date: Sun, 20 Nov 2022 12:05:14 -0600 Subject: [PATCH 2/6] remove stray whitespace --- plugins/autolabor/autohauler.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/plugins/autolabor/autohauler.cpp b/plugins/autolabor/autohauler.cpp index c9b746581..89f710365 100644 --- a/plugins/autolabor/autohauler.cpp +++ b/plugins/autolabor/autohauler.cpp @@ -371,12 +371,12 @@ DFhackCExport command_result plugin_onstatechange(color_ostream &out, state_chan DFhackCExport command_result plugin_onupdate ( color_ostream &out ) { static int step_count = 0; - if(!world || !world->map.block_index || !enable_autohauler) - { - return CR_OK; + if(!world || !world->map.block_index || !enable_autohauler) + { + return CR_OK; } - if (++step_count < frame_skip) + if (++step_count < frame_skip) return CR_OK; step_count = 0; From cea9e144c64c8ca0b84fbbacb78e2f869867f4cb Mon Sep 17 00:00:00 2001 From: Kelly Kinkade Date: Tue, 29 Nov 2022 02:58:43 -0600 Subject: [PATCH 3/6] switch autolabor & autohauler to debugging api might need to change some of the message levels, time will tell --- plugins/autolabor/autohauler.cpp | 32 +++++--------------- plugins/autolabor/autolabor.cpp | 50 ++++++++++++-------------------- 2 files changed, 27 insertions(+), 55 deletions(-) diff --git a/plugins/autolabor/autohauler.cpp b/plugins/autolabor/autohauler.cpp index 89f710365..8eaecf373 100644 --- a/plugins/autolabor/autohauler.cpp +++ b/plugins/autolabor/autohauler.cpp @@ -1,5 +1,6 @@ #include "Core.h" #include +#include #include #include @@ -79,7 +80,9 @@ REQUIRE_GLOBAL(world); DFHACK_PLUGIN_IS_ENABLED(enable_autohauler); -static bool print_debug = false; +namespace DFHack { + DBG_DECLARE(autohauler, cycle, DebugCategory::LINFO); +} static std::vector state_count(NUM_STATE); @@ -461,24 +464,17 @@ DFhackCExport command_result plugin_onupdate ( color_ostream &out ) dwarf_info[dwarf].state = dwarf_states[job]; else { - out.print("Dwarf %i \"%s\" has unknown job %i\n", dwarf, dwarfs[dwarf]->name.first_name.c_str(), job); - dwarf_info[dwarf].state = BUSY; + WARN(cycle, out).print("Dwarf %i \"%s\" has unknown job %i\n", dwarf, dwarfs[dwarf]->name.first_name.c_str(), job); + dwarf_info[dwarf].state = OTHER; } } - // Debug: Output dwarf job and state data - if(print_debug) - out.print("Dwarf %i %s State: %i\n", dwarf, dwarfs[dwarf]->name.first_name.c_str(), - dwarf_info[dwarf].state); - - // Increment corresponding labor in default_labor_infos struct state_count[dwarf_info[dwarf].state]++; + INFO(cycle, out).print("Dwarf %i \"%s\": state %s\n", + dwarf, dwarfs[dwarf]->name.first_name.c_str(), state_names[dwarf_info[dwarf].state]); } - // At this point the debug if present has been completed - print_debug = false; - // This is a vector of all the labors std::vector labors; @@ -720,18 +716,6 @@ command_result autohauler (color_ostream &out, std::vector & param return CR_OK; } - else if (parameters.size() == 1 && parameters[0] == "debug") - { - if (!enable_autohauler) - { - out << "Error: The plugin is not enabled." << std::endl; - return CR_FAILURE; - } - - print_debug = true; - - return CR_OK; - } else { out.print("Automatically assigns hauling labors to dwarves.\n" diff --git a/plugins/autolabor/autolabor.cpp b/plugins/autolabor/autolabor.cpp index aaa1f5053..4136e88a7 100644 --- a/plugins/autolabor/autolabor.cpp +++ b/plugins/autolabor/autolabor.cpp @@ -1,5 +1,6 @@ #include "Core.h" #include +#include #include #include @@ -78,7 +79,9 @@ REQUIRE_GLOBAL(world); DFHACK_PLUGIN_IS_ENABLED(enable_autolabor); -static bool print_debug = false; +namespace DFHack { + DBG_DECLARE(autolabor, cycle, DebugCategory::LINFO); +} static std::vector state_count(NUM_STATE); @@ -675,8 +678,10 @@ static void assign_labor(unit_labor::unit_labor labor, dwarfs[dwarf]->military.pickup_flags.bits.update = 1; } - if (print_debug) - out.print("Dwarf %i \"%s\" assigned %s: value %i %s %s\n", dwarf, dwarfs[dwarf]->name.first_name.c_str(), ENUM_KEY_STR(unit_labor, labor).c_str(), values[dwarf], dwarf_info[dwarf].trader ? "(trader)" : "", dwarf_info[dwarf].diplomacy ? "(diplomacy)" : ""); + TRACE(cycle, out).print("Dwarf % i \"%s\" assigned %s: value %i %s %s\n", + dwarf, dwarfs[dwarf]->name.first_name.c_str(), ENUM_KEY_STR(unit_labor, labor).c_str(), values[dwarf], + dwarf_info[dwarf].trader ? "(trader)" : "", + dwarf_info[dwarf].diplomacy ? "(diplomacy)" : ""); if (dwarf_info[dwarf].state == IDLE || dwarf_info[dwarf].state == BUSY || dwarf_info[dwarf].state == EXCLUSIVE) labor_infos[labor].active_dwarfs++; @@ -737,13 +742,10 @@ DFhackCExport command_result plugin_onupdate ( color_ostream &out ) { df::building_tradedepotst* depot = (df::building_tradedepotst*) build; trader_requested = trader_requested || depot->trade_flags.bits.trader_requested; - if (print_debug) - { - if (trader_requested) - out.print("Trade depot found and trader requested, trader will be excluded from all labors.\n"); - else - out.print("Trade depot found but trader is not requested.\n"); - } + INFO(cycle,out).print(trader_requested + ? "Trade depot found and trader requested, trader will be excluded from all labors.\n" + : "Trade depot found but trader is not requested.\n" + ); } } @@ -820,8 +822,8 @@ DFhackCExport command_result plugin_onupdate ( color_ostream &out ) if (p1 || p2) { dwarf_info[dwarf].diplomacy = true; - if (print_debug) - out.print("Dwarf %i \"%s\" has a meeting, will be cleared of all labors\n", dwarf, dwarfs[dwarf]->name.first_name.c_str()); + INFO(cycle, out).print("Dwarf %i \"%s\" has a meeting, will be cleared of all labors\n", + dwarf, dwarfs[dwarf]->name.first_name.c_str()); break; } } @@ -893,15 +895,15 @@ DFhackCExport command_result plugin_onupdate ( color_ostream &out ) dwarf_info[dwarf].state = dwarf_states[job]; else { - out.print("Dwarf %i \"%s\" has unknown job %i\n", dwarf, dwarfs[dwarf]->name.first_name.c_str(), job); + WARN(cycle, out).print("Dwarf %i \"%s\" has unknown job %i\n", dwarf, dwarfs[dwarf]->name.first_name.c_str(), job); dwarf_info[dwarf].state = OTHER; } } state_count[dwarf_info[dwarf].state]++; - if (print_debug) - out.print("Dwarf %i \"%s\": penalty %i, state %s\n", dwarf, dwarfs[dwarf]->name.first_name.c_str(), dwarf_info[dwarf].mastery_penalty, state_names[dwarf_info[dwarf].state]); + INFO(cycle, out).print("Dwarf %i \"%s\": penalty %i, state %s\n", + dwarf, dwarfs[dwarf]->name.first_name.c_str(), dwarf_info[dwarf].mastery_penalty, state_names[dwarf_info[dwarf].state]); } std::vector labors; @@ -1008,8 +1010,8 @@ DFhackCExport command_result plugin_onupdate ( color_ostream &out ) if (dwarf_info[dwarf].state == IDLE || dwarf_info[dwarf].state == BUSY || dwarf_info[dwarf].state == EXCLUSIVE) labor_infos[labor].active_dwarfs++; - if (print_debug) - out.print("Dwarf %i \"%s\" assigned %s: hauler\n", dwarf, dwarfs[dwarf]->name.first_name.c_str(), ENUM_KEY_STR(unit_labor, labor).c_str()); + TRACE(cycle, out).print("Dwarf %i \"%s\" assigned %s: hauler\n", + dwarf, dwarfs[dwarf]->name.first_name.c_str(), ENUM_KEY_STR(unit_labor, labor).c_str()); } for (size_t i = num_haulers; i < hauler_ids.size(); i++) @@ -1025,8 +1027,6 @@ DFhackCExport command_result plugin_onupdate ( color_ostream &out ) } } - print_debug = false; - return CR_OK; } @@ -1212,18 +1212,6 @@ command_result autolabor (color_ostream &out, std::vector & parame return CR_OK; } - else if (parameters.size() == 1 && parameters[0] == "debug") - { - if (!enable_autolabor) - { - out << "Error: The plugin is not enabled." << std::endl; - return CR_FAILURE; - } - - print_debug = 1; - - return CR_OK; - } else { out.print("Automatically assigns labors to dwarves.\n" From 9b5a6936378a7b3336ff00a620706271d06ba6a9 Mon Sep 17 00:00:00 2001 From: Kelly Kinkade Date: Tue, 29 Nov 2022 03:03:42 -0600 Subject: [PATCH 4/6] death to whitespace --- plugins/autolabor/autohauler.cpp | 2 +- plugins/autolabor/autolabor.cpp | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/plugins/autolabor/autohauler.cpp b/plugins/autolabor/autohauler.cpp index 8eaecf373..5f8c5f3ff 100644 --- a/plugins/autolabor/autohauler.cpp +++ b/plugins/autolabor/autohauler.cpp @@ -471,7 +471,7 @@ DFhackCExport command_result plugin_onupdate ( color_ostream &out ) state_count[dwarf_info[dwarf].state]++; - INFO(cycle, out).print("Dwarf %i \"%s\": state %s\n", + INFO(cycle, out).print("Dwarf %i \"%s\": state %s\n", dwarf, dwarfs[dwarf]->name.first_name.c_str(), state_names[dwarf_info[dwarf].state]); } diff --git a/plugins/autolabor/autolabor.cpp b/plugins/autolabor/autolabor.cpp index 4136e88a7..b4de45de9 100644 --- a/plugins/autolabor/autolabor.cpp +++ b/plugins/autolabor/autolabor.cpp @@ -678,9 +678,9 @@ static void assign_labor(unit_labor::unit_labor labor, dwarfs[dwarf]->military.pickup_flags.bits.update = 1; } - TRACE(cycle, out).print("Dwarf % i \"%s\" assigned %s: value %i %s %s\n", - dwarf, dwarfs[dwarf]->name.first_name.c_str(), ENUM_KEY_STR(unit_labor, labor).c_str(), values[dwarf], - dwarf_info[dwarf].trader ? "(trader)" : "", + TRACE(cycle, out).print("Dwarf % i \"%s\" assigned %s: value %i %s %s\n", + dwarf, dwarfs[dwarf]->name.first_name.c_str(), ENUM_KEY_STR(unit_labor, labor).c_str(), values[dwarf], + dwarf_info[dwarf].trader ? "(trader)" : "", dwarf_info[dwarf].diplomacy ? "(diplomacy)" : ""); if (dwarf_info[dwarf].state == IDLE || dwarf_info[dwarf].state == BUSY || dwarf_info[dwarf].state == EXCLUSIVE) @@ -742,7 +742,7 @@ DFhackCExport command_result plugin_onupdate ( color_ostream &out ) { df::building_tradedepotst* depot = (df::building_tradedepotst*) build; trader_requested = trader_requested || depot->trade_flags.bits.trader_requested; - INFO(cycle,out).print(trader_requested + INFO(cycle,out).print(trader_requested ? "Trade depot found and trader requested, trader will be excluded from all labors.\n" : "Trade depot found but trader is not requested.\n" ); @@ -822,7 +822,7 @@ DFhackCExport command_result plugin_onupdate ( color_ostream &out ) if (p1 || p2) { dwarf_info[dwarf].diplomacy = true; - INFO(cycle, out).print("Dwarf %i \"%s\" has a meeting, will be cleared of all labors\n", + INFO(cycle, out).print("Dwarf %i \"%s\" has a meeting, will be cleared of all labors\n", dwarf, dwarfs[dwarf]->name.first_name.c_str()); break; } @@ -902,7 +902,7 @@ DFhackCExport command_result plugin_onupdate ( color_ostream &out ) state_count[dwarf_info[dwarf].state]++; - INFO(cycle, out).print("Dwarf %i \"%s\": penalty %i, state %s\n", + INFO(cycle, out).print("Dwarf %i \"%s\": penalty %i, state %s\n", dwarf, dwarfs[dwarf]->name.first_name.c_str(), dwarf_info[dwarf].mastery_penalty, state_names[dwarf_info[dwarf].state]); } @@ -1010,7 +1010,7 @@ DFhackCExport command_result plugin_onupdate ( color_ostream &out ) if (dwarf_info[dwarf].state == IDLE || dwarf_info[dwarf].state == BUSY || dwarf_info[dwarf].state == EXCLUSIVE) labor_infos[labor].active_dwarfs++; - TRACE(cycle, out).print("Dwarf %i \"%s\" assigned %s: hauler\n", + TRACE(cycle, out).print("Dwarf %i \"%s\" assigned %s: hauler\n", dwarf, dwarfs[dwarf]->name.first_name.c_str(), ENUM_KEY_STR(unit_labor, labor).c_str()); } From 5a2ee6ee23b6b889b8bafebe2639a987eeb46645 Mon Sep 17 00:00:00 2001 From: Kelly Kinkade Date: Wed, 30 Nov 2022 21:54:58 -0600 Subject: [PATCH 5/6] update documentation for autolabor & autohauler --- docs/changelog.txt | 1 + docs/plugins/autohauler.rst | 5 +++-- docs/plugins/autolabor.rst | 3 +++ 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/docs/changelog.txt b/docs/changelog.txt index 3748134c1..a55473335 100644 --- a/docs/changelog.txt +++ b/docs/changelog.txt @@ -44,6 +44,7 @@ changelog.txt uses a syntax similar to RST, with a few special sequences: - `tiletypes`: no longer resets dig priority to the default when updating other properties of a tile - `automaterial`: fix rendering errors with box boundary markers - `autolabor` & `autohauler`: properly handle jobs 241, 242, and 243 +- `autolabor` & `autohauler`: refactored to use DFHack's messaging system for info/debug/trace messages - `autofarm`: add missing output flushes - `hotkeys`: correctly detect hotkeys bound to number keys, F11, or F12 - `labormanager`: fix quern construction labor diff --git a/docs/plugins/autohauler.rst b/docs/plugins/autohauler.rst index 08c84327a..09c3c2f8f 100644 --- a/docs/plugins/autohauler.rst +++ b/docs/plugins/autohauler.rst @@ -22,6 +22,9 @@ Autohauler allows a skill to be used as a flag to exempt a dwarf from autohauler's effects. By default, this is the unused ALCHEMIST labor, but it can be changed by the user. +Autohauler uses DFHack's `debug` functionality to display information about the changes it makes. The amount of +information displayed can be controlled through appropriate use of the ``debugfilter`` command. + Usage ----- @@ -42,8 +45,6 @@ Usage Show the active configuration for all labors. ``autohauler frameskip `` Set the number of frames between runs of autohauler. -``autohauler debug`` - In the next cycle, output the state of every dwarf. Examples -------- diff --git a/docs/plugins/autolabor.rst b/docs/plugins/autolabor.rst index 63b830dfe..4b92e5fd8 100644 --- a/docs/plugins/autolabor.rst +++ b/docs/plugins/autolabor.rst @@ -56,6 +56,9 @@ labor, then to additional dwarfs that meet any of these conditions: We stop assigning dwarves when we reach the maximum allowed. +Autolabor uses DFHack's `debug` functionality to display information about the changes it makes. The amount of +information displayed can be controlled through appropriate use of the ``debugfilter`` command. + Examples -------- From 437335454d853b18bfc688149a3f07e9c42d1a44 Mon Sep 17 00:00:00 2001 From: Kelly Kinkade Date: Thu, 1 Dec 2022 09:00:50 -0600 Subject: [PATCH 6/6] make autohauler not be a chatty cathy INFO -> TRACE --- plugins/autolabor/autohauler.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/autolabor/autohauler.cpp b/plugins/autolabor/autohauler.cpp index 5f8c5f3ff..e7c2530e1 100644 --- a/plugins/autolabor/autohauler.cpp +++ b/plugins/autolabor/autohauler.cpp @@ -471,7 +471,7 @@ DFhackCExport command_result plugin_onupdate ( color_ostream &out ) state_count[dwarf_info[dwarf].state]++; - INFO(cycle, out).print("Dwarf %i \"%s\": state %s\n", + TRACE(cycle, out).print("Dwarf %i \"%s\": state %s\n", dwarf, dwarfs[dwarf]->name.first_name.c_str(), state_names[dwarf_info[dwarf].state]); }