Don't include raiding units in Units::isActive; make getPosition return invalid coords

docs/changelog.txt

@@ -57,6 +57,9 @@ Template for new versions:
 - `stockpiles`: add simple import/export dialogs to stockpile overlay panel
 
 ## Fixes
+- ``Units::getPosition``, ``Items::getPosition``: return invalid coord for inactive units, items held by units off map
+- ``Units::isUnitInBox``, ``Units::getUnitsInBox``: don't include inactive units
+- ``Units::isActive``: return false for units off map
 - `preserve-rooms`: don't erroneously release reservations for units that have returned from their missions but have not yet entered the fort map
 - `preserve-rooms`: handle case where unit records are culled by DF immediately after a unit leaves the map
 - `preserve-tombs`: properly re-enable after loading a game that had the tool enabled

docs/dev/Lua API.rst

@@ -1647,7 +1647,7 @@ Units module
 
   Returns the true *x,y,z* of the unit, or *nil* if invalid. You should
   generally use this method instead of reading *unit.pos* directly since
-  that field can be inaccurate when the unit is caged.
+  that field can be inaccurate when the unit is caged, off map, or dead.
 
 * ``dfhack.units.teleport(unit, pos)``
 

library/RemoteTools.cpp

@@ -584,7 +584,7 @@ static command_result ListUnits(color_ostream &stream,
 
     if (in->scan_all())
     {
-        auto &vec = df::unit::get_vector();
+        auto &vec = df::global::world->units.active;
 
         for (size_t i = 0; i < vec.size(); i++)
         {

library/include/modules/Items.h

@@ -137,7 +137,7 @@ DFHACK_EXPORT df::building *getHolderBuilding(df::item *item);
 // Get unit that holds the item or NULL.
 DFHACK_EXPORT df::unit *getHolderUnit(df::item *item);
 
-// Returns the true position of the item (non-trivial if in inventory).
+// Get the true position of the item (non-trivial if in inventory). Returns invalid coord if holder off map.
 DFHACK_EXPORT df::coord getPosition(df::item *item);
 
 /// Returns the title of a codex or "tool", either as the codex title or as the title of the

library/include/modules/Units.h

@@ -207,7 +207,7 @@ inline auto citizensRange(std::vector<df::unit *> &vec, bool exclude_residents =
 DFHACK_EXPORT void forCitizens(std::function<void(df::unit *)> fn, bool exclude_residents = false, bool include_insane = false);
 DFHACK_EXPORT bool getCitizens(std::vector<df::unit *> &citizens, bool exclude_residents = false, bool include_insane = false);
 
-// Returns the true position of the unit (non-trivial in case of caged).
+// Get the true position of the unit (non-trivial in case of caged or inactive). Returns invalid coord if dead or off map.
 DFHACK_EXPORT df::coord getPosition(df::unit *unit);
 
 // Moves unit and any riders to the target coordinates. Sets tile occupancy flags.

library/modules/Units.cpp

@@ -123,7 +123,11 @@ constexpr uint32_t exclude_flags2 = (
 
 bool Units::isActive(df::unit *unit) {
     CHECK_NULL_POINTER(unit);
-    return !unit->flags1.bits.inactive;
+    if (unit->flags1.bits.inactive)
+        return false;
+    else if (unit->flags1.bits.move_state || unit->flags1.bits.can_swap)
+        return true; // These are always unset when leaving the map
+    return linear_index(world->units.active, &df::unit::id, unit->id) >= 0;
 }
 
 bool Units::isVisible(df::unit *unit) {
@@ -660,7 +664,8 @@ bool Units::isGreatDanger(df::unit *unit) {
 
 bool Units::isUnitInBox(df::unit *u, const cuboid &box) {
     CHECK_NULL_POINTER(u);
-    return box.containsPos(getPosition(u));
+    auto pos = getPosition(u);
+    return pos.isValid() ? box.containsPos(pos) : false;
 }
 
 bool Units::getUnitsInBox(vector<df::unit *> &units, const cuboid &box, std::function<bool(df::unit *)> filter) {
@@ -745,6 +750,8 @@ bool Units::getCitizens(vector<df::unit *> &citizens, bool exclude_residents, bo
 
 df::coord Units::getPosition(df::unit *unit) {
     CHECK_NULL_POINTER(unit);
+    if (!isActive(unit))
+        return df::coord();
     if (unit->flags1.bits.caged) {
         if (auto cage = getContainer(unit))
             return Items::getPosition(cage);

plugins/preserve-rooms.cpp

@@ -446,7 +446,7 @@ static void handle_missing_assignments(color_ostream &out,
         if (!hf || hf->died_year > -1 || was_expelled(hf))
             continue;
         if (auto unit = df::unit::find(hf->unit_id)) {
-            if (Units::isActive(unit) && !Units::isDead(unit) && active_unit_ids.contains(unit->id)) {
+            if (!Units::isDead(unit) && Units::isActive(unit) && active_unit_ids.contains(unit->id)) {
                 // unit is still alive on the map; assume the unassigment was intentional/expected
                 continue;
             }
@@ -458,7 +458,7 @@ static void handle_missing_assignments(color_ostream &out,
         auto spouse_hf = df::historical_figure::find(spouse_hfid);
         auto spouse = spouse_hf ? df::unit::find(spouse_hf->unit_id) : nullptr;
         if (spouse_hf && share_with_spouse) {
-            if (spouse && Units::isActive(spouse) && !Units::isDead(spouse) && active_unit_ids.contains(spouse->id))
+            if (spouse && !Units::isDead(spouse) && Units::isActive(spouse) && active_unit_ids.contains(spouse->id))
             {
                 DEBUG(cycle,out).print("assigning zone %d (%s) to spouse %s\n",
                     zone_id, ENUM_KEY_STR(civzone_type, zone->type).c_str(),