mojira.dev
MC-120545

Teleportation wraps rotation angles causing first-person hand to rotate incorrectly

/tp and /teleport forcibly wrap player rotation, which causes the first person hand/held item to rotate incorrectly.

Case 1

(Does not work since 1.19.3)

If you rotate in circles several times, and then run /tp @p ~ ~ ~ or /teleport @p ~ ~ ~, your hand will spin in circles several times extremely quickly, even though the actual visual angle will not change.

Case 2

If, after running /tp @p ~ ~ ~ 0 ~ to ensure consistency, you run /tp @p ~ ~ ~ ~90 ~, your hand will make 3 small rotations from the left and then one big rotation from the right, even though in each case you've been making small rotations from the left. The inverse happens with /tp @p ~ ~ ~ ~-90 ~.

Cause

The hand rotation interpolates between Entity.prevRotationYaw and Entity.rotationYaw. This would normally be fine, but these values are wrapped differently. rotationYaw is modulo'd by 360 within setRotation, while prevRotationYaw is left unwrapped by setPositionAndRotation (the one that the player position packet handler uses) except for a +/- 360 change based off of the difference from the old value.

Potential fix

A fix by @unknown can be found in this comment.

This can also be hack-fixed by simply removing both cases of angle wrapping:

double d0 = (double)(this.prevRotationYaw - yaw);

if (d0 < -180.0D)
{
    this.prevRotationYaw += 360.0F;
}

if (d0 >= 180.0D)
{
    this.prevRotationYaw -= 360.0F;
}

in Entity.setPositionAndRotation (Entity.a(DDDFF)V) should be removed completely (without wrapping it doesn't make sense).

protected void setRotation(float yaw, float pitch)
{
    this.rotationYaw = yaw; // % 360
    this.rotationPitch = pitch; // % 360
}

The % 360 should be removed from setRotation.

The problem with this fix is that now there is no limit to the angle. That isn't too big of a problem (as clientside there already is no limit - F3 displays the wrapped version), but it could be mitigated by calling MathHelper.wrapDegrees when reading/writing the entity from/to NBT (as there won't be any previous rotations to worry about there).

Demo

See this video. This video was recorded in my MCP test install, and there are some things that are non-standard, but this bug has been tested in vanilla. Of important note is the unwrapped angle in F3, displayed in red. This is the exact value of rotationYaw, without calling wrapDegrees. The game was also slowed down to .25x the normal speed using cheat engine, again for visibility.

Related issues

Comments

migrated

If prevRotationYaw could have the exact same type of wrapping as rotationYaw, would that also fix the problem?

pokechu22

I thought about that, and I decided that it probably wouldn't solve the issue entirely - it would prevent the spinning hand when one is wrapped and the other isn't, but it wouldn't help with case 2 (inconsistent hand rotation with relative TPs). Granted, case 2 is much more minor, but it still wouldn't be ideal IMO.

migrated

Might be worth a test. The hand behavior, if I understand correctly, is caused by it trying to correct the wrapping while teleporting, right? If it tries to iterate more than once while teleporting, it makes the arm visually glitchy, which is the main problem with this bug.

migrated

Can you explain what each of the yaw values is supposed to represent? There's prevRotationYaw, rotationYaw, and yaw? Without more context it is difficult to tell what each one is for. It's clear that somewhere in the code (likely in the code regarding the player manually rotating) there is a value that is going outside the -360 to 360 degree range, which is likely behind the cause of this ticket and MC-124827.

My guess is that there is a yaw value that remains unbounded when the player manually rotates. Under this assumption, if this yaw value in question reached a value of -1830, and a player ran the command /tp @p ~ ~ ~ ~0 ~, then the command would apply the proper wrapping to -1830 and make it -30. However, the player is still seen as being at a rotation of -1830, meaning that this command would make the player rotate 5 full rotations to get to a value of -30. This would cause the behavior described in this ticket, in which the player's hand rotates around even though the player rotation wasn't supposed to change.

Any additional feedback would be appreciated. I don't have any of the specific details that cause this behavior, such as code, I only have observations to go on and a bit of understanding on how rotation wrapping works.

pokechu22

@@unknown The information I have in this ticket is basically all I have; it's all based off of the code I have (via MCP; here's info about MCP for other versions). There's two fields in Entity: rotationYaw and prevRotationYaw. The yaw one in those snippets is simply the parameter to the function. It's also worth reading the documentation on the player position and look packet.

Unfortunately the code that MCP produces, being based on decompilation, doesn't have any decompilation on it to understand what was intended/why it was written a specific way. In some cases it's possible to figure it out, but in others (like this) it's hard.

migrated

Case 2 no longer seems to apply as of 1.14.3 - it has become entirely consistent. However, it seems it still goes from the uncapped rotation to a value from 180 to 180 upon teleporting, meaning the hand still jitters. It would be amazing if this were looked at now, since 1.14 had just fixed MC-4686, especially since this seems like an easy fix.

Avoma

Can confirm in 20w48a.

Avoma

Can confirm in 21w03a.

Avoma

Can confirm in 21w05b.

ampolive

Can confirm in 1.17.1.

ampolive

Can confirm in 21w42a.

Moulberry

As of 1.19.2, this bug still exists, but the cause identified and fix proposed in the original report is inaccurate.

Whenever you want to find the difference between two angles, you always have to wrap the result of the subtraction.

This isn't being done in two places, causing this bug to occur when the yaw changes by multiples of 360.

 

(Note: the two changes below only affect rendering code, so they won't inadvertently break calculations involving yaw/pitch.)

 

1. LocalPlayer#serverAiStep calculates 

this.yBob += (this.getYRot() - this.yBob) * 0.5F;

If getYRot is 0 (because of a teleport) and yBob is 360, 720, etc., the result of this calculation is clearly wrong.

The correct calculation is as follows:

this.yBob += Mth.wrapDegrees(this.getYRot() - this.yBob) * 0.5F;

 

2. ItemInHandRenderer calculates

poseStack.mulPose(Vector3f.YP.rotationDegrees((localPlayer.getViewYRot(f) - k) * 0.1f));

Similarly to above, the correct calculation is:

poseStack.mulPose(Vector3f.YP.rotationDegrees(Mth.wrapDegrees(localPlayer.getViewYRot(f) - k) * 0.1f));
Moulberry

As of 1.19.3, the first reproducer (Case 1) is no longer working since the handling of relative teleports by ClientPacketListener#handleMovePlayer has changed. The change results in no angle wrapping, similar to the "hack-fix" proposed in the original report.

However, the underlying issue is still present and can be reproduced using absolute teleports. Simply rotate in circles several times, then perform /teleport ~ ~ ~ X ~ where X is the yaw value observed via the F3 menu.

The fix below is still applicable.

pokechu22

(Unassigned)

Confirmed

Platform

Low

Commands, Player Animation

/tp, angle, teleport, visual

Minecraft 1.12.1, Minecraft 1.12.2 Pre-Release 1, Minecraft 1.12.2 Pre-Release 2, Minecraft 1.12.2, Minecraft 17w43a, ..., 1.19.1 Release Candidate 2, 1.19.2, 1.19.3, 1.19.4 Pre-release 1, 24w36a

Retrieved