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

Monitoring Area2D checks for any layer-mask and mask-layer for area enter detection #7644

Closed
eon-s opened this issue Jan 25, 2017 · 25 comments
Closed

Comments

@eon-s
Copy link
Contributor

eon-s commented Jan 25, 2017

Operating system or device - Godot version:
Tested on Godot 2.1.1 stable and 2.1.2 stable
I was confused trying to understand the issue but will try to be clear...

Issue description:
A monitoring area detects other areas by looking at any mask-layer and layer-mask match, not mask against layer only.

Steps to reproduce:
Make an impossible situation for detection:

  1. Create a monitoring area on layer 1.

  2. Create a monitored area on mask 1.

  3. Create a signal on monitoring area to detect areas.
    arealayerbug1

  4. Move it over the monitored and area_enter will be triggered even if does it not have masks.
    arealayerbug2

The monitoring area should only look for matching its masks with other monitored layers, not the layers against other's masks.

Link to minimal example project:
AreaEnterLayerBug.zip
Arrows to move left and right

@NiclasEriksen
Copy link

#7589 I guess it's broader than just raycasts.

@eon-s
Copy link
Contributor Author

eon-s commented Jan 25, 2017

@NiclasEriksen yeah, mentioned something on that post before fully understanding this issue, still not sure where the problem is (if just areas or something else on the server), and need to test more collision objects.

@pwnSquirrel
Copy link
Contributor

Can confirm. Additionally the monitoring one also reports when monitoring is disabled at all (it is sufficient to have monitorable enabled on the other one).

@bojidar-bg
Copy link
Contributor

I'm not sure which of the two issues should be closed (and the other repurposed), since it currently looks like it is the same problem to me. WDYT?

@pwnSquirrel
Copy link
Contributor

pwnSquirrel commented Jan 26, 2017

I don't think that they are completely the same problem, because the RayCast issue only affects the 3D raycast. I was not able to reproduce #7589 for the Raycast2D node. It just works fine. On the other side this issue here probably affects 3D areas too (didn't test that). So for 2D only this issue matters, for 3D both do.

EDIT: Yes, Areas in 3D suffer from the same problem it seems. The monitoring switch works though (unlike in 2D as I mentioned above)

@RandomShaper
Copy link
Member

RandomShaper commented Jun 2, 2017

  • Regarding the collision layers/mask thing, I recall the docs telling about how to expect them to work but I can't find that anymore. Anyway some users (like me) haven found a way of understanding them; for instance, https://godotengine.org/qa/3020/collision-masks-and-its-propper-uses?show=3047#a3047
    I assume that monitoring areas detect monitorable areas and receive signals for that, yet the collision layers/mask are still checked two-ways.
    So when monitoring/monitorable are involved this is even more subject to each one's interpretation. Is something in the docs about that? If not, I think that shoudn't be treated as a bug but at most a bad design decision. It'd be hard to change something of such importance at this point, but for 3.0 it can be re-thought if enough users find it counterintuitive.
  • About the monitoring issue with 2D Areas, please try Fix multiple 2D & 3D physics issues (2.1) #8999 and tell me if you still get wrong results so I can add a fix for that to it.
    - On the RayCast(2D) issue, I've checked the code and both 2D & 3D check their layer mask against other objects' layer masks. Does this match your observations? I've seen no difference in code between 2D and 3D ray casting. Raycast to Area not adhering to Layer/Mask #7589 fixed (in terms of mask).

@eon-s
Copy link
Contributor Author

eon-s commented Jun 2, 2017

@RandomShaper it could be bad design but sometimes you want to have a set of states that will make areas detectable when they should not interact.

IMHO, monitored should not be detected by monitoring if the former layer is not in the latter mask, because that is what monitoring status means and layer/masks are for (I have not tested if space override is affected by layer/mask pairs too).

The "always monitoring" seems to be gone on 2.1.3 already.

@RandomShaper
Copy link
Member

RandomShaper commented Jun 2, 2017

I meant bad design decision on the Godot creators' part, not yours...

Or at least there is a lack of documentation that states clearly how the masks and monitoring stuff works. If that was written, everyone could obtain reliable knowledge about how these work instead of having to guess how they intended them to work.

EDIT:
I hope no one gets me wrong on this: Godot creators know a lot about games and have created a great engine and tool. I'm only talking about the conflict between how things work and how people interpret them and that maybe there is a better point of balance between them on this matter.

@eon-s
Copy link
Contributor Author

eon-s commented Jun 2, 2017

I hope the Area and other CollisionObject features get more clear before 3, so they can get proper fixes.

@eon-s
Copy link
Contributor Author

eon-s commented Nov 13, 2017

Affects master too.

Project for Godot 3:
AreaEnterLayerBugv3.zip

@menip
Copy link
Contributor

menip commented Apr 1, 2018

This is still present in 098c7ba. This behavior caused me a lot of trouble while learning how layers and masks are supposed to work. I intuitively expect that an object would live in it's layers, and see stuff in it's mask. While I feel I understand how the current implementation works, I'm not sure as to why it was made that way.

@eon-s
Copy link
Contributor Author

eon-s commented Apr 1, 2018

I do not know how the area overlap is made, looks like one check adds both as overlapping without checking the layer-mask situation of the second one.

This overlap solve bug affects the result obtained by get_overlapping_areas too.

