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

Make scripts and shaders embedded in .tscn be more readable and diffable #6749

Closed
raould opened this issue Oct 8, 2016 · 13 comments
Closed

Comments

@raould
Copy link

raould commented Oct 8, 2016

Operating system or device - Godot version:
Win 7 32 bit Pro; Linux Ubuntu 16.04 64 bit.
Godot 2.1 in both cases.
My project is stored in a Git repo (hosted on Bitbucket).

Issue description (what happened, and what was expected):
The files are saved in a format that (1) doesn't let humans easily study them, nor (2) let automatic diff tools understand them.

Personally I think the .tscn file should be split up into a parent folder and individual files underneath it. Putting the entire scene with all the various GDScripts into one file is poor ux.

AND GOD HELP YOU WHEN THERE ARE MERGE CONFLICTS.

Steps to reproduce:

  1. have a Godot project with GDScript files in it.
  2. make changes.
    Expected: to be able to see the individual diffs in a diff tool. Also to be able to ready or edit the file manually easily.
    Actual: Things are not diffable because (1) too many things are stored in a single .tscn file; (2) whitespace inside GDScripts are saved as excape characters. So the diffs become entire giant chunks of entire GDScripts.

Link to minimal example project (optional but very welcome):
See screen shot.
saddiff

@akien-mga
Copy link
Member

akien-mga commented Oct 8, 2016

Personally I think the .tscn file should be split up into a parent folder and individual files underneath it. Putting the entire scene with all the various GDScripts into one file is poor ux.

So... why are you using an embedded script instead of saving your script to a separate file? :)

@williamd1k0
Copy link
Contributor

I think .tscn is actually human readable.
I can read files and diffs easily (saving scripts to a separate file).

@Zylann
Copy link
Contributor

Zylann commented Oct 8, 2016

The only problem TSCN has IMO is this: #6527
Other than that, I find it human readable and easily diffable.

@raould
Copy link
Author

raould commented Oct 9, 2016

@akien-mga because that is what Godot does out of the box, maybe? Are you trying to tell me that I have to learn and perform yet another customization of the system to get it to not do things that are actively breaking basic software engineering principles, tools, and habits?

Thanks for the tip, I will try to go learn about it and set it up. Although I believe it is still a bad UX.

I do not understand if/why it would not be possible for Godot to save .tscn files using real newlines and real tabs etc. in the GDScripts. That is the main problem I see here. (I am manually converting them with sed so that I can diff 2 branches and get back to work.)

@bojidar-bg
Copy link
Contributor

bojidar-bg commented Oct 9, 2016

Anyway, if this isn't a problem for scripts (as @akien-mga said, since embedding the script in the scene is optional and off by default), it is a also problem for shaders (which get embedded in the same ugly way).
I think I would retitle the issue though, since it currently looks as if diff and cat both crash when they open the file... 😆

Amendment: Tagging as core, since that should require VariantParser if I'm not wrong.

@bojidar-bg bojidar-bg changed the title fix .tscn format to be readable & diffable Make scripts and shaders embedded in .tscn be more readable and diffable Oct 9, 2016
@akien-mga
Copy link
Member

because that is what Godot does out of the box, maybe? Are you trying to tell me that I have to learn and perform yet another customization of the system to get it to not do things that are actively breaking basic software engineering principles, tools, and habits?

Ah?
spectacle x24349
I see "Built-In Script" as set to OFF by default personally.

@akien-mga
Copy link
Member

But sure, the code embedded in tscn files could of course be made a bit more readable. But the real solution is still to write your scripts in .gd files and not in .tscn, unless it's just a two-liner shader.

@raould
Copy link
Author

raould commented Oct 15, 2016

@aiken-mga there is more than one way to create a script in godot. One is as you have shown. The other is clicking on a node in the tree, going down in the list of its properties in the Inspector panel, and seeing the "New GDScript" command.

guess where that puts the new script? :-) :-(

@Zylann
Copy link
Contributor

Zylann commented Oct 16, 2016

@raould in the scene, as every other resource in Godot that is created from the inspector. And every resource created this way can still be made a file if you click the right arrow on the property (to edit the resource inside) and click "Save as". But I find myself script addition could get some improvements.

Would behaving differently and opening the "create script" window make more sense when clicking "Create GDScript"?

@raould
Copy link
Author

raould commented Oct 16, 2016

hi,

If there is any way that by default (as in the least number of clicks, the default values for commands or menus along the way) a file is kept inside the tscn file, then that fits what I am talking about.

so i guess the changes i'd like to see are:
(1) make the tscn file not use escapes for whitespace. as i said (buried by now) above in the thread.
(2) there is no #2.

:-)

@akien-mga
Copy link
Member

Right I had forgotten about the way to create a script in the inspector, I never use that. IMO all ways to create a script should default to external files, and only create embedded scripts when users actively request it.

@RandomShaper
Copy link
Member

I also use built-in scripts a lot and diffs are useless. (For shaders too.)

I've proposed #7002 to try to improve the state of things.

@bojidar-bg
Copy link
Contributor

Closing since #7002 was merged.

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

6 participants