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 AllowWorldChange, AllowTeleport and AfterTeleport #4089

Closed
wants to merge 6 commits into from

Conversation

fishshi
Copy link

@fishshi fishshi commented Sep 13, 2024

I implemented an API that allows for custom actions to be executed before and after a player teleports to inject custom code at the beginning and end of the teleport method. This approach provides more control and flexibility over teleport behavior, such as restricting teleportation for some case or triggering specific effects after teleportation.
Added three APIs:
AllowWorldChange: Called when the player's dimension changes to check if the player is allowed to cross dimensions.
AllowTeleport: Called before the player teleports to check if the player is allowed to teleport.
AfterTeleport: Called after the player teleports to execute custom logic.
The first API can be used for portals, while the other two can be applied to various teleport commands and mods that use teleport commands.

ServerPlayerEntity player = (ServerPlayerEntity) (Object) this;
boolean allowed = ServerPlayerEvents.ALLOW_TELEPORT.invoker().allowTeleport(player, new Vec3d(destX, destY, destZ));
if (!allowed) {
throw new CommandSyntaxException(null, new LiteralMessage(""));
Copy link
Member

Choose a reason for hiding this comment

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

Question (I havent got the code in front of me), why is a CommandSyntaxException thrown? Does it need a message, and does this prevent the player from being teleported when they use a portal?

Copy link
Author

Choose a reason for hiding this comment

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

I threw an exception because, whether the teleportation is successful or not, a teleportation message will always pop up when using coordinates or player-to-player teleportation. An empty exception can somewhat resolve this issue.

@@ -80,6 +80,20 @@ public void onInitialize() {
LOGGER.info("Respawned {}, [{}, {}]", oldPlayer.getGameProfile().getName(), oldPlayer.getWorld().getRegistryKey().getValue(), newPlayer.getWorld().getRegistryKey().getValue());
});

ServerPlayerEvents.ALLOW_TELEPORT.register((player, pos) -> {
if(pos.getY() < -256)
Copy link
Member

Choose a reason for hiding this comment

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

Might be better to prevent the player from teleporting if they are holding a specific item, this way its easier to test portals.

Copy link
Author

Choose a reason for hiding this comment

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

Using portals to travel across dimensions is different from teleporting with coordinates or between players, making it difficult to unify the behaviors. AllowTeleport and AfterTeleport is only suitable for the latter, whether using the tp command or a map mod.

@fishshi
Copy link
Author

fishshi commented Sep 14, 2024

I added AllowChangeWorld to handle portal events, which will be called before the player goes through the portal.

@fishshi fishshi changed the title add AllowTeleport and AfterTeleport add AllowWorldChange, AllowTeleport and AfterTeleport Sep 16, 2024
@modmuss50 modmuss50 added the enhancement New feature or request label Sep 18, 2024
@@ -77,6 +84,16 @@ private void notifyDeath(DamageSource source, CallbackInfo ci) {
ServerLivingEntityEvents.AFTER_DEATH.invoker().afterDeath((ServerPlayerEntity) (Object) this, source);
}

@Inject(method = "teleportTo", at = @At(value = "FIELD", target = "Lnet/minecraft/server/network/ServerPlayerEntity;inTeleportationState:Z", opcode = Opcodes.PUTFIELD, shift = At.Shift.BEFORE), cancellable = true)

Choose a reason for hiding this comment

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

Shift.BEFORE should never be used. Inject already places its code before the target instruction, this places is one instruction back unnecessarily.

Copy link
Author

Choose a reason for hiding this comment

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

I see, thank you!

boolean allowed = ServerEntityWorldChangeEvents.ALLOW_PLAYER_CHANGE_WORLD.invoker().allowChangeWorld(player, player.getServerWorld(), teleportTarget.world());
if (!allowed) {
cir.setReturnValue(null);
cir.cancel();

Choose a reason for hiding this comment

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

You don't need to call cancel, that's the first thing setReturnValue does.

Copy link
Author

Choose a reason for hiding this comment

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

I see, thank you!

Comment on lines 110 to 119
@Inject(method = "teleport(Lnet/minecraft/server/world/ServerWorld;DDDLjava/util/Set;FF)Z", at = @At("HEAD"), cancellable = true)
private void beforeTeleport(ServerWorld world, double destX, double destY, double destZ, Set<PositionFlag> flags, float yaw, float pitch, CallbackInfoReturnable<Boolean> cir) throws CommandSyntaxException {
ServerPlayerEntity player = (ServerPlayerEntity) (Object) this;
boolean allowed = ServerPlayerEvents.ALLOW_TELEPORT.invoker().allowTeleport(player, world, new Vec3d(destX, destY, destZ));
if (!allowed) {
throw new CommandSyntaxException(null, new LiteralMessage(""));
}
}

@Inject(method = "teleport(Lnet/minecraft/server/world/ServerWorld;DDDLjava/util/Set;FF)Z", at = @At("TAIL"), cancellable = true)

Choose a reason for hiding this comment

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

There's no need for these to be cancellable. There's also a potential issue with the before event being called and the after event not being called if someone cancels the method. Not sure how worried we are about that, and if it'd be worth requiring loader 0.16 to get https://github.com/LlamaLad7/MixinExtras/wiki/WrapMethod instead.

Copy link
Author

Choose a reason for hiding this comment

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

I understand, thank you for pointing that out!

@fishshi
Copy link
Author

fishshi commented Sep 18, 2024

Thank you all for your guidance and feedback! I've made the necessary changes based on your suggestions. Additionally, I've also addressed the issues that occurred during the build.

@fishshi
Copy link
Author

fishshi commented Sep 20, 2024

I have discovered a bug and am working on fixing it. I will submit part of the PR later once I have made some progress.

@fishshi fishshi closed this Sep 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants