Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

KinematicBody/2D uses any combination of layer-mask for move_* collision detection #15243

Closed
Tracked by #45334 ...
eon-s opened this issue Jan 1, 2018 · 28 comments · Fixed by #42641
Closed
Tracked by #45334 ...

KinematicBody/2D uses any combination of layer-mask for move_* collision detection #15243

eon-s opened this issue Jan 1, 2018 · 28 comments · Fixed by #42641

Comments

@eon-s
Copy link
Contributor

eon-s commented Jan 1, 2018

Godot version: 3.0 d6a1125, 2.1.4

Issue description:
KinematicBody2D move methods collides against bodies if the other bodies have the kinematic layer on the collision mask, meaning KB is using the layer too, not the mask only for collision resolution.

The expected is for the body to ignore the others that are not in their masks, this issue makes some common setups more complicated than they should, and renders the move_* methods almost useless.

Steps to reproduce:

  • Create a KinematicBody2D on layer 2.
  • Create another body with mask 2.
  • move_and_slide or move_and_collide the KB against the other body.
  • Watch how collides when should not.

Minimal reproduction project:
KBmaskbug.zip

Probably related to #7644

@eon-s
Copy link
Contributor Author

eon-s commented Mar 7, 2018

The same happens with KinematicBody (3D) with any physics engine.

The same project with 3D example:
KBmaskbug-2D3D.zip

@eon-s eon-s changed the title KinematicBody2D uses any combination of layer-mask for move_* collision detection KinematicBody/2D uses any combination of layer-mask for move_* collision detection Mar 7, 2018
@mrcdk
Copy link
Contributor

mrcdk commented Apr 4, 2018

I don't think this is a bug and it makes sense that there's a collision but it's not that the KinematicBody doesn't care about layers/masks when using move_* it's that the other bodies are colliding with the KinematicBody layer. The same thing will happen with the same setup and 2 RigidBodies.

KinematicBodies (or any body) respect the layer/mask setup as long as it makes sense. If you don't want your KinematicBody to collide with anything that's checking layer 2 (its layer 2 mask is enabled) then don't set your KinematicBody layer to 2.

@eon-s
Copy link
Contributor Author

eon-s commented Apr 4, 2018

@mrcdk if body with the corresponding mask is not moving, it cannot collide, if it does it should emit a signal at least.

This also limits some usage of the KB, where you do want to ignore some bodies (not on mask) and obstruct these at the same time while using move.

@mrcdk
Copy link
Contributor

mrcdk commented Apr 4, 2018

if body with the corresponding mask is not moving, it cannot collide,

