-
-
Notifications
You must be signed in to change notification settings - Fork 97
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
Rename the Mouse Filter modes and add a Skip mouse filter mode #3613
Comments
Not sure how deeply it is related, but this proposal is worth considering when deciding on the implementation: #3272 (it also has a PR ready: godotengine/godot#53075) |
I'm even more confused by this than the current behavior.
How is "Discard" and "Ignore" any different? Neither of them seem to do or imply the behavior I would currently expect from "Pass" as a user: To handle the Input and pass it on to any other node at the current mouse position. If "Discard" is used to pass the signal to other Nodes at the same mouse location, why is it not called "Pass"? |
@golddotasksquestions Better wording welcome, but the general idea is that the first should make it as if the control did not exist for the mouse. The second will do the entire opposite, it gets the event but completely discards it. Its a way to simply disable input for a control (imagine you don't want a mouse to get input for some reason, but you dont want to pass it to something else). We might simply merge both as the same thing to be honest and have a single IGNORE or SKIP mode. |
@pycbouh I suppose we could have an extra mode INHERIT for this and make it default? |
Okay @golddotasksquestions @pycbouh Added both your feedback in the proposal. |
Adding However I still don't understand at all what you are planning to replace "PASS" with for sibling Controls.
The current implementation of What we need is a true Which one of your proposed Mouse filters would do this, regardless of scene tree composition? |
Because there can be several other situations where you still want it to reach another control, but that control is not a sibling but something else, so you would need to make it work for literally everything. Additionally if you do that and you pass to any other control under the mouse it becomes very confusing because a) you don't really know in which order it happens. b) controls covered by other controls may still get the input and you would be unaware. The pass functionality is only intended to work with parents because its for very specific use cases. It means that the control will handle the input and then pass it to the parent. What you (and others) actually want is not this, you simply want the control to do nothing (as if it was hidden) and let the next available one take the input. Hacking the pass functionality for this is complex and does not really make sense anyway, when all you want is an "ignore this control" mode. So, the solution is to add a new mode that actually does what you want, not hack an existing one you were not even aware it worked the way it does. |
And to clarify this is not your fault, it is mine for not making it clear enough. |
No, that's definitely not what I or others want. We already have this. This is already what the current implementation of What I and I think many other expect is what you would matches the visual experience, matches the 2D sorting of the Control nodes in the viewport, regardless of their position in the scene tree. I have no idea if that's feasible to implement, but I know this is what is intuitively expected: You click on a Control, and if it has mouse filter set to The way I imagined it to work is to first check which Controls are at a given location, and then, starting from the one furthest from the root, simultaneously "walk" the scene tree on multiple branches only on the nodes at this location towards the root. Whenever one of the simultainous "walk" hits a I'll see if I can create an animation to visualize this. |
@golddotasksquestions If you want more than one control to handle a single event that's not going to happen. As I mentioned in the proposal:
Breaking this convention would make the entire UI code extremely complex for no good reason because many behaviours (click dragging specially, which is needed for drag and drop as well as scrolling) would just not work. No UI library that I am aware of works this way either. It's better that you work around what you want to do in a different way. |
I find the current names easier to understand than the proposed ones. |
How does that relate to godotengine/godot#54656 ? But if this PR is still OK then the new naming is worse than the old one since MOUSE_FILTER_HANDLE_WITH_PARENT (the new PASS) does not guarantee that the input will be handled. |
Are you sure that's how Ignore works? This sounds more like Stop, although Stop doesn't ignore the input. In my experience, Ignore makes input events behave as if the node isn't even there, allowing input to pass to other nodes that are below it.
This sounds exactly like how Ignore currently works, based on personal experience and the way it's described in the docs page for Control.
|
Couldn't agree more. I understand that the current behavior makes sense to the developers, but it makes no sense and it's not at all intuitive for the users of the product. If the current behavior of the pass filter is correct for some feature the developers envisioned, that's fine. But the users desperately want another feature then, a feature that the name "pass" implies. One that passes the input to the node bellow in rendering order, not in scene hierarchical order. Name this new filter option "pass4realz" or whatever, but this is what would be an actually useful feature for the vast majority of the userbase. |
To be honest, I like the current UI system but I feel like this might make it a little bit more complicated for people to pick up, especially beginners. Here is my recommendation on how mouse filters should work:
When the mouse filter is set to # When _gui_input is not overridden, the event gets discarded or handled by the Control node we are extending
# func _gui_input(events):
# accept_event()
# The same as the first function above where the Control node we are extending handles the _gui_input
func _gui_input(events):
super._gui_input(events)
# This is what _gui_input on mouse_filter Stop does by default
func _gui_input(events):
accept_event()
# When accept_event is not called, we pass the event to the next node
func _gui_input(events):
# accept_event()
pass |
I find having to add a script and override a built in function to be faaaaaar less beginner friendly than just setting mouse filter to "pass" in the Inspector. Even for myself as an intermediate/experienced user this would feel like a downgrade in user friendliness, not an improvement. |
Aka |
Kicked to 4.x instead of 5.0 because the proposed Skip mode does not require renaming the existing mouse filter modes, but the renaming part of this proposal breaks compat so it will have to wait for Godot 5.0 in a decade or so. |
@aaronfranke This has been mostly agreed on for 2 years and wasn't touched, and now you're saying we won't see it fixed until ten years from now? Seriously? Some usability things need to win over strict "no breaking compatibility" (or Godot 5 needs to come earlier) |
I still find the current way it's working simpler, so no complaints from me to move it to Godot 5. |
@Zireael07 We can't work on every proposal for Godot 4.0, or else Godot 4.0 itself would take ten years. It's much better to actually get the features like Vulkan that have been waiting since 2019 out the door. The actual issue can be fixed by adding a new Skip mode, only the rename part is kicked to 5.0, it's a cosmetic change. |
There were two years to get this done, this is not something huge like Vulkan itself, though. That's why I'm confused, because it looks like the proposal was just forgotten/swept to the side and then now you're like "oh no too late to do anything oh noes". Why the cosmetic rename has to wait a decade? (Thanks for clarifying the actual issue can be fixed earlier) |
@Zireael07 Because it breaks compatibility. Users are generally unhappy when they upgrade their Godot projects to a new minor release and their project is broken. This issue is not 2 years old, it's 1 year and 3 months old. Even if it was 2 years old, that does not impact how likely it is to get done. There are tons of proposals from 2018 and 2019. We can't get to all of them. The simple fact is that nobody has stepped up to do the work in the last year, so it has not gotten done. In order for something to get done, somebody has to do it, and that time is then not spent on doing other things (again, we can't get to everything). This is not a vital or release-blocking change. The mouse filter is working fine for the vast majority of users. For example, see @Whimfoome's comment. Additionally, if you read the discussion above, there is not even a clear conclusion to the discussion of what the API should look like. |
I don't care at all about the rename since I think the issue lies much deeper. But the matter of fact is mouse filtering is definitely NOT working fine for the vast majority of users. I actually doubt the majority of users even understands what "pass" does. I know I and many others have been trying to understand it for years and I still fail to use it reliably. |
For now, I'm using a custom propogate function: public static void PropogateMouseFilter(Control c, Control.MouseFilterEnum e) {
c.MouseFilter = e;
foreach (Node n in c.GetChildren()) {
PropogateMouseFilter(n as Control, e);
}
} Ideally, I believe a "Skip"/"Inherit" MouseFilter mode should be in Godot Core. I mean, did the Godot editor itself actually never needed such a functionality? ;-) Edit: |
I have been actively working with Godot for more than 2 years. I already have a relatively successful game on steam store which was built in godot, and I'm currently working on my second game, so I'd like to think I'm pretty efficient with the engine... Nevertheless, I NEVER use the pass filter. Obviously when I started out I tested it and as it turned out it doesn't do what I expected it to do, I moved on. It's sort of a noob trap, but I don't think a game engine needs a noob trap. Maybe I'm the lazy one for not trying to fully grasp to "usefulness" of this filter, but this is a genuine question to other developers: Do you actually ever use the pass filter, do you ever feel the need to use this feature? Because after implementing probably hundreds of custom controls, I surely don't. |
If it would actually do what it I think it should do (allow to handle the mouse action within the control with the pass filter, then pass on or trigger it in the next Control node below, located at the same screen position), I would use it all the time. |
That was my point as well, I don't use it either because it doesn't do what I expected it to do. And it seems this is a trend and and probably lots of developers feel exactly the same... |
Hey, anyone. I always thought that the mouse_filter PASS was the right way to do this, but realized that NO, pass doesn't propagate the input to all the lower controls, it just passes it to it's parent. Like. WHAT? Now if pass is just not what I'm looking for, then okay. but. What would be the correct way to solve this? Anyone know? |
Godot uses Event Bubbling for input events. Most other gui mouse input systems use Event Bubbling (Unity, Unreal, HTML, Windows, Qt, etc). Some also have a capturing phase, which is basically reverse order bubbling. func _input(event: InputEvent) -> void:
if event is InputEventMouse:
if Rect2(Vector2.ZERO, size).has_point(get_global_transform_with_canvas().affine_inverse() * event.position):
# The mouse event is in this Control.
pass And since mouse_entered happens before mouse move events and mouse_exited happens after, you can use those to determine if the mouse is underneath something else. I could not find any event systems that send gui mouse events to a Control visually behind them in a controlled way (not just a bounds test). I feel like the existing system makes sense and is sufficient for the vast majority of users. If it is not sufficient, then why is it not a problem in other UI systems? What do they have that would solve the issue that godot does not? I think the cause of confusion is the name of MOUSE_FILTER_PASS which can imply functionality it doesn't have. If mouse event passthrough functionality is still desired a separate proposal can be used for it (such as #7167 or a new one), though it needs more details to how it will work with the current system, how existing UIs may be affected by other Controls also receiving mouse events, and use cases. Edit: I linked to the wrong proposal. |
I don't think MOUSE_FILTER_INHERIT is a good idea.
|
Just adding my cents to the mouse_filter enum names, I think that renaming I also think that the current In the end we would have the enums:
With this implementation, it could also be used in #3272, so the parent pre-handle the event, mark it as handled if needed so the event can be propagated to the children or not. In the HTML/JS event handling as example, you can pass an optional flag to add an event listener to callback when the event is propagated from the root to the leaf nodes (inverse of bubbling) or when it's propagated from the leaf to the root nodes (default, bubbling). |
For anyone looking for a work around, and it may not be perfect for your needs, but hopefully it helps any ways. extends TextureButton
var __texture_scale: Vector2 = Vector2.ONE
var __mouse_entered: bool = false
var texture_click_mask_size: Vector2 = Vector2.ZERO
# Called when the node enters the scene tree for the first time.
func _ready() -> void:
texture_click_mask_size = Vector2(texture_click_mask.get_size())
__texture_scale = texture_click_mask_size / Vector2(self.size)
# Called every frame. 'delta' is the elapsed time since the previous frame.
func _process(_delta: float) -> void:
pass
func _input(event: InputEvent) -> void:
if event is InputEventMouse:
var mouse_pos_local_scaled = (get_global_transform_with_canvas().affine_inverse()
* event.position * Vector2(self.__texture_scale)
)
if (mouse_pos_local_scaled.x < texture_click_mask_size.x and
mouse_pos_local_scaled.y < texture_click_mask_size.y and
mouse_pos_local_scaled.x > Vector2.ZERO.x and mouse_pos_local_scaled.y > Vector2.ZERO.y
):
if texture_click_mask.get_bitv(mouse_pos_local_scaled):
if self.__mouse_entered == false:
self.__mouse_entered = true
emit_signal("mouse_entered")
print("Mouse is in ", name, ".")
else:
if self.__mouse_entered == true:
self.__mouse_entered = false
emit_signal("mouse_exited")
else:
if self.__mouse_entered == true:
self.__mouse_entered = false
emit_signal("mouse_exited") |
I agree with this comment about I feel like the solution here is better documentation. Quite a few people here, including the original post, are just misunderstanding the way that certain modes work. I have been confused by some of the behaviour, although I do actually think it's a good system. I understand that if a lot of people are misunderstanding, it may be a problem with the name, but renaming breaks compatability so it isn't really possible for now. I'm thinking a dedicated tutorial for I think its also worth explicitly stating that setting the |
I actually came upon this proposal (or really, the related complaints about Looking at some of the code for the editor, that feature is handled by a RefCounted Meanwhile, the more "intuitive" My point is, making it work the way users expect might break some existing conventions, but I think it has legitimate use cases. And even things that would seemingly "break" can benefit from it on the user's side. Plus, making it not rely on Control parents like the current |
I'd go with this combination:
I come from Android development, and if nothing clickable is in the way, I'd prefer to pass the event to all the Controls, that are UNDER mouse event position instead of passing it up to the parents. I would REALLY like to have
^ WIth these names, it's obvious when Control handles the event and what happens to it next ^ An example of how not having real ignore option is a problem for me: I think that it's total nonsense, that filter named IGNORE doesn't let events through and there is no mode to actually ignore any mouse events. I came here quite annoyed at Godot |
This is how it works in Android, and it's analog of I don't see any reason why transparent Control in Godot can't have a mouse_filter that allows it to just not interact with the mouse in any way |
Total rename is probably for 5.0, but in 4.x we could just add MOUSE_FILTER_SKIP. |
@Ariorick The part about Android input handling is interesting, I'll look into it. |
@kitbdev yep turns out IGNORE does ignore clicks... I thought I tested it, but something went wrong, I guess, and docs are also weird about it MOUSE_FILTER_IGNORE Is this an error in docs? Or what |
WHAT |
Describe the project you are working on
Godot
Describe the problem or limitation you are having in your project
Godot GUI controls have something called the Mouse Filter, which determines how mouse events are handled by the controls.
To give a bit more insight on how mouse filtering work, the following logic is used: The mouse input will always reach a single control. This is expected in any GUI library. Once the input reaches a control, this control will decide on what to do based on the Mouse Filter, which can be any of the following:
This works for the most part, but there are situations where users want the input to be ignored and passed to a sibling control instead, which can be seen in issues like:
godotengine/godot#55432
godotengine/godot#55288
and many others.
This is currently not possible.
Additionally other proposals (such as #3272) make the need to propagate the mode in tree fashion (like we do with other options like the process mode) easier.
Describe the feature / enhancement and how it helps to overcome the problem or limitation
This proposal aims to do three different things:
Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams
The new proposed mouse filter modes would be:
If this enhancement will not be used often, can it be worked around with a few lines of script?
n/a
Is there a reason why this should be core and not an add-on in the asset library?
n/a
The text was updated successfully, but these errors were encountered: