godotengine / godot

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

Editing scenes files (.tscn) in a git-friendly manner #22324

Closed salvob41 closed 3 years ago

salvob41 commented 6 years ago

I usually work in team and when we have to merge branches, it often happens that if one of us added a node. It is a mess to figure it out how to not make it crash irremediably.

In the scenes files - like *.tscn Especially when it says [gd_scene load_steps=6 format=2]

Do we really need to have the load_steps ? can it not be calculated automatically from the editor?

QbieShay commented 5 years ago

Up for this.

jamie-pate commented 4 years ago

I have also noticed grid maps and fields with numbers having their values jump around depending on the OS which saves the file? (windows vs linux)

-"aabb": AABB( -4.5, 0, -7.5, 11, 1e-005, 14 ),
+"aabb": AABB( -4.5, 0, -7.5, 11, 1e-05, 14 ),
-transform = Transform( -4.37114e-008, 0, 1, 0, 1, 0, -1, 0, -4.37114e-008, 38.9742, 0.50733, -21.9354 )
+transform = Transform( -4.37114e-08, 0, 1, 0, 1, 0, -1, 0, -4.37114e-08, 38.9742, 0.50733, -21.9354 )
 data = {
-"cells": PoolIntArray( 0, 0, 6, 1, 0, 6, 2, 0, 6, 3, 0, 6, 4, 0, 6, 5, 0, 6, 6, 0, 6, 65532, 0, 6, 65533, 0, 6, 65534, 0, 6, 65535, 0, 6, 0, 1, 6, 1, 1, 6, 2, 1, 6, 3, 1, 6, 4, 1, 6, 5, 1, 6, 6, 1, 6, 65532, 1, 6, 65533, 1, 6, 65534, 1, 6, 65535, 1, 6, 0, 2, 6, 1, 2, 6, 2, 2, 6, 3, 2, 6, 4, 2, 6, 5, 2, 6, 6, 2, 6, 65532, 2, 6, 65533, 2, 6, 65534, 2, 6, 65535, 2, 6, 0, 3, 6, 1, 3, 6, 2, 3, 6, 3, 3, 6, 4, 3, 6, 5, 3, 6, 6, 3, 6, 65532, 3, 6, 65533, 3, 6, 65534, 3, 6, 65535, 3, 6, 0, 4, 6, 1, 4, 6, 2, 4, 6, 3, 4, 6, 4, 4, 6, 5, 4, 6, 6, 4, 6, 65532, 4, 6, 65533, 4, 6, 65534, 4, 6, 65535, 4, 6, 0, 5, 6, 1, 5, 6, 2, 5, 6, 3, 5, 6, 4, 5, 6, 5, 5, 6, 6, 5, 6, 65532, 5, 6, 65533, 5, 6, 65534, 5, 6, 65535, 5, 6, 65533, 6, 6, 65534, 6, 655366, 65535, 6, 655366, 0, 65522, 7, 1, 65522, 7, 2, 65522, 7, 3, 65522, 7, 4, 65522, 7, 5, 65522, 7, 6, 65522, 7, 65532, 65522, 7, 65533, 65522, 7, 65534, 65522, 7, 65535, 65522, 7, 0, 65523, 7, 1, 65523, 7, 2, 65523, 7, 3, 65523, 7, 4, 65523, 7, 5, 65523, 7, 6, 65523, 7, 65532, 65523, 7, 65533, 65523, 7, 65534, 65523, 7, 65535, 65523, 7, 0, 65524, 7, 1, 65524, 7, 2, 65524, 7, 3, 65524, 7, 4, 65524, 7, 5, 65524, 7, 6, 65524, 7, 65532, 65524, 7, 65533, 65524, 7, 65534, 65524, 7, 65535, 65524, 7, 0, 65525, 7, 1, 65525, 7, 2, 65525, 7, 3, 65525, 7, 4, 65525, 7, 5, 65525, 7, 6, 65525, 7, 65532, 65525, 7, 65533, 65525, 7, 65534, 65525, 7, 65535, 65525, 7, 0, 65526, 7, 1, 65526, 7, 2, 65526, 7, 3, 65526, 7, 4, 65526, 7, 5, 65526, 7, 6, 65526, 7, 65532, 65526, 7, 65533, 65526, 7, 65534, 65526, 7, 65535, 65526, 7, 0, 65527, 7, 1, 65527, 7, 2, 65527, 7, 3, 65527, 7, 4, 65527, 7, 5, 65527, 7, 6, 65527, 7, 65532, 65527, 7, 65533, 65527, 7, 65534, 65527, 7, 65535, 65527, 7, 0, 65528, 7, 1, 65528, 7, 2, 65528, 7, 3, 65528, 7, 4, 65528, 7, 5, 65528, 7, 6, 65528, 7, 65532, 65528, 7, 65533, 65528, 7, 65534, 65528, 7, 65535, 65528, 7, 0, 65529, 7, 1, 65529, 7, 2, 65529, 7, 3, 65529, 7, 4, 65529, 7, 5, 65529, 7, 6, 65529, 1441799, 65532, 65529, 1441799, 65533, 65529, 1441799, 65534, 65529, 1441799, 65535, 65529, 1441799, 0, 65530, 1441799, 1, 65530, 1441799, 2, 65530, 1441799, 3, 65530, 1441799, 4, 65530, 1441799, 5, 65530, 1441799, 6, 65530, 1441799, 65532, 65530, 1441799, 65533, 65530, 7, 65534, 65530, 7, 65535, 65530, 1441799, 0, 65531, 6, 1, 65531, 6, 2, 65531, 6, 3, 65531, 6, 4, 65531, 6, 5, 65531, 1073741830, 6, 65531, 1073741830, 65532, 65531, 1073741830, 65533, 65531, 1073741830, 65534, 65531, 1073741830, 65535, 65531, 1074397190, 0, 65532, 1073741830, 1, 65532, 1073741830, 2, 65532, 1073741830, 3, 65532, 1073741830, 4, 65532, 1073741830, 5, 65532, 1073741830, 6, 65532, 1073741830, 65532, 65532, 1073741830, 65533, 65532, 1073741830, 65534, 65532, 1073741830, 65535, 65532, 536870918, 0, 65533, 1073741830, 1, 65533, 1073741830, 2, 65533, 1073741830, 3, 65533, 1073741830, 4, 65533, 1073741830, 5, 65533, 1073741830, 6, 65533, 1073741830, 65532, 65533, 1073741830, 65533, 65533, 1073741830, 65534, 65533, 1073741830, 65535, 65533, 1073741830, 0, 65534, 1073741830, 1, 65534, 1073741830, 2, 65534, 1073741830, 3, 65534, -536870906, 4, 65534, -1610612730, 5, 65534, 1073741830, 6, 65534, 1073741830, 65532, 65534, -2147483642, 65533, 65534, 6, 65534, 65534, 1073741830, 65535, 65534, 1073741830, 0, 65535, 1073741830, 1, 65535, 1073741830, 2, 65535, 1073741830, 3, 65535, 1073741830, 4, 65535, 1073741830, 5, 65535, 1073741830, 6, 65535, 1073741830, 65532, 65535, 1073741830, 65533, 65535, 1073741830, 65534, 65535, 1073741830, 65535, 65535, -2147483642 )
+"cells": PoolIntArray( 0, 0, 6, 1, 0, 6, 2, 0, 6, 3, 0, 6, 4, 0, -1610612730, 5, 0, 1073741830, 6, 0, 1073741830, 65532, 0, 1073741830, 65533, 0, 1073741830, 65534, 0, 6, 65535, 0, 6, 0, 1, 6, 1, 1, 1073741830, 2, 1, 1073741830, 3, 1, 1073741830, 4, 1, -2147483642, 5, 1, 1073741830, 6, 1, 1073741830, 65532, 1, 1073741830, 65533, 1, 1073741830, 65534, 1, 1073741830, 65535, 1, 1073741830, 0, 2, -2147483642, 1, 2, 1073741830, 2, 2, 6, 3, 2, 1073741830, 4, 2, 1073741830, 5, 2, 1073741830, 6, 2, -1610612730, 65532, 2, 1073741830, 65533, 2, 1073741830, 65534, 2, 1073741830, 65535, 2, 1073741830, 0, 3, 1073741830, 1, 3, -536870906, 2, 3, 1073741830, 3, 3, 1073741830, 4, 3, 536870918, 5, 3, 1073741830, 6, 3, 1073741830, 65532, 3, 1073741830, 65533, 3, 1073741830, 65534, 3, 1073741830, 65535, 3, 1073741830, 0, 4, 1073741830, 1, 4, 1073741830, 2, 4, 1073741830, 3, 4, 1073741830, 4, 4, 1073741830, 5, 4, 1073741830, 6, 4, 1073741830, 65532, 4, 1073741830, 65533, 4, 1073741830, 65534, 4, 1073741830, 65535, 4, 1073741830, 0, 5, 6, 1, 5, 6, 2, 5, 1073741830, 3, 5, 1073741830, 4, 5, 1073741830, 5, 5, 6, 6, 5, 6, 65532, 5, 6, 65533, 5, 6, 65534, 5, 6, 65535, 5, 6, 65533, 6, 6, 65534, 6, 655366, 65535, 6, 655366, 0, 65522, 7, 1, 65522, 7, 2, 65522, 7, 3, 65522, 7, 4, 65522, 7, 5, 65522, 7, 6, 65522, 7, 65532, 65522, 7, 65533, 65522, 7, 65534, 65522, 7, 65535, 65522, 7, 0, 65523, 7, 1, 65523, 7, 2, 65523, 7, 3, 65523, 7, 4, 65523, 7, 5, 65523, 7, 6, 65523, 7, 65532, 65523, 7, 65533, 65523, 7, 65534, 65523, 7, 65535, 65523, 7, 0, 65524, 7, 1, 65524, 7, 2, 65524, 7, 3, 65524, 7, 4, 65524, 7, 5, 65524, 7, 6, 65524, 7, 65532, 65524, 7, 65533, 65524, 7, 65534, 65524, 7, 65535, 65524, 7, 0, 65525, 7, 1, 65525, 7, 2, 65525, 7, 3, 65525, 7, 4, 65525, 7, 5, 65525, 7, 6, 65525, 7, 65532, 65525, 7, 65533, 65525, 7, 65534, 65525, 7, 65535, 65525, 7, 0, 65526, 7, 1, 65526, 7, 2, 65526, 7, 3, 65526, 7, 4, 65526, 7, 5, 65526, 7, 6, 65526, 7, 65532, 65526, 7, 65533, 65526, 7, 65534, 65526, 7, 65535, 65526, 7, 0, 65527, 7, 1, 65527, 7, 2, 65527, 7, 3, 65527, 7, 4, 65527, 7, 5, 65527, 7, 6, 65527, 7, 65532, 65527, 7, 65533, 65527, 7, 65534, 65527, 7, 65535, 65527, 7, 0, 65528, 7, 1, 65528, 7, 2, 65528, 7, 3, 65528, 7, 4, 65528, 7, 5, 65528, 7, 6, 65528, 7, 65532, 65528, 7, 65533, 65528, 7, 65534, 65528, 7, 65535, 65528, 7, 0, 65529, 7, 1, 65529, 7, 2, 65529, 7, 3, 65529, 7, 4, 65529, 7, 5, 65529, 7, 6, 65529, 1441799, 65532, 65529, 1441799, 65533, 65529, 1441799, 65534, 65529, 1441799, 65535, 65529, 1441799, 0, 65530, 1441799, 1, 65530, 1441799, 2, 65530, 1441799, 3, 65530, 1441799, 4, 65530, 1441799, 5, 65530, 1441799, 6, 65530, 1441799, 65532, 65530, 1441799, 65533, 65530, 7, 65534, 65530, 7, 65535, 65530, 1441799, 0, 65531, 6, 1, 65531, 6, 2, 65531, 6, 3, 65531, 6, 4, 65531, 6, 5, 65531, 6, 6, 65531, 6, 65532, 65531, 6, 65533, 65531, 6, 65534, 65531, 6, 65535, 65531, 655366, 0, 65532, 6, 1, 65532, 6, 2, 65532, 6, 3, 65532, 6, 4, 65532, 6, 5, 65532, 6, 6, 65532, 6, 65532, 65532, 6, 65533, 65532, 6, 65534, 65532, 6, 65535, 65532, 6, 0, 65533, 6, 1, 65533, 6, 2, 65533, 6, 3, 65533, 6, 4, 65533, 6, 5, 65533, 6, 6, 65533, 6, 65532, 65533, 6, 65533, 65533, 6, 65534, 65533, 6, 65535, 65533, 6, 0, 65534, 6, 1, 65534, 6, 2, 65534, 6, 3, 65534, 6, 4, 65534, 6, 5, 65534, 6, 6, 65534, 6, 65532, 65534, 6, 65533, 65534, 6, 65534, 65534, 6, 65535, 65534, 6, 0, 65535, 6, 1, 65535, 6, 2, 65535, 6, 3, 65535, 6, 4, 65535, 6, 5, 65535, 536870918, 6, 65535, 6, 65532, 65535, 536870918, 65533, 65535, -1610612730, 65534, 65535, 536870918, 65535, 65535, 536870918 )
 }

