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

Suggestion: Built-In scripts should be removed for v4 (or the limitations better advertised) #31758

Closed
jaykyburz opened this issue Aug 29, 2019 · 35 comments

Comments

@jaykyburz
Copy link

Godot version:
Version 3.1.1 Stable Official

OS/device including version:
Windows 10. (1803)

Issue description:
New to Godot and I've been working on a prototype for about a week. I opted for a style where every scene had just one script that was built in. (rather than a separate .gd file). I thought this might reduce clutter and as my res:// directory have less stuff in it.

I discovered this style is not well supported and I had problems with class_name (not supported), find in files(broken), and others (navigation etc). Some of these issue are listed below.

#29863
#26178
#5535
#28285
#25579
#31364

From the look of things, built in scripts seem to be a problem area and although I like the idea, seem to be more trouble than they are worth.

Anyhow, just letting you know I lost a few hours to these issues, and now have to pull all my scripts out and save them as gd files.

@Xrayez
Copy link
Contributor

Xrayez commented Aug 29, 2019

Also, such built-in scripts are going to be exposed as plain text once exported, which in most cases not desired imo, and they cannot be encrypted (same goes for built-in shaders unfortunately).

@Xrayez
Copy link
Contributor

Xrayez commented Aug 29, 2019

It's also strange how I manage to make existing scripts to become built-in on accident, and then realize I've edited the wrong script. If this is a bug, I'm not even willing to report it because of mentioned issues already present with built-in scripts. 😛

@groud
Copy link
Member

groud commented Aug 29, 2019

I think the feature is great and should not be removed. But indeed it might be good to add a warning in the documentation to avoid problem.

@jaykyburz
Copy link
Author

jaykyburz commented Aug 29, 2019

I don't think a warning in the documentation is sufficient. Should be right here on the dialogue.
image

Should say something like, "Built-in scripts have limited functionality. Read more in the docs. "

@volzhs
Copy link
Contributor

volzhs commented Aug 30, 2019

I like the built-in script also.
it's pretty useful when need a very simple script (like rotating a node) or just for testing.

@marcospb19
Copy link
Contributor

marcospb19 commented Apr 3, 2020

I agree with @jaykyburz

Should be right here on the dialogue.
Should say something like, "Built-in scripts have limited functionality. Read more in the docs. "

It's very disappointing to find out bug by ourselves while we use Godot, if the limitations are correctly advertised, they don't even "look like bugs" anymore (they're just limitations, right?) and the frustration is gone.

4 of the issues listed are still open.

@Calinou
Copy link
Member

Calinou commented Apr 3, 2020

I also think we should remove built-in scripts in 4.0 due to the various limitations and their worse VCS-ability. This would mean it's no longer possible to distribute single "component" files that include everything (scene and scripts), but that doesn't seem to be a goal for most people anyway.

@sulai
Copy link

sulai commented Apr 3, 2020

I love in-built scripts simply because I don't need to care where to save the script file. Additionally, the scene file is self-contained and safe to pass around.

However, I can see the problems in-built scripts cause, and I agree on the VCS issue as well.

If we remove built-in scripts, I would suggest (maybe as an option) that godot takes full control on where to store these scripts in a standardized way. So the user doesn't have take care as of where to store all the scripts belonging to one scene. Even more useful when renaming nodes (the attached script file should rename itself too), or "saving branch as Scene", in which case the scripts of the extracted scene need to be moved to an appropriate folder to comply with the to be defined standard.

In my mind, this is a way to keep most of the comfort and modularity built-in scripts provide.

@magpie514
Copy link

magpie514 commented May 1, 2020

Please don't remove built-in scripts.
The project folder is already a mess with metadata for every image and there are times there is just not sane to add new files to it when a 4-line builtin script does the job.
Not all logic has to be reusable and managed as separate files. Some scripts can be literal one-liners.
Not to mention projects using them heavily will be fatally crippled by this. The maintenance cost would be plain absurd. There's already plenty of work to be done to port a project from 3.2 to 4.
I don't want years of work to go down the drain again. I know some of the developers have an aversion to builtins, but consider users too. If there's a feature offered for me to use and I deem it convenient to use, it's my right to use it, whatever the limitations are.
With that I mean that I pondered the pros and cons and decided that it was still the way to go. Having them just yanked out from my grasp doesn't seem very fair.

@Zireael07
Copy link
Contributor

Not to mention projects using them heavily will be fatally crippled by this. The maintenance cost would be plain absurd

Are there any such projects out there in the wild, though? The only usecase for a built-in is when it's a one-liner or so...

@magpie514
Copy link

magpie514 commented May 1, 2020

Mine sure does. Specially for UI elements that don't require much more than initializing some icon or text label or changing a color based on it being used for an enemy or a player character, or just very simple effects to show stuff like damage numbers progressively writing themselves, or something like a generic lightweight life bar with its own simple _draw() routine.
Of course I'm not going to put extremely complex logic as builtin, but for UI elements, very minor effects, maybe a timer that just needs a nudge, an icon that fades in/out for emphasis...there's really not much of a point to pollute the folder further with such scraps.
Once support is gone, all those elements would need manual review, and they are a lot. The time investment is brutal and there's always a chance of forgetting something and then you just can't figure out why the thing is not right. (We are fallible beings).

@Xrayez
Copy link
Contributor

Xrayez commented May 1, 2020

To be honest there's just too many changes already which justify a complete rewrite of a project from scratch (if you're still at the development stage), and we may expect even more compatibility breakage in 3.2 → 4.0 compared to 2.1 → 3.0...

