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

Indent dictionaries and arrays in text scene/resource formats #32124

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Calinou
Copy link
Member

@Calinou Calinou commented Sep 13, 2019

  • Write each value on its own line in arrays, dictionaries and variable-size constructors for better VCS diffs.
  • Add trailing commas in arrays, dictionaries and variable-size constructors for better VCS diffs.
  • Write the groups array on the same line for better readability, since it's part of the section header.
  • Remove spaces before and after parentheses for better readability.

Note that tabs were used in indentation to be consistent with the C++ and GDScript style guide. Remember that tabs appear as 8 spaces wide on GitHub, but most text editors will display them as 4 spaces wide.

This should be fully backwards-compatible, but please test in your projects for possible regressions 🙂

It'd be nice to figure out a way to indent nested arrays/dictionaries properly too.

This closes #16051. This closes #16783.

Comparison of a saved scene

Before

[gd_scene load_steps=14 format=2]

[ext_resource path="res://icon.png" type="Texture" id=1]
[ext_resource path="res://Node2D.gd" type="Script" id=2]
[ext_resource path="res://RigidBody2D.gd" type="Script" id=3]

[sub_resource type="RectangleShape2D" id=1]
extents = Vector2( 24.71, 22.51 )

[sub_resource type="OccluderPolygon2D" id=2]
polygon = PoolVector2Array( 85.9506, 411.251, 112.55, 492.001, 38.7991, 523.2, -67.9497, 561.351, -121.15, 542.351, -152.5, 525.251, -144.9, 493.901 )

[sub_resource type="ShaderMaterial" id=3]

[sub_resource type="StyleBoxFlat" id=4]
corner_radius_top_left = 8
corner_radius_top_right = 8
corner_radius_bottom_right = 8
corner_radius_bottom_left = 8

[sub_resource type="SpatialMaterial" id=5]
albedo_color = Color( 0.607843, 0.607843, 0.607843, 1 )

[sub_resource type="CubeMesh" id=6]
material = SubResource( 5 )

[sub_resource type="Animation" id=7]
loop = true
tracks/0/type = "value"
tracks/0/path = NodePath("Sprite2:position")
tracks/0/interp = 1
tracks/0/loop_wrap = true
tracks/0/imported = false
tracks/0/enabled = true
tracks/0/keys = {
"times": PoolRealArray( 0, 1 ),
"transitions": PoolRealArray( 1, 1 ),
"update": 0,
"values": [ Vector2( 695, 496 ), Vector2( 866, 442 ) ]
}

[sub_resource type="BoxShape" id=8]

[sub_resource type="ParticlesMaterial" id=9]
flag_disable_z = true
gravity = Vector3( 0, 98, 0 )
orbit_velocity = 0.0
orbit_velocity_random = 0.0

[sub_resource type="SpriteFrames" id=10]
animations = [ {
"frames": [ ExtResource( 1 ), ExtResource( 1 ) ],
"loop": true,
"name": "default",
"speed": 5.0
} ]

[node name="Node2D" type="Node2D" groups=[
"some_group",
]]
position = Vector2( -79, -32 )
script = ExtResource( 2 )
__meta__ = {
"_edit_horizontal_guides_": [ -179.616 ],
"_edit_vertical_guides_": [ 63.6369 ]
}

[node name="RigidBody2D" type="RigidBody2D" parent="." groups=[
"some_group",
]]
position = Vector2( 796.095, 187.746 )
linear_velocity = Vector2( 0, 200 )
script = ExtResource( 3 )
__meta__ = {
"_edit_group_": true
}

[node name="CollisionPolygon2D" type="CollisionPolygon2D" parent="RigidBody2D" groups=[
"some_group",
]]
position = Vector2( 177.537, 13.2786 )
rotation = 1.12923
build_mode = 1
polygon = PoolVector2Array( 48, -34, 31, 33, -36, 32, -22.1588, -8.28403, -37, -36, -1, -60, 29.4206, -60.8425 )
one_way_collision = true

