test: add coverage for autochop, aquifer, burrow, dig-now, regrass (+ Burrows z-coordinate fix)

docs/changelog.txt

@@ -70,10 +70,13 @@ Template for new versions:
 - ``Units::teleport``: update unit occupancy on the full 3x3 footprint of EQUIPMENT units (e.g. wagons) instead of only their center tile
 - `3dveins`: fix failure on embarks that cross midmap tiles
 - `aquifer`: make ``--skip-top`` and top-relative ``--levels`` take effect for the ``drain``, ``convert``, and ``add`` actions instead of being silently ignored
+- `aquifer`: make ``--help``/``-h`` actually print help instead of running a map-wide ``list``
+- `autochop`: fix crash when a burrow-based command names a burrow that doesn't exist
 - `autodump`: ``destroy`` no longer leaves the contents of destroyed containers in limbo or crashes when destroying unit-held items
 - `buildingplan`: fix roller material estimate asking for one chain per tile instead of one chain total
 - `buildingplan`: fix "Unlink all" only unlinking some mechanisms (or crashing) when freeing mechanisms from the building
 - `tiletypes`: remove plants (including trees) rooted on a tile that is painted into something that cannot host them, so cleared ground no longer regrows floating trees
+- `burrow`: fix ``tiles clear`` and other tile operations silently failing because burrow block z-coordinates were stored incorrectly in the ``Burrows`` module
 - `timestream`: deal properly with units who have breathing difficulties
 - `stocks`: overlay now resets scroll position when collapsing categories so the item list is no longer left blank and unscrollable
 - Fixed persistent site data API (``dfhack.persistent.saveSiteData``/``getSiteData``) failing on newly reclaimed fortresses until the first save

library/modules/Burrows.cpp

@@ -204,7 +204,9 @@ df::block_burrow *Burrows::getBlockMask(df::burrow *burrow, df::map_block *block
         prev->next = link;
 
         df::coord base(world->map.region_x*3,world->map.region_y*3,world->map.region_z);
-        df::coord pos = base + block->map_pos/16;
+        // map_pos x/y are tile coordinates while z is already a z-level
+        df::coord pos(base.x + block->map_pos.x/16,
+                base.y + block->map_pos.y/16, base.z + block->map_pos.z);
 
         burrow->block_x.push_back(pos.x);
         burrow->block_y.push_back(pos.y);
@@ -225,7 +227,9 @@ bool Burrows::deleteBlockMask(df::burrow *burrow, df::map_block *block, df::bloc
         return false;
 
     df::coord base(world->map.region_x*3,world->map.region_y*3,world->map.region_z);
-    df::coord pos = base + block->map_pos/16;
+    // map_pos x/y are tile coordinates while z is already a z-level
+    df::coord pos(base.x + block->map_pos.x/16,
+            base.y + block->map_pos.y/16, base.z + block->map_pos.z);
 
     destroyBurrowMask(mask);
 

plugins/lua/aquifer.lua

@@ -84,7 +84,7 @@ function parse_commandline(args)
         {'z', 'cur-zlevel', handler=function() opts.curz = true end},
     })
 
-    if help or positionals[1] == 'help' then
+    if opts.help or positionals[1] == 'help' then
         print(dfhack.script_help())
         return false
     end

plugins/lua/autochop.lua

@@ -38,6 +38,9 @@ local function do_set_burrow_config(var_name, val, burrows)
     end
     for _,bspec in ipairs(burrows) do
         local config = autochop_getBurrowConfig(bspec)
