Allow to create a node at specific position

[KoBeWi]

Closes 2D part of godotengine/godot-proposals#435 (not sure how much possible/relevant is 3D in this case)

The shortcut is Ctrl + Right Click

For some reason it doesn't work with Control nodes if the parent node is Node2D. An error appears saying

 scene\gui\control.cpp:1638 - Condition "parent_rect_size.x == 0.0" is true.

which is rather weird, because I'm pretty sure _edit_set_position() is used without problem in other places. Is there any way to fix it, other than doing what I did in #36309? (i.e. differentiate between Node2D and Control when setting position)

Also this probably needs testing.
Also also some of the code might look like hacks. If something can be done in a better way please comment.

[setanarut]

What happens if the node doesn't have the position property?

[groud]

The idea is good but the node creation menu should only show nodes that are extending CanvasItem, as they are the only ones having a position value.

Regarding the bug, I can't tell for sure what happen from my phone :/

[KoBeWi]

What happens if the node doesn't have the position property?

Nothing, the editor checks if it can set position, before setting position.

the node creation menu should only show nodes that are extending CanvasItem, as they are the only ones having a position value.

Probably doable, but what about instanced scenes? (the second option)

[groud]

For instanced scenes, I guess we have to check if the root node extends CanvasItem too. Since, if you have a non-CanvasItem node as the root, the transform would not be applied to children nodes anyway.

Though I believe that for scenes, we might want to show the scenes as disabled instead of not showing them at all

[KoBeWi]

For instanced scenes, I guess we have to check if the root node extends CanvasItem too.

Well, to filter them from instance dialog the only way is to load each scene and check the root type. This is overkill for big projects.

[groud]

Not sure that is true. I believe we could cache the root node type at import time. As this information could be used somewhere else too (like to display the scene type in the FileSystem dock).

But I mean, if that is complex to implement, it can be improved later or.

[KoBeWi]

Sooo I can't filter out non-CanvasItem nodes. When I do this

you can create a CanvasItem node, which results in a crash. This probably requires some CreateDialog modifications.

[groud]

Oh, you should probably check if the node can be instanced then. I am not sure about it, but there might be an option in the CreateDialog node to avoid the user being able to select abstract classes.

[KoBeWi]

there might be an option in the CreateDialog node to avoid the user being able to select abstract classes.

Yeah, the problem is that it doesn't seem to work if the abstract node is root of the dialog.

EDIT:
Nevermind, it was easy to fix. Only the Control bug remains now.

[Xrayez]

Does it work with "Use Grid Snap" option? Or is it still required to move the instantiated node to align it to a grid?

[KoBeWi]

It does support snapping.

[setanarut]

Nested nodes. this does not seem ideal. Nodes must be created at the same level. similar to the duplicate function.

[KoBeWi]

The new node is created under the selected node. This is consistent with regular Add/Instance Child Node.

[setanarut]

The new node is created under the selected node. This is consistent with regular Add/Instance Child Node.

True but, I think it should also be fixed, "Add / Instance Child Node." should not change the selection when used. User gives the command to godot create child node, not gives "select child" command. Nested nodes are less used. This is another topic of discussion. (this must be optional settings)

[KoBeWi]

@hazarek Create a separate proposal for that.

[KoBeWi]

Ok, I fixed the Control bug by checking Node2D and Control separately. This actually allowed me to drop the _edit_set_position modifications (I previously added a global argument), so in the end it's better.

As for the nesting nodes issue (even if it's unrelated), you can just click on blank space to unselect any nodes and the node will be created under root.

[groud]

Ok, I fixed the Control bug by checking Node2D and Control separately. This actually allowed me to drop the _edit_set_position modifications (I previously added a global argument), so in the end it's better.

The _edit_set_position() function positioning depends, in the editor, on whether or not the "anchors mode" is active (even if the button is not visible, it's the anchor button at the top of the toolbar when you select a Control). This function should likely be modified to make sure the parent node is correct (having a rect != 0) before calling the set_position() function.

[KoBeWi]

I think my current solution is fine. Changing it to _edit_set_position() would still require two casts (need to check if parent is CanvasItem to apply correct position).

[groud]

would still require two casts (need to check if parent is CanvasItem to apply correct position).

Well, the second cast would have to be inside the _edit_set_position() function itself, so it would be more like a fix, as the function should work whatever the situation.

I mean, your solution is not incorrect, it is just that it kind of workarounds in its own place something that what supposed to be already handled by _edit_set_position(). I don't mind code duplication much, but I'd rather avoid it when the fix is simple.

[KoBeWi]

Well, the second cast would have to be inside the _edit_set_position() function itself, so it would be more like a fix, as the function should work whatever the situation.

No I mean I have to set the global position, and _edit_set_position() supports only local position. Previously I modified the _edit_set_position() to support global, but that required modifying 6 files. Now I realized I could just use to_local() on node's parent (probably), but I'd have to check whether the parent is a CanvasItem. So we'd still have 2 checks in the same place, except one would nested. And then I'd have to do the necessary _edit_set_position() modifications. Not sure if it's really worth it.

[groud]

I don't think you would have to do that, here is what I would use (this is similar to how it's done in canvas_item_editor.cpp) :

Transform2D xform = canvas_item->get_global_transform_with_canvas().affine_inverse() * canvas_item->get_transform();
canvas_item->_edit_set_position(xform.xform(new_pos));

As the get_global_transform_with_canvas() function already handle checking the parent's type and all, this should solve the problem. If it does not, you might need to remove the * canvas_item->get_transform(), but I am not sure about it.

[KoBeWi]

Ok changed.

[KoBeWi]

I rebased the PR and fixed a bug that caused recent nodes to be removed. It should be fine to merge now.

[jcostello]

Can this be implemented for 3d too?

[KoBeWi]

It can, but not in this PR. (and likely not by me, as my 3D knowledge is limited)

[groud]

Looks good. :)

[akien-mga]

Thanks!

[jordanlis]

Hello, small question there : would it be possible to edit in the settings the shortcut and define only right click for opening the menu ? I don't see the point of adding the need to press ctrl with the right click.

[groud]

Hello, small question there : would it be possible to edit in the settings the shortcut and define only right click for opening the menu ? I don't see the point of adding the need to press ctrl with the right click.

Right-click is used to pan the view. It would be annoying to have the menu spawn everytime you want to pan. If we merge #53471 though, we could remove the need to press control for that.

[jordanlis]

Hello, small question there : would it be possible to edit in the settings the shortcut and define only right click for opening the menu ? I don't see the point of adding the need to press ctrl with the right click.

Right-click is used to pan the view. It would be annoying to have the menu spawn everytime you want to pan. If we merge #53471 though, we could remove the need to press control for that.

But panning is already possible with the scroll button (middle mouse button) or with the press of space for mac users right ? So what's the need to use right click in addition of that ?

[KoBeWi]

So what's the need to use right click in addition of that ?

There is none, that's why it's being removed. But we need to be careful about this change, because some users are used to having this option.

[chucklepie]

Late to the party, sorry, this feature is great, but my main need for this functionality (I imagine many others) is to paste a node into a new place not create a new node, i.e. you're creating a level/map/etc and you never want the item to be in the same place as the one you copied from. Otherwise it's just as tedious to draw anything, especially in a big scene. Is this not an option?

[KoBeWi]

You mean something like "Paste Node Here"? Sure, it could be added.