Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add library support to Luacontrollers #557

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Add library support to Luacontrollers #557

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9904be9
Add library support to Luacontrollers
Mar 21, 2021
91e3b13
Luacontroller library enhancements
Mar 26, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions mesecons/util.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,25 @@ function mesecon.tablecopy(obj) -- deep copy
return obj
end

-- Performs a deep copy of a table, changing the environment of any functions.
-- Adapted from the builtin table.copy() function.
function mesecon.tablecopy_change_env(t, env, seen)
local n = {}
seen = seen or {}
seen[t] = n
for k, v in pairs(t) do
if type(v) == "function" then
local newfunc = v
setfenv(newfunc, env)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the new variable? newfunc is still the same function as v.
You are setting the env of v. This might be useful for functions that use functions from the luacontroller env. But in general I think this is at least rather inconvenient. If you want to make the whole minetest table accessible, like you suggested in the title comment, you will have to wrap nearly every function because otherwise it will not work, or crash because it's not a lua function.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose I misunderstood the function passing behavior in Lua here - after reading up on that some, I see I could clean this up a little. But the setfenv() is still needed (I can just do it on v) in order for the main purpose of these libraries - interacting with LuaC capabilities - to work. Shoving the entire "minetest" table in there was more of just sort of a wild suggestion and not really an intended use case.

n[(type(k) == "table" and (seen[k] or mesecon.tablecopy_change_env(k, env, seen))) or k] = newfunc
else
n[(type(k) == "table" and (seen[k] or mesecon.tablecopy_change_env(k, env, seen))) or k] =
(type(v) == "table" and (seen[v] or mesecon.tablecopy_change_env(v, env, seen))) or v
end
end
return n
end

-- Returns whether two values are equal.
-- In tables, keys are compared for identity but values are compared recursively.
-- There is no protection from infinite recursion.
Expand Down
12 changes: 12 additions & 0 deletions mesecons_luacontroller/init.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -459,6 +459,16 @@ local function get_digiline_send(pos, itbl, send_warning)
end
end

mesecon.luacontroller_libraries = {}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to have this mentioned somewhere, eg. in the code comment at the top of the file.


local function get_require(env)
return function(name)
if mesecon.luacontroller_libraries[name] then
return mesecon.tablecopy_change_env(mesecon.luacontroller_libraries[name],env)
end
end
end
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could make this more general by having library getter functions in mesecon.luacontroller_libraries:

Suggested change
local function get_require(env)
return function(name)
if mesecon.luacontroller_libraries[name] then
return mesecon.tablecopy_change_env(mesecon.luacontroller_libraries[name],env)
end
end
end
local function get_require(env, pos)
return function(name)
if mesecon.luacontroller_libraries[name] then
return mesecon.luacontroller_libraries[name](env, vector.new(pos))
end
end
end

Note that I've also added pos here. I think there was somewhen the request to be able to add some sort upgrades to the luacontroller, like a sensor node that is placed beneath the luacontroller, having the pos allows this.

Such a getter function could still change the env of their functions if it wants to. And it has to ensure there are no security flaws, but that's the case anyway, a simpler api without the deepcopy-with-env-change might make this even easier.


local safe_globals = {
-- Don't add pcall/xpcall unless willing to deal with the consequences (unless very careful, incredibly likely to allow killing server indirectly)
"assert", "error", "ipairs", "next", "pairs", "select",
Expand Down Expand Up @@ -546,6 +556,8 @@ local function create_environment(pos, mem, event, itbl, send_warning)
for _, name in pairs(safe_globals) do
env[name] = _G[name]
end

env.require = get_require(env)

return env
end
Expand Down