From 4733aadba1f224ece685c999cc26f042e80965ae Mon Sep 17 00:00:00 2001 From: SilasD Date: Fri, 25 Sep 2026 16:35:05 -0700 Subject: [PATCH] Units::setPathGoal check for NULL unit. --- docs/changelog.txt | 1 + library/modules/Units.cpp | 1 + test/modules/units_fortress.lua | 44 +++++++++++++++++++++++++++++++++ 3 files changed, 46 insertions(+) diff --git a/docs/changelog.txt b/docs/changelog.txt index 8c2bca09b0..b8d691fd26 100644 --- a/docs/changelog.txt +++ b/docs/changelog.txt @@ -33,6 +33,7 @@ Template for new versions: ## New Features ## Fixes +- ``Units::setPathGoal``: no longer crashes when passed NULL as a unit. Also fixes ``dfhack.units.setPathGoal()``. ## Misc Improvements diff --git a/library/modules/Units.cpp b/library/modules/Units.cpp index 975202d629..3dc17f449d 100644 --- a/library/modules/Units.cpp +++ b/library/modules/Units.cpp @@ -1029,6 +1029,7 @@ void Units::setAutomaticProfessions(df::unit* unit) { // functionality reverse-engineered from DF's unitst::set_goal void Units::setPathGoal(df::unit *unit, df::coord pos, df::unit_path_goal goal) { + CHECK_NULL_POINTER(unit); if (unit->path.dest != pos || unit->path.goal != goal) { unit->path.dest = pos; diff --git a/test/modules/units_fortress.lua b/test/modules/units_fortress.lua index aaeab7d9c5..c0f4cfdfd9 100644 --- a/test/modules/units_fortress.lua +++ b/test/modules/units_fortress.lua @@ -1,10 +1,18 @@ config.mode = 'fortress' config.target = 'core' +local utils = require('utils') + +---TODO when we get LuaLS integrated, declare coord in just one place. +---@alias coord { x:integer, y:integer, z:integer } | df.coord + +---@param pos coord +---@return df.tile_occupancy local function tile_occupancy(pos) return select(2, dfhack.maps.getTileFlags(pos)) end +---@return df.unit, df.unit local function two_citizens() local a, b for _, unit in ipairs(df.global.world.units.active) do @@ -21,6 +29,9 @@ local function two_citizens() end -- find an allocated, walkable tile with no unit occupancy near pos +---@param pos coord +---@return coord? +---@return string? local function free_tile_near(pos) for dx = -4, 4 do for dy = -4, 4 do if dx ~= 0 or dy ~= 0 then @@ -40,6 +51,8 @@ end -- recompute the unit occupancy flags of the given tiles from the given -- units, so fabricated flag states do not leak into later tests +---@param tiles coord[] +---@param units df.unit[] local function resync_occupancy(tiles, units) for _, pos in ipairs(tiles) do local occ = tile_occupancy(pos) @@ -138,3 +151,34 @@ function test.teleport_keeps_unit_flag_with_other_standing_unit() expect.false_(tile_occupancy(shared).unit) end) end + +function test.setPathGoal() + -- catch regression of issue #5978, setPathGoal() is missing CHECK_NULL_POINTER(unit) + expect.error(function() + ---@diagnostic disable-next-line: param-type-mismatch + dfhack.units.setPathGoal(nil, xyz2pos(1, 2, 3), df.unit_path_goal.None) + end, + "dfhack.units.setPathGoal should have thrown an error.") + + local unit + for _,u in ipairs(dfhack.units.getCitizens()) do + if #u.path.path.x > 0 then + unit = u + break + end + end + expect.ne(unit, nil) + + local oldpath = utils.clone(unit.path, true) + + local retval = dfhack.units.setPathGoal(unit, xyz2pos(1, 2, 3), df.unit_path_goal.None) + expect.nil_(retval) + expect.eq(df.unit_path_goal.None, unit.path.goal) + expect.table_eq(xyz2pos(1, 2, 3), utils.clone(unit.path.dest, true)) + expect.table_eq({ x = {}, y = {}, z = {} }, utils.clone(unit.path.path, true)) + + unit.path:assign(oldpath) + expect.eq(oldpath.goal, unit.path.goal) + expect.table_eq(oldpath.dest, utils.clone(unit.path.dest, true)) + expect.table_eq(oldpath.path, utils.clone(unit.path.path, true)) +end