diff --git a/docs/changelog.txt b/docs/changelog.txt index 3008dac024..fe3b11b476 100644 --- a/docs/changelog.txt +++ b/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 diff --git a/docs/dev/Lua API.rst b/docs/dev/Lua API.rst index 64bea03c3a..ad0c354f58 100644 --- a/docs/dev/Lua API.rst +++ b/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)`` diff --git a/library/RemoteTools.cpp b/library/RemoteTools.cpp index d8d5eb61e1..9653bcca6e 100644 --- a/library/RemoteTools.cpp +++ b/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++) { diff --git a/library/include/modules/Items.h b/library/include/modules/Items.h index 79aed92c36..250ae9b99a 100644 --- a/library/include/modules/Items.h +++ b/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 diff --git a/library/include/modules/Units.h b/library/include/modules/Units.h index 2e0f188b5c..ebbf9db5c1 100644 --- a/library/include/modules/Units.h +++ b/library/include/modules/Units.h @@ -207,7 +207,7 @@ inline auto citizensRange(std::vector &vec, bool exclude_residents = DFHACK_EXPORT void forCitizens(std::function fn, bool exclude_residents = false, bool include_insane = false); DFHACK_EXPORT bool getCitizens(std::vector &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. diff --git a/library/modules/Units.cpp b/library/modules/Units.cpp index 2a0e59edca..5231ac6f7d 100644 --- a/library/modules/Units.cpp +++ b/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 &units, const cuboid &box, std::function filter) { @@ -745,6 +750,8 @@ bool Units::getCitizens(vector &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); diff --git a/plugins/preserve-rooms.cpp b/plugins/preserve-rooms.cpp index 52f6151637..1852bac809 100644 --- a/plugins/preserve-rooms.cpp +++ b/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(),