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

Bring Entity#teleportAsync up to parity with Entity#teleport #12019

Closed
wants to merge 1 commit into from

Conversation

zJanny
Copy link

@zJanny zJanny commented Jan 26, 2025

The Entity.teleportAsync function did not have a overload to be able to teleport to another entity directly, and this has annoyed me quite a bit while coding, because the normal teleport has it.

So i made this small change to add this functionality

@zJanny zJanny requested a review from a team as a code owner January 26, 2025 13:49
@lynxplay
Copy link
Contributor

Hi and welcome to paper 👋

I don't see the usage of this? Why would you teleportAsync to an entity?
The entire point of that method is that you might teleport into a chunk that isn't loaded yet.
An entity instance existing means the respective chunk said entity is in also exists.
Just on a type-basis I don't see a usecase for these, but maybe I am just missing something. Could you elaborate on that a bit?

@lynxplay lynxplay added scope: api type: feature Request for a new Feature. labels Jan 26, 2025
@zJanny
Copy link
Author

zJanny commented Jan 26, 2025

Hi and welcome to paper 👋

I don't see the usage of this? Why would you teleportAsync to an entity? The entire point of that method is that you might teleport into a chunk that isn't loaded yet. An entity instance existing means the respective chunk said entity is in also exists. Just on a type-basis I don't see a usecase for these, but maybe I am just missing something. Could you elaborate on that a bit?

As far as i know, folia needs the teleportAsync method, so this would allow someone to create a Paper plugin with Folia support that uses this functionallity.

This would be one example i can think of, because i just encountered this issue with GrimAC

@lynxplay
Copy link
Contributor

If this is porting methods up from folia, then yea, this might be something to do.
Tho that is a lot different of a premise than your PRs description.

@electronicboy
Copy link
Member

These methods don't exist in folia, this is a desire to pull up some of the helper util flood for the teleport method to the teleportAsync methods; which, I'm generally on the edge around adding more helper functions for trivial operations

@zJanny
Copy link
Author

zJanny commented Jan 26, 2025

If this is porting methods up from folia, then yea, this might be something to do. Tho that is a lot different of a premise than your PRs description.

I just wanted to word it as a more general thing.
Anyways if this is not needed feel free to close this PR :D

@lynxplay
Copy link
Contributor

Yea, I think I'll close this then.

Thank you anyway for the PR <3
For documentation purposes as to why the PR was closed:

Teleporting to an entity instance means the target entity exists.
Unless a plugin is doing terrible things, it is also loaded, as such the teleport target location is loaded.
As visible in the PR, this is then pretty much just syntax sugar for CompletableFuture.completedFuture(entity.teleport(targetEntity)), which is just not enough code saved to really be worth yet another method around teleportation, which is already one of the most overloaded bits of logic we have 😅

Thank you again! Hope my line of reasoning here was understandable.

@lynxplay lynxplay closed this Jan 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope: api type: feature Request for a new Feature.
Projects
Status: Closed
Development

Successfully merging this pull request may close these issues.

3 participants