Add "apply changes to base scene" option when disabling "editable children"

QbieShay commented 1 year ago

Describe the project you are working on

Big Godot game

Describe the problem or limitation you are having in your project

Large scenes in Godot need to be split in order for them to work with multiple people editing the same scene, but it's really clunky to apply changes to sub-scenes that go well with the main scene.

Example: An open-world scale terrain, with a village on it. Scene hierarchy

- main
-- terrain
-- village1
-- forest1
-- village2

Now, if the environment artist modifies the terrain in a way that requires adjustment to village1, there's no way currently to do it in a user friendly way.

This is a pattern adopted in many large project and there's no good support for it.

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

If it was possible to toggle "editable children" but in a way that modifies the base scene, the issue would be solved in an elegant way.

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

Add "apply to base scene" option when toggling off editable children, or when editable children is on.

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

No, it's an editor feature

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

It's an editor feature

Mickeon commented 1 year ago

Simple but neat quality of life. Sometimes it's easier to do this than going back 'n' forth between scenes of the same hierarchy

passivestar commented 1 year ago

I believe this should not only work for when you disable editable children, but also be its own button near the "Editable children" toggle. In addition to @QbieShay scene organization example, this is also convenient when you have a lot of copies of the same base scene and want to tweak them all. Instead of going to the child scene, you make changes to one of the copies, press "Apply" and see the changes reflected on all of the instances. This allows you to work seamlessly in one space with less jumps between the scenes.

So to recap the solution to the initial proposal is:

Add a button here, when you disable editable children:
And here:

Also I want to expand on the proposal by pointing out these things:

Currently you can selectively revert individual properties to their base scene values. You can do it by pressing the nice revert icon:

But you can't selectively apply them. It could be a separate apply icon:

Or it can be put into the right click context menu.

Since scenes can be inherited, it might be useful to be able to choose what scene you want to apply the changes to

Unity solves it in this fashion:

In case of the editable children disable popup window, since there's little space, I suppose this can be a dropdown. For individual properties it can be a list of base scenes in the hierarchy in the right click context menu

Finally, it would be convenient to be able to only see the changed properties to quickly choose what to apply and what to discard. Could be a small toggle button "Show only changed". Inspector already supports property filtering (the search bar on top), that logic can be used for the ui side of this feature

So there are several layers of how advanced this can get, depending on how much control users want. Personally, I consider unity's flexible prefab system to be one of the strongest sides of unity, so I'd love it if any of these changes made it into godot in some shape or form

fire commented 1 year ago

As part of the import and animation teams I support this proposal being a pr.

Flynsarmy commented 1 year ago

(Excuse my terrible photoshop job)

I'd put the ok and cancel buttons on the far right and apply button on the far left. Otherwise there are two potential scenarios that can occur - each pretty horrifying:

You thought you pressed OK and didn't realise you pressed apply. This may come back to bite you later.
You realised you pressed apply by accident but how can you fix that? Does undo work here?

The apply button can be quite dangerous so protecting it against accidental clicks should be a priority. I do like the idea of this PR though. Very useful functionality.

passivestar commented 1 year ago

@Flynsarmy I agree, I just put together some quick sketches to roughly indicate how the things would look in general, but the details need some thought. In fact I'm not sure that the pop up should present reverting as the default option at all (it's probably not gonna be what most people do anyway, doing changes just to revert them is a weird scenario). So it should probably present them as equally viable choices, along the lines of "Would you like to apply the changes or revert them?". And then the buttons would be "Apply Changes", "Revert Changes" and "Cancel"

TokisanGames commented 1 year ago

- main
-- terrain
-- village1
-- forest1
-- village2

We use this structure quite extensively. We make a subscene local, edit the tree, then resave it as a branch. We don't use editable children for scene structure. I don't think it's a good practice as it means the subscene transforms are always wrong. I'd rather save the correct information to disk in the file that owns it. We do use editable children elsewhere for other very specific cases.

An option to save editable changes to the subscene is useful for people who already have a lot of data stored in parent scenes, but I don't see it giving our team any benefit. It's a similar number of clicks. And per above, I wouldn't recommend anyone use editable children for scene structure moving forward.

I think what would be much more useful would be an option on the viewport menu (like snap to grid) that is something like apply changes locally, and another option without children. These could work like this:

apply changes locally: When enabled if you transform a node, it transforms that node wherever it lives in the tree. If it's in a subscene, it modifies the subscene without needing to mess with make local/save branch or editable children. Presumably the changes are queued until saved.
without children: With children is the current behavior. Without would transform the node but not the children. They retain their existing global transform. Countless times I've needed to move a parent without the children and have to go through a multi step process of unparenting, transforming, reparenting.

RE: comments above, Base scene suggests parent scene, or a base class scene this one is derived from, not a child scene, which is where we are discussing saving the changes.

QbieShay commented 1 year ago

We make a subscene local, edit the tree, then resave it as a branch

That's exactly what I'm proposing, just a better UX ^^

passivestar commented 1 year ago

We make a subscene local, edit the tree, then resave it as a branch.

I'm having too much trouble trying to make this work for scenes that have multiple instances in the host scene:

