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 some issues with the retract event & sticky pistons #9258

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

Conversation

Machine-Maker
Copy link
Member

Reported by @Lulu13022002 and SPIGOT-2677.

When cancelling the retract event for sticky pistons, the piston head did not stay extended but the "stickyied" blocks were left behind. This doesn't match the normal piston behavior of keeping the head extended.

Root of the issue is that sticky events are called later due to need to collect the blocks being moved.

I didn't find any issues, and observers function correctly, not updating when the retract event is cancelled, but this could break some obscure redstone mechanic I'm not aware of.

@Machine-Maker Machine-Maker requested a review from a team as a code owner June 4, 2023 04:00
Copy link
Contributor

@Lulu13022002 Lulu13022002 left a comment

Choose a reason for hiding this comment

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

See minor comments below but this patch can creates a weird piston when i try to pull two slime blocks on the ground

@Machine-Maker Machine-Maker force-pushed the fix/sticky-piston-retract-event branch from 41ec5a5 to 610db1b Compare June 6, 2023 23:30
@Machine-Maker Machine-Maker force-pushed the fix/sticky-piston-retract-event branch 2 times, most recently from ae9c2c4 to 0c1bf94 Compare December 2, 2023 18:11
@Machine-Maker
Copy link
Member Author

Rebased for 1.20.4. Tested by @Lulu13022002

@Thorinwasher
Copy link

Thorinwasher commented Feb 14, 2024

Found a bug: When the piston does not move a block during retraction, the event PistonRetractEvent will show an opposite facing compared to when it does move a block.

@Krakenied
Copy link

Krakenied commented Mar 18, 2024

Found a bug: When the piston does not move a block during retraction, the event PistonRetractEvent will show an opposite facing compared to when it does move a block.

  • First option: getDirection should return the actual piston moving direction

These two should use to enumdirection.getOpposite() to fix that:
https://github.com/PaperMC/Paper/pull/9258/files#diff-e7381a22d3936796a90f4898d99ec21d6099cc8e49f45267c11841c942feb240R78
https://github.com/PaperMC/Paper/pull/9258/files#diff-e7381a22d3936796a90f4898d99ec21d6099cc8e49f45267c11841c942feb240R108

However I'm not sure if it is not actually the correct behavior. Probably it is:
https://hub.spigotmc.org/javadocs/spigot/org/bukkit/event/block/BlockPistonEvent.html#getDirection()
Return the direction in which the piston will operate.

  • Second option: getDirection should return the piston block face

If it's actually correct, then it requires some changes as well - this one (above the line referenced in the diff - it's in else):
https://github.com/PaperMC/Paper/pull/9258/files#diff-e7381a22d3936796a90f4898d99ec21d6099cc8e49f45267c11841c942feb240R166
ought to be changed to use getOpposite().

@Krakenied
Copy link

2024-03-18.03-52-42.mp4

@Machine-Maker Machine-Maker force-pushed the fix/sticky-piston-retract-event branch 2 times, most recently from fc67bf1 to 9cd1f43 Compare March 20, 2024 21:43
@Machine-Maker
Copy link
Member Author

Ok, I think the correct solution is for getDirection to return the same value regardless of if its an extend or retract, just returning the direction the piston head is relative to the block itself.

@Machine-Maker Machine-Maker force-pushed the fix/sticky-piston-retract-event branch from 9cd1f43 to cbd3c3d Compare March 20, 2024 22:10
@Krakenied
Copy link

Krakenied commented Mar 26, 2024

2024-03-26.11-39-22.mp4

Works properly, I tested cancelling too and it works as well.

@WarnDa

This comment was marked as spam.

@NoltoxGit

This comment was marked as spam.

@Krakenied
Copy link

Ok, I think the correct solution is for getDirection to return the same value regardless of if its an extend or retract, just returning the direction the piston head is relative to the block itself.

However it will break plugins behavior on Spigot. Especially plugins tracking sticky pistons blocks movement as on Spigot the method (for sticky pistons) returns piston head direction for extend and the opposite for retract. It's not ideal though as well because currently on Spigot normal piston event getDirection method returns piston head direction for both extend and retract. There is one more inconsistency on Spigot too - the sticky piston doesn't fire retract event if there is no blocks attached to it.

@Machine-Maker Machine-Maker added the build-pr-jar Enables a workflow to build Paperclip jars on the pull request. label Apr 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build-pr-jar Enables a workflow to build Paperclip jars on the pull request.
Projects
Status: Awaiting review
Development

Successfully merging this pull request may close these issues.

None yet

6 participants