Skip to content

Add EntityFreezeWaterEvent #6673

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

Open
wants to merge 10 commits into
base: minor-next
Choose a base branch
from
Open

Conversation

zSALLAZAR
Copy link
Contributor

@zSALLAZAR zSALLAZAR commented Apr 14, 2025

Add EntityWaterFreezeEvent

Related issues & PRs

#6614

Changes

API changes

Added class pocketmine\event\entity\EntityWaterFreezeEvent - called when an entity with boots enchanted with frost walker freezes nearby water.

Behavioural changes

Backwards compatibility

Follow-up

Tests

@zSALLAZAR zSALLAZAR requested a review from a team as a code owner April 14, 2025 14:53
@dktapps
Copy link
Member

dktapps commented Apr 14, 2025

EntityWaterFreezeEvent would fit better with the most commonly used naming conventions (subject object verb event)

@ipad54 ipad54 added Category: API Related to the plugin API Type: Enhancement Contributes features or other improvements to PocketMine-MP labels Apr 14, 2025
@dktapps
Copy link
Member

dktapps commented Apr 14, 2025

I'm wondering whether we want to fire an event for every block, vs firing an event once.

e.g. if we had something like EntityFrostWalkerEvent, we could have stuff like getRadius(), getBlocks() etc.

Currently this event doesn't give any context, so the only real thing a plugin can do is just cancel it (which means cancelling somewhere in the ballpark of 30+ events per tick).

@zSALLAZAR
Copy link
Contributor Author

I'm wondering whether we want to fire an event for every block, vs firing an event once.

e.g. if we had something like EntityFrostWalkerEvent, we could have stuff like getRadius(), getBlocks() etc.

Currently this event doesn't give any context, so the only real thing a plugin can do is just cancel it (which means cancelling somewhere in the ballpark of 30+ events per tick).

Good idea but im unsure how this would look like

@dktapps
Copy link
Member

dktapps commented Apr 16, 2025

Right, I didn't realize it would be a pain to include the block list.

I suppose there are 2 choices:

  1. make the blocks list mutable and radius immutable
  2. get rid of blocks list and allow changing radius only

I'm slightly leaning towards number 2. I think being able to set the radius would be more ergonomic than having to manually recalculate the blocks list. Possibly a setTargetBlock() could be useful too, in case plugins want to change the result block from frosted ice to something else (or a different state of frosted ice).

A filter for blocks to change would be nice too (e.g. to allow freezing lava) but I'm not sure how that would work on the API side.

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 Type: Enhancement Contributes features or other improvements to PocketMine-MP
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants