More accurate ground status translation.#5481
Conversation
onebeastchris
left a comment
onebeastchris
left a comment
There was a problem hiding this comment.
Thanks for the PR! PRs related to player movement are very appreciated :)
| } | ||
|
|
||
| // Due to how ladder works on Bedrock, we won't get climbing velocity from tick end unless if you're colliding horizontally. So we account for it ourselves. | ||
| boolean onClimbableBlock = session.getTagCache().is(BlockTag.CLIMBABLE, session.getGeyser().getWorldManager().blockAt(session, entity.getPosition().sub(0, EntityDefinitions.PLAYER.offset(), 0).toInt()).block()); |
There was a problem hiding this comment.
What would happen here if a block is marked as climbable, but isn't on Bedrock?
There was a problem hiding this comment.
I don't think that will actually happen, but if it were to happened I guess I can remove the horizontalCollision part
since it is not needed in this case anyway and it should be fine since the player is "jumping" or atleast pressing jump so it will prob still correct.
There was a problem hiding this comment.
I don't think that will actually happen, but if it were to happened I guess I can remove the horizontalCollision part since it is not needed in this case anyway and it should be fine since the player is "jumping" or atleast pressing jump so it will prob still correct.
Impressive work there Oryxel, would any of this issue also apply to when players are mounted on a horse and jumping it? We've long had an issue where Bedrock players outperform Java players when they are racing on horses.
|
Pushed the requested changes. |
|
One thing I'd like to point out is that if player is colliding on the y axis while taking velocity the onGround status might be wrong because the velocity we should use to check here is not the last tick end but the velocity that player received. You could use NetworkStackLatencyPacket to keep track of what velocity player should be taking to improve the current implementation but I won't be doing that since rn there is another PR that making changes to NetworkStackLatencyTranslator anyway #5460 |
3 participants
Does what it said, this issue was previously mentioned by cotander (@SamB440) in a thread on the main discord server, this is his video showing the issues https://youtu.be/px0tzkzOzMQ
There is 2 main problems causing this:
Geyser is using the packet.getDelta() which is inaccurate since VERTICAL_COLLISION is the result of last tick delta after applied collision.
Ex: This is easily reproduceable by jumping with a block two blocks above you (again mentioned by cotander) since when we collide our y velocity got reset and then applied gravity (-0.0784) which is what the client send this tick -0.0784 < 0 and VERTICAL_COLLISION is true so Geyser thinks that player ground is true but VERTICAL_COLLISION is actually use the last tick delta to calculate stuff (aka around 0.3332 or 0.248136) which means Geyser is going to be wrong!
Climbing ladder being wrong is actually caused by Bedrock movement differences, if it acts the same as Java the y tick end we received should be 0.2 but it acts differently, it actually applied the 0.2 motion BEFORE collision got calculate, then when we're colliding our y motion got reset to 0 again (since we're are colliding on y axis) then applied gravity -0.0784 and then it send the server that value, therefore it never lets server know the 0.2 motion exist, I won't talk too much about this, I already documented this here (https://github.com/Oryxel/Boar/blob/new-engine/DIFFERENCES_WIKI.md#climbing-ladder) which should explains why this is the case (this should be accurate since it is according to BDS).
This is fixed by use the right tick end velocity and accounting for both jumping and climbing ladder at tick start.