[node name="Sprite" type="Sprite" parent="RigidBody2D" groups=[
"some_group",
"test",
]]
position = Vector2( 0, -2.10803 )
texture = ExtResource( 1 )

[node name="RigidBody2D2" type="RigidBody2D" parent="."]
position = Vector2( 1086.09, 189.746 )
linear_velocity = Vector2( 0, 200 )
linear_damp = 100.0
__meta__ = {
"_edit_group_": true
}

[node name="CollisionPolygon2D" type="CollisionPolygon2D" parent="RigidBody2D2"]
position = Vector2( 177.537, 13.2786 )
rotation = 1.12923
build_mode = 1
polygon = PoolVector2Array( 48, -34, 31, 33, -36, 32, -22.1588, -8.28403, -37, -36, -1, -60, 29.4206, -60.8425 )
one_way_collision = true

[node name="Sprite" type="Sprite" parent="RigidBody2D2"]
position = Vector2( 0, -2.10803 )
texture = ExtResource( 1 )

[node name="StaticBody2D" type="StaticBody2D" parent="."]
position = Vector2( 491.095, 289.746 )

[node name="CollisionShape2D" type="CollisionShape2D" parent="StaticBody2D"]
position = Vector2( -0.0526123, 99.2191 )
shape = SubResource( 1 )

[node name="LightOccluder2D" type="LightOccluder2D" parent="."]
position = Vector2( 558.201, -346.2 )
occluder = SubResource( 2 )

[node name="Sprite" type="Sprite" parent="."]
position = Vector2( 247, 29 )
rotation = 0.261799
texture = ExtResource( 1 )
offset = Vector2( 61.2328, 261.126 )

[node name="Control" type="Control" parent="."]
__meta__ = {
"_edit_use_anchors_": false
}

[node name="SpinBox" type="SpinBox" parent="Control"]
anchor_top = 1.0
anchor_bottom = 1.0
margin_left = 65.0
margin_top = -85.0
margin_right = 139.0
margin_bottom = -61.0
__meta__ = {
"_edit_use_anchors_": false
}

[node name="Polygon2D" type="Polygon2D" parent="."]
material = SubResource( 3 )
position = Vector2( 72.892, 24 )
texture = ExtResource( 1 )
polygon = PoolVector2Array( 734.366, 109.02, 844.061, 200.987, 834.089, 347.247, 759, 402, 678.964, 327.302, 666.56, 214.201 )
uv = PoolVector2Array( 734.366, 109.02, 844.061, 200.987, 834.089, 347.247, 759, 384, 678.964, 327.302, 666.56, 214.201 )

[node name="Camera" type="Camera" parent="."]
transform = Transform( 1, 0, 0, 0, -0.0156461, 0.999878, 0, -0.999878, -0.0156461, 0, 0, 0 )

[node name="Button" type="Button" parent="."]
margin_left = 407.0
margin_top = 243.0
margin_right = 439.0
margin_bottom = 263.0
custom_styles/normal = SubResource( 4 )
text = "hello"
__meta__ = {
"_edit_use_anchors_": false
}

[node name="MeshInstance" type="MeshInstance" parent="."]
transform = Transform( 1, 0, 0, 0, 1, 0, 0, 0, 1, 4.26556, 0, 0 )
mesh = SubResource( 6 )
material/0 = null

[node name="AnimationPlayer" type="AnimationPlayer" parent="."]
"anims/New Anim" = SubResource( 7 )

[node name="Sprite2" type="Sprite" parent="."]
position = Vector2( 841.716, 449.669 )
texture = ExtResource( 1 )

[node name="Tween" type="Tween" parent="."]

[node name="LineEdit" type="LineEdit" parent="."]
margin_left = 261.0
margin_top = 56.0
margin_right = 319.0
margin_bottom = 80.0
editable = false
__meta__ = {
"_edit_use_anchors_": false
}

[node name="TextEdit" type="TextEdit" parent="."]
margin_left = 139.0
margin_top = 53.0
margin_right = 179.0
margin_bottom = 93.0
readonly = true
__meta__ = {
"_edit_use_anchors_": false
}

[node name="Node" type="Node" parent="."]

