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

Add teleport flags #6498

Open
wants to merge 16 commits into
base: dev/feature
Choose a base branch
from

Conversation

TheLimeGlass
Copy link
Collaborator

Description

Adds Paper's TeleportFlags in 1.19+
Allows to retain location, passengers, x-coordinate etc, during a teleportation.


Target Minecraft Versions: Paper 1.19+
Requirements: Paper 1.19+
Related Issues: #6119

@AyhamAl-Ali AyhamAl-Ali added the feature Pull request adding a new feature. label Mar 16, 2024
Copy link
Member

@AyhamAl-Ali AyhamAl-Ali left a comment

Choose a reason for hiding this comment

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

src/main/java/ch/njol/skript/effects/EffTeleport.java Outdated Show resolved Hide resolved
src/main/java/ch/njol/skript/effects/EffTeleport.java Outdated Show resolved Hide resolved
src/main/java/ch/njol/skript/effects/EffTeleport.java Outdated Show resolved Hide resolved
src/main/java/ch/njol/skript/effects/EffTeleport.java Outdated Show resolved Hide resolved
src/main/resources/lang/default.lang Outdated Show resolved Hide resolved
src/main/resources/lang/default.lang Show resolved Hide resolved
src/main/java/ch/njol/skript/effects/EffTeleport.java Outdated Show resolved Hide resolved
Copy link
Member

@Pikachu920 Pikachu920 left a comment

Choose a reason for hiding this comment

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

I think this is a good feature to support, but I'm a little worried that this implementation is a little more complicated than it has to be. The syntax seems good for users, but the implementation feels a little hard to reason around because there are so many methods with the same name and, in my opinion, some methods are in odd places.

@TheLimeGlass
Copy link
Collaborator Author

I think this is a good feature to support, but I'm a little worried that this implementation is a little more complicated than it has to be. The syntax seems good for users, but the implementation feels a little hard to reason around because there are so many methods with the same name and, in my opinion, some methods are in odd places.

The usage of the functional interface allows for keeping a dynamic calling, and also exactly as you said. There are very similar methods with similar parameters which is the exact purpose of a functional interface.

This can easily be expanded apon to add support for TeleportCauseEvent if and when needed, as there are also those methods in the Entity class.

src/main/resources/lang/default.lang Outdated Show resolved Hide resolved
Copy link
Member

@sovdeeth sovdeeth left a comment

Choose a reason for hiding this comment

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

looks good!

src/main/java/ch/njol/skript/effects/EffTeleport.java Outdated Show resolved Hide resolved
Co-authored-by: sovdee <10354869+sovdeeth@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Pull request adding a new feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants