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

Can't drag a node to the end of the scene tree when the current last node is expanded #54400

Open
starry-abyss opened this issue Oct 29, 2021 · 13 comments

Comments

@starry-abyss
Copy link
Contributor

starry-abyss commented Oct 29, 2021

Godot version

3.3.4, 3.4-rc2

System information

openSUSE 15.3

Issue description

When I want to drag a node to the end of scene tree, it's often a hassle.

I can only release the dragged after the current last node if either the current last node has no children or it's in collapsed state. So I have to artificially create a situation where at least one of these conditions is true, as a workaround.

If the expanded node is not the last in the tree, it's possible to release a node after it. Depending whether one releases the node a bit higher or lower, it's possible to choose whether it should be a child or a sibling of the expanded node. But when the expanded node is the last one, the only option is a child.

Steps to reproduce

First, have a node with children in the end of the scene tree, expanded (Exit on the picture below).
Second, drag some other node in the end of the scene tree (Camera on the picture below).

It's impossible to release Camera in a way that it's placed after Exit as a sibling.

image

Minimal reproduction project

No response

@kleonc
Copy link
Member

kleonc commented Oct 29, 2021

Kinda a duplicate of #46339?

It's impossible to release Camera in a way that it's placed after Exit as a sibling.

image

You can drop Camera on Exit's parent to make it be added/moved as last child of it (which is what you want). But yeah, it might be not obvious.

@starry-abyss
Copy link
Contributor Author

That issue mentions it's fixed, but what I'm describing is still the case, so not a duplicate.

Thanks for letting me know about another workaround, it works too!

@kleonc
Copy link
Member

kleonc commented Oct 30, 2021

What you describing (dropping as the last child) is currently possible by dropping on the parent. I'd say that's not a workaround, that's just how it's supposed to be done according to the current design. As of now things seem to work properly as designed and I don't see a bug from that kind of perspective.

What you're reporting for me is that the current design/approach is unintuitive. As I've already said: "yeah, it might be not obvious". But the question is: how do you expect it to work then? What would be intuitive? For example:
Godot_v3 4-rc2_win64_RRMimSLW4I
Where would Spatial need to be dropped to make it be the last child of Control/Node2D/Timer/AnimationTree/AnimationPlayer/Tween/Viewport?

@starry-abyss
Copy link
Contributor Author

I'm not reporting to you specifically, I'm reporting to Godot team.

The expected way is as described in OP:

If the expanded node is not the last in the tree, it's possible to release a node after it. Depending whether one releases the node a bit higher or lower, it's possible to choose whether it should be a child or a sibling of the expanded node.

This is how I did it for years. But it doesn't work if the expanded node is the last in the list. Then only workarounds work.

@kleonc
Copy link
Member

kleonc commented Oct 30, 2021

I'm not reporting to you specifically, I'm reporting to Godot team.

Yeah, that's what I thought, didn't state otherwise. I've only said how I see things.

The expected way is as described in OP:

If the expanded node is not the last in the tree, it's possible to release a node after it. Depending whether one releases the node a bit higher or lower, it's possible to choose whether it should be a child or a sibling of the expanded node.

This is how I did it for years. But it doesn't work if the expanded node is the last in the list. Then only workarounds work.

The thing is what you're saying is not always true. See the example I've shown above:

Godot_v3 4-rc2_win64_RRMimSLW4I

By dropping Spatial somewhere between AnimationTree and AnimationPlayer there's currently no way to drop it as sibling right after Node2D or Timer. By dropping in there you can only drop it as sibling right after Control or AnimationTree.
Similarily where would you need to drop it to make it the last child of Tween? And so on.
That's not a problem existing only for the last (bottommost) node in the tree. Hence the question about expected behavior cause I'm not sure what would be intuitive in cases like that.

@starry-abyss
Copy link
Contributor Author

Ah, good point! Now I get what you mean.
So the issue not only happens for the last child of the tree, but also for other last children.

From the point of view of intuitive there should be no difference between the last child and other children.
I'm going to see if some other engine's editor does this better.

@starry-abyss
Copy link
Contributor Author

starry-abyss commented Oct 30, 2021

I've played a bit with UE4, and they seem to prefer to sort things in alphabetic order or require detaching and reattaching instead of allowing to move things freely... :(

But I have an idea. What if instead of such positioning for a sibling
image

we'd have this approach?
image

It's different, but still quite intuitive I think (at least I've tried if such approach works when I encountered the problem in question), because similar to the collapsed case.

