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

.TSCN merging tooling (git merge but for TSCN changes on reimport...) #1281

Closed
RevoluPowered opened this issue Jul 30, 2020 · 4 comments
Closed

Comments

@RevoluPowered
Copy link

Describe the project you are working on:
I cannot say anything more than we have a large number of 3D scenes, due to NDA, they get imported via FBX and other importers.

Describe the problem or limitation you are having in your project:

  • When an importer runs it often can break any scenes which inherit the scene if you import a scene and change the node layout, it can result in your inherited scenes losing or orphaning nodes since the hierarchy is different.
  • When we want to 'merge' two authors or more changes we have to make each content author 'lock' a scene essentially, so only one person can make edits to this scene
  • When the importer's behavior is fixed, there is no way of noticing it's orphaned nodes or fundamental changes which may have broken the inherited scene on reimport.
  • Example previous FBX importer was adding a fake node called /rootnode/ when I fixed the FBX importer this removed any fake nodes, from the scene, thus any 'inherited scenes' broke with colliders and required propagating mass amounts of assets.

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

  • I believe we should implement a git merge driver, to allow us to merge .tscn files
  • this would allow us to compare node paths, and work out when there is a conflict.
  • each 'user-added or edited node' would have a unique id when the imported content would not.
  • this would make the changes diff-able and able to show in git 'hey your project changed these paths'
  • we can then implement a 'merge strategy' so it can in some cases auto-correct merge differences.

Describe how your proposal will work, with code, pseudocode, mockups, and/or diagrams:

  • We'd implement a git merge driver for 'godot projects'
  • the merge driver ideally would do all comparison using an extendable script, outside the merge driver.
  • a comparison would be done by git against base and 'current' reimported file and check against the diff, if the diff makes a large change, and edits 'node paths' significantly (or reparents) then we should make a 'conflict'
  • the conflict would be used with a 'merge driver' the merge driver would try to auto-resolve conflict and compare previous asset commit.

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

  • nope

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

it's a requirement to be able to update your TSCN and reimport without your scenes breaking basically, so we need a good solution to this problem above, the merge driver is but one, but I think it's practical if we even started with a simple bash script to do the work in the beginning to prototype a few 'merge tool solutions' this IMHO would bridge the 'reimport breaks things' problem in Godot and make 3D more practical when you have thousands of assets.

@RevoluPowered
Copy link
Author

@RevoluPowered
Copy link
Author

Example use case 1:

  • FBX importer fixes buggy import code to remove extra node /fake_root_node from scene
  • FBX importer corrected it's behaviour.
  • Reimport our game
  • Sub scenes are broken we had a collider on something under fake_root_node and the node is orphaned

Example use case 2:

  • are a large company
  • have 1000 assets
  • have one master scene 'space invaders.tscn' all artists must work in this file

@RevoluPowered
Copy link
Author

9:00 PM it seems like something which should be done outside git
9:00 PM i just don't know how to do it
9:00 PM :P
9:01 PM if you have a large game with a single scene in vk for example
9:01 PM and 10 artists
9:01 PM they all gotta work on subscenes and 1 can only work on the 'master'
9:01 PM 
<Akien> Rémi Verschelde Yeah a scene merging tool would be great.
9:01 PM 
<Xrayez[m]> @xrayez:matrix.org I can only imagine, never worked in teams myself...
9:01 PM 
<RevoluPowered> Gordon so it 'locks' and doesn't scale for the commercial bretherin
9:01 PM 
<iFire> RevoluPowered: can you define the cases for
9:01 PM CRUD.
9:01 PM Create, Read, Update, Delete
9:02 PM 
<Akien> Rémi Verschelde But I wouldn't use Git for that.
9:02 PM 
<iFire> it's like a generic tscn merge tool
9:02 PM 
<Akien> Rémi Verschelde You have the full blown Godot that can actually make sense of the data, so you can show what each scene is like, and let users pick the changes they want.
9:02 PM 
<iFire> When we have two scenes, what are the operations we need for CRUD
9:02 PM 
<RevoluPowered> Gordon yeah would be cool
9:02 PM like any way of doing it im game
9:02 PM driver is overkill
9:02 PM i think
9:03 PM 
<iFire> I looked into this, ask me anytime
9:03 PM 
<RevoluPowered> Gordon possibly TSCN format changes are required to make it work so it's why i mentioned it now
9:03 PM 
<Akien> Rémi Verschelde I would show the scene tree of each scene, with some way to see which properties differ (changed, added, removed)
9:04 PM Then you can move nodes from one to the other, choose what property changes to use if some conflict, and you get a third scene which is the merge of the two.
9:04 PM 
<RevoluPowered> Gordon yeah this would be nice
9:04 PM we could have a 'orphaned nodes' box
9:04 PM 
<Akien> Rémi Verschelde That would also work for binary scenes too.
9:04 PM 
<RevoluPowered> Gordon for anything in 'conflict' which breaks compat
9:05 PM Akien: dragging and dropping sounds good if the node paths are different
9:06 PM possibly foreach( nodepath? in the tscn) , diff against the foreach (nodepath in the old version?)
9:06 PM then just in each foreach have diff_property() -> lookup for props on other nodes
9:07 PM then we would need move detection which could be done somehow
9:07 PM so then if SomeINheritedFBXLamp.tscn -> updates it conflicts on reimport? -> then you can have 'Fix assets' dialog
9:08 PM this is just ramblings but I don't see the whole picture yet

knightofiam added a commit to forerunnergames/coa that referenced this issue Jun 8, 2022
Redo tree tilemaps & colliders in Godot editor after integrating new
modularized player animation system. This is necessary because of
conflicts in the scene file when merging branches - there is as of yet
no coherent way to deal with internal resource id conflicts.

For more information about scene file merging issues in Godot, see:

godotengine/godot-proposals#1281
godotengine/godot#50693
@RevoluPowered
Copy link
Author

closing due to lack of activity

@RevoluPowered RevoluPowered closed this as not planned Won't fix, can't repro, duplicate, stale Feb 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants