Only attempt to resync display entities' position/rotation when it has actually changed to fix teleport interpolation issues #11695
Conversation
…y changed to fix teleport interpolation issues On display entities, any new Pos/Rot packet causes the teleport duration interpolation to reset While this does not cause issues when you spawn a display entity -> set the interpolation -> teleport after the data has been sent to the client, this DOES cause issues on entities that have lived for >60 ticks, where if you spawn a display with interpolation -> wait ~8 ticks -> teleport it, the end of the teleport will look slow due to the server resyncing the display entity position/rotation
|
Can we potentially do this ONLY when a display entity is currently interpolating? |
It would be possible, but it is a bit more complex...
What I've decided to do is using packets to create the text display & stuff, so technically this PR is not need, but I do think that an warning should be added to the https://docs.papermc.io/paper/dev/display-entities page because I spent an entire day pulling my hair out, only to find out that this is a vanilla issue. |
|
Is there an MC bug report that explains this? |
I couldn't find any reports about this bug report on the tracker, I do know that Mojang seems to know about this issue, which is why they clamp the teleport duration value to 59 (which is below the periodic teleport sync delay of 60 ticks lived) and they did say that they clamp the value because "Please note that this value is clamped to avoid glitches due to periodic position updates", however I think that they don't expect users to keep teleporting the same display entity over and over, thinking that players would actually create new display entities every time they need to teleport them. |
|
Scratch that, I found a bug report: https://bugs.mojang.com/browse/MC-272599?jql=text%20~%20%22teleport_duration%22 |
How is it more complex? Imo, what I would do is expose the |
On the server the However I think it wouldn't be hard to make the server call those functions tho... But here's another issue: Another issue is that this: display.setTeleportDuration(59)
display.teleport(...)Does NOT work as how you would expect. What I expected is that the display entity will teleport to the destination over 59 ticks. In reality the entity is teleported before the entity is updated on the client, so the display entity will teleport directly to the new location without any easing, so you need to wait 1 tick before attempting to teleport the display entity.
If everything is true, then the teleport position update should be ignored. It isn't impossible to track all of that, but it does get a bit messier. |
|
Ah yeah this is in general kinda scuffed, then your solution is prolly fine. Although I would say that this should have the mojira issue linked in the patch. |
On display entities, any new Pos/Rot packet causes the teleport duration interpolation to reset
While this does not cause issues when you spawn a display entity -> set the interpolation -> teleport after the data has been sent to the client, this DOES cause issues on entities that have lived for >60 ticks, where if you spawn a display with interpolation -> wait ~8 ticks -> teleport it, the end of the teleport will look slow due to the server resyncing the display entity position/rotation
HOWEVER I will be honest, I'm not sure what are the side effects of not periodically resyncing entity positions, this could be a Very Bad Idea:tm:
I must say that there is a workaround for this issue... and the workaround is "create a new entity display every 60 ticks". So this PR is mostly to get a "is this ACTUALLY a good idea, or is this ACTUALLY a bug that Paper must fix, or should plugin devs manually workaround this issue?" discussion going. If this is something that plugin devs should manually workaround, I think that it would be a good idea to document that behavior on Paper’s docs on the website (which has a section dedicated to displays) because wow, the only place that somewhat touches on this issue is in the snapshot 23w31a changelog on Minecraft's website and even then they do not go in depth, because they said that they clamp the
teleport_durationvalue to 59 because "this value is clamped to avoid glitches due to periodic position updates"... but that doesn't fix the issue at all?!?!I thought about an alternative implementation that tracks when was the last teleport +
teleport_duration, but that would be way more complex than the current implementation, and, due to circumstances, the entity may not be interpolating on the client when the teleport_duration is set, example: if you set the teleport duration and teleport the entity on the same tick, the teleport_duration would be set but the client would not interpolate it, because you need to force a entity refresh to the client to cause it to interpolate on the same tick.Fixes #11694
This also has the cool side effect that you can actually set teleport duration of display values above >59 ticks and the client interpolates it without any issues. (
(cameraEntity as CraftTextDisplay).handle.entityData.set(net.minecraft.world.entity.Display.DATA_POS_ROT_INTERPOLATION_DURATION_ID, 120))