godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
90.56k stars 21.08k forks source link

Mouse event handling doesn't work correctly for Area2D when overlaping ColorRect is on CanvasLayer with lower index #84796

Open p10tr3k opened 11 months ago

p10tr3k commented 11 months ago

Godot version

v4.1.3.stable.official [f06b6836a]

System information

MacOs 14.1 (intel)

Issue description

I've encountered a potential issue with mouse event handling when using a Sprite with Area2D and CollisionShape2D in conjunction with a CanvasLayer and ColorRect. The setup is as follows:

Expected behavior: The input_event on the Area2D should trigger for all mouse events, regardless of the presence of the ColorRect in the CanvasLayer.

Current behavior: When the ColorRect is added to the scene:

Workaround: Setting the Mouse Filter property of the ColorRect to Pass allows the Area2D input_event to receive left and right mouse button events.

Steps to reproduce

Create a scene with a Sprite that includes Area2D and CollisionShape2D. Connect an input_event signal on the Area2D. Add a CanvasLayer with an index of -1. Place a ColorRect in the CanvasLayer. Observe that left and right mouse click events on the Sprite do not trigger the input_event, while mouse wheel events do.

Minimal reproduction project

area2dButtonEvent.zip

KoBeWi commented 11 months ago

It's because of mouse_force_pass_scroll_events property.

p10tr3k commented 11 months ago

You're correct, however, this behavior seems somewhat counterintuitive. If the CanvasLayer's index is set to a lower value, one would logically expect that the ColorRect shouldn't act as if it's positioned above the Area2D. This inconsistency in layering and event handling merits further investigation.

Deozaan commented 11 months ago

Sorry, I'm not currently able to check at my computer, but I'm pretty sure I've seen a warning in the inspector recently when editing something's z index, saying it only affects render order, not input/mouse event order. So I think this is expected behavior.

Daniel-The-Fox commented 11 months ago

This issue lost me 4 hours of debugging until I found a 2 year old reddit post pointing me in the right direction. After then googling for "Godot ColorRect input_event" I found a 4 year old answer to this question on stack overflow which saved the day for me. It seems, this topic, or better pitfall, has been around a while now. 😞

In conclusion, I believe the default value of mouse_filterfor ColorRectshould be changed to Ignore or Passion in order to save other (new) users from this kind of headache. I would not have expected from a rectangle box I use as a simple background to block input_event signal calls for all other objects in the tree. 😭 🀯 😲

In addition, the docs on ColorRect should include a warning on this... πŸ“– The docs of the parent object type Control state:

[...] Sets mouse_filter to MOUSE_FILTER_IGNORE to tell a Control node to ignore mouse or touch events. You'll need it if you place an icon on top of a button. [...]

Well, this doesn't seem to be the case for ColorRect, which would be desirable! πŸ˜‰ En contraire, the default seems to be MOUSE_FILTER_STOP! 😭 🀯

Deozaan commented 11 months ago

I just tested this using the MRP and as soon as I changed the ColorRect's Z Index this warning appeared in the scene tree:

Godot - ColorRect Z Index Warning

So there is "built-in documentation" giving a clue about how this works.

But I want to clarify that I'm not arguing against improving the documentation or changing the behavior. I'm just stating that there already is some indication that things work this way. Perhaps the documentation or behavior could be improved/changed. In fact, when I first noticed this little warning recently, it was with some annoyance because I couldn't find a way to dismiss it so it didn't appear in the scene tree.

πŸ±β€πŸ‘€ EDIT (while you were typing up your reply):

Sets mouse_filter to MOUSE_FILTER_IGNORE to tell a Control node to ignore mouse or touch events.

And to me, this looks like a typo in the docs. I think it is giving an instruction to "set it to MOUSE_FILTER_IGNORE if you want the Control node to ignore mouse or touch events." I don't think it is saying that the default value for all controls is MOUSE_FILTER_IGNORE. In other words, "Sets" should say "Set" instead.

Daniel-The-Fox commented 11 months ago

I just tested this using the MRP and as soon as I changed the ColorRect's Z Index this warning appeared in the scene tree:

Godot - ColorRect Z Index Warning

So there is "built-in documentation" giving a clue about how this works.

But I want to clarify that I'm not arguing against improving the documentation or changing the behavior. I'm just stating that there already is some indication that things work this way. Perhaps the documentation or behavior could be improved/changed. In fact, when I first noticed this little warning recently, it was with some annoyance because I couldn't find a way to dismiss it so it didn't appear in the scene tree.

Thanks, for getting back to this! πŸ‘πŸ» I don't get the warning you are referring to. Here's my node tree: πŸ€” 😲

Bildschirmfoto 2023-11-14 um 20 14 03

Is it maybe, because I have a different order of the nodes? πŸ’­

Apart from that, the warning you show doesn't make it fully clear that it blocks input_event signals for all other nodes/objects in the tree... 😒

Daniel-The-Fox commented 11 months ago

Sets mouse_filter to MOUSE_FILTER_IGNORE to tell a Control node to ignore mouse or touch events.