[node name="FileDialog" type="FileDialog" parent="."]
visible = true
margin_left = 224.0
margin_top = 102.0
margin_right = 679.0
margin_bottom = 400.0
rect_min_size = Vector2( 400, 140 )
current_file = "3d.tscn"
current_path = "res://3d.tscn"
__meta__ = {
"_edit_use_anchors_": false
}

[node name="RayCast" type="RayCast" parent="."]
transform = Transform( 1, 0, 0, 0, 1, 0, 0, 0, 1, 0, 2, 3 )

[node name="RayCast2" type="RayCast" parent="."]
transform = Transform( 1, 0, 0, 0, 1, 0, 0, 0, 1, 0, 2, 2 )
enabled = true

[node name="CollisionShape" type="CollisionShape" parent="."]
transform = Transform( 1, 0, 0, 0, 1, 0, 0, 0, 1, 0, 2, -3 )
shape = SubResource( 8 )

[node name="CollisionShape2" type="CollisionShape" parent="."]
transform = Transform( 1, 0, 0, 0, 1, 0, 0, 0, 1, 0, 2, -6 )
shape = SubResource( 8 )
disabled = true

[node name="CollisionPolygon" type="CollisionPolygon" parent="."]
polygon = PoolVector2Array( -14.2938, 2.25676, -22.0017, 0.105297, -19.431, -3.78434, -2.85363, 3.25932 )

[node name="CPUParticles2D" type="CPUParticles2D" parent="."]
visible = false
texture = ExtResource( 1 )

[node name="Particles2D" type="Particles2D" parent="."]
visible = false
position = Vector2( 150, 190 )
process_material = SubResource( 9 )
texture = ExtResource( 1 )

[node name="TheNodeWithAnUnnecessaryLongName" type="AnimatedSprite" parent="."]
visible = false
position = Vector2( 150, 190 )
frames = SubResource( 10 )

[node name="AudioStreamPlayer" type="AudioStreamPlayer" parent="."]

[node name="SoftBody" type="SoftBody" parent="."]

[node name="TextEdit2" type="TextEdit" parent="."]
margin_left = 771.0
margin_top = 268.0
margin_right = 1026.0
margin_bottom = 535.0
__meta__ = {
"_edit_use_anchors_": false
}
[connection signal="frame_changed" from="RigidBody2D/Sprite" to="." method="_on_Sprite_frame_changed"]
[connection signal="draw" from="RigidBody2D2/CollisionPolygon2D" to="." method="_on_CollisionPolygon2D_draw"]

After

[gd_scene load_steps=14 format=2]

[ext_resource path="res://icon.png" type="Texture" id=1]
[ext_resource path="res://Node2D.gd" type="Script" id=2]
[ext_resource path="res://RigidBody2D.gd" type="Script" id=3]

[sub_resource type="RectangleShape2D" id=1]
extents = Vector2(24.71, 22.51)

[sub_resource type="OccluderPolygon2D" id=2]
polygon = PoolVector2Array(
	85.9506, 411.251,
	112.55, 492.001,
	38.7991, 523.2,
	-67.9497, 561.351,
	-121.15, 542.351,
	-152.5, 525.251,
	-144.9, 493.901
)

[sub_resource type="ShaderMaterial" id=3]

[sub_resource type="StyleBoxFlat" id=4]
corner_radius_top_left = 8
corner_radius_top_right = 8
corner_radius_bottom_right = 8
corner_radius_bottom_left = 8

[sub_resource type="SpatialMaterial" id=5]
albedo_color = Color(0.607843, 0.607843, 0.607843, 1)

[sub_resource type="CubeMesh" id=6]
material = SubResource(5)

[sub_resource type="Animation" id=7]
loop = true
tracks/0/type = "value"
tracks/0/path = NodePath("Sprite2:position")
tracks/0/interp = 1
tracks/0/loop_wrap = true
tracks/0/imported = false
tracks/0/enabled = true
tracks/0/keys = {
	"times": PoolRealArray(
	0,
	1
),
	"transitions": PoolRealArray(
	1,
	1
),
	"update": 0,
	"values": [
	Vector2(695, 496),
	Vector2(866, 442),
],
}