When I resave the branch as a scene the changes aren't reflected on other instances of that scene.
If that scene is already opened in another tab, you get "can't overwrite a scene that is still opened"
When you save the branch as a scene it doesn't have any knowledge of where that original scene was, it just opens the most recent location. You gotta go and manually save in the correct place
While I was playing with it I also got a lot of weird behaviour, like scenes losing their transform values, or sometimes even changes in nested nodes get lost for some reason, I'd have to test this more to see why it's acting weird

Also, if there's any scene inheritance going on, there's no way to apply changes to some specific scene in the inheritance hierarchy like I suggested, as far as I can tell

If your workaround actually works for multiple scene instances, I'd be grateful if you (or anyone else who's using this approach) showed me how, a small demo video would be very nice. I'm using Godot 4.1.1

TokisanGames commented 1 year ago

We make a subscene local, edit the tree, then resave it as a branch.

Note this also works for a scene where changes have already been made with editable children. Right now you can take your edited child, make it local, and save it as a branch, as this proposal was seeking. So as QB mentioned, the proposal is just a user interface change as the functionality is already there. Though I suggested functionality changes.

I'm having too much trouble trying to make this work for scenes that have multiple instances in the host scene:

This is specifically for structuring your game or levels at the highest level. eg. You have a world, with one unique subscene for a village, another for a forest, another for a temple.

- main
-- terrain
-- village1
-- forest1
-- village2

Most games should have only one instance of these types of subscenes. This is where I discourage the use of editable children. The modular structure is used for transforms so the subscene can house the assets but they are placed on a terrain in the main scene. We break it up so that teams can work on subsections independently without conflicts in the scene. However if you're using editable children, all of those transforms are saved in the main scene, defeating the purpose of using the modular structure in the first place. So if using this structure, save the data in the subscenes.