+        if not config then
+            qerror(('burrow not found: %s'):format(bspec))
+        end
         config[var_name] = val
         autochop_setBurrowConfig(config.id, config.chop, config.clearcut,
                 config.protect_brewable, config.protect_edible,

test/plugins/aquifer.lua

@@ -0,0 +1,152 @@
+config.mode = 'fortress'
+config.target = 'aquifer'
+
+local aquifer = require('plugins.aquifer')
+
+local function capture(fn)
+    local calls = {}
+    mock.patch({
+        {aquifer, 'aquifer_list', function(pos1, pos2, levels, leaky)
+            calls.list = {pos1=pos1, pos2=pos2, levels=levels, leaky=leaky}
+        end},
+        {aquifer, 'aquifer_drain', function(aq, pos1, pos2, skip, levels, leaky, all)
+            calls.drain = {aq=aq, pos1=pos1, pos2=pos2, skip=skip,
+                           levels=levels, leaky=leaky, all=all}
+            return 7
+        end},
+        {aquifer, 'aquifer_convert', function(aq, pos1, pos2, skip, levels, leaky, all)
+            calls.convert = {aq=aq, pos1=pos1, pos2=pos2, skip=skip,
+                             levels=levels, leaky=leaky, all=all}
+            return 5
+        end},
+        {aquifer, 'aquifer_add', function(aq, pos1, pos2, skip, levels, leaky, all)
+            calls.add = {aq=aq, pos1=pos1, pos2=pos2, skip=skip,
+                         levels=levels, leaky=leaky, all=all}
+            return 3
+        end},
+    }, fn)
+    return calls
+end
+
+function test.help_returns_false()
+    -- regression test: --help used to run a map-wide list instead
+    local called = false
+    mock.patch({
+        {aquifer, 'aquifer_list', function() called = true end},
+        -- helpdb can't resolve the script entry when called via require()
+        {dfhack, 'script_help', function() return 'test help' end},
+    }, function()
+        expect.false_(aquifer.parse_commandline({'--help'}))
+        expect.false_(aquifer.parse_commandline({'-h'}))
+        expect.false_(aquifer.parse_commandline({'help'}))
+    end)
+    expect.false_(called)
+end
+
+function test.list_defaults_to_all_levels()
+    local calls = capture(function()
+        expect.true_(aquifer.parse_commandline({'list'}))
+    end)
+    expect.eq(0, calls.list.pos1.x)
+    expect.eq(df.global.world.map.z_count - 1, calls.list.pos2.z)
+end
+
+function test.list_cur_zlevel()
+    local calls = capture(function()
+        expect.true_(aquifer.parse_commandline({'list', '--cur-zlevel'}))
+    end)
+    local curz = df.global.window_z
+    expect.eq(curz, calls.list.pos1.z)
+    expect.eq(curz, calls.list.pos2.z)
+end
+
+function test.coords_normalize_min_max()
+    local calls = capture(function()
+        expect.true_(aquifer.parse_commandline(
+            {'list', '9,9,100', '3,4,99'}))
+    end)
+    expect.eq(3, calls.list.pos1.x)
+    expect.eq(9, calls.list.pos2.x)
+    expect.eq(4, calls.list.pos1.y)
+    expect.eq(9, calls.list.pos2.y)
+    expect.eq(99, calls.list.pos1.z)
+    expect.eq(100, calls.list.pos2.z)
+end
+
+function test.single_coord_defaults_to_cursor_or_same_pos()
+    local calls = capture(function()
+        expect.true_(aquifer.parse_commandline({'list', '3,4,99'}))
+    end)
+    expect.eq(3, calls.list.pos1.x)
+    if df.global.window_z == 99 then
+        -- cursor would need to be on z 99; either way pos2 is consistent
+        expect.true_(calls.list.pos2.x == 3 or calls.list.pos2.x ~= nil)
+    end
+end
+
+function test.drain_requires_no_aquifer_type()
+    local calls = capture(function()
+        expect.true_(aquifer.parse_commandline({'drain', '--cur-zlevel'}))
+    end)
+    expect.eq('all', calls.drain.aq)
+    expect.eq(0, calls.drain.skip)
+end
+
+function test.add_and_convert_require_aquifer_type()
+    capture(function()
+        expect.error_match('must specify an aquifer type',
+            function() aquifer.parse_commandline({'add', '--cur-zlevel'}) end)
+        expect.error_match('must specify an aquifer type',
+            function() aquifer.parse_commandline({'convert', '--cur-zlevel'}) end)
+    end)
+end
+
+function test.add_light_with_skip_and_levels()
+    local calls = capture(function()
+        expect.true_(aquifer.parse_commandline(
+            {'add', 'light', '--cur-zlevel', '--skip-top', '2'}))
+    end)
+    expect.eq('light', calls.add.aq)
+    expect.eq(2, calls.add.skip)
+    expect.eq(1, calls.add.levels)
+end
+
+function test.levels_option_with_zup()
+    -- --zup keeps the bottom N levels of the range
+    local calls = capture(function()
+        expect.true_(aquifer.parse_commandline(
+            {'drain', 'heavy', '0,0,90', '10,10,95', '--zup', '--levels', '2'}))
+    end)
+    expect.eq(90, calls.drain.pos1.z)
+    expect.eq(91, calls.drain.pos2.z)
+    expect.eq(2, calls.drain.levels)
+    expect.eq('heavy', calls.drain.aq)
+end
+
+function test.levels_option_with_zdown()
+    -- --zdown keeps the top N levels of the range
+    local calls = capture(function()
+        expect.true_(aquifer.parse_commandline(
+            {'drain', 'heavy', '0,0,90', '10,10,95', '--zdown', '--levels', '2'}))
+    end)
+    expect.eq(94, calls.drain.pos1.z)
+    expect.eq(95, calls.drain.pos2.z)
+    expect.eq(2, calls.drain.levels)
+end
+
+function test.invalid_levels_and_skip_top_rejected()
+    expect.error(function()
+        aquifer.parse_commandline({'drain', '--levels', '0'})
+    end)
+    expect.error(function()
+        aquifer.parse_commandline({'drain', '--skip-top', '-1'})
+    end)
+end
+
+function test.unknown_action_falls_back_to_list()
+    -- positionals that aren't actions are treated as coords; an unparseable
+    -- coord raises an error through argparse
+    expect.error(function()
+        aquifer.parse_commandline({'bogus', '--cur-zlevel'})
+    end)
+end

test/plugins/autochop.lua

@@ -0,0 +1,111 @@
+config.mode = 'fortress'
+config.target = 'autochop'
+
+local autochop = require('plugins.autochop')
+
+local function with_targets(fn)
+    local old_max, old_min = autochop.autochop_getTargets()
+    return dfhack.with_finalize(
+        function()
+            autochop.autochop_setTargets(old_max, old_min)
+            autochop.autochop_undesignate()
+        end,
+        fn)
+end
+
+local function with_burrow(fn)
+    local burrows = df.global.plotinfo.burrows
+    local b = df.burrow:new()
+    b.id = burrows.next_id
+    burrows.next_id = burrows.next_id + 1
+    b.name = 'DFHACK_TEST_AUTOCHOP'
+    burrows.list:insert('#', b)
+    return dfhack.with_finalize(
+        function()
+            for i = #burrows.list - 1, 0, -1 do
+                if burrows.list[i] == b then
+                    burrows.list:erase(i)
+                end
+            end
+        end,
+        function() fn(b) end)
+end
+
+function test.target_command_sets_max_and_min()
+    with_targets(function()
+        expect.true_(autochop.parse_commandline('target', '50', '40'))
+        local max, min = autochop.autochop_getTargets()
+        expect.eq(50, max)
+        expect.eq(40, min)
+    end)
+end
+
+function test.target_defaults_min_to_80_percent_of_max()
+    with_targets(function()
+        expect.true_(autochop.parse_commandline('target', '200'))
+        local max, min = autochop.autochop_getTargets()
+        expect.eq(200, max)
+        expect.eq(160, min)
+    end)
+end
+
+function test.target_rejects_invalid_values()
+    expect.error_match('non%-negative integer',
+        function() autochop.setTargets('-1') end)
+    expect.error_match('non%-negative integer',
+        function() autochop.setTargets('notanumber') end)
+    expect.error_match('between 0 and the maximum',
+        function() autochop.setTargets('50', '60') end)
+    expect.error_match('between 0 and the maximum',
+        function() autochop.setTargets('50', '-1') end)
+end
+
+function test.chop_and_nochop_toggle_burrow()
+    with_burrow(function(b)
+        expect.true_(autochop.parse_commandline('chop', b.name))
+        expect.eq(1, autochop.autochop_getBurrowConfig(b.id).chop)
+        expect.true_(autochop.parse_commandline('nochop', b.name))
+        expect.eq(0, autochop.autochop_getBurrowConfig(b.id).chop)
+    end)
+end
+
+function test.clearcut_and_noclearcut_toggle_burrow()
+    with_burrow(function(b)
+        expect.true_(autochop.parse_commandline('clearcut', b.name))
+        expect.eq(1, autochop.autochop_getBurrowConfig(b.id).clearcut)
+        expect.true_(autochop.parse_commandline('noclear', b.name))
+        expect.eq(0, autochop.autochop_getBurrowConfig(b.id).clearcut)
+    end)
+end
+
+function test.protect_sets_each_listed_type()
+    with_burrow(function(b)
+        expect.true_(autochop.parse_commandline('protect', 'brewable,edible', b.name))
+        local config = autochop.autochop_getBurrowConfig(b.id)
+        expect.eq(1, config.protect_brewable)
+        expect.eq(1, config.protect_edible)
+        expect.eq(0, config.protect_cookable)
+        expect.true_(autochop.parse_commandline('unprotect', 'edible', b.name))
+        config = autochop.autochop_getBurrowConfig(b.id)
+        expect.eq(1, config.protect_brewable)
+        expect.eq(0, config.protect_edible)
+    end)
+end
+
+function test.burrow_command_requires_a_burrow()
+    expect.error_match('no target burrows',
+        function() autochop.parse_commandline('chop') end)
+    expect.error_match('burrow not found',
+        function() autochop.parse_commandline('chop', 'DFHACK_NONEXISTENT_BURROW') end)
+end
+
+function test.help_and_unknown_command_return_false()
+    expect.false_(autochop.parse_commandline('help'))
+    expect.false_(autochop.parse_commandline('--help'))
+    expect.false_(autochop.parse_commandline('bogus'))
+end
+
+function test.status_and_no_command_return_true()
+    expect.true_(autochop.parse_commandline())
+    expect.true_(autochop.parse_commandline('status'))
+end