[sub_resource type="BoxShape" id=8]

[sub_resource type="ParticlesMaterial" id=9]
flag_disable_z = true
gravity = Vector3(0, 98, 0)
orbit_velocity = 0.0
orbit_velocity_random = 0.0

[sub_resource type="SpriteFrames" id=10]
animations = [
	{
	"frames": [
	ExtResource(1),
	ExtResource(1),
],
	"loop": true,
	"name": "default",
	"speed": 5.0,
},
]

[node name="Node2D" type="Node2D" groups=["some_group"]]
position = Vector2(-79, -32)
script = ExtResource(2)
__meta__ = {
	"_edit_horizontal_guides_": [
	-179.616,
],
	"_edit_vertical_guides_": [
	63.6369,
],
}

[node name="RigidBody2D" type="RigidBody2D" parent="." groups=["some_group"]]
position = Vector2(796.095, 187.746)
linear_velocity = Vector2(0, 200)
script = ExtResource(3)
__meta__ = {
	"_edit_group_": true,
}

[node name="CollisionPolygon2D" type="CollisionPolygon2D" parent="RigidBody2D" groups=["some_group"]]
position = Vector2(177.537, 13.2786)
rotation = 1.12923
build_mode = 1
polygon = PoolVector2Array(
	48, -34,
	31, 33,
	-36, 32,
	-22.1588, -8.28403,
	-37, -36,
	-1, -60,
	29.4206, -60.8425
)
one_way_collision = true

[node name="Sprite" type="Sprite" parent="RigidBody2D" groups=["some_group", "test"]]
position = Vector2(0, -2.10803)
texture = ExtResource(1)

[node name="RigidBody2D2" type="RigidBody2D" parent="."]
position = Vector2(1086.09, 189.746)
linear_velocity = Vector2(0, 200)
linear_damp = 100.0
__meta__ = {
	"_edit_group_": true,
}

[node name="CollisionPolygon2D" type="CollisionPolygon2D" parent="RigidBody2D2"]
position = Vector2(177.537, 13.2786)
rotation = 1.12923
build_mode = 1
polygon = PoolVector2Array(
	48, -34,
	31, 33,
	-36, 32,
	-22.1588, -8.28403,
	-37, -36,
	-1, -60,
	29.4206, -60.8425
)
one_way_collision = true

[node name="Sprite" type="Sprite" parent="RigidBody2D2"]
position = Vector2(0, -2.10803)
texture = ExtResource(1)

[node name="StaticBody2D" type="StaticBody2D" parent="."]
position = Vector2(491.095, 289.746)

[node name="CollisionShape2D" type="CollisionShape2D" parent="StaticBody2D"]
position = Vector2(-0.0526123, 99.2191)
shape = SubResource(1)

[node name="LightOccluder2D" type="LightOccluder2D" parent="."]
position = Vector2(558.201, -346.2)
occluder = SubResource(2)

[node name="Sprite" type="Sprite" parent="."]
position = Vector2(247, 29)
rotation = 0.261799
texture = ExtResource(1)
offset = Vector2(61.2328, 261.126)

[node name="Control" type="Control" parent="."]
__meta__ = {
	"_edit_use_anchors_": false,
}

[node name="SpinBox" type="SpinBox" parent="Control"]
anchor_top = 1.0
anchor_bottom = 1.0
margin_left = 65.0
margin_top = -85.0
margin_right = 139.0
margin_bottom = -61.0
__meta__ = {
	"_edit_use_anchors_": false,
}

[node name="Polygon2D" type="Polygon2D" parent="."]
material = SubResource(3)
position = Vector2(72.892, 24)
texture = ExtResource(1)
polygon = PoolVector2Array(
	734.366, 109.02,
	844.061, 200.987,
	834.089, 347.247,
	759, 402,
	678.964, 327.302,
	666.56, 214.201
)
uv = PoolVector2Array(
	734.366, 109.02,
	844.061, 200.987,
	834.089, 347.247,
	759, 384,
	678.964, 327.302,
	666.56, 214.201
)

