respond to review:

- expand documentation - do not (try to) dump/melt artifacts - disable melting (pending API availability) - implcit filters for actions and --dry-run option
DFHack · Nov 20, 2023 · 2827911 · 2827911
1 parent 9ec1c95
commit 2827911
Show file tree

Hide file tree

Showing 2 changed files with 53 additions and 28 deletions.
diff --git a/docs/item.rst b/docs/item.rst
@@ -7,19 +7,35 @@ item
 
 Filter items in you fort by various properties (e.g., item type, material,
 wear-level, quality, ...), and perform the bulk operations forbid, dump, melt,
-and their inverses. By default, the tool does not act on artifacts.
+and their inverses. By default, the tool does not consider artifacts. Outputs
+the number of items that matched the filters and were modified.
 
 Usage
 -----
 
 ``item [ help | count | [un]forbid | [un]dump | [un]melt ] <filter option>``
 
+Action names should be self explanatory. All actions other than ``help`` and
+``count`` have implicit filters associated with them. For instance, ``item
+forbid --unreachable`` will neither report nor test reachability of items that
+are already forbidden.
+
 Options
 -------
 
 ``-h, --help``
     Print help.
 
+``-n, --dry-run``
+    Use get an accurate count of the items that would be affected, including the
+    implicit filters of the selected main action.
+
+``-i, --inside <burrow>``
+    Only include items inside the given burrow.
+
+``-o, --outside <burrow>``
+    Only include items outside the given burrow.
+
 ``-r, --reachable``
     Only include items reachable by one of your citizens.
 
@@ -28,18 +44,21 @@ Options
 
 ``-t, --type <string>``
     Filter by item type (e.g., BOULDER, CORPSE, ...). Also accepts lower case
-    spelling (e.g. "corpse")
+    spelling (e.g. "corpse"). Use ``:lua @df.item_type`` to get the list of all
+    item types.
 
 ``-m, --material <string>``
     Filter by material the item is made out of (e.g., "iron").
 
 ``-d, --description <string>``
     Filter by item description (singular form without stack sizes). Example:
-    "cave spider silk web". Note: This does not work for animal products such as
-    wool, because their description always includes the stack size.
+    "cave spider silk web". Note: Due to a bug, this is of limited use for some
+    animal products such as wool, because their description always includes the
+    stack size.
 
 ``-a, --include-artifacts``
-    Include artifacts in the item list.
+    Include artifacts in the item list. Regardless of this setting, artifacts
+    are never dumped or melted
 
 ``--min-wear <integer>``
     Only include items whose wear/damage level is at least ``integer``. Useful

diff --git a/item.lua b/item.lua
@@ -47,6 +47,7 @@ local function fastReachable(item,wgroups)
     end
 end
 
+--- @return table<integer,boolean>
 local function citizenWalkabilityGroups()
     local cgroups = {}
     for _, unit in pairs(dfhack.units.getCitizens(true)) do
@@ -64,6 +65,7 @@ local function isMeltable(item)
     local matinfo = dfhack.matinfo.decode(item)
     return
         matinfo:matches{metal=true} and
+	not item.flags.artifact and
         df.item_type[type] ~= 'CORPSE' and
         df.item_type[type] ~= 'CORPSEPIECE' and
         df.item_type[type] ~= 'REMAINS' and
@@ -155,7 +157,7 @@ end
 function item.act(action,conditons,options)
     local count = 0
     local changed = 0
-    local invisible = 0
+
     for _, item in pairs(df.global.world.items.other.IN_PLAY) do
         -- never act on items used for constructions/building materials and carried by hostiles
         -- also skip artifacts, unless explicitly told to include them
@@ -167,51 +169,54 @@ function item.act(action,conditons,options)
             goto skipitem
         end
 
+	-- implicit filters:
+        if  action == 'melt' and (item.flags.melt or not isMeltable(item)) or
+	    action == 'unmelt' and not item.flags.melt or
+	    action == 'forbid' and item.flags.forbid or
+	    action == 'unforbid' and not item.flags.forbid or
+	    action == 'dump' and (item.flags.dump or item.flags.artifact) or
+	    action == 'undump' and not item.flags.dump
+	then
+            goto skipitem
+        end
+
         -- check conditions provided via options
         for _, condition in pairs(conditons) do
             if not condition(item) then goto skipitem end
         end
 
-        -- only try to melt things that are actually meltable
-        -- TOTHINK: treated as implicit filter, should we count the item?
-        if action == 'melt' and not isMeltable(item) then
-            goto skipitem
-        end
-
-        -- item matches the filters
-        count = count + 1
-
         -- skip items that are in unrevealed parts of the map
         local x,y,z = dfhack.items.getPosition(item)
         if x and not dfhack.maps.isTileVisible(x,y,z) then
-            invisible = invisible+1
             goto skipitem
         end
 
+	-- item matches the filters
+        count = count + 1
+
         -- carry out the action
-        if action == 'forbid' and not item.flags.forbid then
+        if action == 'forbid' and not options.dryrun then
             item.flags.forbid = true
             changed = changed + 1
-        elseif action == 'unforbid' and item.flags.forbid then
+        elseif action == 'unforbid' and not options.dryrun then
             item.flags.forbid = false
             changed = changed + 1
-        elseif action == 'dump' and not item.flags.dump then
+        elseif action == 'dump' and not options.dryrun then
             item.flags.dump = true
             changed = changed + 1
-        elseif action == 'undump' and item.flags.dump then
+        elseif action == 'undump' and not options.dryrun then
             item.flags.dump = false
             changed = changed + 1
-        elseif action == 'melt' and not item.flags.melt then
+        elseif action == 'melt' and not options.dryrun then
             item.flags.melt = true
             changed = changed + 1
-        elseif action == 'unmelt' and item.flags.melt then
+        elseif action == 'unmelt' and not options.dryrun then
             item.flags.melt = false
             changed = changed + 1
         end
         :: skipitem ::
     end
     print(count, 'items matched the filter options')
-    print(invisible, 'invisible items were skipped')
     print(changed, 'items were acted upon')
 end
 
@@ -222,14 +227,16 @@ local argparse = require('argparse')
 
 local options, args = {
     help = false,
-    artifact = false
+    artifact = false,
+    dryrun = false
 }, { ... }
 
 local conditions = {}
 
 local positionals = argparse.processArgsGetopt(args, {
   { 'h', 'help', handler = function() options.help = true end },
   { 'a', 'include-artifacts', handler = function() options.artifact = true end },
+  { 'n', '--dry-run', handler = function() options.dryrun = true end },
   { 'i', 'inside', hasArg = true,
     handler = function (name)
         local burrow = dfhack.burrows.findByName(name)
@@ -275,15 +282,14 @@ local positionals = argparse.processArgsGetopt(args, {
 local action = nil
 
 if options.help or positionals[1] == 'help' then
-    -- print(dfhack.script_help())
-    print("HELP!")
+    print(dfhack.script_help())
     return
 elseif positionals[1] == 'forbid' then action = 'forbid'
 elseif positionals[1] == 'unforbid' then action = 'unforbid'
 elseif positionals[1] == 'dump' then action = 'dump'
 elseif positionals[1] == 'undump' then action = 'undump'
-elseif positionals[1] == 'melt' then action = 'melt'
-elseif positionals[1] == 'unmelt' then action = 'unmelt'
+-- elseif positionals[1] == 'melt' then action = 'melt'
+-- elseif positionals[1] == 'unmelt' then action = 'unmelt'
 elseif positionals[1] == 'count' then action = 'count'
 end