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

Remove load_steps in text resource formats #11802

Open
Meorge opened this issue Feb 18, 2025 · 2 comments
Open

Remove load_steps in text resource formats #11802

Meorge opened this issue Feb 18, 2025 · 2 comments

Comments

@Meorge
Copy link

Meorge commented Feb 18, 2025

Describe the project you are working on

A VR game in Godot, which I am using Git version control for

Describe the problem or limitation you are having in your project

I often make multiple changes to my project within a single session that I realize after the fact would make sense to split into multiple commits.
For example, recently I added a ColorRect to a scene to act as a background, and also added some other objects to the scene. While I could go in with my Git client and individually stage + commit these changes in different commits, the load_steps property of the tscn file would briefly be out of sync.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

From what I recall a Godot maintainer saying, as well as looking into the Godot source code myself, load_steps only serves to help the engine display a progress bar as the text resource file is being loaded. The engine appears to read through files in a single pass, fully parsing and loading each resource/"load step" as it comes across it.

My proposed solution is to remove load_steps from the text resource format header, and use a process like the one described below to calculate load_steps at runtime.

Warning

I don't think this would be a breaking change for the engine itself. It could simply ignore the load_steps property in existing text resource files.

However, it could be a breaking change for external/third-party tools that others have developed to parse Godot text resource files, if they do something that relies on load_steps. I don't know how much of a priority/concern that would be for the main engine.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

The engine could perform a pass over the file in which it would not attempt to fully parse any resources. Instead, it would convert each "load step" it found into an intermediate format (perhaps a dictionary, etc) and add that to a vector. Once it reaches the end of the file, the length of the vector would be the value of load_steps. The file could then be closed, and the engine could iterate over the items in the vector to perform the actual loading.

Warning

I do recognize that this means the engine would be iterating 2 * load_steps times, instead of load_steps as it is right now. This doesn't seem great on paper, but I don't know if it would end up being significant in practice (i.e., if would users be able to feel it taking so much longer).

My hope is that both passes (the text-reading pass, and then the resource-loading pass) would each be more lightweight than the current read-text-and-load-resource single pass, so them together would be closer to what we have now than what it would seem like at first.

But I thought this was a good concern with my proposed approach that was worth bringing up.

If this enhancement will not be used often, can it be worked around with a few lines of script?

No

Is there a reason why this should be core and not an add-on in the asset library?

It's part of the engine's loading system and formats itself.

@dsnopek
Copy link

dsnopek commented Feb 18, 2025

Overall, Godot's tscn/tres format is robust to Git conflicts and merging issues. However, load_steps is a frequent source of Git conflicts, even when Git is capable of successfully merging the rest automatically. This is something that comes up for me every time I'm working on a project with multiple people.

So, I would personally be in support of removing it!

@Calinou
Copy link
Member

Calinou commented Feb 18, 2025

I also agree with removing it personally, even if it means loading progress bars may be less accurate in certain scenarios. Saving progress bars in the editor shouldn't be affected by this.

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

3 participants