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

Area/Area2D doesn't detect static bodies if "Monitorable" is disabled (3D issue only when using GodotPhysics) #17238

Open
Tracked by #45334
YeldhamDev opened this issue Mar 4, 2018 · 28 comments

Comments

@YeldhamDev
Copy link
Member

Godot version:
184b2fe

Issue description:
This could also affect the 3D version of it, but I just tested in 2D.

If an Area2D's Monitorable property is disabled, it will fail to detect static bodies.

@Ranoller
Copy link
Contributor

Ranoller commented Mar 4, 2018

http://docs.godotengine.org/en/3.0/tutorials/physics/physics_introduction.html

Static body and area should not report contact

@YeldhamDev
Copy link
Member Author

YeldhamDev commented Mar 4, 2018

@Ranoller Well, then the bug is that is detecting static bodies when Monitorable is enabled then?

@Ranoller
Copy link
Contributor

Ranoller commented Mar 4, 2018

If you trust documentation... Yes

@mhilbrunner
Copy link
Member

Monitorable seems wonky in other cases too: #17330

@rilpires
Copy link

rilpires commented May 14, 2018

I was looking how to detect static bodies with a non-physical colliders and ENABLING MONITORABLE in Area2D is the only way.

It shouldn't even be called a bug since area2D isn't monitorable anyway(except by others Area2D, I know, but it can be handled with collision_mask bits).
So, if Area2D isn't supposed to be detecting static bodies in any way, please let this "bug" continue

@akien-mga
Copy link
Member

See also test project from duplicate issue: https://github.com/godotengine/godot/files/2217245/StaticBodyAreaBug.tar.gz

@fossegutten
Copy link
Contributor

fossegutten commented Jun 20, 2019

Why is this and the referenced issue #20364 closed? Is it intended that area2D shouldn't detect static bodies and tilemaps?
I just encountered this issue in Godot 3.1.1 and its bugging me because I don't understand why it behaves like this. Performance also drops by over 50% if I enable monitorable.

Edit: Sorry @YeldhamDev ! Not so experienced with github..
But it also applies for tilemap, not only staticbody2D

@YeldhamDev
Copy link
Member Author

@fossegutten This issue is currently open, the other one was closed for being a duplicate of this one.

@alastair-JL
Copy link