And moving the choice near the first child rather than near the last should leave only two options: inside the node as the new first child or after the node. As opposed to the current way, where the number of options depends on the nesting depth (with only one or two of them executable).

@Zireael07
Copy link
Contributor

IIRC you don't have to drop near the last child - I've been dropping somewhere in the middle and it works too.

@starry-abyss
Copy link
Contributor Author

starry-abyss commented Oct 30, 2021

@Zireael07 Not sure if I understand what you mean. You can drag a node in the beginning, the middle or the end of the children list. If you drag near (after) the last child, there are currently two options, which can be both predicted by the blue line length:

  • insert after the last child of the most deep node (the screenshot in the first message);
  • insert after the parent (the first picture in my comment above).

However, I'm describing a case where the second option is absent, and kleonc is describing cases where there are more options in theory, but Godot is only offering these two.

@kleonc
Copy link
Member

kleonc commented Nov 1, 2021

@starry-abyss I think I kinda understand what you mean but not fully. To make sure you understand how that dropping works: currently for each item in the Scene Tree there are 3 drop sections based on the item's rect: top (0..~25% of the height), middle (~25..~75% of the height), bottom (~75..100% of the height):
JH07FBB4Lw

So are you suggesting that dropping node A at the bottom section of node B should always result in A being a sibling of B right after it (no matter whether B has children and is uncollapsed)?

If not, here are some images you could use to visualize what you mean (just draw expected destination lines with the matching color):
godot windows tools 64_buQ7Qc5jIK mJZBn6RZuf
dduRPAK5sI 2QR3pm5id9

Also check out godotengine/godot-proposals#3201 and #52053 (which are both linked to #46339 which I've already linked in here).

@starry-abyss
Copy link
Contributor Author

@kleonc Thank you for explaining the current implementation details! I wasn't aware about the concept of top, middle and bottom areas.

Those details shouldn't be an excuse for inconvenient UI design though, because they are just a means to achieve an intended design, not the other way. But still good to know so an easier fix could be proposed (which is not far from the current implementation).

So are you suggesting that dropping node A at the bottom section of node B should always result in A being a sibling of B right after it (no matter whether B has children and is uncollapsed)?

Yes. Then, taking into account the implementation details you shared, dropping as a child of B should probably be triggered by the top area of current first child of B. Or, if B has no children yet, then dropping at the middle area of B.

The main point of the idea is that one can choose any node B to drop as a next sibling, regardless of its depth in the tree. And since the top area of the currently next sibling of B is separate from the bottom area of B, it's possible to add the new behavior while also allowing the current behavior.

For perfection, an extra area after all nodes should be also added, so it's possible to just drop a node in the very end of the tree for it to appear as the last child of a root node. Maybe this change makes implementation less elegant, but it makes design more elegant.

@kleonc
Copy link
Member

kleonc commented Nov 1, 2021

@kleonc Thank you for explaining the current implementation details! I wasn't aware about the concept of top, middle and bottom areas.

The concept of sections is specific to the Tree node and it's exposed to the user (see Tree.get_drop_section_at_position() or Tree.drop_mode_flags). What's specific to the scene tree is: its drop_mode_flags = DROP_MODE_ON_ITEM | DROP_MODE_INBETWEEN (meaning it detecs all 3 sections), and the actual logic it performs on drag-drop (it's user defined, it's not hardcoded in the Tree).

Those details shouldn't be an excuse for inconvenient UI design though, because they are just a means to achieve an intended design, not the other way.

Sure thing. However, note that some of these details are exposed to the user so it would be nice if such exposed details would be preserved just for the compatibility with the previous versions. Although according to the Godot's release policy minor compatibility breakage between minor versions is acceptable so maybe it would't be a problem even for 3.5. For 4.0 I'd guess it can still be changed anyhow.

So are you suggesting that dropping node A at the bottom section of node B should always result in A being a sibling of B right after it (no matter whether B has children and is uncollapsed)?

Yes. Then, taking into account the implementation details you shared, dropping as a child of B should probably be triggered by the top area of current first child of B. Or, if B has no children yet, then dropping at the middle area of B.

If dropping on the middle of B would always result in adding as its first child then I think you'd be proposing exactly the same thing as godotengine/godot-proposals#3201.

The main point of the idea is that one can choose any node B to drop as a next sibling, regardless of its depth in the tree. And since the top area of the currently next sibling of B is separate from the bottom area of B, it's possible to add the new behavior while also allowing the current behavior.