And to me, this looks like a typo in the docs. I think it is giving an instruction to "set it to MOUSE_FILTER_IGNORE if you want the Control node to ignore mouse or touch events." I don't think it is saying that the default value for all controls is MOUSE_FILTER_IGNORE. In other words, "Sets" should say "Set" instead.

Well that's a whole different story, agreed! πŸ‘πŸ» I can open a PR for the respective docs page, if you like? I would also add a 2nd hint reg. the value "Pass". And add both hints then also to the docs page for "ColorRect". Even if a default of "Stop" is questionable imho, at least the docs point you in the right direction then... πŸ˜‰

Deozaan commented 11 months ago

I don't get the warning you are referring to. Here's my node tree: πŸ€” 😲 Is it maybe, because I have a different order of the nodes? πŸ’­

It doesn't seem to matter how I order my nodes, the ColorRect (and other control nodes) shows me the warning if the Z Index is anything but 0.

Godot ColorRect Z Index

I even replicated your scene tree layout:

Godot Control Z Index Warnings

Are you using 4.1.3? Maybe it's a new feature. I never noticed it until recently. But I'm also not sure I've ever changed the Z Index of a control node until recently.

Even if a default of "Stop" is questionable imho, at least the docs point you in the right direction then... πŸ˜‰

Each control node has its own default mouse_filter setting. Some block by default. Some pass or ignore by default. It depends on the node and what it's usually used for. But the default for each control node is usually what you would want it to be under normal circumstances, or at least that's the intent. But that also means it can lead to some confusion and hunting down why mouse events aren't working as expected. I learned early on to "drill down" (or up) the scene tree to check each control node's mouse_filter setting if mouse events were not propagating properly in my project.

I can open a PR for the respective docs page, if you like?

Sounds good to me. πŸ˜…

Daniel-The-Fox commented 11 months ago

Yes, I'm using Godot version 4.1.3.

It doesn't seem to matter how I order my nodes, the ColorRect (and other control nodes) shows me the warning if the Z Index is anything but 0.

You're right, I didn't noticed that as I've never changed the Z Index of a ColorRect so far. I'm using Godot for about 2 weeks now, so there's still a lot of things to learn and special cases to discover for me 🀣😝

But the default for each control node is usually what you would want it to be under normal circumstances, or at least that's the intent.

So what's the intent of blocking mouse input events for all other nodes/objects in the tree when adding a ColorRect? πŸ˜›πŸ˜‰ What's the use case covered in doing so by default for ColorRect? πŸ€”

I can open a PR for the respective docs page, if you like?

Sounds good to me. πŸ˜…

See PR https://github.com/godotengine/godot/pull/84939 πŸ˜‰

Deozaan commented 11 months ago

So what's the intent of blocking mouse input events for all other nodes/objects in the tree when adding a ColorRect? πŸ˜›πŸ˜‰ What's the use case covered in doing so by default for ColorRect? πŸ€”

Sometimes multiple UIs are overlaid on top of each other. If you used a ColorRect as the background of a UI that was shown on top of another UI (for instance, to make the background UI appear faded out or darkened), you wouldn't want errant clicks to go through the front UI into the buttons and other control nodes behind it.

Or someone could use a ColorRect in the foreground to handle screen fade in/out transitions. In which case you probably wouldn't want clicks to go through while you're fading in or out.

I think the most common reason a person would use a ColorRect would be as a background or foreground in a similar manner. Either way, it seems like a sensible default to me to block mouse event propagation.

Daniel-The-Fox commented 11 months ago

So what's the intent of blocking mouse input events for all other nodes/objects in the tree when adding a ColorRect? πŸ˜›πŸ˜‰ What's the use case covered in doing so by default for ColorRect? πŸ€”

Sometimes multiple UIs are overlaid on top of each other. If you used a ColorRect as the background of a UI that was shown on top of another UI (for instance, to make the background UI appear faded out or darkened), you wouldn't want errant clicks to go through the front UI into the buttons and other control nodes behind it.

Or someone could use a ColorRect in the foreground to handle screen fade in/out transitions. In which case you probably wouldn't want clicks to go through while you're fading in or out.

I think the most common reason a person would use a ColorRect would be as a background or foreground in a similar manner. Either way, it seems like a sensible default to me to block mouse event propagation.

I understand and agree with the use cases, and they all have one thing in common: The game dev wants to block inputs to objects behind/below the ColorRect.

I added a ColorRect as the background of my intro screen which I blend in via show()in my main scene. In the respective GDScript for the main scene, I then add a child node to the linked intro screen scene and a message to shoot the enemy (CharacterBody2D) spawned in order to continue. With the default setting for mouse_filter set to Stop the enemy spawned (on top of the ColorRect, same Z index) doesn't receive any input event actions (like the "left_click" I defined in the Input Map of the project settings) at all.

Is it really the desired default behaviour to block input events for all objects in the same tree as a ColorRect regardless if they're placed behind or in front of it?

Just in case, this is how my use case looks like:

Bildschirmfoto 2023-11-16 um 15 30 07