[node name="Camera" type="Camera" parent="."]
transform = Transform(1, 0, 0, 0, -0.0156461, 0.999878, 0, -0.999878, -0.0156461, 0, 0, 0)

[node name="Button" type="Button" parent="."]
margin_left = 407.0
margin_top = 243.0
margin_right = 439.0
margin_bottom = 263.0
custom_styles/normal = SubResource(4)
text = "hello"
__meta__ = {
	"_edit_use_anchors_": false,
}

[node name="MeshInstance" type="MeshInstance" parent="."]
transform = Transform(1, 0, 0, 0, 1, 0, 0, 0, 1, 4.26556, 0, 0)
mesh = SubResource(6)
material/0 = null

[node name="AnimationPlayer" type="AnimationPlayer" parent="."]
"anims/New Anim" = SubResource(7)

[node name="Sprite2" type="Sprite" parent="."]
position = Vector2(841.716, 449.669)
texture = ExtResource(1)

[node name="Tween" type="Tween" parent="."]

[node name="LineEdit" type="LineEdit" parent="."]
margin_left = 261.0
margin_top = 56.0
margin_right = 319.0
margin_bottom = 80.0
editable = false
__meta__ = {
	"_edit_use_anchors_": false,
}

[node name="TextEdit" type="TextEdit" parent="."]
margin_left = 139.0
margin_top = 53.0
margin_right = 179.0
margin_bottom = 93.0
readonly = true
__meta__ = {
	"_edit_use_anchors_": false,
}

[node name="Node" type="Node" parent="."]

[node name="FileDialog" type="FileDialog" parent="."]
visible = true
margin_left = 224.0
margin_top = 102.0
margin_right = 679.0
margin_bottom = 400.0
rect_min_size = Vector2(400, 140)
current_file = "3d.tscn"
current_path = "res://3d.tscn"
__meta__ = {
	"_edit_use_anchors_": false,
}

[node name="RayCast" type="RayCast" parent="."]
transform = Transform(1, 0, 0, 0, 1, 0, 0, 0, 1, 0, 2, 3)

[node name="RayCast2" type="RayCast" parent="."]
transform = Transform(1, 0, 0, 0, 1, 0, 0, 0, 1, 0, 2, 2)
enabled = true

[node name="CollisionShape" type="CollisionShape" parent="."]
transform = Transform(1, 0, 0, 0, 1, 0, 0, 0, 1, 0, 2, -3)
shape = SubResource(8)

[node name="CollisionShape2" type="CollisionShape" parent="."]
transform = Transform(1, 0, 0, 0, 1, 0, 0, 0, 1, 0, 2, -6)
shape = SubResource(8)
disabled = true

[node name="CollisionPolygon" type="CollisionPolygon" parent="."]
polygon = PoolVector2Array(
	-14.2938, 2.25676,
	-22.0017, 0.105297,
	-19.431, -3.78434,
	-2.85363, 3.25932
)

[node name="CPUParticles2D" type="CPUParticles2D" parent="."]
visible = false
texture = ExtResource(1)

[node name="Particles2D" type="Particles2D" parent="."]
visible = false
position = Vector2(150, 190)
process_material = SubResource(9)
texture = ExtResource(1)

[node name="TheNodeWithAnUnnecessaryLongName" type="AnimatedSprite" parent="."]
visible = false
position = Vector2(150, 190)
frames = SubResource(10)

[node name="AudioStreamPlayer" type="AudioStreamPlayer" parent="."]

[node name="SoftBody" type="SoftBody" parent="."]

[node name="TextEdit2" type="TextEdit" parent="."]
margin_left = 771.0
margin_top = 268.0
margin_right = 1026.0
margin_bottom = 535.0
__meta__ = {
	"_edit_use_anchors_": false,
}
[connection signal="frame_changed" from="RigidBody2D/Sprite" to="." method="_on_Sprite_frame_changed"]
[connection signal="draw" from="RigidBody2D2/CollisionPolygon2D" to="." method="_on_CollisionPolygon2D_draw"]