My previous claim:

Also, such built-in scripts are going to be exposed as plain text once exported

may be outdated because of: #38308

So I'm for better documentation regarding this.

Not to mention I had some ideas of creating test scenes with built-in scripts, but I wouldn't want to use built-in scripts "in production" as they say anyways.

Also that's kinda how the engine works as reduz would say, so removing support might not be trivial anyways.

@magpie514
Copy link

magpie514 commented May 1, 2020

I mean, I'm trying to provide a counterpoint, since it's assumed no one cares/wants this.

My project is a RPG, meaning it's very heavy on menus and small interface nodes that provide info that need to be reused frequently (the node itself, not the script controlling different nodes).
Using the favored method means your folders, no matter how tidy you try to keep them, are always going to have double the number of elements that it should have, and when the project grows large it's not trivial and complicates navigation. Those little conveniences save time and avoid distractions.

If it ends up being a final decision, some tool that can decouple and link the builtins to separate script files should be ideal.

@TicklesTheBrain
Copy link

I don't think game developers should expect to be able to port their complex projects painlessly to a new version of the engine every time a major version comes out. Even commercial engine projects break compat in non-trivial ways. To be able to ship anything, you have to stick to a version and yeah, unfortunately, it sometimes means not being to take advantage of the freshest updates.

This is all to say 'it will be a pain to port built-in scripts' is not a good reason to not get rid of them.

@JayBlueManchu
Copy link

I agree with @TicklesTheBrain. We should not carry a bad feature across major features "just because". The only plus off this feature is that you can have one less file in the filesystem. There are many downsides.

@katoneko
Copy link

My 2 cents on this:

If you're making a big enough project, you're going to end up with a lot of scripts (and other files) in your project folder anyway. Stuffing one-liners into built-ins could spare you a couple of files in the project directory but in the end it wouldn't be much difference.

Removing built-ins could also theoretically promote better project dir organization as the place where you could put a script in a fire-and-forget manner would be gone, which would make you actually think of where to put it.

@mrimvo
Copy link

mrimvo commented May 14, 2020

I would just like to point out another possible way to deal with built-in scripts: Make them work.
I do like to use built-in scripts in my current projects, simply because they make scenes self-contained. You can easily extract nodes from my big game scene and store them to a good place without having to worry about where all the scripts end up. They are contained. It's easy to share scenes to other projects too. You don't need to worry about where to save script files, so creating a new script is a no-brainer. No need to organize and no need to re-organize is a big plus for built-ins.

@Calinou
Copy link
Member

Calinou commented May 14, 2020

@mrimvo Making them work seamlessly with external editors is arguably a difficult task. Let's not forget about external tools such as godot-gdscript-toolkit as well.

You don't need to worry about where to save script files, so creating a new script is a no-brainer.

If your scene only has a single script, it's recommended to place the script in the same folder with the same name (only the file extension will differ). See Project organization in the documentation for details.

@marcospb19
Copy link
Contributor

#37565 is already merged, the warning is very informative, it won't prevent users from using built-in scripts, but instead advise them that there are known bugs (this way, they don't need to confusedly search for help online, or on the Issues page on GitHub, where we are).

I think that warnings for known bugs should be welcome, the worst part of facing bugs, IMO, is not knowing what is happening and spend hours investigating it.

When you know the existence of a bug, you just get around it and enjoy the engine.

@marcospb19
Copy link
Contributor

marcospb19 commented May 15, 2020

The limitations are already warned, but some people will continue using it because it is a useful feature, and they don't want it to be removed. Instead, maybe it should be reworked. I had an idea.

Instead of Built-in Scripts, we can call it Discrete Scripts (Originally I named it Fixed Scripts, but Discrete sounds better)

Discrete Scripts behave almost exactly like Built-in Scripts from the user's eyes, but internally, the engine treats it differently.

What if a Discrete Scripts are stored at a separated folder instead of inside of scenes?

