Allow the Container children selection but don't allow them to move #23059
Conversation
| @@ -449,16 +459,14 @@ void CanvasItemEditor::_find_canvas_items_at_pos(const Point2 &p_pos, Node *p_no | |||
| for (int i = p_node->get_child_count() - 1; i >= 0; i--) { | |||
| if (canvas_item) { | |||
| if (!canvas_item->is_set_as_toplevel()) { | |||
| _find_canvas_items_at_pos(p_pos, p_node->get_child(i), r_items, p_limit, p_parent_xform * canvas_item->get_transform(), p_canvas_xform); | |||
| _find_canvas_items_at_pos(p_pos, p_node->get_child(i), r_items, p_parent_xform * canvas_item->get_transform(), p_canvas_xform); | |||
There was a problem hiding this comment.
I know it's pre-existing code, but consider swapping around the code in the if and else blocks and removing the !. It would be more readable.
|
Yes, this will be helpful. Glad you're already on it. X) Tested it out, looks good. I can quickly select the container controls again. |
|
Remember we discussed adding a text in the 2D viewport when children of containers are selected: "Children of a container get their position and size determined only by their parent" |
ghost
mentioned this pull request
groud
force-pushed
the
change_ci_selection
branch
from
1e68b3c
to
b6ecdd7
Compare
|
Ok I updated the PR with a system to display warnings. |
groud
force-pushed
the
change_ci_selection
branch
from
b6ecdd7
to
0c2ebbe
Compare
| if (_is_node_locked(p_node)) { | ||
| return false; | ||
| } | ||
| if (Object::cast_to<Control>(p_node) && Object::cast_to<Container>(p_node->get_parent())) { | ||
| if (p_popup_warning) { | ||
| _popup_warning_temporarily(warning_child_of_container, 3.0); |
There was a problem hiding this comment.
It's a bit weird to pass a reference to a Label IMO, and it means that if we want other warnings we'll have to create labels for each of them. I think it's simpler to reuse the same design as for the warning dialog, where you popup the dialog directly with the arbitrary string you want to display (and thus reuse the same Label for all warnings).
There was a problem hiding this comment.
The problem is that I need the ability to check whether or not the warning is already displayed, so that it does not popup twice if it is already there. Using a string comparison is not really safe : as I want to expose this to plugins, they might not understand why their two warnings with the same text does do not popup.
So a solution could be to change the API to something like that:
int create_popup_warning("Warning: blablabla") // returns an ID
void popup_warning(int id, float time)
If you think this is cleaner I can change the API to that.
There was a problem hiding this comment.
Why not just use draw_string directly on the viewport, like in the other implementations?
There was a problem hiding this comment.
Well, depending on what you have behind it might be more or less easier to read (on white background for example). Also, this allow for several messages to be displayed, so we can reuse the system for other plugins.
So in the end, it's easier using containers + labels instead of drawing a string.
|
Thanks! |
Fixes #22877
Fixes #23606
Also fixes #22960