- Write each value on its own line in arrays, dictionaries
  and variable-size constructors for better VCS diffs.
- Add trailing commas in arrays, dictionaries and variable-size constructors
  for better VCS diffs.
- Write the groups array on the same line for better readability,
  since it's part of the section header.
- Remove spaces before and after parentheses for better readability.

This closes godotengine#16051. This closes godotengine#16783.
@girng
Copy link

girng commented Sep 18, 2019

Nice. Just out of curiosity, would this affect performance of scene instantiating?

@bojidar-bg
Copy link
Contributor

Not in release builds, as .tscn-s are converted to .scn-s. Otherwise, it would only affect the performance of initially loading packed scenes.

@anissen
Copy link
Contributor

anissen commented Sep 18, 2019

I noticed that the polygon, keys:times and keys:transitions arrays in your example does not have a trailing comma. Just a heads-up if you missed those parts :)

@Calinou
Copy link
Member Author

Calinou commented Sep 18, 2019

@anissen This is because those values contain PoolByteArray()s, which currently don't accept trailing commas (like all constructors). This could be changed in a future PR, which will allow us to get better diffs for pooled arrays.

Likewise, trailing commas in function declarations and calls could be supported in GDScript (see #18151).

@akien-mga
Copy link
Member

It'd be nice to figure out a way to indent nested arrays/dictionaries properly too.

Indeed, it looks weird right now:

tracks/0/keys = {
	"times": PoolRealArray(
	0,
	1
),
	"transitions": PoolRealArray(
	1,
	1
),
	"update": 0,
	"values": [
	Vector2(695, 496),
	Vector2(866, 442),
],
}

@akien-mga
Copy link
Member

At this stage I think it's best to leave this for 4.0, to review and merge together with other changes to the way data is serialized in tscn (e.g. removing extra whitespace as in #32313).

Another thing that could be worth having for 4.0 is wrapping long lines (typically in PoolByteArrays with hundreds of values).

@aaronfranke
Copy link
Member

@Calinou Is this still desired? If so, it needs to be rebased on the latest master branch.

tracks/0/keys = {
"times": PoolRealArray( 0, 1 ),
"transitions": PoolRealArray( 1, 1 ),
tracks/0/keys = {
	"times": PoolRealArray(
	0,
	1
),
	"transitions": PoolRealArray(
	1,
	1
),

IMO this is not an improvement, it looks much worse to me and takes up vastly more vertical space. Instead of a newline for each item, I think it should do a newline every 10 or so items, or maybe based on the width of the line.

@Zireael07
Copy link
Contributor

Nested arrays do look weird, but for all other cases, it's a marked improvement in tscn readability.

@akien-mga
Copy link
Member

Likewise, trailing commas in function declarations and calls could be supported in GDScript (see #18151).

This is now supported for 4.0, so you could use it.

Nested arrays/dictionaries still need a better indentation to match how you'd do it e.g. in GDScript.

Instead of a newline for each item, I think it should do a newline every 10 or so items, or maybe based on the width of the line.

I tend to agree that splitting every element on its own line can create files with a huge amount of lines without clear benefit, if you think e.g. of the data serialization of a tilemap.

For example this tile_data PoolIntArray: https://github.com/KOBUGE-Games/jetpaca/blob/master/stages/world_1/forest_fun.tscn#L72
would now take 13,560 lines.

I think it would be better to implement a sensible limit for the number of items per line, either something like 10 per line as suggested by @aaronfranke, or wrapped based on the line length (e.g. max length 240 or something).

Overall, there are various good improvements here which might be consensual enough, so it might be worth splitting those changes into separate PRs that can be merged each in their own time depending on consensus. For example I can see those two as fairly consensual:

  • Write the groups array on the same line for better readability, since it's part of the section header.
  • Remove spaces before and after parentheses for better readability.

@YuriSizov YuriSizov requested a review from a team August 24, 2021 22:22
@reduz
Copy link
Member

reduz commented Aug 31, 2021

I do agree this needs a limit of elements to use this format, when past it it should remain single line.

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