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

Allow pistons to break blocks at borders #3729

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

CommandDan
Copy link

Overview

Description

Pistons are now able to break blocks when pushing against a plot border or plot area border

Submitter Checklist

  • Make sure you are opening from a topic branch (/feature/fix/docs/ branch (right side)) and not your main branch.
  • Ensure that the pull request title represents the desired changelog entry.
  • New public fields and methods are annotated with @since TODO.
  • I read and followed the contribution guidelines.

Pistons are now able to break blocks when pushing against a plot border or plot area border
@CommandDan CommandDan requested a review from a team as a code owner July 16, 2022 12:57
Copy link
Member

@NotMyFault NotMyFault left a comment

Choose a reason for hiding this comment

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

The plot border is meant to be immutable by in game edits, unless you have certain permission.
This change circumvenst the intended limitation.

What is the purpose of this PR?

Blocking review

@CommandDan
Copy link
Author

It does not affect the plot border. Nothing will be pushed outside the plot area as the block is broken when being pushed.

This change fixes the current issue that breakable blocks, in my situation sugarcane, next to the border can not be broken by pistons facing against the border.

It is currently possible to break these blocks using pistons as long as the breakable block is not the last in the row.

@CommandDan
Copy link
Author

Here are previews of the change
Current behaviour
New behaviour
Top Bottom comparison

@CommandDan CommandDan requested a review from NotMyFault July 16, 2022 17:52
@CommandDan
Copy link
Author

Just realised where my change has a flaw. Am going on camping vacation so I won't be able to make the changes in the coming 2 weeks but will look at fixing it when I am back home.

But yea I see it would have an issue if the border contains a breakable block

Only break if the pushed block is within plot or outside plot area
@CommandDan
Copy link
Author

Oops forgot to remove comments

@CommandDan
Copy link
Author

Should be good now

@NotMyFault NotMyFault added the Feature This PR proposes a new feature label Jul 19, 2022
@NotMyFault
Copy link
Member

The change looks good now, but justification is needed still. Items dropped on the last block on the plot inevetibly land on the border or road, which possibly leads to unwanted side effects for servers.
More feedback is needed.

@NotMyFault NotMyFault requested a review from a team July 19, 2022 21:52
@CommandDan
Copy link
Author

Yea that is true but wouldn't that be the case either way? I guess, if not working like that already, we can implement so blocks breaking keep their drops inside the plot or outside the area

@RedstoneFuture
Copy link
Member

RedstoneFuture commented Jul 25, 2022

I like the idea - thanks CommandDan!

The change looks good now, but justification is needed still. Items dropped on the last block on the plot inevetibly land on the border or road, which possibly leads to unwanted side effects for servers. More feedback is needed.

Dropping items will not be the problem, I think. You can use the plot and way flags and the kill-road-items option.

With this change, the behavior of the piston looks more "normal".

@CommandDan
Copy link
Author

Np. And yeah that is what I meant for both points

@CommandDan
Copy link
Author

The change looks good now, but justification is needed still. Items dropped on the last block on the plot inevetibly land on the border or road, which possibly leads to unwanted side effects for servers.
More feedback is needed.

What would you want to be done regards to the items?

If anything should be done

@PierreSchwang
Copy link
Member

The change looks good now, but justification is needed still. Items dropped on the last block on the plot inevetibly land on the border or road, which possibly leads to unwanted side effects for servers.
More feedback is needed.

What would you want to be done regards to the items?

If anything should be done

I'd say, if kill-road-items is set to true in the config, kill the item when dropping onto the road / border.
Not sure if the position of the final item can be predicted tbh - could be quite hard.

Otherwise, what might be unsafe, is dropping the item based on the first solid block underneath the piston retraction (Using dropItem instead of dropItemNaturally). Basically, accessing the Blocks from the event, calling getDrops on those and drop these in the middle of the lowest block.
But feedback regarding that would be highly appreciated, as that solution isn't the best probably.

@CommandDan
Copy link
Author

Yea, I can add the check for kill-road-items

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature This PR proposes a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants