Create a virtual mouse move event after moving child nodes in tree (partially reverted)

[Sauermann]

This patch updates mouse cursor and mouse-over states without the need for additional mouse movements after a Node::move_child. It utilizes the infrastructure introduced in #58995 for updating the mouse cursor on scene changes.

resolve #40012

Update 2022-10-02: Add check, that base_window exists
Update 2022-11-03: Resolve merge conflict
Update 2022-11-12: Resolve merge conflict
Update 2023-01-06: Expose update_mouse_cursor_state to GDScript in order to close godotengine/godot-proposals#6052
Update 2023-01-28: Resolve merge conflict
Update 2023-05-29: Address changes of #75701 and resolve merge conflict

[KoBeWi]

I checked your MRP in the issue and the signals seem to be emitted without mouse movement (in vanilla build). Can you confirm whether this PR is still relevant?

[Sauermann]

Yes, I have verified, that the bug is still happening with v4.0.beta.custom_build [bc5d67c] on Linux X11 Xfce.
Also I have verified, that this PR still fixes this bug on Linux.

I can't reproduce the bug on Windows 10 with v4.0.beta.custom_build [bc5d67c]. The likely reason for this is #20357.
But when #20357 gets fixed, then this PR will also be needed on Windows 10.

[KoBeWi]

I could approve based solely on the code (as I can't test), but I'm wondering why you made update_mouse_cursor_shape() virtual.

[Sauermann]

The actual update needs to happen in a Window and not in a SubViewport, because SubViewportContainers might be hidden behind other Controls.
SubViewport::update_mouse_cursor_shape simply checks, if a containing Window is available and calls the update-function Window::update_mouse_cursor_shape in the containing Window. If the call happens within a Window, then there is no need to check for a containing Window (for example when called directly in the root-Window).

My impression was, that this scenario (where SubViewport and Window need to react differently to the same function-call) would be best solved by using virtual.

A different way would be to use if (Object::cast_to<Window>(this)) {} else {} in the Viewport::-function and remove the Window::-function (But this would add a dependency from Viewport to Window).
If that would be the preferred way, please let me know.

[KoBeWi]

Makes sense.

[KoBeWi]

Or actually... isn't this only relevant for Control nodes? Maybe the update_mouse_cursor_shape() call could be moved to some move child callback in Control?

[Sauermann]

Interesting idea. I will investigate it.

[Sauermann]

I had a look at this idea, which brought me to the following setting:

What happens, if Node and Node2 exchange places with respect to their common parent (besides #69979)?

In this case neither Button-node currently receives any Notification, that their position in the SceneTree has changed and that the mouse cursor needs to be updated.
I found the following possible solutions that could happen in Node::_move_child:

• Introduce a new NOTIFICATION_ANCESTOR_MOVED_IN_TREE which gets sent to every child node (restricted to the current Viewport). If any Control-node receives this notification, it triggers a mouse-cursor-state update.
• Send a change notification directly to the Viewport.

With this setting we are getting very close to the problem described in #42822.

[KoBeWi]

Soo is the original solution sufficient here? Or at least is it not something that would need to be reworked later to fix the other issues you mentioned? If yes, I think the PR is fine as is (still needs the comment tho).

[Sauermann]

Soo is the original solution sufficient here

Yes, it is.

Or at least is it not something that would need to be reworked later to fix the other issues you mentioned

The basic idea is to create a mouse move event in order to update mouse cursor and mouse-over state after a change in the SceneTree. I believe, that this method is the right way to do it, because otherwise we would need a different function, that does the same things as Viewport::_gui_input_event for mouse-move-events (and that would need to be kept in sync with).

The other mentioned issue is a display-bug, as far as I understand it and will not interfere with cursor-updates.

So I don't think, that the approach will need to be overhauled at a later time. However some fine-adjustments will be necessary, like #69318.

Also I have updated the PR and included additional comments. It makes sense to document this behavior.

[Sauermann]

This was discussed in PR-review-meeting.
There are changes planned after 4.0 for the process of moving nodes. So at the moment it doesn't make sense to adjust this behavior. I will change this PR to focus only on solving the linked proposal.

[Sauermann]

Please move to 4.1

[KoBeWi]

There are changes planned after 4.0 for the process of moving nodes.

I guess this PR can be revised after #75701 was merged.

[Sauermann]

I guess this PR can be revised after #75701 was merged.

I had a look at #75701 and made the necessary adjustments.

[RandomShaper]

LGTM.

[YuriSizov]

Thanks!