Implements plugin: spectate v0.5

Fixes spectate not starting with the first job

Updates spectate.cpp

- refactors features/settings to under a `Configuration` struct with a global variable `config`
- refactors existing `config` => `pconfig`
- moves plugin logic, mostly, to namespace SP (spectate plugin)
- utilizes debugging log macros
- updates status format
- refactors status print code into a separate function

docs/plugins/spectate.rst

@@ -45,4 +45,4 @@ Features
 Settings
 --------
 :tick-threshold: Set the plugin's tick interval for changing the followed dwarf.
-                 Acts as a maximum follow time when used with focus-jobs enabled. (default: 50)
+                 Acts as a maximum follow time when used with focus-jobs enabled. (default: 1000)

plugins/spectate/spectate.cpp

@@ -4,11 +4,13 @@
 
 #include "pause.h"
 
-#include "Core.h"
+#include <Debug.h>
-#include <modules/Gui.h>
+#include <Core.h>
 #include <Console.h>
 #include <Export.h>
 #include <PluginManager.h>
+
+#include <modules/Gui.h>
 #include <modules/World.h>
 #include <modules/EventManager.h>
 #include <modules/Job.h>
@@ -25,6 +27,11 @@
 #include <random>
 #include <cinttypes>
 
+// Debugging
+namespace DFHack {
+    DBG_DECLARE(spectate, plugin, DebugCategory::LINFO);
+}
+
 DFHACK_PLUGIN("spectate");
 DFHACK_PLUGIN_IS_ENABLED(enabled);
 REQUIRE_GLOBAL(world);
@@ -36,14 +43,14 @@ using namespace DFHack;
 using namespace Pausing;
 using namespace df::enums;
 
-void onTick(color_ostream& out, void* tick);
+struct Configuration {
-void onJobStart(color_ostream &out, void* job);
+    bool debug = false;
-void onJobCompletion(color_ostream &out, void* job);
+    bool jobs_focus = false;
+    bool unpause = false;
+    bool disengage = false;
+    int32_t tick_threshold = 1000;
+} config;
 
-uint64_t tick_threshold = 1000;
-bool focus_jobs_enabled = false;
-bool disengage_enabled = false;
-bool unpause_enabled = false;
 Pausing::AnnouncementLock* pause_lock = nullptr;
 bool lock_collision = false;
 bool announcements_disabled = false;
@@ -57,9 +64,6 @@ std::set<int32_t> job_tracker;
 std::map<uint16_t,int16_t> freq;
 std::default_random_engine RNG;
 
-void enable_auto_unpause(color_ostream &out, bool state);
-
-
 #define base 0.99
 
 static const std::string CONFIG_KEY = std::string(plugin_name) + "/config";
@@ -70,18 +74,225 @@ enum ConfigData {
     TICK_THRESHOLD
 };
 
-static PersistentDataItem config;
+static PersistentDataItem pconfig;
-inline void saveConfig() {
-    if (config.isValid()) {
-        config.ival(UNPAUSE) = unpause_enabled;
-        config.ival(DISENGAGE) = disengage_enabled;
-        config.ival(JOB_FOCUS) = focus_jobs_enabled;
-        config.ival(TICK_THRESHOLD) = tick_threshold;
-    }
-}
 
+DFhackCExport command_result plugin_enable(color_ostream &out, bool enable);
 command_result spectate (color_ostream &out, std::vector <std::string> & parameters);
 