These diffs don't appear to affect the functionality of the GridMap, and the difference between 1e-05 and 1e-005 doesn't seem material for my use cases.

tcoxon commented 4 years ago

I have also noticed grid maps and fields with numbers having their values jump around depending on the OS which saves the file? (windows vs linux)

We're experiencing this too, even on the same OS and the same system. It's been a constant annoyance. I've tested it in 3.1.1 (Windows and Linux) and 3.2 (only Linux so far). I've made a minimal reproducible test case: https://github.com/tcoxon/godot-gh-22324

In our main project we're also seeing our mesh libraries undergo changes like this, however the above test case doesn't seem to reproduce it, and since meshlibs are a binary format my ability to narrow it down is rather limited.

Calinou commented 4 years ago

@tcoxon @jamie-pate This is probably caused by differences in scientific notation handling between binaries compiled with GCC and MinGW. This should be solved in 3.2 thanks to #33376. See also #24137.

jamie-pate commented 4 years ago

Yup that was my pr ;)

akien-mga commented 4 years ago

Fixed by #33376.

jamie-pate commented 4 years ago

Sorry, but #33376 doesn't actually solve this issue. Git merging scene files is still painful due to the other reasons listed above

In the scenes files - like *.tscn Especially when it says [gd_scene load_steps=6 format=2]