Imagine I'm editing Vehicles/Car.tscn, and I attach a Discrete Script to the child with name Engine, everything behaves like Built-in Scripts, but internally Godot is storing it at "discrete_scripts/Vehicles/Car::Engine.gd" (if the file already contains : in the name, it should be escaped)

Why?
All the benefits from Built-in Scripts would be preserved, useful oneliners that help building levels, no messy folders and the abstraction of not having to deal any extra files, all of this because the folder discrete_scripts would be hidden by default in the Engine FileSystem Panel
Also, Discrete Scripts are VCS-friendly (reviewing .gd scripts changes, instead of .tscn, as @Calilou complained).

Downside: A hidden folder will be created at the root directory of the project.

This practically solves ALL the current issues built-in scripts while preserving the feature, while also making it really easy to integrate with external editors (which I really, really appreciate :D, see #29863).

@h0lley
Copy link

h0lley commented Oct 13, 2020

I suggest to go back to the roots and ask what people really want from built-in scripts. In my understanding, it is to get rid of those .tscn & .gd pairs in the FileSystem panel. After all, the script files can be conveniently accessed from the node list already, so we feel that they do not need to also clutter the FileSystem panel. Of course, this is very much a preference, and other users may see it exactly the other way around.

Built-in scripts have the distinct advantage that they can drastically reduce the amount of files one has to browse in the FileSystem panel, and that they bundle logic together with the one scene or node it is every going to be used with. I think that this functionality definitely has its place and should not be forsaken, but I agree that it should be approached differently: no longer as built-in scripts, but rather as "discrete" scripts that are their own .gd files, but come with most of the benefits of built-in scripts.

Here - at the bottom of the comment - you can find a proposition to solve this that I find is a little bit less convoluted than marcospb19's and mrimvo's concept. Basically, most of the functionality already exists by utilizing the fact that Godot hides files prefixed with ., and all that remains is for Godot to be made aware that .tscn and .gd files that share the same name and folder belong together.

@aaronfranke
Copy link
Member

@Serenitor One problem with your argument is that this situation isn't always the case, there isn't always a pair. It's perfectly possible to have scenes without scripts, or multiple scripts to a scene, or one script in many scenes. The proposal you linked is useful for more than just pairs as it handles the multiple scripts to a scene case, but your solution only handles one script per scene. This isn't a dismissal of your argument, handling one use case is fine if you can argue that it's a strong usability improvement and it's a common enough use case, but just keep in mind that it's often good to build general-purpose tools. For me, the general-purpose solution would be to just use external scripts with no special behavior.

Copy/pasting part of my reply from godotengine/godot-proposals#1274 here, which I still agree with:

I'm 100% in favor of removing built-in scripts entirely. What some users cite as benefits of built-in scripts (mostly seems to be the ability to have simple scripts without their own file), to me are 1) really minor things and 2) things that annoy me (when I'm reviewing diffs on Git, I absolutely do not want GDScript files mixed in with tscn files).

@h0lley
Copy link

h0lley commented Oct 13, 2020

@aaronfranke at this point, I'm entirely with you in thinking that embedding GDScript into scene files has no future.

Though I, like others, still see value in what built-in scripts provide.
You are using an external code editor, correct? I figure you are using VSCode's file explorer most of the time, so the file-hiding functionality of which several variants have been proposed here isn't really relevant to you anyways, which is why it makes sense that these ideas don't seem useful to you. Though I think we can all agree that there's no one size fits all, which is why project and editor settings exist.

The proposal you linked is useful for more than just pairs as it handles the multiple scripts to a scene case, but your solution only handles one script per scene.

True. This is still improvable. Here's what comes to mind:

my_scne.tscn
.my_scene.gd
.my_scene.my_node_a.gd
.my_scene.my_node_b.gd
.my_scene.my_node_c.gd

The .-prefixed .gd files do not appear in the FileSystem panel and are instead accessed through the scene file - which is effectively identical to built-in scripts.

Godot would pick up the rename/move of the my_scne.tscn file, then check for neighboring .gd files starting with .my_scene., and also rename/move them accordingly (only rename the my_scene part). This preserves they contained/bundled nature of built-in scripts (except when someone is moving a scene file out of the project, but I reckon that's fine).

Godot would take care of initially naming a .gd file when a checkbox in the Attach Node Script dialogue is ticked (replacing the Built-In Script checkbox).

I agree that a major undertaking to keep built-in scripts alive is not appropriate, but I think what I am proposing should be well within scope.

@aaronfranke
Copy link
Member

aaronfranke commented Oct 14, 2020

@Serenitor The solution you are proposing has few downsides and I think it's a good idea if the community wants it. I think this feature should be called "bundled scripts", since it makes a script tied to a scene such that they come bundled together.

Broadly speaking, I see two possible ways this could be implemented, so I'm curious which you think is better and which you were thinking of. The first thing I thought of when reading your comment is that the above files you listed would attach to nodes like this:

AnyNamedRootNode -> .my_scene.gd
    my_node_a -> .my_scene.my_node_a.gd
    my_node_b -> .my_scene.my_node_b.gd
    my_node_c -> .my_scene.my_node_c.gd

And if my_node_c had a child named MyScriptTeStiNg123 then a script on it would be called .my_scene.my_node_c.MyScriptTeStiNg123.gd (the casing would have to match and the full node path would be needed). The usage of a . to separate them is a good idea as . is already a forbidden character in node names.

Or alternatively, another implementation I thought of is that any files starting with .my_scene.* would be bundled with the scene, regardless of what their file type is, and could be attached to any node by referencing the script inside of the tscn. So on the MyScriptTeStiNg123 node, the tscn could have script="res://.my_scene.test.gd". This seems more flexible, but maybe less portable, and possibly more confusing because it means there isn't a correlation between the file name and node path. It would also allow for this to work for non-script assets like PNG images and allow bundled assets to be used more than once.

I think this is ready for a proposal if you want to write one (EDIT: and/or a PR).

@marcospb19
Copy link
Contributor

marcospb19 commented Oct 14, 2020

I think this is ready for a proposal if you want to write one.

[This proposal] is totally open for debate and suggestions, I think that the discussion should continue there, as @Serenitor mentioned it in his comment.

Also, what's the difference from his suggestion to discrete scripts? The . prefix? (am I missing something?)

Let's continue the discussion (instead of discarding it).

@h0lley
Copy link

h0lley commented Oct 14, 2020

@aaronfranke @marcospb19
I managed to confuse myself with the two threads.
I have responded to your comments in the thread over at godot-proposals..
Suggest to continue discussion there.

@Shadowblitz16

This comment has been minimized.

@chucklepie
Copy link

chucklepie commented Jan 28, 2021

Could I also suggest that shader scripts be removed from scene files. I believe a scene file should be just that - describes a scene - not be used to embed random code snippets.

It might be a slight pain forcing people to save shader files as scripts, but at least then it promotes reuse and less chance of unnecessary duplication of shaders.

@Shadowblitz16
Copy link

I think the limitations should be removed and it should be kept.
I don't want to have to create a script file for every scene I create

@h0lley
Copy link

h0lley commented Feb 1, 2021

while the issues related to built-in scripts are annoying, they are just that: annoyances.

it's not like the feature is inherently broken. it still provides value & usability, so it would be rather disappointing to see it go.

in my experience, the vast majority of scripts that are not meant to be inherited from - that is to say they do not contain the class_name keyword - are specific to a certain node of a certain scene - and for all of those scripts, the built-in feature makes sense and is helpful. it enables us to encapsulate the script with the one scene it is relevant to. this does not just help a lot with preventing clutter in the FileSystem panel, but is also the underlying idea of how scenes and their sub-resources should be used as units of abstraction, is it not?

@marcospb19
Copy link
Contributor

@Serenitor
I mostly agree with you, except for this part:

it's not like the feature is inherently broken

We discussed about the inherently broken parts of Built-in Scripts design in godotengine/godot-proposals#1274, here is a summary:

  1. Unreliability to use external editors.
  2. Worse VCS-ability.
  3. Harder for the community to develop external tools (may be true for other built-in resources).

IMHO, that isn't just a annoyance, the first one looks like a major flaw. If BIscripts cannot be removed, cause it's useful, then it should be fixed/reworked.

I even like your rework idea, anything that fixes the design and we would have nothing to complain here.

@chucklepie

Could I also suggest that shader scripts be removed from scene files. I believe a scene file should be just that - describes a scene - not be used to embed random code snippets.

I agree, and it sounds simple to uncouple the resources and have them in a folder at the root of the project.
Like, it is simple, preserves built-in functionality, has many benefits for tooling, and would work.

Do we really need to be able to copy .tscn files between different projects? I don't think so, such unusual task shouldn't be prioritized, if you really want to, reorganize your project resources and move them correctly.

@Calinou
Copy link
Member

Calinou commented Nov 4, 2021

Closing, as there is no clear consensus for removing support for built-in scripts as per #54553. The limitations are documented in the script creation dialog now (and using class_name within a built-in script prints a suitable error message).

@Zireael07
Copy link
Contributor

Zireael07 commented Nov 4, 2021

Judging by #54553 (comment), the limitations aren't advertised clearly enough.

Suggestion in that thread:

add an icon or a badge to the script editor that would appear for built-in scripts and when clicked open a short explanation to the nature of the beast? Tool scripts could also use such a badge.

The idea is that when people work on a script, they may pay more attention than when they create it. Plus it can fit more details there about present problems.

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

Successfully merging a pull request may close this issue.