+namespace SP {
+
+    void PrintStatus(color_ostream &out) {
+        out.print("Spectate is %s\n", enabled ? "ENABLED." : "DISABLED.");
+        out.print(" FEATURES:\n");
+        out.print("  %-20s\t%s\n", "focus-jobs: ", config.jobs_focus ? "on." : "off.");
+        out.print("  %-20s\t%s\n", "auto-unpause: ", config.unpause ? "on." : "off.");
+        out.print("  %-20s\t%s\n", "auto-disengage: ", config.disengage ? "on." : "off.");
+        out.print(" SETTINGS:\n");
+        out.print("  %-20s\t%" PRIi32 "\n", "tick-threshold: ", config.tick_threshold);
+    }
+
+    void SetUnpauseState(bool state) {
+        // we don't need to do any of this yet if the plugin isn't enabled
+        if (enabled) {
+            // todo: R.E. UNDEAD_ATTACK event [still pausing regardless of announcement settings]
+            // lock_collision == true means: enable_auto_unpause() was already invoked and didn't complete
+            // The onupdate function above ensure the procedure properly completes, thus we only care about
+            // state reversal here ergo `enabled != state`
+            if (lock_collision && config.unpause != state) {
+                WARN(plugin).print("Spectate auto-unpause: Not enabled yet, there was a lock collision. When the other lock holder releases, auto-unpause will engage on its own.\n");
+                // if unpaused_enabled is true, then a lock collision means: we couldn't save/disable the pause settings,
+                // therefore nothing to revert and the lock won't even be engaged (nothing to unlock)
+                lock_collision = false;
+                config.unpause = state;
+                if (config.unpause) {
+                    // a collision means we couldn't restore the pause settings, therefore we only need re-engage the lock
+                    pause_lock->lock();
+                }
+                return;
+            }
+            // update the announcement settings if we can
+            if (state) {
+                if (World::SaveAnnouncementSettings()) {
+                    World::DisableAnnouncementPausing();
+                    announcements_disabled = true;
+                    pause_lock->lock();
+                } else {
+                    WARN(plugin).print("Spectate auto-unpause: Could not fully enable. There was a lock collision, when the other lock holder releases, auto-unpause will engage on its own.\n");
+                    lock_collision = true;
+                }
+            } else {
+                pause_lock->unlock();
+                if (announcements_disabled) {
+                    if (!World::RestoreAnnouncementSettings()) {
+                        // this in theory shouldn't happen, if others use the lock like we do in spectate
+                        WARN(plugin).print("Spectate auto-unpause: Could not fully disable. There was a lock collision, when the other lock holder releases, auto-unpause will disengage on its own.\n");
+                        lock_collision = true;
+                    } else {
+                        announcements_disabled = false;
+                    }
+                }
+            }
+            if (lock_collision) {
+                ERR(plugin).print("Spectate auto-unpause: Could not fully enable. There was a lock collision, when the other lock holder releases, auto-unpause will engage on its own.\n");
+                WARN(plugin).print(
+                        " auto-unpause: must wait for another Pausing::AnnouncementLock to be lifted.\n"
+                        " The action you were attempting will complete when the following lock or locks lift.\n");
+                pause_lock->reportLocks(Core::getInstance().getConsole());
+            }
+        }
+        config.unpause = state;
+    }
+
+    void SaveSettings() {
+        if (pconfig.isValid()) {
+            pconfig.ival(UNPAUSE) = config.unpause;
+            pconfig.ival(DISENGAGE) = config.disengage;
+            pconfig.ival(JOB_FOCUS) = config.jobs_focus;
+            pconfig.ival(TICK_THRESHOLD) = config.tick_threshold;
+        }
+    }
+
+    void LoadSettings() {
+        pconfig = World::GetPersistentData(CONFIG_KEY);
+
+        if (!pconfig.isValid()) {
+            pconfig = World::AddPersistentData(CONFIG_KEY);
+            SaveSettings();
+        } else {
+            config.unpause = pconfig.ival(UNPAUSE);
+            config.disengage = pconfig.ival(DISENGAGE);
+            config.jobs_focus = pconfig.ival(JOB_FOCUS);
+            config.tick_threshold = pconfig.ival(TICK_THRESHOLD);
+            pause_lock->unlock();
+            SetUnpauseState(config.unpause);
+        }
+    }
+
+    void Enable(color_ostream &out, bool enable) {
+
+    }
+
+    void onUpdate(color_ostream &out) {
+        // keeps announcement pause settings locked
+        World::Update(); // from pause.h
+        if (lock_collision) {
+            if (config.unpause) {
+                // player asked for auto-unpause enabled
+                World::SaveAnnouncementSettings();
+                if (World::DisableAnnouncementPausing()) {
+                    // now that we've got what we want, we can lock it down
+                    lock_collision = false;
+                }
+            } else {
+                if (World::RestoreAnnouncementSettings()) {
+                    lock_collision = false;
+                }
+            }
+        }
+        int failsafe = 0;
+        while (config.unpause && !world->status.popups.empty() && ++failsafe <= 10) {
+            // dismiss announcement popup(s)
+            Gui::getCurViewscreen(true)->feed_key(interface_key::CLOSE_MEGA_ANNOUNCEMENT);
+            if (World::ReadPauseState()) {
+                // WARNING: This has a possibility of conflicting with `reveal hell` - if Hermes himself runs `reveal hell` on precisely the right moment that is
+                World::SetPauseState(false);
+            }
+        }
+        if (failsafe >= 10) {
+            out.printerr("spectate encountered a problem dismissing a popup!\n");
+        }
+        if (config.disengage && !World::ReadPauseState()) {
+            if (our_dorf && our_dorf->id != df::global::ui->follow_unit) {
+                plugin_enable(out, false);
+            }
+        }
+    }
+
+    // every tick check whether to decide to follow a dwarf
+    void TickHandler(color_ostream& out, void* ptr) {
+        int32_t tick = df::global::world->frame_counter;
+        if (our_dorf) {
+            if (!Units::isAlive(our_dorf)) {
+                following_dwarf = false;
+                df::global::ui->follow_unit = -1;
+            }
+        }
+        if (!following_dwarf || (config.jobs_focus && !job_watched) || timestamp == -1 || (tick - timestamp) > config.tick_threshold) {
+            std::vector<df::unit*> dwarves;
+            for (auto unit: df::global::world->units.active) {
+                if (!Units::isCitizen(unit)) {
+                    continue;
+                }
+                dwarves.push_back(unit);
+            }
+            std::uniform_int_distribution<uint64_t> follow_any(0, dwarves.size() - 1);
+            // if you're looking at a warning about a local address escaping, it means the unit* from dwarves (which aren't local)
+            our_dorf = dwarves[follow_any(RNG)];
+            df::global::ui->follow_unit = our_dorf->id;
+            job_watched = our_dorf->job.current_job;
+            following_dwarf = true;
+            if (config.jobs_focus && !job_watched) {
+                timestamp = tick;
+            }
+        }
+        // todo: refactor event manager to respect tick listeners
+        namespace EM = EventManager;
+        EM::EventHandler ticking(TickHandler, config.tick_threshold);
+        EM::registerTick(ticking, config.tick_threshold, plugin_self);
+    }
+
+    // every new worked job needs to be considered
+    void JobStartEvent(color_ostream& out, void* job_ptr) {
+        // todo: detect mood jobs
+        int32_t tick = df::global::world->frame_counter;
+        auto job = (df::job*) job_ptr;
+        // don't forget about it
+        int zcount = ++freq[job->pos.z];
+        job_tracker.emplace(job->id);
+        // if we're not doing anything~ then let's pick something
+        if ((config.jobs_focus && !job_watched) || timestamp == -1 || (tick - timestamp) > config.tick_threshold) {
+            timestamp = tick;
+            following_dwarf = true;
+            // todo: allow the user to configure b, and also revise the math
+            const double b = base;
+            double p = b * ((double) zcount / job_tracker.size());
+            std::bernoulli_distribution follow_job(p);
+            if (!job->flags.bits.special && follow_job(RNG)) {
+                job_watched = job;
+                if (df::unit* unit = Job::getWorker(job)) {
+                    our_dorf = unit;
+                    df::global::ui->follow_unit = unit->id;
+                }
+            } else {
+                timestamp = tick;
+                std::vector<df::unit*> nonworkers;
+                for (auto unit: df::global::world->units.active) {
+                    if (!Units::isCitizen(unit) || unit->job.current_job) {
+                        continue;
+                    }
+                    nonworkers.push_back(unit);
+                }
+                std::uniform_int_distribution<> follow_drunk(0, nonworkers.size() - 1);
+                df::global::ui->follow_unit = nonworkers[follow_drunk(RNG)]->id;
+            }
+        }
+    }
+
+    // every job completed can be forgotten about
+    void JobCompletedEvent(color_ostream &out, void* job_ptr) {
+        auto job = (df::job*) job_ptr;
+        // forget about it
+        freq[job->pos.z]--;
+        freq[job->pos.z] = freq[job->pos.z] < 0 ? 0 : freq[job->pos.z];
+        // the job doesn't exist, so we definitely need to get rid of that
+        job_tracker.erase(job->id);
+        // the event manager clones jobs and returns those clones for completed jobs. So the pointers won't match without a refactor of EM passing clones to both events
+        if (job_watched && job_watched->id == job->id) {
+            job_watched = nullptr;
+        }
+    }
+};
+
 DFhackCExport command_result plugin_init (color_ostream &out, std::vector <PluginCommand> &commands) {
     commands.push_back(PluginCommand("spectate",
                                      "Automated spectator mode.",
@@ -97,19 +308,8 @@ DFhackCExport command_result plugin_shutdown (color_ostream &out) {
 }
 
 DFhackCExport command_result plugin_load_data (color_ostream &out) {
-    config = World::GetPersistentData(CONFIG_KEY);
+    SP::LoadSettings();
-
+    SP::PrintStatus(out);
-    if (!config.isValid()) {
-        config = World::AddPersistentData(CONFIG_KEY);
-        saveConfig();
-    } else {
-        unpause_enabled = config.ival(UNPAUSE);
-        disengage_enabled = config.ival(DISENGAGE);
-        focus_jobs_enabled = config.ival(JOB_FOCUS);
-        tick_threshold = config.ival(TICK_THRESHOLD);
-        pause_lock->unlock();
-        enable_auto_unpause(out, unpause_enabled);
-    }
     return DFHack::CR_OK;
 }
 
@@ -118,22 +318,23 @@ DFhackCExport command_result plugin_enable(color_ostream &out, bool enable) {
     if (enable && !enabled) {
         out.print("Spectate mode enabled!\n");
         using namespace EM::EventType;
-        EM::EventHandler ticking(onTick, 15);
+        EM::EventHandler ticking(SP::TickHandler, config.tick_threshold);
-        EM::EventHandler start(onJobStart, 0);
+        EM::EventHandler start(SP::JobStartEvent, 0);
-        EM::EventHandler complete(onJobCompletion, 0);
+        EM::EventHandler complete(SP::JobCompletedEvent, 0);
-        EM::registerListener(EventType::TICK, ticking, plugin_self);
+        //EM::registerListener(EventType::TICK, ticking, plugin_self);
+        EM::registerTick(ticking, config.tick_threshold, plugin_self);
         EM::registerListener(EventType::JOB_STARTED, start, plugin_self);
         EM::registerListener(EventType::JOB_COMPLETED, complete, plugin_self);
         enabled = true; // enable_auto_unpause won't do anything without this set now
-        enable_auto_unpause(out, unpause_enabled);
+        SP::SetUnpauseState(config.unpause);
     } else if (!enable && enabled) {
         // warp 8, engage!
         out.print("Spectate mode disabled!\n");
         EM::unregisterAll(plugin_self);
         // we need to retain whether auto-unpause is enabled, but we also need to disable its effect
-        bool temp = unpause_enabled;
+        bool temp = config.unpause;
-        enable_auto_unpause(out, false);
+        SP::SetUnpauseState(false);
-        unpause_enabled = temp;
+        config.unpause = temp;
         job_tracker.clear();
         freq.clear();
     }
@@ -158,93 +359,10 @@ DFhackCExport command_result plugin_onstatechange(color_ostream &out, state_chan
 }
 
 DFhackCExport command_result plugin_onupdate(color_ostream &out) {
-    // keeps announcement pause settings locked
+    SP::onUpdate(out);
-    World::Update(); // from pause.h
-    if (lock_collision) {
-        if (unpause_enabled) {
-            // player asked for auto-unpause enabled
-            World::SaveAnnouncementSettings();
-            if (World::DisableAnnouncementPausing()) {
-                // now that we've got what we want, we can lock it down
-                lock_collision = false;
-            }
-        } else {
-            if (World::RestoreAnnouncementSettings()) {
-                lock_collision = false;
-            }
-        }
-    }
-    int failsafe = 0;
-    while (unpause_enabled && !world->status.popups.empty() && ++failsafe <= 10) {
-        // dismiss announcement popup(s)
-        Gui::getCurViewscreen(true)->feed_key(interface_key::CLOSE_MEGA_ANNOUNCEMENT);
-        if (World::ReadPauseState()) {
-            // WARNING: This has a possibility of conflicting with `reveal hell` - if Hermes himself runs `reveal hell` on precisely the right moment that is
-            World::SetPauseState(false);
-        }
-    }
-    if (failsafe >= 10) {
-        out.printerr("spectate encountered a problem dismissing a popup!\n");
-    }
-    if (disengage_enabled && !World::ReadPauseState()) {
-        if (our_dorf && our_dorf->id != df::global::ui->follow_unit) {
-            plugin_enable(out, false);
-        }
-    }
     return DFHack::CR_OK;
 }
 
-void enable_auto_unpause(color_ostream &out, bool state) {
-    // we don't need to do any of this yet if the plugin isn't enabled
-    if (enabled) {
-        // todo: R.E. UNDEAD_ATTACK event [still pausing regardless of announcement settings]
-        // lock_collision == true means: enable_auto_unpause() was already invoked and didn't complete
-        // The onupdate function above ensure the procedure properly completes, thus we only care about
-        // state reversal here ergo `enabled != state`
-        if (lock_collision && unpause_enabled != state) {
-            out.print("handling collision\n");
-            // if unpaused_enabled is true, then a lock collision means: we couldn't save/disable the pause settings,
-            // therefore nothing to revert and the lock won't even be engaged (nothing to unlock)
-            lock_collision = false;
-            unpause_enabled = state;
-            if (unpause_enabled) {
-                // a collision means we couldn't restore the pause settings, therefore we only need re-engage the lock
-                pause_lock->lock();
-            }
-            return;
-        }
-        // update the announcement settings if we can
-        if (state) {
-            if (World::SaveAnnouncementSettings()) {
-                World::DisableAnnouncementPausing();
-                announcements_disabled = true;
-                pause_lock->lock();
-            } else {
-                out.printerr("lock collision enabling auto-unpause\n");
-                lock_collision = true;
-            }
-        } else {
-            pause_lock->unlock();
-            if (announcements_disabled) {
-                if (!World::RestoreAnnouncementSettings()) {
-                    // this in theory shouldn't happen, if others use the lock like we do in spectate
-                    out.printerr("lock collision disabling auto-unpause\n");
-                    lock_collision = true;
-                } else {
-                    announcements_disabled = false;
-                }
-            }
-        }
-        if (lock_collision) {
-            out.printerr(
-                    "auto-unpause: must wait for another Pausing::AnnouncementLock to be lifted.\n"
-                    "The action you were attempting will complete when the following lock or locks lift.\n");
-            pause_lock->reportLocks(out);
-        }
-    }
-    unpause_enabled = state;
-}
-
 command_result spectate (color_ostream &out, std::vector <std::string> & parameters) {
     if (!parameters.empty()) {
         if (parameters.size() >= 2 && parameters.size() <= 3) {
@@ -260,14 +378,14 @@ command_result spectate (color_ostream &out, std::vector <std::string> & paramet
                 return DFHack::CR_WRONG_USAGE;
             }
             if(parameters[1] == "auto-unpause"){
-                enable_auto_unpause(out, state);
+                SP::SetUnpauseState(state);
             } else if (parameters[1] == "auto-disengage") {
-                disengage_enabled = state;
+                config.disengage = state;
             } else if (parameters[1] == "focus-jobs") {
-                focus_jobs_enabled = state;
+                config.jobs_focus = state;
             } else if (parameters[1] == "tick-threshold" && set && parameters.size() == 3) {
                 try {
-                    tick_threshold = std::abs(std::stol(parameters[2]));
+                    config.tick_threshold = std::abs(std::stol(parameters[2]));
                 } catch (const std::exception &e) {
                     out.printerr("%s\n", e.what());
                 }
@@ -276,91 +394,8 @@ command_result spectate (color_ostream &out, std::vector <std::string> & paramet
             }
         }
     } else {
-        out.print("Spectate is %s\n", enabled ? "ENABLED." : "DISABLED.");
+        SP::PrintStatus(out);
-        out.print("tick-threshold: %" PRIu64 "\n", tick_threshold);
-        out.print("focus-jobs: %s\n", focus_jobs_enabled ? "on." : "off.");
-        out.print("auto-unpause: %s\n", unpause_enabled ? "on." : "off.");
-        out.print("auto-disengage: %s\n", disengage_enabled ? "on." : "off.");
     }
-    saveConfig();
+    SP::SaveSettings();
     return DFHack::CR_OK;
 }
-
-// every tick check whether to decide to follow a dwarf
-void onTick(color_ostream& out, void* ptr) {
-    int32_t tick = df::global::world->frame_counter;
-    if (our_dorf) {
-        if (!Units::isAlive(our_dorf)) {
-            following_dwarf = false;
-            df::global::ui->follow_unit = -1;
-        }
-    }
-    if (!following_dwarf || (focus_jobs_enabled && !job_watched) || (tick - timestamp) > (int32_t) tick_threshold) {
-        std::vector<df::unit*> dwarves;
-        for (auto unit: df::global::world->units.active) {
-            if (!Units::isCitizen(unit)) {
-                continue;
-            }
-            dwarves.push_back(unit);
-        }
-        std::uniform_int_distribution<uint64_t> follow_any(0, dwarves.size() - 1);
-        // if you're looking at a warning about a local address escaping, it means the unit* from dwarves (which aren't local)
-        our_dorf = dwarves[follow_any(RNG)];
-        df::global::ui->follow_unit = our_dorf->id;
-        job_watched = our_dorf->job.current_job;
-        following_dwarf = true;
-        if (!job_watched) {
-            timestamp = tick;
-        }
-    }
-}
-
-// every new worked job needs to be considered
-void onJobStart(color_ostream& out, void* job_ptr) {
-    // todo: detect mood jobs
-    int32_t tick = df::global::world->frame_counter;
-    auto job = (df::job*) job_ptr;
-    // don't forget about it
-    int zcount = ++freq[job->pos.z];
-    job_tracker.emplace(job->id);
-    // if we're not doing anything~ then let's pick something
-    if ((focus_jobs_enabled && !job_watched) || (tick - timestamp) > (int32_t) tick_threshold) {
-        following_dwarf = true;
-        // todo: allow the user to configure b, and also revise the math
-        const double b = base;
-        double p = b * ((double) zcount / job_tracker.size());
-        std::bernoulli_distribution follow_job(p);
-        if (!job->flags.bits.special && follow_job(RNG)) {
-            job_watched = job;
-            if (df::unit* unit = Job::getWorker(job)) {
-                our_dorf = unit;
-                df::global::ui->follow_unit = unit->id;
-            }
-        } else {
-            timestamp = tick;
-            std::vector<df::unit*> nonworkers;
-            for (auto unit: df::global::world->units.active) {
-                if (!Units::isCitizen(unit) || unit->job.current_job) {
-                    continue;
-                }
-                nonworkers.push_back(unit);
-            }
-            std::uniform_int_distribution<> follow_drunk(0, nonworkers.size() - 1);
-            df::global::ui->follow_unit = nonworkers[follow_drunk(RNG)]->id;
-        }
-    }
-}
-
-// every job completed can be forgotten about
-void onJobCompletion(color_ostream &out, void* job_ptr) {
-    auto job = (df::job*) job_ptr;
-    // forget about it
-    freq[job->pos.z]--;
-    freq[job->pos.z] = freq[job->pos.z] < 0 ? 0 : freq[job->pos.z];
-    // the job doesn't exist, so we definitely need to get rid of that
-    job_tracker.erase(job->id);
-    // the event manager clones jobs and returns those clones for completed jobs. So the pointers won't match without a refactor of EM passing clones to both events
-    if (job_watched && job_watched->id == job->id) {
-        job_watched = nullptr;
-    }
-}