godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
89.46k stars 20.25k forks source link

Keep object child transform information intact when re-parenting #2284

Closed Ace-Dragon closed 8 years ago

Ace-Dragon commented 9 years ago

Currently

Re-parenting an object in the node tree will often send the newly minted child somewhere else in the level, requiring the user to find it and move it back to where it is needed.

Proposal

Change the operation so it doesn't change the transform values of the new child object at all (the object stays exactly where it was placed).

Benefits

It will make it much easier to reorganize parent/child hierarchies as the desired setup remains intact as nodes are moved around. Saving time and reducing frustration.

I hope this is seen as a good suggestion for the 1.2 usability release.

Thanks.

romulox-x commented 9 years ago

If I understand what you're describing correcty, the behavior you're seeing is precisely because the node's local transformation isn't being changed when it's reparented. What you're like to see is the node's local transformation changed so that, when it's reparented to a new node (that has a different transformation than the old parent), its global transformation remains the same, and it's ultimately in the same place in the scene. Is this right?

Ace-Dragon commented 9 years ago

I just don't want objects moved to some random spot when I do a re-parenting operation.

That's all.

romulox-x commented 9 years ago

Does what's happening right now make sense to you? The new position isn't random at all - it's exactly where you'd expect it to be if you keep in mind that the object's local transformation is going to be applied after the new parent's. I agree that an option to make the node end up with the same global transformation would be useful, though, so you don't have to adjust its local transformation yourself to get it back to the same spot.

Apologies if it seems like I'm grilling you about this. I just don't want you to be under the mistaken impression that Godot's picking a random crazy spot to put the node when it reparents it.

bojidar-bg commented 9 years ago

:+1: But I think it should be an option, because I sometimes want to keep the same local transformation, and change the global instead.

OvermindDL1 commented 9 years ago

Global in relation to what? The parent? Grandparent? /root? Something else? It is a nonsensical question since everything in the screenshot can be defined as relative to anything else. The current functionality is the only method that makes sense to do.

OvermindDL1 commented 9 years ago

Scene graph* autotext fail....

slapin commented 9 years ago

I think we just need reparent method for node2d/spatial object which keeps global transform unchanged, while leaving the same behavior intact. Reparent procedure is tedious enough for noobs to demand dedicated method, and I'm not against saving a few lines of code.

On Wed, Jul 22, 2015 at 1:23 AM, OvermindDL1 notifications@github.com wrote:

Scene graph* autotext fail....

— Reply to this email directly or view it on GitHub https://github.com/okamstudio/godot/issues/2284#issuecomment-123496940.

OvermindDL1 commented 9 years ago

There is no such thing as 'keeps the global transform' as what do you mean by global? Global in relation to the parent, to its parent, to the local scene root, to the global root, to any of potentially thousands of other references? This is one thing Panda3D did right was to enforce that there was no root, everything was relative to everything else as the concept of root and global is highly fuzzy in such mutatable scene trees like Godot uses.

FEDE0D commented 9 years ago

He means global as in set_global_pos(). I think he basically he wants a way to do this in the editor: var old_pos = get_node("a").get_global_pos() get_node("b").add_child(get_node("a")) get_node("b/a").set_global_pos(old_pos)

romulox-x commented 9 years ago

I assume the ambiguity that you describe, OvermindDL1, is (at least part of) the reason you can't set the global transform in editor, since it's not well defined whether you mean with respect to the current scene root or the real root of the scene tree. At run time, this isn't an issue, because the scene tree is well defined, and each instance's transformation can be calculated correctly when using set_global_transform.

So I'll agree that the word global isn't well defined here. If you're doing this operation in editor, wanting to see the node not move on the screen (as I understand the request) when you reparent it would have to mean the transformation will be constant with respect to the highest ancestor you can access, which would be the scene's root. In layman's terms, I would imagine the checkbox in the reparent dialog would essentially be "Make the node stay where it is on the screen when I reparent it"

akien-mga commented 8 years ago

Global in relation to what? The parent? Grandparent? /root? Something else? It is a nonsensical question since everything in the screenshot can be defined as relative to anything else.

Don't be a smartass, just run Godot once and you'll see that "global_pos" is what is used everywhere to refer to the position relative to the /root node of the scene currently being edited, i.e. the origin of the main Viewport. And that's also, by chance, what common sense tells you when you're not trying to be annoying.

akien-mga commented 8 years ago

IMO the current behaviour is the most logical for people who use nodes and parenting extensively, e.g. if you have a monster running around on a platform, it's transform should ideally be defined relatively to the platform. Then if you want to move it to another platform, you just need to reparent it, and tadaaa (provided the platform is similar).

vnen commented 8 years ago

Current behavior should be default. But I can see a use for keeping the global position, e.g. if you want to reorganize your scene tree. It'd be good to have such option in the reparenting panel.

reduz commented 8 years ago

Ok gentlemen. Thanks for this very good discussion. The option exists now in the reparent dialog to keep transform and is set to default, mainly because I've seen new users most of the time find their nodes moved and can't realize what is going on. In apps such as Blender keeping transform is the default

kubecz3k commented 8 years ago

@reduz I think similar possibility should be also given when reparenting in runtime via code.

vnen commented 8 years ago

@kubecz3k AFAIK there's no direct reparent via code, you have to remove_child from parent and add_child to new parent, so I don't think there's a proper way to handle position besides you keeping track of it in the code.

kubecz3k commented 8 years ago

@vnen you are right.. in this case I think there could be added, node.changeParent(inNewParent) and reparenting via this method would by default keep the same object global transform . Or eventually reparent(newParent, node2Reparent) in global scope?

vnen commented 8 years ago

@kubecz3k I don't think it is really needed as I can't see a use for extensive reparenting via code. If anyone wants this, another request should be opened.