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

Fix MoveEntityEvent.Teleport isn't fired when a player uses the spectator-menu to teleport #2239

Closed
wants to merge 4 commits into from

Conversation

karayuumac
Copy link

fix #2099

@karayuumac karayuumac changed the title Fix MoveEntityEvent.Teleport isn't fired when a player uses the spectator menu to teleport Fix MoveEntityEvent.Teleport isn't fired when a player uses the spectator-menu to teleport Apr 19, 2019
this.player.setSpectatingEntity(this.player);
this.player.dismountRidingEntity();
// Sponge start - handle MoveEntityEvent.Teleport
double x = entity.posX;
Copy link
Member

@gabizou gabizou Apr 19, 2019

Choose a reason for hiding this comment

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

If all that is being done is based on the player, and entity, you should be able to reduce this to a single cancellable injection:

    @Inject(
        method = "handleSpectate",
        at = @At(
            value = "FIELD",
            target = "Lnet/minecraft/entity/Entity;world:Lnet/minecraft/world/World;",
            ordinal = 0
        ),
        locals = LocalCapture.CAPTURE_FAILHARD,
        cancellable = true
    )
    private void onSpectateTeleportForSponge(CPacketSpectate packetIn, CallbackInfo ci, Entity spectatingEntity) {
        double x = spectatingEntity.posX;
        double y = spectatingEntity.posY;
        double z = spectatingEntity.posZ;
        float yaw = spectatingEntity.rotationYaw;
        float pitch = spectatingEntity.rotationPitch;
        MoveEntityEvent.Teleport event = EntityUtil.handleDisplaceEntityTeleportEvent(this.player, x, y, z, yaw, pitch);
        if (event.isCancelled()) {
            ci.cancel();
        }

    }

Also, make sure your javadoc includes @author and the date, and the version of Minecraft targeted.

I've tested this injection in development and this is the result:
Screen Shot 2019-04-19 at 1 10 59 PM

@karayuumac
Copy link
Author

Change to cancellable injection.
Also, change to use Transform because players can teleport to a different dimension.

Because I don't want to use @Inject three times, I use @overwrite
@karayuumac
Copy link
Author

  • fix: When the toTransform is changed by the plug-in, the player's transform is changed correctly.

I also have a question.
Plug-in can also set whether the player's velocity is keeping after teleportation by using
MoveEntityEvent.Teleport.setKeepsVelocity(true).
I have no idea how to implement this.

@ImMorpheus
Copy link
Contributor

@karayuumac Thanks for your contribution.

You seem to have reverted your changes with this commit ce629ae.
Please, change back to cancellable injection.

Plug-in can also set whether the player's velocity is keeping after teleportation by using
MoveEntityEvent.Teleport.setKeepsVelocity(true).
I have no idea how to implement this.

As far as I can see you don't need to. It's already handled in EntityUtil#handleDisplaceEntityPortalEvent
https://github.com/SpongePowered/SpongeCommon/blob/4cd5f78d60ed71d034327e674f9527e03498ee49/src/main/java/org/spongepowered/common/entity/EntityUtil.java#L492-L496

@ImMorpheus ImMorpheus added status: needs review a code review is needed status: needs testing does this run, does it solve the issue etc system: event type: bug Something isn't working labels Apr 26, 2019
@karayuumac
Copy link
Author

As far as I can see you don't need to. It's already handled in EntityUtil#handleDisplaceEntityPortalEvent

Thank you for asking my question. I understood.

You seem to have reverted your changes with this commit ce629ae.
Please, change back to cancellable injection.

OK. I'll try to change to cancellable injection.

@parlough
Copy link
Contributor

parlough commented Aug 9, 2019

Thanks for the contribution! It is possible to implement it all without overwriting the method, it just gets a little messy.

Superseded by 620cc8a

@parlough parlough closed this Aug 9, 2019
@parlough parlough added resolution: superseded This isssue/PR was superseded by another one and removed status: needs review a code review is needed status: needs testing does this run, does it solve the issue etc labels Aug 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
resolution: superseded This isssue/PR was superseded by another one system: event type: bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MoveEntityEvent.teleport does not apply to spectator mode teleporting
5 participants