GlowstoneMC / Glowstone

A fast, customizable and compatible open source server for Minecraft: Java Edition
https://glowstone.net
Other
1.88k stars 271 forks source link

Implement BlockPistonRetractEvent #1014

Closed Red-Teapot closed 5 years ago

Red-Teapot commented 5 years ago

Addresses #922

Although I have a couple of questions.

BlockPistonRetractEvent.java, lines 23-24:

     * Get an immutable list of the blocks which will be moved by the
     * extending.

Should there be 'retracting' instead of 'extending'?

Is the list of moved blocks as the event parameter needed at all? Isn't just one block enough? Pistons can't move more than one block when retracting anyways.

mastercoms commented 5 years ago

Hi, I believe the comment should be retracting!

Also, slime blocks allow pistons to pull multiple blocks:

When a slime block is pushed or pulled by a piston it will attempt to move, in the same direction, all adjacent blocks.

https://minecraft.gamepedia.com/Slime_Block#Pistons

Red-Teapot commented 5 years ago

Forgot about slime blocks, thanks. Although it seems like slime blocks are not pushed properly yet. Maybe I'll try to implement slime pushing then but I'm not sure if I can do that.

mastercoms commented 5 years ago

You don't have to, would be out of the scope of this PR currently. Just wanted to let you know why the list exists :wink:

But it's your choice if you want to include that functionality, I'm happy to accept it as part of this PR.

Red-Teapot commented 5 years ago

I can't fix the comment since it's in Glowkit.

I think I'll create a separate PR when/if I implement slime pushing so it would be named properly and won't hang here.