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

Polygon2D node loses its UV toolbar when added as a child and/or removed as a child from another node. #96238

Closed
Zinume opened this issue Aug 28, 2024 · 8 comments · Fixed by #96466

Comments

@Zinume
Copy link

Zinume commented Aug 28, 2024

Tested versions

Reproducible in Godot 4.3
Not reproducible in Godot 4.2.2

System information

Windows 11, Godot 4.3-stable

Issue description

While working on my project, I noticed a small bug in the UI. When manipulating my Polygon2D nodes, I accidentally added one as a child of another Polygon2D node, and at that moment, I lost my UV toolbar. I clicked on the other Polygon2D nodes and still couldn’t recover it. By clicking on a different type of node and then returning to my Polygon2D, I recovered my toolbar.

I noticed that by adding it as a child and/or removing it as a child, I lost the toolbar. Testing in Godot 4.2.2, I noticed that during the process of moving the nodes, after assigning the priority place in the hierarchy, the node should lose focus, and to restore it, I have to click the node again. In the case of Godot 4.3, when moving the node and assigning it as a child, I remain in focus, but it seems to behave as if it had lost focus, and to recover the state, I have to click another type of node.

Steps to reproduce

  1. Create a new project or new scene.
  2. Create two Polygon2D nodes.
  3. Assign one of the Polygon2D nodes as a child of the other, and you will notice that the UV toolbar disappears.
  4. If it doesn’t happen on the first attempt, you can return it to the original hierarchy by removing it as a child, and you will notice that the toolbar disappears.

Minimal reproduction project (MRP)

It’s not necessary, in any project following the steps you should notice it.

@Zinume Zinume changed the title Polygon2D node loses its UV tool submenu when added as a child and/or removed as a child from another node. Polygon2D node loses its UV toolbar when added as a child and/or removed as a child from another node. Aug 29, 2024
@object71
Copy link
Contributor

object71 commented Sep 1, 2024

It seems it might be due to one of thes: b51d911, 09f967b or 393fb68. I will check it out further.

@matheusmdx
Copy link
Contributor

Bisecting points to #84135 as the culprit, @aXu-AP

image

@aXu-AP
Copy link
Contributor

aXu-AP commented Sep 1, 2024

Thanks for notifying me! I'll try to see if I can fix it but I remember that working in that area didn't feel quite clear to me (so no big surprises that it caused some regression).

@object71
Copy link
Contributor

object71 commented Sep 1, 2024

Learned something new today :). Bisecting seems to be a very powerful tool. Okay I will leave the issue to you guys.

@aXu-AP
Copy link
Contributor

aXu-AP commented Sep 1, 2024

Calling it a regression is a little off maybe, since earlier build deselects the moved node. The behaviour likely was the same, but there was no way to trigger it.
Anyway, I did little testing and noticed that this bug affects all nodes that use base polygon editor (Polygon2D, CollisionPolygon2D, LightOccluder2D). Interestingly, the editor is restored also when you change between types (ie. reparent CollisionPolygon2D, select LightOccluder2D). So there must be something wrong in that editor's initialization logic, since other editors (Control, Path2D/3D) don't share the problem.

@object71
Copy link
Contributor

object71 commented Sep 1, 2024

In the abstract class of those editors on the node_removed signal from the scene tree, the node that is edited is set to nullptr. Reparenting triggers the node_removed but will not trigger a similar node_added signal. One could probably handle an on reparented event, but I followed through to the fact that the node is not reselected after being reparented, which seemed like the more general issue. If the node is reselected in the scene_tree_dock.cpp, then it should call the edit function on the abstract polygon editor class and reset the node to a value.

It seems like the need_edit variable on the _do_reparent function of the scene tree dock is not being set correctly with the abstract polygon editor. Debugging shows that this variable is false causing the branch to not call EditorNode::get_singleton()->edit_current(); (which is indeed introduced later than @aXu-AP 's commit but could be the better solution to resolve it in the _do_reparent so that the edit_current function is called).

@aXu-AP
Copy link
Contributor

aXu-AP commented Sep 2, 2024

@object71 It seems to me that you're well equipped to fix this problem, can you do a pr?

object71 added a commit to object71/godot that referenced this issue Sep 2, 2024
@object71
Copy link
Contributor

object71 commented Sep 2, 2024

Well, I continued to debug it and traced it to the fact that we just need to call the show method on the AbstractPolygon2DEditor when we assign the node to the editor.

@akien-mga akien-mga added this to the 4.4 milestone Sep 13, 2024
akien-mga pushed a commit to akien-mga/godot that referenced this issue Sep 16, 2024
…ethod when assinging a node.

Fixes godotengine#96238

(cherry picked from commit dabeaa6)
akien-mga pushed a commit to akien-mga/godot that referenced this issue Sep 17, 2024
…ethod when assinging a node.

Fixes godotengine#96238

(cherry picked from commit dabeaa6)
RadiantUwU pushed a commit to RadiantUwU/godot that referenced this issue Sep 30, 2024
…ethod when assinging a node.

Fixes godotengine#96238

(cherry picked from commit dabeaa6)
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.

6 participants