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.

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

don't remove node above door if it's not a doors:hidden node #3045

Merged
merged 2 commits into from
Aug 11, 2023
Merged
Changes from 1 commit
Commits
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
25 changes: 12 additions & 13 deletions mods/doors/init.lua
Original file line number Diff line number Diff line change
Expand Up @@ -392,10 +392,11 @@ function doors.register(name, def)
end
end
def.after_dig_node = function(pos, node, meta, digger)
if is_doors_upper_node(pos) then
minetest.remove_node({x = pos.x, y = pos.y + 1, z = pos.z})
local above = pos:offset(0, 1, 0)
if is_doors_upper_node(above) then
minetest.remove_node(above)
end
minetest.check_for_falling({x = pos.x, y = pos.y + 1, z = pos.z})
minetest.check_for_falling(above)
end
def.on_rotate = function(pos, node, user, mode, new_param2)
return false
Expand Down Expand Up @@ -432,20 +433,20 @@ function doors.register(name, def)
def.node_dig_prediction = ""
else
def.on_blast = function(pos, intensity)
if is_doors_upper_node(pos) then
minetest.remove_node(pos)
end
minetest.remove_node(pos)
local above = pos:offset(0, 1, 0)
-- hidden node doesn't get blasted away.
if is_doors_upper_node(pos) then
minetest.remove_node({x = pos.x, y = pos.y + 1, z = pos.z})
if is_doors_upper_node(above) then
minetest.remove_node(above)
end
return {name}
end
end

def.on_destruct = function(pos)
if is_doors_upper_node(pos) then
minetest.remove_node({x = pos.x, y = pos.y + 1, z = pos.z})
local above = pos:offset(0, 1, 0)
if is_doors_upper_node(above) then
minetest.remove_node(above)
end
end

Expand Down Expand Up @@ -654,9 +655,7 @@ function doors.register_trapdoor(name, def)
def.node_dig_prediction = ""
else
def.on_blast = function(pos, intensity)
if is_doors_upper_node(pos) then
minetest.remove_node(pos)
end
minetest.remove_node(pos)
return {name}
Copy link
Contributor

Choose a reason for hiding this comment

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

Should both of these return {name}? I believe the return {name} here should be removed, otherwise we get varying drops based on a race condition:

  • If the top is blasted first, we get 2 doors.
  • If the bottom is blasted first, the top is removed, and we only get one door.

Also: Am I missing something, or could you duplicate doors by blowing them up with TNT (before this patch)?

And in general, concerning the logic: Shouldn't both nodes remove each other to avoid inconsistent states? A "bottom without hidden top" shouldn't happen either, should it?

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'm deliberately not looking into whether return {name} is correct right now, or what the deal is w/ tnt. i'll try to look at it soon though. perhaps that should be a separate issue.

the door and "hidden" node can currently be separated by e.g. a mesecons piston. i really doubt anyone wants the doors mod to depend (optionally) on mesecons, and other mods could introduce that same kind of mechanic in the future, so that wouldn't even solve the problem in general. i suppose mesecons could automatically register all doors as MVPS stoppers? e.g. iterate doors.registered_doors and also override doors.register.

end
end
Expand Down
Loading