Skip to content

fix NPE on EntityTeleportEvent getTo#10016

Merged
Machine-Maker merged 1 commit intoPaperMC:masterfrom
Machine-Maker:fix/EntityTeleportEvent-NPE
Dec 26, 2023
Merged

fix NPE on EntityTeleportEvent getTo#10016
Machine-Maker merged 1 commit intoPaperMC:masterfrom
Machine-Maker:fix/EntityTeleportEvent-NPE

Conversation

@Machine-Maker
Copy link
Copy Markdown
Member

Almost everywhere EntityTeleportEvent (or a subclass) was used, the nullability of getTo was never checked despite it being marked as nullable (as well as setTo). This is really dumb, as what is null supposed to do? So I just made it act as being cancelled.

Also in one spot, the firing of the event for /teleport, the to location wasn't even respected if you called setTo.

@Machine-Maker Machine-Maker requested a review from a team as a code owner December 10, 2023 03:32
@kennytv
Copy link
Copy Markdown
Member

kennytv commented Dec 10, 2023

Why not just cancel the event if passed null if the param should stay nullable?

@kennytv
Copy link
Copy Markdown
Member

kennytv commented Dec 10, 2023

Or honestly, just make it notnull if it's broken anyways lol

@Machine-Maker
Copy link
Copy Markdown
Member Author

Or honestly, just make it notnull if it's broken anyways lol

I considered that, but I would also have to change it for all the subclasses of EntityTeleportEvent which have constructors that are marked with @Nullable. Specifically for the EntityPortalEvent which has always respected null getTo.

@Machine-Maker Machine-Maker force-pushed the fix/EntityTeleportEvent-NPE branch from 5e2a006 to daafa23 Compare December 26, 2023 19:06
@Machine-Maker Machine-Maker merged commit f483b38 into PaperMC:master Dec 26, 2023
@Machine-Maker Machine-Maker deleted the fix/EntityTeleportEvent-NPE branch December 26, 2023 19:22
lynxplay pushed a commit to lynxplay/paper that referenced this pull request Feb 23, 2024
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.

3 participants