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

Add toggling visibility of multiple nodes in scene tree by click-and-dragging #102009

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

keestak
Copy link

@keestak keestak commented Jan 25, 2025

Other software such as Blender, Photoshop, and Aseprite let you click and drag over layer/object visibility icons to toggle the visibility of multiple objects at once. I think this is a nice quality-of-life feature that improves workflow in the editor. This PR implements that feature for the scene tree dock.

10

@keestak keestak requested review from a team as code owners January 25, 2025 04:07
@Repiteo Repiteo added this to the 4.x milestone Jan 25, 2025
@keestak keestak force-pushed the scene_tree_click_drag_visibility branch from 8ea67d2 to afdbc9b Compare January 25, 2025 07:04
@keestak keestak marked this pull request as draft January 25, 2025 07:26
@keestak keestak force-pushed the scene_tree_click_drag_visibility branch 2 times, most recently from 9bdf6fc to 097d3b7 Compare January 25, 2025 10:17
@keestak keestak marked this pull request as ready for review January 25, 2025 10:18
editor/gui/scene_tree_editor.cpp Outdated Show resolved Hide resolved
editor/gui/scene_tree_editor.cpp Outdated Show resolved Hide resolved
@keestak keestak force-pushed the scene_tree_click_drag_visibility branch from 097d3b7 to 500e097 Compare January 25, 2025 10:37
Copy link
Member

@Calinou Calinou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested locally, it works as expected.

simplescreenrecorder-2025-01-25_16.23.10.mp4

Some feedback:

  • The pressed icon background is only present on the first clicked icon, not the icons you've dragged the mouse over. I feel this background should display on all icons your mouse has gone over until you release the mouse button.
  • Remember to update the class reference XML and fill in the descriptions as per the documentation.

@keestak
Copy link
Author

keestak commented Jan 26, 2025

  • The pressed icon background is only present on the first clicked icon, not the icons you've dragged the mouse over. I feel this background should display on all icons your mouse has gone over until you release the mouse button.

I'm not sure there's an easy way to do this, especially without adding unnecessary clutter to the tree and scene tree editor classes. It would be a lot easier to just make the pressed icon background for the first clicked icon become 'unpressed' when the click drag happens (so all the icons appear 'unpressed' during the drag). Would that be acceptable?

@keestak keestak marked this pull request as draft January 30, 2025 07:49
@keestak keestak force-pushed the scene_tree_click_drag_visibility branch from 500e097 to b3b4175 Compare February 1, 2025 22:28
@keestak
Copy link
Author

keestak commented Feb 1, 2025

I went ahead and made it so the background of the initial button becomes unclicked when the click-and-drag happens. It looks like this now:
10

I think this makes sense because leaving the button with a clicked background implies that it will receive a click event once the mouse button is released (as it does when you mouse down on a button, then drag off the button, then mouse up - the button will still be clicked, and that's not the case during click-and-drag). It's also just a two-line change to do this, while making all the buttons show as clicked during the event would require a lot more additions. Hopefully that's all alright.

I also updated the class reference, so everything should be good now I think...

@keestak keestak marked this pull request as ready for review February 1, 2025 22:39
@keestak keestak requested a review from a team as a code owner February 1, 2025 22:39
@KoBeWi
Copy link
Member

KoBeWi commented Feb 2, 2025

The behavior is fine, but the implementation is too complex. It should not modify Tree, or at least not introduce new API.
I think it can be implemented entirely in SceneTreeEditor, with a combination of button_clicked and gui_input signals.

If it turns out impossible, at least button_click_dragged should be removed. It's emitted with every mouse movement, which should not be required. Also instead of button_click_drag_end you could just stop dragging when mouse button is released.

@keestak
Copy link
Author

keestak commented Feb 2, 2025

@KoBeWi
Someone in the issue I mentioned in my initial post said that it would be nice to have Tree support click-and-drag as a built-in feature, so that was part of my approach; maybe I should've asked around more. It is possible to do most of this in SceneTreeEditor only - because all of this functionality stems from gui_input, the relevant parts can just be re-implemented in SceneTreeEditor only which would remove all the new api I added (I am a little concerned about creating unnecessary duplicate code though). That would mean that I wouldn't be able to change the way Tree draws its buttons though, so I'm not sure there would be a way to control the clicked background of the button when dragging starts, which means that it would visually look like the first gif I posted. I do think that's a minor visual detail, but it's worth considering.
If it sounds good to you, I can move all the functionality into the SceneTreeEditor class and avoid changes to Tree or additions to the API.

Also for the record, the button_click_dragged signal emits every time a button becomes hovered during the click-and-drag event, it doesn't emit every frame the mouse is moved if that's what you meant.

@KoBeWi
Copy link
Member

KoBeWi commented Feb 2, 2025

I'd wait for feedback from other maintainers. Maybe extra signals are fine, especially if it avoids code duplication and allows better visual feedback.

Copy link
Member

@KoBeWi KoBeWi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new functionality is fine, but the implementation might deserve more discussion.

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