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 issue 645: prevent pistons from pushing blocks into their current position, deleting them. #659

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

fix issue 645: prevent pistons from pushing blocks into their current position, deleting them. #659

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e7d11b3
fix issue 645
lolbinarycat Dec 16, 2023
c9f3915
fix crash caused by missing nil check
lolbinarycat Dec 16, 2023
5d58293
rename variable to satisfy linter
lolbinarycat Dec 19, 2023
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
9 changes: 9 additions & 0 deletions mesecons_pistons/init.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,16 @@ local piston_on = function(pos, node)
local pistonspec = get_pistonspec(node.name, "offname")
local dir = vector.multiply(minetest.facedir_to_dir(node.param2), -1)
local pusher_pos = vector.add(pos, dir)
local behind_pos = vector.subtract(pos, dir)
local meta = minetest.get_meta(pos)
-- NOTE: this gets calculated twice, but fixing this would mean changing the public api.
local stack_preview = mesecon.mvps_get_stack(pusher_pos, dir, max_push, meta:get_string("owner"))
for _, n in ipairs(stack_preview or {}) do
-- fix issue 645
if vector.equals(n.pos, behind_pos) then
return
end
end
local success, stack, oldstack = mesecon.mvps_push(pusher_pos, dir, max_push, meta:get_string("owner"))
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't mvps_push return success = false when the node cannot be pushed? That would seem to be the logical behaviour.

Copy link
Contributor

Choose a reason for hiding this comment

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

But a piston can be pushed. It just shouldn’t be able to push itself. Unlike a movestone for which that shouldn’t be a problem.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

my first thought was swapping the piston for the immoveable extended version before calling mvps_push (then swap it back if it was unsuccessful), which would return false and also wouldn't have any significant performance overhead.

unfortunately, a sticky block on the face of the piston will grab said piston and try to pull it along, even if it is immoveable.

this is different from the behavior or minecraft slime blocks, which have no problems pulling away from immoveable blocks.

however, the purpose of this commit is to fix a bug, not to change behavior, so i opted for this instead, which doesn't change any public apis, ensuring it doesn't break other mods.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Shouldn't mvps_push return success = false when the node cannot be pushed? That would seem to be the logical behaviour.

the biggest reason i don't implement the new logic in mvps_push is because pistons and moveblocks have fundimentally different behavior:

  • pistons get bigger when pushing things
  • moveblocks stay the same size

one option would be to add an is_piston or piston_pos parameter to mvps_push, and somehow handle the logic there, but once again, i didn't want to amend the public api on my first contribution.

the simplest change to fix the performance loss would be to add an optional nodes parameter to mvps_push, then add nodes = nodes or mesecons.mvps_get_stack(<...>).

i can implement any of these changes if desired.

if not success then
if stack == "protected" then
Expand Down