From b5d7d0a32a4172e02ab630009a627fba3469fc17 Mon Sep 17 00:00:00 2001 From: Ryan Williams Date: Fri, 20 Sep 2024 03:39:15 -0700 Subject: [PATCH 01/10] Update Lua API.rst * Add docs for units.isInPlay * Update docs w/r/t item/unit active assumptions --- docs/dev/Lua API.rst | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/docs/dev/Lua API.rst b/docs/dev/Lua API.rst index 232daea9eb..ee14172c66 100644 --- a/docs/dev/Lua API.rst +++ b/docs/dev/Lua API.rst @@ -1402,7 +1402,16 @@ Units module * ``dfhack.units.isActive(unit)`` - The unit is active (non-dead and on the map). + The unit is active (non-dead and probably on the map). Unit must also be + present in ``world.units.active`` to rule out raid missions. Use + ``dfhack.units.isInPlay`` instead if you aren't certain. + +* ``dfhack.units.isInPlay(unit)`` + + The unit is active and in play (non-dead and definitely on the map). + This function scans ``world.units.active`` to make sure the unit isn't + out on raid. If you're already iterating ``world.units.active``, just use + ``dfhack.units.isActive`` for efficiency. * ``dfhack.units.isVisible(unit)`` @@ -1608,12 +1617,14 @@ Units module * ``dfhack.units.isUnitInBox(unit,x1,y1,z1,x2,y2,z2)`` - Returns true if the unit is within a box defined by the - specified coordinates. + Returns true if the unit is within a box defined by the specified + coordinates. Make sure the unit is in play first, as this can return true + for a death location or where the unit left the map. * ``dfhack.units.getUnitsInBox(x1,y1,z1,x2,y2,z2[,filter])`` - Returns a table of all units within the specified coordinates. + Returns a table of all units within the specified coordinates. Returned + units are guaranteed to be in play (unlike ``isUnitInBox`` above). If the ``filter`` argument is given, only units where ``filter(unit)`` returns true will be included. Note that ``pos2xyz()`` cannot currently be used to convert coordinate objects to the arguments required by @@ -1643,7 +1654,9 @@ 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. Make sure the unit is in + play first (using ``dfhack.units.isActive`` or ``dfhack.units.isInPlay``) + or the result can indicate a death location or where the unit left the map. * ``dfhack.units.teleport(unit, pos)`` @@ -2033,7 +2046,9 @@ Items module Returns the true *x,y,z* of the item, or *nil* if invalid. You should generally use this method instead of reading *item.pos* directly since that field only stores - the last position where the item was on the ground. + the last position where the item was on the ground. Make sure the item is present in + ``world.items.other.IN_PLAY`` first, otherwise the result can indicate where a unit + left the map with the item. * ``dfhack.items.getBookTitle(item)`` From b42d5c8e24e6c8852feb0ea6485cbfa135362d5b Mon Sep 17 00:00:00 2001 From: Ryan Williams Date: Fri, 20 Sep 2024 03:48:21 -0700 Subject: [PATCH 02/10] Update LuaApi.cpp - Add isInPlay --- library/LuaApi.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/library/LuaApi.cpp b/library/LuaApi.cpp index e193bb8cc6..8872d1797a 100644 --- a/library/LuaApi.cpp +++ b/library/LuaApi.cpp @@ -1990,6 +1990,7 @@ static const luaL_Reg dfhack_textures_funcs[] = { static const LuaWrapper::FunctionReg dfhack_units_module[] = { WRAPM(Units, isActive), + WRAPM(Units, isInPlay), WRAPM(Units, isVisible), WRAPM(Units, isCitizen), WRAPM(Units, isResident), From 068a8b9b39cdc0612d7532301efb07d4c9db72be Mon Sep 17 00:00:00 2001 From: Ryan Williams Date: Fri, 20 Sep 2024 03:57:41 -0700 Subject: [PATCH 03/10] Update RemoteTools.cpp - Use units.active vector Since we're ruling out inactive non-dead units, prevent raiding unit jank. --- library/RemoteTools.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/RemoteTools.cpp b/library/RemoteTools.cpp index c8fc810c8b..79fcf7b225 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++) { From c6873903bc6ce0d5fecf80dfab19b192fd22c1c9 Mon Sep 17 00:00:00 2001 From: Ryan Williams Date: Fri, 20 Sep 2024 03:58:49 -0700 Subject: [PATCH 04/10] Update Items.h - getPosition caveat --- library/include/modules/Items.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/library/include/modules/Items.h b/library/include/modules/Items.h index 79aed92c36..0f797ae50e 100644 --- a/library/include/modules/Items.h +++ b/library/include/modules/Items.h @@ -137,7 +137,8 @@ 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). +/// Returns the true position of the item (non-trivial if in inventory). +/// Note: Make sure the item is in world.items.other.IN_PLAY first, else can be inaccurate. 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 From bff9f484b3853749fe11001909758d89723c22ef Mon Sep 17 00:00:00 2001 From: Ryan Williams Date: Fri, 20 Sep 2024 04:01:22 -0700 Subject: [PATCH 05/10] Update Units.h - isInPlay; caveats to unit pos --- library/include/modules/Units.h | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/library/include/modules/Units.h b/library/include/modules/Units.h index 7eaa61ec62..9c3a3a3ac0 100644 --- a/library/include/modules/Units.h +++ b/library/include/modules/Units.h @@ -69,8 +69,12 @@ namespace Units { * The Units module - allows reading all non-vermin units and their properties */ -// Unit is non-dead and on the map. +/// Unit is non-dead and on the map (usually). Unit must also be present in world.units.active +/// to rule out raid missions. Use Units::isInPlay instead if you aren't certain. DFHACK_EXPORT bool isActive(df::unit *unit); +/// Unit is non-dead and on the map. Scans world.units.active for the unit to make certain. +/// (This is inefficient if you're already iterating the active units vector. Use Units::isActive.) +DFHACK_EXPORT bool isInPlay(df::unit *unit); // Unit is on visible map tile. Doesn't account for ambushing. DFHACK_EXPORT bool isVisible(df::unit *unit); // Unit is a non-dead (optionally sane) citizen of fort. @@ -174,12 +178,12 @@ DFHACK_EXPORT bool isDanger(df::unit *unit); // Megabeasts, titans, forgotten beasts, and demons. DFHACK_EXPORT bool isGreatDanger(df::unit *unit); -// Check if unit is inside the cuboid area. +// Check if unit is inside the cuboid area. Note: Make sure unit is in play first, else can be inaccurate. DFHACK_EXPORT bool isUnitInBox(df::unit *u, const cuboid &box); DFHACK_EXPORT inline bool isUnitInBox(df::unit *u, int16_t x1, int16_t y1, int16_t z1, int16_t x2, int16_t y2, int16_t z2) { return isUnitInBox(u, cuboid(x1, y1, z1, x2, y2, z2)); } -// Fill vector with units in box matching filter. +// Fill vector with units in box matching filter. Note: Units guaranteed to be in play. DFHACK_EXPORT bool getUnitsInBox(std::vector &units, const cuboid &box, std::function filter = [](df::unit *u) { return true; }); DFHACK_EXPORT inline bool getUnitsInBox(std::vector &units, int16_t x1, int16_t y1, int16_t z1, @@ -202,7 +206,8 @@ 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). +/// Returns the true position of the unit (non-trivial in case of caged). +/// Note: Make sure unit is in play first, else can be inaccurate. DFHACK_EXPORT df::coord getPosition(df::unit *unit); // Moves unit and any riders to the target coordinates. Sets tile occupancy flags. From de9eb123e365aeeabd8e91059a98b15dc58bcc61 Mon Sep 17 00:00:00 2001 From: Ryan Williams Date: Fri, 20 Sep 2024 04:05:59 -0700 Subject: [PATCH 06/10] Update Units.cpp - isInPlay; rule out dead units getUnitsInBox --- library/modules/Units.cpp | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/library/modules/Units.cpp b/library/modules/Units.cpp index 44fec6ae2c..0e0b794243 100644 --- a/library/modules/Units.cpp +++ b/library/modules/Units.cpp @@ -125,6 +125,12 @@ bool Units::isActive(df::unit *unit) { return !unit->flags1.bits.inactive; } +bool Units::isInPlay(df::unit *unit) { + CHECK_NULL_POINTER(unit); + return isActive(unit) && + linear_index(world->units.active, &df::unit::id, unit->id) >= 0; +} + bool Units::isVisible(df::unit *unit) { CHECK_NULL_POINTER(unit); return Maps::isTileVisible(getPosition(unit)); @@ -667,7 +673,7 @@ bool Units::getUnitsInBox(vector &units, const cuboid &box, std::fun units.clear(); for (auto unit : world->units.active) - if (filter(unit) && isUnitInBox(unit, box)) + if (isActive(unit) && filter(unit) && isUnitInBox(unit, box)) units.push_back(unit); return true; } From 1b5ccd653bc9b94e50bcb7e13674bcfe03731f6d Mon Sep 17 00:00:00 2001 From: Ryan Williams Date: Fri, 20 Sep 2024 04:18:51 -0700 Subject: [PATCH 07/10] Update spectate.cpp - Don't follow death locations --- plugins/spectate/spectate.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/plugins/spectate/spectate.cpp b/plugins/spectate/spectate.cpp index 61584e8258..dd3e3b54be 100644 --- a/plugins/spectate/spectate.cpp +++ b/plugins/spectate/spectate.cpp @@ -202,6 +202,8 @@ namespace SP { } }; static auto valid = [](df::unit* unit) { + if (!Units::isActive(unit)) + return false; if (Units::isAnimal(unit)) { return config.animals; } From accb6800c67b7d4bfdb9a91efe9352bcaf780931 Mon Sep 17 00:00:00 2001 From: Ryan Williams Date: Fri, 20 Sep 2024 04:29:55 -0700 Subject: [PATCH 08/10] Update changelog.txt --- docs/changelog.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/changelog.txt b/docs/changelog.txt index 7fbf560635..5465ec031e 100644 --- a/docs/changelog.txt +++ b/docs/changelog.txt @@ -33,8 +33,10 @@ Template for new versions: ## Misc Improvements ## Documentation +Document caveats about unit/item position fns w/r/t raiding units. ## API +``Units::isInPlay``: A fn that scans active unit vector to rule out units off map raiding. Use instead of ``Units::isActive`` when uncertain if unit present in ``world.units.active``. ## Lua From 04110bda9124a5f7fa0091aca6525a7228ad08da Mon Sep 17 00:00:00 2001 From: Ryan Williams Date: Sat, 21 Sep 2024 05:01:46 -0700 Subject: [PATCH 09/10] Undo add Units::isInPlay * Update changelog.txt * Update Lua API.rst * Update LuaApi.cpp * Update Units.h * Update Units.cpp --- docs/changelog.txt | 1 - docs/dev/Lua API.rst | 25 ++++++++++--------------- library/LuaApi.cpp | 1 - library/include/modules/Units.h | 11 ++++------- library/modules/Units.cpp | 6 ------ 5 files changed, 14 insertions(+), 30 deletions(-) diff --git a/docs/changelog.txt b/docs/changelog.txt index 5465ec031e..b45e459f9a 100644 --- a/docs/changelog.txt +++ b/docs/changelog.txt @@ -36,7 +36,6 @@ Template for new versions: Document caveats about unit/item position fns w/r/t raiding units. ## API -``Units::isInPlay``: A fn that scans active unit vector to rule out units off map raiding. Use instead of ``Units::isActive`` when uncertain if unit present in ``world.units.active``. ## Lua diff --git a/docs/dev/Lua API.rst b/docs/dev/Lua API.rst index ee14172c66..79075379ab 100644 --- a/docs/dev/Lua API.rst +++ b/docs/dev/Lua API.rst @@ -1403,15 +1403,9 @@ Units module * ``dfhack.units.isActive(unit)`` The unit is active (non-dead and probably on the map). Unit must also be - present in ``world.units.active`` to rule out raid missions. Use - ``dfhack.units.isInPlay`` instead if you aren't certain. - -* ``dfhack.units.isInPlay(unit)`` - - The unit is active and in play (non-dead and definitely on the map). - This function scans ``world.units.active`` to make sure the unit isn't - out on raid. If you're already iterating ``world.units.active``, just use - ``dfhack.units.isActive`` for efficiency. + present in the ``world.units.active`` vector to rule out raid missions. Use + ``utils.linear_index`` after this function returns true if you aren't + certain (i.e., you aren't already iterating that vector). * ``dfhack.units.isVisible(unit)`` @@ -1618,13 +1612,14 @@ Units module * ``dfhack.units.isUnitInBox(unit,x1,y1,z1,x2,y2,z2)`` Returns true if the unit is within a box defined by the specified - coordinates. Make sure the unit is in play first, as this can return true - for a death location or where the unit left the map. + coordinates. Make sure the unit is flagged active and is present in + ``world.units.active`` first, as the result may indicate that the unit + died or left map here. * ``dfhack.units.getUnitsInBox(x1,y1,z1,x2,y2,z2[,filter])`` Returns a table of all units within the specified coordinates. Returned - units are guaranteed to be in play (unlike ``isUnitInBox`` above). + units are guaranteed to be active (unlike ``isUnitInBox`` above). If the ``filter`` argument is given, only units where ``filter(unit)`` returns true will be included. Note that ``pos2xyz()`` cannot currently be used to convert coordinate objects to the arguments required by @@ -1654,9 +1649,9 @@ 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. Make sure the unit is in - play first (using ``dfhack.units.isActive`` or ``dfhack.units.isInPlay``) - or the result can indicate a death location or where the unit left the map. + that field can be inaccurate when the unit is caged. Make sure the unit is + active (and present in ``world.units.active``) first or else the result can + indicate where the unit died or left the map. * ``dfhack.units.teleport(unit, pos)`` diff --git a/library/LuaApi.cpp b/library/LuaApi.cpp index 8872d1797a..e193bb8cc6 100644 --- a/library/LuaApi.cpp +++ b/library/LuaApi.cpp @@ -1990,7 +1990,6 @@ static const luaL_Reg dfhack_textures_funcs[] = { static const LuaWrapper::FunctionReg dfhack_units_module[] = { WRAPM(Units, isActive), - WRAPM(Units, isInPlay), WRAPM(Units, isVisible), WRAPM(Units, isCitizen), WRAPM(Units, isResident), diff --git a/library/include/modules/Units.h b/library/include/modules/Units.h index 9c3a3a3ac0..0fd16bc3f9 100644 --- a/library/include/modules/Units.h +++ b/library/include/modules/Units.h @@ -70,11 +70,8 @@ namespace Units { */ /// Unit is non-dead and on the map (usually). Unit must also be present in world.units.active -/// to rule out raid missions. Use Units::isInPlay instead if you aren't certain. +/// to rule out raid missions. DFHACK_EXPORT bool isActive(df::unit *unit); -/// Unit is non-dead and on the map. Scans world.units.active for the unit to make certain. -/// (This is inefficient if you're already iterating the active units vector. Use Units::isActive.) -DFHACK_EXPORT bool isInPlay(df::unit *unit); // Unit is on visible map tile. Doesn't account for ambushing. DFHACK_EXPORT bool isVisible(df::unit *unit); // Unit is a non-dead (optionally sane) citizen of fort. @@ -178,12 +175,12 @@ DFHACK_EXPORT bool isDanger(df::unit *unit); // Megabeasts, titans, forgotten beasts, and demons. DFHACK_EXPORT bool isGreatDanger(df::unit *unit); -// Check if unit is inside the cuboid area. Note: Make sure unit is in play first, else can be inaccurate. +// Check if unit is inside the cuboid area. Note: Make sure unit is truly active first, else can be inaccurate. DFHACK_EXPORT bool isUnitInBox(df::unit *u, const cuboid &box); DFHACK_EXPORT inline bool isUnitInBox(df::unit *u, int16_t x1, int16_t y1, int16_t z1, int16_t x2, int16_t y2, int16_t z2) { return isUnitInBox(u, cuboid(x1, y1, z1, x2, y2, z2)); } -// Fill vector with units in box matching filter. Note: Units guaranteed to be in play. +// Fill vector with units in box matching filter. Note: Units guaranteed to be active. DFHACK_EXPORT bool getUnitsInBox(std::vector &units, const cuboid &box, std::function filter = [](df::unit *u) { return true; }); DFHACK_EXPORT inline bool getUnitsInBox(std::vector &units, int16_t x1, int16_t y1, int16_t z1, @@ -207,7 +204,7 @@ DFHACK_EXPORT void forCitizens(std::function fn, bool exclude_ 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). -/// Note: Make sure unit is in play first, else can be inaccurate. +/// Note: Make sure unit is truly active first, else can be inaccurate. 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 0e0b794243..ee835f67af 100644 --- a/library/modules/Units.cpp +++ b/library/modules/Units.cpp @@ -125,12 +125,6 @@ bool Units::isActive(df::unit *unit) { return !unit->flags1.bits.inactive; } -bool Units::isInPlay(df::unit *unit) { - CHECK_NULL_POINTER(unit); - return isActive(unit) && - linear_index(world->units.active, &df::unit::id, unit->id) >= 0; -} - bool Units::isVisible(df::unit *unit) { CHECK_NULL_POINTER(unit); return Maps::isTileVisible(getPosition(unit)); From 43a20e1017d6b13e9da7b155f3828ac69ff47986 Mon Sep 17 00:00:00 2001 From: Ryan Williams Date: Sat, 21 Sep 2024 05:11:38 -0700 Subject: [PATCH 10/10] Update Lua API.rst --- docs/dev/Lua API.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/dev/Lua API.rst b/docs/dev/Lua API.rst index 79075379ab..64c4646121 100644 --- a/docs/dev/Lua API.rst +++ b/docs/dev/Lua API.rst @@ -1405,7 +1405,7 @@ Units module The unit is active (non-dead and probably on the map). Unit must also be present in the ``world.units.active`` vector to rule out raid missions. Use ``utils.linear_index`` after this function returns true if you aren't - certain (i.e., you aren't already iterating that vector). + certain (i.e., not already iterating active units). * ``dfhack.units.isVisible(unit)`` @@ -1614,7 +1614,7 @@ Units module Returns true if the unit is within a box defined by the specified coordinates. Make sure the unit is flagged active and is present in ``world.units.active`` first, as the result may indicate that the unit - died or left map here. + died or left the map here. * ``dfhack.units.getUnitsInBox(x1,y1,z1,x2,y2,z2[,filter])``