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

Event when a block support is broken #6258

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

Conversation

roimee6
Copy link

@roimee6 roimee6 commented Feb 18, 2024

Introduction

Currently, when a block is broken because its support is broken, we can't modify the drops as in blockbreakevent, which can create duplication bugs in our plugins

I've searched all the uses of the "useBreakOn" function without a player is linked to a broken block because its support has been broken

@roimee6
Copy link
Author

roimee6 commented Feb 18, 2024

The name of the event seems a bit off, but I can't think of anything else to say in English

@pandaaaBE
Copy link
Contributor

What exactly do you mean by support?

@roimee6
Copy link
Author

roimee6 commented Feb 18, 2024

What exactly do you mean by support?

Support block, when the block holding the seeds, for example, is broken, this event is called for the seed block

@kaxyum
Copy link

kaxyum commented Feb 18, 2024

This could be very useful, but the name needs to be revised. Alternatively, why not modify the blockbreak event so that it can be called here?

@dktapps
Copy link
Member

dktapps commented Feb 18, 2024

Messing with existing events would break BC.

@ShockedPlot7560
Copy link
Member

I don't think this is the best name for this type of event. In any case, changing the BlockBreak event isn't the best solution either.

@ShockedPlot7560 ShockedPlot7560 added Category: API Related to the plugin API Type: Enhancement Contributes features or other improvements to PocketMine-MP labels Feb 19, 2024
@dktapps
Copy link
Member

dktapps commented Feb 19, 2024

Really BlockBreakEvent itself is mis-named (it should be PlayerBlockBreakEvent).

Removal of support isn't the only case in which a block will get destroyed without a player present. For example, water flowing into a block will destroy it, triggering this event.

@roimee6
Copy link
Author

roimee6 commented Feb 19, 2024

So what name could be representative?

@dktapps
Copy link
Member

dktapps commented Feb 19, 2024

I don't have a problem with it personally. My only issue is that this will currently be called in cases where there wasn't any support to begin with.

@kaxyum
Copy link

kaxyum commented Feb 20, 2024

Really BlockBreakEvent itself is mis-named (it should be PlayerBlockBreakEvent).

Removal of support isn't the only case in which a block will get destroyed without a player present. For example, water flowing into a block will destroy it, triggering this event.

So why not create a PlayerBlockBreakEvent and a BlockBreakEvent ?

@dktapps
Copy link
Member

dktapps commented Feb 20, 2024

Really BlockBreakEvent itself is mis-named (it should be PlayerBlockBreakEvent).
Removal of support isn't the only case in which a block will get destroyed without a player present. For example, water flowing into a block will destroy it, triggering this event.

So why not create a PlayerBlockBreakEvent and a BlockBreakEvent ?

That's a big change that will break a lot of plugins

@kaxyum
Copy link

kaxyum commented Feb 20, 2024

Really BlockBreakEvent itself is mis-named (it should be PlayerBlockBreakEvent).
Removal of support isn't the only case in which a block will get destroyed without a player present. For example, water flowing into a block will destroy it, triggering this event.

So why not create a PlayerBlockBreakEvent and a BlockBreakEvent ?

That's a big change that will break a lot of plugins

Yes, so why not do it for PM6?

@dktapps
Copy link
Member

dktapps commented Mar 4, 2024

Really BlockBreakEvent itself is mis-named (it should be PlayerBlockBreakEvent).
Removal of support isn't the only case in which a block will get destroyed without a player present. For example, water flowing into a block will destroy it, triggering this event.

So why not create a PlayerBlockBreakEvent and a BlockBreakEvent ?

That's a big change that will break a lot of plugins

Yes, so why not do it for PM6?

Just because we have a major version doesn't mean we should just break plugins. The benefits of making a BC break need to be weighed against the costs of changing it.

Joshy3282 added a commit to Joshy3282/PocketMine-MP that referenced this pull request Oct 3, 2024
@dktapps dktapps added Status: Blocked Depends on other changes which are yet to be completed and removed Status: Waiting on Author labels Nov 10, 2024
@dktapps dktapps added this to the 6.0 milestone Nov 29, 2024
@dktapps
Copy link
Member

dktapps commented Nov 29, 2024

I think we could implement this as BlockSupportRemoveEvent if the PR was adapted to only fire the event when block support was actually deleted. This would (in theory) be backwards-compatible.

The best way to do this would probably be to create a wrapper for useBreakOn() that would fire the event, and then update all the places where lack of support causes a block to be destroyed to use the wrapper method. (or perhaps a cause parameter to useBreakOn.)

@dktapps dktapps added Status: Waiting on Author and removed Status: Blocked Depends on other changes which are yet to be completed labels Nov 29, 2024
@dktapps dktapps removed this from the 6.0 milestone Nov 29, 2024
@dktapps dktapps added the Easy task Probably really easy to do, good task for first-time contributors label Dec 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: API Related to the plugin API Easy task Probably really easy to do, good task for first-time contributors Status: Waiting on Author Type: Enhancement Contributes features or other improvements to PocketMine-MP
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants