gui/settings-manager: recompute unit labors after work detail import

changelog.txt

@@ -34,6 +34,8 @@ Template for new versions:
 
 ## Fixes
 - `bodyswap`: fix "invalid argument count" when the target unit has no nemesis record
+- `gui/settings-manager`: preserve built-in work details added after saved settings were created
+- `gui/settings-manager`: apply imported work details to renamed built-ins and recompute unit labors so imported settings take effect immediately
 - `fix/loyaltycascade`: guard against citizens that are not historical figures and emit a warning.
 - `gui/siegemanager`: fix nil index if there are no siege engines on the map
 

docs/gui/settings-manager.rst

@@ -50,8 +50,4 @@ Autostart page.
 There is a similar panel on the Labor -> Work Details page that allows for
 saving and restoring of work detail definitions. Be aware that work detail
 assignments to units cannot be saved, so you have to assign the work details to
-individual units after you restore the definitions. Another caveat is that DF
-doesn't evaluate work detail definitions until a change (any change) is made on
-the work details screen. Therefore, after importing work detail definitions,
-including auto-loading them for new embarks, you have to go to the work details
-page and make a change before your imported work details will take effect.
+individual units after you restore the definitions.

gui/settings-manager.lua

@@ -46,6 +46,8 @@ local function save_difficulty(df_difficulty)
 end
 
 local function load_difficulty(df_difficulty)
+    -- re-read in case the file changed since the config was first opened
+    config:read()
     local difficulty = utils.clone(config.data.difficulty or {}, true)
     for _, v in pairs(difficulty) do
         if type(v) == 'table' and v[1] then
@@ -398,6 +400,8 @@ local function save_standing_orders()
 end
 
 local function load_standing_orders()
+    -- re-read in case the file changed since the config was first opened
+    config:read()
     for name, val in pairs(config.data.standing_orders or {}) do
         df.global[name] = val
     end
@@ -516,26 +520,81 @@ local function save_work_details()
     config:write()
 end
 
+local function apply_work_detail(detail, wd)
+    local flags = wd.flags or wd.work_detail_flags or {} -- compat for old name
+    if wd.name then detail.name = wd.name end
+    if wd.icon then detail.icon = wd.icon end
+    detail.flags.cannot_be_everybody = flags.cannot_be_everybody
+    detail.flags.no_modify = flags.no_modify
+    detail.flags.mode = flags.mode
+    for i,v in ipairs(wd.allowed_labors or {}) do
+        detail.allowed_labors[i-1] = v
+    end
+end
+
+-- built-in work details are identified by their unique icon; custom details
+-- get CUSTOM_* icons (or NONE), so a built-in icon can only belong to a
+-- built-in
+local function is_builtin_icon(icon)
+    return type(icon) == 'number' and icon >= 0 and
+        (icon < df.work_detail_icon_type.CUSTOM_1 or
+         icon == df.work_detail_icon_type.SIEGE_OPERATORS)
+end
+
 local function load_work_details()
+    -- re-read in case the file changed since the config was first opened
+    config:read()
     if not config.data.work_details or #config.data.work_details < 10 then
         -- not enough data to cover built-in work details
         return
     end
