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

Scene Tree Dock: added ability to cut/paste nodes. #13309

Closed

Conversation

Krakean
Copy link
Contributor

@Krakean Krakean commented Nov 26, 2017

Added ability to cut/paste of nodes on scene tree dock.
Of course we can just select nodes and move them via drag'n'drop to desired node, but cut/paste on tree/lists is something that is expected (nice to have, at least for Windows users).

This is how it works: https://i.imgur.com/7BdIdQY.gifv

Cut/paste icons: https://files.fm/u/w33v6624#_

@akien-mga
Copy link
Member

It fails to build on X11 tools=yes: https://travis-ci.org/godotengine/godot/jobs/307510048

editor/scene_tree_dock.cpp: In member function 'void SceneTreeDock::_tool_selected(int, bool)':
editor/scene_tree_dock.cpp:511:16: error: 'CPC_ACTION' is not a class or namespace
     m.action = CPC_ACTION::NODE_CUT;
                ^
editor/scene_tree_dock.cpp:569:24: error: 'CPC_ACTION' is not a class or namespace
     if (item.action == CPC_ACTION::NODE_CUT)
                        ^

@reduz
Copy link
Member

reduz commented Nov 26, 2017

This is cool, probably too late for 3.0 (we are releasing beta imminently and feature freeze has kicked in).
but would be nice to merge after 3.0 is out. Does it support cutting and pasting between scenes? also would not Cuy/Copy/Paste make sense too?

Finally, the RMB menu likely is too crowded, so maybe it would need some reordering or using submenus for the least used options, too.

@Krakean
Copy link
Contributor Author

Krakean commented Nov 26, 2017

@akien-mga To be honest have no idea why it fails. May be some nix-user can advice why it fails, because compiles fine on Visual Studio and I don't use any special syntax there...

@reduz

Does it support cutting and pasting between scenes?

Yes, it is.

also would not Cuy/Copy/Paste make sense too?

I decided to start from just cut/paste (after all, at least having two of them is better than having nothing at all? :).
If people will like it, I was planned to add support for Copy in 3.1 :)
Problem is, since most of my recents PRs are not merged, its kinda uncomfortable for me to make new changes because either some of them requires my previous PRs or I have to carefully line-by-line merge PR changes within my local copy with up-to-date godot master instead of just files copy-over. So trying to make as much less changes, as possible.

P.S. I think its pretty safe to merge it into 3.0. Just try it, ensure yourself that is works pretty seamless and smooth. :) And just in case if you will do, don't forget to take also #13308, as it makes possible UX like "mouse clicked on empty space in scene tree dock to reset selection from current [non-root, for example] element, CTRL+V to paste element into root without manually selecting it".

Finally, the RMB menu likely is too crowded, so maybe it would need some reordering or using submenus for the least used options, too.

I think the same. But do nothing currently related to interface/UX changes, due to feature freeze state. Once 3.1 development will be opened, I will start send more bold PRs :)

@letheed
Copy link
Contributor

letheed commented Nov 27, 2017

Maybe this doesn't need to be exposed in the RMB menu yet. It really is crowded. I'd also like Copy, though Ctrl+C is bound to "Copy node path" already.

@Krakean
Copy link
Contributor Author

Krakean commented Nov 27, 2017

@letheed

  1. Actually I assume that people will use hot keys. I added them to popup menu just to let people know that these cut/paste command are exists :) But yes, gonna to think about how reorganize scene dock tree popup menu better for 3.1
  2. Initial problem with copy was that we already have Duplicate command there, didn't have enough time to decide. And yes, as you noted correctly - ctrl c is already bound. So I decided leave Copy implementation for 3.1, more time to think and experiment with.

@fracteed
Copy link

Being able to copy nodes and trees between scenes is sorely lacking in Godot. If you can add this, it would be a massive boost to productivity :)

@eon-s
Copy link
Contributor

eon-s commented Nov 28, 2017

Can copy-paste as text too? could be interesting to share partial scene branches (something I have asked for visual script too), useful for tutorials, troubleshooting, etc., instead of sharing full projects or scene files.

@Krakean
Copy link
Contributor Author

Krakean commented Nov 28, 2017

@fracteed
@akien-mga

There is demonstration for cut-paste between scenes: https://i.imgur.com/qZn4TZf.gifv
Again, this my PR support cut-paste between scenes too. :)

@Krakean
Copy link
Contributor Author

Krakean commented Nov 28, 2017

