From f71bff3285bc74e319d6258e819873aa79dda5ea Mon Sep 17 00:00:00 2001 From: copperwater Date: Tue, 30 Aug 2022 18:07:07 -0400 Subject: [PATCH] Standardize all core and obj functions with relative coords This is a large iteration on a previous implementation of making nh.getmap() parse its coordinates as relative to the last defined map or room rather than absolute to the entire level. Now, everything in the nh.* and obj.* functions interprets coords as relative rather than absolute. (By default; if no map or room has been defined, or if the lua code is executing after level creation is done, they will interpret the coordinates as absolute). The general motivation is basically the same - routines that use absolute coordinates are difficult to use in level creation routines, because then the designer has to remember to convert the relative coordinate to an absolute one (and that was impossible before nh.abscoord was added, particularly in themed rooms). And once nh.getmap() takes relative coordinates, it would be very strange to have all the other functions (setting timers, burying objects, etc) remain with absolute ones. In a couple places, code is changed to account for coordinates that are relative to a *room* (which uses g.coder->croom->[lx,ly] as an offset, instead of relative to a *map*, which uses [xstart,ystart]. Specifically, selection.iterate did not account for this, and without this the ice themed room timer was not being started in the proper place. All tests are updated to respect the new behavior. Most of the modified functions are not actually used anywhere in level files; the one exception is starting a timer in a themed room, and that has been adjusted. Documentation updated as well to clarify when various things are tossing around relative and absolute coordinates, both in comments and in lua.adoc. --- dat/themerms.lua | 3 +- doc/lua.adoc | 12 ++++++-- include/extern.h | 2 ++ src/nhlobj.c | 7 ++++- src/nhlsel.c | 11 +++++-- src/nhlua.c | 31 +++++++++----------- src/sp_lev.c | 75 +++++++++++++++++++++++++++++++++++++++++------ test/test_des.lua | 62 +++++++++++++++++++-------------------- test/test_sel.lua | 2 +- 9 files changed, 138 insertions(+), 67 deletions(-) diff --git a/dat/themerms.lua b/dat/themerms.lua index 2b6587b5b..4b2aeb05d 100644 --- a/dat/themerms.lua +++ b/dat/themerms.lua @@ -80,8 +80,7 @@ themerooms = { if (percent(25)) then local mintime = 1000 - (nh.level_difficulty() * 100); local ice_melter = function(x,y) - local ax,ay = nh.abscoord(x,y); - nh.start_timer_at(ax,ay, "melt-ice", mintime + nh.rn2(1000)); + nh.start_timer_at(x,y, "melt-ice", mintime + nh.rn2(1000)); end; ice:iterate(ice_melter); end diff --git a/doc/lua.adoc b/doc/lua.adoc index e3f571a7a..e0959480d 100644 --- a/doc/lua.adoc +++ b/doc/lua.adoc @@ -6,13 +6,16 @@ Functions exposed from the NetHack core. They are all in the `nh` table. +All core functions involving xy coordinates interpret these as relative to the +last defined map or room. + === abscoord -Convert a room-relative coordinate to map-absolute. +Convert a room- or map-relative coordinate to absolute. Can accept one table with x and y keys (and in that case, returns similar), or two integer values (and returns two integer values) -des-routines tend to use relative coordinates, nh and obj use absolute. -(This mess is still very much in need of improvement.) +des, nh, and obj routines all treat inputs as relative coordinates, but this is +here in case an absolute one is needed for some reason (debugging?). Example: @@ -385,6 +388,9 @@ Example: Functions for creating special levels. They are in the `des` table. +All special level functions involving xy coordinates interpret these as relative +to the last defined map or room. + === altar Create an altar of certain type and alignment. diff --git a/include/extern.h b/include/extern.h index 6b4156bcd..c1234b848 100644 --- a/include/extern.h +++ b/include/extern.h @@ -2610,6 +2610,8 @@ extern void selection_do_gradient(struct selectionvar *, long, long, long, extern int lspo_reset_level(lua_State *); extern int lspo_finalize_level(lua_State *); extern boolean get_coord(lua_State *, int, lua_Integer *, lua_Integer *); +extern void cvt_to_abscoord(coordxy *, coordxy *); +extern void cvt_to_relcoord(coordxy *, coordxy *); extern int nhl_abs_coord(lua_State *); extern void update_croom(void); extern const char *get_trapname_bytype(int); diff --git a/src/nhlobj.c b/src/nhlobj.c index d9e46e6f0..32c8b02d2 100644 --- a/src/nhlobj.c +++ b/src/nhlobj.c @@ -363,6 +363,8 @@ l_obj_at(lua_State *L) x = (coordxy) luaL_checkinteger(L, 1); y = (coordxy) luaL_checkinteger(L, 2); + cvt_to_abscoord(&x, &y); + lua_pop(L, 2); (void) l_obj_push(L, g.level.objects[x][y]); return 1; @@ -386,6 +388,8 @@ l_obj_placeobj(lua_State *L) x = (coordxy) luaL_checkinteger(L, 2); y = (coordxy) luaL_checkinteger(L, 3); + cvt_to_abscoord(&x, &y); + lua_pop(L, 3); if (lobj_is_ok(lo)) { @@ -569,6 +573,7 @@ l_obj_bury(lua_State *L) } else if (argc == 3) { x = (coordxy) lua_tointeger(L, 2); y = (coordxy) lua_tointeger(L, 3); + cvt_to_abscoord(&x, &y); } else nhl_error(L, "l_obj_bury: Wrong args"); @@ -617,7 +622,7 @@ l_obj_register(lua_State *L) luaL_setfuncs(L, l_obj_meta, 0); /* metatable.__index points at the object method table. */ lua_pushvalue(L, -2); - lua_setfield(L, -2, "__index"); + lua_setfield(L, -2, "__index"); /* Don't let lua code mess with the real metatable. Instead offer a fake one that only contains __gc. */ diff --git a/src/nhlsel.c b/src/nhlsel.c index 3f766abc9..085ac5996 100644 --- a/src/nhlsel.c +++ b/src/nhlsel.c @@ -839,7 +839,10 @@ l_selection_gradient(lua_State *L) return 1; } -/* sel:iterate(function(x,y) ... end); */ +/* sel:iterate(function(x,y) ... end); + * The x, y coordinates passed to the function are map- or room-relative + * rather than absolute, unless there has been no previous map or room defined. + */ static int l_selection_iterate(lua_State *L) { @@ -854,9 +857,11 @@ l_selection_iterate(lua_State *L) for (y = rect.ly; y <= rect.hy; y++) for (x = max(1,rect.lx); x <= rect.hx; x++) if (selection_getpoint(x, y, sel)) { + coordxy tmpx = x, tmpy = y; + cvt_to_relcoord(&tmpx, &tmpy); lua_pushvalue(L, 2); - lua_pushinteger(L, x - g.xstart); - lua_pushinteger(L, y - g.ystart); + lua_pushinteger(L, tmpx); + lua_pushinteger(L, tmpy); if (nhl_pcall(L, 2, 0)) { impossible("Lua error: %s", lua_tostring(L, -1)); /* abort the loops to prevent possible error cascade */ diff --git a/src/nhlua.c b/src/nhlua.c index dafee5753..2b74fd84c 100644 --- a/src/nhlua.c +++ b/src/nhlua.c @@ -172,7 +172,7 @@ void nhl_error(lua_State *L, const char *msg) { lua_Debug ar; - char buf[BUFSZ]; + char buf[BUFSZ*2]; lua_getstack(L, 1, &ar); lua_getinfo(L, "lS", &ar); @@ -389,6 +389,7 @@ nhl_gettrap(lua_State *L) } x = (coordxy) lx; y = (coordxy) ly; + cvt_to_abscoord(&x, &y); if (isok(x, y)) { struct trap *ttmp = t_at(x,y); @@ -439,7 +440,8 @@ nhl_deltrap(lua_State *L) } x = (coordxy) lx; y = (coordxy) ly; - + cvt_to_abscoord(&x, &y); + if (isok(x, y)) { struct trap *ttmp = t_at(x,y); @@ -452,6 +454,11 @@ nhl_deltrap(lua_State *L) /* get parameters (XX,YY) or ({ x = XX, y = YY }) or ({ XX, YY }), and set the x and y values. return TRUE if there are such params in the stack. + Note that this does not adjust the values of x and y at all from what is + specified in the level file; so, it returns absolute coordinates rather than + map-relative coordinates. Callers of this function must decide if they want + to interpret the values as absolute or as map-relative, and adjust + accordingly. */ boolean nhl_get_xy_params(lua_State *L, lua_Integer *x, lua_Integer *y) @@ -487,23 +494,9 @@ nhl_getmap(lua_State *L) nhl_error(L, "Incorrect arguments"); return 0; } - if (g.in_mklev) { - /* xstart and ystart are set by the des.map() command to give - * coordinates relative to the 0,0 of that map. It can be quite useful - * to check what terrain is on a given space. But without compensating - * for the change in xstart and ystart here, this will lead to results - * like des.terrain(4,6,'L') and then nh.getmap(4,6) not being lava. - * - * Only valid during mklev, because xstart and ystart are not saved - * with the level and can change during the level creating process when - * additional des.map() are executed. They will not necessarily be the - * same later. */ - x += g.xstart; - y += g.ystart; - } - x = (coordxy) lx; y = (coordxy) ly; + cvt_to_abscoord(&x, &y); if (isok(x, y)) { char buf[BUFSZ]; @@ -1177,6 +1170,7 @@ nhl_timer_has_at(lua_State *L) x = (coordxy) lx; y = (coordxy) ly; + cvt_to_abscoord(&x, &y); if (isok(x, y)) { when = spot_time_expires(x, y, timertype); @@ -1205,6 +1199,7 @@ nhl_timer_peek_at(lua_State *L) x = (coordxy) lx; y = (coordxy) ly; + cvt_to_abscoord(&x, &y); if (timer_is_pos(timertype) && isok(x, y)) when = spot_time_expires(x, y, timertype); @@ -1230,6 +1225,7 @@ nhl_timer_stop_at(lua_State *L) x = (coordxy) lx; y = (coordxy) ly; + cvt_to_abscoord(&x, &y); if (timer_is_pos(timertype) && isok(x, y)) spot_stop_timers(x, y, timertype); @@ -1254,6 +1250,7 @@ nhl_timer_start_at(lua_State *L) x = (coordxy) lx; y = (coordxy) ly; + cvt_to_abscoord(&x, &y); if (timer_is_pos(timertype) && isok(x, y)) { long where = ((long) x << 16) | (long) y; diff --git a/src/sp_lev.c b/src/sp_lev.c index dfa3feaae..60e5ee8ca 100644 --- a/src/sp_lev.c +++ b/src/sp_lev.c @@ -1150,11 +1150,15 @@ rndtrap(void) } /* - * Coordinates in special level files are handled specially: + * Translate a given coordinate from a special level definition into an actual + * location on the map. * - * if x or y is < 0, we generate a random coordinate. - * The "humidity" flag is used to ensure that engravings aren't - * created underwater, or eels on dry land. + * If x or y is negative, we generate a random coordinate within the area. If + * not negative, they are interpreted as relative to the last defined map or + * room, and are output as absolute g.level.locations coordinates. + * + * The "humidity" flag is used to ensure that engravings aren't created + * underwater, or eels on dry land. */ static void get_location( @@ -3085,6 +3089,12 @@ get_table_montype(lua_State *L, int *mgender) return ret; } +/* Get x and y values from a table (which the caller has already checked for the + * existence of), handling both a table with x= and y= specified and a table + * with coord= specified. + * Returns absolute rather than map-relative coordinates; the caller of this + * function must decide if it wants to interpret the coordinates as map-relative + * and adjust accordingly. */ static void get_table_xy_or_coord(lua_State *L, lua_Integer *x, lua_Integer *y) { @@ -5237,7 +5247,56 @@ l_table_getset_feature_flag( } } -/* convert relative coordinate to map-absolute. +/* guts of nhl_abs_coord; convert a coordinate relative to a map or room into an + * absolute coordinate in g.level.locations. + * + * If there is no enclosing map or room, the coordinates are assumed to be + * absolute already. + * + * Part of the reason this is a function is to make it clearer in the calling + * code that this conversion is what is intended. + * + * NOTE: if the coordinates are going to get passed to one of the get_location + * family of functions, this should NOT be called; get_location already makes + * an adjustment like this. (What this function supports which get_location + * doesn't is the input coordinates being negative. get_location will treat that + * as "level designer wants a random coordinate".) */ +void +cvt_to_abscoord(coordxy *x, coordxy *y) +{ + /* since commit 99715e0, xstart and ystart are only relevant in mklev when + * maps are being used, and 0 otherwise. It is possible in the future that + * map positions and dimensions can be saved and retrieved outside of mklev + * which would reintroduce nonzero xstart/ystart/xsiz/ysiz, but this is not + * currently implemented, so this function can be assumed to have no effect + * outside of mklev. + */ + if (g.coder && g.coder->croom) { + *x += g.coder->croom->lx; + *y += g.coder->croom->ly; + } + else { + *x += g.xstart; + *y += g.ystart; + } +} + +/* inverse of cvt_to_abscoord; turn an absolute g.level.locations coordinate + * into one relative to the current map or room. */ +void +cvt_to_relcoord(coordxy *x, coordxy *y) +{ + if (g.coder && g.coder->croom) { + *x -= g.coder->croom->lx; + *y -= g.coder->croom->ly; + } + else { + *x -= g.xstart; + *y -= g.ystart; + } +} + +/* convert map-relative coordinate to absolute. local ax,ay = nh.abscoord(rx, ry); local pt = nh.abscoord({ x = 10, y = 5 }); */ @@ -5250,16 +5309,14 @@ nhl_abs_coord(lua_State *L) if (argc == 2) { x = (coordxy) lua_tointeger(L, 1); y = (coordxy) lua_tointeger(L, 2); - x += g.xstart; - y += g.ystart; + cvt_to_abscoord(&x, &y); lua_pushinteger(L, x); lua_pushinteger(L, y); return 2; } else if (argc == 1 && lua_type(L, 1) == LUA_TTABLE) { x = (coordxy) get_table_int(L, "x"); y = (coordxy) get_table_int(L, "y"); - x += g.xstart; - y += g.ystart; + cvt_to_abscoord(&x, &y); lua_newtable(L); nhl_add_table_entry_int(L, "x", x); nhl_add_table_entry_int(L, "y", y); diff --git a/test/test_des.lua b/test/test_des.lua index d0163f2c5..afd7fcb4d 100644 --- a/test/test_des.lua +++ b/test/test_des.lua @@ -3,7 +3,7 @@ -- reset_level is only needed here, not in normal special level scripts. function is_map_at(x,y, mapch, lit) - local rm = nh.getmap(x + 1, y); -- + 1 == g.xstart + local rm = nh.getmap(x, y); if rm.mapchr ~= mapch then error("Terrain at (" .. x .. "," .. y .. ") is not \"" .. mapch .. "\", but \"" .. rm.mapchr .. "\""); end @@ -29,7 +29,7 @@ function check_loc_flag(x, y, flag, value) end function check_trap_at(x,y, name) - local t = nh.gettrap(x + 1, y); -- + 1 == g.xstart + local t = nh.gettrap(x, y); if (t.ttyp_name ~= name) then error("Trap at " .. x .. "," .. y .. " is " .. t.ttyp_name .. ", not " .. name); end @@ -211,10 +211,10 @@ LTL]]) FFF F.F FFF]] }) - for x = 60, 62 do - for y = 5, 7 do + for x = 0,2 do + for y = 0,2 do local nam = "iron bars"; - if (x == 61 and y == 6) then + if (x == 1 and y == 1) then nam = "room"; end check_loc_name(x, y, nam); @@ -224,10 +224,10 @@ FFF]] }) ... .T. ...]] }) - for x = 60, 62 do - for y = 5, 7 do + for x = 0, 2 do + for y = 0, 2 do local nam = "room"; - if (x == 61 and y == 6) then + if (x == 1 and y == 1) then nam = "tree"; end check_loc_name(x, y, nam); @@ -244,50 +244,50 @@ III]] }) des.terrain(0,0, "L"); des.terrain(map.width-1,map.height-1, "T"); end}); - check_loc_name(30, 5, "lava pool"); - check_loc_name(32, 7, "tree"); + check_loc_name(0, 0, "lava pool"); + check_loc_name(2, 2, "tree"); end function test_feature() des.reset_level(); des.level_init({ style = "solidfill", fg = ".", lit = 1 }); des.feature("fountain", 40, 08); - check_loc_name(40 + 1, 08, "fountain"); + check_loc_name(40, 08, "fountain"); des.feature("sink", {41, 08}); - check_loc_name(41 + 1, 08, "sink"); + check_loc_name(41, 08, "sink"); des.feature({ type = "pool", x = 42, y = 08 }); - check_loc_name(42 + 1, 08, "pool"); + check_loc_name(42, 08, "pool"); des.feature({ type = "sink", coord = {43, 08} }); - check_loc_name(43 + 1, 08, "sink"); + check_loc_name(43, 08, "sink"); des.feature({ type = "throne", coord = {44, 08}, looted=true }); - check_loc_name(44 + 1, 08, "throne"); - check_loc_flag(44 + 1, 08, "looted", true); + check_loc_name(44, 08, "throne"); + check_loc_flag(44, 08, "looted", true); des.feature({ type = "throne", coord = {44, 08}, looted=false }); - check_loc_name(44 + 1, 08, "throne"); - check_loc_flag(44 + 1, 08, "looted", false); + check_loc_name(44, 08, "throne"); + check_loc_flag(44, 08, "looted", false); des.feature({ type = "tree", coord = {45, 08}, looted=true, swarm=false }); - check_loc_name(45 + 1, 08, "tree"); - check_loc_flag(45 + 1, 08, "looted", true); - check_loc_flag(45 + 1, 08, "swarm", false); + check_loc_name(45, 08, "tree"); + check_loc_flag(45, 08, "looted", true); + check_loc_flag(45, 08, "swarm", false); des.feature({ type = "tree", coord = {45, 08}, looted=false, swarm=true }); - check_loc_name(45 + 1, 08, "tree"); - check_loc_flag(45 + 1, 08, "looted", false); - check_loc_flag(45 + 1, 08, "swarm", true); + check_loc_name(45, 08, "tree"); + check_loc_flag(45, 08, "looted", false); + check_loc_flag(45, 08, "swarm", true); des.feature({ type = "fountain", coord = {46, 08}, looted=false, warned=true }); - check_loc_name(46 + 1, 08, "fountain"); - check_loc_flag(46 + 1, 08, "looted", false); - check_loc_flag(46 + 1, 08, "warned", true); + check_loc_name(46, 08, "fountain"); + check_loc_flag(46, 08, "looted", false); + check_loc_flag(46, 08, "warned", true); des.feature({ type = "sink", coord = {47, 08}, pudding=false, dishwasher=true, ring=true }); - check_loc_name(47 + 1, 08, "sink"); - check_loc_flag(47 + 1, 08, "pudding", false); - check_loc_flag(47 + 1, 08, "dishwasher", true); - check_loc_flag(47 + 1, 08, "ring", true); + check_loc_name(47, 08, "sink"); + check_loc_flag(47, 08, "pudding", false); + check_loc_flag(47, 08, "dishwasher", true); + check_loc_flag(47, 08, "ring", true); end function test_gold() diff --git a/test/test_sel.lua b/test/test_sel.lua index 72c40896c..72c450456 100644 --- a/test/test_sel.lua +++ b/test/test_sel.lua @@ -40,7 +40,7 @@ function sel_are_equal(sela, selb, msg) end function is_map_at(x,y, mapch, lit) - local rm = nh.getmap(x + 1, y); -- + 1 == g.xstart + local rm = nh.getmap(x, y); if rm.mapchr ~= mapch then error("Terrain at (" .. x .. "," .. y .. ") is not \"" .. mapch .. "\", but \"" .. rm.mapchr .. "\""); end