-    li.work_details:resize(#config.data.work_details)
-    -- keep unit assignments for overwritten indices
-    for idx, wd in ipairs(config.data.work_details) do
-        local detail = {
+
+    local saved_builtins, builtin_by_icon, saved_custom = {}, {}, {}
+    for _,wd in ipairs(config.data.work_details) do
+        local flags = type(wd) == 'table' and
+            (wd.flags or wd.work_detail_flags) or nil
+        if flags and flags.no_modify then
+            saved_builtins[('%s\0%s'):format(wd.icon, wd.name)] = wd
+            if is_builtin_icon(wd.icon) then
+                -- built-ins may have been renamed by the user or by a DF
+                -- version update, so fall back to matching by icon
+                builtin_by_icon[wd.icon] = wd
+            end
+        elseif flags then
+            table.insert(saved_custom, wd)
+        end
+        -- entries without flag data are malformed; skip them
+    end
+
+    local builtin_count = 0
+    for idx = 0, #li.work_details - 1 do
+        local detail = li.work_details[idx]
+        if not detail.flags.no_modify then break end
+        builtin_count = builtin_count + 1
+        local wd = saved_builtins[('%s\0%s'):format(detail.icon, detail.name)] or
+            builtin_by_icon[detail.icon]
+        if wd then apply_work_detail(detail, wd) end
+    end
+
+    li.work_details:resize(builtin_count + #saved_custom)
+    for idx,wd in ipairs(saved_custom) do
+        local detail_idx = builtin_count + idx - 1
+        li.work_details[detail_idx] = {
             new=df.work_detail,
             name=wd.name,
             icon=wd.icon,
-            flags=wd.flags or wd.work_detail_flags, -- compat for old name
+            flags=wd.flags or wd.work_detail_flags,
         }
-        li.work_details[idx-1] = detail
-        local al = li.work_details[idx-1].allowed_labors
-        for i,v in ipairs(wd.allowed_labors) do
+        local al = li.work_details[detail_idx].allowed_labors
+        for i,v in ipairs(wd.allowed_labors or {}) do
             al[i-1] = v
         end
     end
+    -- applying work details through the UI recomputes each unit's effective
+    -- labors; do the same here so the imported details take effect
+    for _,unit in ipairs(dfhack.units.getCitizens()) do
+        dfhack.units.setAutomaticProfessions(unit)
+    end
     local scr = dfhack.gui.getDFViewscreen(true)
     if dfhack.gui.matchFocusString('dwarfmode/Info/LABOR/WORK_DETAILS', scr) then
         gui.simulateInput(scr, 'LEAVESCREEN')

test/gui/settings-manager.lua

@@ -0,0 +1,214 @@
+config = {
+    mode = 'fortress',
+    target = 'gui/settings-manager',
+}
+
+local settings_manager = reqscript('gui/settings-manager')
+
+local function make_vector(entries)
+    local data = entries or {}
+    return setmetatable({}, {
+        __len=function() return #data end,
+        __index=function(_, key)
+            if key == 'resize' then
+                return function(_, size)
+                    for i = #data, size - 1 do data[i+1] = false end
+                    for i = #data, size + 1, -1 do data[i] = nil end
+                end
+            end
+            return type(key) == 'number' and data[key+1] or nil
+        end,
+        __newindex=function(_, key, value)
+            if type(key) ~= 'number' then return end
+            if type(value) == 'table' and value.new == df.work_detail then
+                value = {
+                    name=value.name,
+                    icon=value.icon,
+                    flags=value.flags,
+                    allowed_labors={},
+                }
+            end
+            data[key+1] = value
+        end,
+    })
+end
+
+local function with_work_details(work_details, saved, fn)
+    local load_fn = settings_manager.WorkDetailsOverlay.ATTRS.load_fn
+    local li_idx, old_li
+    for i = 1, 10 do
+        local name, value = debug.getupvalue(load_fn, i)
+        if name == 'li' then
+            li_idx, old_li = i, value
+            break
+        end
+    end
+    local old_config = settings_manager.config
+    debug.setupvalue(load_fn, li_idx, {work_details=work_details})
+    settings_manager.config = {
+        data={work_details=saved},
+        read=function() end,
+        write=function() end,
+    }
+    dfhack.with_finalize(
+        function()
+            debug.setupvalue(load_fn, li_idx, old_li)
+            settings_manager.config = old_config
+        end,
+        function() fn(load_fn) end)
+end
+
+local function saved_detail(index)
+    return {
+        name=('built-in %d'):format(index),
+        icon=index,
+        flags={cannot_be_everybody=false, no_modify=true, mode=1},
+        allowed_labors={},
+    }
+end
+
+local function current_details()
+    local current = {}
+    for i = 1, 10 do current[i] = saved_detail(i) end
+    current[11] = {
+        name='Siege Operators',
+        icon=df.work_detail_icon_type.SIEGE_OPERATORS,
+        flags={cannot_be_everybody=false, no_modify=true, mode=1},
+        allowed_labors={},
+    }
+    return current
+end
+
+function test.loading_old_details_preserves_new_builtin()
+    local saved = {}
+    for i = 1, 10 do saved[i] = saved_detail(i) end
+    local work_details = make_vector(current_details())
+
+    with_work_details(work_details, saved, function(load_fn)
+        load_fn()
+        expect.eq(11, #work_details)
+        if #work_details < 11 then return end
+        expect.eq('Siege Operators', work_details[10].name)
+    end)
+end
+
+function test.loading_legacy_work_detail_flags()
+    local saved = {}
+    for i = 1, 10 do saved[i] = saved_detail(i) end
+    saved[1].work_detail_flags = saved[1].flags
+    saved[1].flags = nil
+    saved[1].work_detail_flags.mode = 3
+    local work_details = make_vector(current_details())
+
+    with_work_details(work_details, saved, function(load_fn)
+        load_fn()
+        expect.eq(3, work_details[0].flags.mode)
+    end)
+end
+
+function test.loading_renamed_builtin_matches_by_icon()
+    -- a saved built-in whose name was changed (by the user or a DF update)
+    -- is still identified by its unique built-in icon
+    local saved = {}
+    for i = 1, 10 do saved[i] = saved_detail(i) end
+    saved[3] = {
+        name='Old Hunters Name',
+        icon=3,
+        flags={cannot_be_everybody=false, no_modify=true, mode=3},
+        allowed_labors={true, false, true},
+    }
+    local work_details = make_vector(current_details())
+
+    with_work_details(work_details, saved, function(load_fn)
+        load_fn()
+        local detail = work_details[2]
+        expect.eq('Old Hunters Name', detail.name)
+        expect.eq(3, detail.flags.mode)
+        expect.eq(true, detail.allowed_labors[0])
+        expect.eq(true, detail.allowed_labors[2])
+    end)
+end
+
+function test.builtin_icon_fallback_ignores_custom_icons()
+    -- a saved no_modify entry with a custom icon must not steal a built-in
+    local saved = {}
+    for i = 1, 10 do saved[i] = saved_detail(i) end
+    saved[3] = {
+        name='built-in 3',
+        icon=df.work_detail_icon_type.CUSTOM_1,
+        flags={cannot_be_everybody=false, no_modify=true, mode=3},
+        allowed_labors={true},
+    }
+    local work_details = make_vector(current_details())
+
+    with_work_details(work_details, saved, function(load_fn)
+        load_fn()
+        local detail = work_details[2]
+        expect.eq('built-in 3', detail.name)
+        expect.eq(1, detail.flags.mode)
+        expect.ne(true, detail.allowed_labors[0])
+    end)
+end
+
+function test.loading_malformed_entries_are_skipped()
+    local saved = {}
+    for i = 1, 10 do saved[i] = saved_detail(i) end
+    saved[5] = {name='no flags', icon=4} -- no flags/work_detail_flags
+    saved[6] = 'not a table'
+    local work_details = make_vector(current_details())
+
+    with_work_details(work_details, saved, function(load_fn)
+        load_fn()
+        -- the two malformed entries are skipped, not recreated as customs
+        expect.eq(11, #work_details)
+        expect.eq('Siege Operators', work_details[10].name)
+        -- unmatched built-ins are left alone
+        expect.eq('built-in 5', work_details[4].name)
+    end)
+end
+
+function test.loading_recomputes_unit_labors()
+    local saved = {}
+    for i = 1, 10 do saved[i] = saved_detail(i) end
+    local work_details = make_vector(current_details())
+    local citizens = {{id=1}, {id=2}}
+    local recomputed = {}
+    mock.patch({
+        {dfhack.units, 'getCitizens', function() return citizens end},
+        {dfhack.units, 'setAutomaticProfessions', function(unit)
+            recomputed[unit] = true
+        end},
+    }, function()
+        with_work_details(work_details, saved, function(load_fn)
+            load_fn()
+            expect.true_(recomputed[citizens[1]])
+            expect.true_(recomputed[citizens[2]])
+        end)
+    end)
+end
+
+function test.loading_old_custom_details_after_new_builtins()
+    local saved = {}
+    for i = 1, 10 do saved[i] = saved_detail(i) end
+    saved[11] = {
+        name='Custom detail',
+        icon=1,
+        flags={cannot_be_everybody=false, no_modify=false, mode=1},
+        allowed_labors={},
+    }
+    local current = current_details()
+    current[12] = {
+        name='Unsaved custom detail',
+        icon=2,
+        flags={cannot_be_everybody=false, no_modify=false, mode=1},
+        allowed_labors={},
+    }
+    local work_details = make_vector(current)
+
+    with_work_details(work_details, saved, function(load_fn)
+        load_fn()
+        expect.eq(12, #work_details)
+        expect.eq('Siege Operators', work_details[10].name)
+        expect.eq('Custom detail', work_details[11].name)
+    end)
+end