diff --git a/docs/changelog.txt b/docs/changelog.txt index 8b4ab870bb..bec963b9bc 100644 --- a/docs/changelog.txt +++ b/docs/changelog.txt @@ -121,6 +121,7 @@ Template for new versions: ## Lua - Added ``dfhack.maps.forEachTile`` to scan a cuboid of tiles with a declarative filter and apply actions (count, set tiletype or per-tiletype replacements, set designation/occupancy fields, spawn constructions, Lua callback) in a single native call +- Fixed ``ipairs()`` on enum attribute tables (e.g. ``df.item_type.attrs``) never terminating (issue #1860) ## Removed diff --git a/library/LuaWrapper.cpp b/library/LuaWrapper.cpp index df81ea808e..ed0f8f5706 100644 --- a/library/LuaWrapper.cpp +++ b/library/LuaWrapper.cpp @@ -1055,20 +1055,12 @@ static int meta_ptr_tostring(lua_State *state) } /** - * Metamethod: __index for enum.attrs + * Push the attr entry for the given enum item value. + * Out-of-range values resolve to the default entry. */ -static int meta_enum_attr_index(lua_State *state) +static void push_enum_attr(lua_State *state, enum_identity *id, int64_t idx) { - if (!lua_isnumber(state, 2)) - lua_rawget(state, UPVAL_FIELDTABLE); - if (!lua_isnumber(state, 2)) - luaL_error(state, "Invalid index in enum.attrs[]"); - - auto id = (enum_identity*)lua_touserdata(state, lua_upvalueindex(2)); - auto *complex = id->getComplex(); - - int64_t idx = lua_tonumber(state, 2); - if (complex) + if (auto *complex = id->getComplex()) { auto it = complex->value_index_map.find(idx); if (it != complex->value_index_map.end()) @@ -1087,6 +1079,20 @@ static int meta_enum_attr_index(lua_State *state) auto atype = id->getAttrType(); push_object_internal(state, atype, ptr + unsigned(atype->byte_size()*idx)); +} + +/** + * Metamethod: __index for enum.attrs + */ +static int meta_enum_attr_index(lua_State *state) +{ + if (!lua_isnumber(state, 2)) + lua_rawget(state, UPVAL_FIELDTABLE); + if (!lua_isnumber(state, 2)) + luaL_error(state, "Invalid index in enum.attrs[]"); + + auto id = (enum_identity*)lua_touserdata(state, lua_upvalueindex(2)); + push_enum_attr(state, id, lua_tonumber(state, 2)); return 1; } @@ -1462,6 +1468,83 @@ static int complex_enum_ipairs(lua_State *L) return 3; } +/* + * enum.attrs ipairs() support + * + * enum.attrs[] returns a default entry for out-of-range indexes, so a plain + * ipairs() would never terminate. These iterators stop at the same place as + * ipairs() on the enum itself (DFHack/dfhack#1860). + * + * upvalues for the *_ipairs functions: + * 1: enum_identity + * 2: enum_identity::ComplexData (complex enums only) + * + * upvalues for the *_inext iterators (UPVAL_TYPETABLE is required by + * push_object_internal): + * 1: DFHACK_TYPETABLE + * 2: enum_identity (plain) or enum_identity::ComplexData (complex) + * 3: enum_identity (complex only) + */ + +static int meta_enum_attr_inext(lua_State *L) +{ + auto id = (enum_identity*)lua_touserdata(L, lua_upvalueindex(2)); + int64_t i = luaL_checkint(L, 2) + 1; + if (i <= id->getLastItem()) + { + lua_pushinteger(L, i); + push_enum_attr(L, id, i); + return 2; + } + else + { + lua_pushnil(L); + return 1; + } +} + +static int meta_enum_attr_ipairs(lua_State *L) +{ + auto id = (enum_identity*)lua_touserdata(L, lua_upvalueindex(1)); + lua_rawgetp(L, LUA_REGISTRYINDEX, &DFHACK_TYPETABLE_TOKEN); + lua_pushvalue(L, lua_upvalueindex(1)); + lua_pushcclosure(L, meta_enum_attr_inext, 2); + lua_pushnil(L); + lua_pushinteger(L, id->getFirstItem() - 1); + return 3; +} + +static int complex_enum_attr_inext(lua_State *L) +{ + bool is_first = lua_isuserdata(L, 2); + int64_t i = (is_first) + ? ((enum_identity::ComplexData*)lua_touserdata(L, lua_upvalueindex(2)))->index_value_map[0] + : luaL_checkint(L, 2); + if (is_first || complex_enum_next_item_helper(L, i)) + { + auto id = (enum_identity*)lua_touserdata(L, lua_upvalueindex(3)); + lua_pushinteger(L, i); + push_enum_attr(L, id, i); + return 2; + } + else + { + lua_pushnil(L); + return 1; + } +} + +static int complex_enum_attr_ipairs(lua_State *L) +{ + lua_rawgetp(L, LUA_REGISTRYINDEX, &DFHACK_TYPETABLE_TOKEN); + lua_pushvalue(L, lua_upvalueindex(2)); + lua_pushvalue(L, lua_upvalueindex(1)); + lua_pushcclosure(L, complex_enum_attr_inext, 3); + lua_pushnil(L); + lua_pushlightuserdata(L, (void*)1); + return 3; +} + static void RenderTypeChildren(lua_State *state, const std::vector &children); @@ -1562,7 +1645,21 @@ static void FillEnumKeys(lua_State *state, int ix_meta, int ftable, enum_identit lua_pushvalue(state, base+1); lua_pushcclosure(state, meta_enum_attr_index, 3); - freeze_table(state, false, (eid->getFullName()+".attrs").c_str()); + freeze_table(state, true, (eid->getFullName()+".attrs").c_str()); + + lua_pushlightuserdata(state, eid); + if (complex) + { + lua_pushlightuserdata(state, (void*)complex); + lua_pushcclosure(state, complex_enum_attr_ipairs, 2); + } + else + { + lua_pushcclosure(state, meta_enum_attr_ipairs, 1); + } + lua_setfield(state, -2, "__ipairs"); + lua_pop(state, 1); + lua_setfield(state, ftable, "attrs"); } diff --git a/test/structures/enum_attrs.lua b/test/structures/enum_attrs.lua new file mode 100644 index 0000000000..b2df75f14f --- /dev/null +++ b/test/structures/enum_attrs.lua @@ -0,0 +1,54 @@ +config.target = 'core' + +-- https://github.com/DFHack/dfhack/issues/1860 +-- enum.attrs indexes never return nil (out-of-range indexes return a default +-- entry), so ipairs() on an attrs table needs a bounded __ipairs that stops +-- where ipairs() on the enum itself stops + +local function enum_keys(enum) + local keys = {} + for i in ipairs(enum) do + table.insert(keys, i) + end + return keys +end + +-- iterate attrs with a hard cap so a regression fails instead of hanging +local function attrs_keys(enum, limit) + local keys, values = {}, {} + for i, attrs in ipairs(enum.attrs) do + table.insert(keys, i) + values[i] = attrs + if #keys > limit then + return keys, values, false + end + end + return keys, values, true +end + +local function check_attrs_ipairs(enum, name) + local expected = enum_keys(enum) + local keys, values, terminated = attrs_keys(enum, #expected + 1) + expect.true_(terminated, 'ipairs(' .. name .. '.attrs) did not terminate') + expect.table_eq(expected, keys) + for _, i in ipairs(keys) do + expect.eq(values[i], enum.attrs[i], + 'wrong attrs entry yielded for ' .. name .. '[' .. i .. ']') + end +end + +function test.plain_enum() + check_attrs_ipairs(df.item_type, 'df.item_type') +end + +function test.complex_enum() + expect.true_(df.pronoun_type._complex) + check_attrs_ipairs(df.pronoun_type, 'df.pronoun_type') +end + +function test.out_of_range_index() + -- invalid indexes must keep returning the default entry + expect.true_(df.item_type.attrs[df.item_type._last_item + 1]) + expect.eq(df.item_type.attrs[df.item_type._last_item + 1], + df.item_type.attrs[df.item_type._last_item + 100]) +end