Also, using transient numeric identifiers for resource references causes huge problems

ExtResource( 2 ) and SubResource( 3 ) should use a persistent random Id instead to avoid collisions and renumbering in each merge or change

tcoxon commented 4 years ago

@Calinou

This is probably caused by differences in scientific notation handling between binaries compiled with GCC and MinGW.

I might not have been clear. In my case, using the same binary on the same machine creates changes in the .tscn each time you play the scene. Something is behaving nondeterministically (and still affects 3.2).

jamie-pate commented 4 years ago

@Calinou sounds like a separate issue? This issue is concerned with the .tscn format and making it better for source control in general. (I regret polluting it with my semi-related issue earlier :) )

follower commented 4 years ago

@salvob41 I was curious about the answer to your question about load_steps.

From an initial quick look it seems like it may only be used for loading progress feedback?

The value from load_steps gets stored in resources_total here:

https://github.com/godotengine/godot/blob/a17fba3f21241beb68127d30b8d07db267e65e35/scene/resources/resource_format_text.cpp#L913-L917

And resources_total gets (optionally?) used to calculate progress here:

https://github.com/godotengine/godot/blob/a17fba3f21241beb68127d30b8d07db267e65e35/scene/resources/resource_format_text.cpp#L575-L577

And resources_total also gets exposed here:

