add command to group selected nodes

[NathanLovato]

This PR aims to add group and ungroup commands for selected nodes.

Close #10

[NathanLovato]

I have short timeframes to work on contributions like these (~30mn at a time), so it'll take me some time to complete this. I'm starting by trying to make the command work well destructively before converting to undo and redo

[NathanLovato]

Grouping is now working. There's an error that triggers for some nodes that I need to look into, probably due to the order of the undo redo methods. It's a minor thing, doesn´t prevent the feature from working.

Next up is ungrouping.

[NathanLovato]

Hi, I added ungrouping this morning. There are still some non-blocking undo errors to fix before merging, but could you tell me if the behavior is what you'd expect? I made a short recording with voice to showcase how it works right now:

https://www.dropbox.com/scl/fi/folfj182wbru6xg06wij8/2023-06-03-grouping-and-ungrouping.mp4?dl=0&rlkey=34o7o111yizp5huj9227ik4ew

Please let me know if you'd like anything to work differently as part of this PR.

When I next have a moment to work on this I'll work on fixing the undo errors that appear in the output.

[imjp94]

Yes, this is working great!
But I just push a commit for:

• Maintain order of grouped nodes after undo/redo
• Ensure name of group node always readable
• Select group node after grouping

Let me know if you are fine with the changes

2023-06-03.23-24-24.mp4

Not sure if this happens to you, but sometime it just doesn't work. It's pretty random, I am not sure what is causing this bug.

2023-06-03.23-13-20.mp4

[NathanLovato]

Thanks for adding changes, they're great!

Not sure if this happens to you, but sometime it just doesn't work. It's pretty random, I am not sure what is causing this bug.

I think it's because I put the input code in _forward_gui_input(), so the shortcut only works when the mouse cursor is hovering the viewport. Moving the code to _input() should fix this.

[NathanLovato]

Just fixed it!

[imjp94]

All good!
Should be ready to merge after the errors are resolved

[NathanLovato]

I could fix the errors when ungrouping nodes. There's one kind of error I don't know how to fix right now: UndoRedo history mismatch: expected 0, got 2.

I'll be away for ~2 days but I'll look into it once I'm back

[NathanLovato]

I'm sorry for the delay, we're swamped with work. It's still on my todo but we have to prioritize our work to ensure we have enough money until the end of the year. So I'm not forgetting this, I'll try to get to it as soon as possible.

[imjp94]

Don't worry, just work in your timeline =)

[NathanLovato]

I hope you'll excuse the very long delay! I've been in a 6+-months crunch, that's still ongoing actually, and I didn't get much time for personal projects (like this contribution).

I just got back to this group/ungroup feature and could fix the "UndoRedo mismatch" error: it was due to the order of undo-redo operations, which matter to the system.

Now, it should be good to go. Let me know if thereś anything more you'd like me to change.

[imjp94]

Don't worry about that, just take you time =)
These projects are pretty much in "maintenance mode", I am grateful that there are many contributors contributing bugfix and adding new feature for me.

But there are some issues with ungroup node, which I might had overlooked last time:

• Undo "Ungroup Group Node" doesn't bring back the group node and its children

undo-ungroup-error.mp4

• Children of "group node" are not inserted to "group node" position after ungrouped. When ungrouping nodes. The "group node" is removed and its children are added to the back of the list. I think it would makes more sense if the children are inserted at "group node" original position.

Btw, I have pushed a commit to remove the "!is_inside_tre()" error when undo "Group Node".

[NathanLovato]

Thanks for taking the time to test and review :). Great catches!

I'm keeping this in my open Github notifications. I've got to find time to tackle it. I'll get to it as soon as possible (which, probably, isn't gonna be very soon..!)