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

Fix conductor aliases again #615

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix conductor aliases again #615

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
26 changes: 21 additions & 5 deletions mesecons/internal.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -372,17 +372,33 @@ end
-- The set of conductor states which require light updates when they change.
local light_update_conductors

-- Calculate the contents of the above set if they have not been calculated.
local function find_light_update_conductors()
-- Prepare all conductor content before the first turnon/turnoff:
-- - Resolve aliases in conductor state lists.
-- - Calculate the contents of light_update_conductors.
local function prepare_conductors()
-- The expensive calculation is only done the first time.
if light_update_conductors then return end

light_update_conductors = {}

-- Find conductors whose lighting characteristics change depending on their state.
local checked = {}
for name, def in pairs(minetest.registered_nodes) do
local conductor = mesecon.get_conductor(name)

-- Resolve aliases in state list.
if conductor then
if conductor.onstate then
conductor.onstate = minetest.registered_aliases[conductor.onstate] or conductor.onstate
elseif conductor.offstate then
conductor.offstate = minetest.registered_aliases[conductor.offstate] or conductor.offstate
else
for i, state in ipairs(conductor.states) do
conductor.states[i] = minetest.registered_aliases[state] or state
end
end
end

-- Find conductors whose lighting characteristics change depending on their state.
if conductor and not checked[name] then
-- Find the other states of the conductor besides the current one.
local other_states
Expand Down Expand Up @@ -420,7 +436,7 @@ end
-- Follow all all conductor paths replacing conductors that were already
-- looked at, activating / changing all effectors along the way.
function mesecon.turnon(pos, link)
find_light_update_conductors()
prepare_conductors()

local frontiers = mesecon.fifo_queue.new()
frontiers:add({pos = pos, link = link})
Expand Down Expand Up @@ -483,7 +499,7 @@ end
-- depth = indicates order in which signals wire fired, higher is later
-- }
function mesecon.turnoff(pos, link)
find_light_update_conductors()
prepare_conductors()

local frontiers = mesecon.fifo_queue.new()
frontiers:add({pos = pos, link = link})
Expand Down
2 changes: 2 additions & 0 deletions mesecons/util.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -403,6 +403,7 @@ function mesecon.vm_get_node(pos)
end

-- Sets a node’s name during a VoxelManipulator-based transaction.
-- Aliases may not work correctly.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
-- Aliases may not work correctly.
-- Aliases may not work correctly if mesecon.get_conductor(name).states was touched by a mod.

That's how I understood the problem, right? If you want to make sure they're not modified, consider metatables. At least the reason why it may not work correct should be stated here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The issue can occur if a mod swaps in a node whose name is actually an alias.

Copy link
Member

Choose a reason for hiding this comment

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

Then please be so nice to mention in which cases this function is expected to work, and when not.

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 will make some changes after #614 is merged or closed. If it is merged, I can make some further changes to the code then remove the notice. This PR is basically blocked for now.

--
-- Existing param1, param2, and metadata are left alone.
--
Expand Down Expand Up @@ -453,6 +454,7 @@ end
-- the server’s main map data cache and then accessed from there.
--
-- Inside a VM transaction, the transaction’s VM cache is used.
-- Aliases may not work correctly.
--
-- This function can only be used to change the node’s name, not its parameters
-- or metadata.
Expand Down