https://github.com/godotengine/godot/blob/a17fba3f21241beb68127d30b8d07db267e65e35/scene/resources/resource_format_text.cpp#L694-L696

Which makes me wonder if it (aside from no or weird progress feedback) there might be no ill effect if load_steps attribute is removed, incorrect or 0.

mnn commented 3 years ago

Also, using transient numeric identifiers for resource references causes huge problems ExtResource( 2 ) and SubResource( 3 ) should use a persistent random Id instead to avoid collisions and renumbering in each merge or change

This random rearrangement of scene files after each open or save is a huge pain and still present in 3.3.0. In merge/rebase it leads to corrupted files, conflicts even though in one branch nothing was changed by a user in the affected scene. It's virtually impossible to resolve conflicts when a scene was edited in both branches - it means accepting stuff only from one branch (to avoid corruption), throwing away hours of work from a second branch and redoing the changes from the second branch. So much wasted time, Godot is not good at all for working in teams, even tiny ones of just 2 people... :disappointed:

I don't understand how this critical issue can be unsolved after 2 years.

Calinou commented 3 years ago

This should now be discussed in the proposal that was opened a while ago: https://github.com/godotengine/godot-proposals/issues/471

Calinou commented 3 years ago

For people stumbling upon this issue from a search engine, note that this is being addressed by https://github.com/godotengine/godot/pull/50676.