@eon-s can you clarify your (feature) request? :) There is already "Copy Node Path", no? Or it's not work for you the way you want?

@eon-s
Copy link
Contributor

eon-s commented Nov 28, 2017

Like:
Copy a branch, paste on a text document.
Copy the text of a node branch structure, paste on a scene.

@Krakean
Copy link
Contributor Author

Krakean commented Nov 28, 2017

@eon-s Oh, now I see.
Looks like "Copy Node Path" support only one node, and can't work when multiple nodes are selected.

Copy a branch, paste on a text document.
Copy the text of a node branch structure, paste on a scene.

I can implement both as separate PR (for v3.1, too late for v3.0), but there is small problem as I just found - "Copy Node Path" copy node current name, not their type, so there is need to think about of a some kind of text "format" which can be reinterpreted correctly by Godot (for example CTRL+C on nodes branch, and CTRL+V it to text document like: "CurrentNodeName1 (NodeType) > CurrentNodeName2 (NodeType)", instead of current "CurrentNodeName1") and work fine for both ways (Nodes Branch -> Text Document and Text Document -> Nodes Branch), but this "format" assumes only nodes name/type, not nodes properties though having of which I assume can be kinda important for tutorials purpose. But I can be wrong.
In short, thing to think about.

@eon-s
Copy link
Contributor

eon-s commented Nov 28, 2017

@Krakean yes, it will need to copy everything, like a part of a scene file to keep relationships and values, I can open an issue with some example of this and ping you if interested.

@Krakean Krakean force-pushed the scenetreedock_addcutpaste_feat branch from 3596fee to d803b83 Compare November 28, 2017 11:46
@Krakean
Copy link
Contributor Author

Krakean commented Nov 28, 2017

@eon-s Yes, go ahead please. All details and possible test-cases. Will examine it after 3.0 release.

Added icons

Small adjustments
Possible fix X11 compilation issue.
@Krakean Krakean force-pushed the scenetreedock_addcutpaste_feat branch from d803b83 to a2d08f8 Compare November 28, 2017 11:54
@fracteed
Copy link

@Krakean awesome...this will be so useful! The screen capture goes so quickly, but does the menu option say "copy" or "cut" when you copy these? There should be an option for both. Also, does this allow copying/cutting a complex nested tree and maintaining this relationship when pasting? Anyway...great work so far :)

@Krakean
Copy link
Contributor Author

Krakean commented Nov 28, 2017

@fracteed

The screen capture goes so quickly, but does the menu option say "copy" or "cut" when you copy these?

Its only cut/paste. "Copy" a bit more complicated, so I decided to leave it for 3.1

Also, does this allow copying/cutting a complex nested tree and maintaining this relationship when pasting?

Allow. :)

@fracteed
Copy link

@Krakean...thanks, I look forward to it. In all honesty, I think it would be rare for me to use a cut function, as I mainly want to copy arrangements of meshes,lights etc between scenes without having to instance them. Also copying particle systems between scenes would be very helpful...

@akien-mga
Copy link
Member

IMO, this feature will be complete when copying will be supported. Cut doesn't add much compared to a simple drag and drop of the nodes, and cut/paste from one scene to another doesn't seem like a common use case. So I propose to postpone it to 3.1, and we can add functional copy, cut and paste working across scenes then as described in #3720.

@Krakean
Copy link
Contributor Author

Krakean commented Nov 29, 2017

@akien-mga Thing is, for example, for me much preferred way to do CTRL+X/V than dragging nodes by mouse, especially if you need to do this more than once. I assume that I'm not alone in this workflow.
So, why not just merge this PR? It does not ruin anything. Yes, it doesn't have Copy yet, but uhm, well, I assume that having even just Cut/Paste is better than just nothing at all (dragging only) :)
No one will be harmed due merge this PR :)
In v3.1 I'll add Copy.
Please, Remi. :)

@akien-mga akien-mga modified the milestones: 3.0, 3.1 Dec 9, 2017
@karroffel
Copy link
Contributor

bump? :)

@akien-mga
Copy link
Member

@Krakean Are you still up for implementing Copy support? We're closing in on 3.1 alpha and later on the feature freeze, so time is running short.

@akien-mga
Copy link
Member

So we now have three PRs for the same feature (#13309, #14810 and #19327) and implentation-wise we're preferring #19327 (though it still needs some more changes). Therefore closing as superseded by #19327. Thanks a lot for your work though!

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.

7 participants