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

Allow batch drag and drop in typed array of Node and NodePath. #89406

Merged
merged 1 commit into from
Apr 6, 2024

Conversation

AeioMuch
Copy link
Contributor

@AeioMuch AeioMuch commented Mar 12, 2024

This PR gives the ability to drag and drop multiples nodes into TypedArray. It does not include a way to pin the inspector for a specific node, but with an existing workaround it is still useful as is : select multiples nodes, start to drag them, keep the right mouse button clicked while right clicking the node to inspect, then right click again to close the submenu, then drop the nodes into the array.

Another solution than the pinning of the Inspector dock that would probably be good since it is often how it works with others software, would be to be able to hover the node for a certain amount of time while draging and to switch the inspector to that node properties then. But that should be another proposals/PR I guess ?

Aims to fix godotengine/godot-proposals#8193
and partially godotengine/godot-proposals#9265

godot.windows.editor.dev.x86_64_Gsf2ERy6wj.mp4

@AdriaandeJongh
Copy link
Contributor

I want this feature badly, but the UX of this is very uncommon and doesn't work for all use cases. You hold RMB while dragging files so you can click on the node you want to inspect with left, correct? This won't work on trackpads and touch screens, and if they get separate solutions, that will feel hack-y indeed.

@AeioMuch
Copy link
Contributor Author

AeioMuch commented Mar 12, 2024

I want this feature badly, but the UX of this is very uncommon and doesn't work for all use cases. You hold RMB while dragging files so you can click on the node you want to inspect with left, correct? This won't work on trackpads and touch screens, and if they get separate solutions, that will feel hack-y indeed.

This PR only allow to be able to drop multiples node into the typed array, it does not provide a way to switch the inspector like what I propose above (hovering the node for a certain amount of time to inspect it), or a way to pin the inspector. So that's why I mentionned an existing workaround to be able to test it. Sure it is not very user friendly to do it this way tho.

editor/editor_properties_array_dict.cpp Outdated Show resolved Hide resolved
editor/editor_properties_array_dict.cpp Outdated Show resolved Hide resolved
editor/editor_properties_array_dict.cpp Outdated Show resolved Hide resolved
@AeioMuch AeioMuch force-pushed the drag_drop_array_nodes branch 2 times, most recently from ae86b8b to 9521387 Compare March 12, 2024 15:25
@AeioMuch AeioMuch marked this pull request as draft March 13, 2024 22:01
@AeioMuch AeioMuch marked this pull request as ready for review March 14, 2024 15:05
@KoBeWi
Copy link
Member

KoBeWi commented Mar 15, 2024

The title of this PR is a bit misleading. Typed array already supports multiple resource dropping, this only adds support for nodes.

@AeioMuch AeioMuch changed the title Allow batch drag and drop in typed array. Allow batch drag and drop in typed array of Node and NodePath. Mar 15, 2024
@ajreckof
Copy link
Member

ajreckof commented Apr 1, 2024

I really like this but the inspector change on hovering or the inspector pinning should really be done before this as it feels hacky even with a mouse.

editor/editor_properties_array_dict.cpp Outdated Show resolved Hide resolved
editor/editor_properties_array_dict.cpp Outdated Show resolved Hide resolved
editor/editor_properties_array_dict.cpp Outdated Show resolved Hide resolved
editor/editor_properties_array_dict.cpp Outdated Show resolved Hide resolved
editor/editor_properties_array_dict.cpp Outdated Show resolved Hide resolved
@AeioMuch AeioMuch force-pushed the drag_drop_array_nodes branch 2 times, most recently from 249179a to 6df6bb2 Compare April 4, 2024 11:42
@akien-mga akien-mga requested a review from KoBeWi April 4, 2024 15:16
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.

Some more style comments, but seems fine now.

@akien-mga akien-mga modified the milestones: 4.x, 4.3 Apr 5, 2024
@KoBeWi
Copy link
Member

KoBeWi commented Apr 6, 2024

All looks functional now. One more possible improvement is highlighting valid drop positions when dragging multiple nodes, because right now it works only for single node:

godot.windows.editor.dev.x86_64_pvzeFygM1O.mp4

But that's something for later.

@AeioMuch
Copy link
Contributor Author

AeioMuch commented Apr 6, 2024

All looks functional now. One more possible improvement is highlighting valid drop positions when dragging multiple nodes, because right now it works only for single node

Yes indeed, but it is not because of dragging multiple nodes per se but when inspecting another node while dragging, that's when it breaks, even when dragging only one. That should be taking care of with the methods to do it properly I guess.

@akien-mga akien-mga merged commit e6d0d0b into godotengine:master Apr 6, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks!

@AeioMuch
Copy link
Contributor Author

AeioMuch commented Apr 6, 2024

Thanks a lot everyone for your help and patience with me 🤗

@AeioMuch AeioMuch deleted the drag_drop_array_nodes branch April 6, 2024 12:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve workflow when adding elements to an @export array
6 participants