This bug also hurts any simple optimization attempt by a correct layer-mask design (signals get fired when should not).

@eon-s
Copy link
Contributor Author

eon-s commented Apr 2, 2018

Tested on Spatial areas and the issue is the same, if this buggy behavior is the intended one then should be documented as an exception to the layer and mask rules or general limitation of the area nodes (the same for kinematic bodies in the related issue).

SpatialAreaEnterLayerBugv3.zip

@mrcdk
Copy link
Contributor

mrcdk commented Apr 4, 2018

I don't think this is an issue and it's the correct behavior.

Let's clarify some points:

  • monitoring means that the object will actively look for stuff to overlap.
  • monitorable means that if anything is actively looking for stuff it will announce itself to them.
  • Layer and masks aren't a one way contract, both objects' layers and masks will be used to determine if there's a collision/overlap or not.

So, Area A which is monitoring (actively looking for stuff) is in layer 1 and Area B which is monitorable (if anyone is looking for it, it will say something) is checking for anything in layer 1. When they overlap Area A says "hey, is there anyone here? I'm in layer 1", Area B thinks "wait, that dude is in layer 1 and I'm looking for stuff in layer 1" so it says "Yeah! I'm here! I was looking for anything in layer 1" and that's why the overlap was announced to everyone.

If, let's say, Area B weren't checking for anything in layer 1 but only checking for stuff in layer 2 then there wouldn't be an overlap because Area B would had thought "this dude is in layer 1 but I'm only checking for stuff in layer 2"

@eon-s
Copy link
Contributor Author

eon-s commented Apr 4, 2018

Then masks are useless...

And that should be documented because http://docs.godotengine.org/en/3.0/tutorials/physics/physics_introduction.html#collision-layers-and-masks says:

  • 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.

I'll ping @cbscribe @mhilbrunner if this is a docs issue then.

@MutantOctopus
Copy link

I recently came into this same issue and I have to agree with @eon-s -- If signals get fired off by both parties, when only one of them is supposed to care about a given thing, then it severely limits the utility of layers and masks - let alone how non-intuitive it is for the developer.

Let's even ignore the monitoring/monitorable part of the equation and just focus on the layer + mask part. If an Area A enters another Area B, the only time I want B to react is if A is something it's been told to look for - whether or not A is looking for B. As it stands it just requires additional custom logic to ignore something that seems like should already be ignored by the very nature of the layer/mask system.

@mhilbrunner
Copy link
Member

Not sure what the behaviour here should be. Also, I'm assuming this is still valid for 3.0/master?

@mhilbrunner mhilbrunner added this to the 3.1 milestone Jul 3, 2018
@eon-s
Copy link
Contributor Author

eon-s commented Jul 3, 2018

@mhilbrunner yes, still valid for all branches.

@MutantOctopus
Copy link

@mhilbrunner - while obviously there are some complications with consistency when it comes to PhysicsBodies, for Areas, at least, I feel it would be best if a given Area only raises a signal when an overlapping body/other Area shares a Layer bit with one of the Area's Mask bits. That is to say, if A is on layer 1 and is looking for layer 2, and B is on layer 2 and looking for layer 3, then only A would raise a signal when the two overlap (B's layer is part of A's mask) while B would stay silent (A's layer is not part of B's mask).

@akien-mga
Copy link
Member

Moving to the next milestone as 3.1 is now feature frozen.

@akien-mga akien-mga modified the milestones: 3.1, 3.2 Sep 15, 2018
@Nolic0321
Copy link

While the monitoring/monitored bits add more confusion to the matter I believe that the example of the original issue is valid. I'll also bet that the monitoring/monitored flags are there to "easily" turn on and off monitoring instead of trying to figure out bit ids using set_collision_layer/mask_bit(int,bool). I ran into this trying to figure out an issue that is somewhat related but just wanted to add some extra notes.

To confirm I agree with @MutantOctopus (assumed conclusion) that the monitor bits should probably be removed unless there is a large part of the community that agrees trying to turn specific layer/mask bits is "too hard". I think that the sheer fact that we can set the bit to true/false assumes responsibility of whether it is monitored or monitoring

@akien-mga
Copy link
Member

Feature and improvement proposals for the Godot Engine are now being discussed and reviewed in a dedicated Godot Improvement Proposals (GIP) (godotengine/godot-proposals) issue tracker. The GIP tracker has a detailed issue template designed so that proposals include all the relevant information to start a productive discussion and help the community assess the validity of the proposal for the engine.

The main (godotengine/godot) tracker is now solely dedicated to bug reports and Pull Requests, enabling contributors to have a better focus on bug fixing work. Therefore, we are now closing all older feature proposals on the main issue tracker.

If you are interested in this feature proposal, please open a new proposal on the GIP tracker following the given issue template (after checking that it doesn't exist already). Be sure to reference this closed issue if it includes any relevant discussion (which you are also encouraged to summarize in the new proposal). Thanks in advance!

@madmiraal
Copy link
Contributor

Reopening, because this is a bug not a feature proposal.

@sttifer
Copy link

sttifer commented Apr 2, 2021

Hey i'm having a problem:
When A enter in area B, but B is not in area A, both has signal trigered!

@pouleyKetchoupp
Copy link
Contributor

Fixed by #51801.

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.