Imagine such tree:

Godot_v3 4-rc2_win64_AxIEcf4C1je1XfUyIhUGIBPTvoLOmV

And that you want to drag Control to be a sibling of Node2D right after it. Without collapsing some nodes it still wouldn't be straightforward with what you're proposing.

What I like about the current state of things is that indicator lines are always being drawn near the mouse. That's what makes behavior intuitive for me: where I drop is kinda where the Node ends up being (mid section is obviously an exception since you can't really add item onto another item). Of course, as mentioned, current design has its flaws. Just saying that with what you're proposing that aspect is kinda lost so I'm afraid it might not be that intuitive.

For perfection, an extra area after all nodes should be also added, so it's possible to just drop a node in the very end of the tree for it to appear as the last child of a root node. Maybe this change makes implementation less elegant, but it makes design more elegant.

I'd say allowing dropping in the area after all nodes should be doable even for the current state of things (even if the implementation would involve a special case for it). The main problem is most likely how to include it into the existing Tree's API (you know, compatibility).


I wonder if splitting the bottom section vertically based on the indentation level of the item could be intuitive. So instead of:
139671935-bdf7449e-3262-42c8-9deb-933f931ee729.png
something like:
dduRPAK5sI_splitted

and depending on on which subsection of the bottom section the node is being dropped it would be added as a sibling right after Control/Node2D/Timer/AnimationTree respectively:

zJTYkvh49mdwmKh7EoQv
JRFWCdfr4IwXDsSWt1ce

The top section would be unchanged, it would just add as a sibling before the given node:
fiwKLwUcfJ

The only obvious problem with such approach I see are quite small subsections of the bottom section. On the other hand dropping in there are kind of corner cases so it may be acceptable. And it should be intuitive as when dragging and seeing the line only below the deepmost node:
wXDsSWt1ce
you'd probably try to drop more to the left instead (if that's not the desired destination). But maybe I'm just hoping intuition will act like that. 🤔

Edit: also for better visual feedback (because the difference in the line lenghts for subsections are quite small) maybe a potential new tree branch could be visualized somehow. For example:

zJTYkvh49m_


Anyway, even if we'd agree on something and I'd implement it, it still won't mean it will be merged in. We should probably move that discussion to a proposal (godotengine/godot-proposals#3201 or a new one) as this is a bug tracker (and unintuitive design is not really a bug itself).

@starry-abyss
Copy link
Contributor Author

If dropping on the middle of B would always result in adding as its first child then I think you'd be proposing exactly the same thing as godotengine/godot-proposals#3201.

I don't mind the original behavior of the middle area of B.

And that you want to drag Control to be a sibling of Node2D right after it. Without collapsing some nodes it still wouldn't be straightforward with what you're proposing.
What I like about the current state of things is that indicator lines are always being drawn near the mouse.

Vertical-wise the blue line indicator could be drawn right in the bottom of B, which means after B. That's how I imagined it actually. And the main difference is the horizontal length, the same pattern as already used (I described it as "two options" above).

I'd say allowing dropping in the area after all nodes should be doable even for the current state of things (even if the implementation would involve a special case for it). The main problem is most likely how to include it into the existing Tree's API (you know, compatibility).

I don't think I can help here. It's either include it inside the Tree for everyone, or as a separate node on top just for the editor. Pick whatever fits better.

and depending on on which subsection of the bottom section the node is being dropped it would be added as a sibling right after Control/Node2D/Timer/AnimationTree respectively:

Good idea to consider.

But maybe I'm just hoping intuition will act like that. thinking

My intuition first tries hard to find some sort of a solution. Then fails and goes to Github issues. :)

as this is a bug tracker (and unintuitive design is not really a bug itself).

I had no idea where the current behavior comes from, before your comments. To me it was a bug, so I posted it here. And I still think it's a bug, edge case type. The same dropping pattern works in middle of the scene tree, if collapsed, etc. But not in the end expanded.

And it's certainly not a duplicate of other issues, where there were some changes already merged. Because even after merging, my issue still happens.

The additional cases you highlighted in this thread - I guess it's a limitation (as in "not implemented yet")? Let's discuss those separately then, as they are more about design. :) I've subscribed to that proposal you've linked.

It's possible that solving your cases would also solve this case as a side effect, but not strictly necessary. While this case is quite clear how to solve right now without even discussing your cases. And this case IMHO is not by design, but by implementation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants