Add 'mouse_entered', 'mouse_exited' and 'mouse_pressed' to Sprite2D and Polygon2D#114776
Add 'mouse_entered', 'mouse_exited' and 'mouse_pressed' to Sprite2D and Polygon2D#114776Ezequias-Rodrigues wants to merge 36 commits intogodotengine:masterfrom
Conversation
Introduces a virtual _mouse_hit_test method in CanvasItem, overridden in Sprite2D and Polygon2D for accurate mouse interaction detection. Adds mouse_entered, mouse_exited, and mouse_pressed signals to Sprite2D and Polygon2D, and implements input event handling in CanvasItem to emit these signals based on mouse events. This enhances interactivity for 2D nodes by enabling per-item mouse event handling.
Updated CanvasItem::input to use global mouse position and improved mouse enter/exit signal logic. Removed debug print statements and made mouse_inside non-mutable for clarity.
|
what about performance? will this negatively impact nodes that don’t implement this feature? |
|
The input function and _mouse_hit_test will only be called if input processing is set to true for a given node |
Added a check in CanvasItem::input to return early if the item is not visible in the scene tree, preventing unnecessary input processing for hidden items.
This comment was marked as resolved.
This comment was marked as resolved.
I tried to use copilot at first, but got frustrated with it trying to override a inexistent function and decided to do the work myself. At first i tried to change the scene tree because i wasn't understanding how input was being propragated but then i went to canvas item I'll take a better look when i get to home, sorry for the mess, this is my first PR |
|
Please test your code before opening a PR, making sure it compiles first of all but just making sure it actually does what you want it to do is a basic requirement (might have missed something but the code doesn't look like it compiles to me, but it doesn't seem to be tested with that in mind) Did you intend to add the functionality to |
|
So, what makes the code fails is the styling(i took a time to read the logs, GHA / 📊 Static checks / Code style, file formatting, and docs (pull_request) is what is failing) The code actually compiles, i've tested it before with a minimal script. The only thing i haven't tested, because was like the last thing i made before i went to sleep was the addition of the check if the node is visible or not, i had the idea right after reading the comment about performance. And yes, i intended to edit line 2D, but haven't implemented yet because of laziness, if this PR goes thru, i'll take a better look at it(Being totally honest, polygon2d alone solves what i want to my game, i extended it to Sprite2D because seemed natural, and wanted Line2D but this one wasn't so straightforward) |
|
Good then that was just a misunderstanding from the code, I meant there looked like there was some compile issues with the code (not from CI) so I asked But this PR is not really in a good state for being ready it seems to be a work in progress, with all the unrelated changes and formatting |
|
The unrelated file change is just a linebreak/whitespace. I tried changing scenetree to implement this, but after i discovered a better way to do it, i just deleted the function i was writing, and the linebreak/whitespace went unnoticed. |
|
I'll implement this "mouse_picking", good idea. I think i don't quite understand what you mean in your second point. Isn't it expected that the input is "consumed", and ignored if it was already "consumed" by something up in the propagation path? (English is not my mother tongue, sorry) The signals exists in control, i can "bring them up", which i haven't done yet because it may cause some unexpected behavior, or i can rename and put them at canvas item, something like "cursor_entered/exited/pressed". I'm accepting suggestions. About the _has_point: will do. |
|
The second point is related to the order in which inputs are propagated in the engine (however i was mistakenly beliving _gui_input was happening before _input) If your Sprite captures mouse_enter, and then another Sprite captures the MouseMotion because you entered it, that was ment to cause the mouse_exit, and you click the mouse, then the Engine will still believe the mouse is inside the first sprite and not call mouse_exited. Also the order of propagation of All of this complex interactions is the reason why mouse picking was considered exclusive to Controls, _gui_input is processed from bottom to top, meaning the node that handles the input will be the one drawn last, and also viewport keeps internal state related to the last hovered Control to mark it as unhovered when another control takes that place A simple workaround to mouse enter/exit in sprites is useing a TextureRect and override _has_point(local_position) to check the texture pixel and return color.a > 0.01 Polygon2D requires to override _input tho |
|
Ok about the second point, i think that i'll have to make the change from scene_tree(so, my initial idea was not that far off, i should've tried that first instead of going for canvasitem) In line 1435 in scene_tree.cpp, there's a loop that iterate over all the nodes, from the last to the first(if i got this correctly). So, i think that i should use scene_tree to keep track of which node is being focused(like viewport for Controls elements), and "intercept" the input in this iteration, which already exists so it shouldn't have much of a performance impact. This then should respect the priority order and, if i implement the tracking correctly, avoid the situation that you've described. About the workarounds the sprite2D kinda of does that in _mouse_hit_test(i'll refactor it to _has_point for better clarity). Note: Please correct me if i got anything wrong. I don't understand why the function the said loop is called _call_input_pause, but it seems that it is the one where the input propagation starts from. |
|
as i pointed out, the same input is propagated several times trugh the game So, i dont know which one of all those events is the loop you are seeing in scene_tree, or maybe is all of them this is all much better explained in the documentation |
Introduces a unified mouse picking system for CanvasItem, replacing per-node mouse signals in Sprite2D and Polygon2D with generic signals (mouse_entered_node, mouse_exited_node, mouse_pressed_node) in CanvasItem. Adds has_point and mouse_picking methods to CanvasItem and updates SceneTree to handle mouse picking and signal emission. Removes mouse-related signals and refactored _hit_mouse_test to has_point from Sprite2D and Polygon2D, centralizing logic in CanvasItem, that may be overriden. Let's pray now that the code styling test won't obliterate this commit
I must concede that i don't fully understand why this won't compile for android/ios, but it think it has something to do with canvasitem being a "friend class", i'll take a while to study this later. For now, i will just hotfix it, because the casting is not really necessary i think
|
Please do not push so many times in a row, you are a first time contributor so you need approval to run CI, but otherwise this would really take up a lot of CI processing and time, so please make your changes in a batch |
|
Ok, i'm sorry, i wasn't paying attention to this PR actually |
|
Ok, i've tested the code by hand, and all 20 automated checks were successfully in my branch I'm sorry for the mess i created, i really wanted to help but was not really familiarized with how things are done. I hope my contribution help someone, and thank you for the patience. Also, newbie question: These commits that appeared after i merged my branch: are they ok? i was expecting that only the final one showed up here |
|
you need to squash the 36 commits in a single one to be considered for merging, this can wait until it's reviewed, but is good practice to keep the PR clean |
|
I believe this feature doesn't fit with Godot's methodology as specified in the documentation: https://docs.godotengine.org/en/stable/about/faq.html#why-does-godot-aim-to-keep-its-core-feature-set-small |
|
Well, one can say that you can implement this feature with addons or scripts, but imho it's so convoluted, like, why do i need to use physics or create another function that checks for inputs to pick mouse events in a Sprite2D when the engine code already provides the base for this system? It's also not quite intuitive why this system won't affect canvas items but is present in controls. |
5 participants
This PR improves mouse input handling for CanvasItem-based nodes by integrating _mouse_hit_test() into the input propagation path and emitting mouse-related signals directly from CanvasItem.
It enables accurate mouse interaction for 2D nodes without relying on physics.
Summary of changes
Motivation
Currently, CanvasItem nodes receive mouse input in a decentralized way, requiring each node to manually perform hit tests, often using only bounding rectangles.
This PR:
Use cases
Compatibility
Notes