So.... what is the best way of detecting static bodies?
Like, let say I want a character to jump BEFORE running off a ledged, then I want some kind of detector running in front of them that says when there isn't any ground. I don't want the detector to have any physics in it (hence shouldn't be a body), I want it to only be a zone of detection.

This seems like something that "area2d" should do, or at least, have the ability to do.
If not, then, what type of object should have that behavior.

I'm all for fixing some other thing that is a bug, but ... if you do I'm not sure how to implement the thing I'm imagining.

@AtomaFajrovulpo
Copy link

AtomaFajrovulpo commented Oct 29, 2019

#17238 (comment)
@Ranoller

http://docs.godotengine.org/en/3.0/tutorials/physics/physics_introduction.html

Static body and area should not report contact

Then how would you detect StaticBody2Ds ?

@wingedadventurer
Copy link
Contributor

What's the status on this issue? I'm still experiencing it in 3.2.2 stable.

@andrea-calligaris
Copy link
Contributor

andrea-calligaris commented Oct 3, 2020

Experiencing this in 3.2.3 stable.
It's confusing enough that it took me quite a while to understand what was going on in my project.
In my case I was trying to detect tiles from a TileMap.

@andrea-calligaris
Copy link
Contributor

andrea-calligaris commented Oct 4, 2020

http://docs.godotengine.org/en/3.0/tutorials/physics/physics_introduction.html

Static body and area should not report contact

In my case I was trying to detect tiles from a TileMap.
The documentation for Area2D says:

body_entered(body: Node)
Emitted when a physics body enters.
The body argument can either be a PhysicsBody2D or a TileMap instance (while TileMaps are not physics body themselves, they register their tiles with collision shapes as a virtual physics body).

But it doesn't work unless you switch "monitorable" on, on the detector, which makes no sense.

@madmiraal
Copy link
Contributor

madmiraal commented Oct 4, 2020

The problem is with the way the physics engine is designed. To make things efficient, StaticBodies are not checked if they're colliding with anything, because they don't move. Instead it relies on the other CollisionObjects moving and detecting collisions with it. Similarly, to make things efficient, Areas that are not monitorable are not checked if they're colliding with anything. The problem arises when moving Areas that are not monitorable collide with StaticBodies. Neither is checking for collisions, so nothing is detected.

@YeldhamDev
Copy link
Member Author

@madadam So it's a quirk, rather than a bug? If so, it should be documented.

@madmiraal
Copy link
Contributor

No, it's definitely a bug. It's just not an easy one to fix, because of the way the physics engine was designed.

@andrea-calligaris
Copy link
Contributor

@madadam So it's a quirk, rather than a bug? If so, it should be documented.

It is a bug, please see #17238 (comment)

@andrea-calligaris
Copy link
Contributor

andrea-calligaris commented Oct 5, 2020

The problem arises when moving Areas that are not monitoring collide with StaticBodies. Neither is checking for collisions, so nothing is detected.

No. The problem arises even when Areas that are monitoring collide with StaticBodies, or at least with tiles, in my case. Nothing is detected until your Areas have monitorable On, which doesn't make sense, and that's the bug, for what I'm able to understand.

In other words, to solve the bug, either:

  • nothing should be detected also when Area has monitorable On, but in this case you couldn't detect tiles, so something would have to be done for making tiles detectable by Areas

OR

  • An Area should always detect StaticBodies if the Area has the monitoring parameter On (not the monitorable parameter).

@madmiraal
Copy link
Contributor

@char0xff Thanks. I've corrected the typo.

@pouleyKetchoupp
Copy link
Contributor

As a note, the same things happens in Godot Physics 3D, but not when using Bullet (see #50050).

toasterofbread added a commit to toasterofbread/godot-metroid-engine that referenced this issue Jul 27, 2021
Changed Samus's hurtbox from a StaticBody2D to a KinematicBody2D, because there is a [Godot engine bug](godotengine/godot#17238) which makes Area2Ds unable to detect StaticBody2Ds

Applied the ExArea2D node to several existing scenes
Various bugfixes and optimisations

Added the SamusCannonPosition scene to assist with the WeaponManager and add additional functionality
Used the SamusCannonPosition to enable carrying a chargebeam during an airspark (part of #50)

Added an animation to the SaveStation for when Samus spawns into the room when starting the game (part of #35)
Reorganised the global game settings by renaming the "window" group to "visuals", and moving several settings to more appropriate groups
Added the main_menu_scene_preview setting, which will determine whether the most recent room of each savefile will be loaded and displayed when selecting a savefile in the mainmenu (part of #35)

The Enums singleton now manages setting and getting node groups
Fixed the collision of doors from stopping Samus's momenum when she enters a door at high speed (such as when shinesparking)
Added the can_airspark_while_grounded physics value. It's set to off by default (part of #50).
@barbaros83
Copy link

lol, i was going crazy thinking i did something wrong, turns out its a long standing bug.
is this going to be fixed in 3.x ?
1 of the comments suggests its not a bug, but it definitely is, because the dev should decide what the node should do, not asuming anything.
in my game i have a building that i can place, wich is a staticbody, and the enemy characters are kinematicbody's, with an area for detection, it makes sense the way its set up, and the way you would asume it works, if detection is not desired there are many ways to handle that, put it on a diffrent layer, or disable the body, filter the bodies that enter the area.

@Calinou Calinou changed the title Area2D doesn't detect static bodies if "Monitorable" is disabled Area/Area2D doesn't detect static bodies if "Monitorable" is disabled (3D issue only when using GodotPhysics) Jul 29, 2021
@pouleyKetchoupp
Copy link
Contributor

@barbaros83 I can confirm this is considered a bug. When it's fixed, it will very likely be on both 4.x and 3.x.

@lawnjelly
Copy link
Member

Just looking at fixing this now. Here is a minimum reprod project (there was another earlier but it didn't work well for me.

Just run it using either Godot physics or Bullet, it prints the "Area entered" message with Bullet but not with Godot physics.

Lawn_StaticAreaCollision.zip

@lawnjelly
Copy link
Member

lawnjelly commented Dec 8, 2021

Incidentally to give more info, the reason this behaviour occurred is that the old octree (and hence the BVH up until #55640) is split into "pairable" and "non-pairable" groups.

Pairable

  • Moving bodies
  • monitorable Areas

Non-Pairable

  • Areas
  • Statics

Pairable can pair (detect collisions) with pairable
Pairable can pair with non-pairable
Non-pairable cannot pair with non-pairable

Having non-pairable not collide with non-pairable means that if you move one (say an Area) you can omit a large amount of collision checks. The moment you decide you do want areas to collide with statics, then you lose out on this performance boost completely.

Bullet presumably doesn't suffer from this because it doesn't make the distinction between pairable and non-pairable (or not in the same way that gains efficiency, I haven't examined the bullet code).

So it comes down to this, currently we need to make a decision, and from what I'm hearing here it sounds like users want everything to collide against everything.

In #55640 I've moved away from pairable and non-pairable, instead using a definable number of trees (up to 32, but only 2 are currently used). For each object added it can only go in one tree, but you can then decide which trees it can collide against. I'm still using 2 trees for now for physics, to make the distinction as it may reduce housekeeping, at a cost of having to do collision against 2 trees.

But I have for now set it up so that all objects collide by default against both trees. This makes the decision to solve the "bug" at the cost of this efficiency. Although the increased efficiency would only have occurred before if you had moving areas. For statics there is no loss to making them collide with everything, because, by definition they do not move, and except for placing them initially, all the collide tests will be against them rather than caused by them, which is essentially the same as the current status quo.

We also have the additional ability to specify per-object tree collision behaviour explicitly, which could give much finer control over this kind of thing efficiency wise. Although it is not yet exposed to the user, we have to discuss whether or not we want to do this, as it will probably not be available in bullet.

This is kind of similar-ish in nature to collision layers and collision masks, except done with entire trees. The former is about user control and is highly inefficient. The latter gives slightly less control (because collision masks are NOT reciprocal) but is highly efficient and can speed up the physics significantly.

@TokisanGames
Copy link
Contributor

TokisanGames commented Sep 13, 2022

So it comes down to this, currently we need to make a decision, and from what I'm hearing here it sounds like users want everything to collide against everything.

The GD4 Area3D documentation says: 3D area that detects CollisionObject3D nodes overlapping, entering, or exiting.

So yes, that is what we expect (and how it works in GD3/Bullet). If any PhysicsBody with an enabled CollisionShape and matching layer/mask enters an Area, we should get body entered signals. If any PhysicsBodies collide with other PhysicsBodies with matching parameters, they should collide and send signals.

We also understand that we should be using StaticBodies for immovable objects, RigidBodies for physics movable objects, Kinematic/Character bodies for user movable object, and Areas for detection.

If there's a better way to do it, fine. Us users don't care about or understand pairable or not pairable or the BVH or the other magic you guys are doing under the hood. We just expect the code to work as advertised in the documentation and tutorials, and provides the functionality we need (present in GD3/Bullet). Currently GD4 doesn't quite do that and the documentation is inconsistent.

@lawnjelly
Copy link
Member

Saw a few people had been bumping this and similar bugs. Just to review some options for fixing this (as I remember, it's been a while so my memory may be wonky) for the benefit of new physics contributors such as @rburing :

Imo there are two good options which solve the bug (and don't necessarily adversely affect performance in the situation where this mode is not needed). Both involve the need for an extra flag on Areas, whether or not the Area should collide test against Statics. Or alternatively the monitorable state could become more of an enum:

  • Normal (as before, Areas in the static tree)
  • Collide all (with statics)
  • Monitorable (collide with all and report collisions)

The two ways obvious ways of implementing this:

  1. Keep the same number of BVH trees (2), but alter the physics code so that when collide all is set, the Area is added to the dynamic tree instead of the static tree. This would be more expensive than normal mode, but not as expensive as monitable because there is no reporting.
  2. Add a third BVH tree (mostly similar to the code I already wrote in [3.x] BVH - Separate into dynamic, area and static trees #58505 ) and place collide all Areas into this third tree instead of the static tree. This has the advantage that we could eliminate Area to Area collision checks.

Just to emphasize that we made the BVH totally configurable with the templated trees in #55640 , so it is possible to add / remove trees / configure collision masks without changing the actual BVH code at all (and thus requires no specialist knowledge), and this can be done by a physics contributor as in the approach in #58505 .

@FeldrinH
Copy link

I am not knowledgeable enough about physics engine internals to comment on the technical merits of any choice, but as a user, the idea that 'monitorable' and 'collide all' are two options in the same enum feels weird, since 'monitorable' is about being detectable whereas 'collide all' is about detecting other bodies.

@Zireael07
Copy link
Contributor

@FeldrinH that was my first thought upon reading the comment, too

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.