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 an option to reset the transform of the root node when saving a branch as an instanced scene #3962

Closed
mieldepoche opened this issue Feb 12, 2022 · 12 comments · Fixed by godotengine/godot#80561
Milestone

Comments

@mieldepoche
Copy link

Describe the project you are working on

unrelated

Describe the problem or limitation you are having in your project

When saving a branch as a scene, the transform of the root node in the new scene is the transform the node had originally:

drag_and_drop_2-2022-02-12_20.02.58.mp4

However, when saving a branch as a scene the idea is in my experience almost always to "promote" a subtree and make it a component on its own, which has no reason not to be centered in its own scene.
What I have to do in the current state of things to achieve this is:

  1. save branch as scene
  2. reset the transform of the root in the newly created scene
  3. re-setup the instance that is not in the right position anymore

Describe the feature / enhancement and how it helps to overcome the problem or limitation

Therefore imo it makes sense to add a checkbox somewhere in the "save branch as scene" dialog, titled "reset root transform" and checked by default that if checked does the following:

  1. reset the transform of the root node in the newly created scene so that the scene is at the origin of its "scene space",
  2. set the transform of the instance that replaces the branch to match the original state.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

see above.

If this enhancement will not be used often, can it be worked around with a few lines of script?

I am not sure wether this feature is used often at all. I do use it often. It can't be a script.

Is there a reason why this should be core and not an add-on in the asset library?

I don't know if it can.

@Mickeon
Copy link

Mickeon commented Feb 12, 2022

I'm struggling to find a reason on why this shouldn't be the case by default. The current way leads to the position property to behave unintuitively (on the Scene the operation starts from, position of the branch is (0,0) but the "Restore to Default" symbol shows up because in the newly created Scene, position is, say, (40, 50) ).

@Calinou
Copy link
Member

Calinou commented Feb 12, 2022

I'm struggling to find a reason on why this shouldn't be the case by default. The current way leads to the position property to behave unintuitively (on the Scene the operation starts from, position of the branch is (0,0) but the "Restore to Default" symbol shows up because in the newly created Scene, position is, say, (40, 50) ).

I think having this "reset transform" behavior as the default is a good idea, but I don't know if some people will want the old behavior back.

@Mickeon
Copy link

Mickeon commented Feb 12, 2022

I believe the current behaviour only makes sense if the parent Node isn't Node2D, since it would not be able to inherit the position from anything.

@HolonProduction
Copy link
Member

I tried to implement this (without knowing about the proposal). During the process the following questions were brought up:

  • Are there known workflows which rely on the new scene having a translated root?
  • What about other transformations like rotation or scale?

Given that this proposal is here since more then a year and no usecase against it was brought up, I think that we can answer the first point with no.

However I would like to address the question about other transformations. Taking the idea of checkboxes from this proposal I could imagine having three checkboxes with only the translation checked by default. This would give users controll over the behaviour.

However I don't know where I would place such checkboxes and maybe the additional checkboxes would be distracting and confusing for most users. In that case I think it would be reasonable to just reset position right now.

@Bitlytic
Copy link

Bitlytic commented Nov 16, 2023

I personally believe having a checkbox in the editor settings to use the old way would be a good idea just for the freedom of having the option, but using this proposed solution by default sounds like the a good way to go.

As for things like rotation and scale, I think if someone has scaled and rotated something, that should be applied to the saved scene from the branch. Something like rotation and scale seems very deliberate and intentional, whereas I know a lot of people will position something in the level just to test that it works before saving off as a separate scene.

@HolonProduction
Copy link
Member

I could imagine that scale and rotation would be interesting for saving decoration objects that might have been placed to fit with the rest of a level. But going ahead with translation should be fine. If issues are raised about other transforms, adding them later should be no problem

@nanodeath
Copy link

+1 on this issue. I think this is intuitive, and...it's also the way Unity does it, for precedence.

Something related that perhaps we can piggyback on this same issue is Reparent to New Node does the exact same thing, which is also confusing behavior.

20231201073134.mp4

@Bitlytic
Copy link

Bitlytic commented Dec 1, 2023

+1 on this issue. I think this is intuitive, and...it's also the way Unity does it, for precedence.

Something related that perhaps we can piggyback on this same issue is Reparent to New Node does the exact same thing, which is also confusing behavior.
20231201073134.mp4

Also taking care of that issue here, because I felt the same way
godotengine/godot#84995

@wirelessfuture
Copy link

Did this go anywhere?

@Mickeon
Copy link

Mickeon commented Sep 2, 2024

godotengine/godot#80561 implements this, but it's not been merged yet. It also happens always rather than being an option.

@akien-mga akien-mga added this to the 4.4 milestone Oct 1, 2024
@UrgUrgUrg
Copy link

Would love to see this get added!

@Zireael07
Copy link

You can see the related PR was merged and this is closed?

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

Successfully merging a pull request may close this issue.

10 participants