-
-
Notifications
You must be signed in to change notification settings - Fork 21.5k
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
base: master
Are you sure you want to change the base?
Conversation
- 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.
Nice. Just out of curiosity, would this affect performance of scene instantiating? |
Not in release builds, as |
I noticed that the |
@anissen This is because those values contain Likewise, trailing commas in function declarations and calls could be supported in GDScript (see #18151). |
Indeed, it looks weird right now:
|
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). |
@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. |
Nested arrays do look weird, but for all other cases, it's a marked improvement in tscn readability. |
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.
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 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:
|
I do agree this needs a limit of elements to use this format, when past it it should remain single line. |
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
After