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

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

Conversation

lolbinarycat
Copy link
Contributor

this isn't the most efficient fix, but as far as i can tell, anything better would require changing the public api, and i don't want to break any other mods that may depend on mesecons_mvps.

some things i noticed while fixing this bug:

  • when pushing a sticky block, the piston pushing the block actually gets dragged along behind it. the only reason this isn't visible is because a piston extension places two nodes.
  • an immovable node behind a sticky block will prevent that block from being moved forward.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants