KinematicBody2D passing through tile when there is one cell between parallel tiles

[raphaklaus]

Godot version: 3.2.1

OS/device including version: Ubuntu 19.10

Issue description:

KinematicBody2D passes through Tilemap when tiles are just one space from another. Look at this gif:

That doesn't happen when there is more than 1 cell of space.

Steps to reproduce:

1. Create a KinematicBody2D
2. Setup a Tilemap (16x16 tile size)
3. Create two rows of parallel tiles just one space from one another.
4. Set safe-margin to 0.001 (minimal) in the KB2D
5. Set the velocity to be 16 per frame, so it walks snapping to the cell size.
6. Test collisions.

Minimal reproduction project:
https://github.com/raphaklaus/tilemap-godot-issue

[bcsmith2846]

The issue here is that the CollisionShapes from the obstacles and the player are on exactly the same line after forcing 3 same-sized pixel-perfect squares onto a grid that is made out of the exact size of the sprites.

Evidence that this is the issue: I reduced the CollisionShape2D object by 1px and this doesn't happen anymore.

More technical explanation: there is no such thing as a half-pixel, but sometimes math will spit out a non-whole number when doing physics calculations. The engine rounds that pixel the best it can, but part of the player ends up inside the obstacle before you even press a movement key. Then, when calling the move_and_collide(...) function, the physics server thinks the player is already inside the obstacle and just moves it to the next part of the CollisionShape2D (the far wall). Exiting back out is also strange because the player is still in a half-in/half-out state regarding the collision.

This is flirting with not being a bug, it's the engine working as intended when placed in edge-cases. I was playing around with animating collisions and I got myself stuck in a wall. That's not a bug -- it was a poor design choice on my part.

I would recommend rethinking why you need the grids to be so pixel perfect to begin with and see if there isn't another way to solve the problem.

If anyone else would like me to dig deeper into this (like where the pixel rounding is happening), I'd be willing to do so, but I don't think this edge case that I believe to be working correctly warrants more time.

Thank's for submitting the bug @raphaklaus ! Let me know if I can help you further in this one.

[raphaklaus]

If we reduce the collision shape by 1 pixel and increase the safe-margin to 1 pixel, the issue doesn't happen. So it looks like they are handling the AABB differently for the safe-margin property. To be honest, I don't know if this behavior is on purpose or not, but it's confusing nevertheless.

Edit: I am building a grid-based game. AFAIK, KinematicBody2D should handle normally this kind of games, I don't see a reason for the opposite.

[madmiraal]

KinematicBody (2D and 3D) move_and_collide() function uses test_body_motion() to determine how far a KinematicBody can be moved without a collision. (If there is an AABB overlap) `test_body_motion() returns false if no collision is detected when extracting collision information. To extract collision information the body is moved into an unsafe position. The safe and unsafe proportions of the attempted motion are calculated by doing eight iterations of a binary search.

However, there is a difference between what is considered a collision when doing the binary search, and when extracting collision information. When extracting collision information, only penetrations deeper than the test_motion_min_contact_depth (which by default is 0.005 in 2D and 0.00001 in 3D) are considered collisions. In 2D, if the KinematicBody Safe Margin is reduced to its minimum of 0.001 (as is the case in the minimal reproduction project) i.e. less than the 0.005 test_motion_min_contact_depth, the binary search can return a collision that is not reported as a collision when the collision information is extracted. If no collision is detected during the extraction of collision information stage, with the body in an "unsafe" position, test_body_motion() returns false i.e. no collision, even if there is an AABB overlap and a collision is detected during the binary search. This is what allows the KinematicBody with a low Safe Margin to slide along tiles when they first collide on a corner, and the most likely reason the Safe Margin has been reduced.

In addition, before testing whether a KinematicBody can be moved without a collision, test_body_motion() attempts to free a KinematicBody by attempting to move the body the Safe Margin distance away from any nearby CollisionShapes. When the KinematicBody has been allowed to slides between two blocks, test_body_motion() cannot move the body into a safe position, and it is considered stuck.

The problem arises when attempting to move into one of tiles when the KinematicBody is between two tiles. It cannot be moved into a safe position, so It is already considered to be in an unsafe position, but when attempting to extract collision information there is no penetration greater than the 0.005 test_motion_min_contact_depth. If there were, it wouldn't have been allowed to move there in the first place. Unfortunately, in the same way this allows the KinematicBody to slide along a tile, it also allow a KinematicBody to slide into a tile i.e. the problem described in the issue. This does not happen when the KinematicBody is against a single tile, because the body is moved into a safe position first, and collision information is extracted, because the body is moved into an unsafe position which is deeper than the 0.005 test_motion_min_contact_depth.

[Srekel]

@madmiraal I found what's seemingly a bug in Space2DSW::test_body_motion, though I'm not sure if it causes any actual problem.

At the end of the for loop on line 1029 (on 3.2 head), there's a continue. I'm guessing it should be a break? Or maybe it's just there for debugging and it doesn't really do anything bad.

[madmiraal]

@Srekel Actually, you do want it to continue, testing the other Shapes, if there is no collision. However, you're right in the sense that this check does nothing. It's left over from 7ca29bf, when updating collision information was moved out of this loop. Previously, the continue was required to avoid updating collision information when there was no collision.