Skip to content

Entity#teleport changes to use vanilla logic#12078

Closed
ShaneBeee wants to merge 2 commits into
PaperMC:mainfrom
ShaneBeee:shane/teleport-vanilla
Closed

Entity#teleport changes to use vanilla logic#12078
ShaneBeee wants to merge 2 commits into
PaperMC:mainfrom
ShaneBeee:shane/teleport-vanilla

Conversation

@ShaneBeee
Copy link
Copy Markdown
Contributor

@ShaneBeee ShaneBeee commented Feb 7, 2025

This PR aims to create a new method for teleporting using vanilla logic.
Minecraft handles a lot of the teleport logic that Bukkit/CB has used over the years.
A lot of the CraftBukkit teleport code is so old, and limits what users can do with teleporting.
This chance allows the user to rely on Minecraft to handle the teleport logic rather than CraftBukkit limitations.

A Few Notes:

  • I chose the method name teleportVanilla as a temporary placeholder. Would definitely like a better name. Maybe teleportWithPassengers?
  • I haven't added all the possible methods yet for Bukkit Entity, I wanted to get feedback first before writing all those out
  • I slightly changed ServerPlayer teleport logic to handle the player's passengers (Minecraft by default doesn't allow players having passengers, this is seen when attempting to use the /ride command... the API however let's us do this, so I wanted to make sure that worked.).

I have done a fair bit of testing, including:

  • Player with a stack of passengers (I had 5 chickens above me)
  • Player riding another mob (a sheep)
  • Player riding a stack of mobs (5 chickens again)
  • Player riding a player (and attempting to teleport both players)
    All of this has worked as expected, all teleports were successful.

Final Note:

As discussed in a previous PR of mine fixing the retain passengers flag... a lot of this NMS stuff often goes over my head.
I've spent a decent amount of time digging around to make sure I was fully understanding the logic, and making sure not to break anything, but as always, Im open to feedback.

@Malfrador
Copy link
Copy Markdown
Member

I personally would prefer if the Craftbukkit teleport logic was simply aligned with vanilla, instead of introducing an entirely new method. The goal should be to align API behavior with vanilla where it makes sense, and here there really is little reason to keep the limitations of the Craftbukkit method.

Of course this then should be done in a major update so people are aware of the change.

@lynxplay
Copy link
Copy Markdown
Contributor

lynxplay commented Feb 8, 2025

See https://discord.com/channels/289587909051416579/925530366192779286/1336425811187859456 for some discussion, seems we have a good chunk of the team on the idea to just align existing methods in the next release window with notice to developers.

@ShaneBeee
Copy link
Copy Markdown
Contributor Author

I do agree with having this take over Bukkit logic over a new method.

The only reason I wrote a new method was because I attempted something along these lines a year or so ago, back on CraftBukkit.
It was noted by someone not to change Bukkit logic.

I personally agree the Bukkit logic is old and silly (why are we explicitly blocking Passengers from TPing when Minecraft allows it?)

I am happy to remove the new method and just plop this into the existing methods if that is what you guys would like me to do.

@lynxplay
Copy link
Copy Markdown
Contributor

lynxplay commented Feb 8, 2025

I'll just close this for now and put a mental marker on the 1.21.5 issue.
Not much point in keeping it around until then and it is a super simple change too 👍

@lynxplay lynxplay closed this Feb 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Closed

Development

Successfully merging this pull request may close these issues.

3 participants