It doesn't matter if it moves or not. A collision happens between a pair of objects and each object will test their layer/mask combinations because the physics server doesn't know if object A was creating the collision with B or B was creating the collision with A. (AFAIK this is normal in any physics engine it's not a Godot only behavior)

Example:

  • Object A layer = 3 mask = 2
  • Object B layer = 2 mask = 1
  • Pair A-B: A checks if B will collide with an object in layer 3. B isn't checking for collisions in layer 3. No collision.
  • Pair B-A: B checks if A will collide with an object in layer 2. A is checking for collisions in layer 2. Collision.
  • Result: There's a collision between objects A and B

if it does it should emit a signal at least.

I'm not sure what do you mean here. What signal? Who should emit a signal?

This also limits some usage of the KB, where you do want to ignore some bodies (not on mask) and obstruct these at the same time while using move.

Do you mean that you want to collide only when you aren't moving the KinematicBody? In that case I guess you will need to enable or disable the layer/mask bit when moving.

@eon-s
Copy link
Contributor Author

eon-s commented Apr 4, 2018

The body that expects a collision (the one with a mask, that are used for that reason) should emit a signal if hit a body that do not expect one (without mask).

You have a moving wall that ignores a character, but is stopped by other walls, on the current state it cannot use move for collision because will be stopped by the character and disabling layer is not an option in that case because will be ignored by the character.


But if this is the intended behavior, it should be documented (like with areas) and masks may need to be disused for removal in the future, because the usage brings more confusion to users than what they offer (with only layers again it will make more sense).

@mrcdk
Copy link
Contributor

mrcdk commented Apr 4, 2018

You have a moving wall that ignores a character, but is stopped by other walls, on the current state it cannot use move for collision because will be stopped by the character and disabling layer is not an option in that case because will be ignored by the character.

I guessing you have it setup it like (I'm using names here):

  • walls layer = walls mask = character
  • character layer = character mask = walls

Which makes sense. A static wall will collide with a character (although this doesn't matter because a static wall doesn't care about that. It's not actively looking for collisions) and a character will collide with a wall. But you are adding a third variable, a moving wall that will still collide with other static walls and that will not collide with a character. You have two options here:

  • Move the moving wall layer to a different one. Like:
    • walls layer = walls mask = character, moving_walls
    • moving_walls layer = moving_walls mask = walls
    • character layer = character mask = walls
  • Add the character as a collision exception in the moving wall. Like, add_collision_exception_with($Character) and add walls to the moving wall's mask.

The body that expects a collision (the one with a mask, that are used for that reason) should emit a signal if hit a body that do not expect one (without mask).

Using your example, when connecting the body_entered signal of the RigidBody to a script and setting the contact monitoring correctly the signal is fired when the KinematicBody collides with it. Is that what you mean?

But if this is the intended behavior, it should be documented (like with areas) and masks may need to be disused for removal in the future, because the usage brings more confusion to users than what they offer (with only layers again it will make more sense).

I don't think an API is broken or useless and should be removed just because I don't understand how to use it or how it works. Sometimes it's broken and or useless and it's a pain to make sense out of it don't get me wrong, but not in this case. Collision filtering using layers and masks is a common thing in any game/physics engine.

If this needs better documentation or not I'll let others decide.

@eon-s
Copy link
Contributor Author

eon-s commented Apr 4, 2018

Collision exception works but may be too complex in some setups when it could be managed by a one direction detection made by the corresponding mask (and the other body can have its mask too if need 2 direction).

@MightyPrinny
Copy link
Contributor

This seems to still be a problem in 3.1, I'm trying to make a solid character stop the player but the solid character is not supposed to be stopped by the player which is impossible at the moment without using exceptions which I can't really use in this particular case, I already tried every layer and mask combination but nothing works.

@voidexp
Copy link

voidexp commented Jul 21, 2019

This is really annoying. Seems that KinematicBody2D just ignores the expected behavior on collision layer and collision masks. Still present in Godot 3.1.1

@voidexp
Copy link

voidexp commented Jul 21, 2019

diff --git a/servers/physics_2d/collision_object_2d_sw.h b/servers/physics_2d/collision_object_2d_sw.h
index ed5946987..8a11381d2 100644
--- a/servers/physics_2d/collision_object_2d_sw.h
+++ b/servers/physics_2d/collision_object_2d_sw.h
@@ -187,7 +187,8 @@ public:
 
 	_FORCE_INLINE_ bool test_collision_mask(CollisionObject2DSW *p_other) const {
 
-		return collision_layer & p_other->collision_mask || p_other->collision_layer & collision_mask;
+		//return collision_layer & p_other->collision_mask || p_other->collision_layer & collision_mask;
+		return collision_layer & p_other->collision_mask;
 	}
 
 	virtual ~CollisionObject2DSW() {}

Patched the engine for myself meanwhile, please try this one. Seems to work properly.

@eon-s
Copy link
Contributor Author

eon-s commented Jul 24, 2019

@V0idExp I think it is the other way around, p_other->collision_layer & collision_mask, this may need to be tested for performance and interactions between bodies.

That is also used in body pair and area pair so probably the behavior of that method is correct, having the option of getting notified only one on the pair may be better.
It would be interesting to test if it is called twice and reports the same collision, then the patch could be harmless.

@voidexp
Copy link

voidexp commented Jul 24, 2019

I thinked about it this way: object 'this' should check whether it collides with someone and be notified for such cases. And it fits pretty well and intuitevely for cases such as the following.
Imagine you have an asteroid and a bunch of robotized mining drones. Everything driven by KinematicBody2D. You want the asteroids to collide with each other, but ignore the drones completely, on the other hand, drones ignore each other, but want to know when they've reached an asteroid (in their pathfinding state machine, for example). That way, you mark asteroids on layers 1-2 and mask 1, and the drones on layer 3 and mask 2.
So far I haven't encountered issues with this fix, but I have to finish my prototype yet.

@MightyPrinny
Copy link
Contributor

MightyPrinny commented Jul 24, 2019

I would use (collision_mask & p_other->collision_layer) != 0. The way I see it is that if an object has a mask set for example layer 3 and 4, it will collide with any object in layer 3 or 4

@voidexp
Copy link

voidexp commented Jul 28, 2019

The expression proposed above doesn't seem to work, collision signals aren't triggered on objects which I expect them to be, and weird stuff happens, although I can't see right now why :|
The function test_collision_mask(CollisionObject2DSW *p_other) is used as one of checks to fill an array of potential collision objects to check against, it could be that it's not called for all existing objects.
I have to investigate this more.

@soccerob
Copy link

soccerob commented Nov 16, 2019

@V0idExp I think this might be the issue that I’ve run into. I was prototyping a tower defense game where all the enemies are kinematic bodies, on layer 1 and mask set to layer 2. when I spawn the enemies and they overlap one another, my collision pairs go through the roof and my frame rate drops because of the number of collisions that are being checked, even though they should be ignoring each other.

did your solution above from July 21 seem to correct the issue for you? I’m really enjoying godot but started looking at other engines when I encountered this issue.

@voidexp
Copy link

voidexp commented Nov 16, 2019 via email

@Eoin-ONeill-Yokai
Copy link
Contributor

@V0idExp You might want to repost your patch on Godot Proposals (or submit a PR here, either way.)

Frankly, the original behavior seems more like a bug than a feature, so it's probably worth trying to push this upstream.

@soccerob
Copy link

I agree that it seems like a bug, but maybe they would consider adding an option to the project settings to allow you to choose which behavior you want for your project.

@Eoin-ONeill-Yokai
Copy link
Contributor

I think a single flag within the kinematic body would be good enough (only accessible through gdscript so that it weeds out people who don't know what they're doing?)

@voidexp
Copy link

voidexp commented Apr 12, 2020

Apprently, in 3.2.1 the bug does not reproduce anymore.

@eon-s
Copy link
Contributor Author

eon-s commented Apr 13, 2020

@V0idExp still happens between KinematicBody2D bodies, but the behavior with rigids is different (it does not stop but move the rigid instead) which makes everything less consistent now.

@eon-s
Copy link
Contributor Author

eon-s commented Apr 13, 2020

I have made another test to be sure, and the change of behavior with rigids is due to infinite inertia being default on true, if it is set to false, the KB still stops against a rigid body that should not be detected (even sleeping bodies).

@voidexp
Copy link

voidexp commented Apr 13, 2020 via email

@voidexp
Copy link

voidexp commented Apr 15, 2020

The question is: if the KB is to ignore the RB, but the RB mask includes the KB layer, should the RB be moved by the KB?

It would be probably a flexible feature, since you can have a projectile KB being shot and just place it on "projectiles layer", and all enemies will have that layer in their mask to be affected by it, without the need of the KB to mark explicitly the enemies layer... but that requires some invasive changes of code and I'm not sure that's easily doable, and probably, breaks a lot of existing code.

@voidexp
Copy link

voidexp commented Apr 15, 2020

I did some research on the workings of the Godot's 2D physics (which roughly, mimics the 3D counterpart, down to the structure, class names and most of the collision checking code): godot_physics_2d.md.pdf

I have to agree with @mrcdk , there's no simple way to exclude a collision check of a CollisionObject2D (being it a kinematic, rigid or static body or a collision area) just one way, so it is not notified about the collision, but the collision effects to be still propagated to objects which are interested in being notified about.

One can always ignore the result of move_and_collide() or move_and_slide() call and continue nonetheless, by performing an after-check of the masks in code, to not stop the KinematicBody2D.

I'd close this one as not a bug.

@voidexp
Copy link

voidexp commented Apr 15, 2020

Same for #7644

@MightyPrinny
Copy link
Contributor

I don't think it ever was a bug, just an unwanted behavior for many, choosing to keep the original less flexible design doesn't sound good to me.

@Thenend
Copy link

Thenend commented Jun 5, 2020

Then state this behaviour clearly in the documentation please. The current text, clearly says that the body would be ignored if it's not in the layer checked in the mask.

collision_mask
This describes what layers the body will scan for collisions. If an object isn’t in one of the mask layers, the body will ignore it. By default, all bodies scan layer 1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants