Beds and slabs in low rooms can get you stuck, but only across y = 0 #6197
Comments
|
This issue needs to be moved to https://github.com/minetest/minetest_game. |
|
@Wuzzy2 this is where I came from in the first place. I got sent here by @SmallJoker. Please see the link at the end of my bug description. |
|
@Wuzzy2, I requested to move this issue as it addresses the collision detection, which is a core thing. MTG's beds only trigger this behaviour. |
SmallJoker
added
@ Server / Client / Env.
Bug
|
I just found out that the bug can be triggered only near sea level where ceiling and ground Y coordinates have different (math.) sign. I edited my bug description above to describe the precise conditions. |
|
Happens for fancy and simple beds? |
|
It previously happened for both kinds of beds on Dark Lands. |
|
Tested and confirmed it's at those two specific heights only. |
|
Confirmed in latest master with new move code (the default). |
paramat
changed the title
|
Happens with a slab of height 0.56 (same height as beds). |
|
Confirmed collision is determined by nodeboxes not selectionbox, which is why the fancy bed head blocks approach. |
|
Also happens in singleplayer 0.4.16 stable so not caused by 'settable player collisionbox' changes. |
|
During experimentation this fixes it: Seems to be a float to int rounding error when collecting nearby nodeboxes, such that the nodeboxes of the low ceiling are not collected. |
|
If i print out max_y it is 1 while the ceiling is at y = 2, so nodeboxes just above the player are not being collected for collision detection. |
|
By printing max_y i confirm that the current code correctly includes the ceiling when both player feet and ceiling are above, or below, y = 0. |
|
I'll try some casts, in my experience converting floats to ints can produce errors when division is involved. |
|
If this only happens across y = 0, it seems to me it must be related to round-towards-zero division or cast behaviour. Consistently rounding towards negative infinity should fix that easily. |
|
With current code printing y_min also: and with player not moving (speed = 0). Across y = 0 i get: Well below y = 0 i get: Well above y = 0 i get: So the volume of nodebox collection is too big by 1 node in almost all situations. Which means we can gain performance by fixing this. |
|
Current code, also printing 'oldpos_i.Y':https://github.com/minetest/minetest/blob/master/src/collision.cpp#L260 Well above y = 0 i get: Across y = 0 i get: Well below y = 0 i get: |
|
I can correct oldpos_i by adding a small value to it's Y component: So it seems that when below y = 0 the player pos '*pos_f' is being rounded to a value 1 node too low by 'floatToInt()'. But how is this possible when 'floatToInt()' was specifically created to avoid this type of error? |
However when doing so running up steps seems different and not as smooth, and movement sometimes seems jittery. |
|
@paramat I think I have a fix, hang on ... |
To determine the area (nodes) where a player movement took place collisionMoveSimple() first took the old/new player coordinates and rounded them to integers, then added the player character's collision box and implicitely rounded the result. This has 2 problems: Rounding the position and the box seperately, then adding the resulting integers means you get twice the rounding error. And implicit rounding always rounds towards 0.0, unlike floatToInt(), which rounds towards the closest integer. Previous (simplified) behavior: round(pos)+(int)box, for example player at Y=0.9, body is 1.75m high: round(0.9)+(int)1.75 = 1+1 = 2. ==> A character's height of 1.75m always got rounded down to 1m, its width of +/-0.3 even became 0. Fixed by adding the floats first, then rounding properly: round(pos+box) = round(0.9+1.75) = round(2.65) = 3.
To determine the area (nodes) where a player movement took place collisionMoveSimple() first took the old/new player coordinates and rounded them to integers, then added the player character's collision box and implicitely rounded the result. This has 2 problems: Rounding the position and the box seperately, then adding the resulting integers means you get twice the rounding error. And implicit rounding always rounds towards 0.0, unlike floatToInt(), which rounds towards the closest integer. Previous (simplified) behavior: round(pos)+(int)box, for example player at Y=0.9, body is 1.75m high: round(0.9)+(int)1.75 = 1+1 = 2. ==> A character's height of 1.75m always got rounded down to 1m, its width of +/-0.3 even became 0. Fixed by adding the floats first, then rounding properly: round(pos+box) = round(0.9+1.75) = round(2.65) = 3.
|
I also suspect another small general problem: the player is always standing exactly on the bondary between 2 nodes e.g. Y=1.5 is exactly between nodes Y=1 and Y=2. floatToInt() and myround() will round this differently depending where you are: rounded up above sea level and rounded down when underground. This inconsistency comes from the way the coordinates are calculated, independent of the specific C++ code. If I understand this right the only negative effect should be a tiny bit of lost performance when moving underground, because 1 node level more than necessary is checked for collisions. This might be amended by adding a tiny offset to min.Y, like you (@paramat) already considered above. I tested the workaround, and it seemed to work, but I didn't want to do it, because it felt ugly and arbitrary. What you you think? |
|
I thought |
|
floatToInt() and myround() round to the nearest integer i.e. +1.3-->+1, -0.7-->-1. In case of +/-x.5 they always round "outwards" i,e, +1.5-->+2, -1.5-->-2. That's absolutely correct and logical for an abstract rounding function.. But in the specific case of a player standing on +1.5 rounding (upwards) to +2 selects the air node containing the player's legs. Whereas with a player standing on -1.5 rounding (downwards) to -2 selects the ground node below his/her feet. Finally we extend the search area with +/-v3s16(1,1,1) ==> above sea level-->ground node-->ok; whereas underground-->even 1 node below ground-->also ok, but more node checking than absolutely necessary. I was thinking about this change (additional to what I already committed) to work around it, but it felt like an ugly hack, so my vote would be NOT to do that and just accept that we might check a few nodes more than necessary, that's no big deal: |
|
Ok, this text seemed to suggest otherwise, but of course we have to go by the code of This is a bit worrying since MT uses floatToInt() heavily for converting float co-ords to int co-ords. I'm not sure about the behaviour of this, as it returns integers from expressions including floats and divisions, i've experienced weird behaviour in these situations. Best to test how it behaves due to negative sign. If your suggested small value is actually needed i think it's worth it, because if we are collecting nodeboxes in 9 too many nodes this can be a lot of extra nodeboxes collected. There may be complex nodes with multiple nodeboxes. The single addition is much more lightweight. |
To determine the area (nodes) where a player movement took place collisionMoveSimple() first took the old/new player coordinates and rounded them to integers, then added the player character's collision box and implicitely rounded the result. This has 2 problems: Rounding the position and the box seperately, then adding the resulting integers means you get twice the rounding error. And implicit rounding always rounds towards 0.0, unlike floatToInt(), which rounds towards the closest integer. Previous (simplified) behavior: round(pos)+(int)box, for example player at Y=0.9, body is 1.75m high: round(0.9)+(int)1.75 = 1+1 = 2. ==> A character's height of 1.75m always got rounded down to 1m, its width of +/-0.3 even became 0. Fixed by adding the floats first, then rounding properly: round(pos+box) = round(0.9+1.75) = round(2.65) = 3.
To determine the area (nodes) where a player movement took place collisionMoveSimple() first took the old/new player coordinates and rounded them to integers, then added the player character's collision box and implicitely rounded the result. This has 2 problems: Rounding the position and the box seperately, then adding the resulting integers means you get twice the rounding error. And implicit rounding always rounds towards 0.0, unlike floatToInt(), which rounds towards the closest integer. Previous (simplified) behavior: round(pos)+(int)box, for example player at Y=0.9, body is 1.75m high: round(0.9)+(int)1.75 = 1+1 = 2. ==> A character's height of 1.75m always got rounded down to 1m, its width of +/-0.3 even became 0. Fixed by adding the floats first, then rounding properly: round(pos+box) = round(0.9+1.75) = round(2.65) = 3.
To determine the area (nodes) where a player movement took place collisionMoveSimple() first took the old/new player coordinates and rounded them to integers, then added the player character's collision box and implicitely rounded the result. This has 2 problems: Rounding the position and the box seperately, then adding the resulting integers means you get twice the rounding error. And implicit rounding always rounds towards 0.0, unlike floatToInt(), which rounds towards the closest integer. Previous (simplified) behavior: round(pos)+(int)box, for example player at Y=0.9, body is 1.75m high: round(0.9)+(int)1.75 = 1+1 = 2. ==> A character's height of 1.75m always got rounded down to 1m, its width of +/-0.3 even became 0. Fixed by adding the floats first, then rounding properly: round(pos+box) = round(0.9+1.75) = round(2.65) = 3.
To determine the area (nodes) where a player movement took place collisionMoveSimple() first took the old/new player coordinates and rounded them to integers, then added the player character's collision box and implicitely rounded the result. This has 2 problems: Rounding the position and the box seperately, then adding the resulting integers means you get twice the rounding error. And implicit rounding always rounds towards 0.0, unlike floatToInt(), which rounds towards the closest integer. Previous (simplified) behavior: round(pos)+(int)box, for example player at Y=0.9, body is 1.75m high: round(0.9)+(int)1.75 = 1+1 = 2. ==> A character's height of 1.75m always got rounded down to 1m, its width of +/-0.3 even became 0. Fixed by adding the floats first, then rounding properly: round(pos+box) = round(0.9+1.75) = round(2.65) = 3.
To determine the area (nodes) where a player movement took place collisionMoveSimple() first took the old/new player coordinates and rounded them to integers, then added the player character's collision box and implicitely rounded the result. This has 2 problems: Rounding the position and the box seperately, then adding the resulting integers means you get twice the rounding error. And implicit rounding always rounds towards 0.0, unlike floatToInt(), which rounds towards the closest integer. Previous (simplified) behavior: round(pos)+(int)box, for example player at Y=0.9, body is 1.75m high: round(0.9)+(int)1.75 = 1+1 = 2. ==> A character's height of 1.75m always got rounded down to 1m, its width of +/-0.3 even became 0. Fixed by adding the floats first, then rounding properly: round(pos+box) = round(0.9+1.75) = round(2.65) = 3.
To determine the area (nodes) where a player movement took place collisionMoveSimple() first took the old/new player coordinates and rounded them to integers, then added the player character's collision box and implicitely rounded the result. This has 2 problems: Rounding the position and the box seperately, then adding the resulting integers means you get twice the rounding error. And implicit rounding always rounds towards 0.0, unlike floatToInt(), which rounds towards the closest integer. Previous (simplified) behavior: round(pos)+(int)box, for example player at Y=0.9, body is 1.75m high: round(0.9)+(int)1.75 = 1+1 = 2. ==> A character's height of 1.75m always got rounded down to 1m, its width of +/-0.3 even became 0. Fixed by adding the floats first, then rounding properly: round(pos+box) = round(0.9+1.75) = round(2.65) = 3.
To determine the area (nodes) where a player movement took place collisionMoveSimple() first took the old/new player coordinates and rounded them to integers, then added the player character's collision box and implicitely rounded the result. This has 2 problems: Rounding the position and the box seperately, then adding the resulting integers means you get twice the rounding error. And implicit rounding always rounds towards 0.0, unlike floatToInt(), which rounds towards the closest integer. Previous (simplified) behavior: round(pos)+(int)box, for example player at Y=0.9, body is 1.75m high: round(0.9)+(int)1.75 = 1+1 = 2. ==> A character's height of 1.75m always got rounded down to 1m, its width of +/-0.3 even became 0. Fixed by adding the floats first, then rounding properly: round(pos+box) = round(0.9+1.75) = round(2.65) = 3.
To determine the area (nodes) where a player movement took place collisionMoveSimple() first took the old/new player coordinates and rounded them to integers, then added the player character's collision box and implicitely rounded the result. This has 2 problems: Rounding the position and the box seperately, then adding the resulting integers means you get twice the rounding error. And implicit rounding always rounds towards 0.0, unlike floatToInt(), which rounds towards the closest integer. Previous (simplified) behavior: round(pos)+(int)box, for example player at Y=0.9, body is 1.75m high: round(0.9)+(int)1.75 = 1+1 = 2. ==> A character's height of 1.75m always got rounded down to 1m, its width of +/-0.3 even became 0. Fixed by adding the floats first, then rounding properly: round(pos+box) = round(0.9+1.75) = round(2.65) = 3.
To reproduce, build a small room which is 2 nodes high. Place a bed in it. So, all nodes counted from bottom to top should be 1x ground, 1x bed, 1x air, 1x ceiling. Then walk towards the bed. Normally the bed blocks you from walking on top of it, because the ceiling is too low for that. BUT, if this is done near sea level then you CAN walk onto the bed and will get stuck with your head inside the ceiling.
To be precise, the bug can be triggered at the 2 possible different heights where ceiling Y>0, but ground (and player) Y<0:
On servers it's quite possible that the ceiling blocks may be protected by someone else so you can't dig yourself out, and that you don't have noclip or teleport permissions either. In this case the only way out is dying. If you've also slept in the bed and respawn there then you're in trouble.
Tested locally with Minetest Game from Minetest 0.4.15 and 0.4.16 on Debian 9, 64 bit, and on a public game server running 0.4.16. Previously minetest/minetest_game#1865.
The text was updated successfully, but these errors were encountered: