Skip to content

Make EntityUnleashEvent cancellable#4993

Merged
Machine-Maker merged 1 commit intoPaperMC:masterfrom
Machine-Maker:feature/EntityUnleashEvent-cancellation
Aug 13, 2021
Merged

Make EntityUnleashEvent cancellable#4993
Machine-Maker merged 1 commit intoPaperMC:masterfrom
Machine-Maker:feature/EntityUnleashEvent-cancellation

Conversation

@Machine-Maker
Copy link
Member

@Machine-Maker Machine-Maker commented Jan 4, 2021

Closes #4780

One thing to note. Should it be possible to cancel the event if the leashed entity dies? I tested if the player dies and you do stay leashed to it, but if the leashed entity dies, maybe it should unleash regardless of event cancellation.

@stale
Copy link

stale bot commented Mar 5, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@Owen1212055
Copy link
Member

No.

@stale stale bot removed the resolution: stale label Mar 5, 2021
@Machine-Maker Machine-Maker force-pushed the feature/EntityUnleashEvent-cancellation branch 4 times, most recently from e5c8967 to febfdf4 Compare March 22, 2021 00:25
@Machine-Maker
Copy link
Member Author

Rebased after the NMS repackage

@kennytv kennytv force-pushed the feature/EntityUnleashEvent-cancellation branch from febfdf4 to d440d33 Compare April 25, 2021 08:50
@Machine-Maker Machine-Maker requested a review from a team as a code owner April 25, 2021 08:50
Copy link
Member

@kennytv kennytv left a comment

Choose a reason for hiding this comment

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

You definitely have to still call Vanilla's unleash method when a) the holder disconnected (otherwise the server will freak out trying to unleash the entity for eternity), or b) the leashed entity dies (for internal distance tracking purposes). Im pretty sure both of these should be covered in EntityInsentient#eA(), tho Bukkit's lacking unleash reasons didn't really help looking at that 😛 so there might even be more cases, but not sure

@Machine-Maker
Copy link
Member Author

Hmm, if you can't cancel it in several situations, how should this be handled? Just document in the event that cancelling in those situations will not actually do anything?

@kennytv
Copy link
Member

kennytv commented Apr 25, 2021

Any "good" suggestions I'd have would require slightly breaking the event, I can't really think of anything but document what cases aren't cancellable and that setting cancel to true won't have any effect there, yeah.

@Machine-Maker Machine-Maker force-pushed the feature/EntityUnleashEvent-cancellation branch from d440d33 to 347186b Compare May 24, 2021 18:18
@Machine-Maker
Copy link
Member Author

Ok, made it uncancellable in those situations I think.

@Machine-Maker Machine-Maker force-pushed the feature/EntityUnleashEvent-cancellation branch from 347186b to a7920ed Compare July 9, 2021 05:16
@Machine-Maker
Copy link
Member Author

Rebased for 1.17.1

@Machine-Maker Machine-Maker force-pushed the feature/EntityUnleashEvent-cancellation branch from a7920ed to 573be62 Compare August 10, 2021 05:13
@Machine-Maker Machine-Maker requested a review from kennytv August 10, 2021 05:14
@Machine-Maker Machine-Maker force-pushed the feature/EntityUnleashEvent-cancellation branch from 573be62 to d77793b Compare August 13, 2021 03:24
@Machine-Maker Machine-Maker force-pushed the feature/EntityUnleashEvent-cancellation branch from d77793b to 09d6a2a Compare August 13, 2021 03:25
@Machine-Maker Machine-Maker requested a review from kennytv August 13, 2021 03:25
@Machine-Maker Machine-Maker merged commit ce43ce8 into PaperMC:master Aug 13, 2021
@Machine-Maker Machine-Maker deleted the feature/EntityUnleashEvent-cancellation branch August 13, 2021 18:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow Canceling the EntityUnleashEvent

5 participants