For scenes that you're using multiple instances of; I'm thinking props, rocks, foliage, houses or house components, enemies. This structure doesn't apply. You can use editable children here, where you typically don't want to save these changes to the child scene (so this proposal isn't applicable to this situation at all).

Eg. I have cows with a character body and a mesh child. We have 3 materials for a brown, white, and mixed cow. I have used editable children in the past to be able to swap the material on the child node. However nowadays I added material slots to the parent script and when I drop materials in there, it adds it to the child mesh. I don't need to save these changes to the child scene. The data belongs to the main level so that's where it should be saved.

Also, if there's any scene inheritance going on, there's no way to apply changes to some specific scene in the inheritance hierarchy like I suggested, as far as I can tell

Inheritance typically refers to a scene inheriting from a GLB import. I hope you aren't importing a whole scene full of objects. I do one object per GLB. Or maybe you're referring to setting up scenes that derive from base scenes like OOP classes. We do that as well.

Scenes can be thought of as either an arrangement of objects in a scene like a level, or a single prefab object (like a player character or a rock). Only the latter should have any sort of GLB inheritance or OOP inheritance. Only the former needs this proposal discussion for all the instances I can think of now.

Our player and all of our NPCs/enemies have prefab scenes that begin with a physics body, and they OOP inherit base NPC, and base Unit scenes. Then they have child nodes for state machines, 3d sprite indicators, sounds, and a mesh. The mesh is GLB inherited, which contains the mesh and armature. Then I have all the other mesh elements linked externally: animation, textures, materials.

I have no problem making many instances of these prefab scenes, nor much simpler rock/tree prefabs. Editing children to replace the material or adjusting script settings works fine. If you are trying the make local/edit/save approach on prefab type scenes, you may not be approaching your scene structure the ideal way.

passivestar commented 1 year ago

For scenes that you're using multiple instances of; I'm thinking props, rocks, foliage, houses or house components, enemies. This structure doesn't apply. You can use editable children here, where you typically don't want to save these changes to the child scene (so this proposal isn't applicable to this situation at all)

I wasn't really talking about project architecture though. You're talking about the best way to do variations of props in your example. But my first comment about scene instances is about UX when tweaking values.

Example: you have a bunch of red explosive barrels spread across the level, and you don't like how the shade of red on it looks with the lighting in your main scene (just using material for this example but it applies to basically anything that can be inside: particle systems, decal projectors, anything). To do this effortlessly I should be able to do this without switching scenes. I would do my tweaks on one of the barrels for preview, and hit apply when satisfied with the result.

In Unity, not only you can unpack and repack prefabs (which doesn't work in Godot for multiple instances like I described in the previous comment), but you also get the ability to quickly apply any overriden values to the prefab, and then in addition to that you also get a special "edit in context" mode, which makes it even faster because you don't even have to apply manually.

The point is, this is really about UX and ease of iteration, not project structure

ywmaa commented 1 year ago

Any PR for this ?

meabefir commented 10 months ago

why is this still not in...

Calinou commented 10 months ago

why is this still not in...

The pull request that implements this proposal is still in draft state since there are some problems with the editor not reloading properly. That said, testing that pull request is still welcome if you want to help the feature get merged.

ArdaE commented 6 months ago

I think a better option would have been to have 2 different ways of "instantiating" (i.e. linking/embedding) a scene. The first, like the current behaviour, and the second as a live link. In the live version, changes to the linked nodes would be saved to the base scene automatically whenever the owner scene is saved. That's it.

While the PR allows one to apply changes manually, it's still clunky and error-prone, in my opinion, as one can end up with linked scenes that were meant to be updated but were forgotten.

saminton commented 6 months ago

I think a better option would have been to have 2 different ways of "instantiating" (i.e. linking/embedding) a scene. The first, like the current behaviour, and the second as a live link. In the live version, changes to the linked nodes would be saved to the base scene automatically whenever the owner scene is saved. That's it.

While the PR allows one to apply changes manually, it's still clunky and error-prone, in my opinion, as one can end up with linked scenes that were meant to be updated but were forgotten.

To start off, having 2 different behaviours would introduce complexity, the UI would have to have some way of telling you if your using a linked scene or an embedded scene. And what if your using a link but you want to switch to a embed, do you loose all your changes, do they get applied directly, would there be a UI to choose what items get "pushed" to the main component ?

And second I think that this is a globally a bad idea. It would make it very easy to override the base scene if you're not paying attention to what type it is (linked or embedded) which imo is much worse that forgetting to update the parent scene as it can completely break your base scene if not careful.

As someone coming from a "web" background that has used tools like Sketch and Figma for the last 10 years or so, components work very similarly to what @passivestar proposed at the beginning, they inherit the parent properties and any changes can be "pushed" to the main component or "reset". https://help.figma.com/hc/en-us/articles/360039150733-Apply-overrides-to-instances

ArdaE commented 6 months ago

UI would have to have some way of telling you if your using a linked scene or an embedded scene.

Yes. Godot already uses color to identify instantiated nodes. A different color can identify live-linked nodes.

what if your using a link but you want to switch to a embed, do you loose all your changes, do they get applied directly,

The rule I outlined is very simple. Changes are saved to the linked file when you save the owner scene. That's it.

So, if you modify some of the nodes and then switch to embed (non-live link) without saving the scene, then your changes would only live in the owner scene. But, switching types of linking would be such a rare operation that I wouldn't implement it as a one-step operation. Users can save or discard their changes first, then unlink and re-link as desired.

would there be a UI to choose what items get "pushed" to the main component ?

Will there be a UI in @passivestar's proposed solution to "choose what items get pushed to the main component" when one manually selects "Apply to Base Scene"? Of course not. Under both scenarios, the answer to the question "what if I want to push the changes I made to these 5 nodes but not those 3 other nodes" is "well, you can't".

And second I think that this is a globally a bad idea. It would make it very easy to override the base scene if you're not paying attention to what type it is (linked or embedded) which imo is much worse that forgetting to update the parent scene as it can completely break your base scene if not careful.

The very reason you label this idea as "bad" is precisely what makes it great: The base scene is updated even if you're not paying attention. That's the whole point. Why would you live-link a file if you don't want it to be foolproof.

As someone coming from a "web" background that has used tools like Sketch and Figma for the last 10 years or so, components work very similarly to what @passivestar proposed at the beginning, they inherit the parent properties and any changes can be "pushed" to the main component or "reset". https://help.figma.com/hc/en-us/articles/360039150733-Apply-overrides-to-instances

I think we can always do better than past tools and methods, especially when in a different context.

QbieShay commented 6 months ago

I don't like the live linking idea. It makes it very easy to accidentally make changes. I prefer having to explicitely say "apply to scene" so i'm free to experiment with it if i need to.

QbieShay commented 6 months ago

To be clear: if live linking scenes is wanted, I think it should be discussed. However this proposal is about applying changes deliberately at the user's discretion. I don't think they are the same feature. I think that the proposed live linking introduces a number of uncertainties on what is going to happen if there's multiple scenes open at the same time, and the editor saves. Those things should be taken into consideration and designed for.

For my usecase i just need to "commit" those changes from time to time to the base scene, when I'm satisfied with what I did.

passivestar commented 6 months ago

Will there be a UI in @passivestar's proposed solution to "choose what items get pushed to the main component" when one manually selects "Apply to Base Scene"?

It would be nice to be able to apply selectively. Like I mentioned above, it could look like this for properties (an apply icon):

And we could also add a button to the Scene dock's context menu to apply changes on selected nodes only

I don't have anything against live linking but like Qbie said it would be better to discuss that in a separate proposal because it's a bigger change. 2 different ways to instantiate scenes would mean bigger learning curve for users, more UI/UX challenges and changes to the scripting API. It'd have to be a big rework if people reach an agreement on design

ArdaE commented 6 months ago

Will there be a UI in @passivestar's proposed solution to "choose what items get pushed to the main component" when one manually selects "Apply to Base Scene"?

It would be nice to be able to apply selectively. Like I mentioned above, it could look like this for properties (an apply icon):

Interesting. I missed that one above and didn't get it from the PR. Live-linking is a much simpler case that wouldn't have any selective apply. It would always be all or nothing.

There's no reason why you can't have both solutions though. Selective apply makes this current solution valuable enough to have.

godotengine / godot-proposals