If I want the spawned enemy (scene) to receive any input events (like "left_click" defined in Input Map), I have to set mouse_filter of BackgroundBox (the ColorRect) to Ignore or Pass although the ColorRect is the first child in the node tree and therefore the "lowest". For me as a fairly new user, this was very unexpected and disturbing and cost me hours of debugging...

Deozaan commented 11 months ago

I'm sorry for your frustration and lost time. Being new to anything will often result in surprises. Some of them will be pleasant, and some of them will be unpleasant.

If the default for a ColorRect was to Ignore or Pass mouse events then there would be many more people creating bug reports about how being able to interact with UI behind it was unexpected and would cost even more hours of debugging or creating workarounds. I think the goal is to have sensible defaults that work for people 90% of the time. But if you're doing something outside of the norm, there's a simple setting you can change so that it allows mouse events to go through.

As for your specific setup: I'm honestly not sure how Control nodes should interact with Node2Ds. I don't really mix them like that. I may add Control nodes as children of Node2Ds, or place Controls as children of a CanvasLayer which is a sibling/child of Node2Ds, but I've never put full-screen Controls as siblings/parents of Node2Ds like you have. It doesn't make sense to me to organize them that way. In my opinion, this is a case of doing something outside the norm.

As an aside, because you've stated that you're relatively new I also feel I should make it clear to you that I'm just some random person who is trying to be helpful. We're both learners. I'm not a core developer, and there's plenty about Godot that I don't know. In other words, I'm not in a position of authority here, nor am I an expert--in fact, my understanding of things may be objectively/technically wrong--and my opinions are just opinions.

So if you feel strongly that something needs to change about Controls or ColorRects specifically, it may be a good idea to first create a proposal or open a discussion and get some consensus on how to proceed in a way that benefits the most users most of the time, and then either implement the change yourself by submitting a Pull Request, or wait patiently in the hopes that someone else will implement it.

Daniel-The-Fox commented 11 months ago

I'm sorry for your frustration and lost time. Being new to anything will often result in surprises. Some of them will be pleasant, and some of them will be unpleasant.

Thanks for the kind words! πŸ‘πŸ»

If the default for a ColorRect was to Ignore or Pass mouse events then there would be many more people creating bug reports about how being able to interact with UI behind it was unexpected and would cost even more hours of debugging or creating workarounds. I think the goal is to have sensible defaults that work for people 90% of the time. But if you're doing something outside of the norm, there's a simple setting you can change so that it allows mouse events to go through.

True and the more I think about it, I guess the issue lies in the way mouse_filter works. It's an all or nothing. It would be nice if it would also consider the layers/hierarchy in the node tree. πŸ€”

As for your specific setup: I'm honestly now sure how Control nodes should interact with Node2Ds. I don't really mix them like that. I may add Control nodes as children of Node2Ds, or place Controls as children of a CanvasLayer which is a sibling/child of Node2Ds, but I've never put full-screen Controls as siblings/parents of Node2Ds like you have. It doesn't make sense to me to organize them that way. In my opinion, this is a case of doing something outside the norm.

So if I would've used a Button (inherits from Control AFAIR) instead of a CharacterBody2D, it would've worked, even with a default value of Stop for mouse_filter on the ColorRect? Interesting, I'll try that within the next days... πŸ’­

I do agree that re-using an instance of my enemy scene as a button to click on is out of norm. Cool idea heh? 😜

As an aside, because you've stated that you're relatively new I also feel I should make it clear to you that I'm just some random person who is trying to be helpful. We're both learners. I'm not a core developer, and there's plenty about Godot that I don't know. In other words, I'm not in a position of authority here, nor am I an expert--in fact, my understanding of things may be objectively/technically wrong--and my opinions are just opinions.

All good! I appreciate the feedback and the discussion! Many thanks! πŸ‘πŸ»

So if you feel strongly that something needs to change about Controls or ColorRects specifically, it may be a good idea to first create a proposal or open a discussion and get some consensus on how to proceed in a way that benefits the most users most of the time, and then either implement the change yourself by submitting a Pull Request, or wait patiently in the hopes that someone else will implement it.

Thanks, I wasn't aware that only real bugs should go here and for anything else there's a different ticket system (= GitHub repo with completely separate issues). 😲 I'll keep that in mind before opening another issue then. πŸ‘πŸ»

@p10tr3k I guess it's now up to you to decide how you want to continue with the issue you opened... πŸ˜‰

novalis commented 8 months ago

This seems to be somewhat buggy. Consider this attached scene:

control-area.zip

Here, an Area2D covers a hboxcontainer containing a button. The hboxcontainer's mouse filter is set to ignore, and the button is set to pass (it has to be pass not ignore so that the button can still be pressed). But there seems to be a slight seam right next to the button, so when you enter the button, you briefly exit the area before immediately re-entering it. Notice when your mouse enters the button, you see "exit" immediately followed by "enter".

Even if it weren't buggy, it would be immensely irritating to have to carefully set all my mouse filters, but since it is buggy, that's not even an option.

What I think I would really like is to have a mouse z index.